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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

Jan


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