[PATCH] target/riscv: use xlen in forging isa string

[PATCH] target/riscv: use xlen in forging isa string

[Frédéric Pétrot]

Since we now have xlen in misa, let's not use TARGET_LONG_BITS while
forging the isa string, and use instead riscv_cpu_mxl_bits.

Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
---
 target/riscv/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0c774056c5..0644b3843e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -984,7 +984,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
     int i;
     const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts);
     char *isa_str = g_new(char, maxlen);
-    char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
+    char *p = isa_str + snprintf(isa_str, maxlen, "rv%lu",
+                                 riscv_cpu_mxl_bits(&cpu->env));
     for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
         if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
             *p++ = qemu_tolower(riscv_single_letter_exts[i]);
-- 
2.35.1

[PATCH] target/riscv: use xlen in forging isa string

[Frédéric Pétrot]

Since we now have xlen in misa, let's not use TARGET_LONG_BITS while
forging the isa string, and use instead riscv_cpu_mxl_bits.

Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
---
 target/riscv/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0c774056c5..0644b3843e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -984,7 +984,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
     int i;
     const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts);
     char *isa_str = g_new(char, maxlen);
-    char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
+    char *p = isa_str + snprintf(isa_str, maxlen, "rv%lu",
+                                 riscv_cpu_mxl_bits(&cpu->env));
     for (i = 0; i < sizeof(riscv_single_letter_exts) - 1; i++) {
         if (cpu->env.misa_ext & RV(riscv_single_letter_exts[i])) {
             *p++ = qemu_tolower(riscv_single_letter_exts[i]);
-- 
2.35.1

[PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub

[Frédéric Pétrot]

Now that we have misa xlen, use that in riscv gdbstub.c instead of the
TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of
bits in a consistent way.

Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
---
 target/riscv/gdbstub.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 9ed049c29e..1602f76d2b 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
     CPURISCVState *env = &cpu->env;
     GString *s = g_string_new(NULL);
     riscv_csr_predicate_fn predicate;
-    int bitsize = 16 << env->misa_mxl_max;
+    int bitsize = riscv_cpu_mxl_bits(env);
     int i;
 
     /* Until gdb knows about 128-bit registers */
@@ -385,10 +385,11 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
 
     for (i = 0; i < 7; i++) {
         g_string_append_printf(s,
-                               "<reg name=\"%s\" bitsize=\"%d\""
+                               "<reg name=\"%s\" bitsize=\"%lu\""
                                " regnum=\"%d\" group=\"vector\""
                                " type=\"int\"/>",
-                               vector_csrs[i], TARGET_LONG_BITS, base_reg++);
+                               vector_csrs[i], riscv_cpu_mxl_bits(&cpu->env),
+                               base_reg++);
         num_regs++;
     }
 
-- 
2.35.1

[PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub

[Frédéric Pétrot]

Now that we have misa xlen, use that in riscv gdbstub.c instead of the
TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of
bits in a consistent way.

Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
---
 target/riscv/gdbstub.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
index 9ed049c29e..1602f76d2b 100644
--- a/target/riscv/gdbstub.c
+++ b/target/riscv/gdbstub.c
@@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
     CPURISCVState *env = &cpu->env;
     GString *s = g_string_new(NULL);
     riscv_csr_predicate_fn predicate;
-    int bitsize = 16 << env->misa_mxl_max;
+    int bitsize = riscv_cpu_mxl_bits(env);
     int i;
 
     /* Until gdb knows about 128-bit registers */
@@ -385,10 +385,11 @@ static int ricsv_gen_dynamic_vector_xml(CPUState *cs, int base_reg)
 
     for (i = 0; i < 7; i++) {
         g_string_append_printf(s,
-                               "<reg name=\"%s\" bitsize=\"%d\""
+                               "<reg name=\"%s\" bitsize=\"%lu\""
                                " regnum=\"%d\" group=\"vector\""
                                " type=\"int\"/>",
-                               vector_csrs[i], TARGET_LONG_BITS, base_reg++);
+                               vector_csrs[i], riscv_cpu_mxl_bits(&cpu->env),
+                               base_reg++);
         num_regs++;
     }
 
-- 
2.35.1

Re: [PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub

[Richard Henderson]

On 4/9/22 02:46, Frédéric Pétrot wrote:
> Now that we have misa xlen, use that in riscv gdbstub.c instead of the
> TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of
> bits in a consistent way.
> 
> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> ---
>   target/riscv/gdbstub.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 9ed049c29e..1602f76d2b 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>       CPURISCVState *env = &cpu->env;
>       GString *s = g_string_new(NULL);
>       riscv_csr_predicate_fn predicate;
> -    int bitsize = 16 << env->misa_mxl_max;
> +    int bitsize = riscv_cpu_mxl_bits(env);

This isn't correct, changing from using max to current mxl.

You might think this is ok, because this will run up in startup, but it really runs 
whenever gdb attaches to the stub.  Which could be anytime.


r~

Re: [PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub

[Richard Henderson]

On 4/9/22 02:46, Frédéric Pétrot wrote:
> Now that we have misa xlen, use that in riscv gdbstub.c instead of the
> TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of
> bits in a consistent way.
> 
> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> ---
>   target/riscv/gdbstub.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
> index 9ed049c29e..1602f76d2b 100644
> --- a/target/riscv/gdbstub.c
> +++ b/target/riscv/gdbstub.c
> @@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int base_reg)
>       CPURISCVState *env = &cpu->env;
>       GString *s = g_string_new(NULL);
>       riscv_csr_predicate_fn predicate;
> -    int bitsize = 16 << env->misa_mxl_max;
> +    int bitsize = riscv_cpu_mxl_bits(env);

This isn't correct, changing from using max to current mxl.

You might think this is ok, because this will run up in startup, but it really runs 
whenever gdb attaches to the stub.  Which could be anytime.


r~

Re: [PATCH] target/riscv: use xlen in forging isa string

[Richard Henderson]

On 4/9/22 02:46, Frédéric Pétrot wrote:
> Since we now have xlen in misa, let's not use TARGET_LONG_BITS while
> forging the isa string, and use instead riscv_cpu_mxl_bits.
> 
> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> ---
>   target/riscv/cpu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0c774056c5..0644b3843e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -984,7 +984,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
>       int i;
>       const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts);
>       char *isa_str = g_new(char, maxlen);
> -    char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
> +    char *p = isa_str + snprintf(isa_str, maxlen, "rv%lu",
> +                                 riscv_cpu_mxl_bits(&cpu->env));

The fact that you need to use %lu here means riscv_cpu_mxl_bits needs fixing: use of 
unsigned long is always a mistake in QEMU.  Either int is fine (as in this case), or you 
need uint64_t and ULL.


r~

Re: [PATCH] target/riscv: use xlen in forging isa string

[Richard Henderson]

On 4/9/22 02:46, Frédéric Pétrot wrote:
> Since we now have xlen in misa, let's not use TARGET_LONG_BITS while
> forging the isa string, and use instead riscv_cpu_mxl_bits.
> 
> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
> ---
>   target/riscv/cpu.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0c774056c5..0644b3843e 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -984,7 +984,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
>       int i;
>       const size_t maxlen = sizeof("rv128") + sizeof(riscv_single_letter_exts);
>       char *isa_str = g_new(char, maxlen);
> -    char *p = isa_str + snprintf(isa_str, maxlen, "rv%d", TARGET_LONG_BITS);
> +    char *p = isa_str + snprintf(isa_str, maxlen, "rv%lu",
> +                                 riscv_cpu_mxl_bits(&cpu->env));

The fact that you need to use %lu here means riscv_cpu_mxl_bits needs fixing: use of 
unsigned long is always a mistake in QEMU.  Either int is fine (as in this case), or you 
need uint64_t and ULL.


r~

Re: [PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub

[Frédéric Pétrot]

Le 09/04/2022 à 17:39, Richard Henderson a écrit :
> On 4/9/22 02:46, Frédéric Pétrot wrote:
>> Now that we have misa xlen, use that in riscv gdbstub.c instead of the
>> TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of
>> bits in a consistent way.
>>
>> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
>> ---
>>   target/riscv/gdbstub.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
>> index 9ed049c29e..1602f76d2b 100644
>> --- a/target/riscv/gdbstub.c
>> +++ b/target/riscv/gdbstub.c
>> @@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int 
>> base_reg)
>>       CPURISCVState *env = &cpu->env;
>>       GString *s = g_string_new(NULL);
>>       riscv_csr_predicate_fn predicate;
>> -    int bitsize = 16 << env->misa_mxl_max;
>> +    int bitsize = riscv_cpu_mxl_bits(env);
> 
> This isn't correct, changing from using max to current mxl.
> 
> You might think this is ok, because this will run up in startup, but it really 
> runs whenever gdb attaches to the stub.  Which could be anytime.

   I guess it should then be gdb responsibility to check mxl/sxl/uxl and act
   accordingly.
   I'll introduce a new macro then.
   Thanks,
   Frédéric

> 
> 
> r~

-- 
+---------------------------------------------------------------------------+
| Frédéric Pétrot, Pr. Grenoble INP-Ensimag/TIMA,   Ensimag deputy director |
| Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70      Ad augusta  per angusta |
| http://tima.univ-grenoble-alpes.fr frederic.petrot@univ-grenoble-alpes.fr |
+---------------------------------------------------------------------------+

Re: [PATCH] target/riscv: replace TARGET_LONG_BITS in gdbstub

[Frédéric Pétrot]

Le 09/04/2022 à 17:39, Richard Henderson a écrit :
> On 4/9/22 02:46, Frédéric Pétrot wrote:
>> Now that we have misa xlen, use that in riscv gdbstub.c instead of the
>> TARGET_LONG_BITS define, and use riscv_cpu_mxl_bits to provide the number of
>> bits in a consistent way.
>>
>> Signed-off-by: Frédéric Pétrot <frederic.petrot@univ-grenoble-alpes.fr>
>> ---
>>   target/riscv/gdbstub.c | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/riscv/gdbstub.c b/target/riscv/gdbstub.c
>> index 9ed049c29e..1602f76d2b 100644
>> --- a/target/riscv/gdbstub.c
>> +++ b/target/riscv/gdbstub.c
>> @@ -305,7 +305,7 @@ static int riscv_gen_dynamic_csr_xml(CPUState *cs, int 
>> base_reg)
>>       CPURISCVState *env = &cpu->env;
>>       GString *s = g_string_new(NULL);
>>       riscv_csr_predicate_fn predicate;
>> -    int bitsize = 16 << env->misa_mxl_max;
>> +    int bitsize = riscv_cpu_mxl_bits(env);
> 
> This isn't correct, changing from using max to current mxl.
> 
> You might think this is ok, because this will run up in startup, but it really 
> runs whenever gdb attaches to the stub.  Which could be anytime.

   I guess it should then be gdb responsibility to check mxl/sxl/uxl and act
   accordingly.
   I'll introduce a new macro then.
   Thanks,
   Frédéric

> 
> 
> r~

-- 
+---------------------------------------------------------------------------+
| Frédéric Pétrot, Pr. Grenoble INP-Ensimag/TIMA,   Ensimag deputy director |
| Mob/Pho: +33 6 74 57 99 65/+33 4 76 57 48 70      Ad augusta  per angusta |
| http://tima.univ-grenoble-alpes.fr frederic.petrot@univ-grenoble-alpes.fr |
+---------------------------------------------------------------------------+