Re: [PATCH] xenbus: Allow PVH dom0 a non-local xenstore

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-05-05 16:44, Stefano Stabellini wrote:
> On Sat, 3 May 2025, Jason Andryuk wrote:
> > Make xenbus_init() allow a non-local xenstore for a PVH dom0 - it is
> > currently forced to XS_LOCAL.  With Hyperlaunch booting dom0 and a
> > xenstore stubdom, dom0 can be handled as a regular XS_HVM following the
> > late init path.
> >
> > Drop the use of xen_initial_domain() and just check for the event
> > channel instead.  This matches the PV case where there is no check for
> > initial domain.
> >
> > Check the full 64bit HVM_PARAM_STORE_EVTCHN value to catch the off
> > chance that high bits are set for the 32bit event channel.
> >
> > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Thanks, Stefano.  But I'm wondering if this might break ARM enhanced 
no-xenstore.
> > ---
> >   drivers/xen/xenbus/xenbus_probe.c | 14 ++++++++------
> >   1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/xen/xenbus/xenbus_probe.c 
> > b/drivers/xen/xenbus/xenbus_probe.c
> > index 6d32ffb01136..7604f70ee108 100644
> > --- a/drivers/xen/xenbus/xenbus_probe.c
> > +++ b/drivers/xen/xenbus/xenbus_probe.c
> > @@ -966,9 +966,15 @@ static int __init xenbus_init(void)
> >         if (xen_pv_domain())
> >                 xen_store_domain_type = XS_PV;
> >         if (xen_hvm_domain())
> > +       {
> >                 xen_store_domain_type = XS_HVM;
ARM would have everything set to XS_HVM...

> > -       if (xen_hvm_domain() && xen_initial_domain())
> > -               xen_store_domain_type = XS_LOCAL;
...and only dom0 set to XS_LOCAL.

> > +               err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
> > +               if (err)
> > +                       goto out_error;
> > +               xen_store_evtchn = (int)v;
> > +               if (!v)
> > +                       xen_store_domain_type = XS_LOCAL;
> > +       }
> >         if (xen_pv_domain() && !xen_start_info->store_evtchn)
> >                 xen_store_domain_type = XS_LOCAL;
> >         if (xen_pv_domain() && xen_start_info->store_evtchn)
> > @@ -987,10 +993,6 @@ static int __init xenbus_init(void)
> >                 xen_store_interface = gfn_to_virt(xen_store_gfn);
> >                 break;
> >         case XS_HVM:
> > -               err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
> > -               if (err)
> > -                       goto out_error;
> > -               xen_store_evtchn = (int)v;
> >                 err = hvm_get_parameter(HVM_PARAM_STORE_PFN, &v);
> >                 if (err)
> >                         goto out_error;
                /*
                 * Uninitialized hvm_params are zero and return no error.
                 * Although it is theoretically possible to have
                 * HVM_PARAM_STORE_PFN set to zero on purpose, in 
reality it is
                 * not zero when valid. If zero, it means that Xenstore 
hasn't
                 * been properly initialized. Instead of attempting to 
map a
                 * wrong guest physical address return error.
                 *
                 * Also recognize all bits set as an 
invalid/uninitialized value.
                 */
                if (!v) {
                        err = -ENOENT;
                        goto out_error;
                }

IIUC, this !v check is for enhanced no-xenstore to end up in XS_UNKNOWN. 
 I'll have to re-work to handle that case.
Regards,
Jason


> > --
> > 2.34.1