MIDR_EL1 a 64-bit system register with the top 32-bit being RES0.
This fixes when compiling with -Werror=conversion:
target/arm/cpu64.c: In function ‘aarch64_max_initfn’:
target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion]
628 | cpu->midr = t;
| ^
Suggested-by: Laurent Desnogues <laurent.desnogues@gmail.com>
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Since v2: Do not use RESERVED bits.
Since v1: Follow Laurent and Peter suggestion.
---
target/arm/cpu.h | 2 +-
target/arm/cpu.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8b9f2961ba..592fb217d6 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -894,7 +894,7 @@ struct ARMCPU {
uint64_t id_aa64dfr0;
uint64_t id_aa64dfr1;
} isar;
- uint32_t midr;
+ uint64_t midr;
uint32_t revidr;
uint32_t reset_fpsid;
uint32_t ctr;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a79f233b17..7ff80894b6 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2757,7 +2757,7 @@ static const ARMCPUInfo arm_cpus[] = {
static Property arm_cpu_properties[] = {
DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false),
DEFINE_PROP_UINT32("psci-conduit", ARMCPU, psci_conduit, 0),
- DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0),
+ DEFINE_PROP_UINT64("midr", ARMCPU, midr, 0),
DEFINE_PROP_UINT64("mp-affinity", ARMCPU,
mp_affinity, ARM64_AFFINITY_INVALID),
DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID),
--
2.21.1
On Tue, Apr 28, 2020 at 7:26 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > MIDR_EL1 a 64-bit system register with the top 32-bit being RES0. > > This fixes when compiling with -Werror=conversion: > > target/arm/cpu64.c: In function ‘aarch64_max_initfn’: > target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion] > 628 | cpu->midr = t; > | ^ > > Suggested-by: Laurent Desnogues <laurent.desnogues@gmail.com> > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com> Thanks, Laurent > --- > Since v2: Do not use RESERVED bits. > Since v1: Follow Laurent and Peter suggestion. > --- > target/arm/cpu.h | 2 +- > target/arm/cpu.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/target/arm/cpu.h b/target/arm/cpu.h > index 8b9f2961ba..592fb217d6 100644 > --- a/target/arm/cpu.h > +++ b/target/arm/cpu.h > @@ -894,7 +894,7 @@ struct ARMCPU { > uint64_t id_aa64dfr0; > uint64_t id_aa64dfr1; > } isar; > - uint32_t midr; > + uint64_t midr; > uint32_t revidr; > uint32_t reset_fpsid; > uint32_t ctr; > diff --git a/target/arm/cpu.c b/target/arm/cpu.c > index a79f233b17..7ff80894b6 100644 > --- a/target/arm/cpu.c > +++ b/target/arm/cpu.c > @@ -2757,7 +2757,7 @@ static const ARMCPUInfo arm_cpus[] = { > static Property arm_cpu_properties[] = { > DEFINE_PROP_BOOL("start-powered-off", ARMCPU, start_powered_off, false), > DEFINE_PROP_UINT32("psci-conduit", ARMCPU, psci_conduit, 0), > - DEFINE_PROP_UINT32("midr", ARMCPU, midr, 0), > + DEFINE_PROP_UINT64("midr", ARMCPU, midr, 0), > DEFINE_PROP_UINT64("mp-affinity", ARMCPU, > mp_affinity, ARM64_AFFINITY_INVALID), > DEFINE_PROP_INT32("node-id", ARMCPU, node_id, CPU_UNSET_NUMA_NODE_ID), > -- > 2.21.1 >
On Tue, 28 Apr 2020 at 18:26, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > > MIDR_EL1 a 64-bit system register with the top 32-bit being RES0. > > This fixes when compiling with -Werror=conversion: > > target/arm/cpu64.c: In function ‘aarch64_max_initfn’: > target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion] > 628 | cpu->midr = t; > | ^ > > Suggested-by: Laurent Desnogues <laurent.desnogues@gmail.com> > Suggested-by: Peter Maydell <peter.maydell@linaro.org> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Applied to target-arm.next, with the commit message fixed up to match the patch contents: target/arm: Use uint64_t for midr field in CPU state struct MIDR_EL1 is a 64-bit system register with the top 32-bit being RES0. Represent it in QEMU's ARMCPU struct with a uint64_t, not a uint32_t. This fixes an error when compiling with -Werror=conversion because we were manipulating the register value using a local uint64_t variable: target/arm/cpu64.c: In function ‘aarch64_max_initfn’: target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t ’ {aka ‘unsigned int’} may change value [-Werror=conversion] 628 | cpu->midr = t; | ^ and future-proofs us against a possible future architecture change using some of the top 32 bits. thanks -- PMM
On 4/30/20 5:59 PM, Peter Maydell wrote: > On Tue, 28 Apr 2020 at 18:26, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> >> MIDR_EL1 a 64-bit system register with the top 32-bit being RES0. >> >> This fixes when compiling with -Werror=conversion: >> >> target/arm/cpu64.c: In function ‘aarch64_max_initfn’: >> target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ {aka ‘long unsigned int’} to ‘uint32_t’ {aka ‘unsigned int’} may change value [-Werror=conversion] >> 628 | cpu->midr = t; >> | ^ >> >> Suggested-by: Laurent Desnogues <laurent.desnogues@gmail.com> >> Suggested-by: Peter Maydell <peter.maydell@linaro.org> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > > Applied to target-arm.next, with the commit message fixed > up to match the patch contents: > > target/arm: Use uint64_t for midr field in CPU state struct > > MIDR_EL1 is a 64-bit system register with the top 32-bit being RES0. > Represent it in QEMU's ARMCPU struct with a uint64_t, not a > uint32_t. > > This fixes an error when compiling with -Werror=conversion > because we were manipulating the register value using a > local uint64_t variable: > > target/arm/cpu64.c: In function ‘aarch64_max_initfn’: > target/arm/cpu64.c:628:21: error: conversion from ‘uint64_t’ > {aka ‘long unsigned int’} to ‘uint32_t ’ {aka ‘unsigned int’} may > change value [-Werror=conversion] > 628 | cpu->midr = t; > | ^ > > and future-proofs us against a possible future architecture > change using some of the top 32 bits. Thanks Peter for updating the description. > > thanks > -- PMM >
© 2016 - 2024 Red Hat, Inc.