Re: [PATCH] x86: XENMAPSPACE_gmfn{,_batch,_range} want to special case idx == gpfn

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.10.2020 11:39, Roger Pau Monné wrote:
> On Fri, Oct 16, 2020 at 08:44:10AM +0200, Jan Beulich wrote:
>> --- a/xen/arch/x86/mm.c
>> +++ b/xen/arch/x86/mm.c
>> @@ -4555,7 +4555,7 @@ int xenmem_add_to_physmap_one(
>>          if ( is_special_page(mfn_to_page(prev_mfn)) )
>>              /* Special pages are simply unhooked from this phys slot. */
>>              rc = guest_physmap_remove_page(d, gpfn, prev_mfn, 
>> PAGE_ORDER_4K);
>> -        else
>> +        else if ( !mfn_eq(mfn, prev_mfn) )
>>              /* Normal domain memory is freed, to avoid leaking memory. */
>>              rc = guest_remove_page(d, gfn_x(gpfn));
> 
> What about the access differing between the old and the new entries,
> while pointing to the same mfn, would Xen install the new entry
> successfully?

Yes - guest_physmap_add_page() doesn't get bypassed.

> Seems easier to me to use guest_physmap_remove_page in that case to
> remove the entry from the p2m without freeing the page.

Why do any removal when none is really needed? I also don't see
this fit the "special pages" clause and comment very well. I'd
question the other way around whether guest_physmap_remove_page()
needs calling at all (the instance above; the other one of course
is needed).

Jan