H provides additional instructions and CSRs that control the new stage of
address translation and support hosting a guest OS in virtual S-mode
(VS-mode).

According to the Unprivileged Architecture (version 20240411) specification:
```
Table 74 summarizes the standardized extension names. The table also defines
the canonical order in which extension names must appear in the name string,
with top-to-bottom in table indicating first-to-last in the name string, e.g.,
RV32IMACV is legal, whereas RV32IMAVC is not.
```
According to Table 74, the h extension is placed last in the one-letter
extensions name part of the ISA string.

`h` is a standalone extension based on the patch [1] but it wasn't so
before.
As the minimal supported GCC version to build Xen for RISC-V is 12.2.0,
and for that version, h is still considered a prefix for the hypervisor
extension but the name of hypervisor extension must be more then 1 letter
extension, a workaround ( with using `hh` as an H extension name ) is
implemented as otherwise the following compilation error will occur:
 error: '-march=rv64gc_h_zbb_zihintpause': name of hypervisor extension
        must be more than 1 letter

After GCC version 13.1.0, the commit [1] introducing H extension support
allows us to drop the workaround with `hh` as hypervisor extension name
and use only one h in -march.

To implement this, the h-extension-name is introduced, which is filled with
hh or h depending on the GCC version.

[1] https://github.com/gcc-mirror/gcc/commit/0cd11d301013af50a3fae0694c909952e94e20d5#diff-d6f7db0db31bfb339b01bec450f1b905381eb4730cc5ab2b2794971e34647d64R148

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 docs/misc/riscv/booting.txt | 4 ++++
 xen/arch/riscv/arch.mk      | 3 ++-
 xen/arch/riscv/cpufeature.c | 1 +
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/docs/misc/riscv/booting.txt b/docs/misc/riscv/booting.txt
index cb4d79f12c..3a8474a27d 100644
--- a/docs/misc/riscv/booting.txt
+++ b/docs/misc/riscv/booting.txt
@@ -3,6 +3,10 @@ System requirements
 
 The following extensions are expected to be supported by a system on which
 Xen is run:
+- H:
+  Provides additional instructions and CSRs that control the new stage of
+  address translation and support hosting a guest OS in virtual S-mode
+  (VS-mode).
 - Zbb:
   RISC-V doesn't have a CLZ instruction in the base ISA.
   As a consequence, __builtin_ffs() emits a library call to ffs() on GCC,
diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk
index 236ea7c8a6..14d1f0ada0 100644
--- a/xen/arch/riscv/arch.mk
+++ b/xen/arch/riscv/arch.mk
@@ -9,7 +9,8 @@ riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64
 riscv-march-$(CONFIG_RISCV_64) := rv64
 riscv-march-y += ima
 riscv-march-$(CONFIG_RISCV_ISA_C) += c
-riscv-march-y += _zicsr_zifencei_zbb
+h-extension-name := $(call cc-ifversion,-lt,1301, hh, h)
+riscv-march-y += $(h-extension-name)_zicsr_zifencei_zbb
 
 riscv-generic-flags := $(riscv-abi-y) -march=$(subst $(space),,$(riscv-march-y))
 
diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index bf09aa1170..5aafab0f49 100644
--- a/xen/arch/riscv/cpufeature.c
+++ b/xen/arch/riscv/cpufeature.c
@@ -146,6 +146,7 @@ static const struct riscv_isa_ext_data __initconst required_extensions[] = {
 #ifdef CONFIG_RISCV_ISA_C
     RISCV_ISA_EXT_DATA(c),
 #endif
+    RISCV_ISA_EXT_DATA(h),
     RISCV_ISA_EXT_DATA(zicsr),
     RISCV_ISA_EXT_DATA(zifencei),
     RISCV_ISA_EXT_DATA(zihintpause),
-- 
2.48.1

On 11.03.2025 16:45, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/arch.mk
> +++ b/xen/arch/riscv/arch.mk
> @@ -9,7 +9,8 @@ riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64
>  riscv-march-$(CONFIG_RISCV_64) := rv64
>  riscv-march-y += ima
>  riscv-march-$(CONFIG_RISCV_ISA_C) += c
> -riscv-march-y += _zicsr_zifencei_zbb
> +h-extension-name := $(call cc-ifversion,-lt,1301, hh, h)

Instead of a version check, did you consider probing the compiler? With the
hard-coded version, how are things going to work with Clang?

Jan

On 3/11/25 4:48 PM, Jan Beulich wrote:
> On 11.03.2025 16:45, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/arch.mk
>> +++ b/xen/arch/riscv/arch.mk
>> @@ -9,7 +9,8 @@ riscv-abi-$(CONFIG_RISCV_64) := -mabi=lp64
>>   riscv-march-$(CONFIG_RISCV_64) := rv64
>>   riscv-march-y += ima
>>   riscv-march-$(CONFIG_RISCV_ISA_C) += c
>> -riscv-march-y += _zicsr_zifencei_zbb
>> +h-extension-name := $(call cc-ifversion,-lt,1301, hh, h)
> Instead of a version check, did you consider probing the compiler? With the
> hard-coded version, how are things going to work with Clang?

Initially, it was implemented using:

+$(h-extension-name)-insn := "hfence.gvma"
+$(call check-extension,$(h-extension-name))

with

+h-extension-name := $(call cc-ifversion,-lt,1301, hh, h)

But it seems that we still need this hard-coded version for h-extension-name
because of this behavior that h extensions should be longer then single letter
for some compilers.

Probably, we could consider option to check "hfence.gvma" twice:
h-insn := "hfence.gvma"
$(call check-extension,h)

hh-insn := "hfence.gvma"
$(call check-extension,hh)

And filter-out/skip the second check-extension if the previous one check succeeded.
But it looks a weird.

~ Oleksii