Messages in this thread | | | From | Dongli Si <> | Subject | Re: [PATCH v4] perf/x86/amd: Don't touch the Host-only bit inside the guest hypervisor | Date | Fri, 1 Apr 2022 16:29:11 +0800 |
| |
On 28/03/2022 14:03, Peter Zijlstra wrote: > Better I suppose, but I think the comments can be improved by covering > the 'why' of things. We can all read the code to see the what of it.
I will add comments to the code to explain 'why'.
> Anyway, doesn't this also affect behaviour? I'm guessing this HO bit is > only set by perf-record for events it wants to record on the host. But > by not setting it, we'll also record the activity of the guest.
I think the HO/GO bit can only be set on the host, and should only be set if SVM is enabled.
When the SVM is disabled, set the HO/GO bit will cause the performance counters to not work.
Set the HO/GO bit inside the guest will cause the guest emitted "unchecked MSR access error" warning, can be triggered by running "perf stat -e instructions:G ls" in the guest, because this will set the GO bit in the guest, and perf_ctr_virt_mask just mask the HO bit.
My patch does not affect the host, it just fixes the bug in the guest.
> So suppose we create a CPU wide HO event, then it will only count L0 > activity, right? Any L1 (or higher) activite will be invisible.
I don't quite understand your question.
> But with this change on, the L1 HV doesn't provide these same semantics, > it's guest will be included in that host counter.
I don't think applying this patch will cause L2 guests to be included in the host counter.
> Or is there additional counter {dis,en}abling on virt enter,exit (resp.) > to achieve these semantics?
I don't think there is such a counter.
Also, I found that the L1 HV will emitted "unchecked MSR access error" warning when "perf record" is executed on L2, because the GO bit of L1 HV is set.
I wrote a new patch to mask the HO/GO bit in the guest, I will send it later.
Regards, Dongli
| |