Re: [Xen-devel] [PATCH] x86/mm: subsume set_gpfn_from_mfn() into guest_physmap_add_entry()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.05.19 at 17:08, <george.dunlap@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel