| Message ID | 20221113163832.3154370-5-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: PMU: Fixing chained events, and PMUv3p5 support | expand |
On Sun, Nov 13, 2022 at 04:38:20PM +0000, Marc Zyngier wrote: > The PMU architecture makes a subtle difference between a 64bit > counter and a counter that has a 64bit overflow. This is for example > the case of the cycle counter, which can generate an overflow on > a 32bit boundary if PMCR_EL0.LC==0 despite the accumulation being > done on 64 bits. > > Use this distinction in the few cases where it matters in the code, > as we will reuse this with PMUv3p5 long counters. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kvm/pmu-emul.c | 43 ++++++++++++++++++++++++++++----------- > 1 file changed, 31 insertions(+), 12 deletions(-) > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > index 69b67ab3c4bf..d050143326b5 100644 > --- a/arch/arm64/kvm/pmu-emul.c > +++ b/arch/arm64/kvm/pmu-emul.c > @@ -50,6 +50,11 @@ static u32 kvm_pmu_event_mask(struct kvm *kvm) > * @select_idx: The counter index > */ > static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > +{ > + return (select_idx == ARMV8_PMU_CYCLE_IDX); > +} > + > +static bool kvm_pmu_idx_has_64bit_overflow(struct kvm_vcpu *vcpu, u64 select_idx) > { > return (select_idx == ARMV8_PMU_CYCLE_IDX && > __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC); > @@ -57,7 +62,8 @@ static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > > static bool kvm_pmu_counter_can_chain(struct kvm_vcpu *vcpu, u64 idx) > { > - return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX); > + return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX && > + !kvm_pmu_idx_has_64bit_overflow(vcpu, idx)); > } > > static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc) > @@ -97,7 +103,7 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx) > counter += perf_event_read_value(pmc->perf_event, &enabled, > &running); > > - if (select_idx != ARMV8_PMU_CYCLE_IDX) > + if (!kvm_pmu_idx_is_64bit(vcpu, select_idx)) > counter = lower_32_bits(counter); > > return counter; > @@ -423,6 +429,23 @@ static void kvm_pmu_counter_increment(struct kvm_vcpu *vcpu, > } > } > > +/* Compute the sample period for a given counter value */ > +static u64 compute_period(struct kvm_vcpu *vcpu, u64 select_idx, u64 counter) > +{ > + u64 val; > + > + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { > + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) > + val = -(counter & GENMASK(31, 0)); If I understand things correctly, this might be missing another mask: + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) { + val = -(counter & GENMASK(31, 0)); + val &= GENMASK(31, 0); + } else { For example, if the counter is 64-bits wide, it overflows at 32-bits, and it is _one_ sample away from overflowing at 32-bits: 0x01010101_ffffffff Then "val = (-counter) & GENMASK(63, 0)" would return 0xffffffff_00000001. But the right period is 0x00000000_00000001 (it's one sample away from overflowing). > + else > + val = (-counter) & GENMASK(63, 0); > + } else { > + val = (-counter) & GENMASK(31, 0); > + } > + > + return val; > +} > + > /** > * When the perf event overflows, set the overflow status and inform the vcpu. > */ > @@ -442,10 +465,7 @@ static void kvm_pmu_perf_overflow(struct perf_event *perf_event, > * Reset the sample period to the architectural limit, > * i.e. the point where the counter overflows. > */ > - period = -(local64_read(&perf_event->count)); > - > - if (!kvm_pmu_idx_is_64bit(vcpu, pmc->idx)) > - period &= GENMASK(31, 0); > + period = compute_period(vcpu, idx, local64_read(&perf_event->count)); > > local64_set(&perf_event->hw.period_left, 0); > perf_event->attr.sample_period = period; > @@ -571,14 +591,13 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) > > /* > * If counting with a 64bit counter, advertise it to the perf > - * code, carefully dealing with the initial sample period. > + * code, carefully dealing with the initial sample period > + * which also depends on the overflow. > */ > - if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { > + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) > attr.config1 |= PERF_ATTR_CFG1_COUNTER_64BIT; > - attr.sample_period = (-counter) & GENMASK(63, 0); > - } else { > - attr.sample_period = (-counter) & GENMASK(31, 0); > - } > + > + attr.sample_period = compute_period(vcpu, select_idx, counter); > > event = perf_event_create_kernel_counter(&attr, -1, current, > kvm_pmu_perf_overflow, pmc); > -- > 2.34.1 > >
On Thu, Dec 01, 2022 at 08:47:47AM -0800, Ricardo Koller wrote: > On Sun, Nov 13, 2022 at 04:38:20PM +0000, Marc Zyngier wrote: > > The PMU architecture makes a subtle difference between a 64bit > > counter and a counter that has a 64bit overflow. This is for example > > the case of the cycle counter, which can generate an overflow on > > a 32bit boundary if PMCR_EL0.LC==0 despite the accumulation being > > done on 64 bits. > > > > Use this distinction in the few cases where it matters in the code, > > as we will reuse this with PMUv3p5 long counters. > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/kvm/pmu-emul.c | 43 ++++++++++++++++++++++++++++----------- > > 1 file changed, 31 insertions(+), 12 deletions(-) > > > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > > index 69b67ab3c4bf..d050143326b5 100644 > > --- a/arch/arm64/kvm/pmu-emul.c > > +++ b/arch/arm64/kvm/pmu-emul.c > > @@ -50,6 +50,11 @@ static u32 kvm_pmu_event_mask(struct kvm *kvm) > > * @select_idx: The counter index > > */ > > static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > > +{ > > + return (select_idx == ARMV8_PMU_CYCLE_IDX); > > +} > > + > > +static bool kvm_pmu_idx_has_64bit_overflow(struct kvm_vcpu *vcpu, u64 select_idx) > > { > > return (select_idx == ARMV8_PMU_CYCLE_IDX && > > __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC); > > @@ -57,7 +62,8 @@ static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > > > > static bool kvm_pmu_counter_can_chain(struct kvm_vcpu *vcpu, u64 idx) > > { > > - return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX); > > + return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX && > > + !kvm_pmu_idx_has_64bit_overflow(vcpu, idx)); > > } > > > > static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc) > > @@ -97,7 +103,7 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx) > > counter += perf_event_read_value(pmc->perf_event, &enabled, > > &running); > > > > - if (select_idx != ARMV8_PMU_CYCLE_IDX) > > + if (!kvm_pmu_idx_is_64bit(vcpu, select_idx)) > > counter = lower_32_bits(counter); > > > > return counter; > > @@ -423,6 +429,23 @@ static void kvm_pmu_counter_increment(struct kvm_vcpu *vcpu, > > } > > } > > > > +/* Compute the sample period for a given counter value */ > > +static u64 compute_period(struct kvm_vcpu *vcpu, u64 select_idx, u64 counter) > > +{ > > + u64 val; > > + > > + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { > > + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) > > + val = -(counter & GENMASK(31, 0)); > > If I understand things correctly, this might be missing another mask: > > + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) { > + val = -(counter & GENMASK(31, 0)); > + val &= GENMASK(31, 0); > + } else { > > For example, if the counter is 64-bits wide, it overflows at 32-bits, > and it is _one_ sample away from overflowing at 32-bits: > > 0x01010101_ffffffff > > Then "val = (-counter) & GENMASK(63, 0)" would return 0xffffffff_00000001. Sorry, this should be: Then "val = -(counter & GENMASK(31, 0))" would return 0xffffffff_00000001. > But the right period is 0x00000000_00000001 (it's one sample away from > overflowing). > > > + else > > + val = (-counter) & GENMASK(63, 0); > > + } else { > > + val = (-counter) & GENMASK(31, 0); > > + } > > + > > + return val; > > +} > > + > > /** > > * When the perf event overflows, set the overflow status and inform the vcpu. > > */ > > @@ -442,10 +465,7 @@ static void kvm_pmu_perf_overflow(struct perf_event *perf_event, > > * Reset the sample period to the architectural limit, > > * i.e. the point where the counter overflows. > > */ > > - period = -(local64_read(&perf_event->count)); > > - > > - if (!kvm_pmu_idx_is_64bit(vcpu, pmc->idx)) > > - period &= GENMASK(31, 0); > > + period = compute_period(vcpu, idx, local64_read(&perf_event->count)); > > > > local64_set(&perf_event->hw.period_left, 0); > > perf_event->attr.sample_period = period; > > @@ -571,14 +591,13 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) > > > > /* > > * If counting with a 64bit counter, advertise it to the perf > > - * code, carefully dealing with the initial sample period. > > + * code, carefully dealing with the initial sample period > > + * which also depends on the overflow. > > */ > > - if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { > > + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) > > attr.config1 |= PERF_ATTR_CFG1_COUNTER_64BIT; > > - attr.sample_period = (-counter) & GENMASK(63, 0); > > - } else { > > - attr.sample_period = (-counter) & GENMASK(31, 0); > > - } > > + > > + attr.sample_period = compute_period(vcpu, select_idx, counter); > > > > event = perf_event_create_kernel_counter(&attr, -1, current, > > kvm_pmu_perf_overflow, pmc); > > -- > > 2.34.1 > > > >
On Thu, 01 Dec 2022 16:51:46 +0000, Ricardo Koller <ricarkol@google.com> wrote: > > On Thu, Dec 01, 2022 at 08:47:47AM -0800, Ricardo Koller wrote: > > On Sun, Nov 13, 2022 at 04:38:20PM +0000, Marc Zyngier wrote: > > > The PMU architecture makes a subtle difference between a 64bit > > > counter and a counter that has a 64bit overflow. This is for example > > > the case of the cycle counter, which can generate an overflow on > > > a 32bit boundary if PMCR_EL0.LC==0 despite the accumulation being > > > done on 64 bits. > > > > > > Use this distinction in the few cases where it matters in the code, > > > as we will reuse this with PMUv3p5 long counters. > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > --- > > > arch/arm64/kvm/pmu-emul.c | 43 ++++++++++++++++++++++++++++----------- > > > 1 file changed, 31 insertions(+), 12 deletions(-) > > > > > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > > > index 69b67ab3c4bf..d050143326b5 100644 > > > --- a/arch/arm64/kvm/pmu-emul.c > > > +++ b/arch/arm64/kvm/pmu-emul.c > > > @@ -50,6 +50,11 @@ static u32 kvm_pmu_event_mask(struct kvm *kvm) > > > * @select_idx: The counter index > > > */ > > > static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > > > +{ > > > + return (select_idx == ARMV8_PMU_CYCLE_IDX); > > > +} > > > + > > > +static bool kvm_pmu_idx_has_64bit_overflow(struct kvm_vcpu *vcpu, u64 select_idx) > > > { > > > return (select_idx == ARMV8_PMU_CYCLE_IDX && > > > __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC); > > > @@ -57,7 +62,8 @@ static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > > > > > > static bool kvm_pmu_counter_can_chain(struct kvm_vcpu *vcpu, u64 idx) > > > { > > > - return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX); > > > + return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX && > > > + !kvm_pmu_idx_has_64bit_overflow(vcpu, idx)); > > > } > > > > > > static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc) > > > @@ -97,7 +103,7 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx) > > > counter += perf_event_read_value(pmc->perf_event, &enabled, > > > &running); > > > > > > - if (select_idx != ARMV8_PMU_CYCLE_IDX) > > > + if (!kvm_pmu_idx_is_64bit(vcpu, select_idx)) > > > counter = lower_32_bits(counter); > > > > > > return counter; > > > @@ -423,6 +429,23 @@ static void kvm_pmu_counter_increment(struct kvm_vcpu *vcpu, > > > } > > > } > > > > > > +/* Compute the sample period for a given counter value */ > > > +static u64 compute_period(struct kvm_vcpu *vcpu, u64 select_idx, u64 counter) > > > +{ > > > + u64 val; > > > + > > > + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { > > > + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) > > > + val = -(counter & GENMASK(31, 0)); > > > > If I understand things correctly, this might be missing another mask: > > > > + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) { > > + val = -(counter & GENMASK(31, 0)); > > + val &= GENMASK(31, 0); > > + } else { > > > > For example, if the counter is 64-bits wide, it overflows at 32-bits, > > and it is _one_ sample away from overflowing at 32-bits: > > > > 0x01010101_ffffffff > > > > Then "val = (-counter) & GENMASK(63, 0)" would return 0xffffffff_00000001. > > Sorry, this should be: > > Then "val = -(counter & GENMASK(31, 0))" would return 0xffffffff_00000001. > > > But the right period is 0x00000000_00000001 (it's one sample away from > > overflowing). Yup, this is a bit bogus. But this can be simplified by falling back to the normal 32bit handling (on top of the pmu-unchained branch): diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index d8ea39943086..24908400e190 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -461,14 +461,10 @@ static u64 compute_period(struct kvm_pmc *pmc, u64 counter) { u64 val; - if (kvm_pmc_is_64bit(pmc)) { - if (!kvm_pmc_has_64bit_overflow(pmc)) - val = -(counter & GENMASK(31, 0)); - else - val = (-counter) & GENMASK(63, 0); - } else { + if (kvm_pmc_is_64bit(pmc) && kvm_pmc_has_64bit_overflow(pmc)) + val = (-counter) & GENMASK(63, 0); + else val = (-counter) & GENMASK(31, 0); - } return val; } which satisfies the requirement without any extra masking, and makes it plain that only a 64bit counter with 64bit overflow gets its period computed on the full 64bit, and that anyone else gets the 32bit truncation. I'll stash yet another patch on top and push it onto -next. Thanks! M.
On Mon, Dec 05, 2022 at 12:05:32PM +0000, Marc Zyngier wrote: > On Thu, 01 Dec 2022 16:51:46 +0000, > Ricardo Koller <ricarkol@google.com> wrote: > > > > On Thu, Dec 01, 2022 at 08:47:47AM -0800, Ricardo Koller wrote: > > > On Sun, Nov 13, 2022 at 04:38:20PM +0000, Marc Zyngier wrote: > > > > The PMU architecture makes a subtle difference between a 64bit > > > > counter and a counter that has a 64bit overflow. This is for example > > > > the case of the cycle counter, which can generate an overflow on > > > > a 32bit boundary if PMCR_EL0.LC==0 despite the accumulation being > > > > done on 64 bits. > > > > > > > > Use this distinction in the few cases where it matters in the code, > > > > as we will reuse this with PMUv3p5 long counters. > > > > > > > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > > > --- > > > > arch/arm64/kvm/pmu-emul.c | 43 ++++++++++++++++++++++++++++----------- > > > > 1 file changed, 31 insertions(+), 12 deletions(-) > > > > > > > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > > > > index 69b67ab3c4bf..d050143326b5 100644 > > > > --- a/arch/arm64/kvm/pmu-emul.c > > > > +++ b/arch/arm64/kvm/pmu-emul.c > > > > @@ -50,6 +50,11 @@ static u32 kvm_pmu_event_mask(struct kvm *kvm) > > > > * @select_idx: The counter index > > > > */ > > > > static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > > > > +{ > > > > + return (select_idx == ARMV8_PMU_CYCLE_IDX); > > > > +} > > > > + > > > > +static bool kvm_pmu_idx_has_64bit_overflow(struct kvm_vcpu *vcpu, u64 select_idx) > > > > { > > > > return (select_idx == ARMV8_PMU_CYCLE_IDX && > > > > __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC); > > > > @@ -57,7 +62,8 @@ static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) > > > > > > > > static bool kvm_pmu_counter_can_chain(struct kvm_vcpu *vcpu, u64 idx) > > > > { > > > > - return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX); > > > > + return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX && > > > > + !kvm_pmu_idx_has_64bit_overflow(vcpu, idx)); > > > > } > > > > > > > > static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc) > > > > @@ -97,7 +103,7 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx) > > > > counter += perf_event_read_value(pmc->perf_event, &enabled, > > > > &running); > > > > > > > > - if (select_idx != ARMV8_PMU_CYCLE_IDX) > > > > + if (!kvm_pmu_idx_is_64bit(vcpu, select_idx)) > > > > counter = lower_32_bits(counter); > > > > > > > > return counter; > > > > @@ -423,6 +429,23 @@ static void kvm_pmu_counter_increment(struct kvm_vcpu *vcpu, > > > > } > > > > } > > > > > > > > +/* Compute the sample period for a given counter value */ > > > > +static u64 compute_period(struct kvm_vcpu *vcpu, u64 select_idx, u64 counter) > > > > +{ > > > > + u64 val; > > > > + > > > > + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { > > > > + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) > > > > + val = -(counter & GENMASK(31, 0)); > > > > > > If I understand things correctly, this might be missing another mask: > > > > > > + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) { > > > + val = -(counter & GENMASK(31, 0)); > > > + val &= GENMASK(31, 0); > > > + } else { > > > > > > For example, if the counter is 64-bits wide, it overflows at 32-bits, > > > and it is _one_ sample away from overflowing at 32-bits: > > > > > > 0x01010101_ffffffff > > > > > > Then "val = (-counter) & GENMASK(63, 0)" would return 0xffffffff_00000001. > > > > Sorry, this should be: > > > > Then "val = -(counter & GENMASK(31, 0))" would return 0xffffffff_00000001. > > > > > But the right period is 0x00000000_00000001 (it's one sample away from > > > overflowing). > > Yup, this is a bit bogus. But this can be simplified by falling back > to the normal 32bit handling (on top of the pmu-unchained branch): > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > index d8ea39943086..24908400e190 100644 > --- a/arch/arm64/kvm/pmu-emul.c > +++ b/arch/arm64/kvm/pmu-emul.c > @@ -461,14 +461,10 @@ static u64 compute_period(struct kvm_pmc *pmc, u64 counter) > { > u64 val; > > - if (kvm_pmc_is_64bit(pmc)) { > - if (!kvm_pmc_has_64bit_overflow(pmc)) > - val = -(counter & GENMASK(31, 0)); > - else > - val = (-counter) & GENMASK(63, 0); > - } else { > + if (kvm_pmc_is_64bit(pmc) && kvm_pmc_has_64bit_overflow(pmc)) Great, thanks! Yes, that definitely makes things simpler ^. > + val = (-counter) & GENMASK(63, 0); > + else > val = (-counter) & GENMASK(31, 0); > - } > > return val; > } > > which satisfies the requirement without any extra masking, and makes > it plain that only a 64bit counter with 64bit overflow gets its period > computed on the full 64bit, and that anyone else gets the 32bit > truncation. > > I'll stash yet another patch on top and push it onto -next. > > Thanks! > > M. > > -- > Without deviation from the norm, progress is not possible.
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index 69b67ab3c4bf..d050143326b5 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -50,6 +50,11 @@ static u32 kvm_pmu_event_mask(struct kvm *kvm) * @select_idx: The counter index */ static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) +{ + return (select_idx == ARMV8_PMU_CYCLE_IDX); +} + +static bool kvm_pmu_idx_has_64bit_overflow(struct kvm_vcpu *vcpu, u64 select_idx) { return (select_idx == ARMV8_PMU_CYCLE_IDX && __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC); @@ -57,7 +62,8 @@ static bool kvm_pmu_idx_is_64bit(struct kvm_vcpu *vcpu, u64 select_idx) static bool kvm_pmu_counter_can_chain(struct kvm_vcpu *vcpu, u64 idx) { - return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX); + return (!(idx & 1) && (idx + 1) < ARMV8_PMU_CYCLE_IDX && + !kvm_pmu_idx_has_64bit_overflow(vcpu, idx)); } static struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc) @@ -97,7 +103,7 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx) counter += perf_event_read_value(pmc->perf_event, &enabled, &running); - if (select_idx != ARMV8_PMU_CYCLE_IDX) + if (!kvm_pmu_idx_is_64bit(vcpu, select_idx)) counter = lower_32_bits(counter); return counter; @@ -423,6 +429,23 @@ static void kvm_pmu_counter_increment(struct kvm_vcpu *vcpu, } } +/* Compute the sample period for a given counter value */ +static u64 compute_period(struct kvm_vcpu *vcpu, u64 select_idx, u64 counter) +{ + u64 val; + + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { + if (!kvm_pmu_idx_has_64bit_overflow(vcpu, select_idx)) + val = -(counter & GENMASK(31, 0)); + else + val = (-counter) & GENMASK(63, 0); + } else { + val = (-counter) & GENMASK(31, 0); + } + + return val; +} + /** * When the perf event overflows, set the overflow status and inform the vcpu. */ @@ -442,10 +465,7 @@ static void kvm_pmu_perf_overflow(struct perf_event *perf_event, * Reset the sample period to the architectural limit, * i.e. the point where the counter overflows. */ - period = -(local64_read(&perf_event->count)); - - if (!kvm_pmu_idx_is_64bit(vcpu, pmc->idx)) - period &= GENMASK(31, 0); + period = compute_period(vcpu, idx, local64_read(&perf_event->count)); local64_set(&perf_event->hw.period_left, 0); perf_event->attr.sample_period = period; @@ -571,14 +591,13 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx) /* * If counting with a 64bit counter, advertise it to the perf - * code, carefully dealing with the initial sample period. + * code, carefully dealing with the initial sample period + * which also depends on the overflow. */ - if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) { + if (kvm_pmu_idx_is_64bit(vcpu, select_idx)) attr.config1 |= PERF_ATTR_CFG1_COUNTER_64BIT; - attr.sample_period = (-counter) & GENMASK(63, 0); - } else { - attr.sample_period = (-counter) & GENMASK(31, 0); - } + + attr.sample_period = compute_period(vcpu, select_idx, counter); event = perf_event_create_kernel_counter(&attr, -1, current, kvm_pmu_perf_overflow, pmc);
The PMU architecture makes a subtle difference between a 64bit counter and a counter that has a 64bit overflow. This is for example the case of the cycle counter, which can generate an overflow on a 32bit boundary if PMCR_EL0.LC==0 despite the accumulation being done on 64 bits. Use this distinction in the few cases where it matters in the code, as we will reuse this with PMUv3p5 long counters. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kvm/pmu-emul.c | 43 ++++++++++++++++++++++++++++----------- 1 file changed, 31 insertions(+), 12 deletions(-)