KVM: arm64: PMU: Don't overwrite PMUSERENR with vcpu loaded

Currently, with VHE, KVM sets ER, CR, SW and EN bits of
PMUSERENR_EL0 to 1 on vcpu_load(), and saves and restores
the register value for the host on vcpu_load() and vcpu_put().
If the value of those bits are cleared on a pCPU with a vCPU
loaded (armv8pmu_start() would do that when PMU counters are
programmed for the guest), PMU access from the guest EL0 might
be trapped to the guest EL1 directly regardless of the current
PMUSERENR_EL0 value of the vCPU.

Fix this by not letting armv8pmu_start() overwrite PMUSERENR_EL0
on the pCPU where PMUSERENR_EL0 for the guest is loaded, and
instead updating the saved shadow register value for the host
so that the value can be restored on vcpu_put() later.
While vcpu_{put,load}() are manipulating PMUSERENR_EL0, disable
IRQs to prevent a race condition between these processes and IPIs
that attempt to update PMUSERENR_EL0 for the host EL0.

Suggested-by: Mark Rutland <mark.rutland@arm.com>
Suggested-by: Marc Zyngier <maz@kernel.org>
Fixes: 83a7a4d643d3 ("arm64: perf: Enable PMU counter userspace access for perf event")
Signed-off-by: Reiji Watanabe <reijiw@google.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Link: https://lore.kernel.org/r/20230603025035.3781797-3-reijiw@google.com

diff --git a/arch/arm/include/asm/arm_pmuv3.h b/arch/arm/include/asm/arm_pmuv3.h
index 78d3d4b82c6c2598d63d4c691ba0e5ae59b67e5c..92ddd950478edd035d7ce52c86a6d553acd572e2 100644 (file)
--- a/arch/arm/include/asm/arm_pmuv3.h
+++ b/arch/arm/include/asm/arm_pmuv3.h
@@ -222,6 +222,11 @@ static inline bool kvm_pmu_counter_deferred(struct perf_event_attr *attr)
        return false;
 }
 
+static inline bool kvm_set_pmuserenr(u64 val)
+{
+       return false;
+}
+
 /* PMU Version in DFR Register */
 #define ARMV8_PMU_DFR_VER_NI        0
 #define ARMV8_PMU_DFR_VER_V3P4      0x5

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7e7e19ef6993ede45aea71c6f19b624731290fbe..9787503ff43fdfb01703302f7c0e63cb02db901d 100644 (file)
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -699,6 +699,8 @@ struct kvm_vcpu_arch {
 #define SYSREGS_ON_CPU         __vcpu_single_flag(sflags, BIT(4))
 /* Software step state is Active-pending */
 #define DBG_SS_ACTIVE_PENDING  __vcpu_single_flag(sflags, BIT(5))
+/* PMUSERENR for the guest EL0 is on physical CPU */
+#define PMUSERENR_ON_CPU       __vcpu_single_flag(sflags, BIT(6))
 
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
@@ -1065,9 +1067,14 @@ void kvm_arch_vcpu_put_debug_state_flags(struct kvm_vcpu *vcpu);
 #ifdef CONFIG_KVM
 void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr);
 void kvm_clr_pmu_events(u32 clr);
+bool kvm_set_pmuserenr(u64 val);
 #else
 static inline void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr) {}
 static inline void kvm_clr_pmu_events(u32 clr) {}
+static inline bool kvm_set_pmuserenr(u64 val)
+{
+       return false;
+}
 #endif
 
 void kvm_vcpu_load_sysregs_vhe(struct kvm_vcpu *vcpu);

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 0dcb8f7620d1a74a6fbcd3f1c8002735ca176b84..4fe217efa2185a73bea804b1842deff3684918a7 100644 (file)
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -89,6 +89,7 @@ static inline void __activate_traps_common(struct kvm_vcpu *vcpu)
                hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
                ctxt_sys_reg(hctxt, PMUSERENR_EL0) = read_sysreg(pmuserenr_el0);
                write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
+               vcpu_set_flag(vcpu, PMUSERENR_ON_CPU);
        }
 
        vcpu->arch.mdcr_el2_host = read_sysreg(mdcr_el2);
@@ -116,6 +117,7 @@ static inline void __deactivate_traps_common(struct kvm_vcpu *vcpu)
 
                hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
                write_sysreg(ctxt_sys_reg(hctxt, PMUSERENR_EL0), pmuserenr_el0);
+               vcpu_clear_flag(vcpu, PMUSERENR_ON_CPU);
        }
 
        if (cpus_have_final_cap(ARM64_SME)) {

diff --git a/arch/arm64/kvm/hyp/vhe/switch.c b/arch/arm64/kvm/hyp/vhe/switch.c
index 7a1aa511e7da6f68fc88eb4a6b39dcb7927ded09..b37e7c96efea188cf8ed0ed1fb86744948d0065c 100644 (file)
--- a/arch/arm64/kvm/hyp/vhe/switch.c
+++ b/arch/arm64/kvm/hyp/vhe/switch.c
@@ -92,14 +92,28 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 }
 NOKPROBE_SYMBOL(__deactivate_traps);
 
+/*
+ * Disable IRQs in {activate,deactivate}_traps_vhe_{load,put}() to
+ * prevent a race condition between context switching of PMUSERENR_EL0
+ * in __{activate,deactivate}_traps_common() and IPIs that attempts to
+ * update PMUSERENR_EL0. See also kvm_set_pmuserenr().
+ */
 void activate_traps_vhe_load(struct kvm_vcpu *vcpu)
 {
+       unsigned long flags;
+
+       local_irq_save(flags);
        __activate_traps_common(vcpu);
+       local_irq_restore(flags);
 }
 
 void deactivate_traps_vhe_put(struct kvm_vcpu *vcpu)
 {
+       unsigned long flags;
+
+       local_irq_save(flags);
        __deactivate_traps_common(vcpu);
+       local_irq_restore(flags);
 }
 
 static const exit_handler_fn hyp_exit_handlers[] = {

diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
index 7887133d15f0b67f8b04c2c01b8c17ed2d9cb5d8..121f1a14c829c2804127427d4331d91d84d1ab13 100644 (file)
--- a/arch/arm64/kvm/pmu.c
+++ b/arch/arm64/kvm/pmu.c
@@ -209,3 +209,30 @@ void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu)
        kvm_vcpu_pmu_enable_el0(events_host);
        kvm_vcpu_pmu_disable_el0(events_guest);
 }
+
+/*
+ * With VHE, keep track of the PMUSERENR_EL0 value for the host EL0 on the pCPU
+ * where PMUSERENR_EL0 for the guest is loaded, since PMUSERENR_EL0 is switched
+ * to the value for the guest on vcpu_load().  The value for the host EL0
+ * will be restored on vcpu_put(), before returning to userspace.
+ * This isn't necessary for nVHE, as the register is context switched for
+ * every guest enter/exit.
+ *
+ * Return true if KVM takes care of the register. Otherwise return false.
+ */
+bool kvm_set_pmuserenr(u64 val)
+{
+       struct kvm_cpu_context *hctxt;
+       struct kvm_vcpu *vcpu;
+
+       if (!kvm_arm_support_pmu_v3() || !has_vhe())
+               return false;
+
+       vcpu = kvm_get_running_vcpu();
+       if (!vcpu || !vcpu_get_flag(vcpu, PMUSERENR_ON_CPU))
+               return false;
+
+       hctxt = &this_cpu_ptr(&kvm_host_data)->host_ctxt;
+       ctxt_sys_reg(hctxt, PMUSERENR_EL0) = val;
+       return true;
+}

diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
index c98e4039386dbcbdb1de1f3c8848dbf038b0ceb9..93b7edb5f1e7c15fadc945510b3674dfc43374b0 100644 (file)
--- a/drivers/perf/arm_pmuv3.c
+++ b/drivers/perf/arm_pmuv3.c
@@ -677,9 +677,25 @@ static inline u32 armv8pmu_getreset_flags(void)
        return value;
 }
 
+static void update_pmuserenr(u64 val)
+{
+       lockdep_assert_irqs_disabled();
+
+       /*
+        * The current PMUSERENR_EL0 value might be the value for the guest.
+        * If that's the case, have KVM keep tracking of the register value
+        * for the host EL0 so that KVM can restore it before returning to
+        * the host EL0. Otherwise, update the register now.
+        */
+       if (kvm_set_pmuserenr(val))
+               return;
+
+       write_pmuserenr(val);
+}
+
 static void armv8pmu_disable_user_access(void)
 {
-       write_pmuserenr(0);
+       update_pmuserenr(0);
 }
 
 static void armv8pmu_enable_user_access(struct arm_pmu *cpu_pmu)
@@ -695,8 +711,7 @@ static void armv8pmu_enable_user_access(struct arm_pmu *cpu_pmu)
                        armv8pmu_write_evcntr(i, 0);
        }
 
-       write_pmuserenr(0);
-       write_pmuserenr(ARMV8_PMU_USERENR_ER | ARMV8_PMU_USERENR_CR);
+       update_pmuserenr(ARMV8_PMU_USERENR_ER | ARMV8_PMU_USERENR_CR);
 }
 
 static void armv8pmu_enable_event(struct perf_event *event)