[PATCH v2 0/1] target/riscv: misa to ISA string conversion fix

[PATCH v2 0/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

[v1] https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg06350.html

S and U are misa bits but not extensions (instead, they are supported
privilege modes).  Thus, they should not be copied to the ISA string.

[CHANGES: v1 -> v2]

I also removed almost all reserved/dropped single-letter "extensions"
from the list.

-   "B": Not going to be a single-letter extension (misa.B is reserved).
-   "J": Not going to be a single-letter extension (misa.J is reserved).
-   "K": Not going to be a single-letter extension (misa.K is reserved).
-   "L": Dropped.
-   "N": Dropped.
-   "T": Dropped.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>




Tsukasa OI (1):
  target/riscv: misa to ISA string conversion fix

 target/riscv/cpu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)


base-commit: 3d31fe4d662f13c70eb7e87f29513623ccd76322
-- 
2.32.0

[PATCH v2 1/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

Some bits in RISC-V `misa' CSR should not be reflected in the ISA
string.  For instance, `S' and `U' (represents existence of supervisor
and user mode, respectively) in `misa' CSR must not be copied since
neither `S' nor `U' are valid single-letter extensions.

This commit also removes all reserved/dropped single-letter "extensions"
from the list.

-   "B": Not going to be a single-letter extension (misa.B is reserved).
-   "J": Not going to be a single-letter extension (misa.J is reserved).
-   "K": Not going to be a single-letter extension (misa.K is reserved).
-   "L": Dropped.
-   "N": Dropped.
-   "T": Dropped.

It also clarifies that the variable `riscv_single_letter_exts' is a
single-letter extension order list.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
 target/riscv/cpu.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ddda4906ff..1f68c696eb 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -34,7 +34,7 @@
 
 /* RISC-V CPU definitions */
 
-static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
+static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
 
 const char * const riscv_int_regnames[] = {
   "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
@@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
 char *riscv_isa_string(RISCVCPU *cpu)
 {
     int i;
-    const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1;
+    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);
-    for (i = 0; i < sizeof(riscv_exts); i++) {
-        if (cpu->env.misa_ext & RV(riscv_exts[i])) {
-            *p++ = qemu_tolower(riscv_exts[i]);
+    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]);
         }
     }
     *p = '\0';
-- 
2.32.0

Re: [PATCH v2 1/1] target/riscv: misa to ISA string conversion fix

[Frédéric Pétrot]

Hello,

Le 28/03/2022 à 15:11, Tsukasa OI a écrit :
> Some bits in RISC-V `misa' CSR should not be reflected in the ISA
> string.  For instance, `S' and `U' (represents existence of supervisor
> and user mode, respectively) in `misa' CSR must not be copied since
> neither `S' nor `U' are valid single-letter extensions.
> 
> This commit also removes all reserved/dropped single-letter "extensions"
> from the list.
> 
> -   "B": Not going to be a single-letter extension (misa.B is reserved).
> -   "J": Not going to be a single-letter extension (misa.J is reserved).
> -   "K": Not going to be a single-letter extension (misa.K is reserved).
> -   "L": Dropped.
> -   "N": Dropped.
> -   "T": Dropped.
> 
> It also clarifies that the variable `riscv_single_letter_exts' is a
> single-letter extension order list.
> 
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
>   target/riscv/cpu.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..1f68c696eb 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -34,7 +34,7 @@
>   
>   /* RISC-V CPU definitions */
>   
> -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>   
>   const char * const riscv_int_regnames[] = {
>     "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
> @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>   char *riscv_isa_string(RISCVCPU *cpu)
>   {
>       int i;
> -    const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1;
> +    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);

To avoid one dependency on TARGET_LONG_BITS that is not necessary now that we
have mxl, I would suggest to replace the previous line with:
      char *p = isa_str + snprintf(isa_str, maxlen, "rv%d",
                                   1 << (4 + cpu->env.misa_mxl_max));

Frédéric

Re: [PATCH v2 1/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

On 2022/03/30 1:29, Frédéric Pétrot wrote:
> Hello,
> 
> Le 28/03/2022 à 15:11, Tsukasa OI a écrit :
>> Some bits in RISC-V `misa' CSR should not be reflected in the ISA
>> string.  For instance, `S' and `U' (represents existence of supervisor
>> and user mode, respectively) in `misa' CSR must not be copied since
>> neither `S' nor `U' are valid single-letter extensions.
>>
>> This commit also removes all reserved/dropped single-letter "extensions"
>> from the list.
>>
>> -   "B": Not going to be a single-letter extension (misa.B is reserved).
>> -   "J": Not going to be a single-letter extension (misa.J is reserved).
>> -   "K": Not going to be a single-letter extension (misa.K is reserved).
>> -   "L": Dropped.
>> -   "N": Dropped.
>> -   "T": Dropped.
>>
>> It also clarifies that the variable `riscv_single_letter_exts' is a
>> single-letter extension order list.
>>
>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>> ---
>>   target/riscv/cpu.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index ddda4906ff..1f68c696eb 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -34,7 +34,7 @@
>>     /* RISC-V CPU definitions */
>>   -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
>> +static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>>     const char * const riscv_int_regnames[] = {
>>     "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
>> @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>>   char *riscv_isa_string(RISCVCPU *cpu)
>>   {
>>       int i;
>> -    const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1;
>> +    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);
> 
> To avoid one dependency on TARGET_LONG_BITS that is not necessary now that we
> have mxl, I would suggest to replace the previous line with:
>      char *p = isa_str + snprintf(isa_str, maxlen, "rv%d",
>                                   1 << (4 + cpu->env.misa_mxl_max));

LGTM except... that won't be a part of my patch (you are trying to fix
separate issue).  You can submit separate patch for this.

To comment, I like
	16 << cpu->env.misa_mxl_max
rather than
	1 << (4 + cpu->env.misa_mxl_max)
for consistency with target/riscv/gdbstub.c:
	int bitsize = 16 << env->misa_mxl_max;

Tsukasa

> 
> Frédéric
> 

Re: [PATCH v2 1/1] target/riscv: misa to ISA string conversion fix

[Frédéric Pétrot]

Le 30/03/2022 à 04:08, Tsukasa OI a écrit :
> On 2022/03/30 1:29, Frédéric Pétrot wrote:
>> Hello,
>>
>> Le 28/03/2022 à 15:11, Tsukasa OI a écrit :
>>> Some bits in RISC-V `misa' CSR should not be reflected in the ISA
>>> string.  For instance, `S' and `U' (represents existence of supervisor
>>> and user mode, respectively) in `misa' CSR must not be copied since
>>> neither `S' nor `U' are valid single-letter extensions.
>>>
>>> This commit also removes all reserved/dropped single-letter "extensions"
>>> from the list.
>>>
>>> -   "B": Not going to be a single-letter extension (misa.B is reserved).
>>> -   "J": Not going to be a single-letter extension (misa.J is reserved).
>>> -   "K": Not going to be a single-letter extension (misa.K is reserved).
>>> -   "L": Dropped.
>>> -   "N": Dropped.
>>> -   "T": Dropped.
>>>
>>> It also clarifies that the variable `riscv_single_letter_exts' is a
>>> single-letter extension order list.
>>>
>>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>>> ---
>>>    target/riscv/cpu.c | 10 +++++-----
>>>    1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index ddda4906ff..1f68c696eb 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -34,7 +34,7 @@
>>>      /* RISC-V CPU definitions */
>>>    -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
>>> +static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>>>      const char * const riscv_int_regnames[] = {
>>>      "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
>>> @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>>>    char *riscv_isa_string(RISCVCPU *cpu)
>>>    {
>>>        int i;
>>> -    const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1;
>>> +    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);
>>
>> To avoid one dependency on TARGET_LONG_BITS that is not necessary now that we
>> have mxl, I would suggest to replace the previous line with:
>>       char *p = isa_str + snprintf(isa_str, maxlen, "rv%d",
>>                                    1 << (4 + cpu->env.misa_mxl_max));
> 
> LGTM except... that won't be a part of my patch (you are trying to fix
> separate issue).  You can submit separate patch for this.

   Sure, I'll do that.

> To comment, I like
> 	16 << cpu->env.misa_mxl_max
> rather than
> 	1 << (4 + cpu->env.misa_mxl_max)
> for consistency with target/riscv/gdbstub.c:
> 	int bitsize = 16 << env->misa_mxl_max;

   Good point.
   However, perhaps I should rather change target/riscv/gdbstub.c, to better fit
   the note page 16 (no pun intended) of the priviledged spec that expresses
   bitsize as $2^{MXL+4}$.

   Frédéric

> 
> Tsukasa
> 
>>
>> Frédéric
>>

-- 
+---------------------------------------------------------------------------+
| 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 v2 1/1] target/riscv: misa to ISA string conversion fix

[Alistair Francis]

On Mon, Mar 28, 2022 at 11:11 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> Some bits in RISC-V `misa' CSR should not be reflected in the ISA
> string.  For instance, `S' and `U' (represents existence of supervisor
> and user mode, respectively) in `misa' CSR must not be copied since
> neither `S' nor `U' are valid single-letter extensions.
>
> This commit also removes all reserved/dropped single-letter "extensions"
> from the list.
>
> -   "B": Not going to be a single-letter extension (misa.B is reserved).
> -   "J": Not going to be a single-letter extension (misa.J is reserved).
> -   "K": Not going to be a single-letter extension (misa.K is reserved).

Interesting, these are still listed in "ISA Extension Naming Conventions".

They can just be re-added if they are used in the future.

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

Alistair

> -   "L": Dropped.
> -   "N": Dropped.
> -   "T": Dropped.
>
> It also clarifies that the variable `riscv_single_letter_exts' is a
> single-letter extension order list.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
>  target/riscv/cpu.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..1f68c696eb 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -34,7 +34,7 @@
>
>  /* RISC-V CPU definitions */
>
> -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>
>  const char * const riscv_int_regnames[] = {
>    "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
> @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>  char *riscv_isa_string(RISCVCPU *cpu)
>  {
>      int i;
> -    const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1;
> +    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);
> -    for (i = 0; i < sizeof(riscv_exts); i++) {
> -        if (cpu->env.misa_ext & RV(riscv_exts[i])) {
> -            *p++ = qemu_tolower(riscv_exts[i]);
> +    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]);
>          }
>      }
>      *p = '\0';
> --
> 2.32.0
>

Re: [PATCH v2 1/1] target/riscv: misa to ISA string conversion fix

[Alistair Francis]

On Mon, Mar 28, 2022 at 11:11 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> Some bits in RISC-V `misa' CSR should not be reflected in the ISA
> string.  For instance, `S' and `U' (represents existence of supervisor
> and user mode, respectively) in `misa' CSR must not be copied since
> neither `S' nor `U' are valid single-letter extensions.
>
> This commit also removes all reserved/dropped single-letter "extensions"
> from the list.
>
> -   "B": Not going to be a single-letter extension (misa.B is reserved).
> -   "J": Not going to be a single-letter extension (misa.J is reserved).
> -   "K": Not going to be a single-letter extension (misa.K is reserved).
> -   "L": Dropped.
> -   "N": Dropped.
> -   "T": Dropped.
>
> It also clarifies that the variable `riscv_single_letter_exts' is a
> single-letter extension order list.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/cpu.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ddda4906ff..1f68c696eb 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -34,7 +34,7 @@
>
>  /* RISC-V CPU definitions */
>
> -static const char riscv_exts[26] = "IEMAFDQCLBJTPVNSUHKORWXYZG";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
>
>  const char * const riscv_int_regnames[] = {
>    "x0/zero", "x1/ra",  "x2/sp",  "x3/gp",  "x4/tp",  "x5/t0",   "x6/t1",
> @@ -901,12 +901,12 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
>  char *riscv_isa_string(RISCVCPU *cpu)
>  {
>      int i;
> -    const size_t maxlen = sizeof("rv128") + sizeof(riscv_exts) + 1;
> +    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);
> -    for (i = 0; i < sizeof(riscv_exts); i++) {
> -        if (cpu->env.misa_ext & RV(riscv_exts[i])) {
> -            *p++ = qemu_tolower(riscv_exts[i]);
> +    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]);
>          }
>      }
>      *p = '\0';
> --
> 2.32.0
>