[PATCH] KVM: arm64: Refuse to run VCPU if PMU is not initialized

* [PATCH] KVM: arm64: Refuse to run VCPU if PMU is not initialized
@ 2020-11-26 14:49 ` Alexandru Elisei
  0 siblings, 0 replies; 4+ messages in thread
From: Alexandru Elisei @ 2020-11-26 14:49 UTC (permalink / raw)
  To: maz, james.morse, julien.thierry.kdev, suzuki.poulose,
	linux-arm-kernel, kvmarm

When enabling the PMU in kvm_arm_pmu_v3_enable(), KVM returns early if the
PMU flag created is false and skips any other checks. Because PMU emulation
is gated only on the VCPU feature being set, this makes it possible for
userspace to get away with setting the VCPU feature but not doing any
initialization for the PMU. Fix it by returning an error when trying to run
the VCPU if the PMU hasn't been initialized correctly.

The PMU is marked as created only if the interrupt ID has been set when
using an in-kernel irqchip. This means the same check in
kvm_arm_pmu_v3_enable() is redundant, remove it.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
Patch is based on top of [1].

This has been reported at [2]. I tested the patch like in the report, with
a modified version of kvmtool that sets the PMU feature, but doesn't do any
initialization.

Without this patch, when running the pmu kvm-unit-tests test, I get the
warning described in the report. With this patch, KVM refuses to run the
VCPU:

$ ./lkvm-pmu run -c1 -m 64 -f /path/to/arm/pmu.flat --pmu -p cycle-counter
  # lkvm run --firmware /path/to/arm/pmu.flat -m 64 -c 1 --name guest-207
  Info: Placing fdt at 0x80200000 - 0x80210000
KVM_RUN failed: Invalid argument

I also tested what happens if I run a Linux guest without this patch with
the modified version of kvmtool. The PMU is detected, but the guest doesn't
receive any PMU interrupts. With this patch, KVM refuses to run the VCPU.

I decided to return -EINVAL instead of -ENOEXEC because that is the error
code that kvm_arm_pmu_v3_enable() was already returning if the interrupt ID
was not initialized, which implies that the PMU hadn't been initialized.

I also noticed that there are other places which return different error
codes for KVM_RUN, and I'm in the process of untangling that to send a
patch to document them.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git/log/?h=queue
[2] https://www.spinics.net/lists/arm-kernel/msg857927.html

 arch/arm64/kvm/pmu-emul.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 643cf819f3c0..398f6df1bbe4 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -825,9 +825,12 @@ bool kvm_arm_support_pmu_v3(void)
 
 int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
 {
-	if (!vcpu->arch.pmu.created)
+	if (!kvm_vcpu_has_pmu(vcpu))
 		return 0;
 
+	if (!vcpu->arch.pmu.created)
+		return -EINVAL;
+
 	/*
 	 * A valid interrupt configuration for the PMU is either to have a
 	 * properly configured interrupt number and using an in-kernel
@@ -835,9 +838,6 @@ int kvm_arm_pmu_v3_enable(struct kvm_vcpu *vcpu)
 	 */
 	if (irqchip_in_kernel(vcpu->kvm)) {
 		int irq = vcpu->arch.pmu.irq_num;
-		if (!kvm_arm_pmu_irq_initialized(vcpu))
-			return -EINVAL;
-
 		/*
 		 * If we are using an in-kernel vgic, at this point we know
 		 * the vgic will be initialized, so we can check the PMU irq
-- 
2.29.2

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 4+ messages in thread