Fix a bug where FWFT features could be incorrectly exposed to guests after userspace disables their dependent ISA extensions at runtime. The 'supported' field in kvm_sbi_fwft_config was set once during vCPU initialization based on the initial hardware/extension availability. However, when userspace subsequently disables ISA extensions via the KVM ONE_REG interface, the 'supported' field was not updated. This caused the following issues: 1. FWFT features would remain visible and accessible to guests even after their prerequisite ISA extensions were disabled 2. Guests could configure FWFT features that depend on disabled extensions, leading to undefined behavior 3. The static 'supported' flag and the dynamic supported() callback could disagree about feature availability Remove the redundant static 'supported' field from kvm_sbi_fwft_config and replace all conf->supported checks with feature->supported(vcpu) calls that check the current vCPU ISA extension state. This can ensure the feature availability is always determined at runtime based on the current configuration, not initialization-time snapshots. Fixes: 6b72fd170592 ("RISC-V: KVM: add support for FWFT SBI extension") Signed-off-by: Yong-Xuan Wang --- arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h | 1 - arch/riscv/kvm/vcpu_sbi_fwft.c | 17 ++++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h index 5604cec79902..837431867c6f 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi_fwft.h @@ -15,7 +15,6 @@ struct kvm_sbi_fwft_feature; struct kvm_sbi_fwft_config { const struct kvm_sbi_fwft_feature *feature; - bool supported; bool enabled; unsigned long flags; }; diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c index 2eab15339694..d3be059c3822 100644 --- a/arch/riscv/kvm/vcpu_sbi_fwft.c +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c @@ -279,8 +279,8 @@ static int kvm_fwft_get_feature(struct kvm_vcpu *vcpu, u32 feature, return SBI_ERR_DENIED; } - if (!tconf->supported || !tconf->enabled) - return SBI_ERR_NOT_SUPPORTED; + if (!tconf->feature->supported(vcpu)) + return SBI_ERR_NOT_SUPPORTED; *conf = tconf; @@ -361,11 +361,10 @@ static int kvm_sbi_ext_fwft_init(struct kvm_vcpu *vcpu) feature = &features[i]; conf = &fwft->configs[i]; if (feature->supported) - conf->supported = feature->supported(vcpu); + conf->enabled = feature->supported(vcpu); else - conf->supported = true; + conf->enabled = true; - conf->enabled = conf->supported; conf->feature = feature; } @@ -406,7 +405,7 @@ static unsigned long kvm_sbi_ext_fwft_get_reg_count(struct kvm_vcpu *vcpu) continue; conf = kvm_sbi_fwft_get_config(vcpu, feature->id); - if (!conf || !conf->supported) + if (!conf || !feature->supported(vcpu)) continue; ret++; @@ -428,7 +427,7 @@ static int kvm_sbi_ext_fwft_get_reg_id(struct kvm_vcpu *vcpu, int index, u64 *re continue; conf = kvm_sbi_fwft_get_config(vcpu, feature->id); - if (!conf || !conf->supported) + if (!conf || !feature->supported(vcpu)) continue; if (index == idx) { @@ -463,7 +462,7 @@ static int kvm_sbi_ext_fwft_get_reg(struct kvm_vcpu *vcpu, unsigned long reg_num return -ENOENT; conf = kvm_sbi_fwft_get_config(vcpu, feature->id); - if (!conf || !conf->supported) + if (!conf || !feature->supported(vcpu)) return -ENOENT; switch (reg_num - feature->first_reg_num) { @@ -500,7 +499,7 @@ static int kvm_sbi_ext_fwft_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num return -ENOENT; conf = kvm_sbi_fwft_get_config(vcpu, feature->id); - if (!conf || !conf->supported) + if (!conf || !feature->supported(vcpu)) return -ENOENT; switch (reg_num - feature->first_reg_num) { -- 2.43.7 Refactor the get-reg-list test to use unified sublist macros for ISA and SBI extensions, eliminating code duplication and improving maintainability. Previously, each extension had its own hand-coded sublist definition (e.g., SUBLIST_ZICBOM, SUBLIST_AIA, etc.) and the config structures repeated the same pattern. This made the code verbose and error-prone. Signed-off-by: Yong-Xuan Wang --- tools/testing/selftests/kvm/riscv/get-reg-list.c | 80 +++++++++--------------- 1 file changed, 29 insertions(+), 51 deletions(-) diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index 8d6fdb5d38b8..cb16c638ce1a 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -1057,37 +1057,17 @@ static __u64 vector_regs[] = { #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),} -#define SUBLIST_SBI_BASE \ - {"sbi-base", .feature_type = VCPU_FEATURE_SBI_EXT, .feature = KVM_RISCV_SBI_EXT_V01, \ - .regs = sbi_base_regs, .regs_n = ARRAY_SIZE(sbi_base_regs),} -#define SUBLIST_SBI_STA \ - {"sbi-sta", .feature_type = VCPU_FEATURE_SBI_EXT, .feature = KVM_RISCV_SBI_EXT_STA, \ - .regs = sbi_sta_regs, .regs_n = ARRAY_SIZE(sbi_sta_regs),} -#define SUBLIST_SBI_FWFT \ - {"sbi-fwft", .feature_type = VCPU_FEATURE_SBI_EXT, .feature = KVM_RISCV_SBI_EXT_FWFT, \ - .regs = sbi_fwft_regs, .regs_n = ARRAY_SIZE(sbi_fwft_regs),} -#define SUBLIST_ZICBOM \ - {"zicbom", .feature = KVM_RISCV_ISA_EXT_ZICBOM, .regs = zicbom_regs, .regs_n = ARRAY_SIZE(zicbom_regs),} -#define SUBLIST_ZICBOP \ - {"zicbop", .feature = KVM_RISCV_ISA_EXT_ZICBOP, .regs = zicbop_regs, .regs_n = ARRAY_SIZE(zicbop_regs),} -#define SUBLIST_ZICBOZ \ - {"zicboz", .feature = KVM_RISCV_ISA_EXT_ZICBOZ, .regs = zicboz_regs, .regs_n = ARRAY_SIZE(zicboz_regs),} -#define SUBLIST_AIA \ - {"aia", .feature = KVM_RISCV_ISA_EXT_SSAIA, .regs = aia_regs, .regs_n = ARRAY_SIZE(aia_regs),} -#define SUBLIST_SMSTATEEN \ - {"smstateen", .feature = KVM_RISCV_ISA_EXT_SMSTATEEN, .regs = smstateen_regs, .regs_n = ARRAY_SIZE(smstateen_regs),} -#define SUBLIST_FP_F \ - {"fp_f", .feature = KVM_RISCV_ISA_EXT_F, .regs = fp_f_regs, \ - .regs_n = ARRAY_SIZE(fp_f_regs),} -#define SUBLIST_FP_D \ - {"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 SUBLIST_ISA(ext, extu) \ + { \ + .name = #ext, \ + .feature = KVM_RISCV_ISA_EXT_##extu, \ + .regs = ext##_regs, \ + .regs_n = ARRAY_SIZE(ext##_regs), \ + } #define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu) \ -static __u64 regs_##ext[] = { \ +static __u64 ext##_regs[] = { \ KVM_REG_RISCV | KVM_REG_SIZE_ULONG | \ KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | \ KVM_RISCV_ISA_EXT_##extu, \ @@ -1095,18 +1075,22 @@ static __u64 regs_##ext[] = { \ static struct vcpu_reg_list config_##ext = { \ .sublists = { \ SUBLIST_BASE, \ - { \ - .name = #ext, \ - .feature = KVM_RISCV_ISA_EXT_##extu, \ - .regs = regs_##ext, \ - .regs_n = ARRAY_SIZE(regs_##ext), \ - }, \ + SUBLIST_ISA(ext, extu), \ {0}, \ }, \ } \ +#define SUBLIST_SBI(ext, extu) \ + { \ + .name = "sbi-"#ext, \ + .feature_type = VCPU_FEATURE_SBI_EXT, \ + .feature = KVM_RISCV_SBI_EXT_##extu, \ + .regs = sbi_##ext##_regs, \ + .regs_n = ARRAY_SIZE(sbi_##ext##_regs), \ + } + #define KVM_SBI_EXT_SIMPLE_CONFIG(ext, extu) \ -static __u64 regs_sbi_##ext[] = { \ +static __u64 sbi_##ext##_regs[] = { \ KVM_REG_RISCV | KVM_REG_SIZE_ULONG | \ KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | \ KVM_RISCV_SBI_EXT_##extu, \ @@ -1114,13 +1098,7 @@ static __u64 regs_sbi_##ext[] = { \ static struct vcpu_reg_list config_sbi_##ext = { \ .sublists = { \ SUBLIST_BASE, \ - { \ - .name = "sbi-"#ext, \ - .feature_type = VCPU_FEATURE_SBI_EXT, \ - .feature = KVM_RISCV_SBI_EXT_##extu, \ - .regs = regs_sbi_##ext, \ - .regs_n = ARRAY_SIZE(regs_sbi_##ext), \ - }, \ + SUBLIST_SBI(ext, extu), \ {0}, \ }, \ } \ @@ -1129,7 +1107,7 @@ static struct vcpu_reg_list config_sbi_##ext = { \ static struct vcpu_reg_list config_##ext = { \ .sublists = { \ SUBLIST_BASE, \ - SUBLIST_##extu, \ + SUBLIST_ISA(ext, extu), \ {0}, \ }, \ } \ @@ -1138,14 +1116,14 @@ static struct vcpu_reg_list config_##ext = { \ static struct vcpu_reg_list config_sbi_##ext = { \ .sublists = { \ SUBLIST_BASE, \ - SUBLIST_SBI_##extu, \ + SUBLIST_SBI(ext, extu), \ {0}, \ }, \ } \ /* Note: The below list is alphabetically sorted. */ -KVM_SBI_EXT_SUBLIST_CONFIG(base, BASE); +KVM_SBI_EXT_SUBLIST_CONFIG(base, V01); KVM_SBI_EXT_SUBLIST_CONFIG(sta, STA); KVM_SBI_EXT_SIMPLE_CONFIG(pmu, PMU); KVM_SBI_EXT_SIMPLE_CONFIG(dbcn, DBCN); @@ -1153,10 +1131,10 @@ KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP); KVM_SBI_EXT_SIMPLE_CONFIG(mpxy, MPXY); KVM_SBI_EXT_SUBLIST_CONFIG(fwft, FWFT); -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_SUBLIST_CONFIG(aia, SSAIA); +KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, F); +KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, D); +KVM_ISA_EXT_SUBLIST_CONFIG(vector, V); KVM_ISA_EXT_SIMPLE_CONFIG(h, H); KVM_ISA_EXT_SIMPLE_CONFIG(smnpm, SMNPM); KVM_ISA_EXT_SUBLIST_CONFIG(smstateen, SMSTATEEN); @@ -1240,7 +1218,7 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_fp_f, &config_fp_d, &config_h, - &config_v, + &config_vector, &config_smnpm, &config_smstateen, &config_sscofpmf, -- 2.43.7 Divide the monolithic SBI FWFT (Firmware Features) register list into separate sublists, each testing a specific FWFT feature independently with proper dependency checking. Previously, all FWFT features were tested together in a single sublist. This caused issues because: 1. Not all FWFT features are available on all platforms 2. Some features depend on specific ISA extensions (e.g., pointer_masking requires Smnpm) 3. Tests would fail if any single feature was unavailable Add the feature-specific SBI FWFT sublists with the following improvements: - Add check_supported_reg() function to validate register availability based on required ISA extensions - Add check_fwft_feature() helper to verify FWFT feature availability at runtime - Update filter_reg() to handle per-feature FWFT register filtering Signed-off-by: Yong-Xuan Wang --- tools/testing/selftests/kvm/riscv/get-reg-list.c | 92 +++++++++++++++++++++--- 1 file changed, 82 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index cb16c638ce1a..6a34320be78c 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -26,7 +26,21 @@ enum { KVM_RISC_V_REG_OFFSET_MAX, }; -static bool isa_ext_cant_disable[KVM_RISCV_ISA_EXT_MAX]; +static bool isa_ext_enabled[KVM_RISCV_ISA_EXT_MAX]; + +bool check_supported_reg(struct kvm_vcpu *vcpu, __u64 reg) +{ + switch (reg & ~REG_MASK) { + case KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.enable): + case KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.flags): + case KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.value): + return isa_ext_enabled[KVM_RISCV_ISA_EXT_SMNPM]; + default: + break; + } + + return true; +} bool filter_reg(__u64 reg) { @@ -148,7 +162,12 @@ bool filter_reg(__u64 reg) case KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(siph): case KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(iprio1h): case KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(iprio2h): - return isa_ext_cant_disable[KVM_RISCV_ISA_EXT_SSAIA]; + return isa_ext_enabled[KVM_RISCV_ISA_EXT_SSAIA]; + /* KVM misaligned delegation registers are always visible when the host supports it */ + case KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.enable): + case KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.flags): + case KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.value): + return true; default: break; } @@ -193,15 +212,39 @@ static int override_vector_reg_size(struct kvm_vcpu *vcpu, struct vcpu_reg_subli return 0; } +void check_fwft_feature(struct kvm_vcpu *vcpu, struct vcpu_reg_sublist *s, u64 feature) +{ + unsigned long value; + int rc; + + /* Enable SBI FWFT extension so that we can check the supported register */ + rc = __vcpu_set_reg(vcpu, feature, 1); + if (rc) + return; + + for (int i = 0; i < s->regs_n; i++) { + if ((s->regs[i] & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_SBI_STATE) { + rc = __vcpu_get_reg(vcpu, s->regs[i], &value); + __TEST_REQUIRE(!rc, "%s not available, skipping tests", s->name); + } + } + + /* We should assert if disabling failed here while enabling succeeded before */ + vcpu_set_reg(vcpu, feature, 0); +} + void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) { - unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 }; + unsigned long isa_ext_state; struct vcpu_reg_sublist *s; u64 feature; int rc; - for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++) - __vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(i), &isa_ext_state[i]); + for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++) { + rc = __vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(i), &isa_ext_state); + if (!rc) + isa_ext_enabled[i] = !!isa_ext_state; + } /* * Disable all extensions which were enabled by default @@ -209,8 +252,10 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) */ for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++) { rc = __vcpu_set_reg(vcpu, RISCV_ISA_EXT_REG(i), 0); - if (rc && isa_ext_state[i]) - isa_ext_cant_disable[i] = true; + if (rc && isa_ext_enabled[i]) + isa_ext_enabled[i] = true; + else + isa_ext_enabled[i] = false; } for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) { @@ -229,9 +274,15 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) goto skip; } + if (s->feature == KVM_RISCV_SBI_EXT_FWFT) { + feature = RISCV_SBI_EXT_REG(KVM_RISCV_SBI_EXT_FWFT); + check_fwft_feature(vcpu, s, feature); + } + switch (s->feature_type) { case VCPU_FEATURE_ISA_EXT: feature = RISCV_ISA_EXT_REG(s->feature); + isa_ext_enabled[s->feature] = true; break; case VCPU_FEATURE_SBI_EXT: feature = RISCV_SBI_EXT_REG(s->feature); @@ -897,11 +948,15 @@ static __u64 sbi_sta_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_STA | KVM_REG_RISCV_SBI_STA_REG(shmem_hi), }; -static __u64 sbi_fwft_regs[] = { +static __u64 sbi_fwft_misaligned_deleg_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_FWFT, KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.enable), KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.flags), KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.value), +}; + +static __u64 sbi_fwft_pointer_masking_regs[] = { + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_FWFT, KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.enable), KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.flags), KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.value), @@ -1129,7 +1184,6 @@ KVM_SBI_EXT_SIMPLE_CONFIG(pmu, PMU); KVM_SBI_EXT_SIMPLE_CONFIG(dbcn, DBCN); KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP); KVM_SBI_EXT_SIMPLE_CONFIG(mpxy, MPXY); -KVM_SBI_EXT_SUBLIST_CONFIG(fwft, FWFT); KVM_ISA_EXT_SUBLIST_CONFIG(aia, SSAIA); KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, F); @@ -1206,6 +1260,23 @@ KVM_ISA_EXT_SIMPLE_CONFIG(zvksed, ZVKSED); KVM_ISA_EXT_SIMPLE_CONFIG(zvksh, ZVKSH); KVM_ISA_EXT_SIMPLE_CONFIG(zvkt, ZVKT); +static struct vcpu_reg_list config_sbi_fwft_misaligned_deleg = { + .sublists = { + SUBLIST_BASE, + SUBLIST_SBI(fwft_misaligned_deleg, FWFT), + {0}, + }, +}; + +static struct vcpu_reg_list config_sbi_fwft_pointer_masking = { + .sublists = { + SUBLIST_BASE, + SUBLIST_ISA(smnpm, SMNPM), + SUBLIST_SBI(fwft_pointer_masking, FWFT), + {0}, + }, +}; + struct vcpu_reg_list *vcpu_configs[] = { &config_sbi_base, &config_sbi_sta, @@ -1213,7 +1284,8 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_sbi_dbcn, &config_sbi_susp, &config_sbi_mpxy, - &config_sbi_fwft, + &config_sbi_fwft_misaligned_deleg, + &config_sbi_fwft_pointer_masking, &config_aia, &config_fp_f, &config_fp_d, -- 2.43.7