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.

Fixing it with the following steps:
- Recording the GFN into the page's type_info via page_set_gfn() at stolen time
- Retrieving it via page_get_gfn() at consume time instead of mfn_to_gfn().

With the only call site removed in ARM, mfn_to_gfn() macro is no longer
referenced and could be removed in ARM.

Signed-off-by: Penny Zheng <penny.zheng@amd.com>
---
 xen/arch/arm/include/asm/mm.h |  1 -
 xen/common/memory.c           | 11 +++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/include/asm/mm.h b/xen/arch/arm/include/asm/mm.h
index 8d61b74e4f..14cb3c3dfa 100644
--- a/xen/arch/arm/include/asm/mm.h
+++ b/xen/arch/arm/include/asm/mm.h
@@ -307,7 +307,6 @@ struct page_info *get_page_from_gva(struct vcpu *v, vaddr_t va,
 
 /* Xen always owns P2M on ARM */
 #define set_gpfn_from_mfn(mfn, pfn) do { (void) (mfn), (void)(pfn); } while (0)
-#define mfn_to_gfn(d, mfn) ((void)(d), _gfn(mfn_x(mfn)))
 #define shared_info_to_gfn(d) \
     page_get_xenheap_gfn(virt_to_page((d)->shared_info))
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index 918510f287..5c462c302d 100644
--- 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) )
-- 
2.43.0

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