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@xxxxxxx> --- 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@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
