| Message ID | 82c9611b923170b0525a7b76337ef067e359dc96.1739355004.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V runtime detection of extenstions | expand |
On 12.02.2025 17:50, Oleksii Kurochko wrote: > --- a/xen/arch/riscv/Kconfig > +++ b/xen/arch/riscv/Kconfig > @@ -28,16 +28,6 @@ choice > help > This selects the base ISA extensions that Xen will target. > > -config RISCV_ISA_RV64G > - bool "RV64G" > - help > - Use the RV64I base ISA, plus > - "M" for multiply/divide, > - "A" for atomic instructions, > - “F”/"D" for {single/double}-precision floating-point instructions, > - "Zicsr" for control and status register access, > - "Zifencei" for instruction-fetch fence. > - > endchoice Shouldn't the choice be removed altogether then, for now being empty? > --- a/xen/arch/riscv/arch.mk > +++ b/xen/arch/riscv/arch.mk > @@ -6,8 +6,13 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) > riscv-abi-$(CONFIG_RISCV_32) := -mabi=ilp32 > riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64 > > -riscv-march-$(CONFIG_RISCV_ISA_RV64G) := rv64g > -riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c > +riscv-march-$(CONFIG_RISCV_64) := rv64 > + > +riscv-march-y := $(riscv-march-y)ima > + > +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c > + > +riscv-march-y := $(riscv-march-y)_zicsr_zifencei The repeated use of := makes this longer than necessary, and hence harder to read. I understand using += isn't exactly ideal either, because then on the rhs no blanks may appear (aiui), being kind of against our style and potentially hampering readability. Still maybe: riscv-march-$(CONFIG_RISCV_64) := rv64 riscv-march-y+=ima riscv-march-$(CONFIG_RISCV_ISA_C)+=c riscv-march-y+=_zicsr_zifencei ? Jan
On 2/18/25 6:03 PM, Jan Beulich wrote: > On 12.02.2025 17:50, Oleksii Kurochko wrote: >> --- a/xen/arch/riscv/Kconfig >> +++ b/xen/arch/riscv/Kconfig >> @@ -28,16 +28,6 @@ choice >> help >> This selects the base ISA extensions that Xen will target. >> >> -config RISCV_ISA_RV64G >> - bool "RV64G" >> - help >> - Use the RV64I base ISA, plus >> - "M" for multiply/divide, >> - "A" for atomic instructions, >> - “F”/"D" for {single/double}-precision floating-point instructions, >> - "Zicsr" for control and status register access, >> - "Zifencei" for instruction-fetch fence. >> - >> endchoice > Shouldn't the choice be removed altogether then, for now being empty? Overlooked that, "Base ISA" choice could be removed too then. or just change to: choice prompt "Base ISA" default "ima" if RISCV_64 help This selects the base ISA extensions that Xen will target. endchoice > >> --- a/xen/arch/riscv/arch.mk >> +++ b/xen/arch/riscv/arch.mk >> @@ -6,8 +6,13 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) >> riscv-abi-$(CONFIG_RISCV_32) := -mabi=ilp32 >> riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64 >> >> -riscv-march-$(CONFIG_RISCV_ISA_RV64G) := rv64g >> -riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c >> +riscv-march-$(CONFIG_RISCV_64) := rv64 >> + >> +riscv-march-y := $(riscv-march-y)ima >> + >> +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c >> + >> +riscv-march-y := $(riscv-march-y)_zicsr_zifencei > The repeated use of := makes this longer than necessary, and hence harder to > read. I understand using += isn't exactly ideal either, because then on the rhs > no blanks may appear (aiui), being kind of against our style and potentially > hampering readability. Still maybe: > > riscv-march-$(CONFIG_RISCV_64) := rv64 > riscv-march-y+=ima > riscv-march-$(CONFIG_RISCV_ISA_C)+=c > riscv-march-y+=_zicsr_zifencei > > ? From my point of view both options hard to read but `+=`, at the moment, look a little bit better. I will update correspondingly. Thanks. ~ Oleksii
On 19.02.2025 15:55, Oleksii Kurochko wrote: > On 2/18/25 6:03 PM, Jan Beulich wrote: >> On 12.02.2025 17:50, Oleksii Kurochko wrote: >>> --- a/xen/arch/riscv/Kconfig >>> +++ b/xen/arch/riscv/Kconfig >>> @@ -28,16 +28,6 @@ choice >>> help >>> This selects the base ISA extensions that Xen will target. >>> >>> -config RISCV_ISA_RV64G >>> - bool "RV64G" >>> - help >>> - Use the RV64I base ISA, plus >>> - "M" for multiply/divide, >>> - "A" for atomic instructions, >>> - “F”/"D" for {single/double}-precision floating-point instructions, >>> - "Zicsr" for control and status register access, >>> - "Zifencei" for instruction-fetch fence. >>> - >>> endchoice >> Shouldn't the choice be removed altogether then, for now being empty? > > Overlooked that, "Base ISA" choice could be removed too then. or just change to: > choice > prompt "Base ISA" > default "ima" if RISCV_64 > help > This selects the base ISA extensions that Xen will target. > > endchoice Besides me wondering what use that would be (there's no variable to store the "ima" into), I kind of suspect kconfig might choke on the construct. Plus even if there was some variable, I'd then ask where it is used. There isn't a lot of sense in having a Kconfig setting with no user. Jan
On 2/18/25 6:03 PM, Jan Beulich wrote: >> --- a/xen/arch/riscv/arch.mk >> +++ b/xen/arch/riscv/arch.mk >> @@ -6,8 +6,13 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) >> riscv-abi-$(CONFIG_RISCV_32) := -mabi=ilp32 >> riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64 >> >> -riscv-march-$(CONFIG_RISCV_ISA_RV64G) := rv64g >> -riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c >> +riscv-march-$(CONFIG_RISCV_64) := rv64 >> + >> +riscv-march-y := $(riscv-march-y)ima >> + >> +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c >> + >> +riscv-march-y := $(riscv-march-y)_zicsr_zifencei > The repeated use of := makes this longer than necessary, and hence harder to > read. I understand using += isn't exactly ideal either, because then on the rhs > no blanks may appear (aiui), being kind of against our style and potentially > hampering readability. Still maybe: > > riscv-march-$(CONFIG_RISCV_64) := rv64 > riscv-march-y+=ima > riscv-march-$(CONFIG_RISCV_ISA_C)+=c > riscv-march-y+=_zicsr_zifencei > > ? Btw, I think that we will still anyway strip spaces added by '+='. So it will also need to do something like: [1] riscv-generic-flags := $(riscv-abi-y) -march=$(subst $(space),,$(riscv-march-y)) As without this I expect that -march will look like: -march=rv64 ima c _zicsr_zifencei With the change [1] we could have spaces around "+=": riscv-march-y += ima riscv-march-$(CONFIG_RISCV_ISA_C) += c riscv-march-y += _zicsr_zifencei riscv-generic-flags := $(riscv-abi-y) -march=$(subst $(space),,$(riscv-march-y)) ~ Oleksii
diff --git a/xen/arch/riscv/Kconfig b/xen/arch/riscv/Kconfig index 00f329054c..5b72937139 100644 --- a/xen/arch/riscv/Kconfig +++ b/xen/arch/riscv/Kconfig @@ -28,16 +28,6 @@ choice help This selects the base ISA extensions that Xen will target. -config RISCV_ISA_RV64G - bool "RV64G" - help - Use the RV64I base ISA, plus - "M" for multiply/divide, - "A" for atomic instructions, - “F”/"D" for {single/double}-precision floating-point instructions, - "Zicsr" for control and status register access, - "Zifencei" for instruction-fetch fence. - endchoice config RISCV_ISA_C diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk index 17827c302c..1819ec17eb 100644 --- a/xen/arch/riscv/arch.mk +++ b/xen/arch/riscv/arch.mk @@ -6,8 +6,13 @@ $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS)) riscv-abi-$(CONFIG_RISCV_32) := -mabi=ilp32 riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64 -riscv-march-$(CONFIG_RISCV_ISA_RV64G) := rv64g -riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c +riscv-march-$(CONFIG_RISCV_64) := rv64 + +riscv-march-y := $(riscv-march-y)ima + +riscv-march-$(CONFIG_RISCV_ISA_C) := $(riscv-march-y)c + +riscv-march-y := $(riscv-march-y)_zicsr_zifencei riscv-generic-flags := $(riscv-abi-y) -march=$(riscv-march-y)
'G' stands for "imafd_zicsr_zifencei". Extensions 'f' and 'd' aren't really needed for Xen, and allowing floating point registers to be used can lead to crashes. Extensions 'i', 'm', 'a', 'zicsr', and 'zifencei' are necessary for the operation of Xen, which is why they are used explicitly (unconditionally) in -march. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- Changes in v6: - new patch for the patch series. --- xen/arch/riscv/Kconfig | 10 ---------- xen/arch/riscv/arch.mk | 9 +++++++-- 2 files changed, 7 insertions(+), 12 deletions(-)