* [PATCH 1/5] target/riscv: Fix "G" extension expansion typing
2022-05-13 9:45 [PATCH 0/5] target/riscv: Enhanced ISA extension checks Tsukasa OI
@ 2022-05-13 9:45 ` Tsukasa OI
2022-05-16 17:22 ` Víctor Colombo
2022-05-13 9:45 ` [PATCH 2/5] target/riscv: Disable "G" by default Tsukasa OI
` (5 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Tsukasa OI @ 2022-05-13 9:45 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
Because ext_? members are in bool type, operator `&&' should be used
instead of `&'.
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..00bf26ec8b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -596,8 +596,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
return;
}
- if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
- cpu->cfg.ext_a & cpu->cfg.ext_f &
+ if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
+ cpu->cfg.ext_a && cpu->cfg.ext_f &&
cpu->cfg.ext_d)) {
warn_report("Setting G will also set IMAFD");
cpu->cfg.ext_i = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 1/5] target/riscv: Fix "G" extension expansion typing
2022-05-13 9:45 ` [PATCH 1/5] target/riscv: Fix "G" extension expansion typing Tsukasa OI
@ 2022-05-16 17:22 ` Víctor Colombo
0 siblings, 0 replies; 27+ messages in thread
From: Víctor Colombo @ 2022-05-16 17:22 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
On 13/05/2022 06:45, Tsukasa OI wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de e-mail suspeito entre imediatamente em contato com o DTI.
>
> Because ext_? members are in bool type, operator `&&' should be used
> instead of `&'.
>
> 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..00bf26ec8b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -596,8 +596,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
> - cpu->cfg.ext_a & cpu->cfg.ext_f &
> + if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
> + cpu->cfg.ext_a && cpu->cfg.ext_f &&
> cpu->cfg.ext_d)) {
> warn_report("Setting G will also set IMAFD");
> cpu->cfg.ext_i = true;
> --
> 2.34.1
>
>
Reviewed-by: Víctor Colombo <victor.colombo@eldorado.org.br>
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/5] target/riscv: Disable "G" by default
2022-05-13 9:45 [PATCH 0/5] target/riscv: Enhanced ISA extension checks Tsukasa OI
2022-05-13 9:45 ` [PATCH 1/5] target/riscv: Fix "G" extension expansion typing Tsukasa OI
@ 2022-05-13 9:45 ` Tsukasa OI
2022-05-17 0:39 ` Alistair Francis
2022-05-13 9:45 ` [PATCH 3/5] target/riscv: Change "G" expansion Tsukasa OI
` (4 subsequent siblings)
6 siblings, 1 reply; 27+ messages in thread
From: Tsukasa OI @ 2022-05-13 9:45 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
Because "G" virtual extension expands to "IMAFD", we cannot separately
disable extensions like "F" or "D" without disabling "G". Because all
"IMAFD" are enabled by default, it's harmless to disable "G" by default.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 00bf26ec8b..3ea68d5cd7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -812,7 +812,7 @@ static Property riscv_cpu_properties[] = {
/* Defaults for standard extensions */
DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
- DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, true),
+ DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, false),
DEFINE_PROP_BOOL("m", RISCVCPU, cfg.ext_m, true),
DEFINE_PROP_BOOL("a", RISCVCPU, cfg.ext_a, true),
DEFINE_PROP_BOOL("f", RISCVCPU, cfg.ext_f, true),
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 2/5] target/riscv: Disable "G" by default
2022-05-13 9:45 ` [PATCH 2/5] target/riscv: Disable "G" by default Tsukasa OI
@ 2022-05-17 0:39 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2022-05-17 0:39 UTC (permalink / raw)
To: Tsukasa OI
Cc: Frank Chang, qemu-devel@nongnu.org Developers, open list:RISC-V
On Fri, May 13, 2022 at 7:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Because "G" virtual extension expands to "IMAFD", we cannot separately
> disable extensions like "F" or "D" without disabling "G". Because all
> "IMAFD" are enabled by default, it's harmless to disable "G" by default.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 00bf26ec8b..3ea68d5cd7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -812,7 +812,7 @@ static Property riscv_cpu_properties[] = {
> /* Defaults for standard extensions */
> DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
> DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
> - DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, true),
> + DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, false),
> DEFINE_PROP_BOOL("m", RISCVCPU, cfg.ext_m, true),
> DEFINE_PROP_BOOL("a", RISCVCPU, cfg.ext_a, true),
> DEFINE_PROP_BOOL("f", RISCVCPU, cfg.ext_f, true),
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/5] target/riscv: Change "G" expansion
2022-05-13 9:45 [PATCH 0/5] target/riscv: Enhanced ISA extension checks Tsukasa OI
2022-05-13 9:45 ` [PATCH 1/5] target/riscv: Fix "G" extension expansion typing Tsukasa OI
2022-05-13 9:45 ` [PATCH 2/5] target/riscv: Disable "G" by default Tsukasa OI
@ 2022-05-13 9:45 ` Tsukasa OI
2022-05-13 9:45 ` [PATCH 4/5] target/riscv: FP extension requirements Tsukasa OI
` (3 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Tsukasa OI @ 2022-05-13 9:45 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
On ISA version 20190608 or later, "G" expands to "IMAFD_Zicsr_Zifencei".
Both "Zicsr" and "Zifencei" are enabled by default and "G" is supposed to
be (virtually) enabled as well, it should be safe to change its expansion.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3ea68d5cd7..0854ca9103 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -598,13 +598,16 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
cpu->cfg.ext_a && cpu->cfg.ext_f &&
- cpu->cfg.ext_d)) {
- warn_report("Setting G will also set IMAFD");
+ cpu->cfg.ext_d &&
+ cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
+ warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
cpu->cfg.ext_i = true;
cpu->cfg.ext_m = true;
cpu->cfg.ext_a = true;
cpu->cfg.ext_f = true;
cpu->cfg.ext_d = true;
+ cpu->cfg.ext_icsr = true;
+ cpu->cfg.ext_ifencei = true;
}
if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 4/5] target/riscv: FP extension requirements
2022-05-13 9:45 [PATCH 0/5] target/riscv: Enhanced ISA extension checks Tsukasa OI
` (2 preceding siblings ...)
2022-05-13 9:45 ` [PATCH 3/5] target/riscv: Change "G" expansion Tsukasa OI
@ 2022-05-13 9:45 ` Tsukasa OI
2022-05-13 9:45 ` [PATCH 5/5] target/riscv: Move/refactor ISA extension checks Tsukasa OI
` (2 subsequent siblings)
6 siblings, 0 replies; 27+ messages in thread
From: Tsukasa OI @ 2022-05-13 9:45 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
QEMU allowed inconsistent configurations that made floating point
arithmetic effectively unusable.
This commit adds certain checks for consistent FP arithmetic:
- F requires Zicsr
- Zfinx requires Zicsr
- Zfh/Zfhmin require F
- D requires F
- V requires D
Because F/D/Zicsr are enabled by default (and an error will not occur unless
we manually disable one or more of prerequisites), this commit just enforces
the user to give consistent combinations.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0854ca9103..5371b0fd17 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -610,6 +610,31 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
cpu->cfg.ext_ifencei = true;
}
+ if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
+ error_setg(errp, "F extension requires Zicsr");
+ return;
+ }
+
+ if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
+ error_setg(errp, "Zfinx extension requires Zicsr");
+ return;
+ }
+
+ if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
+ error_setg(errp, "Zfh/Zfhmin extensions require F extension");
+ return;
+ }
+
+ if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
+ error_setg(errp, "D extension requires F extension");
+ return;
+ }
+
+ if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
+ error_setg(errp, "V extension requires D extension");
+ return;
+ }
+
if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
cpu->cfg.ext_zhinxmin) {
cpu->cfg.ext_zfinx = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 5/5] target/riscv: Move/refactor ISA extension checks
2022-05-13 9:45 [PATCH 0/5] target/riscv: Enhanced ISA extension checks Tsukasa OI
` (3 preceding siblings ...)
2022-05-13 9:45 ` [PATCH 4/5] target/riscv: FP extension requirements Tsukasa OI
@ 2022-05-13 9:45 ` Tsukasa OI
2022-05-15 2:56 ` [PATCH v2 0/5] target/riscv: Enhanced " Tsukasa OI
2022-05-17 2:17 ` [PATCH 0/5] target/riscv: Enhanced " Alistair Francis
6 siblings, 0 replies; 27+ messages in thread
From: Tsukasa OI @ 2022-05-13 9:45 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
We should separate "check" and "configure" steps as possible.
This commit separates both steps except vector/Zfinx-related checks.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 5371b0fd17..f654a6727f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -635,11 +635,23 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
return;
}
+ if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
+ error_setg(errp, "Zve32f/Zve64f extensions require F extension");
+ return;
+ }
+
+ /* Set the ISA extensions, checks should have happened above */
if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
cpu->cfg.ext_zhinxmin) {
cpu->cfg.ext_zfinx = true;
}
+ if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_f) {
+ error_setg(errp,
+ "Zfinx cannot be supported together with F extension");
+ return;
+ }
+
if (cpu->cfg.ext_zk) {
cpu->cfg.ext_zkn = true;
cpu->cfg.ext_zkr = true;
@@ -663,7 +675,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
cpu->cfg.ext_zksh = true;
}
- /* Set the ISA extensions, checks should have happened above */
if (cpu->cfg.ext_i) {
ext |= RVI;
}
@@ -734,20 +745,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
}
set_vext_version(env, vext_version);
}
- if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
- error_setg(errp, "Zve32f/Zve64f extension depends upon RVF.");
- return;
- }
if (cpu->cfg.ext_j) {
ext |= RVJ;
}
- if (cpu->cfg.ext_zfinx && ((ext & (RVF | RVD)) || cpu->cfg.ext_zfh ||
- cpu->cfg.ext_zfhmin)) {
- error_setg(errp,
- "'Zfinx' cannot be supported together with 'F', 'D', 'Zfh',"
- " 'Zfhmin'");
- return;
- }
set_misa(env, env->misa_mxl, ext);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH v2 0/5] target/riscv: Enhanced ISA extension checks
2022-05-13 9:45 [PATCH 0/5] target/riscv: Enhanced ISA extension checks Tsukasa OI
` (4 preceding siblings ...)
2022-05-13 9:45 ` [PATCH 5/5] target/riscv: Move/refactor ISA extension checks Tsukasa OI
@ 2022-05-15 2:56 ` Tsukasa OI
2022-05-15 2:56 ` [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion Tsukasa OI
` (4 more replies)
2022-05-17 2:17 ` [PATCH 0/5] target/riscv: Enhanced " Alistair Francis
6 siblings, 5 replies; 27+ messages in thread
From: Tsukasa OI @ 2022-05-15 2:56 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
c.f.
<https://lists.gnu.org/archive/html/qemu-riscv/2022-05/msg00221.html>
I was obviously drunk when finalizing PATCH v1.
[BUGS in PATCH v1 (fixed in v2)]
PATCH 1: My English was (or, "is"?) broken
(commit subject and message is rewritten)
PATCH 4: Zfinx requirement test were in the wrong place
PATCH 5: Zfinx/F exclusivity test was completely wrong
I meant Zfinx&&F but when finalizing my patchset, I somehow
changed this place to Zfinx&&!F.
Tsukasa OI (5):
target/riscv: Fix coding style on "G" expansion
target/riscv: Disable "G" by default
target/riscv: Change "G" expansion
target/riscv: FP extension requirements
target/riscv: Move/refactor ISA extension checks
target/riscv/cpu.c | 63 +++++++++++++++++++++++++++++++++-------------
1 file changed, 46 insertions(+), 17 deletions(-)
base-commit: 178bacb66d98d9ee7a702b9f2a4dfcd88b72a9ab
--
2.34.1
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion
2022-05-15 2:56 ` [PATCH v2 0/5] target/riscv: Enhanced " Tsukasa OI
@ 2022-05-15 2:56 ` Tsukasa OI
2022-05-16 17:56 ` Víctor Colombo
2022-05-17 0:38 ` Alistair Francis
2022-05-15 2:56 ` [PATCH v2 2/5] target/riscv: Disable "G" by default Tsukasa OI
` (3 subsequent siblings)
4 siblings, 2 replies; 27+ messages in thread
From: Tsukasa OI @ 2022-05-15 2:56 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
Because ext_? members are boolean variables, operator `&&' should be
used instead of `&'.
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..00bf26ec8b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -596,8 +596,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
return;
}
- if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
- cpu->cfg.ext_a & cpu->cfg.ext_f &
+ if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
+ cpu->cfg.ext_a && cpu->cfg.ext_f &&
cpu->cfg.ext_d)) {
warn_report("Setting G will also set IMAFD");
cpu->cfg.ext_i = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion
2022-05-15 2:56 ` [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion Tsukasa OI
@ 2022-05-16 17:56 ` Víctor Colombo
2022-05-17 0:38 ` Alistair Francis
1 sibling, 0 replies; 27+ messages in thread
From: Víctor Colombo @ 2022-05-16 17:56 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
On 14/05/2022 23:56, Tsukasa OI wrote:
> [E-MAIL EXTERNO] Não clique em links ou abra anexos, a menos que você possa confirmar o remetente e saber que o conteúdo é seguro. Em caso de e-mail suspeito entre imediatamente em contato com o DTI.
>
> Because ext_? members are boolean variables, operator `&&' should be
> used instead of `&'.
>
> 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..00bf26ec8b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -596,8 +596,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
> - cpu->cfg.ext_a & cpu->cfg.ext_f &
> + if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
> + cpu->cfg.ext_a && cpu->cfg.ext_f &&
> cpu->cfg.ext_d)) {
> warn_report("Setting G will also set IMAFD");
> cpu->cfg.ext_i = true;
> --
> 2.34.1
>
>
Sorry, looks like I mistakenly reviewed v1 earlier
Reviewed-by: Víctor Colombo <victor.colombo@eldorado.org.br>
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion
2022-05-15 2:56 ` [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion Tsukasa OI
2022-05-16 17:56 ` Víctor Colombo
@ 2022-05-17 0:38 ` Alistair Francis
1 sibling, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2022-05-17 0:38 UTC (permalink / raw)
To: Tsukasa OI
Cc: Frank Chang, qemu-devel@nongnu.org Developers, open list:RISC-V
On Sun, May 15, 2022 at 12:56 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> Because ext_? members are boolean variables, operator `&&' should be
> used instead of `&'.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Reviewed-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..00bf26ec8b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -596,8 +596,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> - if (cpu->cfg.ext_g && !(cpu->cfg.ext_i & cpu->cfg.ext_m &
> - cpu->cfg.ext_a & cpu->cfg.ext_f &
> + if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
> + cpu->cfg.ext_a && cpu->cfg.ext_f &&
> cpu->cfg.ext_d)) {
> warn_report("Setting G will also set IMAFD");
> cpu->cfg.ext_i = true;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 2/5] target/riscv: Disable "G" by default
2022-05-15 2:56 ` [PATCH v2 0/5] target/riscv: Enhanced " Tsukasa OI
2022-05-15 2:56 ` [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion Tsukasa OI
@ 2022-05-15 2:56 ` Tsukasa OI
2022-05-16 18:04 ` Víctor Colombo
2022-05-15 2:56 ` [PATCH v2 3/5] target/riscv: Change "G" expansion Tsukasa OI
` (2 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Tsukasa OI @ 2022-05-15 2:56 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
Because "G" virtual extension expands to "IMAFD", we cannot separately
disable extensions like "F" or "D" without disabling "G". Because all
"IMAFD" are enabled by default, it's harmless to disable "G" by default.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 00bf26ec8b..3ea68d5cd7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -812,7 +812,7 @@ static Property riscv_cpu_properties[] = {
/* Defaults for standard extensions */
DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
- DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, true),
+ DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, false),
DEFINE_PROP_BOOL("m", RISCVCPU, cfg.ext_m, true),
DEFINE_PROP_BOOL("a", RISCVCPU, cfg.ext_a, true),
DEFINE_PROP_BOOL("f", RISCVCPU, cfg.ext_f, true),
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/5] target/riscv: Disable "G" by default
2022-05-15 2:56 ` [PATCH v2 2/5] target/riscv: Disable "G" by default Tsukasa OI
@ 2022-05-16 18:04 ` Víctor Colombo
2022-05-24 9:07 ` Tsukasa OI
0 siblings, 1 reply; 27+ messages in thread
From: Víctor Colombo @ 2022-05-16 18:04 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
On 14/05/2022 23:56, Tsukasa OI wrote:
> Because "G" virtual extension expands to "IMAFD", we cannot separately
> disable extensions like "F" or "D" without disabling "G". Because all
> "IMAFD" are enabled by default, it's harmless to disable "G" by default.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
> target/riscv/cpu.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 00bf26ec8b..3ea68d5cd7 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -812,7 +812,7 @@ static Property riscv_cpu_properties[] = {
> /* Defaults for standard extensions */
> DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
> DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
> - DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, true),
> + DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, false),
> DEFINE_PROP_BOOL("m", RISCVCPU, cfg.ext_m, true),
> DEFINE_PROP_BOOL("a", RISCVCPU, cfg.ext_a, true),
> DEFINE_PROP_BOOL("f", RISCVCPU, cfg.ext_f, true),
> --
> 2.34.1
>
>
I think the logic looks ok, and (with my limited understanding of the
code) I agree on the reasoning for the changes in patches 2 and 3.
Just some clarification needed: where is the value of 'g' checked?
can the behavior change in this patch cause a situation where
IMAFD_Zicsr_Zifencei is set but 'g' is not, or does patch 3
guarantee that in this case 'g' will be set?
Thanks!
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/5] target/riscv: Disable "G" by default
2022-05-16 18:04 ` Víctor Colombo
@ 2022-05-24 9:07 ` Tsukasa OI
2022-05-24 15:48 ` Víctor Colombo
0 siblings, 1 reply; 27+ messages in thread
From: Tsukasa OI @ 2022-05-24 9:07 UTC (permalink / raw)
To: Víctor Colombo; +Cc: qemu-devel, qemu-riscv
On 2022/05/17 3:04, Víctor Colombo wrote:
> On 14/05/2022 23:56, Tsukasa OI wrote:
>> Because "G" virtual extension expands to "IMAFD", we cannot separately
>> disable extensions like "F" or "D" without disabling "G". Because all
>> "IMAFD" are enabled by default, it's harmless to disable "G" by default.
>>
>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>> ---
>> target/riscv/cpu.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 00bf26ec8b..3ea68d5cd7 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -812,7 +812,7 @@ static Property riscv_cpu_properties[] = {
>> /* Defaults for standard extensions */
>> DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
>> DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
>> - DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, true),
>> + DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, false),
>> DEFINE_PROP_BOOL("m", RISCVCPU, cfg.ext_m, true),
>> DEFINE_PROP_BOOL("a", RISCVCPU, cfg.ext_a, true),
>> DEFINE_PROP_BOOL("f", RISCVCPU, cfg.ext_f, true),
>> --
>> 2.34.1
>>
>>
>
> I think the logic looks ok, and (with my limited understanding of the
> code) I agree on the reasoning for the changes in patches 2 and 3.
> Just some clarification needed: where is the value of 'g' checked?
> can the behavior change in this patch cause a situation where
> IMAFD_Zicsr_Zifencei is set but 'g' is not, or does patch 3
> guarantee that in this case 'g' will be set?
>
>
> Thanks!
>
Probably too late to answer but on Alistair's riscv-to-apply.next tree...
target/riscv/cpu.c (19f13a9) line 599-611:
if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
cpu->cfg.ext_a && cpu->cfg.ext_f &&
cpu->cfg.ext_d &&
cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
cpu->cfg.ext_i = true;
cpu->cfg.ext_m = true;
cpu->cfg.ext_a = true;
cpu->cfg.ext_f = true;
cpu->cfg.ext_d = true;
cpu->cfg.ext_icsr = true;
cpu->cfg.ext_ifencei = true;
}
This is the only place where "G" (ext_g) is read. Here, if G is enabled
and not all IMAFD_Zicsr_Zifencei are enabled, it enables them with a
warning message.
So, even if "G" is disabled alone, it's harmless. Note that
IMAFD_Zicsr_Zifencei are enabled by default.
Thanks,
Tsukasa
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 2/5] target/riscv: Disable "G" by default
2022-05-24 9:07 ` Tsukasa OI
@ 2022-05-24 15:48 ` Víctor Colombo
0 siblings, 0 replies; 27+ messages in thread
From: Víctor Colombo @ 2022-05-24 15:48 UTC (permalink / raw)
To: Tsukasa OI; +Cc: qemu-devel, qemu-riscv
On 24/05/2022 06:07, Tsukasa OI wrote:
> On 2022/05/17 3:04, Víctor Colombo wrote:
>> On 14/05/2022 23:56, Tsukasa OI wrote:
>>> Because "G" virtual extension expands to "IMAFD", we cannot separately
>>> disable extensions like "F" or "D" without disabling "G". Because all
>>> "IMAFD" are enabled by default, it's harmless to disable "G" by default.
>>>
>>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>>> ---
>>> target/riscv/cpu.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index 00bf26ec8b..3ea68d5cd7 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -812,7 +812,7 @@ static Property riscv_cpu_properties[] = {
>>> /* Defaults for standard extensions */
>>> DEFINE_PROP_BOOL("i", RISCVCPU, cfg.ext_i, true),
>>> DEFINE_PROP_BOOL("e", RISCVCPU, cfg.ext_e, false),
>>> - DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, true),
>>> + DEFINE_PROP_BOOL("g", RISCVCPU, cfg.ext_g, false),
>>> DEFINE_PROP_BOOL("m", RISCVCPU, cfg.ext_m, true),
>>> DEFINE_PROP_BOOL("a", RISCVCPU, cfg.ext_a, true),
>>> DEFINE_PROP_BOOL("f", RISCVCPU, cfg.ext_f, true),
>>> --
>>> 2.34.1
>>>
>>>
>>
>> I think the logic looks ok, and (with my limited understanding of the
>> code) I agree on the reasoning for the changes in patches 2 and 3.
>> Just some clarification needed: where is the value of 'g' checked?
>> can the behavior change in this patch cause a situation where
>> IMAFD_Zicsr_Zifencei is set but 'g' is not, or does patch 3
>> guarantee that in this case 'g' will be set?
>>
>>
>> Thanks!
>>
>
> Probably too late to answer but on Alistair's riscv-to-apply.next tree...
>
> target/riscv/cpu.c (19f13a9) line 599-611:
> if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
> cpu->cfg.ext_a && cpu->cfg.ext_f &&
> cpu->cfg.ext_d &&
> cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
> warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
> cpu->cfg.ext_i = true;
> cpu->cfg.ext_m = true;
> cpu->cfg.ext_a = true;
> cpu->cfg.ext_f = true;
> cpu->cfg.ext_d = true;
> cpu->cfg.ext_icsr = true;
> cpu->cfg.ext_ifencei = true;
> }
>
> This is the only place where "G" (ext_g) is read. Here, if G is enabled
> and not all IMAFD_Zicsr_Zifencei are enabled, it enables them with a
> warning message.
>
> So, even if "G" is disabled alone, it's harmless. Note that
> IMAFD_Zicsr_Zifencei are enabled by default.
>
> Thanks,
> Tsukasa
Hello!
Thank you very much for the clarification, it was helpful.
Still getting used to the riscv code base in QEMU
Best regards,
--
Víctor Cora Colombo
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 3/5] target/riscv: Change "G" expansion
2022-05-15 2:56 ` [PATCH v2 0/5] target/riscv: Enhanced " Tsukasa OI
2022-05-15 2:56 ` [PATCH v2 1/5] target/riscv: Fix coding style on "G" expansion Tsukasa OI
2022-05-15 2:56 ` [PATCH v2 2/5] target/riscv: Disable "G" by default Tsukasa OI
@ 2022-05-15 2:56 ` Tsukasa OI
2022-05-17 0:40 ` Alistair Francis
2022-05-15 2:56 ` [PATCH v2 4/5] target/riscv: FP extension requirements Tsukasa OI
2022-05-15 2:56 ` [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks Tsukasa OI
4 siblings, 1 reply; 27+ messages in thread
From: Tsukasa OI @ 2022-05-15 2:56 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
On ISA version 20190608 or later, "G" expands to "IMAFD_Zicsr_Zifencei".
Both "Zicsr" and "Zifencei" are enabled by default and "G" is supposed to
be (virtually) enabled as well, it should be safe to change its expansion.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 3ea68d5cd7..0854ca9103 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -598,13 +598,16 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
cpu->cfg.ext_a && cpu->cfg.ext_f &&
- cpu->cfg.ext_d)) {
- warn_report("Setting G will also set IMAFD");
+ cpu->cfg.ext_d &&
+ cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
+ warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
cpu->cfg.ext_i = true;
cpu->cfg.ext_m = true;
cpu->cfg.ext_a = true;
cpu->cfg.ext_f = true;
cpu->cfg.ext_d = true;
+ cpu->cfg.ext_icsr = true;
+ cpu->cfg.ext_ifencei = true;
}
if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 3/5] target/riscv: Change "G" expansion
2022-05-15 2:56 ` [PATCH v2 3/5] target/riscv: Change "G" expansion Tsukasa OI
@ 2022-05-17 0:40 ` Alistair Francis
0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2022-05-17 0:40 UTC (permalink / raw)
To: Tsukasa OI
Cc: Frank Chang, qemu-devel@nongnu.org Developers, open list:RISC-V
On Sun, May 15, 2022 at 12:56 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> On ISA version 20190608 or later, "G" expands to "IMAFD_Zicsr_Zifencei".
> Both "Zicsr" and "Zifencei" are enabled by default and "G" is supposed to
> be (virtually) enabled as well, it should be safe to change its expansion.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 3ea68d5cd7..0854ca9103 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -598,13 +598,16 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>
> if (cpu->cfg.ext_g && !(cpu->cfg.ext_i && cpu->cfg.ext_m &&
> cpu->cfg.ext_a && cpu->cfg.ext_f &&
> - cpu->cfg.ext_d)) {
> - warn_report("Setting G will also set IMAFD");
> + cpu->cfg.ext_d &&
> + cpu->cfg.ext_icsr && cpu->cfg.ext_ifencei)) {
> + warn_report("Setting G will also set IMAFD_Zicsr_Zifencei");
> cpu->cfg.ext_i = true;
> cpu->cfg.ext_m = true;
> cpu->cfg.ext_a = true;
> cpu->cfg.ext_f = true;
> cpu->cfg.ext_d = true;
> + cpu->cfg.ext_icsr = true;
> + cpu->cfg.ext_ifencei = true;
> }
>
> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 4/5] target/riscv: FP extension requirements
2022-05-15 2:56 ` [PATCH v2 0/5] target/riscv: Enhanced " Tsukasa OI
` (2 preceding siblings ...)
2022-05-15 2:56 ` [PATCH v2 3/5] target/riscv: Change "G" expansion Tsukasa OI
@ 2022-05-15 2:56 ` Tsukasa OI
2022-05-15 14:37 ` Weiwei Li
2022-05-17 0:52 ` Alistair Francis
2022-05-15 2:56 ` [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks Tsukasa OI
4 siblings, 2 replies; 27+ messages in thread
From: Tsukasa OI @ 2022-05-15 2:56 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
QEMU allowed inconsistent configurations that made floating point
arithmetic effectively unusable.
This commit adds certain checks for consistent FP arithmetic:
- F requires Zicsr
- Zfinx requires Zicsr
- Zfh/Zfhmin require F
- D requires F
- V requires D
Because F/D/Zicsr are enabled by default (and an error will not occur unless
we manually disable one or more of prerequisites), this commit just enforces
the user to give consistent combinations.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 0854ca9103..f910a41407 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
cpu->cfg.ext_ifencei = true;
}
+ if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
+ error_setg(errp, "F extension requires Zicsr");
+ return;
+ }
+
+ if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
+ error_setg(errp, "Zfh/Zfhmin extensions require F extension");
+ return;
+ }
+
+ if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
+ error_setg(errp, "D extension requires F extension");
+ return;
+ }
+
+ if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
+ error_setg(errp, "V extension requires D extension");
+ return;
+ }
+
if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
cpu->cfg.ext_zhinxmin) {
cpu->cfg.ext_zfinx = true;
}
+ if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
+ error_setg(errp, "Zfinx extension requires Zicsr");
+ return;
+ }
+
if (cpu->cfg.ext_zk) {
cpu->cfg.ext_zkn = true;
cpu->cfg.ext_zkr = true;
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 4/5] target/riscv: FP extension requirements
2022-05-15 2:56 ` [PATCH v2 4/5] target/riscv: FP extension requirements Tsukasa OI
@ 2022-05-15 14:37 ` Weiwei Li
2022-05-15 14:45 ` Tsukasa OI
2022-05-17 0:52 ` Alistair Francis
1 sibling, 1 reply; 27+ messages in thread
From: Weiwei Li @ 2022-05-15 14:37 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
在 2022/5/15 上午10:56, Tsukasa OI 写道:
> QEMU allowed inconsistent configurations that made floating point
> arithmetic effectively unusable.
>
> This commit adds certain checks for consistent FP arithmetic:
>
> - F requires Zicsr
> - Zfinx requires Zicsr
> - Zfh/Zfhmin require F
> - D requires F
> - V requires D
Why 'V requires D'? I know partial vector instructions require D,
However, I think they just like c.fsd
instruction requires D, not 'C requires D' or 'D requires C'. Is there
any rvv spec change I don't know?
Regards.
Weiwei Li
>
> Because F/D/Zicsr are enabled by default (and an error will not occur unless
> we manually disable one or more of prerequisites), this commit just enforces
> the user to give consistent combinations.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
> target/riscv/cpu.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0854ca9103..f910a41407 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> cpu->cfg.ext_ifencei = true;
> }
>
> + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
> + error_setg(errp, "F extension requires Zicsr");
> + return;
> + }
> +
> + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
> + error_setg(errp, "Zfh/Zfhmin extensions require F extension");
> + return;
> + }
> +
> + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
> + error_setg(errp, "D extension requires F extension");
> + return;
> + }
> +
> + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
> + error_setg(errp, "V extension requires D extension");
> + return;
> + }
> +
> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> cpu->cfg.ext_zhinxmin) {
> cpu->cfg.ext_zfinx = true;
> }
>
> + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
> + error_setg(errp, "Zfinx extension requires Zicsr");
> + return;
> + }
> +
> if (cpu->cfg.ext_zk) {
> cpu->cfg.ext_zkn = true;
> cpu->cfg.ext_zkr = true;
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 4/5] target/riscv: FP extension requirements
2022-05-15 14:37 ` Weiwei Li
@ 2022-05-15 14:45 ` Tsukasa OI
2022-05-15 15:23 ` Weiwei Li
0 siblings, 1 reply; 27+ messages in thread
From: Tsukasa OI @ 2022-05-15 14:45 UTC (permalink / raw)
To: Weiwei Li, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
On 2022/05/15 23:37, Weiwei Li wrote:
>
> 在 2022/5/15 上午10:56, Tsukasa OI 写道:
>> QEMU allowed inconsistent configurations that made floating point
>> arithmetic effectively unusable.
>>
>> This commit adds certain checks for consistent FP arithmetic:
>>
>> - F requires Zicsr
>> - Zfinx requires Zicsr
>> - Zfh/Zfhmin require F
>> - D requires F
>> - V requires D
>
> Why 'V requires D'? I know partial vector instructions require D, However, I think they just like c.fsd
>
> instruction requires D, not 'C requires D' or 'D requires C'. Is there any rvv spec change I don't know?
I thought it was crystal-clear.
RISC-V "V" Vector Extension Version 1.0 (riscv-v-spec-1.0.pdf)
18.3. V: Vector Extension for Application Processors
Page 94:
> The V extension requires the scalar processor implements the F and D
> extensions, and implements all vector floating-point instructions
> (Section Vector Floating-Point Instructions) for floating-point operands
> with EEW=32 or EEW=64 (including widening instructions and conversions
> between FP32 and FP64).
c.f. <https://github.com/riscv/riscv-v-spec/releases/tag/v1.0>
Thanks,
Tsukasa
>
> Regards.
>
> Weiwei Li
>
>>
>> Because F/D/Zicsr are enabled by default (and an error will not occur unless
>> we manually disable one or more of prerequisites), this commit just enforces
>> the user to give consistent combinations.
>>
>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>> ---
>> target/riscv/cpu.c | 25 +++++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 0854ca9103..f910a41407 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>> cpu->cfg.ext_ifencei = true;
>> }
>> + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
>> + error_setg(errp, "F extension requires Zicsr");
>> + return;
>> + }
>> +
>> + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
>> + error_setg(errp, "Zfh/Zfhmin extensions require F extension");
>> + return;
>> + }
>> +
>> + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
>> + error_setg(errp, "D extension requires F extension");
>> + return;
>> + }
>> +
>> + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
>> + error_setg(errp, "V extension requires D extension");
>> + return;
>> + }
>> +
>> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
>> cpu->cfg.ext_zhinxmin) {
>> cpu->cfg.ext_zfinx = true;
>> }
>> + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
>> + error_setg(errp, "Zfinx extension requires Zicsr");
>> + return;
>> + }
>> +
>> if (cpu->cfg.ext_zk) {
>> cpu->cfg.ext_zkn = true;
>> cpu->cfg.ext_zkr = true;
>
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 4/5] target/riscv: FP extension requirements
2022-05-15 14:45 ` Tsukasa OI
@ 2022-05-15 15:23 ` Weiwei Li
0 siblings, 0 replies; 27+ messages in thread
From: Weiwei Li @ 2022-05-15 15:23 UTC (permalink / raw)
To: Tsukasa OI, Weiwei Li, Alistair Francis, Frank Chang
Cc: qemu-devel, qemu-riscv
在 2022/5/15 下午10:45, Tsukasa OI 写道:
> On 2022/05/15 23:37, Weiwei Li wrote:
>> 在 2022/5/15 上午10:56, Tsukasa OI 写道:
>>> QEMU allowed inconsistent configurations that made floating point
>>> arithmetic effectively unusable.
>>>
>>> This commit adds certain checks for consistent FP arithmetic:
>>>
>>> - F requires Zicsr
>>> - Zfinx requires Zicsr
>>> - Zfh/Zfhmin require F
>>> - D requires F
>>> - V requires D
>> Why 'V requires D'? I know partial vector instructions require D, However, I think they just like c.fsd
>>
>> instruction requires D, not 'C requires D' or 'D requires C'. Is there any rvv spec change I don't know?
> I thought it was crystal-clear.
>
> RISC-V "V" Vector Extension Version 1.0 (riscv-v-spec-1.0.pdf)
> 18.3. V: Vector Extension for Application Processors
> Page 94:
>
>> The V extension requires the scalar processor implements the F and D
>> extensions, and implements all vector floating-point instructions
>> (Section Vector Floating-Point Instructions) for floating-point operands
>> with EEW=32 or EEW=64 (including widening instructions and conversions
>> between FP32 and FP64).
> c.f. <https://github.com/riscv/riscv-v-spec/releases/tag/v1.0>
>
> Thanks,
>
> Tsukasa
OK. Thanks.
Regards,
Weiwei Li
>
>> Regards.
>>
>> Weiwei Li
>>
>>> Because F/D/Zicsr are enabled by default (and an error will not occur unless
>>> we manually disable one or more of prerequisites), this commit just enforces
>>> the user to give consistent combinations.
>>>
>>> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
>>> ---
>>> target/riscv/cpu.c | 25 +++++++++++++++++++++++++
>>> 1 file changed, 25 insertions(+)
>>>
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index 0854ca9103..f910a41407 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
>>> cpu->cfg.ext_ifencei = true;
>>> }
>>> + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
>>> + error_setg(errp, "F extension requires Zicsr");
>>> + return;
>>> + }
>>> +
>>> + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
>>> + error_setg(errp, "Zfh/Zfhmin extensions require F extension");
>>> + return;
>>> + }
>>> +
>>> + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
>>> + error_setg(errp, "D extension requires F extension");
>>> + return;
>>> + }
>>> +
>>> + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
>>> + error_setg(errp, "V extension requires D extension");
>>> + return;
>>> + }
>>> +
>>> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
>>> cpu->cfg.ext_zhinxmin) {
>>> cpu->cfg.ext_zfinx = true;
>>> }
>>> + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
>>> + error_setg(errp, "Zfinx extension requires Zicsr");
>>> + return;
>>> + }
>>> +
>>> if (cpu->cfg.ext_zk) {
>>> cpu->cfg.ext_zkn = true;
>>> cpu->cfg.ext_zkr = true;
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2 4/5] target/riscv: FP extension requirements
2022-05-15 2:56 ` [PATCH v2 4/5] target/riscv: FP extension requirements Tsukasa OI
2022-05-15 14:37 ` Weiwei Li
@ 2022-05-17 0:52 ` Alistair Francis
1 sibling, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2022-05-17 0:52 UTC (permalink / raw)
To: Tsukasa OI
Cc: Frank Chang, qemu-devel@nongnu.org Developers, open list:RISC-V
On Sun, May 15, 2022 at 12:56 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> QEMU allowed inconsistent configurations that made floating point
> arithmetic effectively unusable.
>
> This commit adds certain checks for consistent FP arithmetic:
>
> - F requires Zicsr
> - Zfinx requires Zicsr
> - Zfh/Zfhmin require F
> - D requires F
> - V requires D
>
> Because F/D/Zicsr are enabled by default (and an error will not occur unless
> we manually disable one or more of prerequisites), this commit just enforces
> the user to give consistent combinations.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 0854ca9103..f910a41407 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -610,11 +610,36 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> cpu->cfg.ext_ifencei = true;
> }
>
> + if (cpu->cfg.ext_f && !cpu->cfg.ext_icsr) {
> + error_setg(errp, "F extension requires Zicsr");
> + return;
> + }
> +
> + if ((cpu->cfg.ext_zfh || cpu->cfg.ext_zfhmin) && !cpu->cfg.ext_f) {
> + error_setg(errp, "Zfh/Zfhmin extensions require F extension");
> + return;
> + }
> +
> + if (cpu->cfg.ext_d && !cpu->cfg.ext_f) {
> + error_setg(errp, "D extension requires F extension");
> + return;
> + }
> +
> + if (cpu->cfg.ext_v && !cpu->cfg.ext_d) {
> + error_setg(errp, "V extension requires D extension");
> + return;
> + }
> +
> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> cpu->cfg.ext_zhinxmin) {
> cpu->cfg.ext_zfinx = true;
> }
>
> + if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
> + error_setg(errp, "Zfinx extension requires Zicsr");
> + return;
> + }
> +
> if (cpu->cfg.ext_zk) {
> cpu->cfg.ext_zkn = true;
> cpu->cfg.ext_zkr = true;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks
2022-05-15 2:56 ` [PATCH v2 0/5] target/riscv: Enhanced " Tsukasa OI
` (3 preceding siblings ...)
2022-05-15 2:56 ` [PATCH v2 4/5] target/riscv: FP extension requirements Tsukasa OI
@ 2022-05-15 2:56 ` Tsukasa OI
2022-05-16 3:12 ` Weiwei Li
2022-05-17 1:37 ` Alistair Francis
4 siblings, 2 replies; 27+ messages in thread
From: Tsukasa OI @ 2022-05-15 2:56 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
We should separate "check" and "configure" steps as possible.
This commit separates both steps except vector/Zfinx-related checks.
Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
---
target/riscv/cpu.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f910a41407..5ab246bf63 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -630,14 +630,27 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
return;
}
+ if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
+ error_setg(errp, "Zve32f/Zve64f extensions require F extension");
+ return;
+ }
+
+ /* Set the ISA extensions, checks should have happened above */
if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
cpu->cfg.ext_zhinxmin) {
cpu->cfg.ext_zfinx = true;
}
- if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
- error_setg(errp, "Zfinx extension requires Zicsr");
- return;
+ if (cpu->cfg.ext_zfinx) {
+ if (!cpu->cfg.ext_icsr) {
+ error_setg(errp, "Zfinx extension requires Zicsr");
+ return;
+ }
+ if (cpu->cfg.ext_f) {
+ error_setg(errp,
+ "Zfinx cannot be supported together with F extension");
+ return;
+ }
}
if (cpu->cfg.ext_zk) {
@@ -663,7 +676,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
cpu->cfg.ext_zksh = true;
}
- /* Set the ISA extensions, checks should have happened above */
if (cpu->cfg.ext_i) {
ext |= RVI;
}
@@ -734,20 +746,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
}
set_vext_version(env, vext_version);
}
- if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
- error_setg(errp, "Zve32f/Zve64f extension depends upon RVF.");
- return;
- }
if (cpu->cfg.ext_j) {
ext |= RVJ;
}
- if (cpu->cfg.ext_zfinx && ((ext & (RVF | RVD)) || cpu->cfg.ext_zfh ||
- cpu->cfg.ext_zfhmin)) {
- error_setg(errp,
- "'Zfinx' cannot be supported together with 'F', 'D', 'Zfh',"
- " 'Zfhmin'");
- return;
- }
set_misa(env, env->misa_mxl, ext);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks
2022-05-15 2:56 ` [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks Tsukasa OI
@ 2022-05-16 3:12 ` Weiwei Li
2022-05-17 1:37 ` Alistair Francis
1 sibling, 0 replies; 27+ messages in thread
From: Weiwei Li @ 2022-05-16 3:12 UTC (permalink / raw)
To: Tsukasa OI, Alistair Francis, Frank Chang; +Cc: qemu-devel, qemu-riscv
[-- Attachment #1: Type: text/plain, Size: 3230 bytes --]
在 2022/5/15 上午10:56, Tsukasa OI 写道:
> We should separate "check" and "configure" steps as possible.
> This commit separates both steps except vector/Zfinx-related checks.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
> ---
> target/riscv/cpu.c | 31 ++++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f910a41407..5ab246bf63 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -630,14 +630,27 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> + if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> + error_setg(errp, "Zve32f/Zve64f extensions require F extension");
> + return;
> + }
> +
> + /* Set the ISA extensions, checks should have happened above */
> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> cpu->cfg.ext_zhinxmin) {
> cpu->cfg.ext_zfinx = true;
> }
>
> - if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
> - error_setg(errp, "Zfinx extension requires Zicsr");
> - return;
> + if (cpu->cfg.ext_zfinx) {
> + if (!cpu->cfg.ext_icsr) {
> + error_setg(errp, "Zfinx extension requires Zicsr");
> + return;
> + }
> + if (cpu->cfg.ext_f) {
> + error_setg(errp,
> + "Zfinx cannot be supported together with F extension");
> + return;
> + }
> }
I think these checks for non-single-letter extensions are better to
move out of the 'if (env->misa_ext == 0)) { ...}', since they are enabled
directly by cfg property, such as we can set cpu option to sifive-u34
with zfinx=true. This may not be a proper way to set cpu option,
However it's truly a legal command option, but configure an illegal
supported ISA which enable both f and zfinx.
Regards,
Weiwei Li
>
> if (cpu->cfg.ext_zk) {
> @@ -663,7 +676,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> cpu->cfg.ext_zksh = true;
> }
>
> - /* Set the ISA extensions, checks should have happened above */
> if (cpu->cfg.ext_i) {
> ext |= RVI;
> }
> @@ -734,20 +746,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> }
> set_vext_version(env, vext_version);
> }
> - if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> - error_setg(errp, "Zve32f/Zve64f extension depends upon RVF.");
> - return;
> - }
> if (cpu->cfg.ext_j) {
> ext |= RVJ;
> }
> - if (cpu->cfg.ext_zfinx && ((ext & (RVF | RVD)) || cpu->cfg.ext_zfh ||
> - cpu->cfg.ext_zfhmin)) {
> - error_setg(errp,
> - "'Zfinx' cannot be supported together with 'F', 'D', 'Zfh',"
> - " 'Zfhmin'");
> - return;
> - }
>
> set_misa(env, env->misa_mxl, ext);
> }
[-- Attachment #2: Type: text/html, Size: 4059 bytes --]
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks
2022-05-15 2:56 ` [PATCH v2 5/5] target/riscv: Move/refactor ISA extension checks Tsukasa OI
2022-05-16 3:12 ` Weiwei Li
@ 2022-05-17 1:37 ` Alistair Francis
1 sibling, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2022-05-17 1:37 UTC (permalink / raw)
To: Tsukasa OI
Cc: Frank Chang, qemu-devel@nongnu.org Developers, open list:RISC-V
On Sun, May 15, 2022 at 12:56 PM Tsukasa OI
<research_trasio@irq.a4lg.com> wrote:
>
> We should separate "check" and "configure" steps as possible.
> This commit separates both steps except vector/Zfinx-related checks.
>
> Signed-off-by: Tsukasa OI <research_trasio@irq.a4lg.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu.c | 31 ++++++++++++++++---------------
> 1 file changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f910a41407..5ab246bf63 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -630,14 +630,27 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> return;
> }
>
> + if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> + error_setg(errp, "Zve32f/Zve64f extensions require F extension");
> + return;
> + }
> +
> + /* Set the ISA extensions, checks should have happened above */
> if (cpu->cfg.ext_zdinx || cpu->cfg.ext_zhinx ||
> cpu->cfg.ext_zhinxmin) {
> cpu->cfg.ext_zfinx = true;
> }
>
> - if (cpu->cfg.ext_zfinx && !cpu->cfg.ext_icsr) {
> - error_setg(errp, "Zfinx extension requires Zicsr");
> - return;
> + if (cpu->cfg.ext_zfinx) {
> + if (!cpu->cfg.ext_icsr) {
> + error_setg(errp, "Zfinx extension requires Zicsr");
> + return;
> + }
> + if (cpu->cfg.ext_f) {
> + error_setg(errp,
> + "Zfinx cannot be supported together with F extension");
> + return;
> + }
> }
>
> if (cpu->cfg.ext_zk) {
> @@ -663,7 +676,6 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> cpu->cfg.ext_zksh = true;
> }
>
> - /* Set the ISA extensions, checks should have happened above */
> if (cpu->cfg.ext_i) {
> ext |= RVI;
> }
> @@ -734,20 +746,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp)
> }
> set_vext_version(env, vext_version);
> }
> - if ((cpu->cfg.ext_zve32f || cpu->cfg.ext_zve64f) && !cpu->cfg.ext_f) {
> - error_setg(errp, "Zve32f/Zve64f extension depends upon RVF.");
> - return;
> - }
> if (cpu->cfg.ext_j) {
> ext |= RVJ;
> }
> - if (cpu->cfg.ext_zfinx && ((ext & (RVF | RVD)) || cpu->cfg.ext_zfh ||
> - cpu->cfg.ext_zfhmin)) {
> - error_setg(errp,
> - "'Zfinx' cannot be supported together with 'F', 'D', 'Zfh',"
> - " 'Zfhmin'");
> - return;
> - }
>
> set_misa(env, env->misa_mxl, ext);
> }
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 0/5] target/riscv: Enhanced ISA extension checks
2022-05-13 9:45 [PATCH 0/5] target/riscv: Enhanced ISA extension checks Tsukasa OI
` (5 preceding siblings ...)
2022-05-15 2:56 ` [PATCH v2 0/5] target/riscv: Enhanced " Tsukasa OI
@ 2022-05-17 2:17 ` Alistair Francis
6 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2022-05-17 2:17 UTC (permalink / raw)
To: Tsukasa OI
Cc: Frank Chang, qemu-devel@nongnu.org Developers, open list:RISC-V
On Fri, May 13, 2022 at 7:46 PM Tsukasa OI <research_trasio@irq.a4lg.com> wrote:
>
> Hello,
>
> This is another patchset for RISC-V ISA extension / feature handling.
>
> Aside from coding style fix / refactoring patch (PATCH 1 and 5), this
> patchset contains two changes:
>
>
>
> 1. "G" extension handling
>
> 1.A. "G" extension expansion (PATCH 3)
>
> On ISA version 20190608 or later, "G" expands to "IMAFD_Zicsr_Zifencei",
> not just "IMAFD" (this is because "Zicsr" and "Zifencei" extensions are
> splitted from "I"). Because both "Zicsr" and "Zifencei" are enabled by
> default, it should be safe to change "G" extension expansion.
>
> 1.B. Disable "G" by default (PATCH 2)
>
> This seems quite odd but I have a good reason. While "G" is enabled by
> default, all "I", "M", "A", "F", "D", "Zicsr" and "Zifencei" are also
> enabled by default, making default "G" expansion useless.
>
> There's more. If we want to change detailed configuration of a RV32/RV64
> CPU and want to disable some, for example, "F" and "D", we must also
> disable "G". This is obviously bloat.
>
> This doesn't work:
> -cpu rv64,f=off,d=off
>
> This does work (but bloat):
> -cpu rv64,g=off,f=off,d=off
>
> Disabling "G" suppresses such problem and mostly harmless, too.
>
>
>
> 2. Floating point arithmetic consistency (PATCH 4)
>
> With -cpu option, we can configure details of RISC-V CPU emulated by QEMU.
> However, we can set some invalid combinations that would make FP arithmetic
> effectively unusable (and invalid on RISC-V ISA specification).
>
> - F requires Zicsr
> - Zfinx requires Zicsr
> - Zfh/Zfhmin require F
> - D requires F
> - V requires D
>
> Reproducing this kind of problem requires manually disabling certain
> extensions (because all "Zicsr", "F" and "D" are enabled by default). So,
> I consider that just making an error message (when unsatisfied) should be
> enough, not implying related extensions like "zk*" properties.
>
>
> Note that this checking only works on any, rv32 and rv64. On other CPUs
> (for example, sifive-u54), it sets nonzero `misa' value on corresponding
> `set_misa' call (c.f. in rv64_sifive_u_cpu_init in target/riscv/cpu.c) and
> consistency checks are skipped (because nonzero `misa' value is set prior
> to RISC-V CPU realization process).
>
> I think we generally use generic "rv32" or "rv64" on heavy customizing so I
> don't think this is not a big problem. Still, we could fix this later
> (e.g. by setting properties on CPU init function or by checking some
> consistency problems even if previously-set `misa' is nonzero).
>
>
>
>
> Tsukasa OI (5):
> target/riscv: Fix "G" extension expansion typing
> target/riscv: Disable "G" by default
> target/riscv: Change "G" expansion
> target/riscv: FP extension requirements
> target/riscv: Move/refactor ISA extension checks
Thanks!
Applied to riscv-to-apply.next
Alistair
>
> target/riscv/cpu.c | 62 +++++++++++++++++++++++++++++++++-------------
> 1 file changed, 45 insertions(+), 17 deletions(-)
>
>
> base-commit: 178bacb66d98d9ee7a702b9f2a4dfcd88b72a9ab
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 27+ messages in thread