[PATCH (PING) 0/1] target/riscv: misa to ISA string conversion fix

[PATCH (PING) 0/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

[This is the same patch as previous ones]
<https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00098.html>
 (qemu-riscv only)
<https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00097.html>
 (resent due to configuration error of my mail server; qemu-riscv only)

I hope this is applied before the QEMU 7.0 release.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>

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.

I am truly surprised that this patchset is the THIRD attempt to fix this
longstanding problem.

(1) August 2019: by Palmer Dabbelt
<https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00165.html>
<https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00141.html>
<https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00259.html>

(2) April 2021: by Emmanuel Blot
<https://lists.nongnu.org/archive/html/qemu-riscv/2021-04/msg00248.html>

(3) February 2022: by me (this patchset)

I feel this is urgent to eliminate this bug now considering it required
a workaround to RISC-V Linux kernel as I pointed out:
<http://lists.infradead.org/pipermail/linux-riscv/2022-February/012252.html>


Though my patchset is first developed independently, this submitted
version is influenced by (2) Emmanuel Blot's patchset.  Thanks to this,
constant "[n]" can now be variable "[]".

It also fixes an ordering issue where 'C' should be preceded by 'L'
(order: 'L' -> 'C') as per the RISC-V ISA Manual (version 20191213),
Table 27.1.

It clarifies the role of `riscv_exts'.  It's a single-letter extrension
ordering list.




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: f345abe36527a8b575482bb5a0616f43952bf1f4
-- 
2.32.0

[PATCH (PING) 0/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

[This is the same patch as previous ones]
<https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00098.html>
 (qemu-riscv only)
<https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00097.html>
 (resent due to configuration error of my mail server; qemu-riscv only)

I hope this is applied before the QEMU 7.0 release.

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>

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.

I am truly surprised that this patchset is the THIRD attempt to fix this
longstanding problem.

(1) August 2019: by Palmer Dabbelt
<https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00165.html>
<https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00141.html>
<https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00259.html>

(2) April 2021: by Emmanuel Blot
<https://lists.nongnu.org/archive/html/qemu-riscv/2021-04/msg00248.html>

(3) February 2022: by me (this patchset)

I feel this is urgent to eliminate this bug now considering it required
a workaround to RISC-V Linux kernel as I pointed out:
<http://lists.infradead.org/pipermail/linux-riscv/2022-February/012252.html>


Though my patchset is first developed independently, this submitted
version is influenced by (2) Emmanuel Blot's patchset.  Thanks to this,
constant "[n]" can now be variable "[]".

It also fixes an ordering issue where 'C' should be preceded by 'L'
(order: 'L' -> 'C') as per the RISC-V ISA Manual (version 20191213),
Table 27.1.

It clarifies the role of `riscv_exts'.  It's a single-letter extrension
ordering list.




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: f345abe36527a8b575482bb5a0616f43952bf1f4
-- 
2.32.0

[PATCH (PING) 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 restricts which bits to copy from `misa' CSR to ISA string
with another fix: `C' extension should be preceded by `L' extension.

It also clarifies that RISC-V extension order string is actually 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..84877cf24a 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[] = "IEMAFDQLCBJTPVNH";
 
 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

[PATCH (PING) 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 restricts which bits to copy from `misa' CSR to ISA string
with another fix: `C' extension should be preceded by `L' extension.

It also clarifies that RISC-V extension order string is actually 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..84877cf24a 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[] = "IEMAFDQLCBJTPVNH";
 
 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 (PING) 1/1] target/riscv: misa to ISA string conversion fix

[Alistair Francis]

On Sat, Mar 26, 2022 at 3:46 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.

Thanks for the patch.

>
> This commit restricts which bits to copy from `misa' CSR to ISA string
> with another fix: `C' extension should be preceded by `L' extension.

The L extension has been removed, so it probably makes more sense to
just remove it at this stage instead of fixing the order.

>
> It also clarifies that RISC-V extension order string is actually 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..84877cf24a 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[] = "IEMAFDQLCBJTPVNH";

What about K?

Why not use IEMAFDQCBKJTPVNH instead?

Alistair

>
>  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 (PING) 0/1] target/riscv: misa to ISA string conversion fix

[Alistair Francis]

On Sat, Mar 26, 2022 at 3:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> [This is the same patch as previous ones]

Hello,

Thanks for the patch!

> <https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00098.html>
>  (qemu-riscv only)

This never made it to the QEMU mailing list

> <https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00097.html>
>  (resent due to configuration error of my mail server; qemu-riscv only)

and neither did this

>
> I hope this is applied before the QEMU 7.0 release.

Unfortunately you have missed the window for 7.0. This patch will need
to be reviewed then applied for the next QEMU release.

>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
>
> 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.
>
> I am truly surprised that this patchset is the THIRD attempt to fix this
> longstanding problem.

I'm sorry you feel this way, but this is the first time this patch has
been sent to the list since 2019.

I'm not sure why (1) wasn't applied, but (2) and (3) don't appear to
have been sent to the QEMU mailing list.

The separate RISC-V mailing list is confusing, but patches should be
sent to qemu-devel, as described at:
https://wiki.qemu.org/Contribute/MailingLists

Alistair

>
> (1) August 2019: by Palmer Dabbelt
> <https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00165.html>
> <https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00141.html>
> <https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00259.html>
>
> (2) April 2021: by Emmanuel Blot
> <https://lists.nongnu.org/archive/html/qemu-riscv/2021-04/msg00248.html>
>
> (3) February 2022: by me (this patchset)
>
> I feel this is urgent to eliminate this bug now considering it required
> a workaround to RISC-V Linux kernel as I pointed out:
> <http://lists.infradead.org/pipermail/linux-riscv/2022-February/012252.html>
>
>
> Though my patchset is first developed independently, this submitted
> version is influenced by (2) Emmanuel Blot's patchset.  Thanks to this,
> constant "[n]" can now be variable "[]".
>
> It also fixes an ordering issue where 'C' should be preceded by 'L'
> (order: 'L' -> 'C') as per the RISC-V ISA Manual (version 20191213),
> Table 27.1.
>
> It clarifies the role of `riscv_exts'.  It's a single-letter extrension
> ordering list.
>
>
>
>
> 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: f345abe36527a8b575482bb5a0616f43952bf1f4
> --
> 2.32.0
>
>

Re: [PATCH (PING) 1/1] target/riscv: misa to ISA string conversion fix

[Frank Chang]

[-- Attachment #1: Type: text/plain, Size: 2821 bytes --]

On Mon, Mar 28, 2022 at 7:30 AM Alistair Francis <alistair23@gmail.com>
wrote:

> On Sat, Mar 26, 2022 at 3:46 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.
>
> Thanks for the patch.
>
> >
> > This commit restricts which bits to copy from `misa' CSR to ISA string
> > with another fix: `C' extension should be preceded by `L' extension.
>
> The L extension has been removed, so it probably makes more sense to
> just remove it at this stage instead of fixing the order.
>
> >
> > It also clarifies that RISC-V extension order string is actually 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..84877cf24a 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[] = "IEMAFDQLCBJTPVNH";
>
> What about K?
>
> Why not use IEMAFDQCBKJTPVNH instead?
>
> Alistair
>

The RISC-V ISA Manual (version 20191213) is quite old.
Where "L" was not removed and "K" was not introduced.
It seems Unprivileged spec is not "ratified" as often as Privileged spec
does (most of them are drafts...).

But I agree that it's better to remove "L" and add "K" into ISA string.

Regards,
Frank Chang


>
> >
> >  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
> >
> >
>
>

[-- Attachment #2: Type: text/html, Size: 4124 bytes --]

Re: [PATCH (PING) 0/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

On 2022/03/28 8:29, Alistair Francis wrote:
> On Sat, Mar 26, 2022 at 3:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>>
>> [This is the same patch as previous ones]
> 
> Hello,
> 
> Thanks for the patch!
> 
>> <https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00098.html>
>>  (qemu-riscv only)
> 
> This never made it to the QEMU mailing list
> 
>> <https://lists.nongnu.org/archive/html/qemu-riscv/2022-02/msg00097.html>
>>  (resent due to configuration error of my mail server; qemu-riscv only)
> 
> and neither did this

Ah, that was the reason!  I'm happy to know that.

> 
>>
>> I hope this is applied before the QEMU 7.0 release.
> 
> Unfortunately you have missed the window for 7.0. This patch will need
> to be reviewed then applied for the next QEMU release.

Understood.

> 
>>
>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>> Reviewed-by: Frank Chang <frank.chang@sifive.com>
>>
>> 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.
>>
>> I am truly surprised that this patchset is the THIRD attempt to fix this
>> longstanding problem.
> 
> I'm sorry you feel this way, but this is the first time this patch has
> been sent to the list since 2019.
> 
> I'm not sure why (1) wasn't applied, but (2) and (3) don't appear to
> have been sent to the QEMU mailing list.
> 
> The separate RISC-V mailing list is confusing, but patches should be
> sent to qemu-devel, as described at:
> https://wiki.qemu.org/Contribute/MailingLists
> 
> Alistair

Thanks for letting me know.  From the next time, I will follow this.


> 
>>
>> (1) August 2019: by Palmer Dabbelt
>> <https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00165.html>
>> <https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00141.html>
>> <https://lists.nongnu.org/archive/html/qemu-riscv/2019-08/msg00259.html>
>>
>> (2) April 2021: by Emmanuel Blot
>> <https://lists.nongnu.org/archive/html/qemu-riscv/2021-04/msg00248.html>
>>
>> (3) February 2022: by me (this patchset)
>>
>> I feel this is urgent to eliminate this bug now considering it required
>> a workaround to RISC-V Linux kernel as I pointed out:
>> <http://lists.infradead.org/pipermail/linux-riscv/2022-February/012252.html>
>>
>>
>> Though my patchset is first developed independently, this submitted
>> version is influenced by (2) Emmanuel Blot's patchset.  Thanks to this,
>> constant "[n]" can now be variable "[]".
>>
>> It also fixes an ordering issue where 'C' should be preceded by 'L'
>> (order: 'L' -> 'C') as per the RISC-V ISA Manual (version 20191213),
>> Table 27.1.
>>
>> It clarifies the role of `riscv_exts'.  It's a single-letter extrension
>> ordering list.
>>
>>
>>
>>
>> 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: f345abe36527a8b575482bb5a0616f43952bf1f4
>> --
>> 2.32.0
>>
>>
> 

Re: [PATCH (PING) 1/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

Hi Alistair,

On 2022/03/28 8:29, Alistair Francis wrote:
> On Sat, Mar 26, 2022 at 3:46 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.
> 
> Thanks for the patch.
> 
>>
>> This commit restricts which bits to copy from `misa' CSR to ISA string
>> with another fix: `C' extension should be preceded by `L' extension.
> 
> The L extension has been removed, so it probably makes more sense to
> just remove it at this stage instead of fixing the order.

Thanks.  I'll reflect this in v2.


> 
>>
>> It also clarifies that RISC-V extension order string is actually 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..84877cf24a 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[] = "IEMAFDQLCBJTPVNH";
> 
> What about K?

Not having "K" in the string is completely intentional in PATCH v1.

riscv_single_letter_exts is intended to be valid single-letter
extensions (not canonical order of multi-letter Z* extensions).

Considering this (and the fact that we can remove "L"), correct fix to
this might be to "remove B and J", not to "add K".

"B", "K" and "J" are not going to be single-letter extensions (for now)
but prefixes of subextensions (Zb*, Zk* and non-ratified proposal(s?)
with Zj*).

For "B" and "J", this commit might help understanding it:
https://github.com/riscv/riscv-isa-manual/commit/c1c77c4902d5e7c58613d725d09d66a2a743da1c

For "K",
https://github.com/riscv/riscv-crypto/releases/tag/v0.9.3-scalar
    (with misa.K)
https://github.com/riscv/riscv-crypto/releases/tag/v0.9.4-scalar
    (WITHOUT misa.K)
https://github.com/riscv/riscv-crypto/releases/tag/v1.0.0-scalar
    (ratified; WITHOUT misa.K)
https://lists.riscv.org/g/tech-crypto-ext/topic/riscv_k_in_misa_risc_v/85354476

Dropped "N" and "T" extensions are to be removed, too.

Not-yet-ratified "P" could be removed for now but will be kept in v2.


> 
> Why not use IEMAFDQCBKJTPVNH instead?

I noticed the order "K" -> "J" (in the draft RISC-V ISA Manual).  That
will make a patch to GNU Binutils (that has "J" -> "K" order).

Thanks!

Tsukasa


> 
> Alistair
> 
>>
>>  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 (PING) 1/1] target/riscv: misa to ISA string conversion fix

[Tsukasa OI]

Hi Alistair,

On 2022/03/28 8:29, Alistair Francis wrote:
> On Sat, Mar 26, 2022 at 3:46 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.
> 
> Thanks for the patch.
> 
>>
>> This commit restricts which bits to copy from `misa' CSR to ISA string
>> with another fix: `C' extension should be preceded by `L' extension.
> 
> The L extension has been removed, so it probably makes more sense to
> just remove it at this stage instead of fixing the order.

Thanks.  I'll reflect this in v2.


> 
>>
>> It also clarifies that RISC-V extension order string is actually 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..84877cf24a 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[] = "IEMAFDQLCBJTPVNH";
> 
> What about K?

Not having "K" in the string is completely intentional in PATCH v1.

riscv_single_letter_exts is intended to be valid single-letter
extensions (not canonical order of multi-letter Z* extensions).

Considering this (and the fact that we can remove "L"), correct fix to
this might be to "remove B and J", not to "add K".

"B", "K" and "J" are not going to be single-letter extensions (for now)
but prefixes of subextensions (Zb*, Zk* and non-ratified proposal(s?)
with Zj*).

For "B" and "J", this commit might help understanding it:
https://github.com/riscv/riscv-isa-manual/commit/c1c77c4902d5e7c58613d725d09d66a2a743da1c

For "K",
https://github.com/riscv/riscv-crypto/releases/tag/v0.9.3-scalar
    (with misa.K)
https://github.com/riscv/riscv-crypto/releases/tag/v0.9.4-scalar
    (WITHOUT misa.K)
https://github.com/riscv/riscv-crypto/releases/tag/v1.0.0-scalar
    (ratified; WITHOUT misa.K)
https://lists.riscv.org/g/tech-crypto-ext/topic/riscv_k_in_misa_risc_v/85354476

Dropped "N" and "T" extensions are to be removed, too.

Not-yet-ratified "P" could be removed for now but will be kept in v2.


> 
> Why not use IEMAFDQCBKJTPVNH instead?

I noticed the order "K" -> "J" (in the draft RISC-V ISA Manual).  That
will make a patch to GNU Binutils (that has "J" -> "K" order).

Thanks!

Tsukasa


> 
> Alistair
> 
>>
>>  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
>>
>>
>