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é <[email protected]>
> ---
> 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
