[PATCH 14/15] x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree

Daniel P. Smith posted 15 patches 1 month ago
There is a newer version of this series
[PATCH 14/15] x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree
Posted by Daniel P. Smith 1 month ago
Introduce the `cpus` property, named as such for dom0less compatibility, that
represents the maximum number of vpcus to allocate for a domain. In the device
tree, it will be encoded as a u32 value.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
 xen/arch/x86/dom0_build.c             |  3 +++
 xen/arch/x86/domain_builder/fdt.c     | 12 ++++++++++++
 xen/arch/x86/include/asm/bootdomain.h |  2 ++
 3 files changed, 17 insertions(+)

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 1c3b7ff0e658..7ff052016bfd 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -617,6 +617,9 @@ int __init construct_dom0(struct boot_domain *bd)
     if ( !get_memsize(&dom0_max_size, LONG_MAX) && bd->max_pages )
         dom0_size.nr_pages = bd->max_pages;
 
+    if ( opt_dom0_max_vcpus_max == UINT_MAX && bd->max_vcpus )
+        opt_dom0_max_vcpus_max = bd->max_vcpus;
+
     if ( is_hvm_domain(d) )
         rc = dom0_construct_pvh(bd);
     else if ( is_pv_domain(d) )
diff --git a/xen/arch/x86/domain_builder/fdt.c b/xen/arch/x86/domain_builder/fdt.c
index b8ace5c18c6a..d24e265f2378 100644
--- a/xen/arch/x86/domain_builder/fdt.c
+++ b/xen/arch/x86/domain_builder/fdt.c
@@ -197,6 +197,18 @@ static int __init process_domain_node(
             bd->max_pages = PFN_DOWN(kb * SZ_1K);
             printk("  max memory: %ld\n", bd->max_pages << PAGE_SHIFT);
         }
+        if ( match_fdt_property(fdt, prop, "cpus" ) )
+        {
+            uint32_t val = UINT_MAX;
+            if ( fdt_prop_as_u32(prop, &val) != 0 )
+            {
+                printk("  failed processing max_vcpus for domain %s\n",
+                       name == NULL ? "unknown" : name);
+                return -EINVAL;
+            }
+            bd->max_vcpus = val;
+            printk("  max vcpus: %d\n", bd->max_vcpus);
+        }
     }
 
     fdt_for_each_subnode(node, fdt, dom_node)
diff --git a/xen/arch/x86/include/asm/bootdomain.h b/xen/arch/x86/include/asm/bootdomain.h
index 9a5ba2931665..d144d6173400 100644
--- a/xen/arch/x86/include/asm/bootdomain.h
+++ b/xen/arch/x86/include/asm/bootdomain.h
@@ -28,6 +28,8 @@ struct boot_domain {
     unsigned long min_pages;
     unsigned long max_pages;
 
+    unsigned int max_vcpus;
+
     struct boot_module *kernel;
     struct boot_module *ramdisk;
 
-- 
2.30.2
Re: [PATCH 14/15] x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree
Posted by Jan Beulich 3 weeks, 3 days ago
On 23.11.2024 19:20, Daniel P. Smith wrote:
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -617,6 +617,9 @@ int __init construct_dom0(struct boot_domain *bd)
>      if ( !get_memsize(&dom0_max_size, LONG_MAX) && bd->max_pages )
>          dom0_size.nr_pages = bd->max_pages;
>  
> +    if ( opt_dom0_max_vcpus_max == UINT_MAX && bd->max_vcpus )
> +        opt_dom0_max_vcpus_max = bd->max_vcpus;

Isn't this kind of backwards? I.e. aren't you meaning to move us towards
boot-domains?

Also, what about the counterpart opt_dom0_max_vcpus_min? That wants to be
controllable from DT too, I would think?

Jan
Re: [PATCH 14/15] x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree
Posted by Daniel P. Smith 2 weeks, 1 day ago
On 12/2/24 07:19, Jan Beulich wrote:
> On 23.11.2024 19:20, Daniel P. Smith wrote:
>> --- a/xen/arch/x86/dom0_build.c
>> +++ b/xen/arch/x86/dom0_build.c
>> @@ -617,6 +617,9 @@ int __init construct_dom0(struct boot_domain *bd)
>>       if ( !get_memsize(&dom0_max_size, LONG_MAX) && bd->max_pages )
>>           dom0_size.nr_pages = bd->max_pages;
>>   
>> +    if ( opt_dom0_max_vcpus_max == UINT_MAX && bd->max_vcpus )
>> +        opt_dom0_max_vcpus_max = bd->max_vcpus;
> 
> Isn't this kind of backwards? I.e. aren't you meaning to move us towards
> boot-domains?

Prior to domain builder, available construction parameters for dom0 were 
exposed as command line parameters. This allowed for boot-time 
adjustments to the parameters. With domain builder, there are now two 
sources for dom0 construction parameters. Those coming from the device 
tree and those coming from the command line. For most x86 platforms, the 
device tree parameters can only be constructed prior to booting Xen. 
Whereas the command line parameters allow boot-time adjustments, at 
least for dom0. That is the thinking at least. Now if there is interest 
in being able to retire the command line options, that would definitely 
simplify things.

> Also, what about the counterpart opt_dom0_max_vcpus_min? That wants to be
> controllable from DT too, I would think?

Yes, in theory we will eventually be able to do requested/min/max as 
well as cpu pinning/affinity. For now it was requested we focus on 
implementing only requested vcpus.

v/r,
dps
Re: [PATCH 14/15] x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree
Posted by Jan Beulich 2 weeks ago
On 11.12.2024 20:49, Daniel P. Smith wrote:
> On 12/2/24 07:19, Jan Beulich wrote:
>> On 23.11.2024 19:20, Daniel P. Smith wrote:
>>> --- a/xen/arch/x86/dom0_build.c
>>> +++ b/xen/arch/x86/dom0_build.c
>>> @@ -617,6 +617,9 @@ int __init construct_dom0(struct boot_domain *bd)
>>>       if ( !get_memsize(&dom0_max_size, LONG_MAX) && bd->max_pages )
>>>           dom0_size.nr_pages = bd->max_pages;
>>>   
>>> +    if ( opt_dom0_max_vcpus_max == UINT_MAX && bd->max_vcpus )
>>> +        opt_dom0_max_vcpus_max = bd->max_vcpus;
>>
>> Isn't this kind of backwards? I.e. aren't you meaning to move us towards
>> boot-domains?
> 
> Prior to domain builder, available construction parameters for dom0 were 
> exposed as command line parameters. This allowed for boot-time 
> adjustments to the parameters. With domain builder, there are now two 
> sources for dom0 construction parameters. Those coming from the device 
> tree and those coming from the command line. For most x86 platforms, the 
> device tree parameters can only be constructed prior to booting Xen. 
> Whereas the command line parameters allow boot-time adjustments, at 
> least for dom0. That is the thinking at least. Now if there is interest 
> in being able to retire the command line options, that would definitely 
> simplify things.

No, retiring command line options is out of question imo. Yet that also
wasn't my point. Instead I was wondering why we wouldn't make bd->* the
ultimate source of truth. However, ...

>> Also, what about the counterpart opt_dom0_max_vcpus_min? That wants to be
>> controllable from DT too, I would think?
> 
> Yes, in theory we will eventually be able to do requested/min/max as 
> well as cpu pinning/affinity. For now it was requested we focus on 
> implementing only requested vcpus.

... that's pretty much only a reasonable option if these were converted
at the same time, to avoid becoming inconsistent for perhaps an extended
period of time.

Jan
Re: [PATCH 14/15] x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree
Posted by Jason Andryuk 1 month ago
On 2024-11-23 13:20, Daniel P. Smith wrote:
> Introduce the `cpus` property, named as such for dom0less compatibility, that
> represents the maximum number of vpcus to allocate for a domain. In the device
> tree, it will be encoded as a u32 value.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---

> diff --git a/xen/arch/x86/domain_builder/fdt.c b/xen/arch/x86/domain_builder/fdt.c
> index b8ace5c18c6a..d24e265f2378 100644
> --- a/xen/arch/x86/domain_builder/fdt.c
> +++ b/xen/arch/x86/domain_builder/fdt.c
> @@ -197,6 +197,18 @@ static int __init process_domain_node(
>               bd->max_pages = PFN_DOWN(kb * SZ_1K);
>               printk("  max memory: %ld\n", bd->max_pages << PAGE_SHIFT);
>           }
> +        if ( match_fdt_property(fdt, prop, "cpus" ) )

I think I forgot to mention it on earlier ones, but I think all these 
match_fdt_property() should be chained together with "else if".

With that

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

> +        {
> +            uint32_t val = UINT_MAX;
> +            if ( fdt_prop_as_u32(prop, &val) != 0 )
> +            {
> +                printk("  failed processing max_vcpus for domain %s\n",
> +                       name == NULL ? "unknown" : name);
> +                return -EINVAL;
> +            }
> +            bd->max_vcpus = val;
> +            printk("  max vcpus: %d\n", bd->max_vcpus);
> +        }
>       }
>   
>       fdt_for_each_subnode(node, fdt, dom_node)
Re: [PATCH 14/15] x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree
Posted by Daniel P. Smith 2 weeks, 1 day ago
On 11/25/24 19:05, Jason Andryuk wrote:
> On 2024-11-23 13:20, Daniel P. Smith wrote:
>> Introduce the `cpus` property, named as such for dom0less 
>> compatibility, that
>> represents the maximum number of vpcus to allocate for a domain. In 
>> the device
>> tree, it will be encoded as a u32 value.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> ---
> 
>> diff --git a/xen/arch/x86/domain_builder/fdt.c b/xen/arch/x86/ 
>> domain_builder/fdt.c
>> index b8ace5c18c6a..d24e265f2378 100644
>> --- a/xen/arch/x86/domain_builder/fdt.c
>> +++ b/xen/arch/x86/domain_builder/fdt.c
>> @@ -197,6 +197,18 @@ static int __init process_domain_node(
>>               bd->max_pages = PFN_DOWN(kb * SZ_1K);
>>               printk("  max memory: %ld\n", bd->max_pages << PAGE_SHIFT);
>>           }
>> +        if ( match_fdt_property(fdt, prop, "cpus" ) )
> 
> I think I forgot to mention it on earlier ones, but I think all these 
> match_fdt_property() should be chained together with "else if".

Ack.

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

Thanks!

v/r,
dps