[PATCH v5 09/26] xen/riscv: introduce guest riscv,isa string

Oleksii Kurochko posted 26 patches 1 month ago
There is a newer version of this series
[PATCH v5 09/26] xen/riscv: introduce guest riscv,isa string
Posted by Oleksii Kurochko 1 month ago
Introduce build_guest_isa_str() to generate the riscv,isa string to be
passed to the guest via the Device Tree riscv,isa property.

Introduce the per-domain guest ISA bitmap, populated during domain
creation by calling init_guest_isa().

Introduce struct riscv_isa_ext_entry with a new guest_supported field
to filter out ISA extensions that should not be exposed to guests:

- f/d/q/v: FPU and vector context save/restore are not yet implemented
  for guests.
- Z*inx are not exposed either: they aren't in riscv_isa_ext[], so they
  can never be set in riscv_isa and thus never reach a guest, and no
  current hardware/guest-OS advertises or expects them. Supporting them
  would be cheaper than F/D/Q (FP values stay in integer registers Xen
  already context-switches), but is left as future work.
- h: Nested virtualisation is not supported.
- sstc: Xen owns the supervisor timer; guests must use SBI.
- svade: Xen manages hardware A/D bit updates in stage-2 page tables.
- svpbmt: Page-based memory types are not yet wired up in stage-2 code.

Drop __initconst for riscv_isa_ext[] as it can be used after init stage
by init_guest_isa().

Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in v5:
- Introduce struct riscv_isa_ext_entry with a guest_supported field and
  RISCV_ISA_EXT_ENTRY(name, guest_supp) macro for riscv_isa_ext[],
  replacing the ad-hoc guest_unsupp bitmap and init_guest_unsupp().
  Every entry now carries an explicit true/false decision, enforced at
  compile time.
- init_guest_isa() builds d->arch.isa by iterating riscv_isa_ext[]
  directly instead of using bitmap_andnot() against guest_unsupp.
- init_guest_isa() changed to void as it can no longer fail.
- Drop isa_str from struct arch_domain; the ISA string does not need to
  persist over the domain lifetime. build_guest_isa_str() is made
  non-static and declared in cpufeature.h for use when building the
  guest device tree.
- Updated the fix of underflow in build_guest_isa_str().
- Drop unnecessary empty line in cpufeature.h before enum riscv_isa_ext_id.
---
Changes in v4:
 - Add an explicit overflow guard in build_guest_isa_str(): return
   -ENOSPC when buf is non-NULL and total >= size, to avoid the
   size - total underflow being passed to snprintf().
 - Expand the commit message to explain why Zfinx/Zdinx/Zqinx are not
   added to guest_unsupp (not in riscv_isa_ext[], so never set in
   riscv_isa nor exposed to a guest; left as future work)
---
Changes in v3:
 - s/set_bit/__set_bit in init_guest_unsupp() as atomicity isn't needed at
   init time.
 - Drop RISCV_GUEST_ISA_STR_MAX; allocate isa_str dynamically with
   xvmalloc_array().
 - Drop "guest" prefix from d->arch.guest_isa and d->arch.guest_isa_str.
 - Introduce build_guest_isa_str() using snprintf(NULL, 0, ...) to determine
   the needed buffer size; init_guest_isa() calls it once for sizing and once
   to fill, keeping both in a single function so they can't go out of sync.
 - Scope ret inside the loop; initialize total directly from the prefix
   snprintf().
 - Merge "_" separator and extension name into a single snprintf() with
   "%s%s".
 - Replace ASSERT with an explicit error check: if the fill call returns a
   different length, free isa_str and return -EINVAL.
---
Changes in v2:
 - s/guest_unsupp_bmp/guest_unsupp.
 - Drop guest_isa_str.
 - Provide init_guest_isa() instead of polluting match_isa_ext().
 - Drop xlen.
 - Add the comment about guest_unsupp.
 - Update the way how guest_unsupp is init-ed.
 - Drop __initconst for riscv_isa_ext[] as it is used in init_guest_isa()
   which isn't marked as __init as it could be used after init stage.
---
---
 xen/arch/riscv/cpufeature.c             | 132 +++++++++++++++++++-----
 xen/arch/riscv/domain.c                 |   2 +
 xen/arch/riscv/include/asm/cpufeature.h |   6 ++
 xen/arch/riscv/include/asm/domain.h     |   3 +
 4 files changed, 119 insertions(+), 24 deletions(-)

diff --git a/xen/arch/riscv/cpufeature.c b/xen/arch/riscv/cpufeature.c
index 92235fdfd5ab..5002ddc92a22 100644
--- a/xen/arch/riscv/cpufeature.c
+++ b/xen/arch/riscv/cpufeature.c
@@ -14,6 +14,7 @@
 #include <xen/errno.h>
 #include <xen/init.h>
 #include <xen/lib.h>
+#include <xen/sched.h>
 #include <xen/sections.h>
 
 #include <asm/cpufeature.h>
@@ -34,6 +35,19 @@ struct riscv_isa_ext_data {
     .name = #ext_name,                          \
 }
 
+struct riscv_isa_ext_entry {
+    unsigned int id;
+    const char *name;
+    bool guest_supported;
+};
+
+#define RISCV_ISA_EXT_ENTRY(ext_name, guest_supp)       \
+{                                                       \
+    .id              = RISCV_ISA_EXT_ ## ext_name,      \
+    .name            = #ext_name,                       \
+    .guest_supported = guest_supp,                      \
+}
+
 /* Host ISA bitmap */
 static __ro_after_init DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX);
 
@@ -120,29 +134,30 @@ static int __init dt_get_cpuid_from_node(const struct dt_device_node *cpu,
  * and strncmp() is used in match_isa_ext() to compare extension names instead
  * of strncasecmp().
  */
-const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = {
-    RISCV_ISA_EXT_DATA(i),
-    RISCV_ISA_EXT_DATA(m),
-    RISCV_ISA_EXT_DATA(a),
-    RISCV_ISA_EXT_DATA(f),
-    RISCV_ISA_EXT_DATA(d),
-    RISCV_ISA_EXT_DATA(q),
-    RISCV_ISA_EXT_DATA(c),
-    RISCV_ISA_EXT_DATA(h),
-    RISCV_ISA_EXT_DATA(zicntr),
-    RISCV_ISA_EXT_DATA(zicsr),
-    RISCV_ISA_EXT_DATA(zifencei),
-    RISCV_ISA_EXT_DATA(zihintpause),
-    RISCV_ISA_EXT_DATA(zihpm),
-    RISCV_ISA_EXT_DATA(zba),
-    RISCV_ISA_EXT_DATA(zbb),
-    RISCV_ISA_EXT_DATA(zbs),
-    RISCV_ISA_EXT_DATA(smaia),
-    RISCV_ISA_EXT_DATA(smstateen),
-    RISCV_ISA_EXT_DATA(ssaia),
-    RISCV_ISA_EXT_DATA(sstc),
-    RISCV_ISA_EXT_DATA(svade),
-    RISCV_ISA_EXT_DATA(svpbmt),
+const struct riscv_isa_ext_entry riscv_isa_ext[] = {
+    RISCV_ISA_EXT_ENTRY(i,            true),
+    RISCV_ISA_EXT_ENTRY(m,            true),
+    RISCV_ISA_EXT_ENTRY(a,            true),
+    RISCV_ISA_EXT_ENTRY(f,            false),
+    RISCV_ISA_EXT_ENTRY(d,            false),
+    RISCV_ISA_EXT_ENTRY(q,            false),
+    RISCV_ISA_EXT_ENTRY(c,            true),
+    RISCV_ISA_EXT_ENTRY(v,            false),
+    RISCV_ISA_EXT_ENTRY(h,            false),
+    RISCV_ISA_EXT_ENTRY(zicntr,       true),
+    RISCV_ISA_EXT_ENTRY(zicsr,        true),
+    RISCV_ISA_EXT_ENTRY(zifencei,     true),
+    RISCV_ISA_EXT_ENTRY(zihintpause,  true),
+    RISCV_ISA_EXT_ENTRY(zihpm,        true),
+    RISCV_ISA_EXT_ENTRY(zba,          true),
+    RISCV_ISA_EXT_ENTRY(zbb,          true),
+    RISCV_ISA_EXT_ENTRY(zbs,          true),
+    RISCV_ISA_EXT_ENTRY(smaia,        true),
+    RISCV_ISA_EXT_ENTRY(smstateen,    true),
+    RISCV_ISA_EXT_ENTRY(ssaia,        true),
+    RISCV_ISA_EXT_ENTRY(sstc,         false),
+    RISCV_ISA_EXT_ENTRY(svade,        false),
+    RISCV_ISA_EXT_ENTRY(svpbmt,       false),
 };
 
 static const struct riscv_isa_ext_data __initconst required_extensions[] = {
@@ -181,7 +196,7 @@ static void __init match_isa_ext(const char *name, const char *name_end,
 
     for ( unsigned int i = 0; i < riscv_isa_ext_count; i++ )
     {
-        const struct riscv_isa_ext_data *ext = &riscv_isa_ext[i];
+        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
 
         /*
          * `ext->name` (according to initialization of riscv_isa_ext[]
@@ -480,6 +495,74 @@ bool riscv_isa_extension_available(const unsigned long *isa_bitmap,
     return test_bit(id, isa_bitmap);
 }
 
+int build_guest_isa_str(char *buf, size_t size,
+                        const unsigned long *isa_bitmap)
+{
+    char *p = buf;
+    size_t left = size;
+    int total;
+
+#if defined(CONFIG_RISCV_32)
+    total = snprintf(p, left, "rv32");
+#elif defined(CONFIG_RISCV_64)
+    total = snprintf(p, left, "rv64");
+#else
+# error "Unsupported RISC-V bitness"
+#endif
+
+    if ( total < 0 )
+        return total;
+
+    if ( buf )
+    {
+        if ( (size_t)total >= left )
+            return -ENOSPC;
+
+        p += total;
+        left -= total;
+    }
+
+    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
+    {
+        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
+        int ret;
+
+        if ( !riscv_isa_extension_available(isa_bitmap, ext->id) )
+            continue;
+
+        ret = snprintf(p, left, "%s%s",
+                       ext->id >= RISCV_ISA_EXT_BASE ? "_" : "",
+                       ext->name);
+        if ( ret < 0 )
+            return ret;
+
+        total += ret;
+
+        if ( buf )
+        {
+            if ( (size_t)ret >= left )
+                return -ENOSPC;
+
+            p += ret;
+            left -= ret;
+        }
+    }
+
+    return total;
+}
+
+void init_guest_isa(struct domain *d)
+{
+    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
+    {
+        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
+
+        if ( ext->guest_supported &&
+             riscv_isa_extension_available(NULL, ext->id) )
+            __set_bit(ext->id, d->arch.isa);
+    }
+}
+
 void __init riscv_fill_hwcap(void)
 {
     unsigned int i;
@@ -527,4 +610,5 @@ void __init riscv_fill_hwcap(void)
     if ( !all_extns_available )
         panic("Look why the extensions above are needed in "
               "https://xenbits.xenproject.org/docs/unstable/misc/riscv/booting.txt\n");
+
 }
diff --git a/xen/arch/riscv/domain.c b/xen/arch/riscv/domain.c
index 2819ff4e7c92..c9933147595e 100644
--- a/xen/arch/riscv/domain.c
+++ b/xen/arch/riscv/domain.c
@@ -308,6 +308,8 @@ int arch_domain_create(struct domain *d,
     if ( is_idle_domain(d) )
         return 0;
 
+    init_guest_isa(d);
+
     if ( (rc = p2m_init(d, config)) != 0)
         goto fail;
 
diff --git a/xen/arch/riscv/include/asm/cpufeature.h b/xen/arch/riscv/include/asm/cpufeature.h
index 0c48d57a03bb..e26f5f0b66fb 100644
--- a/xen/arch/riscv/include/asm/cpufeature.h
+++ b/xen/arch/riscv/include/asm/cpufeature.h
@@ -5,6 +5,7 @@
 #ifndef __ASSEMBLER__
 
 #include <xen/stdbool.h>
+#include <xen/types.h>
 
 /*
  * These macros represent the logical IDs of each multi-letter RISC-V ISA
@@ -44,7 +45,12 @@ enum riscv_isa_ext_id {
     RISCV_ISA_EXT_MAX
 };
 
+struct domain;
+
 void riscv_fill_hwcap(void);
+void init_guest_isa(struct domain *d);
+int build_guest_isa_str(char *buf, size_t size,
+                        const unsigned long *isa_bitmap);
 
 bool riscv_isa_extension_available(const unsigned long *isa_bitmap,
                                    enum riscv_isa_ext_id id);
diff --git a/xen/arch/riscv/include/asm/domain.h b/xen/arch/riscv/include/asm/domain.h
index 6044ce0feee0..235b20f8a6ba 100644
--- a/xen/arch/riscv/include/asm/domain.h
+++ b/xen/arch/riscv/include/asm/domain.h
@@ -7,6 +7,7 @@
 #include <xen/xmalloc.h>
 #include <public/hvm/params.h>
 
+#include <asm/cpufeature.h>
 #include <asm/guest-layout.h>
 #include <asm/p2m.h>
 #include <asm/vtimer.h>
@@ -94,6 +95,8 @@ struct arch_domain {
     struct p2m_domain p2m;
 
     struct paging_domain paging;
+
+    DECLARE_BITMAP(isa, RISCV_ISA_EXT_MAX);
 };
 
 #include <xen/sched.h>
-- 
2.54.0
Re: [PATCH v5 09/26] xen/riscv: introduce guest riscv,isa string
Posted by Jan Beulich 1 month ago
On 06.07.2026 17:57, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/cpufeature.c
> +++ b/xen/arch/riscv/cpufeature.c
> @@ -14,6 +14,7 @@
>  #include <xen/errno.h>
>  #include <xen/init.h>
>  #include <xen/lib.h>
> +#include <xen/sched.h>
>  #include <xen/sections.h>
>  
>  #include <asm/cpufeature.h>
> @@ -34,6 +35,19 @@ struct riscv_isa_ext_data {
>      .name = #ext_name,                          \
>  }
>  
> +struct riscv_isa_ext_entry {
> +    unsigned int id;
> +    const char *name;
> +    bool guest_supported;
> +};
> +
> +#define RISCV_ISA_EXT_ENTRY(ext_name, guest_supp)       \
> +{                                                       \
> +    .id              = RISCV_ISA_EXT_ ## ext_name,      \
> +    .name            = #ext_name,                       \
> +    .guest_supported = guest_supp,                      \
> +}
> +
>  /* Host ISA bitmap */
>  static __ro_after_init DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX);
>  
> @@ -120,29 +134,30 @@ static int __init dt_get_cpuid_from_node(const struct dt_device_node *cpu,
>   * and strncmp() is used in match_isa_ext() to compare extension names instead
>   * of strncasecmp().
>   */
> -const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = {

I realize it has been this way before, but ...

> -    RISCV_ISA_EXT_DATA(i),
> -    RISCV_ISA_EXT_DATA(m),
> -    RISCV_ISA_EXT_DATA(a),
> -    RISCV_ISA_EXT_DATA(f),
> -    RISCV_ISA_EXT_DATA(d),
> -    RISCV_ISA_EXT_DATA(q),
> -    RISCV_ISA_EXT_DATA(c),
> -    RISCV_ISA_EXT_DATA(h),
> -    RISCV_ISA_EXT_DATA(zicntr),
> -    RISCV_ISA_EXT_DATA(zicsr),
> -    RISCV_ISA_EXT_DATA(zifencei),
> -    RISCV_ISA_EXT_DATA(zihintpause),
> -    RISCV_ISA_EXT_DATA(zihpm),
> -    RISCV_ISA_EXT_DATA(zba),
> -    RISCV_ISA_EXT_DATA(zbb),
> -    RISCV_ISA_EXT_DATA(zbs),
> -    RISCV_ISA_EXT_DATA(smaia),
> -    RISCV_ISA_EXT_DATA(smstateen),
> -    RISCV_ISA_EXT_DATA(ssaia),
> -    RISCV_ISA_EXT_DATA(sstc),
> -    RISCV_ISA_EXT_DATA(svade),
> -    RISCV_ISA_EXT_DATA(svpbmt),
> +const struct riscv_isa_ext_entry riscv_isa_ext[] = {

... is there a reason for this to be non-static? Its type (struct
riscv_isa_ext_entry) is local to this file, and I also can't spot any
declaration elsewhere.

> @@ -480,6 +495,74 @@ bool riscv_isa_extension_available(const unsigned long *isa_bitmap,
>      return test_bit(id, isa_bitmap);
>  }
>  
> +int build_guest_isa_str(char *buf, size_t size,
> +                        const unsigned long *isa_bitmap)
> +{
> +    char *p = buf;
> +    size_t left = size;
> +    int total;
> +
> +#if defined(CONFIG_RISCV_32)
> +    total = snprintf(p, left, "rv32");
> +#elif defined(CONFIG_RISCV_64)
> +    total = snprintf(p, left, "rv64");
> +#else
> +# error "Unsupported RISC-V bitness"
> +#endif

For the longer-term future of this, passing in const struct domain * may
help.

> +    if ( total < 0 )
> +        return total;
> +
> +    if ( buf )
> +    {
> +        if ( (size_t)total >= left )
> +            return -ENOSPC;
> +
> +        p += total;
> +        left -= total;
> +    }
> +
> +    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
> +    {
> +        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
> +        int ret;
> +
> +        if ( !riscv_isa_extension_available(isa_bitmap, ext->id) )
> +            continue;
> +
> +        ret = snprintf(p, left, "%s%s",
> +                       ext->id >= RISCV_ISA_EXT_BASE ? "_" : "",
> +                       ext->name);
> +        if ( ret < 0 )
> +            return ret;
> +
> +        total += ret;
> +
> +        if ( buf )
> +        {
> +            if ( (size_t)ret >= left )
> +                return -ENOSPC;
> +
> +            p += ret;
> +            left -= ret;
> +        }
> +    }
> +
> +    return total;
> +}
> +
> +void init_guest_isa(struct domain *d)
> +{
> +    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
> +    {
> +        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
> +
> +        if ( ext->guest_supported &&
> +             riscv_isa_extension_available(NULL, ext->id) )
> +            __set_bit(ext->id, d->arch.isa);
> +    }
> +}

Right now what this function does is dependent on only global variables.
IOW each guest gets the same bitmap. Is this going to change soon? Else
why not calculate that bitmap once, taking the same shortcut as you take
elsewhere for the time being? Then allowing riscv_isa_ext[] to remain
__initconst (should really have been __initconstrel).

> @@ -527,4 +610,5 @@ void __init riscv_fill_hwcap(void)
>      if ( !all_extns_available )
>          panic("Look why the extensions above are needed in "
>                "https://xenbits.xenproject.org/docs/unstable/misc/riscv/booting.txt\n");
> +
>  }

Stray change once again?

Jan
Re: [PATCH v5 09/26] xen/riscv: introduce guest riscv,isa string
Posted by Oleksii Kurochko 4 weeks, 1 day ago

On 7/9/26 3:05 PM, Jan Beulich wrote:
> On 06.07.2026 17:57, Oleksii Kurochko wrote:
>> --- a/xen/arch/riscv/cpufeature.c
>> +++ b/xen/arch/riscv/cpufeature.c
>> @@ -14,6 +14,7 @@
>>   #include <xen/errno.h>
>>   #include <xen/init.h>
>>   #include <xen/lib.h>
>> +#include <xen/sched.h>
>>   #include <xen/sections.h>
>>   
>>   #include <asm/cpufeature.h>
>> @@ -34,6 +35,19 @@ struct riscv_isa_ext_data {
>>       .name = #ext_name,                          \
>>   }
>>   
>> +struct riscv_isa_ext_entry {
>> +    unsigned int id;
>> +    const char *name;
>> +    bool guest_supported;
>> +};
>> +
>> +#define RISCV_ISA_EXT_ENTRY(ext_name, guest_supp)       \
>> +{                                                       \
>> +    .id              = RISCV_ISA_EXT_ ## ext_name,      \
>> +    .name            = #ext_name,                       \
>> +    .guest_supported = guest_supp,                      \
>> +}
>> +
>>   /* Host ISA bitmap */
>>   static __ro_after_init DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX);
>>   
>> @@ -120,29 +134,30 @@ static int __init dt_get_cpuid_from_node(const struct dt_device_node *cpu,
>>    * and strncmp() is used in match_isa_ext() to compare extension names instead
>>    * of strncasecmp().
>>    */
>> -const struct riscv_isa_ext_data __initconst riscv_isa_ext[] = {
> 
> I realize it has been this way before, but ...
> 
>> -    RISCV_ISA_EXT_DATA(i),
>> -    RISCV_ISA_EXT_DATA(m),
>> -    RISCV_ISA_EXT_DATA(a),
>> -    RISCV_ISA_EXT_DATA(f),
>> -    RISCV_ISA_EXT_DATA(d),
>> -    RISCV_ISA_EXT_DATA(q),
>> -    RISCV_ISA_EXT_DATA(c),
>> -    RISCV_ISA_EXT_DATA(h),
>> -    RISCV_ISA_EXT_DATA(zicntr),
>> -    RISCV_ISA_EXT_DATA(zicsr),
>> -    RISCV_ISA_EXT_DATA(zifencei),
>> -    RISCV_ISA_EXT_DATA(zihintpause),
>> -    RISCV_ISA_EXT_DATA(zihpm),
>> -    RISCV_ISA_EXT_DATA(zba),
>> -    RISCV_ISA_EXT_DATA(zbb),
>> -    RISCV_ISA_EXT_DATA(zbs),
>> -    RISCV_ISA_EXT_DATA(smaia),
>> -    RISCV_ISA_EXT_DATA(smstateen),
>> -    RISCV_ISA_EXT_DATA(ssaia),
>> -    RISCV_ISA_EXT_DATA(sstc),
>> -    RISCV_ISA_EXT_DATA(svade),
>> -    RISCV_ISA_EXT_DATA(svpbmt),
>> +const struct riscv_isa_ext_entry riscv_isa_ext[] = {
> 
> ... is there a reason for this to be non-static? Its type (struct
> riscv_isa_ext_entry) is local to this file, and I also can't spot any
> declaration elsewhere.

I checked downstream changes and it also local to cpufeature.c file so 
it should be static.

> 
>> @@ -480,6 +495,74 @@ bool riscv_isa_extension_available(const unsigned long *isa_bitmap,
>>       return test_bit(id, isa_bitmap);
>>   }
>>   
>> +int build_guest_isa_str(char *buf, size_t size,
>> +                        const unsigned long *isa_bitmap)
>> +{
>> +    char *p = buf;
>> +    size_t left = size;
>> +    int total;
>> +
>> +#if defined(CONFIG_RISCV_32)
>> +    total = snprintf(p, left, "rv32");
>> +#elif defined(CONFIG_RISCV_64)
>> +    total = snprintf(p, left, "rv64");
>> +#else
>> +# error "Unsupported RISC-V bitness"
>> +#endif
> 
> For the longer-term future of this, passing in const struct domain * may
> help.

pointer to const struct domain looks really better I chnage prototype of 
build_guest_isa_str().

> 
>> +    if ( total < 0 )
>> +        return total;
>> +
>> +    if ( buf )
>> +    {
>> +        if ( (size_t)total >= left )
>> +            return -ENOSPC;
>> +
>> +        p += total;
>> +        left -= total;
>> +    }
>> +
>> +    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
>> +    {
>> +        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
>> +        int ret;
>> +
>> +        if ( !riscv_isa_extension_available(isa_bitmap, ext->id) )
>> +            continue;
>> +
>> +        ret = snprintf(p, left, "%s%s",
>> +                       ext->id >= RISCV_ISA_EXT_BASE ? "_" : "",
>> +                       ext->name);
>> +        if ( ret < 0 )
>> +            return ret;
>> +
>> +        total += ret;
>> +
>> +        if ( buf )
>> +        {
>> +            if ( (size_t)ret >= left )
>> +                return -ENOSPC;
>> +
>> +            p += ret;
>> +            left -= ret;
>> +        }
>> +    }
>> +
>> +    return total;
>> +}
>> +
>> +void init_guest_isa(struct domain *d)
>> +{
>> +    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
>> +    {
>> +        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
>> +
>> +        if ( ext->guest_supported &&
>> +             riscv_isa_extension_available(NULL, ext->id) )
>> +            __set_bit(ext->id, d->arch.isa);
>> +    }
>> +}
> 
> Right now what this function does is dependent on only global variables.
> IOW each guest gets the same bitmap. Is this going to change soon?

At the moment, we don't have a use case where domain has unique bitmap. 
But IIRC correctly we agreed that generally it is good idea to have isa 
bitmap per domain and let the use to chose.

  Else
> why not calculate that bitmap once, taking the same shortcut as you take
> elsewhere for the time being?

static __ro_after_init DECLARE_BITMAP(guest_isa, RISCV_ISA_EXT_MAX);

/* called once from riscv_fill_hwcap() or similar */
void __init init_guest_isa_mask(void)
{
     for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
         if ( riscv_isa_ext[i].guest_supported &&
              riscv_isa_extension_available(NULL, riscv_isa_ext[i].id) )
             __set_bit(riscv_isa_ext[i].id, guest_isa);
}

void init_guest_isa(struct domain *d)   /* called per-domain */
{
     bitmap_copy(d->arch.isa, guest_isa, RISCV_ISA_EXT_MAX);
}

Do you mean something like that?

  Then allowing riscv_isa_ext[] to remain
> __initconst (should really have been __initconstrel).

I think you explained me already why but I forgot. Could you please 
remind me again why __initconstrel should be here? Is it because of 
pointers used inside struct riscv_isa_ext_entry?

Thanks.

~ Oleksii
Re: [PATCH v5 09/26] xen/riscv: introduce guest riscv,isa string
Posted by Jan Beulich 3 weeks, 6 days ago
On 10.07.2026 17:00, Oleksii Kurochko wrote:
> On 7/9/26 3:05 PM, Jan Beulich wrote:
>> On 06.07.2026 17:57, Oleksii Kurochko wrote:
>>> +void init_guest_isa(struct domain *d)
>>> +{
>>> +    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
>>> +    {
>>> +        const struct riscv_isa_ext_entry *ext = &riscv_isa_ext[i];
>>> +
>>> +        if ( ext->guest_supported &&
>>> +             riscv_isa_extension_available(NULL, ext->id) )
>>> +            __set_bit(ext->id, d->arch.isa);
>>> +    }
>>> +}
>>
>> Right now what this function does is dependent on only global variables.
>> IOW each guest gets the same bitmap. Is this going to change soon?
> 
> At the moment, we don't have a use case where domain has unique bitmap. 
> But IIRC correctly we agreed that generally it is good idea to have isa 
> bitmap per domain and let the use to chose.

Indeed. But what you're doing (in this series alone) is a mix of things:
Some properties are per-domain, some are global. Such a mix might be okay
if it's justified by something. Otherwise I think consistency it to be
valued higher.

>   Else
>> why not calculate that bitmap once, taking the same shortcut as you take
>> elsewhere for the time being?
> 
> static __ro_after_init DECLARE_BITMAP(guest_isa, RISCV_ISA_EXT_MAX);
> 
> /* called once from riscv_fill_hwcap() or similar */
> void __init init_guest_isa_mask(void)
> {
>      for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
>          if ( riscv_isa_ext[i].guest_supported &&
>               riscv_isa_extension_available(NULL, riscv_isa_ext[i].id) )
>              __set_bit(riscv_isa_ext[i].id, guest_isa);
> }
> 
> void init_guest_isa(struct domain *d)   /* called per-domain */
> {
>      bitmap_copy(d->arch.isa, guest_isa, RISCV_ISA_EXT_MAX);
> }
> 
> Do you mean something like that?

Yes. Not necessarily with bitmap_copy() though; perhaps with a pointer in
struct domain.

>   Then allowing riscv_isa_ext[] to remain
>> __initconst (should really have been __initconstrel).
> 
> I think you explained me already why but I forgot. Could you please 
> remind me again why __initconstrel should be here? Is it because of 
> pointers used inside struct riscv_isa_ext_entry?

Yes, pointers incur relocations, and the need for relocations requires
that the compiler emit the data to a writable section. As opposed to
relocation-free data, which (when declared const) can go into a r/o
section.

Jan