On Wed, Feb 04, 2026 at 03:06:52PM +0100, Jan Beulich wrote:
> 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?
It's kid of an issue we discovered when using strict domain NUMA node
placement. At that point the toolstack would exhaust all memory on
node 0 and by doing that inadvertently consume all memory below 4G.
> > 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. */
Fine with me.
> ?
>
> > + 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 )
Oh right, I copied it from the instance below and didn't notice the
missing spaces.
Thanks, Roger.
