Re: [Xen-devel] [PATCH V2 7/8] tools/libxl: Set logical CPUID in DT node equal to MPIDR for domU

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Chen,

On 23/05/2015 15:52, Chen Baozi wrote:
> From: Chen Baozi <baozich@xxxxxxxxx>
>
> Linux kernel sometimes uses the 'hwid' which is fetched from DT node
> of CPU as the MPIDR. We set the logical CPUID in the corresponding DT
> node to MPIDR to keep consistency.
Hmmm... this is wrong. The field "reg" in the DT contains the affinity 
part of the MPIDR.
The logical CPUID is only an internal representation in Linux which may 
not be equal to the "reg" in the DT.
> Signed-off-by: Chen Baozi <baozich@xxxxxxxxx>
> ---
>   tools/libxl/libxl_arm.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c
> index 75d2aed..6026cab 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -272,6 +272,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
> nr_cpus,
>                             const struct arch_info *ainfo)
>   {
>       int res, i;
> +    uint64_t cpu_id;
>
>       res = fdt_begin_node(fdt, "cpus");
>       if (res) return res;
> @@ -283,7 +284,13 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
> nr_cpus,
>       if (res) return res;
>
>       for (i = 0; i < nr_cpus; i++) {
> -        const char *name = GCSPRINTF("cpu@%d", i);
> +        const char *name;
> +
> +        /*
> +         * Linux kernel assumes that MPIDR is equal to logical CPUID
Wrong. See my comment above.

> +         */
> +        cpu_id = (uint64_t)((i & 0x0f) | (((i >> 4) & 0xff) << 8));
You need to add a comment explaining why we handle only AFF0 and AFF1.

> +        name = GCSPRINTF("cpu@%lx", cpu_id);
>
>           res = fdt_begin_node(fdt, name);
>           if (res) return res;
> @@ -297,7 +304,7 @@ static int make_cpus_node(libxl__gc *gc, void *fdt, int 
> nr_cpus,
>           res = fdt_property_string(fdt, "enable-method", "psci");
>           if (res) return res;
>
> -        res = fdt_property_regs(gc, fdt, 1, 0, 1, (uint64_t)i);
> +        res = fdt_property_regs(gc, fdt, 1, 0, 1, cpu_id);
>           if (res) return res;
>
>           res = fdt_end_node(fdt);
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel