Re: [PATCH v2 05/26] xen/riscv: introduce guest riscv,isa string

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.05.2026 16:43, Oleksii Kurochko wrote:
> @@ -480,6 +488,53 @@ bool riscv_isa_extension_available(const unsigned long 
> *isa_bitmap,
>      return test_bit(id, isa_bitmap);
>  }
>  
> +int init_guest_isa(struct domain *d)
> +{
> +    char *buf = d->arch.guest_isa_str;
> +    size_t len = sizeof(d->arch.guest_isa_str);

Seeing these uses: Is the "guest" prefix really of much use here?

> +    bitmap_andnot(d->arch.guest_isa, riscv_isa, guest_unsupp,
> +                  RISCV_ISA_EXT_MAX);

Same question here, clearly.

> +#if defined(CONFIG_RISCV_32)
> +    if ( snprintf(buf, len, "rv32") >= len )
> +        return -ENOBUFS;
> +#elif defined(CONFIG_RISCV_64)
> +    if ( snprintf(buf, len, "rv64") >= len )
> +        return -ENOBUFS;
> +#else
> +#   error "Unsupported RISC-V bitness"
> +#endif
> +
> +    for ( unsigned int i = 0; i < ARRAY_SIZE(riscv_isa_ext); i++ )
> +    {
> +        const struct riscv_isa_ext_data *ext = &riscv_isa_ext[i];
> +
> +        if ( !riscv_isa_extension_available(d->arch.guest_isa, ext->id) )
> +            continue;
> +
> +        if ( ext->id >= RISCV_ISA_EXT_BASE && strlcat(buf, "_", len) >= len )
> +            return -ENOBUFS;
> +
> +        if ( strlcat(buf, ext->name, len) >= len )
> +            return -ENOBUFS;
> +    }
> +
> +    return 0;
> +}
> +
> +static void __init init_guest_unsupp(void)
> +{
> +    set_bit(RISCV_ISA_EXT_f, guest_unsupp);
> +    set_bit(RISCV_ISA_EXT_d, guest_unsupp);
> +    set_bit(RISCV_ISA_EXT_q, guest_unsupp);
> +    set_bit(RISCV_ISA_EXT_v, guest_unsupp);
> +    set_bit(RISCV_ISA_EXT_h, guest_unsupp);
> +    set_bit(RISCV_ISA_EXT_sstc, guest_unsupp);
> +    set_bit(RISCV_ISA_EXT_svade, guest_unsupp);
> +    set_bit(RISCV_ISA_EXT_svpbmt, guest_unsupp);
> +}

These don't need to be atomic, do they? I.e. __set_bit() would suffice.

> --- a/xen/arch/riscv/include/asm/cpufeature.h
> +++ b/xen/arch/riscv/include/asm/cpufeature.h
> @@ -17,6 +17,8 @@
>   */
>  #define RISCV_ISA_EXT_BASE  26
>  
> +#define RISCV_GUEST_ISA_STR_MAX 256

This looks like it won't be good for very long, seeing how long ISA strings can
get. I wonder anyway whether ...

> @@ -94,6 +95,9 @@ struct arch_domain {
>      struct p2m_domain p2m;
>  
>      struct paging_domain paging;
> +
> +    DECLARE_BITMAP(guest_isa, RISCV_ISA_EXT_MAX);
> +    char guest_isa_str[RISCV_GUEST_ISA_STR_MAX];

... a compile-time sized buffer is suitable here. Can't you allocate a buffer
just large enough to hold the string?

Jan