On Tue, Apr 29, 2025 at 05:18:47PM -0700, Atish Patra wrote:
> Add vector related tests with the ISA extension standard template.
> However, the vector registers are bit tricky as the register length is
> variable based on vlenb value of the system. That's why the macros are
> defined with a default and overidden with actual value at runtime.
> 
> Reviewed-by: Anup Patel <a...@brainfault.org>
> Signed-off-by: Atish Patra <ati...@rivosinc.com>
> ---
>  tools/testing/selftests/kvm/riscv/get-reg-list.c | 133 
> +++++++++++++++++++++++
>  1 file changed, 133 insertions(+)
> 
> diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c 
> b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> index 569f2d67c9b8..814dd981ce0b 100644
> --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> @@ -17,6 +17,15 @@ enum {
>       VCPU_FEATURE_SBI_EXT,
>  };
>  
> +enum {
> +     KVM_RISC_V_REG_OFFSET_VSTART = 0,
> +     KVM_RISC_V_REG_OFFSET_VL,
> +     KVM_RISC_V_REG_OFFSET_VTYPE,
> +     KVM_RISC_V_REG_OFFSET_VCSR,
> +     KVM_RISC_V_REG_OFFSET_VLENB,
> +     KVM_RISC_V_REG_OFFSET_MAX,
> +};
> +
>  static bool isa_ext_cant_disable[KVM_RISCV_ISA_EXT_MAX];
>  
>  bool filter_reg(__u64 reg)
> @@ -143,6 +152,39 @@ bool check_reject_set(int err)
>       return err == EINVAL;
>  }
>  
> +static int override_vector_reg_size(struct kvm_vcpu *vcpu, struct 
> vcpu_reg_sublist *s,
> +                                 uint64_t feature)
> +{
> +     unsigned long vlenb_reg = 0;
> +     int rc;
> +     u64 reg, size;
> +
> +     /* Enable V extension so that we can get the vlenb register */
> +     rc = __vcpu_set_reg(vcpu, feature, 1);
> +     if (rc)
> +             return rc;
> +
> +     __vcpu_get_reg(vcpu, s->regs[KVM_RISC_V_REG_OFFSET_VLENB], &vlenb_reg);

We can remove the underscores from this call since it shouldn't fail, as
we know we've successfully enabled the V extension at this point.

> +
> +     if (!vlenb_reg) {
> +             TEST_FAIL("Can't compute vector register size from zero 
> vlenb\n");
> +             return -EPERM;
> +     }
> +
> +     size = __builtin_ctzl(vlenb_reg);
> +     size <<= KVM_REG_SIZE_SHIFT;
> +
> +     for (int i = 0; i < 32; i++) {
> +             reg = KVM_REG_RISCV | KVM_REG_RISCV_VECTOR | size | 
> KVM_REG_RISCV_VECTOR_REG(i);
> +             s->regs[KVM_RISC_V_REG_OFFSET_MAX + i] = reg;
> +     }
> +
> +     /* We should assert if disabling failed here while enabling succeeded 
> before */
> +     vcpu_set_reg(vcpu, feature, 0);
> +
> +     return 0;
> +}
> +
>  void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
>  {
>       unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 };
> @@ -172,6 +214,13 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct 
> vcpu_reg_list *c)
>               if (!s->feature)
>                       continue;
>  
> +             if (s->feature == KVM_RISCV_ISA_EXT_V) {
> +                     feature = RISCV_ISA_EXT_REG(s->feature);
> +                     rc = override_vector_reg_size(vcpu, s, feature);
> +                     if (rc)
> +                             goto skip;
> +             }
> +
>               switch (s->feature_type) {
>               case VCPU_FEATURE_ISA_EXT:
>                       feature = RISCV_ISA_EXT_REG(s->feature);
> @@ -186,6 +235,7 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct 
> vcpu_reg_list *c)
>               /* Try to enable the desired extension */
>               __vcpu_set_reg(vcpu, feature, 1);
>  
> +skip:
>               /* Double check whether the desired extension was enabled */
>               __TEST_REQUIRE(__vcpu_has_ext(vcpu, feature),
>                              "%s not available, skipping tests", s->name);
> @@ -410,6 +460,35 @@ static const char *fp_d_id_to_str(const char *prefix, 
> __u64 id)
>       return strdup_printf("%lld /* UNKNOWN */", reg_off);
>  }
>  
> +static const char *vector_id_to_str(const char *prefix, __u64 id)
> +{
> +     /* reg_off is the offset into struct __riscv_v_ext_state */
> +     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_VECTOR);
> +     int reg_index = 0;
> +
> +     assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_VECTOR);
> +
> +     if (reg_off >= KVM_REG_RISCV_VECTOR_REG(0))
> +             reg_index = reg_off -  KVM_REG_RISCV_VECTOR_REG(0);
> +     switch (reg_off) {
> +     case KVM_REG_RISCV_VECTOR_REG(0) ...
> +          KVM_REG_RISCV_VECTOR_REG(31):
> +             return strdup_printf("KVM_REG_RISCV_VECTOR_REG(%d)", reg_index);
> +     case KVM_REG_RISCV_VECTOR_CSR_REG(vstart):
> +             return "KVM_REG_RISCV_VECTOR_CSR_REG(vstart)";
> +     case KVM_REG_RISCV_VECTOR_CSR_REG(vl):
> +             return "KVM_REG_RISCV_VECTOR_CSR_REG(vl)";
> +     case KVM_REG_RISCV_VECTOR_CSR_REG(vtype):
> +             return "KVM_REG_RISCV_VECTOR_CSR_REG(vtype)";
> +     case KVM_REG_RISCV_VECTOR_CSR_REG(vcsr):
> +             return "KVM_REG_RISCV_VECTOR_CSR_REG(vcsr)";
> +     case KVM_REG_RISCV_VECTOR_CSR_REG(vlenb):
> +             return "KVM_REG_RISCV_VECTOR_CSR_REG(vlenb)";
> +     }
> +
> +     return strdup_printf("%lld /* UNKNOWN */", reg_off);
> +}
> +
>  #define KVM_ISA_EXT_ARR(ext)         \
>  [KVM_RISCV_ISA_EXT_##ext] = "KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_" 
> #ext
>  
> @@ -639,6 +718,9 @@ void print_reg(const char *prefix, __u64 id)
>       case KVM_REG_SIZE_U128:
>               reg_size = "KVM_REG_SIZE_U128";
>               break;
> +     case KVM_REG_SIZE_U256:
> +             reg_size = "KVM_REG_SIZE_U256";
> +             break;
>       default:
>               printf("\tKVM_REG_RISCV | (%lld << KVM_REG_SIZE_SHIFT) | 0x%llx 
> /* UNKNOWN */,\n",
>                      (id & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT, id & 
> ~REG_MASK);
> @@ -670,6 +752,10 @@ void print_reg(const char *prefix, __u64 id)
>               printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_D | %s,\n",
>                               reg_size, fp_d_id_to_str(prefix, id));
>               break;
> +     case KVM_REG_RISCV_VECTOR:
> +             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_VECTOR | %s,\n",
> +                    reg_size, vector_id_to_str(prefix, id));
> +             break;
>       case KVM_REG_RISCV_ISA_EXT:
>               printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_ISA_EXT | %s,\n",
>                               reg_size, isa_ext_id_to_str(prefix, id));
> @@ -874,6 +960,48 @@ static __u64 fp_d_regs[] = {
>       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | 
> KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D,
>  };
>  
> +/* Define a default vector registers with length. This will be overwritten 
> at runtime */
> +static __u64 vector_regs[] = {
> +     KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_CSR_REG(vstart),
> +     KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_CSR_REG(vl),
> +     KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_CSR_REG(vtype),
> +     KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_CSR_REG(vcsr),
> +     KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_CSR_REG(vlenb),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(0),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(1),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(2),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(3),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(4),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(5),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(6),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(7),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(8),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(9),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(10),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(11),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(12),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(13),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(14),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(15),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(16),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(17),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(18),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(19),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(20),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(21),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(22),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(23),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(24),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(25),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(26),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(27),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(28),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(29),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(30),
> +     KVM_REG_RISCV | KVM_REG_SIZE_U128 | KVM_REG_RISCV_VECTOR | 
> KVM_REG_RISCV_VECTOR_REG(31),
> +     KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | 
> KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_V,
> +};
> +
>  #define SUBLIST_BASE \
>       {"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), \
>        .skips_set = base_skips_set, .skips_set_n = 
> ARRAY_SIZE(base_skips_set),}
> @@ -898,6 +1026,9 @@ static __u64 fp_d_regs[] = {
>       {"fp_d", .feature = KVM_RISCV_ISA_EXT_D, .regs = fp_d_regs, \
>               .regs_n = ARRAY_SIZE(fp_d_regs),}
>  
> +#define SUBLIST_V \
> +     {"v", .feature = KVM_RISCV_ISA_EXT_V, .regs = vector_regs, .regs_n = 
> ARRAY_SIZE(vector_regs),}
> +
>  #define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu)                 \
>  static __u64 regs_##ext[] = {                                        \
>       KVM_REG_RISCV | KVM_REG_SIZE_ULONG |                    \
> @@ -966,6 +1097,7 @@ KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP);
>  KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA);
>  KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F);
>  KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D);
> +KVM_ISA_EXT_SUBLIST_CONFIG(v, V);
>  KVM_ISA_EXT_SIMPLE_CONFIG(h, H);
>  KVM_ISA_EXT_SIMPLE_CONFIG(smnpm, SMNPM);
>  KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN);
> @@ -1040,6 +1172,7 @@ struct vcpu_reg_list *vcpu_configs[] = {
>       &config_fp_f,
>       &config_fp_d,
>       &config_h,
> +     &config_v,
>       &config_smnpm,
>       &config_smstateen,
>       &config_sscofpmf,
> 
> -- 
> 2.43.0
>

Otherwise,

Reviewed-by: Andrew Jones <ajo...@ventanamicro.com>

Reply via email to