Re: [PATCH v1 1/3] xen/arm: generalize per-page GFN storage beyond xenheap pages

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.03.2026 08:50, Penny Zheng wrote:
> As preparation for fixing mfn_to_gfn() on ARM, we extend the existing GFN
> field in page_info's type_info to be usable for not only xenheap ones.
> Another usage will be introduced later for stolen pages in memory exchaging.
> 
> Introduce general-purpose page_get_gfn() and page_set_gfn() helpers
> that read and write the GFN stored in type_info. The old
> page_get_xenheap_gfn() and page_set_xenheap_gfn() are retained as thin
> wrappers with their xenheap ASSERTs, so all current callers remain unchanged.

Why was this GFN setting limited to Xenheap pages back at the time? Depending
on the reasons, retaining the old accessors may or may not be a good idea.

> Also introduce PGT_INVALID_GFN as the general sentinel, with
> PGT_INVALID_XENHEAP_GFN aliased to it for backward compatibility.

This I view as unnecessary, if not confusing.

> --- a/xen/arch/arm/include/asm/mm.h
> +++ b/xen/arch/arm/include/asm/mm.h
> @@ -113,18 +113,21 @@ struct page_info
>  #define PGT_count_mask    PG_mask(3, 3)
>  
>  /*
> - * Stored in bits [28:0] (arm32) or [60:0] (arm64) GFN if page is xenheap 
> page.
> + * Stored in bits [28:0] (arm32) or [60:0] (arm64) GFN if page is xenheap 
> page,
> + * or stolen ones in memory exchanging.
>   */

Does the purpose really need limiting like this? If the field covered by 
PGT_gfn_*
is uniformly available (see the question above), I don't see why a new 
constraint
would need spelling out. If it's not uniformly available, then likely the
description needs expanding as to when the new accessors are okay to use. If
uniformly available, what may want spelling out is under what conditions one can
expect the field to be properly set (until such time where it's set correctly on
all guest-owned pages).

Jan