On 04.02.2026 13:25, 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.
> 
> 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.")

The tag is here because there is the (largely theoretical?) possibility for
a system to have no memory at all left below 4Gb, in which case creation of
a PV64 or non-shadow HVM guest would needlessly fail?

> Signed-off-by: Roger Pau Monné <[email protected]>

Reviewed-by: Jan Beulich <[email protected]>
with two nits:

> --- 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. */
> +    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);
> +        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. */

"references to ... page are dropped" reads as if talk was of page references.
vcpu_info_reset() writes out pointers only, though. May I suggest

        /* Ensure all pointers to the old shared_info page are replaced. */

?

> +        for_each_vcpu( d, v )

A common oddity, where my personal take is that only one of two forms are
possible (without ./CODING_STYLE saying anything explicit): Either such a
for_each_... is deemed a (kind-of) keyword (like "for"), then it wants to be

        for_each_vcpu ( d, v )

or it is deemed not to be one, then it wants to be

        for_each_vcpu(d, v)

Jan

Reply via email to