>>> On 08.05.19 at 17:08, <george.dun...@citrix.com> wrote: > On 5/2/19 7:58 AM, Jan Beulich wrote: >> --- a/xen/arch/x86/mm/p2m.c >> +++ b/xen/arch/x86/mm/p2m.c >> @@ -841,15 +841,19 @@ guest_physmap_add_entry(struct domain *d >> * any guest-requested type changes succeed and remove the IOMMU >> * entry). >> */ >> - if ( !need_iommu_pt_sync(d) || t != p2m_ram_rw ) >> + if ( t != p2m_ram_rw ) >> return 0; > > So, you seem to be claiming that the only way to get here is via > guest_physmap_add_page(),
Well, I'm not "claiming" anything here, I'm just modifying existing code (and no more than what fits under this patch's title). > which will always call this function with > p2m_ram_rw. So then what's the point of this conditional at all > anymore? Would it be better to add an ASSERT(t == p2m_ram_rw) here? > > And if we ever *do* get here with t == p2m_ram_rw, do we really not want > to call set_gpfn_from_mfn()? Thinking about e.g. p2m_grant_map_* I wouldn't want to add the suggested ASSERT(), and the M2P doesn't want updating in that case either. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
