Re: [PATCH 08/23] xen/arm: dom0less seed xenstore grant table entry

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-03-10 05:32, Julien Grall wrote:
> Hi Jason,
>
> On 06/03/2025 22:03, Jason Andryuk wrote:
> > With a split hardware and control domain, the control domain may still
> > want and xenstore access.  Currently this relies on init-dom0less to
> > seed the grants.  This is problematic since we don't want hardware
> > domain to be able to map the control domain's resources.  Instead have
> > the hypervisor see the grant table entry.  The grant is then accessible
> > as normal.
> >
> > This is also useful with a xenstore stubdom to setup the xenbus page
> > much earlier.
> >
> > This works with C xenstored.  OCaml xenstored does not use grants and
> > would fail to foreign map the page.
> >
> > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > ---
> >   xen/arch/arm/dom0less-build.c |  9 +++++++++
> >   xen/common/grant_table.c      | 10 ++++++++++
> >   xen/include/xen/grant_table.h |  8 ++++++++
> >   3 files changed, 27 insertions(+)
> >
> > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less- 
> > build.c
> > index 068bf99294..f1d5bbb097 100644
> > --- a/xen/arch/arm/dom0less-build.c
> > +++ b/xen/arch/arm/dom0less-build.c
> > @@ -21,6 +21,8 @@
> >   #include <asm/static-memory.h>
> >   #include <asm/static-shmem.h>
> > +static domid_t __initdata xs_domid = DOMID_INVALID;
> > +
> >   bool __init is_dom0less_mode(void)
> >   {
> >       struct bootmodules *mods = &bootinfo.modules;
> > @@ -753,6 +755,10 @@ static int __init alloc_xenstore_page(struct 
> > domain *d)
> >       interface->connection = XENSTORE_RECONNECT;
> >       unmap_domain_page(interface);
> > +    if ( xs_domid != DOMID_INVALID )
> Looking at this patch again, is this guarantee that the xenstore domain 
> will be created first? If not, then I think your series needs to be re- 
> ordered so patch #10 is before this patch.
Yes, you are right.

> > +        gnttab_seed_entry(d, GNTTAB_RESERVED_XENSTORE, xs_domid,
> > +                          gfn_x(gfn), GTF_permit_access);
> > +
> >       return 0;
> >   }
> > @@ -1173,6 +1179,9 @@ void __init create_domUs(void)
> >           if ( rc )
> >               panic("Could not set up domain %s (rc = %d)\n",
> >                     dt_node_name(node), rc);
> > +
> > +        if ( d_cfg.flags & XEN_DOMCTL_CDF_xs_domain )
> > +            xs_domid = d->domain_id;
> What if there is multiple domain with XEN_DOMCTL_CDF_xs_domain? Should 
> we throw an error?
Before this series, there was no restriction on its use, but only 
init-xenstore-domain used it.  The XEN_DOMCTL_CDF_xs_domain flag allows 
the domain XSM_XS_PRIV, which grants a few more operations to an 
otherwise unprivileged domU
With the use of xs_domid (only during construction), maybe it should be 
limited to just 1 to avoid surprises.  Otherwise the last built xenstore 
domain would be configured as the backend.  Nothing would break - it 
would just be surprising.  I'll restrict to just 1.
Thanks,
Jason