All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] generalise vcpu0 creation for predefined domains
@ 2025-07-17 17:51 Alejandro Vallejo
  2025-07-17 17:51 ` [PATCH 1/2] arm: Remove alloc_dom0_vcpu0() Alejandro Vallejo
  2025-07-17 17:51 ` [PATCH 2/2] x86: generalise vcpu0 creation for a domain Alejandro Vallejo
  0 siblings, 2 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-07-17 17:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk,
	Daniel P . Smith, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD

Hi,

This patch is a very compressed version of Daniel's. Originally, it
performed code motion between dom0_build.c and domain-builder/domain.c,
but in order for this to go in early, I've compressed everything in a
single hunk and left it in dom0_build.c.

We can move whatever must be moved later on.

I could see inlining the affinity settings and removing this function
altogether (having a regular vcpu_create() in its place). I've kept it
as is for consistency with the previous patch.

See https://lore.kernel.org/xen-devel/20250515131744.3843-5-dpsmith@apertussolutions.com/

I've kept the S-by too, as this patch descends directly from that one.

Differences with Daniel's RFC:
  * Function takes a domain, as the struct already exists by then.
  * Apply dom0 cmdline overrides to control and hardware domains.
  * Keep everything in dom0_build.c for the time being.
  * Remove arm's incarnation of the function. It's just useless.

Cheers,
Alejandro

Alejandro Vallejo (2):
  arm: Remove alloc_dom0_vcpu0()
  x86: generalise vcpu0 creation for a domain

 xen/arch/arm/domain_build.c           |  7 +------
 xen/arch/x86/dom0_build.c             | 12 ++++++++----
 xen/arch/x86/include/asm/dom0_build.h |  5 +++++
 xen/arch/x86/setup.c                  |  6 ++++--
 xen/include/xen/domain.h              |  1 -
 5 files changed, 18 insertions(+), 13 deletions(-)


base-commit: 55719030b0bb0069fc8b57cd808dc98dc9d39add
-- 
2.43.0



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/2] arm: Remove alloc_dom0_vcpu0()
  2025-07-17 17:51 [PATCH 0/2] generalise vcpu0 creation for predefined domains Alejandro Vallejo
@ 2025-07-17 17:51 ` Alejandro Vallejo
  2025-07-17 21:42   ` Jason Andryuk
  2025-07-17 17:51 ` [PATCH 2/2] x86: generalise vcpu0 creation for a domain Alejandro Vallejo
  1 sibling, 1 reply; 12+ messages in thread
From: Alejandro Vallejo @ 2025-07-17 17:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Alejandro Vallejo, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk,
	Daniel P . Smith

It's a straight vcpu_create(), so the alloc_dom0_vcpu0() call is
irrelevant.

Not a functional change.

Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/arm/domain_build.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 04d3dca38a..ed668bd61c 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -115,11 +115,6 @@ unsigned int __init dom0_max_vcpus(void)
     return opt_dom0_max_vcpus;
 }
 
-struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
-{
-    return vcpu_create(dom0, 0);
-}
-
 /*
  * Insert the given pages into a memory bank, banks are ordered by address.
  *
@@ -2085,7 +2080,7 @@ void __init create_dom0(void)
     if ( llc_coloring_enabled && (rc = dom0_set_llc_colors(dom0)) )
         panic("Error initializing LLC coloring for domain 0 (rc = %d)\n", rc);
 
-    if ( alloc_dom0_vcpu0(dom0) == NULL )
+    if ( vcpu_create(dom0, 0) == NULL )
         panic("Error creating domain 0 vcpu0\n");
 
     rc = construct_dom0(dom0);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-17 17:51 [PATCH 0/2] generalise vcpu0 creation for predefined domains Alejandro Vallejo
  2025-07-17 17:51 ` [PATCH 1/2] arm: Remove alloc_dom0_vcpu0() Alejandro Vallejo
@ 2025-07-17 17:51 ` Alejandro Vallejo
  2025-07-17 22:09   ` Jason Andryuk
  2025-07-22 14:45   ` Jan Beulich
  1 sibling, 2 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-07-17 17:51 UTC (permalink / raw)
  To: xen-devel
  Cc: Alejandro Vallejo, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD, Michal Orzel, Julien Grall,
	Stefano Stabellini, Daniel P . Smith

Make alloc_dom0_vcpu0() viable as a general vcpu0 allocator. Keep
behaviour on any hwdom/ctldom identical to that dom0 used to have, and
make non-dom0 have auto node affinity.

Rename the function to alloc_dom_vcpu0() to reflect this change in
scope, and move the prototype to asm/domain.h from xen/domain.h as it's
only used in x86.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
---
 xen/arch/x86/dom0_build.c             | 12 ++++++++----
 xen/arch/x86/include/asm/dom0_build.h |  5 +++++
 xen/arch/x86/setup.c                  |  6 ++++--
 xen/include/xen/domain.h              |  1 -
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 0b467fd4a4..dfae7f888f 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -254,12 +254,16 @@ unsigned int __init dom0_max_vcpus(void)
     return max_vcpus;
 }
 
-struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
+struct vcpu *__init alloc_dom_vcpu0(struct domain *d)
 {
-    dom0->node_affinity = dom0_nodes;
-    dom0->auto_node_affinity = !dom0_nr_pxms;
+    d->auto_node_affinity = true;
+    if ( is_hardware_domain(d) || is_control_domain(d) )
+    {
+        d->node_affinity = dom0_nodes;
+        d->auto_node_affinity = !dom0_nr_pxms;
+    }
 
-    return vcpu_create(dom0, 0);
+    return vcpu_create(d, 0);
 }
 
 #ifdef CONFIG_SHADOW_PAGING
diff --git a/xen/arch/x86/include/asm/dom0_build.h b/xen/arch/x86/include/asm/dom0_build.h
index ff021c24af..46bfd111f2 100644
--- a/xen/arch/x86/include/asm/dom0_build.h
+++ b/xen/arch/x86/include/asm/dom0_build.h
@@ -23,6 +23,11 @@ unsigned long dom0_paging_pages(const struct domain *d,
 void dom0_update_physmap(bool compat, unsigned long pfn,
                          unsigned long mfn, unsigned long vphysmap_s);
 
+/* general domain construction */
+
+/* Create the first vCPU of a domain. Sets up node affinity as a side effect */
+struct vcpu *alloc_dom_vcpu0(struct domain *d);
+
 #endif	/* _DOM0_BUILD_H_ */
 
 /*
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index c6890669b9..77a8ca60c3 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -37,6 +37,7 @@
 #include <asm/bzimage.h>
 #include <asm/cpu-policy.h>
 #include <asm/desc.h>
+#include <asm/dom0_build.h>
 #include <asm/e820.h>
 #include <asm/edd.h>
 #include <asm/genapic.h>
@@ -1054,9 +1055,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(d) == NULL )
         panic("Error creating %pdv0\n", d);
 
     cmdline_size = domain_cmdline_size(bi, bd);
@@ -1093,7 +1096,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");
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index e10baf2615..bf1fc6227f 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -24,7 +24,6 @@ struct vcpu *vcpu_create(struct domain *d, unsigned int vcpu_id);
 
 unsigned int dom0_max_vcpus(void);
 int parse_arch_dom0_param(const char *s, const char *e);
-struct vcpu *alloc_dom0_vcpu0(struct domain *dom0);
 
 int vcpu_reset(struct vcpu *v);
 int vcpu_up(struct vcpu *v);
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] arm: Remove alloc_dom0_vcpu0()
  2025-07-17 17:51 ` [PATCH 1/2] arm: Remove alloc_dom0_vcpu0() Alejandro Vallejo
@ 2025-07-17 21:42   ` Jason Andryuk
  2025-07-17 23:56     ` Stefano Stabellini
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Andryuk @ 2025-07-17 21:42 UTC (permalink / raw)
  To: Alejandro Vallejo, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis, Michal Orzel,
	Volodymyr Babchuk, Daniel P . Smith

On 2025-07-17 13:51, Alejandro Vallejo wrote:
> It's a straight vcpu_create(), so the alloc_dom0_vcpu0() call is
> irrelevant.
> 
> Not a functional change.
> 
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-17 17:51 ` [PATCH 2/2] x86: generalise vcpu0 creation for a domain Alejandro Vallejo
@ 2025-07-17 22:09   ` Jason Andryuk
  2025-07-18  0:04     ` Stefano Stabellini
  2025-07-22 14:45   ` Jan Beulich
  1 sibling, 1 reply; 12+ messages in thread
From: Jason Andryuk @ 2025-07-17 22:09 UTC (permalink / raw)
  To: Alejandro Vallejo, xen-devel
  Cc: Jan Beulich, Andrew Cooper, Roger Pau Monné, Anthony PERARD,
	Michal Orzel, Julien Grall, Stefano Stabellini, Daniel P . Smith

On 2025-07-17 13:51, Alejandro Vallejo wrote:
> Make alloc_dom0_vcpu0() viable as a general vcpu0 allocator. Keep
> behaviour on any hwdom/ctldom identical to that dom0 used to have, and
> make non-dom0 have auto node affinity.
> 
> Rename the function to alloc_dom_vcpu0() to reflect this change in
> scope, and move the prototype to asm/domain.h from xen/domain.h as it's
> only used in x86.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> ---
>   xen/arch/x86/dom0_build.c             | 12 ++++++++----
>   xen/arch/x86/include/asm/dom0_build.h |  5 +++++
>   xen/arch/x86/setup.c                  |  6 ++++--
>   xen/include/xen/domain.h              |  1 -
>   4 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index 0b467fd4a4..dfae7f888f 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -254,12 +254,16 @@ unsigned int __init dom0_max_vcpus(void)
>       return max_vcpus;
>   }
>   
> -struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
> +struct vcpu *__init alloc_dom_vcpu0(struct domain *d)
>   {
> -    dom0->node_affinity = dom0_nodes;
> -    dom0->auto_node_affinity = !dom0_nr_pxms;
> +    d->auto_node_affinity = true;
> +    if ( is_hardware_domain(d) || is_control_domain(d) )

Do we want dom0 options to apply to:
hardware or control
just hardware
just dom0 (hardware && control && xenstore)

?

I think "just dom0" may make the most sense.  My next preference is just 
hardware.  Control I think should be mostly a domU except for having 
is_privileged = true;

The rest of the patch looks good.

Regards,
Jason


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] arm: Remove alloc_dom0_vcpu0()
  2025-07-17 21:42   ` Jason Andryuk
@ 2025-07-17 23:56     ` Stefano Stabellini
  0 siblings, 0 replies; 12+ messages in thread
From: Stefano Stabellini @ 2025-07-17 23:56 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: Alejandro Vallejo, xen-devel, Stefano Stabellini, Julien Grall,
	Bertrand Marquis, Michal Orzel, Volodymyr Babchuk,
	Daniel P . Smith

On Thu, 17 Jul 2025, Jason Andryuk wrote:
> On 2025-07-17 13:51, Alejandro Vallejo wrote:
> > It's a straight vcpu_create(), so the alloc_dom0_vcpu0() call is
> > irrelevant.
> > 
> > Not a functional change.
> > 
> > Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> 
> Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-17 22:09   ` Jason Andryuk
@ 2025-07-18  0:04     ` Stefano Stabellini
  2025-07-18  6:42       ` Jan Beulich
  0 siblings, 1 reply; 12+ messages in thread
From: Stefano Stabellini @ 2025-07-18  0:04 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: Alejandro Vallejo, xen-devel, Jan Beulich, Andrew Cooper,
	Roger Pau Monné, Anthony PERARD, Michal Orzel, Julien Grall,
	Stefano Stabellini, Daniel P . Smith

On Thu, 17 Jul 2025, Jason Andryuk wrote:
> On 2025-07-17 13:51, Alejandro Vallejo wrote:
> > Make alloc_dom0_vcpu0() viable as a general vcpu0 allocator. Keep
> > behaviour on any hwdom/ctldom identical to that dom0 used to have, and
> > make non-dom0 have auto node affinity.
> > 
> > Rename the function to alloc_dom_vcpu0() to reflect this change in
> > scope, and move the prototype to asm/domain.h from xen/domain.h as it's
> > only used in x86.
> > 
> > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> > Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> > ---
> >   xen/arch/x86/dom0_build.c             | 12 ++++++++----
> >   xen/arch/x86/include/asm/dom0_build.h |  5 +++++
> >   xen/arch/x86/setup.c                  |  6 ++++--
> >   xen/include/xen/domain.h              |  1 -
> >   4 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> > index 0b467fd4a4..dfae7f888f 100644
> > --- a/xen/arch/x86/dom0_build.c
> > +++ b/xen/arch/x86/dom0_build.c
> > @@ -254,12 +254,16 @@ unsigned int __init dom0_max_vcpus(void)
> >       return max_vcpus;
> >   }
> >   -struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
> > +struct vcpu *__init alloc_dom_vcpu0(struct domain *d)
> >   {
> > -    dom0->node_affinity = dom0_nodes;
> > -    dom0->auto_node_affinity = !dom0_nr_pxms;
> > +    d->auto_node_affinity = true;
> > +    if ( is_hardware_domain(d) || is_control_domain(d) )
> 
> Do we want dom0 options to apply to:
> hardware or control
> just hardware
> just dom0 (hardware && control && xenstore)
> 
> ?
> 
> I think "just dom0" may make the most sense.  My next preference is just
> hardware.  Control I think should be mostly a domU except for having
> is_privileged = true;

Great question. Certainly dom0 options, such as dom0_mem, should not
apply to Control, and certainly they should apply to regular Dom0.

The interesting question is whether they should apply to the Hardware
Domain. Some of the Dom0 options make sense for the Hardware Domain and
there isn't an equivalent config option available via Dom0less bindings.
I am not thinking about the dom0_* options but things like nmi=dom0. For
simplicity and ease of use I would say they should apply to the Hardware
Domain.

In any case, I think the strategy should apply to all architectures.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-18  0:04     ` Stefano Stabellini
@ 2025-07-18  6:42       ` Jan Beulich
  2025-07-18  9:46         ` Alejandro Vallejo
  0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2025-07-18  6:42 UTC (permalink / raw)
  To: Stefano Stabellini, Jason Andryuk
  Cc: Alejandro Vallejo, xen-devel, Andrew Cooper, Roger Pau Monné,
	Anthony PERARD, Michal Orzel, Julien Grall, Daniel P . Smith

On 18.07.2025 02:04, Stefano Stabellini wrote:
> On Thu, 17 Jul 2025, Jason Andryuk wrote:
>> On 2025-07-17 13:51, Alejandro Vallejo wrote:
>>> Make alloc_dom0_vcpu0() viable as a general vcpu0 allocator. Keep
>>> behaviour on any hwdom/ctldom identical to that dom0 used to have, and
>>> make non-dom0 have auto node affinity.
>>>
>>> Rename the function to alloc_dom_vcpu0() to reflect this change in
>>> scope, and move the prototype to asm/domain.h from xen/domain.h as it's
>>> only used in x86.
>>>
>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>>> ---
>>>   xen/arch/x86/dom0_build.c             | 12 ++++++++----
>>>   xen/arch/x86/include/asm/dom0_build.h |  5 +++++
>>>   xen/arch/x86/setup.c                  |  6 ++++--
>>>   xen/include/xen/domain.h              |  1 -
>>>   4 files changed, 17 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
>>> index 0b467fd4a4..dfae7f888f 100644
>>> --- a/xen/arch/x86/dom0_build.c
>>> +++ b/xen/arch/x86/dom0_build.c
>>> @@ -254,12 +254,16 @@ unsigned int __init dom0_max_vcpus(void)
>>>       return max_vcpus;
>>>   }
>>>   -struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
>>> +struct vcpu *__init alloc_dom_vcpu0(struct domain *d)
>>>   {
>>> -    dom0->node_affinity = dom0_nodes;
>>> -    dom0->auto_node_affinity = !dom0_nr_pxms;
>>> +    d->auto_node_affinity = true;
>>> +    if ( is_hardware_domain(d) || is_control_domain(d) )
>>
>> Do we want dom0 options to apply to:
>> hardware or control
>> just hardware
>> just dom0 (hardware && control && xenstore)
>>
>> ?
>>
>> I think "just dom0" may make the most sense.  My next preference is just
>> hardware.  Control I think should be mostly a domU except for having
>> is_privileged = true;
> 
> Great question. Certainly dom0 options, such as dom0_mem, should not
> apply to Control, and certainly they should apply to regular Dom0.
> 
> The interesting question is whether they should apply to the Hardware
> Domain. Some of the Dom0 options make sense for the Hardware Domain and
> there isn't an equivalent config option available via Dom0less bindings.
> I am not thinking about the dom0_* options but things like nmi=dom0. For
> simplicity and ease of use I would say they should apply to the Hardware
> Domain.

Interesting indeed. So far we more or less aliased hwdom == dom0.

Jan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-18  6:42       ` Jan Beulich
@ 2025-07-18  9:46         ` Alejandro Vallejo
  2025-07-22 23:48           ` Stefano Stabellini
  0 siblings, 1 reply; 12+ messages in thread
From: Alejandro Vallejo @ 2025-07-18  9:46 UTC (permalink / raw)
  To: Jan Beulich, Stefano Stabellini, Jason Andryuk
  Cc: xen-devel, Andrew Cooper, Roger Pau Monné, Anthony PERARD,
	Michal Orzel, Julien Grall, Daniel P . Smith

On Fri Jul 18, 2025 at 8:42 AM CEST, Jan Beulich wrote:
> On 18.07.2025 02:04, Stefano Stabellini wrote:
>> On Thu, 17 Jul 2025, Jason Andryuk wrote:
>>> On 2025-07-17 13:51, Alejandro Vallejo wrote:
>>>> Make alloc_dom0_vcpu0() viable as a general vcpu0 allocator. Keep
>>>> behaviour on any hwdom/ctldom identical to that dom0 used to have, and
>>>> make non-dom0 have auto node affinity.
>>>>
>>>> Rename the function to alloc_dom_vcpu0() to reflect this change in
>>>> scope, and move the prototype to asm/domain.h from xen/domain.h as it's
>>>> only used in x86.
>>>>
>>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>>> Signed-off-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
>>>> ---
>>>>   xen/arch/x86/dom0_build.c             | 12 ++++++++----
>>>>   xen/arch/x86/include/asm/dom0_build.h |  5 +++++
>>>>   xen/arch/x86/setup.c                  |  6 ++++--
>>>>   xen/include/xen/domain.h              |  1 -
>>>>   4 files changed, 17 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
>>>> index 0b467fd4a4..dfae7f888f 100644
>>>> --- a/xen/arch/x86/dom0_build.c
>>>> +++ b/xen/arch/x86/dom0_build.c
>>>> @@ -254,12 +254,16 @@ unsigned int __init dom0_max_vcpus(void)
>>>>       return max_vcpus;
>>>>   }
>>>>   -struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
>>>> +struct vcpu *__init alloc_dom_vcpu0(struct domain *d)
>>>>   {
>>>> -    dom0->node_affinity = dom0_nodes;
>>>> -    dom0->auto_node_affinity = !dom0_nr_pxms;
>>>> +    d->auto_node_affinity = true;
>>>> +    if ( is_hardware_domain(d) || is_control_domain(d) )
>>>
>>> Do we want dom0 options to apply to:
>>> hardware or control
>>> just hardware
>>> just dom0 (hardware && control && xenstore)
>>>
>>> ?
>>>
>>> I think "just dom0" may make the most sense.  My next preference is just
>>> hardware.  Control I think should be mostly a domU except for having
>>> is_privileged = true;

I could get behind just hardware, but I don't think the xenstore cap bears much
importance in this.

>> 
>> Great question. Certainly dom0 options, such as dom0_mem, should not
>> apply to Control, and certainly they should apply to regular Dom0.

But what is a regular dom0, when not booted regularly? What is it that makes
dom0 quack like a dom0 and not like a domU? It may very well be the case that
it's the regularity of dom0 that makes it a dom0, and cannot be sensibly
described in the presence of split ctl/hw domains.

>> 
>> The interesting question is whether they should apply to the Hardware
>> Domain. Some of the Dom0 options make sense for the Hardware Domain and
>> there isn't an equivalent config option available via Dom0less bindings.

Well, bindings can be easily created. nmi= in particular can be trivially
directed anywhere via a boolean binding attached to any domain.

The fact that the bindings are less granular than the dom0 cmdline due
to not being needed until now.

>> I am not thinking about the dom0_* options but things like nmi=dom0. For
>> simplicity and ease of use I would say they should apply to the Hardware
>> Domain.

That's a fun case. So when nmi=dom0 the report goes to the hwdom even if it's
not dom0 (i.e: late hwdom).

>
> Interesting indeed. So far we more or less aliased hwdom == dom0.
>
> Jan

Some arguments do want to be adjusted one way or the other spec_ctrl.c makes
heavy assumptions about there not being any hwdom/ctldom separation, and both
having domain_id == 0. There are other cases.

Yet another option is to single-out the Hyperlaunch/dom0less case and never
apply dom0 commandline overrides there (dom0_*=). It'd be a flag in
boot_domain. Might even allow us to compile them out altogether for
dom0less-only configurations (e.g: CONFIG_DOM0LESS_BOOT && !CONFIG_DOM0_BOOT, or
something like that).

Thoughts?

Cheers,
Alejandro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-17 17:51 ` [PATCH 2/2] x86: generalise vcpu0 creation for a domain Alejandro Vallejo
  2025-07-17 22:09   ` Jason Andryuk
@ 2025-07-22 14:45   ` Jan Beulich
  2025-07-22 17:29     ` Alejandro Vallejo
  1 sibling, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2025-07-22 14:45 UTC (permalink / raw)
  To: Alejandro Vallejo
  Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
	Julien Grall, Stefano Stabellini, Daniel P . Smith, xen-devel

On 17.07.2025 19:51, Alejandro Vallejo wrote:
> Make alloc_dom0_vcpu0() viable as a general vcpu0 allocator. Keep
> behaviour on any hwdom/ctldom identical to that dom0 used to have, and
> make non-dom0 have auto node affinity.
> 
> Rename the function to alloc_dom_vcpu0() to reflect this change in
> scope, and move the prototype to asm/domain.h from xen/domain.h as it's
> only used in x86.

Which makes we wonder what's really x86-specific about it. Yes, the use of
...

> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -254,12 +254,16 @@ unsigned int __init dom0_max_vcpus(void)
>      return max_vcpus;
>  }
>  
> -struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
> +struct vcpu *__init alloc_dom_vcpu0(struct domain *d)
>  {
> -    dom0->node_affinity = dom0_nodes;
> -    dom0->auto_node_affinity = !dom0_nr_pxms;
> +    d->auto_node_affinity = true;
> +    if ( is_hardware_domain(d) || is_control_domain(d) )
> +    {
> +        d->node_affinity = dom0_nodes;
> +        d->auto_node_affinity = !dom0_nr_pxms;

... dom0_nr_pxms here perhaps is. Yet that could be sorted e.g. by making
this a function parameter.

> --- a/xen/arch/x86/include/asm/dom0_build.h
> +++ b/xen/arch/x86/include/asm/dom0_build.h
> @@ -23,6 +23,11 @@ unsigned long dom0_paging_pages(const struct domain *d,
>  void dom0_update_physmap(bool compat, unsigned long pfn,
>                           unsigned long mfn, unsigned long vphysmap_s);
>  
> +/* general domain construction */

Nit: Comment style.

> @@ -1054,9 +1055,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(d) == NULL )
>          panic("Error creating %pdv0\n", d);
>  
>      cmdline_size = domain_cmdline_size(bi, bd);
> @@ -1093,7 +1096,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");

Isn't this movement of the bd->d assignment entirely unrelated?

Jan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-22 14:45   ` Jan Beulich
@ 2025-07-22 17:29     ` Alejandro Vallejo
  0 siblings, 0 replies; 12+ messages in thread
From: Alejandro Vallejo @ 2025-07-22 17:29 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
	Julien Grall, Stefano Stabellini, Daniel P . Smith, xen-devel

On Tue Jul 22, 2025 at 4:45 PM CEST, Jan Beulich wrote:
> On 17.07.2025 19:51, Alejandro Vallejo wrote:
>> Make alloc_dom0_vcpu0() viable as a general vcpu0 allocator. Keep
>> behaviour on any hwdom/ctldom identical to that dom0 used to have, and
>> make non-dom0 have auto node affinity.
>> 
>> Rename the function to alloc_dom_vcpu0() to reflect this change in
>> scope, and move the prototype to asm/domain.h from xen/domain.h as it's
>> only used in x86.
>
> Which makes we wonder what's really x86-specific about it. Yes, the use of
> ...

Mostly that it's the only arch doing it.

>
>> --- a/xen/arch/x86/dom0_build.c
>> +++ b/xen/arch/x86/dom0_build.c
>> @@ -254,12 +254,16 @@ unsigned int __init dom0_max_vcpus(void)
>>      return max_vcpus;
>>  }
>>  
>> -struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
>> +struct vcpu *__init alloc_dom_vcpu0(struct domain *d)
>>  {
>> -    dom0->node_affinity = dom0_nodes;
>> -    dom0->auto_node_affinity = !dom0_nr_pxms;
>> +    d->auto_node_affinity = true;
>> +    if ( is_hardware_domain(d) || is_control_domain(d) )
>> +    {
>> +        d->node_affinity = dom0_nodes;
>> +        d->auto_node_affinity = !dom0_nr_pxms;
>
> ... dom0_nr_pxms here perhaps is. Yet that could be sorted e.g. by making
> this a function parameter.

ARM doesn't dabble with affinities while allocating the first vcpu. It's a
straight vcpu_create(). We could definitely inline setting that affinity
setting and forego the function altogether. I'm not a big fan of functions
with non-obvious side-effects, and this is one such case.

>
>> --- a/xen/arch/x86/include/asm/dom0_build.h
>> +++ b/xen/arch/x86/include/asm/dom0_build.h
>> @@ -23,6 +23,11 @@ unsigned long dom0_paging_pages(const struct domain *d,
>>  void dom0_update_physmap(bool compat, unsigned long pfn,
>>                           unsigned long mfn, unsigned long vphysmap_s);
>>  
>> +/* general domain construction */
>
> Nit: Comment style.

Ack

>
>> @@ -1054,9 +1055,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(d) == NULL )
>>          panic("Error creating %pdv0\n", d);
>>  
>>      cmdline_size = domain_cmdline_size(bi, bd);
>> @@ -1093,7 +1096,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");
>
> Isn't this movement of the bd->d assignment entirely unrelated?
>
> Jan

The prior incarnation of this patch (see Daniel's RFC) took a boot_domain, I
think, for which the change would be required. It indeed doesn't seem to be
required any longer.

Cheers,
Alejandro


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] x86: generalise vcpu0 creation for a domain
  2025-07-18  9:46         ` Alejandro Vallejo
@ 2025-07-22 23:48           ` Stefano Stabellini
  0 siblings, 0 replies; 12+ messages in thread
From: Stefano Stabellini @ 2025-07-22 23:48 UTC (permalink / raw)
  To: Alejandro Vallejo
  Cc: Jan Beulich, Stefano Stabellini, Jason Andryuk, xen-devel,
	Andrew Cooper, Roger Pau Monné, Anthony PERARD, Michal Orzel,
	Julien Grall, Daniel P . Smith

On Fri, 18 Jul 2025, Alejandro Vallejo wrote:
> Some arguments do want to be adjusted one way or the other spec_ctrl.c makes
> heavy assumptions about there not being any hwdom/ctldom separation, and both
> having domain_id == 0. There are other cases.
> 
> Yet another option is to single-out the Hyperlaunch/dom0less case and never
> apply dom0 commandline overrides there (dom0_*=). It'd be a flag in
> boot_domain. Might even allow us to compile them out altogether for
> dom0less-only configurations (e.g: CONFIG_DOM0LESS_BOOT && !CONFIG_DOM0_BOOT, or
> something like that).
> 
> Thoughts?

I have been reviewing this in more detail, including the WIP draft that
Alejandro has not yet submitted to xen-devel, which completes the
hyperlaunch/dom0less enablement on x86. I think we should apply all
dom0 command line arguments exactly as they are to both classic Dom0 and
the hardware domain (and only to the hardware domain).

This is the simplest approach and the only one that would work with the
current code.



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-07-22 23:49 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17 17:51 [PATCH 0/2] generalise vcpu0 creation for predefined domains Alejandro Vallejo
2025-07-17 17:51 ` [PATCH 1/2] arm: Remove alloc_dom0_vcpu0() Alejandro Vallejo
2025-07-17 21:42   ` Jason Andryuk
2025-07-17 23:56     ` Stefano Stabellini
2025-07-17 17:51 ` [PATCH 2/2] x86: generalise vcpu0 creation for a domain Alejandro Vallejo
2025-07-17 22:09   ` Jason Andryuk
2025-07-18  0:04     ` Stefano Stabellini
2025-07-18  6:42       ` Jan Beulich
2025-07-18  9:46         ` Alejandro Vallejo
2025-07-22 23:48           ` Stefano Stabellini
2025-07-22 14:45   ` Jan Beulich
2025-07-22 17:29     ` Alejandro Vallejo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.