[PATCH 11/23] tools/xenstored: Automatically set dom0_domid and priv_domid

Jason Andryuk posted 23 patches 11 months, 1 week ago
There is a newer version of this series
[PATCH 11/23] tools/xenstored: Automatically set dom0_domid and priv_domid
Posted by Jason Andryuk 11 months, 1 week ago
With split hardware and control domains, each domain should be
privileged with respect to xenstore.  When adding domains to xenstore,
look at their privilege and add them to xenstored as appropriate.
dom0_domid is used for the hardware domain, and priv_domid is used for a
control domain.

Only one of each is allowed for now.

Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
---
 tools/xenstored/domain.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
index 64c8fd0cc3..f2394cd6e9 100644
--- a/tools/xenstored/domain.c
+++ b/tools/xenstored/domain.c
@@ -795,6 +795,20 @@ static struct domain *find_or_alloc_existing_domain(unsigned int domid)
 	return domain;
 }
 
+static void domain_set_privileged(struct domain *domain)
+{
+	xc_domaininfo_t dominfo;
+
+	if ( !get_domain_info(domain->domid, &dominfo) )
+		return;
+
+	if ( dominfo.flags & XEN_DOMINF_priv )
+		priv_domid = domain->domid;
+
+	if ( dominfo.flags & XEN_DOMINF_hardware )
+		dom0_domid = domain->domid;
+}
+
 static int new_domain(struct domain *domain, int port, bool restore)
 {
 	int rc;
@@ -831,6 +845,8 @@ static int new_domain(struct domain *domain, int port, bool restore)
 	domain->conn->domain = domain;
 	domain->conn->id = domain->domid;
 
+	domain_set_privileged(domain);
+
 	return 0;
 }
 
-- 
2.48.1
Re: [PATCH 11/23] tools/xenstored: Automatically set dom0_domid and priv_domid
Posted by Jürgen Groß 11 months, 1 week ago
On 06.03.25 23:03, Jason Andryuk wrote:
> With split hardware and control domains, each domain should be
> privileged with respect to xenstore.  When adding domains to xenstore,
> look at their privilege and add them to xenstored as appropriate.
> dom0_domid is used for the hardware domain, and priv_domid is used for a
> control domain.
> 
> Only one of each is allowed for now.
> 
> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
> ---
>   tools/xenstored/domain.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
> index 64c8fd0cc3..f2394cd6e9 100644
> --- a/tools/xenstored/domain.c
> +++ b/tools/xenstored/domain.c
> @@ -795,6 +795,20 @@ static struct domain *find_or_alloc_existing_domain(unsigned int domid)
>   	return domain;
>   }
>   
> +static void domain_set_privileged(struct domain *domain)
> +{
> +	xc_domaininfo_t dominfo;
> +
> +	if ( !get_domain_info(domain->domid, &dominfo) )
> +		return;
> +
> +	if ( dominfo.flags & XEN_DOMINF_priv )
> +		priv_domid = domain->domid;
> +
> +	if ( dominfo.flags & XEN_DOMINF_hardware )
> +		dom0_domid = domain->domid;
> +}

Please no use of libxenctrl. I have worked hard to eliminate the usage
in order to enable a xenstore-stubdom being used across Xen versions
(C Xenstore is relying on stable hypercalls only now).

You need to add the needed flags to the rather new stable domctl
XEN_DOMCTL_get_domain_state and to libxenmanage.

> +
>   static int new_domain(struct domain *domain, int port, bool restore)
>   {
>   	int rc;
> @@ -831,6 +845,8 @@ static int new_domain(struct domain *domain, int port, bool restore)
>   	domain->conn->domain = domain;
>   	domain->conn->id = domain->domid;
>   
> +	domain_set_privileged(domain);

The name implies you are changing the domain to be privileged, but this
is done conditionally only.

Maybe name the function domain_apply_privileges()?


Juergen
Re: [PATCH 11/23] tools/xenstored: Automatically set dom0_domid and priv_domid
Posted by Jason Andryuk 11 months ago

On 2025-03-08 02:02, Jürgen Groß wrote:
> On 06.03.25 23:03, Jason Andryuk wrote:
>> With split hardware and control domains, each domain should be
>> privileged with respect to xenstore.  When adding domains to xenstore,
>> look at their privilege and add them to xenstored as appropriate.
>> dom0_domid is used for the hardware domain, and priv_domid is used for a
>> control domain.
>>
>> Only one of each is allowed for now.
>>
>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>> ---
>>   tools/xenstored/domain.c | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>> index 64c8fd0cc3..f2394cd6e9 100644
>> --- a/tools/xenstored/domain.c
>> +++ b/tools/xenstored/domain.c
>> @@ -795,6 +795,20 @@ static struct domain 
>> *find_or_alloc_existing_domain(unsigned int domid)
>>       return domain;
>>   }
>> +static void domain_set_privileged(struct domain *domain)
>> +{
>> +    xc_domaininfo_t dominfo;
>> +
>> +    if ( !get_domain_info(domain->domid, &dominfo) )
>> +        return;
>> +
>> +    if ( dominfo.flags & XEN_DOMINF_priv )
>> +        priv_domid = domain->domid;
>> +
>> +    if ( dominfo.flags & XEN_DOMINF_hardware )
>> +        dom0_domid = domain->domid;
>> +}
> 
> Please no use of libxenctrl. I have worked hard to eliminate the usage
> in order to enable a xenstore-stubdom being used across Xen versions
> (C Xenstore is relying on stable hypercalls only now).

Right.  Yes, nice work on switching to stable hypercalls.

> You need to add the needed flags to the rather new stable domctl
> XEN_DOMCTL_get_domain_state and to libxenmanage.

Ok.

>> +
>>   static int new_domain(struct domain *domain, int port, bool restore)
>>   {
>>       int rc;
>> @@ -831,6 +845,8 @@ static int new_domain(struct domain *domain, int 
>> port, bool restore)
>>       domain->conn->domain = domain;
>>       domain->conn->id = domain->domid;
>> +    domain_set_privileged(domain);
> 
> The name implies you are changing the domain to be privileged, but this
> is done conditionally only.
> 
> Maybe name the function domain_apply_privileges()?

I'm thinking domain_apply_capabilities() since they are being referred 
to as capabilities.

But I'll have to revisit this.  To make xenstored "just work" when domid 
!= 0, it should auto-detect its domain id, and that has to be done 
earlier than this.

Regards,
Jason


Re: [PATCH 11/23] tools/xenstored: Automatically set dom0_domid and priv_domid
Posted by Jürgen Groß 11 months ago
On 10.03.25 15:32, Jason Andryuk wrote:
> 
> 
> On 2025-03-08 02:02, Jürgen Groß wrote:
>> On 06.03.25 23:03, Jason Andryuk wrote:
>>> With split hardware and control domains, each domain should be
>>> privileged with respect to xenstore.  When adding domains to xenstore,
>>> look at their privilege and add them to xenstored as appropriate.
>>> dom0_domid is used for the hardware domain, and priv_domid is used for a
>>> control domain.
>>>
>>> Only one of each is allowed for now.
>>>
>>> Signed-off-by: Jason Andryuk <jason.andryuk@amd.com>
>>> ---
>>>   tools/xenstored/domain.c | 16 ++++++++++++++++
>>>   1 file changed, 16 insertions(+)
>>>
>>> diff --git a/tools/xenstored/domain.c b/tools/xenstored/domain.c
>>> index 64c8fd0cc3..f2394cd6e9 100644
>>> --- a/tools/xenstored/domain.c
>>> +++ b/tools/xenstored/domain.c
>>> @@ -795,6 +795,20 @@ static struct domain 
>>> *find_or_alloc_existing_domain(unsigned int domid)
>>>       return domain;
>>>   }
>>> +static void domain_set_privileged(struct domain *domain)
>>> +{
>>> +    xc_domaininfo_t dominfo;
>>> +
>>> +    if ( !get_domain_info(domain->domid, &dominfo) )
>>> +        return;
>>> +
>>> +    if ( dominfo.flags & XEN_DOMINF_priv )
>>> +        priv_domid = domain->domid;
>>> +
>>> +    if ( dominfo.flags & XEN_DOMINF_hardware )
>>> +        dom0_domid = domain->domid;
>>> +}
>>
>> Please no use of libxenctrl. I have worked hard to eliminate the usage
>> in order to enable a xenstore-stubdom being used across Xen versions
>> (C Xenstore is relying on stable hypercalls only now).
> 
> Right.  Yes, nice work on switching to stable hypercalls.
> 
>> You need to add the needed flags to the rather new stable domctl
>> XEN_DOMCTL_get_domain_state and to libxenmanage.
> 
> Ok.
> 
>>> +
>>>   static int new_domain(struct domain *domain, int port, bool restore)
>>>   {
>>>       int rc;
>>> @@ -831,6 +845,8 @@ static int new_domain(struct domain *domain, int port, 
>>> bool restore)
>>>       domain->conn->domain = domain;
>>>       domain->conn->id = domain->domid;
>>> +    domain_set_privileged(domain);
>>
>> The name implies you are changing the domain to be privileged, but this
>> is done conditionally only.
>>
>> Maybe name the function domain_apply_privileges()?
> 
> I'm thinking domain_apply_capabilities() since they are being referred to as 
> capabilities.
> 
> But I'll have to revisit this.  To make xenstored "just work" when domid != 0, 
> it should auto-detect its domain id, and that has to be done earlier than this.

I think with the 3 remaining patches of my series [1] this should be handled
already.

[1]: https://lists.xen.org/archives/html/xen-devel/2025-02/msg00120.html
      https://lists.xen.org/archives/html/xen-devel/2025-02/msg00121.html
      https://lists.xen.org/archives/html/xen-devel/2025-02/msg00122.html


Juergen