Re: [PATCH v1 3/3] xen/arm: fix mfn_to_gfn() usage in memory_exchange()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.03.2026 08:50, Penny Zheng wrote:
> On ARM, the only callsite of mfn_to_gfn() is in memory_exchange(). Although
> memory_exchange() is actually not reachable on ARM because steal_page() always
> returns -EOPNOTSUPP. We still need to fix it here to ensure correctness once
> steal_page() is implemented for ARM in the future, and avoid propagating
> broken usage.

steal_page() may need making functional on Arm without the need to permit
memory_exchange(). That would allow a different approach to the problem.

> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -814,6 +814,13 @@ static long 
> memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
>                      goto fail;
>                  }
>  
> +#ifndef CONFIG_X86
> +                /*
> +                 * Record the GFN in the page's type_info,
> +                 * so that we can retrieve it later when consuming the page.
> +                 */
> +                page_set_gfn(page, _gfn(gmfn + k));
> +#endif
>                  page_list_add(page, &in_chunk_list);
>  #ifdef CONFIG_X86
>                  put_gfn(d, gmfn + k);
> @@ -849,7 +856,11 @@ static long 
> memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
>              gfn_t gfn;
>  
>              mfn = page_to_mfn(page);
> +#ifdef CONFIG_X86
>              gfn = mfn_to_gfn(d, mfn);
> +#else
> +            gfn = page_get_gfn(page);
> +#endif
>              /* Pages were unshared above */
>              BUG_ON(SHARED_M2P(gfn_x(gfn)));
>              if ( guest_physmap_remove_page(d, gfn, mfn, 0) )

I pretty strongly object to further CONFIG_<arch> uses in common code. We should
reduce their amount, not increase it. Why can't page_set_gfn() be a no-op on 
x86,
while page_get_gfn() would resolve to mfn_to_gfn()?

The #ifdef-ary here will, btw, likely need reducing if you want to make
steal_page() and memory_exchange() work on Arm: Like x86, a GFN reference would
then presumably need holding across the steal_page() invocation.

After you have fetched the GFN, I think you want to invalidate the field again.
Otherwise, after the subsequent guest_physmap_add_page(), the stored value will
likely be stale.

Jan