* [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state
@ 2024-11-20 0:52 Oliver Upton
2024-11-20 0:52 ` [PATCH v2 1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status Oliver Upton
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Oliver Upton @ 2024-11-20 0:52 UTC (permalink / raw)
To: kvmarm
Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Zenghui Yu,
Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta, Oliver Upton
Respin of Raghavendra's fix for evaluating PMU overflow [*], with
another patch stuck on top for my own blatant disregard of the
architecture in the vEL2 bits.
Since I fiddled with this on the M2 these patches are obviously untested
due to the lack of PMUv3. I'll give it a spin on supporting hardware
before actually applying patches, of course.
[*]: https://lore.kernel.org/kvmarm/20241119205841.268247-1-rananta@google.com/
Oliver Upton (1):
KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters
Raghavendra Rao Ananta (1):
KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status
arch/arm64/kvm/pmu-emul.c | 59 ++++++++++++++++++++++++++++-----------
1 file changed, 43 insertions(+), 16 deletions(-)
base-commit: 60ad25e14ab5a4e56c8bf7f7d6846eacb9cd53df
--
2.39.5
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH v2 1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status 2024-11-20 0:52 [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state Oliver Upton @ 2024-11-20 0:52 ` Oliver Upton 2024-11-20 8:18 ` Marc Zyngier 2024-11-20 0:52 ` [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters Oliver Upton 2024-11-21 8:18 ` [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state Oliver Upton 2 siblings, 1 reply; 9+ messages in thread From: Oliver Upton @ 2024-11-20 0:52 UTC (permalink / raw) To: kvmarm Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta, stable, Oliver Upton From: Raghavendra Rao Ananta <rananta@google.com> DDI0487K D13.1.1 describes the PMU overflow condition, which evaluates to true if any counter's global enable (PMCR_EL0.E), overflow flag (PMOVSSET_EL0[n]), and interrupt enable (PMINTENSET_EL1[n]) are all 1. Of note, this does not require a counter to be enabled (i.e. PMCNTENSET_EL0[n] = 1) to generate an overflow. Align kvm_pmu_overflow_status() with the reality of the architecture and stop using PMCNTENSET_EL0 as part of the overflow condition. The bug was discovered while running an SBSA PMU test [*], which only sets PMCR.E, PMOVSSET<0>, PMINTENSET<0>, and expects an overflow interrupt. Cc: stable@vger.kernel.org Fixes: 76d883c4e640 ("arm64: KVM: Add access handler for PMOVSSET and PMOVSCLR register") Link: https://github.com/ARM-software/sbsa-acs/blob/master/test_pool/pmu/operating_system/test_pmu001.c Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> [ oliver: massaged changelog ] Signed-off-by: Oliver Upton <oliver.upton@linux.dev> --- arch/arm64/kvm/pmu-emul.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index 8ad62284fa23..3855cc9d0ca5 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -381,7 +381,6 @@ static u64 kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) if ((kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) { reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); - reg &= __vcpu_sys_reg(vcpu, PMCNTENSET_EL0); reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); } -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status 2024-11-20 0:52 ` [PATCH v2 1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status Oliver Upton @ 2024-11-20 8:18 ` Marc Zyngier 0 siblings, 0 replies; 9+ messages in thread From: Marc Zyngier @ 2024-11-20 8:18 UTC (permalink / raw) To: Oliver Upton Cc: kvmarm, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta, stable On Wed, 20 Nov 2024 00:52:29 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > From: Raghavendra Rao Ananta <rananta@google.com> > > DDI0487K D13.1.1 describes the PMU overflow condition, which evaluates nit: DDI0487K.a, and D13.3.1. > to true if any counter's global enable (PMCR_EL0.E), overflow flag > (PMOVSSET_EL0[n]), and interrupt enable (PMINTENSET_EL1[n]) are all 1. > Of note, this does not require a counter to be enabled > (i.e. PMCNTENSET_EL0[n] = 1) to generate an overflow. > > Align kvm_pmu_overflow_status() with the reality of the architecture > and stop using PMCNTENSET_EL0 as part of the overflow condition. The > bug was discovered while running an SBSA PMU test [*], which only sets > PMCR.E, PMOVSSET<0>, PMINTENSET<0>, and expects an overflow interrupt. > > Cc: stable@vger.kernel.org > Fixes: 76d883c4e640 ("arm64: KVM: Add access handler for PMOVSSET and PMOVSCLR register") > Link: https://github.com/ARM-software/sbsa-acs/blob/master/test_pool/pmu/operating_system/test_pmu001.c > Signed-off-by: Raghavendra Rao Ananta <rananta@google.com> > [ oliver: massaged changelog ] > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > --- > arch/arm64/kvm/pmu-emul.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > index 8ad62284fa23..3855cc9d0ca5 100644 > --- a/arch/arm64/kvm/pmu-emul.c > +++ b/arch/arm64/kvm/pmu-emul.c > @@ -381,7 +381,6 @@ static u64 kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) > > if ((kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) { > reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); > - reg &= __vcpu_sys_reg(vcpu, PMCNTENSET_EL0); > reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); > } > Reviewed-by: Marc Zyngier <maz@kernel.org> M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters 2024-11-20 0:52 [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state Oliver Upton 2024-11-20 0:52 ` [PATCH v2 1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status Oliver Upton @ 2024-11-20 0:52 ` Oliver Upton 2024-11-20 1:00 ` Oliver Upton 2024-11-20 8:38 ` Marc Zyngier 2024-11-21 8:18 ` [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state Oliver Upton 2 siblings, 2 replies; 9+ messages in thread From: Oliver Upton @ 2024-11-20 0:52 UTC (permalink / raw) To: kvmarm Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta, Oliver Upton The 'global enable control' (as it is termed in the architecture) for counters reserved by EL2 is MDCR_EL2.HPME. Use that instead of PMCR_EL0.E when evaluating the overflow state for hyp counters. Change the return value to a bool while at it, which better reflects the fact that the overflow state is a shared signal and not a per-counter property. Signed-off-by: Oliver Upton <oliver.upton@linux.dev> --- arch/arm64/kvm/pmu-emul.c | 58 +++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 15 deletions(-) diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index 3855cc9d0ca5..fb79fa689ae5 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -274,12 +274,21 @@ void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu) irq_work_sync(&vcpu->arch.pmu.overflow_work); } -bool kvm_pmu_counter_is_hyp(struct kvm_vcpu *vcpu, unsigned int idx) +static u64 kvm_pmu_hyp_counter_mask(struct kvm_vcpu *vcpu) { - unsigned int hpmn; + if (!vcpu_has_nv(vcpu)) + return 0; - if (!vcpu_has_nv(vcpu) || idx == ARMV8_PMU_CYCLE_IDX) - return false; + hpmn = SYS_FIELD_GET(MDCR_EL2, HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2)); + n = vcpu->kvm->arch.pmcr_n; + + /* + * Programming HPMN to a value greater than PMCR_EL0.N is + * CONSTRAINED UNPREDICTABLE. Make the implementation choice that an + * UNKNOWN number of counters (in our case, zero) are reserved for EL2. + */ + if (hpmn >= n) + return 0; /* * Programming HPMN=0 is CONSTRAINED UNPREDICTABLE if FEAT_HPMN0 isn't @@ -288,8 +297,12 @@ bool kvm_pmu_counter_is_hyp(struct kvm_vcpu *vcpu, unsigned int idx) * implementation choice that all counters are included in the second * range reserved for EL2/EL3. */ - hpmn = SYS_FIELD_GET(MDCR_EL2, HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2)); - return idx >= hpmn; + return GENMASK(n - 1, hpmn); +} + +bool kvm_pmu_counter_is_hyp(struct kvm_vcpu *vcpu, unsigned int idx) +{ + return kvm_pmu_hyp_counter_mask(vcpu) & BIT(idx); } u64 kvm_pmu_accessible_counter_mask(struct kvm_vcpu *vcpu) @@ -300,8 +313,7 @@ u64 kvm_pmu_accessible_counter_mask(struct kvm_vcpu *vcpu) if (!vcpu_has_nv(vcpu) || vcpu_is_el2(vcpu)) return mask; - hpmn = SYS_FIELD_GET(MDCR_EL2, HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2)); - return mask & ~GENMASK(vcpu->kvm->arch.pmcr_n - 1, hpmn); + return mask & ~kvm_pmu_hyp_counter_mask(vcpu); } u64 kvm_pmu_implemented_counter_mask(struct kvm_vcpu *vcpu) @@ -375,14 +387,30 @@ void kvm_pmu_disable_counter_mask(struct kvm_vcpu *vcpu, u64 val) } } -static u64 kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) +/* + * Returns the PMU overflow state, which is true if there exists an event + * counter where the values of the global enable control, PMOVSSET_EL0[n], and + * PMINTENSET_EL1[n] are all 1. + */ +static bool kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) { - u64 reg = 0; + u64 reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); - if ((kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) { - reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); - reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); - } + reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); + + /* + * PMCR_EL0.E is the global enable control for event counters available + * to EL0 and EL1. + */ + if (!(kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) + reg &= kvm_pmu_hyp_counter_mask(vcpu); + + /* + * Otherwise, MDCR_EL2.HPME is the global enable control for event + * counters reserved for EL2. + */ + if (!(vcpu_read_sys_reg(vcpu, MDCR_EL2) & MDCR_EL2_HPME)) + reg &= ~kvm_pmu_hyp_counter_mask(vcpu); return reg; } @@ -395,7 +423,7 @@ static void kvm_pmu_update_state(struct kvm_vcpu *vcpu) if (!kvm_vcpu_has_pmu(vcpu)) return; - overflow = !!kvm_pmu_overflow_status(vcpu); + overflow = kvm_pmu_overflow_status(vcpu); if (pmu->irq_level == overflow) return; -- 2.39.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters 2024-11-20 0:52 ` [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters Oliver Upton @ 2024-11-20 1:00 ` Oliver Upton 2024-11-20 8:38 ` Marc Zyngier 1 sibling, 0 replies; 9+ messages in thread From: Oliver Upton @ 2024-11-20 1:00 UTC (permalink / raw) To: kvmarm Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta On Tue, Nov 19, 2024 at 04:52:30PM -0800, Oliver Upton wrote: > The 'global enable control' (as it is termed in the architecture) for > counters reserved by EL2 is MDCR_EL2.HPME. Use that instead of > PMCR_EL0.E when evaluating the overflow state for hyp counters. > > Change the return value to a bool while at it, which better reflects the > fact that the overflow state is a shared signal and not a per-counter > property. ... and the untested garbage award goes to: > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c index fb79fa689ae5..456102bc0b55 100644 --- a/arch/arm64/kvm/pmu-emul.c +++ b/arch/arm64/kvm/pmu-emul.c @@ -276,6 +276,8 @@ void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu) static u64 kvm_pmu_hyp_counter_mask(struct kvm_vcpu *vcpu) { + unsigned int hpmn, n; + if (!vcpu_has_nv(vcpu)) return 0; @@ -308,7 +310,6 @@ bool kvm_pmu_counter_is_hyp(struct kvm_vcpu *vcpu, unsigned int idx) u64 kvm_pmu_accessible_counter_mask(struct kvm_vcpu *vcpu) { u64 mask = kvm_pmu_implemented_counter_mask(vcpu); - u64 hpmn; if (!vcpu_has_nv(vcpu) || vcpu_is_el2(vcpu)) return mask; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters 2024-11-20 0:52 ` [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters Oliver Upton 2024-11-20 1:00 ` Oliver Upton @ 2024-11-20 8:38 ` Marc Zyngier 2024-11-20 8:54 ` Oliver Upton 1 sibling, 1 reply; 9+ messages in thread From: Marc Zyngier @ 2024-11-20 8:38 UTC (permalink / raw) To: Oliver Upton Cc: kvmarm, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta On Wed, 20 Nov 2024 00:52:30 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > The 'global enable control' (as it is termed in the architecture) for > counters reserved by EL2 is MDCR_EL2.HPME. Use that instead of > PMCR_EL0.E when evaluating the overflow state for hyp counters. > > Change the return value to a bool while at it, which better reflects the > fact that the overflow state is a shared signal and not a per-counter > property. > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > --- > arch/arm64/kvm/pmu-emul.c | 58 +++++++++++++++++++++++++++++---------- > 1 file changed, 43 insertions(+), 15 deletions(-) > > diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c > index 3855cc9d0ca5..fb79fa689ae5 100644 > --- a/arch/arm64/kvm/pmu-emul.c > +++ b/arch/arm64/kvm/pmu-emul.c > @@ -274,12 +274,21 @@ void kvm_pmu_vcpu_destroy(struct kvm_vcpu *vcpu) > irq_work_sync(&vcpu->arch.pmu.overflow_work); > } > > -bool kvm_pmu_counter_is_hyp(struct kvm_vcpu *vcpu, unsigned int idx) > +static u64 kvm_pmu_hyp_counter_mask(struct kvm_vcpu *vcpu) > { > - unsigned int hpmn; > + if (!vcpu_has_nv(vcpu)) > + return 0; > > - if (!vcpu_has_nv(vcpu) || idx == ARMV8_PMU_CYCLE_IDX) > - return false; > + hpmn = SYS_FIELD_GET(MDCR_EL2, HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2)); > + n = vcpu->kvm->arch.pmcr_n; > + > + /* > + * Programming HPMN to a value greater than PMCR_EL0.N is > + * CONSTRAINED UNPREDICTABLE. Make the implementation choice that an > + * UNKNOWN number of counters (in our case, zero) are reserved for EL2. > + */ > + if (hpmn >= n) > + return 0; > > /* > * Programming HPMN=0 is CONSTRAINED UNPREDICTABLE if FEAT_HPMN0 isn't > @@ -288,8 +297,12 @@ bool kvm_pmu_counter_is_hyp(struct kvm_vcpu *vcpu, unsigned int idx) > * implementation choice that all counters are included in the second > * range reserved for EL2/EL3. > */ > - hpmn = SYS_FIELD_GET(MDCR_EL2, HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2)); > - return idx >= hpmn; > + return GENMASK(n - 1, hpmn); This explicitly excludes the cycle counter (bit 31), and lead to some unexpected results below. > +} > + > +bool kvm_pmu_counter_is_hyp(struct kvm_vcpu *vcpu, unsigned int idx) > +{ > + return kvm_pmu_hyp_counter_mask(vcpu) & BIT(idx); > } > > u64 kvm_pmu_accessible_counter_mask(struct kvm_vcpu *vcpu) > @@ -300,8 +313,7 @@ u64 kvm_pmu_accessible_counter_mask(struct kvm_vcpu *vcpu) > if (!vcpu_has_nv(vcpu) || vcpu_is_el2(vcpu)) > return mask; > > - hpmn = SYS_FIELD_GET(MDCR_EL2, HPMN, __vcpu_sys_reg(vcpu, MDCR_EL2)); > - return mask & ~GENMASK(vcpu->kvm->arch.pmcr_n - 1, hpmn); > + return mask & ~kvm_pmu_hyp_counter_mask(vcpu); > } > > u64 kvm_pmu_implemented_counter_mask(struct kvm_vcpu *vcpu) > @@ -375,14 +387,30 @@ void kvm_pmu_disable_counter_mask(struct kvm_vcpu *vcpu, u64 val) > } > } > > -static u64 kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) > +/* > + * Returns the PMU overflow state, which is true if there exists an event > + * counter where the values of the global enable control, PMOVSSET_EL0[n], and > + * PMINTENSET_EL1[n] are all 1. > + */ > +static bool kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) > { > - u64 reg = 0; > + u64 reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); > > - if ((kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) { > - reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); > - reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); > - } > + reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); > + > + /* > + * PMCR_EL0.E is the global enable control for event counters available > + * to EL0 and EL1. > + */ > + if (!(kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) > + reg &= kvm_pmu_hyp_counter_mask(vcpu); So if the PMU is disabled at EL1, we remove the cycle counter from the list of counters that are considered for an overflow. I don't think that's what you really want. Thanks, M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters 2024-11-20 8:38 ` Marc Zyngier @ 2024-11-20 8:54 ` Oliver Upton 2024-11-20 9:55 ` Marc Zyngier 0 siblings, 1 reply; 9+ messages in thread From: Oliver Upton @ 2024-11-20 8:54 UTC (permalink / raw) To: Marc Zyngier Cc: kvmarm, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta On Wed, Nov 20, 2024 at 08:38:22AM +0000, Marc Zyngier wrote: > On Wed, 20 Nov 2024 00:52:30 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > +/* > > + * Returns the PMU overflow state, which is true if there exists an event > > + * counter where the values of the global enable control, PMOVSSET_EL0[n], and > > + * PMINTENSET_EL1[n] are all 1. > > + */ > > +static bool kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) > > { > > - u64 reg = 0; > > + u64 reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); > > > > - if ((kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) { > > - reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); > > - reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); > > - } > > + reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); > > + > > + /* > > + * PMCR_EL0.E is the global enable control for event counters available > > + * to EL0 and EL1. > > + */ > > + if (!(kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) > > + reg &= kvm_pmu_hyp_counter_mask(vcpu); > > So if the PMU is disabled at EL1, we remove the cycle counter from the > list of counters that are considered for an overflow. > > I don't think that's what you really want. That's how it is written though, no? My understanding is that the fixed counters are never reserved by EL2: """ The PE signals a request for: [...] - The cycle counter, when the values of PMCR_EL0.E, PMOVSSET[31] and PMINTENSET[31] are all 1 """ PMUOverflowCondition() and PMUCounterIsHyp() seem to match this too. Unless I can't read, which happens a lot. -- Thanks, Oliver ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters 2024-11-20 8:54 ` Oliver Upton @ 2024-11-20 9:55 ` Marc Zyngier 0 siblings, 0 replies; 9+ messages in thread From: Marc Zyngier @ 2024-11-20 9:55 UTC (permalink / raw) To: Oliver Upton Cc: kvmarm, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta On Wed, 20 Nov 2024 08:54:45 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > On Wed, Nov 20, 2024 at 08:38:22AM +0000, Marc Zyngier wrote: > > On Wed, 20 Nov 2024 00:52:30 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > > +/* > > > + * Returns the PMU overflow state, which is true if there exists an event > > > + * counter where the values of the global enable control, PMOVSSET_EL0[n], and > > > + * PMINTENSET_EL1[n] are all 1. > > > + */ > > > +static bool kvm_pmu_overflow_status(struct kvm_vcpu *vcpu) > > > { > > > - u64 reg = 0; > > > + u64 reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); > > > > > > - if ((kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) { > > > - reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0); > > > - reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); > > > - } > > > + reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1); > > > + > > > + /* > > > + * PMCR_EL0.E is the global enable control for event counters available > > > + * to EL0 and EL1. > > > + */ > > > + if (!(kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)) > > > + reg &= kvm_pmu_hyp_counter_mask(vcpu); > > > > So if the PMU is disabled at EL1, we remove the cycle counter from the > > list of counters that are considered for an overflow. > > > > I don't think that's what you really want. > > That's how it is written though, no? My understanding is that the fixed > counters are never reserved by EL2: > > """ > The PE signals a request for: > > [...] > > - The cycle counter, when the values of PMCR_EL0.E, PMOVSSET[31] and > PMINTENSET[31] are all 1 > """ Ah, fair enough. I had the misguided impression that there would be another control for that, but this is actually called out in D13.11.2 ("The PMU does not provide any control that a hypervisor can use to reserve the cycle counter for its own use."). I stand corrected. > > PMUOverflowCondition() and PMUCounterIsHyp() seem to match this too. > Unless I can't read, which happens a lot. Nah, that's definitely me. FWIW, and with the follow-up fixes: Reviewed-by: Marc Zyngier <maz@kernel.org> M. -- Without deviation from the norm, progress is not possible. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state 2024-11-20 0:52 [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state Oliver Upton 2024-11-20 0:52 ` [PATCH v2 1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status Oliver Upton 2024-11-20 0:52 ` [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters Oliver Upton @ 2024-11-21 8:18 ` Oliver Upton 2 siblings, 0 replies; 9+ messages in thread From: Oliver Upton @ 2024-11-21 8:18 UTC (permalink / raw) To: kvmarm, Oliver Upton Cc: Marc Zyngier, Joey Gouly, Suzuki K Poulose, Zenghui Yu, Mingwei Zhang, Colton Lewis, Raghavendra Rao Ananta On Tue, 19 Nov 2024 16:52:28 -0800, Oliver Upton wrote: > Respin of Raghavendra's fix for evaluating PMU overflow [*], with > another patch stuck on top for my own blatant disregard of the > architecture in the vEL2 bits. > > Since I fiddled with this on the M2 these patches are obviously untested > due to the lack of PMUv3. I'll give it a spin on supporting hardware > before actually applying patches, of course. > > [...] Applied to fixes, thanks! [1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status https://git.kernel.org/kvmarm/kvmarm/c/54bbee190d42 [2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters https://git.kernel.org/kvmarm/kvmarm/c/13905f4547b0 -- Best, Oliver ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-11-21 8:18 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-11-20 0:52 [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state Oliver Upton 2024-11-20 0:52 ` [PATCH v2 1/2] KVM: arm64: Ignore PMCNTENSET_EL0 while checking for overflow status Oliver Upton 2024-11-20 8:18 ` Marc Zyngier 2024-11-20 0:52 ` [PATCH v2 2/2] KVM: arm64: Use MDCR_EL2.HPME to evaluate overflow of hyp counters Oliver Upton 2024-11-20 1:00 ` Oliver Upton 2024-11-20 8:38 ` Marc Zyngier 2024-11-20 8:54 ` Oliver Upton 2024-11-20 9:55 ` Marc Zyngier 2024-11-21 8:18 ` [PATCH v2 0/2] KVM: arm64: Fixes for PMU overflow state Oliver Upton
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox