[PATCH for-9.0] target/riscv/cpu.c: fix machine IDs getters

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [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).