Re: [Xen-devel] [PATCH v4] x86: add p2m_mmio_write_dm

["Yu, Zhang" <yu.c.zhang@xxxxxxxxxxxxxxx>]

On 12/1/2014 8:31 PM, Jan Beulich wrote:
> > > > On 01.12.14 at 13:13, <tim@xxxxxxx> wrote:
> > At 11:17 +0000 on 01 Dec (1417429027), Jan Beulich wrote:
> > > > > > On 01.12.14 at 11:30, <tim@xxxxxxx> wrote:
> > > > At 09:32 +0000 on 01 Dec (1417422746), Jan Beulich wrote:
> > > > > > > > On 01.12.14 at 09:49, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote:
> > > > > > To my understanding, pages with p2m_ram_ro are not supposed to be
> > > > > > modified by guest. So in __hvm_copy(), when p2m type of a page is
> > > > > > p2m_ram_rom, no copy will occur.
> > > > > > However, to our usage, we just wanna this page to be write protected, so
> > > > > > that our device model can be triggered to do some emulation. The content
> > > > > > written to this page is supposed not to be dropped. This way, if
> > > > > > sequentially a read operation is performed by guest to this page, the
> > > > > > guest will still see its anticipated value.
> > > > > __hvm_copy() is only a helper function, and doesn't write to
> > > > > mmio_dm space either; instead its (indirect) callers would invoke
> > > > > hvmemul_do_mmio() upon seeing HVMCOPY_bad_gfn_to_mfn
> > > > > returns. The question hence is about the apparent inconsistency
> > > > > resulting from writes to ram_ro being dropped here but getting
> > > > > passed to the DM by hvm_hap_nested_page_fault(). Tim - is
> > > > > that really intentional?
> > > > No - and AFAICT it shouldn't be happening.  It _is_ how it was
> > > > implemented originally, because it involved fewer moving parts and
> > > > didn't need to be efficient (and after all, writes to entirely missing
> > > > addresses go to the device model too).
> > > >
> > > > But the code was later updated to log and discard writes to read-only
> > > > memory (see 4d8aa29 from Trolle Selander).
> > > >
> > > > Early version of p2m_ram_ro were documented in the internal headers as
> > > > sending the writes to the DM, but the public interface (HVMMEM_ram_ro)
> > > > has always said that writes are discarded.
> > > Hmm, so which way do you recommend resolving the inconsistency
> > > then - match what the public interface says or what the apparent
> > > original intention for the internal type was? Presumably we need to
> > > follow the public interface mandated model, and hence require the
> > > new type to be introduced.
> > Sorry, I was unclear -- there isn't an inconsistency; both internal
> > and public headers currently say that writes are discarded and AFAICT
> > that is what the code does.
> Not for hvm_hap_nested_page_fault() afaict - the forwarding to
> DM there contradicts the "writes are discarded" model that other
> code paths follow.
Thanks, Jan.
By "inconsistency", do you mean the p2m_ram_ro shall not trigger the 
handle_mmio_with_translation() in hvm_hap_nested_page_fault()?
I'm also a bit confused with the "writes are discarded/dropped" comments 
in the code. Does this mean writes to the p2m_ram_ro pages should be 
abandoned without going to the dm, or going to the dm and  ignored 
later? The code seems to be the second one.
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel