Re: [PATCH] KVM: x86/pmu: Add '.exclude_hv = 1' for guest perf_event

From: Xu, Like
Date: Wed Aug 12 2020 - 08:57:00 EST

On 2020/8/12 19:32, Paolo Bonzini wrote:
On 12/08/20 13:11, peterz@xxxxxxxxxxxxx wrote:
x86 does not have a hypervisor privilege level, so it never uses
Arguably it does when Xen, but I don't think we support that, so *phew*.
Yeah, I suppose you could imagine having paravirtualized perf counters
where the Xen privileged domain could ask Xen to run perf counters on

exclude_hv; exclude_host already excludes all root mode activity for
both ring0 and ring3.
Right, but we want to tighten the permission checks and not excluding_hv
is just sloppy.
I would just document that it's ignored as it doesn't make sense. ARM64
does that too, for new processors where the kernel is not itself split
between supervisor and hypervisor privilege levels.

There are people that are trying to run Linux-based firmware and have
SMM handlers as part of the kernel. Perhaps they could use exclude_hv
to exclude the SMM handlers from perf (if including them is possible at
Hi Paolo,

My proposal is to define:
the "hypervisor privilege levels" events in the KVM/x86 context as
all the host kernel events plus /dev/kvm user space events.

If we add ".exclude_hv = 1" in the pmc_reprogram_counter(),
do you see any side effect to cover the above usages?

The fact that exclude_hv has never been used in x86 does help
the generic perf code to handle permission checks in a more concise way.

Like Xu
The thing is, we very much do not want to allow unpriv user to be able
to create: exclude_host=1, exclude_guest=0 counters (they currently
That would be the case of an unprivileged user that wants to measure
performance of its guests. It's a scenario that makes a lot of sense,
are you worried about side channels? Can perf-events on guests leak
more about the host than perf-events on a random userspace program?

Also, exclude_host is really poorly defined:

"Suppose we have nested virt:


And we're running in G0, then:

- 'exclude_hv' would exclude L0 events
- 'exclude_host' would ... exclude L1-hv events?
- 'exclude_guest' would ... exclude G1 events?
From the point of view of G0, L0 *does not exist at all*. You just
cannot see L0 events if you're running in G0.

exclude_host/exclude_guest are the right definition.

Then the next question is, if G0 is a host, does the L1-hv run in
G0 userspace or G0 kernel space?
It's mostly kernel, but sometimes you're interested in events from QEMU
or whoever else has opened /dev/kvm. In that case you care about G0
userspace too.

The way it is implemented, you basically have to always set
exclude_host=0, even if there is no virt at all and you want to measure
your own userspace thing -- which is just weird.
I understand regretting having exclude_guest that way; include_guest
(defaulting to 0!) would have made more sense. But defaulting to
exclude_host==0 makes sense: if there is no virt at all, memset(0) does
the right thing so it does not seem weird to me.

I suppose the 'best' option at this point is something like:

* comment that explains the trainwreck.
if (!exclude_host && !exclude_guest)
exclude_guest = 1;

if ((!exclude_hv || !exclude_guest) && !perf_allow_kernel())
return -EPERM;

But that takes away the possibility of actually having:
'exclude_host=0, exclude_guest=0' to create an event that measures both,
which also sucks.
In fact both of the above "if"s suck. :(