'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(-)
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)
--
2.48.1
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: >> --- 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
On 19.02.2025 18:56, Oleksii Kurochko wrote: > > 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)) That would be fine with me of course, for being yet tidier (imo). 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
© 2016 - 2025 Red Hat, Inc.