Re: [PATCH 1/8] KVM: arm64: nv: Disallow KVM_CAP_ARM_MTE + FEAT_EL2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] KVM: arm64: rejecting MTE with EL2 breaks set_id_regs selftest on NV+MTE hosts
--

commit 6863183ae448414619eb0267884e7d66bfaf521c
Author: Oliver Upton <oupton@kernel.org>

KVM: arm64: nv: Disallow KVM_CAP_ARM_MTE + FEAT_EL2

This commit updates kvm_vcpu_init_check_features() to return -EINVAL when
KVM_ARM_VCPU_HAS_EL2 is requested on a VM that has KVM_CAP_ARM_MTE enabled.

> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 157bf4b812020..a750413c684be 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1793,13 +1793,15 @@ static int kvm_vcpu_init_check_features(struct kvm_vcpu *vcpu,
>  	    !test_bit(KVM_ARM_VCPU_PMU_V3, &features))
>  		return -EINVAL;
>  
> +	/* MTE is incompatible with AArch32 and nested virt */
> +	if (kvm_has_mte(vcpu->kvm) &&
> +	    (test_bit(KVM_ARM_VCPU_EL1_32BIT, &features) ||
> +	     test_bit(KVM_ARM_VCPU_HAS_EL2, &features)))
> +		return -EINVAL;

[Severity: Medium]
Does rejecting KVM_ARM_VCPU_HAS_EL2 when kvm_has_mte(vcpu->kvm) is true break
the arm64 set_id_regs selftest on hosts that support both MTE and nested
virtualization?

In tools/testing/selftests/kvm/arm64/set_id_regs.c, main() enables MTE and
then creates a default vCPU:

tools/testing/selftests/kvm/arm64/set_id_regs.c:main() {
	...
	test_wants_mte();

	vm = vm_create(1);
	vm_enable_cap(vm, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS, 0);
	vcpu = vm_vcpu_add(vm, 0, guest_code);
	...
}

During vm_create(), kvm_arch_vm_post_create() in
tools/testing/selftests/kvm/lib/arm64/processor.c enables KVM_CAP_ARM_MTE on
the VM when request_mte is set:

	if (request_mte && vm_check_cap(vm, KVM_CAP_ARM_MTE))
		vm_enable_cap(vm, KVM_CAP_ARM_MTE, 0);

Then during vm_vcpu_add() -> aarch64_vcpu_setup() ->
kvm_get_default_vcpu_target(), the selftest library checks vm_supports_el2()
and sets KVM_ARM_VCPU_HAS_EL2 by default on NV-capable hosts:

tools/testing/selftests/kvm/lib/arm64/processor.c:vm_supports_el2() {
	return vm_check_cap(vm, KVM_CAP_ARM_EL2) && vm->arch.has_gic;
}

tools/testing/selftests/kvm/lib/arm64/processor.c:kvm_get_default_vcpu_target() {
	...
	vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &preferred);
	if (vm_supports_el2(vm))
		preferred.features[0] |= BIT(KVM_ARM_VCPU_HAS_EL2);
	...
}

Because kvm_vm_ioctl_check_extension() in arch/arm64/kvm/arm.c still reports
KVM_CAP_ARM_EL2 as supported on a VM fd where KVM_CAP_ARM_MTE was enabled:

	case KVM_CAP_ARM_EL2:
		r = cpus_have_final_cap(ARM64_HAS_NESTED_VIRT);
		break;

and vm_supports_el2() does not check !request_mte, aarch64_vcpu_setup() passes
KVM_ARM_VCPU_HAS_EL2 to KVM_ARM_VCPU_INIT, which now fails with -EINVAL unless
the test is run with NV=0.

Should kvm_vm_ioctl_check_extension() gate KVM_CAP_ARM_EL2 on
!kvm_has_mte(kvm) (or vm_supports_el2() check !request_mte) so userspace and
selftests do not request EL2 on MTE-enabled VMs?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104192124.3932743-1-oupton@kernel.org?part=1