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. For
ID_AA64DFR0 we accidentally used 'u', meaning that the top 32 bits of
this 64-bit ID register would end up always zero. 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 <peter.maydell@linaro.org>
---
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
On Thu, Apr 23, 2020 at 1:09 PM Peter Maydell <peter.maydell@linaro.org> 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. For > ID_AA64DFR0 we accidentally used 'u', meaning that the top 32 bits of > this 64-bit ID register would end up always zero. 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 <peter.maydell@linaro.org> Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com> Thanks, Laurent > --- > 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 > >
On Thu, Apr 23, 2020 at 1:09 PM Peter Maydell <peter.maydell@linaro.org> 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 <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > 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 > >
Patchew URL: https://patchew.org/QEMU/20200423110915.10527-1-peter.maydell@linaro.org/ Hi, This series seems to have some coding style problems. See output below for more information: Subject: [PATCH] target/arm: Use correct variable for setting 'max' cpu's ID_AA64DFR0 Message-id: 20200423110915.10527-1-peter.maydell@linaro.org Type: series === TEST SCRIPT BEGIN === #!/bin/bash git rev-parse base > /dev/null || exit 0 git config --local diff.renamelimit 0 git config --local diff.renames True git config --local diff.algorithm histogram ./scripts/checkpatch.pl --mailback base.. === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 error: RPC failed; result=22, HTTP code = 504 fatal: The remote end hung up unexpectedly error: Could not fetch 3c8cf5a9c21ff8782164d1def7f44bd888713384 Traceback (most recent call last): File "patchew-tester/src/patchew-cli", line 521, in test_one git_clone_repo(clone, r["repo"], r["head"], logf, True) File "patchew-tester/src/patchew-cli", line 48, in git_clone_repo stdout=logf, stderr=logf) File "/opt/rh/rh-python36/root/usr/lib64/python3.6/subprocess.py", line 291, in check_call raise CalledProcessError(retcode, cmd) subprocess.CalledProcessError: Command '['git', 'remote', 'add', '-f', '--mirror=fetch', '3c8cf5a9c21ff8782164d1def7f44bd888713384', 'https://github.com/patchew-project/qemu']' returned non-zero exit status 1. The full log is available at http://patchew.org/logs/20200423110915.10527-1-peter.maydell@linaro.org/testing.checkpatch/?type=message. --- Email generated automatically by Patchew [https://patchew.org/]. Please send your feedback to patchew-devel@redhat.com
© 2016 - 2024 Red Hat, Inc.