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