On Thu, Apr 04, 2019 at 05:34:00PM +0100, Will Deacon wrote:
> On Thu, Mar 28, 2019 at 10:37:27AM +0000, Andrew Murray wrote:
> > Add support for the :G and :H attributes in perf by handling the
> > exclude_host/exclude_guest event attributes.
> >
> > We notify KVM of counters that we wish to be enabled or disabled on
> > guest entry/exit and thus defer from starting or stopping events based
> > on their event attributes.
> >
> > With !VHE we switch the counters between host/guest at EL2. We are able
> > to eliminate counters counting host events on the boundaries of guest
> > entry/exit when using :G by filtering out EL2 for exclude_host. When
> > using !exclude_hv there is a small blackout window at the guest
> > entry/exit where host events are not captured.
> >
> > Signed-off-by: Andrew Murray <andrew.murray@xxxxxxx>
> > ---
> > arch/arm64/kernel/perf_event.c | 43 ++++++++++++++++++++++++++++------
> > 1 file changed, 36 insertions(+), 7 deletions(-)
> >
> > diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> > index cccf4fc86d67..6bb28aaf5aea 100644
> > --- a/arch/arm64/kernel/perf_event.c
> > +++ b/arch/arm64/kernel/perf_event.c
> > @@ -26,6 +26,7 @@
> >
> > #include <linux/acpi.h>
> > #include <linux/clocksource.h>
> > +#include <linux/kvm_host.h>
> > #include <linux/of.h>
> > #include <linux/perf/arm_pmu.h>
> > #include <linux/platform_device.h>
> > @@ -528,11 +529,21 @@ static inline int armv8pmu_enable_counter(int idx)
> >
> > static inline void armv8pmu_enable_event_counter(struct perf_event *event)
> > {
> > + struct perf_event_attr *attr = &event->attr;
> > int idx = event->hw.idx;
> > + u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
> >
> > - armv8pmu_enable_counter(idx);
> > if (armv8pmu_event_is_chained(event))
> > - armv8pmu_enable_counter(idx - 1);
> > + counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
> > +
> > + kvm_set_pmu_events(counter_bits, attr);
> > +
> > + /* We rely on the hypervisor switch code to enable guest counters */
> > + if (!kvm_pmu_counter_deferred(attr)) {
> > + armv8pmu_enable_counter(idx);
> > + if (armv8pmu_event_is_chained(event))
> > + armv8pmu_enable_counter(idx - 1);
> > + }
> > }
> >
> > static inline int armv8pmu_disable_counter(int idx)
> > @@ -545,11 +556,21 @@ static inline int armv8pmu_disable_counter(int idx)
> > static inline void armv8pmu_disable_event_counter(struct perf_event *event)
> > {
> > struct hw_perf_event *hwc = &event->hw;
> > + struct perf_event_attr *attr = &event->attr;
> > int idx = hwc->idx;
> > + u32 counter_bits = BIT(ARMV8_IDX_TO_COUNTER(idx));
> >
> > if (armv8pmu_event_is_chained(event))
> > - armv8pmu_disable_counter(idx - 1);
> > - armv8pmu_disable_counter(idx);
> > + counter_bits |= BIT(ARMV8_IDX_TO_COUNTER(idx - 1));
> > +
> > + kvm_clr_pmu_events(counter_bits);
> > +
> > + /* We rely on the hypervisor switch code to disable guest counters */
> > + if (!kvm_pmu_counter_deferred(attr)) {
> > + if (armv8pmu_event_is_chained(event))
> > + armv8pmu_disable_counter(idx - 1);
> > + armv8pmu_disable_counter(idx);
> > + }
> > }
> >
> > static inline int armv8pmu_enable_intens(int idx)
> > @@ -829,11 +850,16 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
> > if (!attr->exclude_kernel)
> > config_base |= ARMV8_PMU_INCLUDE_EL2;
> > } else {
> > - if (attr->exclude_kernel)
> > - config_base |= ARMV8_PMU_EXCLUDE_EL1;
> > - if (!attr->exclude_hv)
> > + if (!attr->exclude_hv && !attr->exclude_host)
> > config_base |= ARMV8_PMU_INCLUDE_EL2;
>
> FWIW, that doesn't align with my personal view of what the host is, but
> I'm not going to argue if Marc and Christoffer are happy with it.
Ideally armv8pmu_set_event_filter needs no knowledge of exclude_guest
and exclude_host as we leave the config_base alone and instead rely on
turning the counters on/off at guest entry/exit...
The addition of "&& !attr->exclude_host" here (!VHE) and likewise for VHE in
"arm64: KVM: Enable VHE support for :G/:H perf event modifiers" (in case
you missed it) is to eliminate counters counting host events on the boundaries
of guest entry/exit when counting for guest_only (:G). Thus this is really
an optimisation for more accurate counting.
Consider the case where a user wants to record guest events only - we switch
over to the guest counters at EL2 on world-switch (both VHE and !VHE host) -
now there is a small period of time between when we start the guest counters
and when we actually jump into the guest. This period of time in the host
shouldn't be counted - therefore so long as exclude_host is set we can filter
EL2 to remove these unwanted counts.
Therefore the above hunks likely won't describe what our view of the host is.
>
> Anyway, thanks for persevering with this:
It's fun even though it hurts my head every time each time I come back to this.
>
> Acked-by: Will Deacon <will.deacon@xxxxxxx>
Thanks,
Andrew Murray
>
> Will
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
