On Wed, Sep 30, 2026 at 10:48:20PM +0100, Mark Brown wrote: > In order to optimise fast paths KVM explicitly rejects configurations with > S1PIE or S1POE but not TCR2, add coverage of this in the set_id_regs test. > We have a list of invalid configurations, for each of them we try to run a > VM and fail the test if it succeeds. We do feature detection by validating > that we can write the fields with failing values. > > Since this misfiring can disrupt some of the other tests due to the kernel > refusing to start guests we run the new tests first, improving diagnostics > in the failing case.
Ah so this reflects the changes done to enforce the arch-valid s1pie/poe -> tcr requirement it seems. > > Signed-off-by: Mark Brown <[email protected]> Everything seems sensible, so: Acked-by: Lorenzo Stoakes (ARM) <[email protected]> > --- > tools/testing/selftests/kvm/arm64/set_id_regs.c | 87 > +++++++++++++++++++++++++ > 1 file changed, 87 insertions(+) > > diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c > b/tools/testing/selftests/kvm/arm64/set_id_regs.c > index 7429a1055df5..cb5e6358c59c 100644 > --- a/tools/testing/selftests/kvm/arm64/set_id_regs.c > +++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c > @@ -803,6 +803,89 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu > *vcpu) > ksft_test_result_pass("%s\n", __func__); > } > > +struct reg_ftr_val { > + u64 reg; > + u64 mask; > + u64 val; > +}; > + > +#define REG_FTR_VAL(r, f, v) \ > + { .reg = ARM64_SYS_REG(sys_reg_Op0(SYS_ ## r), \ > + sys_reg_Op1(SYS_ ## r), \ > + sys_reg_CRn(SYS_ ## r), \ > + sys_reg_CRm(SYS_ ## r), \ > + sys_reg_Op2(SYS_ ## r)), \ > + .mask = r ## _ ## f ## _MASK, \ > + .val = (r ## _ ## f ## _ ## v << r ## _ ## f ## _SHIFT) } > + > +static const struct reg_ftr_val s1pie_no_tcr2[] = { > + REG_FTR_VAL(ID_AA64MMFR3_EL1, TCRX, NI), > + REG_FTR_VAL(ID_AA64MMFR3_EL1, S1PIE, IMP), > + { } > +}; > + > +static const struct reg_ftr_val s1poe_no_tcr2[] = { > + REG_FTR_VAL(ID_AA64MMFR3_EL1, TCRX, NI), > + REG_FTR_VAL(ID_AA64MMFR3_EL1, S1POE, IMP), > + { } > +}; > + > +struct ftr_config { > + const char *name; > + const struct reg_ftr_val *regs; > +}; > + > +static const struct ftr_config invalid_configs[] = { > + { .name = "S1PIE without TCRX", .regs = s1pie_no_tcr2 }, > + { .name = "S1POE without TCRX", .regs = s1poe_no_tcr2 }, > +}; > + > +static void test_invalid_config(const struct ftr_config *config) > +{ > + struct kvm_vcpu *vcpu; > + struct kvm_vm *vm; > + const struct reg_ftr_val *field; > + u64 val; > + int ret; > + > + vm = vm_create(1); > + vm_enable_cap(vm, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS, 0); > + vcpu = vm_vcpu_add(vm, 0, guest_code); > + kvm_arch_vm_finalize_vcpus(vm); > + > + /* > + * If we don't manage to set any of the fields assume the > + * system does not support the feature and skip the test. > + */ > + for (field = config->regs; field->reg; field++) { > + val = vcpu_get_reg(vcpu, field->reg); > + val &= ~field->mask; > + val |= field->val; > + __vcpu_set_reg(vcpu, field->reg, val); > + > + if (vcpu_get_reg(vcpu, field->reg) != val) { > + ksft_print_msg("Test setup not supported\n"); > + ksft_test_result_skip("refuse %s\n", config->name); > + goto out; > + } > + } > + > + ret = _vcpu_run(vcpu); > + ksft_test_result(ret < 0 && errno == EINVAL, "refuse %s\n", > + config->name); > +out: > + kvm_vm_free(vm); > +} > + > +static void test_invalid_configs(void) > +{ > + int i; > + > + for (i = 0; i < ARRAY_SIZE(invalid_configs); i++) { > + test_invalid_config(&invalid_configs[i]); > + } VERY NITTY: This is insanely pedantic and I don't mind too much _really_ but in theory should be no {}'s :) > +} > + > int main(void) > { > struct kvm_vcpu *vcpu; > @@ -829,12 +912,16 @@ int main(void) > ksft_print_header(); > > test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST; > + test_cnt += ARRAY_SIZE(invalid_configs); > for (i = 0; i < ARRAY_SIZE(test_regs); i++) > for (j = 0; test_regs[i].ftr_bits[j].type != FTR_END; j++) > test_cnt++; > > ksft_set_plan(test_cnt); > > + /* Do this first in case a break interferes with other tests */ > + test_invalid_configs(); > + > test_vm_ftr_id_regs(vcpu, aarch64_only); > test_vcpu_ftr_id_regs(vcpu); > test_vcpu_non_ftr_id_regs(vcpu); > > -- > 2.47.3 > > -- Cheers, Lorenzo
