KVM: arm64: selftests: Skip ignored AArch32 ID regs in the failed-run test

test_idreg_frozen_after_failed_run() checks that changing a feature ID register after a failed first KVM_RUN is rejected with -EBUSY, and does so for every register in test_regs.

On a host without AArch32 support at EL0, KVM treats the AArch32 ID registers as RAZ/WI: aa32_id_visibility() returns REG_RAZ | REG_USER_WI, and kvm_sys_reg_set_user() ignores the write and returns 0 before set_id_reg() checks whether the ID registers are final. The test then fails on the first such register:

arm64/set_id_regs.c:870: r < 0 && errno == EBUSY ID_DFR0_EL1_PerfMon write after failed first run: r=0 errno=16

KernelCI hits this on every run on an AArch64 only GB300 host.

An ignored write leaves the register unchanged, so the ID registers are still immutable and only the test's expectation is wrong. Skip the AArch32 ID registers that KVM_ARM_GET_REG_WRITABLE_MASKS reports as not writable for this VM, which is decided by the same kvm_supports_32bit_el0() check as the write path. The aarch64_only flag that test_vm_ftr_id_regs() uses is not a good fit here, as it comes from the EL0 field of another VM's vCPU, which reads as AArch64 only whenever that vCPU has EL2 enabled, even on hosts that support AArch32.

---

Tested on an AArch64 only GB300 (Neoverse V2) running KernelCI's arm64 build of mainline 5b0764a53235: set_id_regs fails as above without this change and passes with it (104 pass, 4 skip, 0 fail) in three runs.

This does not change the AArch32 ID register readback issue under nested virtualization that Itaru reported:
https://lore.kernel.org/all/as8x38o9taXgKZym@sm-arm-grace07/

 .../testing/selftests/kvm/arm64/set_id_regs.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

index 10849d21c0dd2..c8c9aaf9d7e86 100644
@@ -815,6 +815,10 @@ static void test_idreg_frozen_after_failed_run(void)
 		SYS_REVIDR_EL1,
 		SYS_AIDR_EL1,
 	};
+	u64 masks[KVM_ARM_FEATURE_ID_RANGE_SIZE];
+	struct reg_mask_range range = {
+		.addr = (__u64)masks,
+	};
 	struct kvm_vcpu_init init;
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;

@@ -831,6 +835,7 @@ static void test_idreg_frozen_after_failed_run(void)
 
 	vm = vm_create(1);
 	vm_enable_cap(vm, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS, 0);
+	vm_ioctl(vm, KVM_ARM_GET_REG_WRITABLE_MASKS, &range);
 	kvm_get_default_vcpu_target(vm, &init);
 	init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
 	vcpu = aarch64_vcpu_add(vm, 0, &init, guest_code);

@@ -852,8 +857,18 @@ static void test_idreg_frozen_after_failed_run(void)
 	 */
 	for (int i = 0; i < ARRAY_SIZE(test_regs); i++) {
 		const struct reg_ftr_bits *ftr_bits = test_regs[i].ftr_bits;
-		u64 reg = KVM_ARM64_SYS_REG(test_regs[i].reg);
-		u64 val = vcpu_get_reg(vcpu, reg);
+		u32 reg_id = test_regs[i].reg;
+		u64 reg = KVM_ARM64_SYS_REG(reg_id);
+		u64 val;
+
+		/*
+		 * KVM ignores writes to the AArch32 ID registers when it can't
+		 * expose AArch32 EL0, and reports them as not writable.
+		 */
+		if (sys_reg_CRm(reg_id) < 4 && !masks[encoding_to_range_idx(reg_id)])
+			continue;
+
+		val = vcpu_get_reg(vcpu, reg);
 
 		for (int j = 0; ftr_bits[j].type != FTR_END; j++) {
 			u64 ftr = (val & ftr_bits[j].mask) >> ftr_bits[j].shift;

2.53.0

Hi Sasha,

[...]

I reproduced this on the FVP AEM model configured with no AArch32 at any EL and with PMUv3, under nVHE: set_id_regs fails on ID_DFR0_EL1_PerfMon without this patch and passes with it. It also passes on QEMU (which supports AArch32 at EL0) under VHE, nVHE and pKVM.

Cheers,
/fuad

Another approach if we're going to change the test would be to have the test support WI registers like the kernel does and also accept a successful write that does not change the value, as you say this is also maintaining immutability. Doing that would handle anything else that gets made WI.

It's been reported by a bunch of people, including me with the test fixup that Itaru was mentioning. It does seem like if we're going to fix this in the test we should fix all the instances rather than just one, or we should just fix the kernel like Marc previously requested (given that we've just stepped on the same landmine...).