Re: [Qemu-devel] [PATCH v1] target/arm/arm-powerctl: mask the cpuid with affinity bits when get cpu

[Peter Maydell]

On Tue, 30 Apr 2019 at 04:22, Yang Chuanlong <address@hidden> wrote:
>
> Currently, the cpuid passed from the device tree may still contain
> non-affinity fields, which will cause arm_set_cpu_on failure.
> Therefore, we mask the cpuid with affinity fields here to
> improve qemu compatibility.
>
> Signed-off-by: Yang Chuanlong <address@hidden>
> ---
>  target/arm/arm-powerctl.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c
> index f77a950db6..ef9fec0b4d 100644
> --- a/target/arm/arm-powerctl.c
> +++ b/target/arm/arm-powerctl.c
> @@ -31,7 +31,13 @@ CPUState *arm_get_cpu_by_id(uint64_t id)
>  {
>      CPUState *cpu;
>
> -    DPRINTF("cpu %" PRId64 "\n", id);
> +#ifdef TARGET_AARCH64
> +    id &= ARM64_AFFINITY_MASK;
> +#else
> +    id &= ARM32_AFFINITY_MASK;
> +#endif
> +
> +    DPRINTF("cpu %" PRId64 " after mask affinity\n", id);
>
>      CPU_FOREACH(cpu) {
>          ARMCPU *armcpu = ARM_CPU(cpu);

Hi -- could you explain what the code path is where we
end up passing a wrong value into this function? I'm
wondering if this is the best place to fix it, or if maybe
the calling function is at fault.

Also, you can't use #ifdef TARGET_AARCH64 here to distinguish
a 64-bit from a 32-bit CPU, because the qemu-system-aarch64
binary (which is built with TARGET_AARCH64) also supports all
the 32-bit CPUs.

thanks
-- PMM