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.ragiada...@amd.com>
> Signed-off-by: Stefano Stabellini <stefano.stabell...@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
