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

[Tim Deegan <tim@xxxxxxx>]

At 12:31 +0000 on 01 Dec (1417433464), 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.

It calls handle_mmio() to emulate the instruction for it, but
handle_mmio() ought not to send anything to the DM.  hvmemul_write()
will call hvm_copy(), which will drop the write and report success.

The shadow code has its own emulator framework for emulting PT writes,
which does much the same thing.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel