On 22/05/2019 17:26, Andrew Murray wrote:
On Wed, May 22, 2019 at 05:07:31PM +0100, Marc Zyngier wrote:
On 22/05/2019 16:30, Andrew Murray wrote:
We currently use pmc->bitmask to determine the width of the pmc - however
it's superfluous as the pmc index already describes if the pmc is a cycle
counter or event counter. The architecture clearly describes the widths of
these counters.
Let's remove the bitmask to simplify the code.
Signed-off-by: Andrew Murray <andrew.murray@xxxxxxx>
---
include/kvm/arm_pmu.h | 1 -
virt/kvm/arm/pmu.c | 15 +++++----------
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index b73f31baca52..2f0e28dc5a9e 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -28,7 +28,6 @@
struct kvm_pmc {
u8 idx; /* index into the pmu->pmc array */
struct perf_event *perf_event;
- u64 bitmask;
};
-
- if (val & ARMV8_PMU_PMCR_LC) {
- pmc = &pmu->pmc[ARMV8_PMU_CYCLE_IDX];
- pmc->bitmask = 0xffffffffffffffffUL;
- }
}
...
static bool kvm_pmu_counter_is_enabled(struct kvm_vcpu *vcpu, u64 select_idx)
@@ -420,7 +415,7 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx)
counter = kvm_pmu_get_counter_value(vcpu, select_idx);
/* The initial sample period (overflow count) of an event. */
- attr.sample_period = (-counter) & pmc->bitmask;
+ attr.sample_period = (-counter) & GENMASK(31, 0);
Isn't this the one case where the bitmask actually matters? If we're
dealing with the cycle counter, it shouldn't be truncated, right?
Ah yes, that should be conditional on idx as well.
The mask for Cycle counter also depends on the PMCR.LC field set by the
guest, isn't it ? So unless we correlate that with the idx, we could be
passing in wrong results ?
Suzuki
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
