[PATCH v2 0/2] target/riscv: ISA string conversion fix and enhancement

[PATCH v2 0/2] target/riscv: ISA string conversion fix and enhancement

[Tsukasa OI]

Hello,

There is two issues related to RISC-V ISA extension string
I want to be fixed before QEMU 7.1 release.

This is PATCH v2.

For details, see cover letter of PATCH v1.
<https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg03817.html>



PATCH v1 -> PATCH v2:

I decided to "move" Zhinx*, not to remove them tentativelly.

Because, although I disagree with Weiwei's opinion (on canonical
ordering, Z* multi-letter extensions are ordered by second character;
that should reflect closely related single-letter extension but that
didn't happened on Zhinx*), ordering of "H" is reviewed and going to be
right after "V".

I considered this is safe enough (despite its "unratified" status):

1.  Zhinx and Zhinxmin are minor extensions
2.  Extension ordering is going to be not that important in the future
    (by upcoming rules) but we have reasons to make extension ordering
    canonical as possible for toolchain compatibility (for now).




Tsukasa OI (2):
  target/riscv: Move Zhinx* extensions on ISA string
  target/riscv: Add short-isa-string option

 target/riscv/cpu.c | 9 ++++++---
 target/riscv/cpu.h | 2 ++
 2 files changed, 8 insertions(+), 3 deletions(-)


base-commit: 178bacb66d98d9ee7a702b9f2a4dfcd88b72a9ab
-- 
2.34.1

[PATCH v2 1/2] target/riscv: Move Zhinx* extensions on ISA string

[Tsukasa OI]

This commit moves ISA string conversion for Zhinx and Zhinxmin extensions.
Because extension category ordering of "H" is going to be after "V",
their ordering is going to be valid (on canonical order).

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

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ccacdee215..9f38e56316 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -999,8 +999,6 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
         ISA_EDATA_ENTRY(zfh, ext_zfh),
         ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
         ISA_EDATA_ENTRY(zfinx, ext_zfinx),
-        ISA_EDATA_ENTRY(zhinx, ext_zhinx),
-        ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
         ISA_EDATA_ENTRY(zdinx, ext_zdinx),
         ISA_EDATA_ENTRY(zba, ext_zba),
         ISA_EDATA_ENTRY(zbb, ext_zbb),
@@ -1021,6 +1019,8 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
         ISA_EDATA_ENTRY(zkt, ext_zkt),
         ISA_EDATA_ENTRY(zve32f, ext_zve32f),
         ISA_EDATA_ENTRY(zve64f, ext_zve64f),
+        ISA_EDATA_ENTRY(zhinx, ext_zhinx),
+        ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
         ISA_EDATA_ENTRY(svinval, ext_svinval),
         ISA_EDATA_ENTRY(svnapot, ext_svnapot),
         ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
-- 
2.34.1

[PATCH v2 2/2] target/riscv: Add short-isa-string option

[Tsukasa OI]

Because some operating systems don't correctly parse long ISA extension
string, this commit adds short-isa-string boolean option to disable
generating long ISA extension strings on Device Tree.

For instance, enabling Zfinx and Zdinx extensions and booting Linux (5.17 or
earlier) with FPU support caused a kernel panic.

Operating Systems which short-isa-string might be helpful:

1.  Linux (5.17 or earlier)
2.  FreeBSD (at least 14.0-CURRENT)
3.  OpenBSD (at least current development version)

Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
 target/riscv/cpu.c | 5 ++++-
 target/riscv/cpu.h | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9f38e56316..4db3dce25a 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -879,6 +879,8 @@ static Property riscv_cpu_properties[] = {
     DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
 
     DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
+
+    DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -1049,7 +1051,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
         }
     }
     *p = '\0';
-    riscv_isa_string_ext(cpu, &isa_str, maxlen);
+    if (!cpu->cfg.short_isa_string)
+        riscv_isa_string_ext(cpu, &isa_str, maxlen);
     return isa_str;
 }
 
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index fe6c9a2c92..f5ff7294c6 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -425,6 +425,8 @@ struct RISCVCPUConfig {
     bool aia;
     bool debug;
     uint64_t resetvec;
+
+    bool short_isa_string;
 };
 
 typedef struct RISCVCPUConfig RISCVCPUConfig;
-- 
2.34.1

Re: [PATCH v2 1/2] target/riscv: Move Zhinx* extensions on ISA string

[Alistair Francis]

On Tue, May 10, 2022 at 9:29 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> This commit moves ISA string conversion for Zhinx and Zhinxmin extensions.
> Because extension category ordering of "H" is going to be after "V",
> their ordering is going to be valid (on canonical order).
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>

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

Alistair

> ---
>  target/riscv/cpu.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index ccacdee215..9f38e56316 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -999,8 +999,6 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
>          ISA_EDATA_ENTRY(zfh, ext_zfh),
>          ISA_EDATA_ENTRY(zfhmin, ext_zfhmin),
>          ISA_EDATA_ENTRY(zfinx, ext_zfinx),
> -        ISA_EDATA_ENTRY(zhinx, ext_zhinx),
> -        ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
>          ISA_EDATA_ENTRY(zdinx, ext_zdinx),
>          ISA_EDATA_ENTRY(zba, ext_zba),
>          ISA_EDATA_ENTRY(zbb, ext_zbb),
> @@ -1021,6 +1019,8 @@ static void riscv_isa_string_ext(RISCVCPU *cpu, char **isa_str, int max_str_len)
>          ISA_EDATA_ENTRY(zkt, ext_zkt),
>          ISA_EDATA_ENTRY(zve32f, ext_zve32f),
>          ISA_EDATA_ENTRY(zve64f, ext_zve64f),
> +        ISA_EDATA_ENTRY(zhinx, ext_zhinx),
> +        ISA_EDATA_ENTRY(zhinxmin, ext_zhinxmin),
>          ISA_EDATA_ENTRY(svinval, ext_svinval),
>          ISA_EDATA_ENTRY(svnapot, ext_svnapot),
>          ISA_EDATA_ENTRY(svpbmt, ext_svpbmt),
> --
> 2.34.1
>

Re: [PATCH v2 2/2] target/riscv: Add short-isa-string option

[Alistair Francis]

On Tue, May 10, 2022 at 9:29 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Because some operating systems don't correctly parse long ISA extension
> string, this commit adds short-isa-string boolean option to disable
> generating long ISA extension strings on Device Tree.
>
> For instance, enabling Zfinx and Zdinx extensions and booting Linux (5.17 or
> earlier) with FPU support caused a kernel panic.
>
> Operating Systems which short-isa-string might be helpful:
>
> 1.  Linux (5.17 or earlier)
> 2.  FreeBSD (at least 14.0-CURRENT)
> 3.  OpenBSD (at least current development version)
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>

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

Alistair

> ---
>  target/riscv/cpu.c | 5 ++++-
>  target/riscv/cpu.h | 2 ++
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9f38e56316..4db3dce25a 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -879,6 +879,8 @@ static Property riscv_cpu_properties[] = {
>      DEFINE_PROP_BOOL("x-aia", RISCVCPU, cfg.aia, false),
>
>      DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
> +
> +    DEFINE_PROP_BOOL("short-isa-string", RISCVCPU, cfg.short_isa_string, false),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>
> @@ -1049,7 +1051,8 @@ char *riscv_isa_string(RISCVCPU *cpu)
>          }
>      }
>      *p = '\0';
> -    riscv_isa_string_ext(cpu, &isa_str, maxlen);
> +    if (!cpu->cfg.short_isa_string)
> +        riscv_isa_string_ext(cpu, &isa_str, maxlen);
>      return isa_str;
>  }
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index fe6c9a2c92..f5ff7294c6 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -425,6 +425,8 @@ struct RISCVCPUConfig {
>      bool aia;
>      bool debug;
>      uint64_t resetvec;
> +
> +    bool short_isa_string;
>  };
>
>  typedef struct RISCVCPUConfig RISCVCPUConfig;
> --
> 2.34.1
>

Re: [PATCH v2 0/2] target/riscv: ISA string conversion fix and enhancement

[Alistair Francis]

On Tue, May 10, 2022 at 9:29 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Hello,
>
> There is two issues related to RISC-V ISA extension string
> I want to be fixed before QEMU 7.1 release.
>
> This is PATCH v2.
>
> For details, see cover letter of PATCH v1.
> <https://lists.gnu.org/archive/html/qemu-devel/2022-04/msg03817.html>
>
>
>
> PATCH v1 -> PATCH v2:
>
> I decided to "move" Zhinx*, not to remove them tentativelly.
>
> Because, although I disagree with Weiwei's opinion (on canonical
> ordering, Z* multi-letter extensions are ordered by second character;
> that should reflect closely related single-letter extension but that
> didn't happened on Zhinx*), ordering of "H" is reviewed and going to be
> right after "V".
>
> I considered this is safe enough (despite its "unratified" status):
>
> 1.  Zhinx and Zhinxmin are minor extensions
> 2.  Extension ordering is going to be not that important in the future
>     (by upcoming rules) but we have reasons to make extension ordering
>     canonical as possible for toolchain compatibility (for now).
>
>
>
>
> Tsukasa OI (2):
>   target/riscv: Move Zhinx* extensions on ISA string
>   target/riscv: Add short-isa-string option
>
>  target/riscv/cpu.c | 9 ++++++---
>  target/riscv/cpu.h | 2 ++
>  2 files changed, 8 insertions(+), 3 deletions(-)

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>
> base-commit: 178bacb66d98d9ee7a702b9f2a4dfcd88b72a9ab
> --
> 2.34.1
>