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@amd.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
---
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
@@ -195,6 +195,14 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info,
info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING;
if ( d->is_dying == DOMDYING_dead )
info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD;
+
+ info->caps = 0;
+ if ( is_control_domain(d) )
+ info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL;
+ if ( is_hardware_domain(d) )
+ info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE;
+ if ( is_xenstore_domain(d) )
+ info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE;
info->unique_id = d->unique_id;
}
@@ -205,7 +213,7 @@ int get_domain_state(struct xen_domctl_get_domain_state *info, struct domain *d,
int rc = -ENOENT;
struct domain *hdl;
- if ( info->pad0 || info->pad1 )
+ if ( info->pad0 )
return -EINVAL;
if ( d )
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. */
uint64_t unique_id; /* Unique domain identifier. */
};
--
2.50.1On 22.07.2025 02:19, Jason Andryuk wrote: > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -195,6 +195,14 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, > info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; > if ( d->is_dying == DOMDYING_dead ) > info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; > + > + info->caps = 0; > + if ( is_control_domain(d) ) > + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL; > + if ( is_hardware_domain(d) ) > + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE; > + if ( is_xenstore_domain(d) ) > + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE; > info->unique_id = d->unique_id; > } This being a stable sub-op, don't we need a way to indicate to the caller _that_ this field has valid data now? When non-zero it's easy to tell, but getting back zero is ambiguous. Jan
On 23.07.25 08:28, Jan Beulich wrote: > On 22.07.2025 02:19, Jason Andryuk wrote: >> --- a/xen/common/domain.c >> +++ b/xen/common/domain.c >> @@ -195,6 +195,14 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, >> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; >> if ( d->is_dying == DOMDYING_dead ) >> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; >> + >> + info->caps = 0; >> + if ( is_control_domain(d) ) >> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL; >> + if ( is_hardware_domain(d) ) >> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE; >> + if ( is_xenstore_domain(d) ) >> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE; >> info->unique_id = d->unique_id; >> } > > This being a stable sub-op, don't we need a way to indicate to the caller > _that_ this field has valid data now? When non-zero it's easy to tell, but > getting back zero is ambiguous. The hypercall sub-op was introduced in this development cycle only, so there is no released Xen hypervisor without the capability setting. In case this patch doesn't make it into 4.21, the case you are mentioning must be handled, of course. Juergen
On 23.07.2025 08:34, Jürgen Groß wrote: > On 23.07.25 08:28, Jan Beulich wrote: >> On 22.07.2025 02:19, Jason Andryuk wrote: >>> --- a/xen/common/domain.c >>> +++ b/xen/common/domain.c >>> @@ -195,6 +195,14 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, >>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; >>> if ( d->is_dying == DOMDYING_dead ) >>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; >>> + >>> + info->caps = 0; >>> + if ( is_control_domain(d) ) >>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL; >>> + if ( is_hardware_domain(d) ) >>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE; >>> + if ( is_xenstore_domain(d) ) >>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE; >>> info->unique_id = d->unique_id; >>> } >> >> This being a stable sub-op, don't we need a way to indicate to the caller >> _that_ this field has valid data now? When non-zero it's easy to tell, but >> getting back zero is ambiguous. > > The hypercall sub-op was introduced in this development cycle only, so > there is no released Xen hypervisor without the capability setting. > > In case this patch doesn't make it into 4.21, the case you are mentioning > must be handled, of course. Hmm, yes, this way it's on the edge. As a stable sub-op, someone could have backported the change, though. IOW (and in general) I wonder whether for stable sub-ops we shouldn't be pretty strict about functional extensions, not tying their addition to releases at all. Jan
On 23.07.25 08:43, Jan Beulich wrote: > On 23.07.2025 08:34, Jürgen Groß wrote: >> On 23.07.25 08:28, Jan Beulich wrote: >>> On 22.07.2025 02:19, Jason Andryuk wrote: >>>> --- a/xen/common/domain.c >>>> +++ b/xen/common/domain.c >>>> @@ -195,6 +195,14 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, >>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; >>>> if ( d->is_dying == DOMDYING_dead ) >>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; >>>> + >>>> + info->caps = 0; >>>> + if ( is_control_domain(d) ) >>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL; >>>> + if ( is_hardware_domain(d) ) >>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE; >>>> + if ( is_xenstore_domain(d) ) >>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE; >>>> info->unique_id = d->unique_id; >>>> } >>> >>> This being a stable sub-op, don't we need a way to indicate to the caller >>> _that_ this field has valid data now? When non-zero it's easy to tell, but >>> getting back zero is ambiguous. >> >> The hypercall sub-op was introduced in this development cycle only, so >> there is no released Xen hypervisor without the capability setting. >> >> In case this patch doesn't make it into 4.21, the case you are mentioning >> must be handled, of course. > > Hmm, yes, this way it's on the edge. As a stable sub-op, someone could have > backported the change, though. IOW (and in general) I wonder whether for > stable sub-ops we shouldn't be pretty strict about functional extensions, > not tying their addition to releases at all. Should we really care about downstreams backporting not yet released functionality? Using your reasoning this would mean we'd need to issue XSAs for not yet released hypervisor issues of stable interfaces, too. I don't think we want to do that. Juergen
On 23.07.2025 08:55, Jürgen Groß wrote: > On 23.07.25 08:43, Jan Beulich wrote: >> On 23.07.2025 08:34, Jürgen Groß wrote: >>> On 23.07.25 08:28, Jan Beulich wrote: >>>> On 22.07.2025 02:19, Jason Andryuk wrote: >>>>> --- a/xen/common/domain.c >>>>> +++ b/xen/common/domain.c >>>>> @@ -195,6 +195,14 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, >>>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; >>>>> if ( d->is_dying == DOMDYING_dead ) >>>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; >>>>> + >>>>> + info->caps = 0; >>>>> + if ( is_control_domain(d) ) >>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL; >>>>> + if ( is_hardware_domain(d) ) >>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE; >>>>> + if ( is_xenstore_domain(d) ) >>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE; >>>>> info->unique_id = d->unique_id; >>>>> } >>>> >>>> This being a stable sub-op, don't we need a way to indicate to the caller >>>> _that_ this field has valid data now? When non-zero it's easy to tell, but >>>> getting back zero is ambiguous. >>> >>> The hypercall sub-op was introduced in this development cycle only, so >>> there is no released Xen hypervisor without the capability setting. >>> >>> In case this patch doesn't make it into 4.21, the case you are mentioning >>> must be handled, of course. >> >> Hmm, yes, this way it's on the edge. As a stable sub-op, someone could have >> backported the change, though. IOW (and in general) I wonder whether for >> stable sub-ops we shouldn't be pretty strict about functional extensions, >> not tying their addition to releases at all. > > Should we really care about downstreams backporting not yet released > functionality? > > Using your reasoning this would mean we'd need to issue XSAs for not yet > released hypervisor issues of stable interfaces, too. I don't think we > want to do that. To me, the latter doesn't necessarily follow from the former. But anyway, I'm not going to insist, but I wanted the situation to at least be considered. In particular also forward-looking, when we may gain more stable sub-ops. At some point it may turn out necessary to backport such even into upstream branches. Jan
On 23.07.25 09:04, Jan Beulich wrote: > On 23.07.2025 08:55, Jürgen Groß wrote: >> On 23.07.25 08:43, Jan Beulich wrote: >>> On 23.07.2025 08:34, Jürgen Groß wrote: >>>> On 23.07.25 08:28, Jan Beulich wrote: >>>>> On 22.07.2025 02:19, Jason Andryuk wrote: >>>>>> --- a/xen/common/domain.c >>>>>> +++ b/xen/common/domain.c >>>>>> @@ -195,6 +195,14 @@ static void set_domain_state_info(struct xen_domctl_get_domain_state *info, >>>>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; >>>>>> if ( d->is_dying == DOMDYING_dead ) >>>>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; >>>>>> + >>>>>> + info->caps = 0; >>>>>> + if ( is_control_domain(d) ) >>>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL; >>>>>> + if ( is_hardware_domain(d) ) >>>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE; >>>>>> + if ( is_xenstore_domain(d) ) >>>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE; >>>>>> info->unique_id = d->unique_id; >>>>>> } >>>>> >>>>> This being a stable sub-op, don't we need a way to indicate to the caller >>>>> _that_ this field has valid data now? When non-zero it's easy to tell, but >>>>> getting back zero is ambiguous. >>>> >>>> The hypercall sub-op was introduced in this development cycle only, so >>>> there is no released Xen hypervisor without the capability setting. >>>> >>>> In case this patch doesn't make it into 4.21, the case you are mentioning >>>> must be handled, of course. >>> >>> Hmm, yes, this way it's on the edge. As a stable sub-op, someone could have >>> backported the change, though. IOW (and in general) I wonder whether for >>> stable sub-ops we shouldn't be pretty strict about functional extensions, >>> not tying their addition to releases at all. >> >> Should we really care about downstreams backporting not yet released >> functionality? >> >> Using your reasoning this would mean we'd need to issue XSAs for not yet >> released hypervisor issues of stable interfaces, too. I don't think we >> want to do that. > > To me, the latter doesn't necessarily follow from the former. But anyway, I'm > not going to insist, but I wanted the situation to at least be considered. In > particular also forward-looking, when we may gain more stable sub-ops. At some > point it may turn out necessary to backport such even into upstream branches. I think it is fine to discuss this situation. I'd suggest not to support any potential downstream backports of not yet released functionality. Consider a new interface being developed in a larger patch series. In case the series is not being committed in one go, would you want to support backports of only a part of it? What about fixes of that interface in the current release cycle, e.g. due to the use cases having been committed only some time later uncovering the need to change the interface to make it safe? Juergen
On 23/07/2025 8:29 am, Jürgen Groß wrote: > On 23.07.25 09:04, Jan Beulich wrote: >> On 23.07.2025 08:55, Jürgen Groß wrote: >>> On 23.07.25 08:43, Jan Beulich wrote: >>>> On 23.07.2025 08:34, Jürgen Groß wrote: >>>>> On 23.07.25 08:28, Jan Beulich wrote: >>>>>> On 22.07.2025 02:19, Jason Andryuk wrote: >>>>>>> --- a/xen/common/domain.c >>>>>>> +++ b/xen/common/domain.c >>>>>>> @@ -195,6 +195,14 @@ static void set_domain_state_info(struct >>>>>>> xen_domctl_get_domain_state *info, >>>>>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DYING; >>>>>>> if ( d->is_dying == DOMDYING_dead ) >>>>>>> info->state |= XEN_DOMCTL_GETDOMSTATE_STATE_DEAD; >>>>>>> + >>>>>>> + info->caps = 0; >>>>>>> + if ( is_control_domain(d) ) >>>>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_CONTROL; >>>>>>> + if ( is_hardware_domain(d) ) >>>>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_HARDWARE; >>>>>>> + if ( is_xenstore_domain(d) ) >>>>>>> + info->caps |= XEN_DOMCTL_GETDOMSTATE_CAP_XENSTORE; >>>>>>> info->unique_id = d->unique_id; >>>>>>> } >>>>>> >>>>>> This being a stable sub-op, don't we need a way to indicate to >>>>>> the caller >>>>>> _that_ this field has valid data now? When non-zero it's easy to >>>>>> tell, but >>>>>> getting back zero is ambiguous. >>>>> >>>>> The hypercall sub-op was introduced in this development cycle >>>>> only, so >>>>> there is no released Xen hypervisor without the capability setting. >>>>> >>>>> In case this patch doesn't make it into 4.21, the case you are >>>>> mentioning >>>>> must be handled, of course. >>>> >>>> Hmm, yes, this way it's on the edge. As a stable sub-op, someone >>>> could have >>>> backported the change, though. IOW (and in general) I wonder >>>> whether for >>>> stable sub-ops we shouldn't be pretty strict about functional >>>> extensions, >>>> not tying their addition to releases at all. >>> >>> Should we really care about downstreams backporting not yet released >>> functionality? >>> >>> Using your reasoning this would mean we'd need to issue XSAs for not >>> yet >>> released hypervisor issues of stable interfaces, too. I don't think we >>> want to do that. >> >> To me, the latter doesn't necessarily follow from the former. But >> anyway, I'm >> not going to insist, but I wanted the situation to at least be >> considered. In >> particular also forward-looking, when we may gain more stable >> sub-ops. At some >> point it may turn out necessary to backport such even into upstream >> branches. > > I think it is fine to discuss this situation. > > I'd suggest not to support any potential downstream backports of not yet > released functionality. Consider a new interface being developed in a > larger > patch series. In case the series is not being committed in one go, > would you > want to support backports of only a part of it? What about fixes of that > interface in the current release cycle, e.g. due to the use cases > having been > committed only some time later uncovering the need to change the > interface > to make it safe? Interfaces cannot be fixed until they're in a RELEASE-* tag. Anything else would be absurd position to put upstream Xen into. Downstreams backporting a not-yet-fixed interface is equivalent to taking a private ABI into their patchqueue. They get to keep both pieces if it goes wrong. (and I say this as someone who frequently walks this tightrope in the XenServer patchqueue.) ~Andrew
On Mon, 21 Jul 2025, 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@amd.com> > Reviewed-by: Juergen Gross <jgross@suse.com> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
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@amd.com>
> Reviewed-by: Juergen Gross <jgross@suse.com>
> ---
> 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.
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.
Regards,
Jason
> uint64_t unique_id; /* Unique domain identifier. */
> };
>
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@amd.com>
>> Reviewed-by: Juergen Gross <jgross@suse.com>
>> ---
>> 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.)
> 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).
Jan
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@amd.com>
>>> Reviewed-by: Juergen Gross <jgross@suse.com>
>>> ---
>>> 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
© 2016 - 2025 Red Hat, Inc.