Re: [PATCH v2] x86/domain: adjust limitation on shared_info allocation below 4G

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 04/02/2026 12:25 pm, Roger Pau Monne wrote:
> The limitation of shared_info being allocated below 4G to fit in the
> start_info field only applies to 32bit PV guests.  On 64bit PV guests the
> start_info field is 64bits wide.  HVM guests don't use start_info at all.

All shared info?  HVM does use it, but doesn't see the MFN.

>
> Drop the restriction in arch_domain_create() and instead free and
> re-allocate the page from memory below 4G if needed in switch_compat(),
> when the guest is set to run in 32bit PV mode.
>
> Fixes: 3cadc0469d5c ("x86_64: shared_info must be allocated below 4GB as it 
> is advertised to 32-bit guests via a 32-bit machine address field in 
> start_info.")
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/x86/domain.c    |  7 ++++---
>  xen/arch/x86/pv/domain.c | 20 ++++++++++++++++++++
>  xen/common/domain.c      |  2 +-
>  xen/include/xen/domain.h |  2 ++
>  4 files changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index edb76366b596..3e701f2146c9 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -882,10 +882,11 @@ int arch_domain_create(struct domain *d,
>          goto fail;
>  
>      /*
> -     * The shared_info machine address must fit in a 32-bit field within a
> -     * 32-bit guest's start_info structure. Hence we specify MEMF_bits(32).
> +     * For 32bit PV guests the shared_info machine address must fit in a 
> 32-bit
> +     * field within the guest's start_info structure.  We might need to free
> +     * and allocate later if the guest turns out to be a 32bit PV one.
>       */
> -    if ( (d->shared_info = alloc_xenheap_pages(0, MEMF_bits(32))) == NULL )
> +    if ( (d->shared_info = alloc_xenheap_page()) == NULL )
>          goto fail;
>  

The comment is now out of place when the source is read naturally.  I'd
suggest dropping the comment entirely, and ...

>      clear_page(d->shared_info);
> diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
> index 01499582d2d6..8ced3d70a52f 100644
> --- a/xen/arch/x86/pv/domain.c
> +++ b/xen/arch/x86/pv/domain.c
> @@ -247,6 +247,26 @@ int switch_compat(struct domain *d)
>      d->arch.has_32bit_shinfo = 1;
>      d->arch.pv.is_32bit = true;
>  
> +    /* Check whether the shared_info page needs to be moved below 4G. */

... extending this one talking about the 32bit field.

> +    if ( virt_to_maddr(d->shared_info) >> 32 )
> +    {
> +        shared_info_t *prev = d->shared_info;
> +
> +        d->shared_info = alloc_xenheap_pages(0, MEMF_bits(32));
> +        if ( !d->shared_info )
> +        {
> +            d->shared_info = prev;
> +            rc = -ENOMEM;
> +            goto undo_and_fail;
> +        }
> +        put_page(virt_to_page(prev));
> +        clear_page(d->shared_info);

I think copy_page() would be more appropriate.  That way there are fewer
implicit ordering dependencies.

> +        share_xen_page_with_guest(virt_to_page(d->shared_info), d, SHARE_rw);
> +        /* Ensure all references to the old shared_info page are dropped. */
> +        for_each_vcpu( d, v )
> +            vcpu_info_reset(v);

switch_compat() can only occur on a domain with no memory.  How can we
have outstanding references?

~Andrew