[PATCH v1 05/27] xen/riscv: introduce guest riscv,isa string

Oleksii Kurochko posted 27 patches 4 weeks ago
[PATCH v1 05/27] xen/riscv: introduce guest riscv,isa string
Posted by Oleksii Kurochko 4 weeks ago
Introduce generation of the riscv,isa string passed to the guest via the
Device Tree riscv,isa property.

The following changes are introduced:

- According to the RISC-V privileged specification, M-mode software controls
  the XLEN value used by S-mode and U-mode. For RV64 harts, the SXL and UXL
  fields of the mstatus register are WARL fields that define the XLEN for
  S-mode and U-mode.

  The XLEN value is provided by M-mode software (OpenSBI in the case of Xen)
  via the riscv,isa DT property. Introduce and initialize an xlen variable
  when parsing the host riscv,isa string in riscv_isa_parse_string().

- Introduce the guest_isa_str variable to store the riscv,isa string
  generated for a guest domain during riscv,isa property parsing. Update
  match_isa_ext() to populate guest_isa_str accordingly.

- Introduce guest_unsupp_bmp and guest_unsupp_exts[] to filter out ISA
  extensions that should not be exposed to guests. For example, FPU-related
  extensions are currently not supported for guests (at the moment) and are
  therefore removed from the guest riscv,isa string.

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
 xen/arch/riscv/cpufeature.c             | 33 +++++++++++++++++++++++++
 xen/arch/riscv/include/asm/cpufeature.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index 987d36dc7eee..d7e483603dbe 100644
--- a/xen/arch/riscv/cpufeature.c
+++ b/xen/arch/riscv/cpufeature.c
@@ -38,6 +38,8 @@ struct riscv_isa_ext_data {
 /* Host ISA bitmap */
 static __ro_after_init DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX);
 
+static __ro_after_init unsigned int xlen;
+
 static int __init dt_get_cpuid_from_node(const struct dt_device_node *cpu,
                                          unsigned long *dt_cpuid)
 {
@@ -160,6 +162,19 @@ static const struct riscv_isa_ext_data __initconst required_extensions[] = {
     RISCV_ISA_EXT_DATA(svpbmt),
 };
 
+static const unsigned int __initconst guest_unsupp_exts[] = {
+    RISCV_ISA_EXT_f,
+    RISCV_ISA_EXT_d,
+    RISCV_ISA_EXT_h,
+    RISCV_ISA_EXT_q,
+    RISCV_ISA_EXT_v,
+};
+
+static __ro_after_init DECLARE_BITMAP(guest_unsupp_bmp, RISCV_ISA_EXT_MAX);
+
+#define MAX_GUEST_ISA_STR_LEN 256
+char guest_isa_str[MAX_GUEST_ISA_STR_LEN];
+
 static bool __init is_lowercase_extension_name(const char *str)
 {
     /*
@@ -193,6 +208,15 @@ static void __init match_isa_ext(const char *name, const char *name_end,
              !memcmp(name, ext->name, name_end - name) )
         {
             __set_bit(ext->id, bitmap);
+
+            if ( riscv_isa_extension_available(guest_unsupp_bmp, ext->id) )
+                break;
+
+            if ( ext->id >= RISCV_ISA_EXT_BASE )
+                safe_strcat(guest_isa_str, "_");
+
+            safe_strcat(guest_isa_str, ext->name);
+
             break;
         }
     }
@@ -207,13 +231,17 @@ static int __init riscv_isa_parse_string(const char *isa,
 #if defined(CONFIG_RISCV_32)
     if ( isa[2] != '3' && isa[3] != '2' )
         return -EINVAL;
+    xlen = 32;
 #elif defined(CONFIG_RISCV_64)
     if ( isa[2] != '6' && isa[3] != '4' )
         return -EINVAL;
+    xlen = 64;
 #else
 # error "unsupported RISC-V bitness"
 #endif
 
+    snprintf(guest_isa_str, sizeof(guest_isa_str), "rv%d", xlen);
+
     /*
      * In unpriv. specification (*_20240411) is mentioned the following:
      * (1) A RISC-V ISA is defined as a base integer ISA, which must be
@@ -487,6 +515,11 @@ void __init riscv_fill_hwcap(void)
     bool all_extns_available = true;
     struct trap_info trap;
 
+    for ( i = 0; i < ARRAY_SIZE(guest_unsupp_exts); i++ )
+    {
+        __set_bit(guest_unsupp_exts[i], guest_unsupp_bmp);
+    }
+
     riscv_fill_hwcap_from_isa_string();
 
     if ( bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX) )
diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h
index ef02a3e26d2c..aabbbf0c2cc3 100644
--- a/xen/arch/riscv/include/asm/cpufeature.h
+++ b/xen/arch/riscv/include/asm/cpufeature.h
@@ -43,6 +43,8 @@ enum riscv_isa_ext_id {
     RISCV_ISA_EXT_MAX
 };
 
+extern char guest_isa_str[];
+
 void riscv_fill_hwcap(void);
 
 bool riscv_isa_extension_available(const unsigned long *isa_bitmap,
-- 
2.53.0
Re: [PATCH v1 05/27] xen/riscv: introduce guest riscv,isa string
Posted by Jan Beulich 6 days, 13 hours ago
On 10.03.2026 18:08, Oleksii Kurochko wrote:
> Introduce generation of the riscv,isa string passed to the guest via the
> Device Tree riscv,isa property.

Title as well as this suggest this is all about guest properties. Then ...

> The following changes are introduced:
> 
> - According to the RISC-V privileged specification, M-mode software controls
>   the XLEN value used by S-mode and U-mode. For RV64 harts, the SXL and UXL
>   fields of the mstatus register are WARL fields that define the XLEN for
>   S-mode and U-mode.
> 
>   The XLEN value is provided by M-mode software (OpenSBI in the case of Xen)
>   via the riscv,isa DT property. Introduce and initialize an xlen variable
>   when parsing the host riscv,isa string in riscv_isa_parse_string().

... suddenly talk is of host aspects? (See below as to what "xlen" really
is meant to hold.)

> --- a/xen/arch/riscv/cpufeature.c
> +++ b/xen/arch/riscv/cpufeature.c
> @@ -38,6 +38,8 @@ struct riscv_isa_ext_data {
>  /* Host ISA bitmap */
>  static __ro_after_init DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX);
>  
> +static __ro_after_init unsigned int xlen;

Nit: Attribute between type and identifier please, whenever possible (it
isn't neatly possible in riscv_isa above, due to DECLARE_BITMAP()).

> @@ -160,6 +162,19 @@ static const struct riscv_isa_ext_data __initconst required_extensions[] = {
>      RISCV_ISA_EXT_DATA(svpbmt),
>  };
>  
> +static const unsigned int __initconst guest_unsupp_exts[] = {
> +    RISCV_ISA_EXT_f,
> +    RISCV_ISA_EXT_d,
> +    RISCV_ISA_EXT_h,
> +    RISCV_ISA_EXT_q,
> +    RISCV_ISA_EXT_v,
> +};

This could do with a comment clarifying what needs (and what doesn't need)
putting here. My expectation would have been that everything in
riscv_isa_ext[] which shouldn't be exposed to guests should appear here.
Yet then there is V (which riscv_isa_ext[] doesn't have), while e.g. Svade
and Svpbmt (which iirc won't be available to guests right away) aren't
there.

> +static __ro_after_init DECLARE_BITMAP(guest_unsupp_bmp, RISCV_ISA_EXT_MAX);

Is the _bmp suffix really needed? riscv_isa, for example, doesn't have it.

> +#define MAX_GUEST_ISA_STR_LEN 256
> +char guest_isa_str[MAX_GUEST_ISA_STR_LEN];

__ro_after_init?

Yet then - can this really be a global? Isn't the set of extensions
available to a guest a per-guest property, i.e. a global could at best
represent an upper bound on features?

> @@ -193,6 +208,15 @@ static void __init match_isa_ext(const char *name, const char *name_end,
>               !memcmp(name, ext->name, name_end - name) )
>          {
>              __set_bit(ext->id, bitmap);
> +
> +            if ( riscv_isa_extension_available(guest_unsupp_bmp, ext->id) )
> +                break;
> +
> +            if ( ext->id >= RISCV_ISA_EXT_BASE )
> +                safe_strcat(guest_isa_str, "_");
> +
> +            safe_strcat(guest_isa_str, ext->name);

Shouldn't you check the (kind-of-)return value? (Yet better would be a build-
time check, but I can't think of a way to achieve that.)

> @@ -207,13 +231,17 @@ static int __init riscv_isa_parse_string(const char *isa,
>  #if defined(CONFIG_RISCV_32)
>      if ( isa[2] != '3' && isa[3] != '2' )
>          return -EINVAL;
> +    xlen = 32;
>  #elif defined(CONFIG_RISCV_64)
>      if ( isa[2] != '6' && isa[3] != '4' )
>          return -EINVAL;
> +    xlen = 64;
>  #else
>  # error "unsupported RISC-V bitness"
>  #endif

This can be had with an initializer of "xlen". Then the (kind-of-)variable
could be const unsigned int. Seeing the use below, is the variable
correctly named, though? I.e. shouldn't it be guest_xlen or some such?

Independently I expect you will want to support 32-bit guests on 64-bit Xen
at some point, in which case encoding this value into a global string won't
work very well.

> +    snprintf(guest_isa_str, sizeof(guest_isa_str), "rv%d", xlen);

%u please with unsigned int.

This being the only use of the variable (afaics), why is it not function-
scope?

> @@ -487,6 +515,11 @@ void __init riscv_fill_hwcap(void)
>      bool all_extns_available = true;
>      struct trap_info trap;
>  
> +    for ( i = 0; i < ARRAY_SIZE(guest_unsupp_exts); i++ )
> +    {
> +        __set_bit(guest_unsupp_exts[i], guest_unsupp_bmp);
> +    }

Nit: No need for braces here. And anyway - can't this be had with an
initializer for guest_unsupp_bmp?

Jan