| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f1a917d9-b225-4453-899b-4c8aa531d3a1@linux.dev>
Date: Wed, 30 Apr 2025 00:22:11 -0700
From: Atish Patra <atish.patra@...ux.dev>
To: Andrew Jones <ajones@...tanamicro.com>
Cc: Anup Patel <anup@...infault.org>, Atish Patra <atishp@...shpatra.org>,
Paolo Bonzini <pbonzini@...hat.com>, Shuah Khan <shuah@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt
<palmer@...belt.com>, Alexandre Ghiti <alex@...ti.fr>, kvm@...r.kernel.org,
kvm-riscv@...ts.infradead.org, linux-riscv@...ts.infradead.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] KVM: riscv: selftests: Add vector extension tests
On 4/30/25 12:17 AM, Andrew Jones wrote:
> 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 <anup@...infault.org>
>> Signed-off-by: Atish Patra <atishp@...osinc.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.
good point. I will remove it.
>> +
>> + 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 <ajones@...tanamicro.com>
Powered by blists - more mailing lists