[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4] x86: add p2m_mmio_write_dm
>>> On 01.12.14 at 09:49, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote: > On 11/28/2014 5:57 PM, Jan Beulich wrote: >>>>> On 28.11.14 at 08:59, <yu.c.zhang@xxxxxxxxxxxxxxx> wrote: >>> --- a/xen/arch/x86/hvm/hvm.c >>> +++ b/xen/arch/x86/hvm/hvm.c >>> @@ -2838,7 +2838,8 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned >>> long gla, >>> * to the mmio handler. >>> */ >>> if ( (p2mt == p2m_mmio_dm) || >>> - (npfec.write_access && (p2mt == p2m_ram_ro)) ) >>> + (npfec.write_access && >>> + ((p2mt == p2m_ram_ro) || (p2mt == p2m_mmio_write_dm))) ) >> >> Why are you corrupting indentation here? > Thanks for your comments, Jan. > The indentation here is to make sure the > ((p2mt == p2m_ram_ro) || (p2mt == p2m_mmio_write_dm)) are grouped > together. But I am not sure if this is correct according to xen coding > style. I may have misunderstood your previous comments on Sep 3, which > said "the indentation would need adjustment" in reply to "[Xen-devel] > [PATCH v3 1/2] x86: add p2m_mmio_write_dm" Getting the alignment right is needed, but no via using hard tabs. >> Furthermore the code you modify here suggests that p2m_ram_ro >> already has the needed semantics - writes get passed to the DM. >> None of the other changes you make, and none of the other uses >> of p2m_ram_ro appear to be in conflict with your intentions, so >> you'd really need to explain better why you need the new type. >> > Thanks Jan. > 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? > Maybe I need to update my commit message to explain this more clearly. :) At the very least, yes. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |