* [PATCH v3] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
@ 2020-04-28 17:26 Philippe Mathieu-Daudé
2020-04-29 6:17 ` Laurent Desnogues
2020-04-30 15:59 ` Peter Maydell
0 siblings, 2 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-28 17:26 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Desnogues, Peter Maydell, qemu-arm, Philippe Mathieu-Daudé
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
2020-04-28 17:26 [PATCH v3] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1 Philippe Mathieu-Daudé
@ 2020-04-29 6:17 ` Laurent Desnogues
2020-04-30 15:59 ` Peter Maydell
1 sibling, 0 replies; 4+ messages in thread
From: Laurent Desnogues @ 2020-04-29 6:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Peter Maydell, qemu-arm, qemu-devel
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
2020-04-28 17:26 [PATCH v3] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1 Philippe Mathieu-Daudé
2020-04-29 6:17 ` Laurent Desnogues
@ 2020-04-30 15:59 ` Peter Maydell
2020-04-30 16:46 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 4+ messages in thread
From: Peter Maydell @ 2020-04-30 15:59 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Laurent Desnogues, qemu-arm, QEMU Developers
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v3] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1
2020-04-30 15:59 ` Peter Maydell
@ 2020-04-30 16:46 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-30 16:46 UTC (permalink / raw)
To: Peter Maydell; +Cc: Laurent Desnogues, qemu-arm, QEMU Developers
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-30 16:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-28 17:26 [PATCH v3] target/arm: Use correct variable for setting 'max' cpu's MIDR_EL1 Philippe Mathieu-Daudé
2020-04-29 6:17 ` Laurent Desnogues
2020-04-30 15:59 ` Peter Maydell
2020-04-30 16:46 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.