Re: [PATCH v5 3/9] xen/arm: Alloc XenStore page for Dom0less DomUs from hypervisor

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 6 Feb 2025, Orzel, Michal wrote:
> On 06/02/2025 02:08, Stefano Stabellini wrote:
> > From: Henry Wang <xin.wang2@xxxxxxx>
> > 
> > There are use cases (for example using the PV driver) in Dom0less
> > setup that require Dom0less DomUs start immediately with Dom0, but
> > initialize XenStore later after Dom0's successful boot and call to
> > the init-dom0less application.
> > 
> > An error message can seen from the init-dom0less application on
> > 1:1 direct-mapped domains:
> > ```
> > Allocating magic pages
> > memory.c:238:d0v0 mfn 0x39000 doesn't belong to d1
> > Error on alloc magic pages
> > ```
> > 
> > The "magic page" is a terminology used in the toolstack as reserved
> > pages for the VM to have access to virtual platform capabilities.
> > Currently the magic pages for Dom0less DomUs are populated by the
> > init-dom0less app through populate_physmap(), and populate_physmap()
> > automatically assumes gfn == mfn for 1:1 direct mapped domains. This
> > cannot be true for the magic pages that are allocated later from the
> > init-dom0less application executed in Dom0. For domain using statically
> > allocated memory but not 1:1 direct-mapped, similar error "failed to
> > retrieve a reserved page" can be seen as the reserved memory list is
> > empty at that time.
> > 
> > Since for init-dom0less, the magic page region is only for XenStore.
> > To solve above issue, this commit allocates the XenStore page for
> > Dom0less DomUs at the domain construction time. The PFN will be
> > noted and communicated to the init-dom0less application executed
> > from Dom0. To keep the XenStore late init protocol, set the connection
> > status to XENSTORE_RECONNECT.
> > 
> > Reported-by: Alec Kwapis <alec.kwapis@xxxxxxxxxxxxx>
> > Suggested-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Henry Wang <xin.wang2@xxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> > ---
> >  xen/arch/arm/dom0less-build.c | 55 ++++++++++++++++++++++++++++++++++-
> >  1 file changed, 54 insertions(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
> > index 49d1f14d65..046439eb87 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -1,5 +1,6 @@
> >  /* SPDX-License-Identifier: GPL-2.0-only */
> >  #include <xen/device_tree.h>
> > +#include <xen/domain_page.h>
> >  #include <xen/err.h>
> >  #include <xen/event.h>
> >  #include <xen/grant_table.h>
> > @@ -11,6 +12,8 @@
> >  #include <xen/sizes.h>
> >  #include <xen/vmap.h>
> >  
> > +#include <public/io/xs_wire.h>
> > +
> >  #include <asm/arm64/sve.h>
> >  #include <asm/dom0less-build.h>
> >  #include <asm/domain_build.h>
> > @@ -704,6 +707,53 @@ static int __init alloc_xenstore_evtchn(struct domain 
> > *d)
> >      return 0;
> >  }
> >  
> > +#define XENSTORE_PFN_OFFSET 1
> > +static int __init alloc_xenstore_page(struct domain *d)
> > +{
> > +    struct page_info *xenstore_pg;
> > +    struct xenstore_domain_interface *interface;
> > +    mfn_t mfn;
> > +    gfn_t gfn;
> > +    int rc;
> > +
> > +    if ( (UINT_MAX - d->max_pages) < 1 )
> > +    {
> > +        printk(XENLOG_ERR "%pd: Over-allocation for d->max_pages by 1 
> > page.\n",
> > +               d);
> > +        return -EINVAL;
> > +    }
> empty line here

Sure


> > +    d->max_pages += 1;
> If this patch is separate from modifying init-dom0less, max_pages will be 
> bumped twice. Here and in init-dom0less.
> Shouldn't we fold it in? The rest is ok.

OK