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

[Marc Zyngier]

On 17/11/16 13:30, Will Deacon wrote:
> On Wed, Nov 16, 2016 at 11:09:20AM -0600, Wei Huang wrote:
>> KVM calls kvm_pmu_set_counter_event_type() when PMCCFILTR is configured.
>> But this function can't deals with PMCCFILTR correctly because the evtCount
>> bits of PMCCFILTR, which is reserved 0, conflits with the SW_INCR event
>> type of other PMXEVTYPER registers. To fix it, when eventsel == 0, this
>> function shouldn't return immediately; 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
>> blindly to attr.config. Instead it ought to convert the request to the
>> "cpu cycle" event type (i.e. 0x11).
>>
>> To support this patch and to prevent duplicated definitions, a limited
>> set of ARMv8 perf event types were relocated from perf_event.c to
>> asm/perf_event.h.
>>
>> Signed-off-by: Wei Huang 
>> ---
>>  arch/arm64/include/asm/perf_event.h | 10 +-
>>  arch/arm64/kernel/perf_event.c  | 10 +-
>>  virt/kvm/arm/pmu.c  |  8 +---
>>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> Acked-by: Will Deacon 
> 
> I'm assuming this will go via kvm-arm.

Yup, I'll take it. Thanks.

M.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

[Will Deacon]

On Wed, Nov 16, 2016 at 11:09:20AM -0600, Wei Huang wrote:
> KVM calls kvm_pmu_set_counter_event_type() when PMCCFILTR is configured.
> But this function can't deals with PMCCFILTR correctly because the evtCount
> bits of PMCCFILTR, which is reserved 0, conflits with the SW_INCR event
> type of other PMXEVTYPER registers. To fix it, when eventsel == 0, this
> function shouldn't return immediately; 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
> blindly to attr.config. Instead it ought to convert the request to the
> "cpu cycle" event type (i.e. 0x11).
> 
> To support this patch and to prevent duplicated definitions, a limited
> set of ARMv8 perf event types were relocated from perf_event.c to
> asm/perf_event.h.
> 
> Signed-off-by: Wei Huang 
> ---
>  arch/arm64/include/asm/perf_event.h | 10 +-
>  arch/arm64/kernel/perf_event.c  | 10 +-
>  virt/kvm/arm/pmu.c  |  8 +---
>  3 files changed, 15 insertions(+), 13 deletions(-)

Acked-by: Will Deacon 

I'm assuming this will go via kvm-arm.

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

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

[Wei Huang]

KVM calls kvm_pmu_set_counter_event_type() when PMCCFILTR is configured.
But this function can't deals with PMCCFILTR correctly because the evtCount
bits of PMCCFILTR, which is reserved 0, conflits with the SW_INCR event
type of other PMXEVTYPER registers. To fix it, when eventsel == 0, this
function shouldn't return immediately; 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
blindly to attr.config. Instead it ought to convert the request to the
"cpu cycle" event type (i.e. 0x11).

To support this patch and to prevent duplicated definitions, a limited
set of ARMv8 perf event types were relocated from perf_event.c to
asm/perf_event.h.

Signed-off-by: Wei Huang 
---
 arch/arm64/include/asm/perf_event.h | 10 +-
 arch/arm64/kernel/perf_event.c  | 10 +-
 virt/kvm/arm/pmu.c  |  8 +---
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/include/asm/perf_event.h 
b/arch/arm64/include/asm/perf_event.h
index 2065f46..38b6a2b 100644
--- a/arch/arm64/include/asm/perf_event.h
+++ b/arch/arm64/include/asm/perf_event.h
@@ -46,7 +46,15 @@
 #defineARMV8_PMU_EVTYPE_MASK   0xc800  /* Mask for writable 
bits */
 #defineARMV8_PMU_EVTYPE_EVENT  0x  /* Mask for EVENT bits 
*/
 
-#define ARMV8_PMU_EVTYPE_EVENT_SW_INCR 0   /* Software increment event */
+/*
+ * PMUv3 event types: required events
+ */
+#define ARMV8_PMUV3_PERFCTR_SW_INCR0x00
+#define ARMV8_PMUV3_PERFCTR_L1D_CACHE_REFILL   0x03
+#define ARMV8_PMUV3_PERFCTR_L1D_CACHE  0x04
+#define ARMV8_PMUV3_PERFCTR_BR_MIS_PRED0x10
+#define ARMV8_PMUV3_PERFCTR_CPU_CYCLES 0x11
+#define ARMV8_PMUV3_PERFCTR_BR_PRED0x12
 
 /*
  * Event filters for PMUv3
diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
index a9310a6..57ae9d9 100644
--- a/arch/arm64/kernel/perf_event.c
+++ b/arch/arm64/kernel/perf_event.c
@@ -31,17 +31,9 @@
 
 /*
  * ARMv8 PMUv3 Performance Events handling code.
- * Common event types.
+ * Common event types (some are defined in asm/perf_event.h).
  */
 
-/* Required events. */
-#define ARMV8_PMUV3_PERFCTR_SW_INCR0x00
-#define ARMV8_PMUV3_PERFCTR_L1D_CACHE_REFILL   0x03
-#define ARMV8_PMUV3_PERFCTR_L1D_CACHE  0x04
-#define ARMV8_PMUV3_PERFCTR_BR_MIS_PRED0x10
-#define ARMV8_PMUV3_PERFCTR_CPU_CYCLES 0x11
-#define ARMV8_PMUV3_PERFCTR_BR_PRED0x12
-
 /* At least one of the following is required. */
 #define ARMV8_PMUV3_PERFCTR_INST_RETIRED   0x08
 #define ARMV8_PMUV3_PERFCTR_INST_SPEC  0x1B
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 6e9c40e..69ccce3 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -305,7 +305,7 @@ void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 
val)
continue;
type = vcpu_sys_reg(vcpu, PMEVTYPER0_EL0 + i)
   & ARMV8_PMU_EVTYPE_EVENT;
-   if ((type == ARMV8_PMU_EVTYPE_EVENT_SW_INCR)
+   if ((type == ARMV8_PMUV3_PERFCTR_SW_INCR)
&& (enable & BIT(i))) {
reg = vcpu_sys_reg(vcpu, PMEVCNTR0_EL0 + i) + 1;
reg = lower_32_bits(reg);
@@ -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_PMUV3_PERFCTR_SW_INCR &&
+   select_idx != ARMV8_PMU_CYCLE_IDX)
return;
 
memset(, 0, sizeof(struct perf_event_attr));
@@ -391,7 +392,8 @@ 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) ?
+   ARMV8_PMUV3_PERFCTR_CPU_CYCLES : eventsel;
 
counter = kvm_pmu_get_counter_value(vcpu, select_idx);
/* The initial sample period (overflow count) of an event. */
-- 
2.7.4

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