* [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters
@ 2023-12-11 17:07 Daniel Henrique Barboza
2023-12-11 17:38 ` Philippe Mathieu-Daudé
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Daniel Henrique Barboza @ 2023-12-11 17:07 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Daniel Henrique Barboza
mvendorid is an uint32 property, mimpid/marchid are uint64 properties.
But their getters are returning bools. The reason this went under the
radar for this long is because we have no code using the getters.
The problem can be seem via the 'qom-get' API though. Launching QEMU
with the 'veyron-v1' CPU, a model with:
VEYRON_V1_MVENDORID: 0x61f (1567)
VEYRON_V1_MIMPID: 0x111 (273)
VEYRON_V1_MARCHID: 0x8000000000010000 (9223372036854841344)
This is what the API returns when retrieving these properties:
(qemu) qom-get /machine/soc0/harts[0] mvendorid
true
(qemu) qom-get /machine/soc0/harts[0] mimpid
true
(qemu) qom-get /machine/soc0/harts[0] marchid
true
After this patch:
(qemu) qom-get /machine/soc0/harts[0] mvendorid
1567
(qemu) qom-get /machine/soc0/harts[0] mimpid
273
(qemu) qom-get /machine/soc0/harts[0] marchid
9223372036854841344
Fixes: 1e34150045 ("target/riscv/cpu.c: restrict 'mvendorid' value")
Fixes: a1863ad368 ("target/riscv/cpu.c: restrict 'mimpid' value")
Fixes: d6a427e2c0 ("target/riscv/cpu.c: restrict 'marchid' value")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
target/riscv/cpu.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 83c7c0cf07..70bf10aa7c 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1573,9 +1573,9 @@ static void cpu_set_mvendorid(Object *obj, Visitor *v, const char *name,
static void cpu_get_mvendorid(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- bool value = RISCV_CPU(obj)->cfg.mvendorid;
+ uint32_t value = RISCV_CPU(obj)->cfg.mvendorid;
- visit_type_bool(v, name, &value, errp);
+ visit_type_uint32(v, name, &value, errp);
}
static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
@@ -1602,9 +1602,9 @@ static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
static void cpu_get_mimpid(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- bool value = RISCV_CPU(obj)->cfg.mimpid;
+ uint64_t value = RISCV_CPU(obj)->cfg.mimpid;
- visit_type_bool(v, name, &value, errp);
+ visit_type_uint64(v, name, &value, errp);
}
static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
@@ -1652,9 +1652,9 @@ static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
- bool value = RISCV_CPU(obj)->cfg.marchid;
+ uint64_t value = RISCV_CPU(obj)->cfg.marchid;
- visit_type_bool(v, name, &value, errp);
+ visit_type_uint64(v, name, &value, errp);
}
static void riscv_cpu_class_init(ObjectClass *c, void *data)
--
2.41.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters
2023-12-11 17:07 [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters Daniel Henrique Barboza
@ 2023-12-11 17:38 ` Philippe Mathieu-Daudé
2023-12-17 23:09 ` Alistair Francis
2023-12-17 23:31 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-12-11 17:38 UTC (permalink / raw)
To: Daniel Henrique Barboza, qemu-devel
Cc: qemu-riscv, alistair.francis, bmeng, liwei1518, zhiwei_liu,
palmer, ajones, Markus Armbruster
On 11/12/23 18:07, Daniel Henrique Barboza wrote:
> mvendorid is an uint32 property, mimpid/marchid are uint64 properties.
> But their getters are returning bools. The reason this went under the
> radar for this long is because we have no code using the getters.
>
> The problem can be seem via the 'qom-get' API though. Launching QEMU
> with the 'veyron-v1' CPU, a model with:
>
> VEYRON_V1_MVENDORID: 0x61f (1567)
> VEYRON_V1_MIMPID: 0x111 (273)
> VEYRON_V1_MARCHID: 0x8000000000010000 (9223372036854841344)
>
> This is what the API returns when retrieving these properties:
>
> (qemu) qom-get /machine/soc0/harts[0] mvendorid
> true
> (qemu) qom-get /machine/soc0/harts[0] mimpid
> true
> (qemu) qom-get /machine/soc0/harts[0] marchid
> true
>
> After this patch:
>
> (qemu) qom-get /machine/soc0/harts[0] mvendorid
> 1567
> (qemu) qom-get /machine/soc0/harts[0] mimpid
> 273
> (qemu) qom-get /machine/soc0/harts[0] marchid
> 9223372036854841344
>
> Fixes: 1e34150045 ("target/riscv/cpu.c: restrict 'mvendorid' value")
> Fixes: a1863ad368 ("target/riscv/cpu.c: restrict 'mimpid' value")
> Fixes: d6a427e2c0 ("target/riscv/cpu.c: restrict 'marchid' value")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> target/riscv/cpu.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 83c7c0cf07..70bf10aa7c 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1573,9 +1573,9 @@ static void cpu_set_mvendorid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_mvendorid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.mvendorid;
> + uint32_t value = RISCV_CPU(obj)->cfg.mvendorid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint32(v, name, &value, errp);
> }
>
> static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
> @@ -1602,9 +1602,9 @@ static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_mimpid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.mimpid;
> + uint64_t value = RISCV_CPU(obj)->cfg.mimpid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint64(v, name, &value, errp);
> }
>
> static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
> @@ -1652,9 +1652,9 @@ static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.marchid;
> + uint64_t value = RISCV_CPU(obj)->cfg.marchid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint64(v, name, &value, errp);
> }
>
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters
2023-12-11 17:07 [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters Daniel Henrique Barboza
2023-12-11 17:38 ` Philippe Mathieu-Daudé
@ 2023-12-17 23:09 ` Alistair Francis
2023-12-17 23:31 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2023-12-17 23:09 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, ajones
On Tue, Dec 12, 2023 at 3:08 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> mvendorid is an uint32 property, mimpid/marchid are uint64 properties.
> But their getters are returning bools. The reason this went under the
> radar for this long is because we have no code using the getters.
>
> The problem can be seem via the 'qom-get' API though. Launching QEMU
> with the 'veyron-v1' CPU, a model with:
>
> VEYRON_V1_MVENDORID: 0x61f (1567)
> VEYRON_V1_MIMPID: 0x111 (273)
> VEYRON_V1_MARCHID: 0x8000000000010000 (9223372036854841344)
>
> This is what the API returns when retrieving these properties:
>
> (qemu) qom-get /machine/soc0/harts[0] mvendorid
> true
> (qemu) qom-get /machine/soc0/harts[0] mimpid
> true
> (qemu) qom-get /machine/soc0/harts[0] marchid
> true
>
> After this patch:
>
> (qemu) qom-get /machine/soc0/harts[0] mvendorid
> 1567
> (qemu) qom-get /machine/soc0/harts[0] mimpid
> 273
> (qemu) qom-get /machine/soc0/harts[0] marchid
> 9223372036854841344
>
> Fixes: 1e34150045 ("target/riscv/cpu.c: restrict 'mvendorid' value")
> Fixes: a1863ad368 ("target/riscv/cpu.c: restrict 'mimpid' value")
> Fixes: d6a427e2c0 ("target/riscv/cpu.c: restrict 'marchid' value")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 83c7c0cf07..70bf10aa7c 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1573,9 +1573,9 @@ static void cpu_set_mvendorid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_mvendorid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.mvendorid;
> + uint32_t value = RISCV_CPU(obj)->cfg.mvendorid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint32(v, name, &value, errp);
> }
>
> static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
> @@ -1602,9 +1602,9 @@ static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_mimpid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.mimpid;
> + uint64_t value = RISCV_CPU(obj)->cfg.mimpid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint64(v, name, &value, errp);
> }
>
> static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
> @@ -1652,9 +1652,9 @@ static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.marchid;
> + uint64_t value = RISCV_CPU(obj)->cfg.marchid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint64(v, name, &value, errp);
> }
>
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters
2023-12-11 17:07 [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters Daniel Henrique Barboza
2023-12-11 17:38 ` Philippe Mathieu-Daudé
2023-12-17 23:09 ` Alistair Francis
@ 2023-12-17 23:31 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2023-12-17 23:31 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-riscv, alistair.francis, bmeng, liwei1518,
zhiwei_liu, palmer, ajones
On Tue, Dec 12, 2023 at 3:08 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> mvendorid is an uint32 property, mimpid/marchid are uint64 properties.
> But their getters are returning bools. The reason this went under the
> radar for this long is because we have no code using the getters.
>
> The problem can be seem via the 'qom-get' API though. Launching QEMU
> with the 'veyron-v1' CPU, a model with:
>
> VEYRON_V1_MVENDORID: 0x61f (1567)
> VEYRON_V1_MIMPID: 0x111 (273)
> VEYRON_V1_MARCHID: 0x8000000000010000 (9223372036854841344)
>
> This is what the API returns when retrieving these properties:
>
> (qemu) qom-get /machine/soc0/harts[0] mvendorid
> true
> (qemu) qom-get /machine/soc0/harts[0] mimpid
> true
> (qemu) qom-get /machine/soc0/harts[0] marchid
> true
>
> After this patch:
>
> (qemu) qom-get /machine/soc0/harts[0] mvendorid
> 1567
> (qemu) qom-get /machine/soc0/harts[0] mimpid
> 273
> (qemu) qom-get /machine/soc0/harts[0] marchid
> 9223372036854841344
>
> Fixes: 1e34150045 ("target/riscv/cpu.c: restrict 'mvendorid' value")
> Fixes: a1863ad368 ("target/riscv/cpu.c: restrict 'mimpid' value")
> Fixes: d6a427e2c0 ("target/riscv/cpu.c: restrict 'marchid' value")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/cpu.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 83c7c0cf07..70bf10aa7c 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1573,9 +1573,9 @@ static void cpu_set_mvendorid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_mvendorid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.mvendorid;
> + uint32_t value = RISCV_CPU(obj)->cfg.mvendorid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint32(v, name, &value, errp);
> }
>
> static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
> @@ -1602,9 +1602,9 @@ static void cpu_set_mimpid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_mimpid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.mimpid;
> + uint64_t value = RISCV_CPU(obj)->cfg.mimpid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint64(v, name, &value, errp);
> }
>
> static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
> @@ -1652,9 +1652,9 @@ static void cpu_set_marchid(Object *obj, Visitor *v, const char *name,
> static void cpu_get_marchid(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> - bool value = RISCV_CPU(obj)->cfg.marchid;
> + uint64_t value = RISCV_CPU(obj)->cfg.marchid;
>
> - visit_type_bool(v, name, &value, errp);
> + visit_type_uint64(v, name, &value, errp);
> }
>
> static void riscv_cpu_class_init(ObjectClass *c, void *data)
> --
> 2.41.0
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-17 23:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-11 17:07 [PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters Daniel Henrique Barboza
2023-12-11 17:38 ` Philippe Mathieu-Daudé
2023-12-17 23:09 ` Alistair Francis
2023-12-17 23:31 ` Alistair Francis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).