* [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support
@ 2024-08-24 0:13 Oliver Upton
2024-08-24 0:14 ` [PATCH 1/3] KVM: arm64: Add helpers to determine if PMC counts at a given EL Oliver Upton
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Oliver Upton @ 2024-08-24 0:13 UTC (permalink / raw)
To: kvmarm
Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni, Oliver Upton
We allow a nested VM to be configured with a vPMU, although it isn't
entirely functional. We do not currently respect the EL2 event filter
configuration from the guest hypervisor, which really needs to be
applied to EL1 when in a hyp context.
Series to do just that. Tested on Neoverse-V2 which fortunately
implements PMUv3 and NV, unlike the fruity stuff I'm typically using...
Oliver Upton (3):
KVM: arm64: Add helpers to determine if PMC counts at a given EL
KVM: arm64: nv: Honor NSH filter when in hyp context
KVM: arm64: nv: Reprogram PMU events affected by nested transition
arch/arm64/kvm/emulate-nested.c | 4 ++
arch/arm64/kvm/pmu-emul.c | 77 ++++++++++++++++++++++++++++-----
include/kvm/arm_pmu.h | 3 ++
3 files changed, 72 insertions(+), 12 deletions(-)
base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
--
2.46.0.295.g3b9ea8a38a-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] KVM: arm64: Add helpers to determine if PMC counts at a given EL
2024-08-24 0:13 [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Oliver Upton
@ 2024-08-24 0:14 ` Oliver Upton
2024-08-24 0:14 ` [PATCH 2/3] KVM: arm64: nv: Honor NSH filter when in hyp context Oliver Upton
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Oliver Upton @ 2024-08-24 0:14 UTC (permalink / raw)
To: kvmarm
Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni, Oliver Upton
Checking the exception level filters for a PMC is a minor annoyance to
open code. Add helpers to check if an event counts at EL0 and EL1, which
will prove useful in a subsequent change.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/pmu-emul.c | 40 +++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index 82a2a003259c..c564bd600326 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -111,6 +111,11 @@ static u32 counter_index_to_evtreg(u64 idx)
return (idx == ARMV8_PMU_CYCLE_IDX) ? PMCCFILTR_EL0 : PMEVTYPER0_EL0 + idx;
}
+static u64 kvm_pmc_read_evtreg(const struct kvm_pmc *pmc)
+{
+ return __vcpu_sys_reg(kvm_pmc_to_vcpu(pmc), counter_index_to_evtreg(pmc->idx));
+}
+
static u64 kvm_pmu_get_pmc_value(struct kvm_pmc *pmc)
{
struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
@@ -589,6 +594,24 @@ static bool kvm_pmu_counter_is_enabled(struct kvm_pmc *pmc)
(__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) & BIT(pmc->idx));
}
+static bool kvm_pmc_counts_at_el0(struct kvm_pmc *pmc)
+{
+ u64 evtreg = kvm_pmc_read_evtreg(pmc);
+ bool nsu = evtreg & ARMV8_PMU_EXCLUDE_NS_EL0;
+ bool u = evtreg & ARMV8_PMU_EXCLUDE_EL0;
+
+ return u == nsu;
+}
+
+static bool kvm_pmc_counts_at_el1(struct kvm_pmc *pmc)
+{
+ u64 evtreg = kvm_pmc_read_evtreg(pmc);
+ bool nsk = evtreg & ARMV8_PMU_EXCLUDE_NS_EL1;
+ bool p = evtreg & ARMV8_PMU_EXCLUDE_EL1;
+
+ return p == nsk;
+}
+
/**
* kvm_pmu_create_perf_event - create a perf event for a counter
* @pmc: Counter context
@@ -599,17 +622,15 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
struct arm_pmu *arm_pmu = vcpu->kvm->arch.arm_pmu;
struct perf_event *event;
struct perf_event_attr attr;
- u64 eventsel, reg, data;
- bool p, u, nsk, nsu;
+ u64 eventsel, evtreg;
- reg = counter_index_to_evtreg(pmc->idx);
- data = __vcpu_sys_reg(vcpu, reg);
+ evtreg = kvm_pmc_read_evtreg(pmc);
kvm_pmu_stop_counter(pmc);
if (pmc->idx == ARMV8_PMU_CYCLE_IDX)
eventsel = ARMV8_PMUV3_PERFCTR_CPU_CYCLES;
else
- eventsel = data & kvm_pmu_event_mask(vcpu->kvm);
+ eventsel = evtreg & kvm_pmu_event_mask(vcpu->kvm);
/*
* Neither SW increment nor chained events need to be backed
@@ -627,18 +648,13 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
!test_bit(eventsel, vcpu->kvm->arch.pmu_filter))
return;
- p = data & ARMV8_PMU_EXCLUDE_EL1;
- u = data & ARMV8_PMU_EXCLUDE_EL0;
- nsk = data & ARMV8_PMU_EXCLUDE_NS_EL1;
- nsu = data & ARMV8_PMU_EXCLUDE_NS_EL0;
-
memset(&attr, 0, sizeof(struct perf_event_attr));
attr.type = arm_pmu->pmu.type;
attr.size = sizeof(attr);
attr.pinned = 1;
attr.disabled = !kvm_pmu_counter_is_enabled(pmc);
- attr.exclude_user = (u != nsu);
- attr.exclude_kernel = (p != nsk);
+ attr.exclude_user = !kvm_pmc_counts_at_el0(pmc);
+ attr.exclude_kernel = !kvm_pmc_counts_at_el1(pmc);
attr.exclude_hv = 1; /* Don't count EL2 events */
attr.exclude_host = 1; /* Don't count host events */
attr.config = eventsel;
--
2.46.0.295.g3b9ea8a38a-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] KVM: arm64: nv: Honor NSH filter when in hyp context
2024-08-24 0:13 [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Oliver Upton
2024-08-24 0:14 ` [PATCH 1/3] KVM: arm64: Add helpers to determine if PMC counts at a given EL Oliver Upton
@ 2024-08-24 0:14 ` Oliver Upton
2024-08-24 0:14 ` [PATCH 3/3] KVM: arm64: nv: Reprogram PMU events affected by nested transition Oliver Upton
2024-08-25 8:16 ` [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Marc Zyngier
3 siblings, 0 replies; 9+ messages in thread
From: Oliver Upton @ 2024-08-24 0:14 UTC (permalink / raw)
To: kvmarm
Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni, Oliver Upton
It hopefully comes as no surprise when I say that vEL2 actually runs at
EL1. So, the guest hypervisor's EL2 event filter (NSH) needs to actually
be applied to EL1 in the perf event.
This isn't quite enough yet, as the backing perf events need to be
reprogrammed upon nested ERET/exception entry to remap the effective
filter onto ::exclude_kernel.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/pmu-emul.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index c564bd600326..edc543574c5e 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -612,6 +612,11 @@ static bool kvm_pmc_counts_at_el1(struct kvm_pmc *pmc)
return p == nsk;
}
+static bool kvm_pmc_counts_at_el2(struct kvm_pmc *pmc)
+{
+ return kvm_pmc_read_evtreg(pmc) & ARMV8_PMU_INCLUDE_EL2;
+}
+
/**
* kvm_pmu_create_perf_event - create a perf event for a counter
* @pmc: Counter context
@@ -654,11 +659,19 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
attr.pinned = 1;
attr.disabled = !kvm_pmu_counter_is_enabled(pmc);
attr.exclude_user = !kvm_pmc_counts_at_el0(pmc);
- attr.exclude_kernel = !kvm_pmc_counts_at_el1(pmc);
attr.exclude_hv = 1; /* Don't count EL2 events */
attr.exclude_host = 1; /* Don't count host events */
attr.config = eventsel;
+ /*
+ * Filter events at EL1 (i.e. vEL2) when in a hyp context based on the
+ * guest's EL2 filter.
+ */
+ if (unlikely(is_hyp_ctxt(vcpu)))
+ attr.exclude_kernel = !kvm_pmc_counts_at_el2(pmc);
+ else
+ attr.exclude_kernel = !kvm_pmc_counts_at_el1(pmc);
+
/*
* If counting with a 64bit counter, advertise it to the perf
* code, carefully dealing with the initial sample period
--
2.46.0.295.g3b9ea8a38a-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] KVM: arm64: nv: Reprogram PMU events affected by nested transition
2024-08-24 0:13 [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Oliver Upton
2024-08-24 0:14 ` [PATCH 1/3] KVM: arm64: Add helpers to determine if PMC counts at a given EL Oliver Upton
2024-08-24 0:14 ` [PATCH 2/3] KVM: arm64: nv: Honor NSH filter when in hyp context Oliver Upton
@ 2024-08-24 0:14 ` Oliver Upton
2024-08-25 8:16 ` [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Marc Zyngier
3 siblings, 0 replies; 9+ messages in thread
From: Oliver Upton @ 2024-08-24 0:14 UTC (permalink / raw)
To: kvmarm
Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni, Oliver Upton
Start reprogramming PMU events at nested boundaries now that everything
is in place to handle the EL2 event filter. Only repaint events where
the filter differs between EL1 and EL2 as a slight optimization.
PMU now 'works' for nested VMs, albeit slow.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
arch/arm64/kvm/emulate-nested.c | 4 ++++
arch/arm64/kvm/pmu-emul.c | 24 ++++++++++++++++++++++++
include/kvm/arm_pmu.h | 3 +++
3 files changed, 31 insertions(+)
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 05166eccea0a..fc56dace61af 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -2322,6 +2322,8 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
kvm_arch_vcpu_load(vcpu, smp_processor_id());
preempt_enable();
+
+ kvm_pmu_reprogram_events(vcpu);
}
static void kvm_inject_el2_exception(struct kvm_vcpu *vcpu, u64 esr_el2,
@@ -2404,6 +2406,8 @@ static int kvm_inject_nested(struct kvm_vcpu *vcpu, u64 esr_el2,
kvm_arch_vcpu_load(vcpu, smp_processor_id());
preempt_enable();
+ kvm_pmu_reprogram_events(vcpu);
+
return 1;
}
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index edc543574c5e..028b20c70344 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -1168,3 +1168,27 @@ u64 kvm_vcpu_read_pmcr(struct kvm_vcpu *vcpu)
return u64_replace_bits(pmcr, vcpu->kvm->arch.pmcr_n, ARMV8_PMU_PMCR_N);
}
+
+void kvm_pmu_reprogram_events(struct kvm_vcpu *vcpu)
+{
+ unsigned long mask;
+ int i;
+
+ if (!kvm_vcpu_has_pmu(vcpu))
+ return;
+
+ mask = __vcpu_sys_reg(vcpu, PMCNTENSET_EL0);
+ for_each_set_bit(i, &mask, 32) {
+ struct kvm_pmc *pmc = kvm_vcpu_idx_to_pmc(vcpu, i);
+
+ /*
+ * We only need to reconfigure events where the filter is
+ * different at EL1 vs. EL2, as we're multiplexing the true EL1
+ * event filter bit for nested.
+ */
+ if (kvm_pmc_counts_at_el1(pmc) == kvm_pmc_counts_at_el2(pmc))
+ continue;
+
+ kvm_pmu_create_perf_event(pmc);
+ }
+}
diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
index 35d4ca4f6122..d9f512bf714d 100644
--- a/include/kvm/arm_pmu.h
+++ b/include/kvm/arm_pmu.h
@@ -96,6 +96,7 @@ int kvm_arm_set_default_pmu(struct kvm *kvm);
u8 kvm_arm_pmu_get_max_counters(struct kvm *kvm);
u64 kvm_vcpu_read_pmcr(struct kvm_vcpu *vcpu);
+void kvm_pmu_reprogram_events(struct kvm_vcpu *vcpu);
#else
struct kvm_pmu {
};
@@ -187,6 +188,8 @@ static inline u64 kvm_vcpu_read_pmcr(struct kvm_vcpu *vcpu)
return 0;
}
+static inline void kvm_pmu_reprogram_events(struct kvm_vcpu *vcpu) {}
+
#endif
#endif
--
2.46.0.295.g3b9ea8a38a-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support
2024-08-24 0:13 [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Oliver Upton
` (2 preceding siblings ...)
2024-08-24 0:14 ` [PATCH 3/3] KVM: arm64: nv: Reprogram PMU events affected by nested transition Oliver Upton
@ 2024-08-25 8:16 ` Marc Zyngier
2024-08-26 17:26 ` Oliver Upton
3 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2024-08-25 8:16 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni
On Sat, 24 Aug 2024 01:13:59 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
>
> We allow a nested VM to be configured with a vPMU, although it isn't
> entirely functional. We do not currently respect the EL2 event filter
> configuration from the guest hypervisor, which really needs to be
> applied to EL1 when in a hyp context.
>
> Series to do just that. Tested on Neoverse-V2 which fortunately
> implements PMUv3 and NV, unlike the fruity stuff I'm typically using...
>
> Oliver Upton (3):
> KVM: arm64: Add helpers to determine if PMC counts at a given EL
> KVM: arm64: nv: Honor NSH filter when in hyp context
> KVM: arm64: nv: Reprogram PMU events affected by nested transition
>
> arch/arm64/kvm/emulate-nested.c | 4 ++
> arch/arm64/kvm/pmu-emul.c | 77 ++++++++++++++++++++++++++++-----
> include/kvm/arm_pmu.h | 3 ++
> 3 files changed, 72 insertions(+), 12 deletions(-)
>
>
> base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
I had a quick look, and definitely like the way it is shaping up (it
does the job without fuss, and NV isn't about performance anyway).
However, I think it needs a bit more work so that MDCR_EL2.HPMN is
correctly handled. This shouldn't be hard to do, and if FEAT_FGT is
supported in the guest, then we could even expose FEAT_HPMN0 (though I
haven't worked out yet why FGT is a dependency).
With that, I think we could take it in.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support
2024-08-25 8:16 ` [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Marc Zyngier
@ 2024-08-26 17:26 ` Oliver Upton
2024-08-27 6:35 ` Marc Zyngier
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Upton @ 2024-08-26 17:26 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni
Hey,
On Sun, Aug 25, 2024 at 09:16:33AM +0100, Marc Zyngier wrote:
> On Sat, 24 Aug 2024 01:13:59 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> >
> > We allow a nested VM to be configured with a vPMU, although it isn't
> > entirely functional. We do not currently respect the EL2 event filter
> > configuration from the guest hypervisor, which really needs to be
> > applied to EL1 when in a hyp context.
> >
> > Series to do just that. Tested on Neoverse-V2 which fortunately
> > implements PMUv3 and NV, unlike the fruity stuff I'm typically using...
> >
> > Oliver Upton (3):
> > KVM: arm64: Add helpers to determine if PMC counts at a given EL
> > KVM: arm64: nv: Honor NSH filter when in hyp context
> > KVM: arm64: nv: Reprogram PMU events affected by nested transition
> >
> > arch/arm64/kvm/emulate-nested.c | 4 ++
> > arch/arm64/kvm/pmu-emul.c | 77 ++++++++++++++++++++++++++++-----
> > include/kvm/arm_pmu.h | 3 ++
> > 3 files changed, 72 insertions(+), 12 deletions(-)
> >
> >
> > base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
>
> I had a quick look, and definitely like the way it is shaping up (it
> does the job without fuss, and NV isn't about performance anyway).
>
> However, I think it needs a bit more work so that MDCR_EL2.HPMN is
> correctly handled. This shouldn't be hard to do, and if FEAT_FGT is
> supported in the guest, then we could even expose FEAT_HPMN0 (though I
> haven't worked out yet why FGT is a dependency).
Right, I was hoping to spoon-feed some more PMU patches going forward,
since we already allow the NV+PMU combination and can treat it as a 'bugfix'.
But I'm happy to throw some more cycles at the problem.
I think there are some gaps in the way the other coarse-grained PMU
traps are handled (TPM, TPMCR), since it looks like they also apply to
Host EL0 based on the pseudocode. I don't want to have a separate
mechanism for describing these 'InHost' traps, so I'm gonna try and
address this like so:
- Indicate if CGTs apply to host EL0 in the trap_bits descriptor
- Steal a trap_config bit to indicate if a sysreg can trap while
InHost, and:
- Mark a trap_config as InHost if any of the associated CGTs are
InHost when inserting in the xarray
That way I can preserve the early return for is_hyp_ctxt() for all but a
few trap configs.
Thoughts?
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support
2024-08-26 17:26 ` Oliver Upton
@ 2024-08-27 6:35 ` Marc Zyngier
2024-08-27 7:01 ` Oliver Upton
0 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2024-08-27 6:35 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni
On Mon, 26 Aug 2024 18:26:36 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
>
> Hey,
>
> On Sun, Aug 25, 2024 at 09:16:33AM +0100, Marc Zyngier wrote:
> > On Sat, 24 Aug 2024 01:13:59 +0100,
> > Oliver Upton <oliver.upton@linux.dev> wrote:
> > >
> > > We allow a nested VM to be configured with a vPMU, although it isn't
> > > entirely functional. We do not currently respect the EL2 event filter
> > > configuration from the guest hypervisor, which really needs to be
> > > applied to EL1 when in a hyp context.
> > >
> > > Series to do just that. Tested on Neoverse-V2 which fortunately
> > > implements PMUv3 and NV, unlike the fruity stuff I'm typically using...
> > >
> > > Oliver Upton (3):
> > > KVM: arm64: Add helpers to determine if PMC counts at a given EL
> > > KVM: arm64: nv: Honor NSH filter when in hyp context
> > > KVM: arm64: nv: Reprogram PMU events affected by nested transition
> > >
> > > arch/arm64/kvm/emulate-nested.c | 4 ++
> > > arch/arm64/kvm/pmu-emul.c | 77 ++++++++++++++++++++++++++++-----
> > > include/kvm/arm_pmu.h | 3 ++
> > > 3 files changed, 72 insertions(+), 12 deletions(-)
> > >
> > >
> > > base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
> >
> > I had a quick look, and definitely like the way it is shaping up (it
> > does the job without fuss, and NV isn't about performance anyway).
> >
> > However, I think it needs a bit more work so that MDCR_EL2.HPMN is
> > correctly handled. This shouldn't be hard to do, and if FEAT_FGT is
> > supported in the guest, then we could even expose FEAT_HPMN0 (though I
> > haven't worked out yet why FGT is a dependency).
>
> Right, I was hoping to spoon-feed some more PMU patches going forward,
> since we already allow the NV+PMU combination and can treat it as a 'bugfix'.
> But I'm happy to throw some more cycles at the problem.
We absolutely can. As long as we don't let userspace get in the way,
we can play the incremental game for as long as we want.
>
> I think there are some gaps in the way the other coarse-grained PMU
> traps are handled (TPM, TPMCR), since it looks like they also apply to
> Host EL0 based on the pseudocode. I don't want to have a separate
> mechanism for describing these 'InHost' traps, so I'm gonna try and
> address this like so:
>
> - Indicate if CGTs apply to host EL0 in the trap_bits descriptor
>
> - Steal a trap_config bit to indicate if a sysreg can trap while
> InHost, and:
>
> - Mark a trap_config as InHost if any of the associated CGTs are
> InHost when inserting in the xarray
>
> That way I can preserve the early return for is_hyp_ctxt() for all but a
> few trap configs.
>
> Thoughts?
I see that you have already posted your proposed approach, but allow
me to say what I have in mind.
We have three possibilities:
- either we go with your approach, which has the advantage of not
requiring any new trap description, but breaks the (unwritten)
promise that we don't do any "local" handling at this stage
- or we perform the handling where we normally do it (in sys_regs.c),
but we start littering the already overly complicated emulation code
for something that is barely a trap reinjection
- or (and this is my preferred option), we treat it as a "fast" trap,
which would match what we do for other things that we trap and that
behave differently when 'InHost' (ERET, TLBIs).
This last option does require another bit of decoding, but has the
following advantages:
- it follows the existing model for 'InHost' trap handling
- it is close to optimal from a performance perspective
- it doesn't change the existing behaviour of the xarray handling
The way I think of it, this sort of EL0->EL2 trap reinjection is
simply the other side of the ERET coin, and I would very much like to
keep this symmetry, unless I have missed something obvious.
What do you think?
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support
2024-08-27 6:35 ` Marc Zyngier
@ 2024-08-27 7:01 ` Oliver Upton
2024-08-27 7:59 ` Marc Zyngier
0 siblings, 1 reply; 9+ messages in thread
From: Oliver Upton @ 2024-08-27 7:01 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni
On Tue, Aug 27, 2024 at 07:35:15AM +0100, Marc Zyngier wrote:
> > Thoughts?
>
> I see that you have already posted your proposed approach, but allow
> me to say what I have in mind.
>
> We have three possibilities:
>
> - either we go with your approach, which has the advantage of not
> requiring any new trap description, but breaks the (unwritten)
> promise that we don't do any "local" handling at this stage
>
> - or we perform the handling where we normally do it (in sys_regs.c),
> but we start littering the already overly complicated emulation code
> for something that is barely a trap reinjection
Hell no :)
> - or (and this is my preferred option), we treat it as a "fast" trap,
> which would match what we do for other things that we trap and that
> behave differently when 'InHost' (ERET, TLBIs).
>
> This last option does require another bit of decoding, but has the
> following advantages:
>
> - it follows the existing model for 'InHost' trap handling
>
> - it is close to optimal from a performance perspective
>
> - it doesn't change the existing behaviour of the xarray handling
>
> The way I think of it, this sort of EL0->EL2 trap reinjection is
> simply the other side of the ERET coin, and I would very much like to
> keep this symmetry, unless I have missed something obvious.
>
> What do you think?
My primary concern about stuffing more things into the fast path is the
limited debuggability since we can't use instrumentation in a
not-quite-kernel context.
I do generally agree with the intent of organizing it all similarly,
just that the traps infrastructure is _really_ handy for doing the job.
Let me take an accounting of how many other 'InHost' trap bits we have
and get a feel for if we need a generalized solution or if I can do
something PMU-specific here.
--
Thanks,
Oliver
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support
2024-08-27 7:01 ` Oliver Upton
@ 2024-08-27 7:59 ` Marc Zyngier
0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2024-08-27 7:59 UTC (permalink / raw)
To: Oliver Upton
Cc: kvmarm, James Morse, Suzuki K Poulose, Zenghui Yu,
Ganapatrao Kulkarni
On Tue, 27 Aug 2024 08:01:11 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Tue, Aug 27, 2024 at 07:35:15AM +0100, Marc Zyngier wrote:
> > > Thoughts?
> >
> > I see that you have already posted your proposed approach, but allow
> > me to say what I have in mind.
> >
> > We have three possibilities:
> >
> > - either we go with your approach, which has the advantage of not
> > requiring any new trap description, but breaks the (unwritten)
> > promise that we don't do any "local" handling at this stage
> >
> > - or we perform the handling where we normally do it (in sys_regs.c),
> > but we start littering the already overly complicated emulation code
> > for something that is barely a trap reinjection
>
> Hell no :)
>
> > - or (and this is my preferred option), we treat it as a "fast" trap,
> > which would match what we do for other things that we trap and that
> > behave differently when 'InHost' (ERET, TLBIs).
> >
> > This last option does require another bit of decoding, but has the
> > following advantages:
> >
> > - it follows the existing model for 'InHost' trap handling
> >
> > - it is close to optimal from a performance perspective
> >
> > - it doesn't change the existing behaviour of the xarray handling
> >
> > The way I think of it, this sort of EL0->EL2 trap reinjection is
> > simply the other side of the ERET coin, and I would very much like to
> > keep this symmetry, unless I have missed something obvious.
> >
> > What do you think?
>
> My primary concern about stuffing more things into the fast path is the
> limited debuggability since we can't use instrumentation in a
> not-quite-kernel context.
Yes and no. We can use *some* instrumentation (such as tracing, and
even - horror - printk). But it is the likes of WARN() that go south
because the VHE vectors are not handling BRK as the rest of the
kernel. This is definitely on my list of things to fix.
> I do generally agree with the intent of organizing it all similarly,
> just that the traps infrastructure is _really_ handy for doing the job.
Maybe there is a middle-ground here. We could, as you suggest, use the
xarray as the repository for trap routing information (including the
'InHost' aspect), and yet have the fast path. It shouldn't be too hard
to expose a lookup function and the relevant helpers to decode the
63bit control word.
> Let me take an accounting of how many other 'InHost' trap bits we have
> and get a feel for if we need a generalized solution or if I can do
> something PMU-specific here.
That'd be super useful indeed.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-08-27 7:59 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-24 0:13 [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Oliver Upton
2024-08-24 0:14 ` [PATCH 1/3] KVM: arm64: Add helpers to determine if PMC counts at a given EL Oliver Upton
2024-08-24 0:14 ` [PATCH 2/3] KVM: arm64: nv: Honor NSH filter when in hyp context Oliver Upton
2024-08-24 0:14 ` [PATCH 3/3] KVM: arm64: nv: Reprogram PMU events affected by nested transition Oliver Upton
2024-08-25 8:16 ` [PATCH 0/3] KVM: arm64: nv: Add EL2 PMU event filtering support Marc Zyngier
2024-08-26 17:26 ` Oliver Upton
2024-08-27 6:35 ` Marc Zyngier
2024-08-27 7:01 ` Oliver Upton
2024-08-27 7:59 ` Marc Zyngier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.