Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 1 Apr 2022, Juergen Gross wrote:
> On 01.04.22 12:21, Julien Grall wrote:
> > Hi,
> > 
> > I have posted some comments in v3 after you sent this version. Please have a
> > look.
> > 
> > On 01/04/2022 01:38, Stefano Stabellini wrote:
> > > +static int init_domain(struct xs_handle *xsh, libxl_dominfo *info)
> > > +{
> > > +    struct xc_interface_core *xch;
> > > +    libxl_uuid uuid;
> > > +    uint64_t xenstore_evtchn, xenstore_pfn;
> > > +    int rc;
> > > +
> > > +    printf("Init dom0less domain: %u\n", info->domid);
> > > +    xch = xc_interface_open(0, 0, 0);
> > > +
> > > +    rc = xc_hvm_param_get(xch, info->domid, HVM_PARAM_STORE_EVTCHN,
> > > +                          &xenstore_evtchn);
> > > +    if (rc != 0) {
> > > +        printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
> > > +        return 1;
> > > +    }
> > > +
> > > +    /* Alloc xenstore page */
> > > +    if (alloc_xs_page(xch, info, &xenstore_pfn) != 0) {
> > > +        printf("Error on alloc magic pages\n");
> > > +        return 1;
> > > +    }
> > > +
> > > +    rc = xc_dom_gnttab_seed(xch, info->domid, true,
> > > +                            (xen_pfn_t)-1, xenstore_pfn, 0, 0);
> > > +    if (rc)
> > > +        err(1, "xc_dom_gnttab_seed");
> > > +
> > > +    libxl_uuid_generate(&uuid);
> > > +    xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray(&uuid));
> > > +
> > > +    rc = gen_stub_json_config(info->domid, &uuid);
> > > +    if (rc)
> > > +        err(1, "gen_stub_json_config");
> > > +
> > > +    /* Now everything is ready: set HVM_PARAM_STORE_PFN */
> > > +    rc = xc_hvm_param_set(xch, info->domid, HVM_PARAM_STORE_PFN,
> > > +                          xenstore_pfn);
> > 
> > On patch #1, you told me you didn't want to allocate the page in Xen because
> > it wouldn't be initialized by Xenstored. But this is what we are doing here.
> 
> Xenstore (at least the C variant) is only using the fixed grant ref
> GNTTAB_RESERVED_XENSTORE, so it doesn't need the page to be advertised
> to the guest. And the mapping is done only when the domain is being
> introduced to Xenstore.
> 
> > 
> > This would be a problem if Linux is still booting and hasn't yet call
> > xenbus_probe_initcall().
> > 
> > I understand we need to have the page setup before raising the event
> > channel. I don't think we can allow Xenstored to set the HVM_PARAM (it may
> > run in a domain with less privilege). So I think we may need to create a
> > separate command to kick the client (not great).
> > 
> > Juergen, any thoughts?
> 
> I think it should work like that:
> 
> - setup the grant via xc_dom_gnttab_seed()
> - introduce the domain to Xenstore
> - call xc_hvm_param_set()
> 
> When the guest is receiving the event, it should wait for the xenstore
> page to appear.


I am OK with what you wrote above, and I understand Julien's concerns
about "waiting". Before discussing that, I would like to make sure I
understood why setting HVM_PARAM_STORE_PFN first (before
xs_introduce_domain) is not possible.

In a previous reply to Julien I wrote that it is not a good idea to
set HVM_PARAM_STORE_PFN in Xen before creating the domains because it
would cause Linux to hang at boot. That is true, Linux hangs on
drivers/xen/xenbus/xenbus_comms.c:xb_init_comms waiting on xb_waitq.
It could wait a very long time as domUs are typically a lot faster than
dom0 to boot.

However, if we set HVM_PARAM_STORE_PFN before calling
xs_introduce_domain in init-dom0less, for Linux to see it before
xs_introduce_domain is done, Linux would need to be racing against
init-dom0less. In that case, the wait in xb_init_comms would be minimal
anyway. It shouldn't be a problem. There would be no "hang", just a wait
a bit longer than usual.

Is that right?