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

[Jan Beulich <jbeulich@xxxxxxxx>]

In this case up to now we've been freeing the page (through
guest_remove_page(), with the actual free typically happening at the
put_page() later in the function), but then failing the call on the
subsequent GFN consistency check. However, in my opinion such a request
should complete as an "expensive" no-op (leaving aside the potential
unsharing of the page).

This points out that f33d653f46f5 ("x86: replace bad ASSERT() in
xenmem_add_to_physmap_one()" would really have needed an XSA, despite
its description claiming otherwise, as in release builds we then put in
place a P2M entry referencing the about to be freed page.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
I've been considering to make such operations "cheap" NOPs rather than
"expensive" ones, by comparing idx and gpfn early in the function in
the XENMAPSPACE_gmfn case block, but I've come to the conclusion that
having the operation perform otherwise normally is better - this way,
errors that would result if idx != gpfn will still result. While I'm
open to reasons towards the other alternative, having the added check be
MFN-based makes crystal clear that we're dealing with the same
underlying physical resource, i.e. also covers the hypothetical(?) case
of two GFNs referring to the same MFN.

I'm unconvinced that it is correct for prev_mfn's p2mt to not be
inspected at all - I don't think things will go right if p2m_shared()
was true for it. But I'm afraid I'm not up to correcting mem-sharing
related logic.

--- 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));
     }