Convert alloc_dom0_vcpu0() to dom0_set_affinity(), making it only set up the
node affinity based on command line parameters passed. At the same time,
introduce alloc_dom_vcpu0() as the replacement for alloc_dom0_vcpu(). Then have
alloc_dom_vcpu0() call dom0_set_affinity() when the boot domain is the control
domain, otherwise set the affinity to auto.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c                 |  4 +---
 xen/arch/x86/domain-builder/domain.c      | 11 +++++++++++
 xen/arch/x86/include/asm/dom0_build.h     |  2 ++
 xen/arch/x86/include/asm/domain-builder.h |  1 +
 xen/arch/x86/setup.c                      |  5 +++--
 5 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 7b3e31a08f7d..77386cd1e20c 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -254,12 +254,10 @@ unsigned int __init dom0_max_vcpus(void)
     return max_vcpus;
 }
 
-struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
+void __init dom0_set_affinity(struct domain *dom0)
 {
     dom0->node_affinity = dom0_nodes;
     dom0->auto_node_affinity = !dom0_nr_pxms;
-
-    return vcpu_create(dom0, 0);
 }
 
 #ifdef CONFIG_SHADOW_PAGING
diff --git a/xen/arch/x86/domain-builder/domain.c b/xen/arch/x86/domain-builder/domain.c
index f2277b9e3cf3..619d36ea0b87 100644
--- a/xen/arch/x86/domain-builder/domain.c
+++ b/xen/arch/x86/domain-builder/domain.c
@@ -9,6 +9,7 @@
 #include <xen/sched.h>
 
 #include <asm/bootinfo.h>
+#include <asm/dom0_build.h>
 
 unsigned int __init dom_max_vcpus(struct boot_domain *bd)
 {
@@ -27,6 +28,16 @@ unsigned int __init dom_max_vcpus(struct boot_domain *bd)
     return bd->max_vcpus;
 }
 
+struct vcpu *__init alloc_dom_vcpu0(struct boot_domain *bd)
+{
+    if ( bd->capabilities & BUILD_CAPS_CONTROL )
+        dom0_set_affinity(bd->d);
+    else
+        bd->d->auto_node_affinity = true;
+
+    return vcpu_create(bd->d, 0);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index ff021c24af9d..426def4115ce 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -8,6 +8,8 @@
 
 extern unsigned int dom0_memflags;
 
+void dom0_set_affinity(struct domain *dom0);
+
 unsigned long dom0_compute_nr_pages(struct domain *d,
                                     struct elf_dom_parms *parms,
                                     unsigned long initrd_len);
diff --git a/xen/arch/x86/include/asm/domain-builder.h b/xen/arch/x86/include/asm/domain-builder.h
index f37f73e2255b..dd47e9ac0dc6 100644
--- a/xen/arch/x86/include/asm/domain-builder.h
+++ b/xen/arch/x86/include/asm/domain-builder.h
@@ -9,5 +9,6 @@ int __init builder_get_cmdline(
 
 void builder_init(struct boot_info *bi);
 unsigned int dom_max_vcpus(struct boot_domain *bd);
+struct vcpu *alloc_dom_vcpu0(struct boot_domain *bd);
 
 #endif
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 86bbd7c72ccd..8ba9d592ed5a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1048,9 +1048,11 @@ static struct domain *__init create_dom0(struct boot_info *bi)
     if ( IS_ERR(d) )
         panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
 
+    bd->d = d;
+
     init_dom0_cpuid_policy(d);
 
-    if ( alloc_dom0_vcpu0(d) == NULL )
+    if ( alloc_dom_vcpu0(bd) == NULL )
         panic("Error creating %pd vcpu 0\n", d);
 
     cmdline_size = domain_cmdline_size(bi, bd);
@@ -1090,7 +1092,6 @@ static struct domain *__init create_dom0(struct boot_info *bi)
         bd->cmdline = cmdline;
     }
 
-    bd->d = d;
     if ( construct_dom0(bd) != 0 )
         panic("Could not construct domain 0\n");
 
-- 
2.30.2

On Sat Apr 19, 2025 at 11:07 PM BST, Daniel P. Smith wrote:
> Convert alloc_dom0_vcpu0() to dom0_set_affinity(), making it only set up the
> node affinity based on command line parameters passed. At the same time,
> introduce alloc_dom_vcpu0() as the replacement for alloc_dom0_vcpu(). Then have
> alloc_dom_vcpu0() call dom0_set_affinity() when the boot domain is the control
> domain, otherwise set the affinity to auto.
>
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
>  xen/arch/x86/dom0_build.c                 |  4 +---
>  xen/arch/x86/domain-builder/domain.c      | 11 +++++++++++
>  xen/arch/x86/include/asm/dom0_build.h     |  2 ++
>  xen/arch/x86/include/asm/domain-builder.h |  1 +
>  xen/arch/x86/setup.c                      |  5 +++--
>  5 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/domain-builder/domain.c b/xen/arch/x86/domain-builder/domain.c
> index f2277b9e3cf3..619d36ea0b87 100644
> --- a/xen/arch/x86/domain-builder/domain.c
> +++ b/xen/arch/x86/domain-builder/domain.c
> @@ -9,6 +9,7 @@
>  #include <xen/sched.h>
>  
>  #include <asm/bootinfo.h>
> +#include <asm/dom0_build.h>
>  
>  unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>  {
> @@ -27,6 +28,16 @@ unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>      return bd->max_vcpus;
>  }
>  
> +struct vcpu *__init alloc_dom_vcpu0(struct boot_domain *bd)
> +{
> +    if ( bd->capabilities & BUILD_CAPS_CONTROL )
> +        dom0_set_affinity(bd->d);

Similar as before, this probably wants to be DOMAIN_CAPS_HARDWARE?

I'll adjust while rebasing.

Cheers,
Alejandro

On 4/25/25 11:22, Alejandro Vallejo wrote:
> On Sat Apr 19, 2025 at 11:07 PM BST, Daniel P. Smith wrote:
>> Convert alloc_dom0_vcpu0() to dom0_set_affinity(), making it only set up the
>> node affinity based on command line parameters passed. At the same time,
>> introduce alloc_dom_vcpu0() as the replacement for alloc_dom0_vcpu(). Then have
>> alloc_dom_vcpu0() call dom0_set_affinity() when the boot domain is the control
>> domain, otherwise set the affinity to auto.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
>>   xen/arch/x86/dom0_build.c                 |  4 +---
>>   xen/arch/x86/domain-builder/domain.c      | 11 +++++++++++
>>   xen/arch/x86/include/asm/dom0_build.h     |  2 ++
>>   xen/arch/x86/include/asm/domain-builder.h |  1 +
>>   xen/arch/x86/setup.c                      |  5 +++--
>>   5 files changed, 18 insertions(+), 5 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain-builder/domain.c b/xen/arch/x86/domain-builder/domain.c
>> index f2277b9e3cf3..619d36ea0b87 100644
>> --- a/xen/arch/x86/domain-builder/domain.c
>> +++ b/xen/arch/x86/domain-builder/domain.c
>> @@ -9,6 +9,7 @@
>>   #include <xen/sched.h>
>>   
>>   #include <asm/bootinfo.h>
>> +#include <asm/dom0_build.h>
>>   
>>   unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>>   {
>> @@ -27,6 +28,16 @@ unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>>       return bd->max_vcpus;
>>   }
>>   
>> +struct vcpu *__init alloc_dom_vcpu0(struct boot_domain *bd)
>> +{
>> +    if ( bd->capabilities & BUILD_CAPS_CONTROL )
>> +        dom0_set_affinity(bd->d);
> 
> Similar as before, this probably wants to be DOMAIN_CAPS_HARDWARE?
> 
> I'll adjust while rebasing.

Does it?

v/r,
dps

On Fri Apr 25, 2025 at 11:04 PM BST, Daniel P. Smith wrote:
> On 4/25/25 11:22, Alejandro Vallejo wrote:
>> On Sat Apr 19, 2025 at 11:07 PM BST, Daniel P. Smith wrote:
>>> Convert alloc_dom0_vcpu0() to dom0_set_affinity(), making it only set up the
>>> node affinity based on command line parameters passed. At the same time,
>>> introduce alloc_dom_vcpu0() as the replacement for alloc_dom0_vcpu(). Then have
>>> alloc_dom_vcpu0() call dom0_set_affinity() when the boot domain is the control
>>> domain, otherwise set the affinity to auto.
>>>
>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>> ---
>>>   xen/arch/x86/dom0_build.c                 |  4 +---
>>>   xen/arch/x86/domain-builder/domain.c      | 11 +++++++++++
>>>   xen/arch/x86/include/asm/dom0_build.h     |  2 ++
>>>   xen/arch/x86/include/asm/domain-builder.h |  1 +
>>>   xen/arch/x86/setup.c                      |  5 +++--
>>>   5 files changed, 18 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/domain-builder/domain.c b/xen/arch/x86/domain-builder/domain.c
>>> index f2277b9e3cf3..619d36ea0b87 100644
>>> --- a/xen/arch/x86/domain-builder/domain.c
>>> +++ b/xen/arch/x86/domain-builder/domain.c
>>> @@ -9,6 +9,7 @@
>>>   #include <xen/sched.h>
>>>   
>>>   #include <asm/bootinfo.h>
>>> +#include <asm/dom0_build.h>
>>>   
>>>   unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>>>   {
>>> @@ -27,6 +28,16 @@ unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>>>       return bd->max_vcpus;
>>>   }
>>>   
>>> +struct vcpu *__init alloc_dom_vcpu0(struct boot_domain *bd)
>>> +{
>>> +    if ( bd->capabilities & BUILD_CAPS_CONTROL )
>>> +        dom0_set_affinity(bd->d);
>> 
>> Similar as before, this probably wants to be DOMAIN_CAPS_HARDWARE?
>> 
>> I'll adjust while rebasing.
>
> Does it?
>
> v/r,
> dps

The situation is similar later on when choosing a CPU policy. Why
mustn't the hardware domain get the same treatment as the control
domains? Using (DOMAIN_CAPS_CONTROL | DOMAIN_CAPS_HARDWARE) at the
very least seems warranted.

All these cases single-out dom0 when dom0 is both a control and a
hardware domain, but as Jason mentioned how is Xen meant to deal with
dom0_X arguments when dom0 is disaggregated? Either it applies to all
its constituents (with the plausible exception of a xenstore domain), or
just one (the hardware domain), or none. Only applying to control
domains and not the hardware domain doesn't look right (to me).

Cheers,
Alejandro

On 28.04.2025 12:33, Alejandro Vallejo wrote:
> On Fri Apr 25, 2025 at 11:04 PM BST, Daniel P. Smith wrote:
>> On 4/25/25 11:22, Alejandro Vallejo wrote:
>>> On Sat Apr 19, 2025 at 11:07 PM BST, Daniel P. Smith wrote:
>>>> Convert alloc_dom0_vcpu0() to dom0_set_affinity(), making it only set up the
>>>> node affinity based on command line parameters passed. At the same time,
>>>> introduce alloc_dom_vcpu0() as the replacement for alloc_dom0_vcpu(). Then have
>>>> alloc_dom_vcpu0() call dom0_set_affinity() when the boot domain is the control
>>>> domain, otherwise set the affinity to auto.
>>>>
>>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>>> ---
>>>>   xen/arch/x86/dom0_build.c                 |  4 +---
>>>>   xen/arch/x86/domain-builder/domain.c      | 11 +++++++++++
>>>>   xen/arch/x86/include/asm/dom0_build.h     |  2 ++
>>>>   xen/arch/x86/include/asm/domain-builder.h |  1 +
>>>>   xen/arch/x86/setup.c                      |  5 +++--
>>>>   5 files changed, 18 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/xen/arch/x86/domain-builder/domain.c b/xen/arch/x86/domain-builder/domain.c
>>>> index f2277b9e3cf3..619d36ea0b87 100644
>>>> --- a/xen/arch/x86/domain-builder/domain.c
>>>> +++ b/xen/arch/x86/domain-builder/domain.c
>>>> @@ -9,6 +9,7 @@
>>>>   #include <xen/sched.h>
>>>>   
>>>>   #include <asm/bootinfo.h>
>>>> +#include <asm/dom0_build.h>
>>>>   
>>>>   unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>>>>   {
>>>> @@ -27,6 +28,16 @@ unsigned int __init dom_max_vcpus(struct boot_domain *bd)
>>>>       return bd->max_vcpus;
>>>>   }
>>>>   
>>>> +struct vcpu *__init alloc_dom_vcpu0(struct boot_domain *bd)
>>>> +{
>>>> +    if ( bd->capabilities & BUILD_CAPS_CONTROL )
>>>> +        dom0_set_affinity(bd->d);
>>>
>>> Similar as before, this probably wants to be DOMAIN_CAPS_HARDWARE?
>>>
>>> I'll adjust while rebasing.
>>
>> Does it?
>>
>> v/r,
>> dps
> 
> The situation is similar later on when choosing a CPU policy. Why
> mustn't the hardware domain get the same treatment as the control
> domains? Using (DOMAIN_CAPS_CONTROL | DOMAIN_CAPS_HARDWARE) at the
> very least seems warranted.
> 
> All these cases single-out dom0 when dom0 is both a control and a
> hardware domain, but as Jason mentioned how is Xen meant to deal with
> dom0_X arguments when dom0 is disaggregated? Either it applies to all
> its constituents (with the plausible exception of a xenstore domain),

This one-fits-all seems very unlikely to me to make sense, while

> or just one (the hardware domain), or none.

... either of these would. "None" in particular might if all config
information is coming from e.g. DT anyway in such an setup.

> Only applying to control
> domains and not the hardware domain doesn't look right (to me).

+1

Jan