[PATCH 1/1] KVM: ARM64: Fix the issues when PMCCFILTR is configured

On Thu, Nov 03 2016 at 02:29:26 PM, Wei Huang [off-list ref] wrote:

On 11/02/2016 11:11 PM, cov at codeaurora.org wrote:

quoted

Hi Wei,

On 2016-11-02 14:55, Wei Huang wrote:

quoted

KVM calls kvm_pmu_set_counter_event_type() when PMCCFILTR is configured.
But this function can't deals with PMCCFILTR correctly because the
evtCount
bit of PMCCFILTR, which is reserved 0, conflits with the SW_INCR event
type of other PMXEVTYPER<n> registers. To fix it, when eventsel == 0, KVM
shouldn't return immediately, but instead it needs to check further if
select_idx is ARMV8_PMU_CYCLE_IDX.

Another issue is that KVM shouldn't copy the eventsel bits of PMCCFILTER
directly to attr.config. Istead it shoudl convert the request to
perf_event of type 0x11 (i.e. the "cpu cycle" event type).

Signed-off-by: Wei Huang <redacted>
---
 virt/kvm/arm/pmu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 6e9c40e..13cc812 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c

@@ -379,7 +379,8 @@ void kvm_pmu_set_counter_event_type(struct

kvm_vcpu *vcpu, u64 data,
     eventsel = data & ARMV8_PMU_EVTYPE_EVENT;

     /* Software increment event does't need to be backed by a perf
event */
-    if (eventsel == ARMV8_PMU_EVTYPE_EVENT_SW_INCR)
+    if (eventsel == ARMV8_PMU_EVTYPE_EVENT_SW_INCR &&
+        select_idx != ARMV8_PMU_CYCLE_IDX)
         return;

     memset(&attr, 0, sizeof(struct perf_event_attr));

@@ -391,7 +392,7 @@ void kvm_pmu_set_counter_event_type(struct

kvm_vcpu *vcpu, u64 data,
     attr.exclude_kernel = data & ARMV8_PMU_EXCLUDE_EL1 ? 1 : 0;
     attr.exclude_hv = 1; /* Don't count EL2 events */
     attr.exclude_host = 1; /* Don't count host events */
-    attr.config = eventsel;
+    attr.config = (select_idx == ARMV8_PMU_CYCLE_IDX) ? 0x011 :
eventsel;

Nit: Is there some way you could use ARMV8_PMUV3_PERFCTR_CPU_CYCLES
currently
defined in arch/arm64/kernel/perf_event.c?

The event definitions in perf_event.c are local to this file. To use
them, I have to re-factor them out to a header file, which is bigger
than this patch intended. How about adding the following line to
arch/arm64/include/asm/perf_event.h (i.e. the same file of
ARMV8_PMU_EVTYPE_EVENT_SW_INCR)?

#define ARMV8_PMU_EVTYPE_EVENT_CPU_CYCLES  0x11

Patch size is not relevant, and I'd rather avoid both magic numbers and
duplicate definitions. It would make sense to me to move all the
required events to this header file, and fix the SW_INCR duplication.

Please CC the ARM perf maintainers when you repost this patch.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.