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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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@xxxxxxxxx>
> > > > 
> > > > 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@xxxxxxxxx>
> > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > > > CC: jgross@xxxxxxxx
> > > > CC: anthony.perard@xxxxxxxxxx
> > > > CC: wl@xxxxxxx
> > > > ---
> > > > 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.