Re: [PATCH] target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0

[Philippe Mathieu-Daudé]

On Thu, Apr 23, 2020 at 1:09 PM Peter Maydell <address@hidden> wrote:
>
> In aarch64_max_initfn() we update both 32-bit and 64-bit ID
> registers.  The intended pattern is that for 64-bit ID registers we
> use FIELD_DP64 and the uint64_t 't' register, while 32-bit ID
> registers use FIELD_DP32 and the uint32_t 'u' register.

Variable names could be improved...

>  For
> ID_AA64DFR0 we accidentally used 'u', meaning that the top 32 bits of
> this 64-bit ID register would end up always zero.

-Wconversion CPPFLAG helps but there are so many places to fix that we
are not using it:

target/arm/cpu64.c:711:13: error: conversion from ‘uint64_t’ {aka
‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change
value [-Werror=conversion]
  711 |         u = cpu->isar.id_aa64dfr0;
      |             ^~~
target/arm/cpu64.c:712:13: note: in expansion of macro ‘FIELD_DP64’
  712 |         u = FIELD_DP64(u, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
      |             ^~~~~~~~~~

>  Luckily at the
> moment that's what they should be anyway, so this bug has no visible
> effects.
>
> Use the right-sized variable.
>
> Fixes: 3bec78447a958d481991
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  target/arm/cpu64.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
> index 95d0c8c101a..4c7105ea1a1 100644
> --- a/target/arm/cpu64.c
> +++ b/target/arm/cpu64.c
> @@ -708,9 +708,9 @@ static void aarch64_max_initfn(Object *obj)
>          u = FIELD_DP32(u, ID_MMFR4, CNP, 1); /* TTCNP */
>          cpu->isar.id_mmfr4 = u;
>
> -        u = cpu->isar.id_aa64dfr0;
> -        u = FIELD_DP64(u, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
> -        cpu->isar.id_aa64dfr0 = u;
> +        t = cpu->isar.id_aa64dfr0;
> +        t = FIELD_DP64(t, ID_AA64DFR0, PMUVER, 5); /* v8.4-PMU */
> +        cpu->isar.id_aa64dfr0 = t;
>
>          u = cpu->isar.id_dfr0;
>          u = FIELD_DP32(u, ID_DFR0, PERFMON, 5); /* v8.4-PMU */
> --
> 2.20.1
>
>