[PATCH v2 0/1] riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

[PATCH v2 0/1] riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

[Daniel Henrique Barboza]

Hi,

In this v2 'reg_width' was renamed to 'bitsize' to provide a bit more
clarity about what's the value type of the variable. It is the same name
used by riscv_gen_dynamic_csr_feature() for a variable that has the same
purpose. The variable rename was suggested by Alex in v1.

Changes from v1:
- rename 'reg_width' to 'bitsize'
- v1 link: https://lore.kernel.org/qemu-riscv/20240516171010.639591-1-dbarboza@ventanamicro.com/

Daniel Henrique Barboza (1):
  riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

 target/riscv/gdbstub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.44.0

[PATCH v2 1/1] riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

[Daniel Henrique Barboza]

Commit 33a24910ae changed 'reg_width' to use 'vlenb', i.e. vector length
in bytes, when in this context we want 'reg_width' as the length in
bits.

Fix 'reg_width' back to the value in bits like 7cb59921c05a
("target/riscv/gdbstub.c: use 'vlenb' instead of shifting 'vlen'") set
beforehand.

While we're at it, rename 'reg_width' to 'bitsize' to provide a bit more
clarity about what the variable represents. 'bitsize' is also used in
riscv_gen_dynamic_csr_feature() with the same purpose, i.e. as an input to
gdb_feature_builder_append_reg().

Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
Cc: Alex Bennée <alex.bennee@linaro.org>
Reported-by: Robin Dapp <rdapp.gcc@gmail.com>
Fixes: 33a24910ae ("target/riscv: Use GDBFeature for dynamic XML")
Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
---
 target/riscv/gdbstub.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index d0cc5762c2..c07df972f1 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -288,7 +288,7 @@ static GDBFeature *riscv_gen_dynamic_csr_feature(CPUState *cs, int base_reg)
 static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs, int base_reg)
 {
     RISCVCPU *cpu = RISCV_CPU(cs);
-    int reg_width = cpu->cfg.vlenb;
+    int bitsize = cpu->cfg.vlenb << 3;
     GDBFeatureBuilder builder;
     int i;
 
@@ -298,7 +298,7 @@ static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs, int base_reg)
 
     /* First define types and totals in a whole VL */
     for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
-        int count = reg_width / vec_lanes[i].size;
+        int count = bitsize / vec_lanes[i].size;
         gdb_feature_builder_append_tag(
             &builder, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
             vec_lanes[i].id, vec_lanes[i].gdb_type, count);
@@ -316,7 +316,7 @@ static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs, int base_reg)
     /* Define vector registers */
     for (i = 0; i < 32; i++) {
         gdb_feature_builder_append_reg(&builder, g_strdup_printf("v%d", i),
-                                       reg_width, i, "riscv_vector", "vector");
+                                       bitsize, i, "riscv_vector", "vector");
     }
 
     gdb_feature_builder_end(&builder);
-- 
2.44.0

Re: [PATCH v2 1/1] riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

[Akihiko Odaki]

On 2024/05/18 5:30, Daniel Henrique Barboza wrote:
> Commit 33a24910ae changed 'reg_width' to use 'vlenb', i.e. vector length
> in bytes, when in this context we want 'reg_width' as the length in
> bits.
> 
> Fix 'reg_width' back to the value in bits like 7cb59921c05a
> ("target/riscv/gdbstub.c: use 'vlenb' instead of shifting 'vlen'") set
> beforehand.
> 
> While we're at it, rename 'reg_width' to 'bitsize' to provide a bit more
> clarity about what the variable represents. 'bitsize' is also used in
> riscv_gen_dynamic_csr_feature() with the same purpose, i.e. as an input to
> gdb_feature_builder_append_reg().
> 
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Robin Dapp <rdapp.gcc@gmail.com>
> Fixes: 33a24910ae ("target/riscv: Use GDBFeature for dynamic XML")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Acked-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Re: [PATCH v2 1/1] riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

[Alistair Francis]

On Sat, May 18, 2024 at 6:32 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Commit 33a24910ae changed 'reg_width' to use 'vlenb', i.e. vector length
> in bytes, when in this context we want 'reg_width' as the length in
> bits.
>
> Fix 'reg_width' back to the value in bits like 7cb59921c05a
> ("target/riscv/gdbstub.c: use 'vlenb' instead of shifting 'vlen'") set
> beforehand.
>
> While we're at it, rename 'reg_width' to 'bitsize' to provide a bit more
> clarity about what the variable represents. 'bitsize' is also used in
> riscv_gen_dynamic_csr_feature() with the same purpose, i.e. as an input to
> gdb_feature_builder_append_reg().
>
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Reported-by: Robin Dapp <rdapp.gcc@gmail.com>
> Fixes: 33a24910ae ("target/riscv: Use GDBFeature for dynamic XML")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> Acked-by: Alex Bennée <alex.bennee@linaro.org>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/gdbstub.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index d0cc5762c2..c07df972f1 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -288,7 +288,7 @@ static GDBFeature *riscv_gen_dynamic_csr_feature(CPUState *cs, int base_reg)
>  static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs, int base_reg)
>  {
>      RISCVCPU *cpu = RISCV_CPU(cs);
> -    int reg_width = cpu->cfg.vlenb;
> +    int bitsize = cpu->cfg.vlenb << 3;
>      GDBFeatureBuilder builder;
>      int i;
>
> @@ -298,7 +298,7 @@ static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs, int base_reg)
>
>      /* First define types and totals in a whole VL */
>      for (i = 0; i < ARRAY_SIZE(vec_lanes); i++) {
> -        int count = reg_width / vec_lanes[i].size;
> +        int count = bitsize / vec_lanes[i].size;
>          gdb_feature_builder_append_tag(
>              &builder, "<vector id=\"%s\" type=\"%s\" count=\"%d\"/>",
>              vec_lanes[i].id, vec_lanes[i].gdb_type, count);
> @@ -316,7 +316,7 @@ static GDBFeature *ricsv_gen_dynamic_vector_feature(CPUState *cs, int base_reg)
>      /* Define vector registers */
>      for (i = 0; i < 32; i++) {
>          gdb_feature_builder_append_reg(&builder, g_strdup_printf("v%d", i),
> -                                       reg_width, i, "riscv_vector", "vector");
> +                                       bitsize, i, "riscv_vector", "vector");
>      }
>
>      gdb_feature_builder_end(&builder);
> --
> 2.44.0
>
>

Re: [PATCH v2 0/1] riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

[Alistair Francis]

On Sat, May 18, 2024 at 6:32 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi,
>
> In this v2 'reg_width' was renamed to 'bitsize' to provide a bit more
> clarity about what's the value type of the variable. It is the same name
> used by riscv_gen_dynamic_csr_feature() for a variable that has the same
> purpose. The variable rename was suggested by Alex in v1.
>
> Changes from v1:
> - rename 'reg_width' to 'bitsize'
> - v1 link: https://lore.kernel.org/qemu-riscv/20240516171010.639591-1-dbarboza@ventanamicro.com/
>
> Daniel Henrique Barboza (1):
>   riscv, gdbstub.c: fix reg_width in ricsv_gen_dynamic_vector_feature()

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/gdbstub.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> --
> 2.44.0
>
>