* [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
* [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 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
* 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