Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
On Wed, 6 Apr 2022, Julien Grall wrote:
> But if we use the 'connection status' field, then you could just delay the
> initialization until you receive an event and the connection status is
> connected.
I prototyped this approach and I managed to validate it successfully.
See attached patches for xen and linux (on top of existing patches). I
allocated the page from init-dom0less (instead of Xen) to achieve best
compatibility with older kernels.
There are some rough edges in the two patches but let me know if you
have any comments on the overall approach.diff --git a/tools/helpers/Makefile b/tools/helpers/Makefile
index 8e42997052..8d78ab1e90 100644
--- a/tools/helpers/Makefile
+++ b/tools/helpers/Makefile
@@ -46,7 +46,7 @@ init-xenstore-domain: $(INIT_XENSTORE_DOMAIN_OBJS)
$(CC) $(LDFLAGS) -o $@ $(INIT_XENSTORE_DOMAIN_OBJS) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenctrl) $(LDLIBS_libxenguest) $(LDLIBS_libxenlight) $(APPEND_LDFLAGS)
init-dom0less: $(INIT_DOM0LESS_OBJS)
- $(CC) $(LDFLAGS) -o $@ $(INIT_DOM0LESS_OBJS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenevtchn) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenlight) $(LDLIBS_libxenguest) $(APPEND_LDFLAGS)
+ $(CC) $(LDFLAGS) -o $@ $(INIT_DOM0LESS_OBJS) $(LDLIBS_libxenctrl) $(LDLIBS_libxenevtchn) $(LDLIBS_libxentoollog) $(LDLIBS_libxenstore) $(LDLIBS_libxenlight) $(LDLIBS_libxenguest) $(LDLIBS_libxenforeignmemory) $(APPEND_LDFLAGS)
.PHONY: install
install: all
diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
index dc9ccee868..329aa44da6 100644
--- a/tools/helpers/init-dom0less.c
+++ b/tools/helpers/init-dom0less.c
@@ -9,9 +9,12 @@
#include
#include
#include
+#include
#include "init-dom-json.h"
+#define XS_CONNECTION_STATE_OFFSET (2068/4)
+#define XS_CONNECTION_STATE_RECONNECTING 0x1
#define XENSTORE_PFN_OFFSET 1
#define STR_MAX_LENGTH 64
@@ -215,12 +218,18 @@ err:
static int init_domain(struct xs_handle *xsh, libxl_dominfo *info)
{
struct xc_interface_core *xch;
+xenforeignmemory_handle *xfh;
libxl_uuid uuid;
uint64_t xenstore_evtchn, xenstore_pfn;
int rc;
+uint32_t *page;
printf("Init dom0less domain: %u\n", info->domid);
xch = xc_interface_open(0, 0, 0);
+xfh = xenforeignmemory_open(0, 0);
+
+if (xch == NULL || xfh == NULL)
+err(1, "Cannot open xc/xenforeignmemory interfaces\n");
rc = xc_hvm_param_get(xch, info->domid, HVM_PARAM_STORE_EVTCHN,
_evtchn);
@@ -235,6 +244,14 @@ static int init_domain(struct xs_handle *xsh, libxl_dominfo *info)
return 1;
}
+page = xenforeignmemory_map(xfh, info->domid, XS_READ | XS_WRITE, 1, _pfn, NULL);
+if (!page) {
+printf("Error mapping xenstore page\n");
+return 1;
+}
+page[XS_CONNECTION_STATE_OFFSET] = XS_CONNECTION_STATE_RECONNECTING;
+xenforeignmemory_unmap(xfh, page, 1);
+
rc = xc_dom_gnttab_seed(xch, info->domid, true,
(xen_pfn_t)-1, xenstore_pfn, 0, 0);
if (rc)
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 0543f49670..7bb8c64d33 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -494,6 +494,7 @@ static struct domain *introduce_domain(const void *ctx,
talloc_steal(domain->conn, domain);
/* Notify the domain that xenstore is available */
+ interface->connection = 0x0;
xenevtchn_notify(xce_handle, domain->port);
if (!is_master_domain && !restore)
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 51e52e175892..dc046d25789e 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -939,6 +939,7 @@ static int __init xenbus_init(void)
{
int err;
uint64_t v = 0;
+ bool wait = false;
xen_store_domain_type = XS_UNKNOWN;
if (!xen_domain())
@@ -992,17 +993,7 @@ static int __init xenbus_init(void)
goto out_error;
}
if (v == ~0ULL) {
- err = bind_evtchn_to_irqhandler(xen_store_evtchn,
- xenbus_late_init,
- 0, "xenstore_late_init",
- _waitq);
- if (err < 0) {
-pr_err("xenstore_late_init couldn't bind irq err=%d\n",
- err);
-return err;
- }
-
- xs_init_irq = err;
+ wait = true;
} else {
/* Avoid truncation on 32-bit. */
#if BITS_PER_LONG == 32
@@ -1017,6 +1008,21 @@ static int __init xenbus_init(void)
xen_store_interface =
xen_remap(xen_store_gfn << XEN_PAGE_SHIFT,
XEN_PAGE_SIZE);
+ if (xen_store_interface->connection != 0)
+wait = true;
+ }
+ if (wait) {
+ err = bind_evtchn_to_irqhandler(xen_store_evtchn,
+ xenbus_late_init,
+ 0, "xenstore_late_init",
+ _waitq);
+ if (err < 0) {
+pr_err("xenstore_late_init couldn't bind irq err=%d\n",
+ err);
+return err;
+ }
+
+ xs_init_irq = err;
}
break;
default:
diff --git a/include/xen/interface/io/xs_wire.h
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
Hi Stefano, On 06/04/2022 03:21, Stefano Stabellini wrote: On Fri, 1 Apr 2022, Juergen Gross wrote: On 01.04.22 12:21, Julien Grall wrote: 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. I looked at the implementation of xb_init_comms in 5.17 and I couldn't find out how it would block here. Can you clarify? 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. From my understanding, Linux may send commands as soon as it sees HVM_PARAM_STORE_PFN. With your proposal, this may happen before xs_introduce_domain() has updated the features supported by Xenstored. With the recent proposal from Juergen [1], an OS will need to read the features field to understand whether Xenstored support the extended version of a command. This means, any commands sent before xs_introduce_domain() would not be able to take advantage of the new features. Therefore, we would need to wait until Xenstored has advertised the features. With your approach, the wait would be a busy loop. Although, I am not entirely sure what you would be waiting on? But if we use the 'connection status' field, then you could just delay the initialization until you receive an event and the connection status is connected. Cheers, [1] https://lore.kernel.org/xen-devel/20220316161017.3579-1-jgr...@suse.com/ -- Julien Grall
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
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,
> > > + _evtchn);
> > > + if (rc != 0) {
> > > + printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
> > > + return 1;
> > > + }
> > > +
> > > + /* Alloc xenstore page */
> > > + if (alloc_xs_page(xch, info, _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();
> > > + xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray());
> > > +
> > > + rc = gen_stub_json_config(info->domid, );
> > > + 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?
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
On 01.04.22 16:04, Julien Grall wrote:
Hi Juergen,
On 01/04/2022 14:52, Juergen Gross wrote:
On 01.04.22 15:35, Julien Grall wrote:
Hi Juergen,
On 01/04/2022 11:46, 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,
+ _evtchn);
+ if (rc != 0) {
+ printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
+ return 1;
+ }
+
+ /* Alloc xenstore page */
+ if (alloc_xs_page(xch, info, _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();
+ xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray());
+
+ rc = gen_stub_json_config(info->domid, );
+ 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.
And we don't expect the guest to use the grant entry to find the xenstore page?
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.
IIUC, this would mean the guest would need to do some sort of busy loop until
the xenstore page to appear.
Looking for it every second or so would be enough.
This is a better than a busy loop but not by much. I would argue a design that
requires to poll after receiving an interrupt is broken.
If so, this doesn't sound great to me. I think it would be better to have a
flag in the page to indicate whether the page is not ready.
Xenstored could then clear the flag before raising the event channel.
Hmm, the "connection" field could be used for that.
I thought about the field but the description doesn't entirely match what we
want. In particular, the spec says only the guest should set the value to 1
(i.e. reconnect). Maybe this could be relaxed?
It would mean, though, that e.g. libxl would need to initialize the
page accordingly before calling xs_introduce()
libxl only create domain paused. So I don't think it would be necessary to
update it.
Maybe not libxl, but whoever is calling xc_dom_gnttab_seed(),
xc_hvm_param_set() and/or xs_introduce() needs to set the field, in
order to have an effect of Xenstore resetting it.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
Hi Juergen,
On 01/04/2022 14:52, Juergen Gross wrote:
On 01.04.22 15:35, Julien Grall wrote:
Hi Juergen,
On 01/04/2022 11:46, 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,
+ _evtchn);
+ if (rc != 0) {
+ printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
+ return 1;
+ }
+
+ /* Alloc xenstore page */
+ if (alloc_xs_page(xch, info, _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();
+ xc_domain_sethandle(xch, info->domid,
libxl_uuid_bytearray());
+
+ rc = gen_stub_json_config(info->domid, );
+ 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.
And we don't expect the guest to use the grant entry to find the
xenstore page?
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.
IIUC, this would mean the guest would need to do some sort of busy
loop until the xenstore page to appear.
Looking for it every second or so would be enough.
This is a better than a busy loop but not by much. I would argue a
design that requires to poll after receiving an interrupt is broken.
If so, this doesn't sound great to me. I think it would be better to
have a flag in the page to indicate whether the page is not ready.
Xenstored could then clear the flag before raising the event channel.
Hmm, the "connection" field could be used for that.
I thought about the field but the description doesn't entirely match
what we want. In particular, the spec says only the guest should set the
value to 1 (i.e. reconnect). Maybe this could be relaxed?
It would mean, though, that e.g. libxl would need to initialize the
page accordingly before calling xs_introduce()
libxl only create domain paused. So I don't think it would be necessary
to update it.
Cheers,
--
Julien Grall
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
On 01.04.22 15:35, Julien Grall wrote:
Hi Juergen,
On 01/04/2022 11:46, 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,
+ _evtchn);
+ if (rc != 0) {
+ printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
+ return 1;
+ }
+
+ /* Alloc xenstore page */
+ if (alloc_xs_page(xch, info, _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();
+ xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray());
+
+ rc = gen_stub_json_config(info->domid, );
+ 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.
And we don't expect the guest to use the grant entry to find the xenstore page?
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.
IIUC, this would mean the guest would need to do some sort of busy loop until
the xenstore page to appear.
Looking for it every second or so would be enough.
If so, this doesn't sound great to me. I think it would be better to have a flag
in the page to indicate whether the page is not ready.
Xenstored could then clear the flag before raising the event channel.
Hmm, the "connection" field could be used for that.
It would mean, though, that e.g. libxl would need to initialize the
page accordingly before calling xs_introduce().
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
Hi Juergen,
On 01/04/2022 11:46, 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,
+ _evtchn);
+ if (rc != 0) {
+ printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
+ return 1;
+ }
+
+ /* Alloc xenstore page */
+ if (alloc_xs_page(xch, info, _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();
+ xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray());
+
+ rc = gen_stub_json_config(info->domid, );
+ 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.
And we don't expect the guest to use the grant entry to find the
xenstore page?
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.
IIUC, this would mean the guest would need to do some sort of busy loop
until the xenstore page to appear.
If so, this doesn't sound great to me. I think it would be better to
have a flag in the page to indicate whether the page is not ready.
Xenstored could then clear the flag before raising the event channel.
Cheers,
--
Julien Grall
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
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,
+ _evtchn);
+ if (rc != 0) {
+ printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
+ return 1;
+ }
+
+ /* Alloc xenstore page */
+ if (alloc_xs_page(xch, info, _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();
+ xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray());
+
+ rc = gen_stub_json_config(info->domid, );
+ 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.
Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature
Description: OpenPGP digital signature
Re: [PATCH v4 8/9] tools: add example application to initialize dom0less PV drivers
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,
+ _evtchn);
+if (rc != 0) {
+printf("Failed to get HVM_PARAM_STORE_EVTCHN\n");
+return 1;
+}
+
+/* Alloc xenstore page */
+if (alloc_xs_page(xch, info, _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();
+xc_domain_sethandle(xch, info->domid, libxl_uuid_bytearray());
+
+rc = gen_stub_json_config(info->domid, );
+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.
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?
Cheers,
--
Julien Grall
