Message ID | 20231003122539.775932-1-dbarboza@ventanamicro.com |
---|---|
State | New |
Headers | show |
Series | target/riscv/tcg: remove RVG warning | expand |
On Tue, Oct 03, 2023 at 09:25:39AM -0300, Daniel Henrique Barboza wrote: > Vendor CPUs that set RVG are displaying user warnings about other > extensions that RVG must enable, one warning per CPU. E.g.: > > $ ./build/qemu-system-riscv64 -smp 8 -M virt -cpu veyron-v1 -nographic > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > > This happens because we decided a while ago that, for simplicity, vendor > CPUs could set RVG instead of setting each G extension individually in > their cpu_init(). Our warning isn't taking that into account, and we're > bugging users with a warning that we're causing ourselves. > > In a closer look we conclude that this warning is not warranted in any > other circumstance since we're just following the ISA [1], which states > in chapter 24: > > "One goal of the RISC-V project is that it be used as a stable software > development target. For this purpose, we define a combination of a base > ISA (RV32I or RV64I) plus selected standard extensions (IMAFD, Zicsr, > Zifencei) as a 'general-purpose' ISA, and we use the abbreviation G for > the IMAFDZicsr Zifencei combination of instruction-set extensions." > > With this in mind, enabling IMAFD_Zicsr_Zifencei if the user explicitly > enables 'G' is an expected behavior and the warning is unneeded. Any > user caught by surprise should refer to the ISA. > > Remove the warning when handling RVG. > > [1] https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf > > Reported-by: Paul A. Clarke <pclarke@ventanamicro.com> > Suggested-by: Andrew Jones <ajones@ventanamicro.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > --- > target/riscv/tcg/tcg-cpu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 08b806dc07..f50ce57602 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -293,7 +293,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > return; > } > > - warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_icsr), true); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_ifencei), true); > > -- > 2.41.0 > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
On Tue, Oct 3, 2023 at 10:26 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > Vendor CPUs that set RVG are displaying user warnings about other > extensions that RVG must enable, one warning per CPU. E.g.: > > $ ./build/qemu-system-riscv64 -smp 8 -M virt -cpu veyron-v1 -nographic > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > > This happens because we decided a while ago that, for simplicity, vendor > CPUs could set RVG instead of setting each G extension individually in > their cpu_init(). Our warning isn't taking that into account, and we're > bugging users with a warning that we're causing ourselves. > > In a closer look we conclude that this warning is not warranted in any > other circumstance since we're just following the ISA [1], which states > in chapter 24: > > "One goal of the RISC-V project is that it be used as a stable software > development target. For this purpose, we define a combination of a base > ISA (RV32I or RV64I) plus selected standard extensions (IMAFD, Zicsr, > Zifencei) as a 'general-purpose' ISA, and we use the abbreviation G for > the IMAFDZicsr Zifencei combination of instruction-set extensions." > > With this in mind, enabling IMAFD_Zicsr_Zifencei if the user explicitly > enables 'G' is an expected behavior and the warning is unneeded. Any > user caught by surprise should refer to the ISA. > > Remove the warning when handling RVG. > > [1] https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf > > Reported-by: Paul A. Clarke <pclarke@ventanamicro.com> > Suggested-by: Andrew Jones <ajones@ventanamicro.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/tcg/tcg-cpu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 08b806dc07..f50ce57602 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -293,7 +293,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > return; > } > > - warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_icsr), true); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_ifencei), true); > > -- > 2.41.0 > >
On Tue, Oct 3, 2023 at 10:26 PM Daniel Henrique Barboza <dbarboza@ventanamicro.com> wrote: > > Vendor CPUs that set RVG are displaying user warnings about other > extensions that RVG must enable, one warning per CPU. E.g.: > > $ ./build/qemu-system-riscv64 -smp 8 -M virt -cpu veyron-v1 -nographic > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei > > This happens because we decided a while ago that, for simplicity, vendor > CPUs could set RVG instead of setting each G extension individually in > their cpu_init(). Our warning isn't taking that into account, and we're > bugging users with a warning that we're causing ourselves. > > In a closer look we conclude that this warning is not warranted in any > other circumstance since we're just following the ISA [1], which states > in chapter 24: > > "One goal of the RISC-V project is that it be used as a stable software > development target. For this purpose, we define a combination of a base > ISA (RV32I or RV64I) plus selected standard extensions (IMAFD, Zicsr, > Zifencei) as a 'general-purpose' ISA, and we use the abbreviation G for > the IMAFDZicsr Zifencei combination of instruction-set extensions." > > With this in mind, enabling IMAFD_Zicsr_Zifencei if the user explicitly > enables 'G' is an expected behavior and the warning is unneeded. Any > user caught by surprise should refer to the ISA. > > Remove the warning when handling RVG. > > [1] https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf > > Reported-by: Paul A. Clarke <pclarke@ventanamicro.com> > Suggested-by: Andrew Jones <ajones@ventanamicro.com> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> Thanks! Applied to riscv-to-apply.next Alistair > --- > target/riscv/tcg/tcg-cpu.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c > index 08b806dc07..f50ce57602 100644 > --- a/target/riscv/tcg/tcg-cpu.c > +++ b/target/riscv/tcg/tcg-cpu.c > @@ -293,7 +293,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) > return; > } > > - warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_icsr), true); > cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_ifencei), true); > > -- > 2.41.0 > >
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c index 08b806dc07..f50ce57602 100644 --- a/target/riscv/tcg/tcg-cpu.c +++ b/target/riscv/tcg/tcg-cpu.c @@ -293,7 +293,6 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp) return; } - warn_report("Setting G will also set IMAFD_Zicsr_Zifencei"); cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_icsr), true); cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_ifencei), true);
Vendor CPUs that set RVG are displaying user warnings about other extensions that RVG must enable, one warning per CPU. E.g.: $ ./build/qemu-system-riscv64 -smp 8 -M virt -cpu veyron-v1 -nographic qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei qemu-system-riscv64: warning: Setting G will also set IMAFD_Zicsr_Zifencei This happens because we decided a while ago that, for simplicity, vendor CPUs could set RVG instead of setting each G extension individually in their cpu_init(). Our warning isn't taking that into account, and we're bugging users with a warning that we're causing ourselves. In a closer look we conclude that this warning is not warranted in any other circumstance since we're just following the ISA [1], which states in chapter 24: "One goal of the RISC-V project is that it be used as a stable software development target. For this purpose, we define a combination of a base ISA (RV32I or RV64I) plus selected standard extensions (IMAFD, Zicsr, Zifencei) as a 'general-purpose' ISA, and we use the abbreviation G for the IMAFDZicsr Zifencei combination of instruction-set extensions." With this in mind, enabling IMAFD_Zicsr_Zifencei if the user explicitly enables 'G' is an expected behavior and the warning is unneeded. Any user caught by surprise should refer to the ISA. Remove the warning when handling RVG. [1] https://github.com/riscv/riscv-isa-manual/releases/download/Ratified-IMAFDQC/riscv-spec-20191213.pdf Reported-by: Paul A. Clarke <pclarke@ventanamicro.com> Suggested-by: Andrew Jones <ajones@ventanamicro.com> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> --- target/riscv/tcg/tcg-cpu.c | 1 - 1 file changed, 1 deletion(-)