1. Patchew
  2. Xen
  3. [PATCH v6 0/7] dom0less PV drivers
  4. View patch

[PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain

Stefano Stabellini posted 7 patches 2 years, 6 months ago
There is a newer version of this series
[PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain
Posted by Stefano Stabellini 2 years, 6 months ago 
From: Luca Miccio <lucmiccio@gmail.com>

When xs_introduce_domain is called, send out a notification on the
xenstore event channel so that any (dom0less) domain waiting for the
xenstore interface to be ready can continue with the initialization.
Before sending the notification, clear XENSTORE_RECONNECTING.

The extra notification is harmless for domains that don't require it.

In xs_wire.h update the commment on top of XENSTORE_RECONNECTING to
generalize its meaning to suit the dom0less use-case better.

Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
CC: jgross@suse.com
CC: anthony.perard@citrix.com
CC: wl@xen.org
---
If you have better suggestions for the wording in xs_wire.h please
suggest!


Changes in v6:
- use XENSTORE_CONNECTED instead of 0x0
- update xs_wire.h

Changes in v5:
- reset XS_CONNECTION_STATE_RECONNECTING before notifying the domU

Changes in v2:
- drop the new late_init parameter
---
 tools/xenstore/xenstored_domain.c | 4 ++++
 xen/include/public/io/xs_wire.h   | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index ae065fcbee..6f34af225c 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -493,6 +493,10 @@ static struct domain *introduce_domain(const void *ctx,
 		/* Now domain belongs to its connection. */
 		talloc_steal(domain->conn, domain);
 
+		/* Notify the domain that xenstore is available */
+		interface->connection = XENSTORE_CONNECTED;
+		xenevtchn_notify(xce_handle, domain->port);
+
 		if (!is_master_domain && !restore)
 			fire_watches(NULL, ctx, "@introduceDomain", NULL,
 				     false, NULL);
diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
index 953a0050a3..c1ec7c73e3 100644
--- a/xen/include/public/io/xs_wire.h
+++ b/xen/include/public/io/xs_wire.h
@@ -141,7 +141,7 @@ struct xenstore_domain_interface {
 
 /* Valid values for the connection field */
 #define XENSTORE_CONNECTED 0 /* the steady-state */
-#define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */
+#define XENSTORE_RECONNECT 1 /* reconnect in progress */
 
 /* Valid values for the error field */
 #define XENSTORE_ERROR_NONE    0 /* No error */
-- 
2.25.1
Re: [PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain
Posted by Julien Grall 2 years, 6 months ago 
Hi,

On 05/05/2022 01:16, Stefano Stabellini wrote:
> From: Luca Miccio <lucmiccio@gmail.com>
> 
> When xs_introduce_domain is called, send out a notification on the
> xenstore event channel so that any (dom0less) domain waiting for the
> xenstore interface to be ready can continue with the initialization.
> Before sending the notification, clear XENSTORE_RECONNECTING.
> 
> The extra notification is harmless for domains that don't require it.
> 
> In xs_wire.h update the commment on top of XENSTORE_RECONNECTING to
> generalize its meaning to suit the dom0less use-case better.
> 
> Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> CC: jgross@suse.com
> CC: anthony.perard@citrix.com
> CC: wl@xen.org
> ---
> If you have better suggestions for the wording in xs_wire.h please
> suggest!
> 
> 
> Changes in v6:
> - use XENSTORE_CONNECTED instead of 0x0
> - update xs_wire.h
> 
> Changes in v5:
> - reset XS_CONNECTION_STATE_RECONNECTING before notifying the domU
> 
> Changes in v2:
> - drop the new late_init parameter
> ---
>   tools/xenstore/xenstored_domain.c | 4 ++++
>   xen/include/public/io/xs_wire.h   | 2 +-

I am not entirely sure this is the right place to mention it. But I 
couldn't find a better one.

The documentation (docs/misc/xenstore-misc.txt) states that the field is 
valid when the server advertised ``Connection State``.

Is there any guarantee the field will be 0 for any previous {C, 
O}xenstored implementation? If not, then I think we need to set the 
feature flag so Linux knows the field can be used.

If yes, then the documentation should be relaxed so an OS knows it can 
safely use the field without checking the feature flag.

>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
> index ae065fcbee..6f34af225c 100644
> --- a/tools/xenstore/xenstored_domain.c
> +++ b/tools/xenstore/xenstored_domain.c
> @@ -493,6 +493,10 @@ static struct domain *introduce_domain(const void *ctx,
>   		/* Now domain belongs to its connection. */
>   		talloc_steal(domain->conn, domain);
>   
> +		/* Notify the domain that xenstore is available */
> +		interface->connection = XENSTORE_CONNECTED;
> +		xenevtchn_notify(xce_handle, domain->port);
> +
>   		if (!is_master_domain && !restore)
>   			fire_watches(NULL, ctx, "@introduceDomain", NULL,
>   				     false, NULL);
> diff --git a/xen/include/public/io/xs_wire.h b/xen/include/public/io/xs_wire.h
> index 953a0050a3..c1ec7c73e3 100644
> --- a/xen/include/public/io/xs_wire.h
> +++ b/xen/include/public/io/xs_wire.h
> @@ -141,7 +141,7 @@ struct xenstore_domain_interface {
>   
>   /* Valid values for the connection field */
>   #define XENSTORE_CONNECTED 0 /* the steady-state */
> -#define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */
> +#define XENSTORE_RECONNECT 1 /* reconnect in progress */

The definition in the docs needs to be updated.

>   
>   /* Valid values for the error field */
>   #define XENSTORE_ERROR_NONE    0 /* No error */

Cheers,

-- 
Julien Grall
Re: [PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain
Posted by Stefano Stabellini 2 years, 6 months ago 
On Wed, 11 May 2022, Julien Grall wrote:
> On 05/05/2022 01:16, Stefano Stabellini wrote:
> > From: Luca Miccio <lucmiccio@gmail.com>
> > 
> > When xs_introduce_domain is called, send out a notification on the
> > xenstore event channel so that any (dom0less) domain waiting for the
> > xenstore interface to be ready can continue with the initialization.
> > Before sending the notification, clear XENSTORE_RECONNECTING.
> > 
> > The extra notification is harmless for domains that don't require it.
> > 
> > In xs_wire.h update the commment on top of XENSTORE_RECONNECTING to
> > generalize its meaning to suit the dom0less use-case better.
> > 
> > Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > CC: jgross@suse.com
> > CC: anthony.perard@citrix.com
> > CC: wl@xen.org
> > ---
> > If you have better suggestions for the wording in xs_wire.h please
> > suggest!
> > 
> > 
> > Changes in v6:
> > - use XENSTORE_CONNECTED instead of 0x0
> > - update xs_wire.h
> > 
> > Changes in v5:
> > - reset XS_CONNECTION_STATE_RECONNECTING before notifying the domU
> > 
> > Changes in v2:
> > - drop the new late_init parameter
> > ---
> >   tools/xenstore/xenstored_domain.c | 4 ++++
> >   xen/include/public/io/xs_wire.h   | 2 +-
> 
> I am not entirely sure this is the right place to mention it. But I couldn't
> find a better one.
> 
> The documentation (docs/misc/xenstore-misc.txt) states that the field is valid
> when the server advertised ``Connection State``.
> 
> Is there any guarantee the field will be 0 for any previous {C, O}xenstored
> implementation? If not, then I think we need to set the feature flag so Linux
> knows the field can be used.
> 
> If yes, then the documentation should be relaxed so an OS knows it can safely
> use the field without checking the feature flag.

The xenstore page is allocated by the toolstack which zeros the page,
*xenstored wouldn't set it, so I think we can assume the field has
always been zero.


> > diff --git a/xen/include/public/io/xs_wire.h
> > b/xen/include/public/io/xs_wire.h
> > index 953a0050a3..c1ec7c73e3 100644
> > --- a/xen/include/public/io/xs_wire.h
> > +++ b/xen/include/public/io/xs_wire.h
> > @@ -141,7 +141,7 @@ struct xenstore_domain_interface {
> >     /* Valid values for the connection field */
> >   #define XENSTORE_CONNECTED 0 /* the steady-state */
> > -#define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */
> > +#define XENSTORE_RECONNECT 1 /* reconnect in progress */
> 
> The definition in the docs needs to be updated.

I wanted to do that but I am very unfamiliar with the xenstore docs.
Can you point me to the place where I need to change the definition? I
cannot find where XENSTORE_RECONNECT is defined...
Re: [PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain
Posted by Juergen Gross 2 years, 6 months ago 
On 13.05.22 03:16, Stefano Stabellini wrote:
> On Wed, 11 May 2022, Julien Grall wrote:
>> On 05/05/2022 01:16, Stefano Stabellini wrote:
>>> From: Luca Miccio <lucmiccio@gmail.com>
>>>
>>> When xs_introduce_domain is called, send out a notification on the
>>> xenstore event channel so that any (dom0less) domain waiting for the
>>> xenstore interface to be ready can continue with the initialization.
>>> Before sending the notification, clear XENSTORE_RECONNECTING.
>>>
>>> The extra notification is harmless for domains that don't require it.
>>>
>>> In xs_wire.h update the commment on top of XENSTORE_RECONNECTING to
>>> generalize its meaning to suit the dom0less use-case better.
>>>
>>> Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
>>> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
>>> CC: jgross@suse.com
>>> CC: anthony.perard@citrix.com
>>> CC: wl@xen.org
>>> ---
>>> If you have better suggestions for the wording in xs_wire.h please
>>> suggest!
>>>
>>>
>>> Changes in v6:
>>> - use XENSTORE_CONNECTED instead of 0x0
>>> - update xs_wire.h
>>>
>>> Changes in v5:
>>> - reset XS_CONNECTION_STATE_RECONNECTING before notifying the domU
>>>
>>> Changes in v2:
>>> - drop the new late_init parameter
>>> ---
>>>    tools/xenstore/xenstored_domain.c | 4 ++++
>>>    xen/include/public/io/xs_wire.h   | 2 +-
>>
>> I am not entirely sure this is the right place to mention it. But I couldn't
>> find a better one.
>>
>> The documentation (docs/misc/xenstore-misc.txt) states that the field is valid
>> when the server advertised ``Connection State``.
>>
>> Is there any guarantee the field will be 0 for any previous {C, O}xenstored
>> implementation? If not, then I think we need to set the feature flag so Linux
>> knows the field can be used.
>>
>> If yes, then the documentation should be relaxed so an OS knows it can safely
>> use the field without checking the feature flag.
> 
> The xenstore page is allocated by the toolstack which zeros the page,
> *xenstored wouldn't set it, so I think we can assume the field has
> always been zero.
> 
> 
>>> diff --git a/xen/include/public/io/xs_wire.h
>>> b/xen/include/public/io/xs_wire.h
>>> index 953a0050a3..c1ec7c73e3 100644
>>> --- a/xen/include/public/io/xs_wire.h
>>> +++ b/xen/include/public/io/xs_wire.h
>>> @@ -141,7 +141,7 @@ struct xenstore_domain_interface {
>>>      /* Valid values for the connection field */
>>>    #define XENSTORE_CONNECTED 0 /* the steady-state */
>>> -#define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */
>>> +#define XENSTORE_RECONNECT 1 /* reconnect in progress */
>>
>> The definition in the docs needs to be updated.
> 
> I wanted to do that but I am very unfamiliar with the xenstore docs.
> Can you point me to the place where I need to change the definition? I
> cannot find where XENSTORE_RECONNECT is defined...
> 

See docs/misc/xenstore-ring.txt


Juergen
Re: [PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain
Posted by Stefano Stabellini 2 years, 6 months ago 
On Fri, 13 May 2022, Juergen Gross wrote:
> On 13.05.22 03:16, Stefano Stabellini wrote:
> > On Wed, 11 May 2022, Julien Grall wrote:
> > > On 05/05/2022 01:16, Stefano Stabellini wrote:
> > > > From: Luca Miccio <lucmiccio@gmail.com>
> > > > 
> > > > When xs_introduce_domain is called, send out a notification on the
> > > > xenstore event channel so that any (dom0less) domain waiting for the
> > > > xenstore interface to be ready can continue with the initialization.
> > > > Before sending the notification, clear XENSTORE_RECONNECTING.
> > > > 
> > > > The extra notification is harmless for domains that don't require it.
> > > > 
> > > > In xs_wire.h update the commment on top of XENSTORE_RECONNECTING to
> > > > generalize its meaning to suit the dom0less use-case better.
> > > > 
> > > > Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> > > > CC: jgross@suse.com
> > > > CC: anthony.perard@citrix.com
> > > > CC: wl@xen.org
> > > > ---
> > > > If you have better suggestions for the wording in xs_wire.h please
> > > > suggest!
> > > > 
> > > > 
> > > > Changes in v6:
> > > > - use XENSTORE_CONNECTED instead of 0x0
> > > > - update xs_wire.h
> > > > 
> > > > Changes in v5:
> > > > - reset XS_CONNECTION_STATE_RECONNECTING before notifying the domU
> > > > 
> > > > Changes in v2:
> > > > - drop the new late_init parameter
> > > > ---
> > > >    tools/xenstore/xenstored_domain.c | 4 ++++
> > > >    xen/include/public/io/xs_wire.h   | 2 +-
> > > 
> > > I am not entirely sure this is the right place to mention it. But I
> > > couldn't
> > > find a better one.
> > > 
> > > The documentation (docs/misc/xenstore-misc.txt) states that the field is
> > > valid
> > > when the server advertised ``Connection State``.
> > > 
> > > Is there any guarantee the field will be 0 for any previous {C,
> > > O}xenstored
> > > implementation? If not, then I think we need to set the feature flag so
> > > Linux
> > > knows the field can be used.
> > > 
> > > If yes, then the documentation should be relaxed so an OS knows it can
> > > safely
> > > use the field without checking the feature flag.
> > 
> > The xenstore page is allocated by the toolstack which zeros the page,
> > *xenstored wouldn't set it, so I think we can assume the field has
> > always been zero.
> > 
> > 
> > > > diff --git a/xen/include/public/io/xs_wire.h
> > > > b/xen/include/public/io/xs_wire.h
> > > > index 953a0050a3..c1ec7c73e3 100644
> > > > --- a/xen/include/public/io/xs_wire.h
> > > > +++ b/xen/include/public/io/xs_wire.h
> > > > @@ -141,7 +141,7 @@ struct xenstore_domain_interface {
> > > >      /* Valid values for the connection field */
> > > >    #define XENSTORE_CONNECTED 0 /* the steady-state */
> > > > -#define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */
> > > > +#define XENSTORE_RECONNECT 1 /* reconnect in progress */
> > > 
> > > The definition in the docs needs to be updated.
> > 
> > I wanted to do that but I am very unfamiliar with the xenstore docs.
> > Can you point me to the place where I need to change the definition? I
> > cannot find where XENSTORE_RECONNECT is defined...
> > 
> 
> See docs/misc/xenstore-ring.txt

Found it, thanks! I'll add:

In certain circumstances (e.g. dom0less guests with PV drivers support)
it is possible for the guest to find the Connection state already set to
1 by someone else during xenstore initialization. In that case, like in
the previous case, the guest must ignore all fields except the
Connection state and wait for it to be set to 0 before proceeding.
Re: [PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain
Posted by Juergen Gross 2 years, 6 months ago 
On 05.05.22 02:16, Stefano Stabellini wrote:
> From: Luca Miccio <lucmiccio@gmail.com>
> 
> When xs_introduce_domain is called, send out a notification on the
> xenstore event channel so that any (dom0less) domain waiting for the
> xenstore interface to be ready can continue with the initialization.
> Before sending the notification, clear XENSTORE_RECONNECTING.
> 
> The extra notification is harmless for domains that don't require it.
> 
> In xs_wire.h update the commment on top of XENSTORE_RECONNECTING to
> generalize its meaning to suit the dom0less use-case better.
> 
> Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> CC: jgross@suse.com
> CC: anthony.perard@citrix.com
> CC: wl@xen.org
> ---
> If you have better suggestions for the wording in xs_wire.h please
> suggest!

Reviewed-by: Juergen Gross <jgross@suse.com>


Juergen

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.