| Message ID | alpine.DEB.2.22.394.2502141615050.3858257@ubuntu-linux-20-04-desktop (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] xen/dom0less: support for vcpu affinity | expand |
On 15/02/2025 01:17, Stefano Stabellini wrote: > > > Add vcpu affinity to the dom0less bindings. Example: > > dom1 { > ... > cpus = <4>; > vcpu0 { > compatible = "xen,vcpu-affinity"; > id = <0>; > hard-affinity = "4-7"; > }; > vcpu1 { > compatible = "xen,vcpu-affinity"; > id = <1>; > hard-affinity = "0-3"; > }; > vcpu2 { > compatible = "xen,vcpu-affinity"; > id = <2>; > hard-affinity = "1,6"; > }; > ... > } What is this indentation for? It reads strangely. > > Note that the property hard-affinity is optional. It is possible to add If it's optional, then this node does not make sense anymore... > other properties in the future not only to specify soft affinity, but > also to provide more precise methods for configuring affinity. For > instance, on ARM the MPIDR could be use to specify the pCPU. For now, it > is left to the future. > > Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com> > Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> > --- > Changes in v2: > - code style > - add binding description to docs/misc/arm/device-tree/booting.txt > --- > > docs/misc/arm/device-tree/booting.txt | 21 +++++++++++ > xen/arch/arm/dom0less-build.c | 51 +++++++++++++++++++++++++++ > 2 files changed, 72 insertions(+) > > diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt > index 9c881baccc..6a2abbef4e 100644 > --- a/docs/misc/arm/device-tree/booting.txt > +++ b/docs/misc/arm/device-tree/booting.txt > @@ -324,6 +324,27 @@ The ramdisk sub-node has the following properties: > property because it will be created by the UEFI stub on boot. > This option is needed only when UEFI boot is used. > > +Under the "xen,domain" compatible node, it is possible optionally to add > +one or more sub-nodes to configure vCPU affinity. The vCPU affinity > +sub-node has the following properties: > + > +- compatible > + > + "xen,vcpu-affinity" > + > +- id > + > + A 32-bit integer that specifies the vCPU id. 0 is the first vCPU. > + The last vCPU is cpus-1, where "cpus" is the number of vCPUs > + specified with the "cpus" property under the "xen,domain" node. > + > +- hard-affinity > + > + Optional. A string specifying the hard affinity configuration for the > + vCPU: a comma-separated list of pCPUs or ranges of pCPUs is used. > + Ranges are hyphen-separated intervals (such as `0-4`) and are inclusive > + on both sides. I think users should know what this number refers to. > + > > Example > ======= > diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c > index 49d1f14d65..35d02689e7 100644 > --- a/xen/arch/arm/dom0less-build.c > +++ b/xen/arch/arm/dom0less-build.c > @@ -818,6 +818,8 @@ void __init create_domUs(void) > const struct dt_device_node *cpupool_node, > *chosen = dt_find_node_by_path("/chosen"); > const char *llc_colors_str = NULL; > + const char *hard_affinity_str = NULL; > + struct dt_device_node *np; > > BUG_ON(chosen == NULL); > dt_for_each_child_node(chosen, node) > @@ -992,6 +994,55 @@ void __init create_domUs(void) > if ( rc ) > panic("Could not set up domain %s (rc = %d)\n", > dt_node_name(node), rc); > + > + dt_for_each_child_node(node, np) Can we please move this to a separate function? create_domUs starts to be difficult to parse due to its length. It will also fix 80chars limit issues. > + { > + const char *s; > + struct vcpu *v; > + cpumask_t affinity; > + > + if ( !dt_device_is_compatible(np, "xen,vcpu-affinity") ) > + continue; > + > + if ( !dt_property_read_u32(np, "id", &val) ) > + continue; empty line here please > + if ( val >= d->max_vcpus ) > + panic("Invalid vcpu_id %u for domain %s\n", val, dt_node_name(node)); > + > + v = d->vcpu[val]; > + rc = dt_property_read_string(np, "hard-affinity", &hard_affinity_str); > + if ( rc < 0 ) > + continue; > + > + s = hard_affinity_str; > + cpumask_clear(&affinity); > + while ( *s != '\0' ) > + { > + unsigned int start, end; > + > + start = simple_strtoul(s, &s, 0); > + > + if ( *s == '-' ) /* Range */ > + { > + s++; > + end = simple_strtoul(s, &s, 0); > + } > + else /* Single value */ > + end = start; > + > + for ( ; start <= end; start++ ) > + cpumask_set_cpu(start, &affinity); What if the pCPU number is invalid? Then we rely on debug ASSERT. I think we should panic here on invalid number. > + > + if ( *s == ',' ) > + s++; > + else if ( *s != '\0' ) > + break; > + } > + > + rc = vcpu_set_hard_affinity(v, &affinity); > + if ( rc ) > + panic("vcpu%d: failed to set hard affinity\n", v->vcpu_id); > + } > } > } > > -- > 2.25.1 > ~Michal
On Mon, 17 Feb 2025, Orzel, Michal wrote: > On 15/02/2025 01:17, Stefano Stabellini wrote: > > > > > > Add vcpu affinity to the dom0less bindings. Example: > > > > dom1 { > > ... > > cpus = <4>; > > vcpu0 { > > compatible = "xen,vcpu-affinity"; > > id = <0>; > > hard-affinity = "4-7"; > > }; > > vcpu1 { > > compatible = "xen,vcpu-affinity"; > > id = <1>; > > hard-affinity = "0-3"; > > }; > > vcpu2 { > > compatible = "xen,vcpu-affinity"; > > id = <2>; > > hard-affinity = "1,6"; > > }; > > ... > > } > What is this indentation for? It reads strangely. > > > > > Note that the property hard-affinity is optional. It is possible to add > If it's optional, then this node does not make sense anymore... The idea is that at least one of the ways to specify affinity should be present, otherwise like you wrote, it is useless. But the property itself I think should be marked as optional (otherwise it would have to be always present.) I addressed all other comments, I'll send a v3.
diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt index 9c881baccc..6a2abbef4e 100644 --- a/docs/misc/arm/device-tree/booting.txt +++ b/docs/misc/arm/device-tree/booting.txt @@ -324,6 +324,27 @@ The ramdisk sub-node has the following properties: property because it will be created by the UEFI stub on boot. This option is needed only when UEFI boot is used. +Under the "xen,domain" compatible node, it is possible optionally to add +one or more sub-nodes to configure vCPU affinity. The vCPU affinity +sub-node has the following properties: + +- compatible + + "xen,vcpu-affinity" + +- id + + A 32-bit integer that specifies the vCPU id. 0 is the first vCPU. + The last vCPU is cpus-1, where "cpus" is the number of vCPUs + specified with the "cpus" property under the "xen,domain" node. + +- hard-affinity + + Optional. A string specifying the hard affinity configuration for the + vCPU: a comma-separated list of pCPUs or ranges of pCPUs is used. + Ranges are hyphen-separated intervals (such as `0-4`) and are inclusive + on both sides. + Example ======= diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c index 49d1f14d65..35d02689e7 100644 --- a/xen/arch/arm/dom0less-build.c +++ b/xen/arch/arm/dom0less-build.c @@ -818,6 +818,8 @@ void __init create_domUs(void) const struct dt_device_node *cpupool_node, *chosen = dt_find_node_by_path("/chosen"); const char *llc_colors_str = NULL; + const char *hard_affinity_str = NULL; + struct dt_device_node *np; BUG_ON(chosen == NULL); dt_for_each_child_node(chosen, node) @@ -992,6 +994,55 @@ void __init create_domUs(void) if ( rc ) panic("Could not set up domain %s (rc = %d)\n", dt_node_name(node), rc); + + dt_for_each_child_node(node, np) + { + const char *s; + struct vcpu *v; + cpumask_t affinity; + + if ( !dt_device_is_compatible(np, "xen,vcpu-affinity") ) + continue; + + if ( !dt_property_read_u32(np, "id", &val) ) + continue; + if ( val >= d->max_vcpus ) + panic("Invalid vcpu_id %u for domain %s\n", val, dt_node_name(node)); + + v = d->vcpu[val]; + rc = dt_property_read_string(np, "hard-affinity", &hard_affinity_str); + if ( rc < 0 ) + continue; + + s = hard_affinity_str; + cpumask_clear(&affinity); + while ( *s != '\0' ) + { + unsigned int start, end; + + start = simple_strtoul(s, &s, 0); + + if ( *s == '-' ) /* Range */ + { + s++; + end = simple_strtoul(s, &s, 0); + } + else /* Single value */ + end = start; + + for ( ; start <= end; start++ ) + cpumask_set_cpu(start, &affinity); + + if ( *s == ',' ) + s++; + else if ( *s != '\0' ) + break; + } + + rc = vcpu_set_hard_affinity(v, &affinity); + if ( rc ) + panic("vcpu%d: failed to set hard affinity\n", v->vcpu_id); + } } }
Add vcpu affinity to the dom0less bindings. Example: dom1 { ... cpus = <4>; vcpu0 { compatible = "xen,vcpu-affinity"; id = <0>; hard-affinity = "4-7"; }; vcpu1 { compatible = "xen,vcpu-affinity"; id = <1>; hard-affinity = "0-3"; }; vcpu2 { compatible = "xen,vcpu-affinity"; id = <2>; hard-affinity = "1,6"; }; ... } Note that the property hard-affinity is optional. It is possible to add other properties in the future not only to specify soft affinity, but also to provide more precise methods for configuring affinity. For instance, on ARM the MPIDR could be use to specify the pCPU. For now, it is left to the future. Signed-off-by: Xenia Ragiadakou <xenia.ragiadakou@amd.com> Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com> --- Changes in v2: - code style - add binding description to docs/misc/arm/device-tree/booting.txt --- docs/misc/arm/device-tree/booting.txt | 21 +++++++++++ xen/arch/arm/dom0less-build.c | 51 +++++++++++++++++++++++++++ 2 files changed, 72 insertions(+)