Re: [PATCH v3 1/2] xen: Add capabilities to get_domain_state

[Jürgen Groß <jgross@xxxxxxxx>]

On 23.07.25 08:25, Jan Beulich wrote:
> On 22.07.2025 14:09, Jason Andryuk wrote:
> > On 2025-07-21 20:19, Jason Andryuk wrote:
> > > Expose a domain's capabilities - control, hardware or xenstore - through
> > > stable get domain state hypercall.
> > >
> > > The xenstore domain can use this information to assign appropriate
> > > permissions on connections.
> > >
> > > Repurpose the 16bit pad field for this purpose.
> > >
> > > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > > Reviewed-by: Juergen Gross <jgross@xxxxxxxx>
> > > ---
> > > v2:
> > > Init info->caps = 0
> > > Remove stale comment on caps field
> > > Add Juergen's R-b
> > > ---
> > >    xen/common/domain.c         | 10 +++++++++-
> > >    xen/include/public/domctl.h |  7 +++++--
> > >    2 files changed, 14 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/xen/common/domain.c b/xen/common/domain.c
> > > index c347de4335..bb33b1f1c7 100644
> > > --- a/xen/common/domain.c
> > > +++ b/xen/common/domain.c
> > > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> > > index f1f6f96bc2..136820ea5b 100644
> > > --- a/xen/include/public/domctl.h
> > > +++ b/xen/include/public/domctl.h
> > > @@ -1269,8 +1269,11 @@ struct xen_domctl_get_domain_state {
> > >    #define XEN_DOMCTL_GETDOMSTATE_STATE_SHUTDOWN  0x0002  /* Shutdown finished. 
> > > */
> > >    #define XEN_DOMCTL_GETDOMSTATE_STATE_DYING     0x0004  /* Domain dying. */
> > >    #define XEN_DOMCTL_GETDOMSTATE_STATE_DEAD      0x0008  /* Domain dead. */
> > > -    uint16_t pad0;           /* Must be 0 on input, returned as 0. */
> > > -    uint32_t pad1;           /* Must be 0 on input, returned as 0. */
> > > +    uint16_t caps;
> > > +#define XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL     0x0001  /* Control domain. */
> > > +#define XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE    0x0002  /* Hardware domain. */
> > > +#define XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE    0x0004  /* Xenstore domain. */
> > > +    uint32_t pad0;           /* Must be 0 on input, returned as 0. */
> > I have wondered if we should use some of this padding to start returning
> > the valid capability bits.  When the hypercall (and library) will be
> > ready in case the number increases.
> How would a caller use that information? The hypervisor wouldn't return
> "invalid" bits set. (If any such plausible use could be envisioned, it
> being a stable sub-op, providing such information would certainly make
> sense.)
When scanning all domains there is a difference between "there is no domain
with capability FOO" and "hypervisor doesn't report capability FOO".

The question is what a caller would do in the latter case.

> > The other alternative would be to return the bits in some other call, in
> > which case this one would not need to change.  And returning the
> > unchanging valids bits on each call seems unnecessary.
> Indeed. Yet the same interface could still be used (then perhaps also
> to indicate the valid XEN_DOMCTL_GETDOMSTATE_STATE_* bits): Have the
> caller pass in another special DOMID_* (DOMID_INVALID already has a
> meaning, though).
I'd rather go this route, as this would not burn another padding field for
rare use cases. DOMID_XEN might be a good fit.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature