[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/5] target/riscv: Disable "G" by default

From: Víctor Colombo
Subject: Re: [PATCH v2 2/5] target/riscv: Disable "G" by default
Date: Tue, 24 May 2022 12:48:09 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.1

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

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?


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.


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>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]