[PATCH kvm-unit-tests] pmu: fix conditions for emulation test

[PATCH kvm-unit-tests] pmu: fix conditions for emulation test

[Paolo Bonzini]

Right now, unittests.cfg only supports a single check line.  Multiple
checks must be space separated.

However, the pmu_emulation test does not really need nmi_watchdog=0;
it is only needed by the PMU counters test because Linux reserves one
counter if nmi_watchdog=1, but the pmu_emulation test does not
allocate all counters in the same way.  By removing the counters
tests from pmu_emulation, the check on nmi_watchdog=0 can be
removed.

This also hid a typo for the force_emulation_prefix module parameter,
which is part of the kvm module rather than the kvm_intel module,
so fix that.

Reported-by: Like Xu <like.xu.linux@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 x86/pmu.c         | 17 +++++++++--------
 x86/unittests.cfg |  3 +--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/x86/pmu.c b/x86/pmu.c
index a159333..92206ad 100644
--- a/x86/pmu.c
+++ b/x86/pmu.c
@@ -638,16 +638,17 @@ int main(int ac, char **av)
 
 	apic_write(APIC_LVTPC, PC_VECTOR);
 
-	check_counters();
-
-	if (ac > 1 && !strcmp(av[1], "emulation"))
+	if (ac > 1 && !strcmp(av[1], "emulation")) {
 		check_emulated_instr();
-
-	if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES) {
-		gp_counter_base = MSR_IA32_PMC0;
-		report_prefix_push("full-width writes");
+	} else {
 		check_counters();
-		check_gp_counters_write_width();
+
+		if (rdmsr(MSR_IA32_PERF_CAPABILITIES) & PMU_CAP_FW_WRITES) {
+			gp_counter_base = MSR_IA32_PMC0;
+			report_prefix_push("full-width writes");
+			check_counters();
+			check_gp_counters_write_width();
+		}
 	}
 
 	return report_summary();
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 6585df4..27ecd31 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -189,8 +189,7 @@ check = /proc/sys/kernel/nmi_watchdog=0
 file = pmu.flat
 arch = x86_64
 extra_params = -cpu max -append emulation
-check = /sys/module/kvm_intel/parameters/force_emulation_prefix=Y
-check = /proc/sys/kernel/nmi_watchdog=0
+check = /sys/module/kvm/parameters/force_emulation_prefix=Y
 
 [vmware_backdoors]
 file = vmware_backdoors.flat
-- 
2.27.0

Re: [PATCH kvm-unit-tests] pmu: fix conditions for emulation test

[Jim Mattson]

On Tue, Nov 16, 2021 at 2:50 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> Right now, unittests.cfg only supports a single check line.  Multiple
> checks must be space separated.
>
> However, the pmu_emulation test does not really need nmi_watchdog=0;
> it is only needed by the PMU counters test because Linux reserves one
> counter if nmi_watchdog=1, but the pmu_emulation test does not
> allocate all counters in the same way.  By removing the counters
> tests from pmu_emulation, the check on nmi_watchdog=0 can be
> removed.

Thanks for fixing this. By the way, one of the reasons that we don't
expose a virtual PMU to more customers is the conflict with the NMI
watchdog. We aren't willing to give up the NMI watchdog on the host,
and we don't really want to report a reduced number of general purpose
counters to the guest. (On AMD, we *can't* report a reduced number of
counters to the guest; the architectural specification doesn't allow
it.)

We can't be the only ones running with the NMI watchdog enabled. How
do others deal with this? Is there any hope of suspending the NMI
watchdog while in VMX non-root mode (or guest mode on AMD)?

> This also hid a typo for the force_emulation_prefix module parameter,
> which is part of the kvm module rather than the kvm_intel module,
> so fix that.
>
> Reported-by: Like Xu <like.xu.linux@gmail.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

Re: [PATCH kvm-unit-tests] pmu: fix conditions for emulation test

[Paolo Bonzini]

On 11/16/21 18:49, Jim Mattson wrote:
> Thanks for fixing this. By the way, one of the reasons that we don't
> expose a virtual PMU to more customers is the conflict with the NMI
> watchdog. We aren't willing to give up the NMI watchdog on the host,
> and we don't really want to report a reduced number of general purpose
> counters to the guest. (On AMD, we *can't* report a reduced number of
> counters to the guest; the architectural specification doesn't allow
> it.)

FWIW we also generally use the PMU emulation only for debugging of guest 
performance issues.

> We can't be the only ones running with the NMI watchdog enabled. How
> do others deal with this? Is there any hope of suspending the NMI
> watchdog while in VMX non-root mode (or guest mode on AMD)?

Like, what do you think?

Paolo

>> This also hid a typo for the force_emulation_prefix module parameter,
>> which is part of the kvm module rather than the kvm_intel module,
>> so fix that.
>>
>> Reported-by: Like Xu <like.xu.linux@gmail.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Jim Mattson <jmattson@google.com>
> 

Re: [PATCH kvm-unit-tests] pmu: fix conditions for emulation test

[Jim Mattson]

On Tue, Nov 16, 2021 at 10:03 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 11/16/21 18:49, Jim Mattson wrote:
> > Thanks for fixing this. By the way, one of the reasons that we don't
> > expose a virtual PMU to more customers is the conflict with the NMI
> > watchdog. We aren't willing to give up the NMI watchdog on the host,
> > and we don't really want to report a reduced number of general purpose
> > counters to the guest. (On AMD, we *can't* report a reduced number of
> > counters to the guest; the architectural specification doesn't allow
> > it.)
>
> FWIW we also generally use the PMU emulation only for debugging of guest
> performance issues.

We do have quite a few customers who want it, but I'm not sure that
they really know what it is they will be getting. :-)

> > We can't be the only ones running with the NMI watchdog enabled. How
> > do others deal with this? Is there any hope of suspending the NMI
> > watchdog while in VMX non-root mode (or guest mode on AMD)?
>
> Like, what do you think?
>
> Paolo
>
> >> This also hid a typo for the force_emulation_prefix module parameter,
> >> which is part of the kvm module rather than the kvm_intel module,
> >> so fix that.
> >>
> >> Reported-by: Like Xu <like.xu.linux@gmail.com>
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Reviewed-by: Jim Mattson <jmattson@google.com>
> >
>

Re: [PATCH kvm-unit-tests] pmu: fix conditions for emulation test

[Like Xu]

On 17/11/2021 4:00 am, Jim Mattson wrote:
> On Tue, Nov 16, 2021 at 10:03 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>> On 11/16/21 18:49, Jim Mattson wrote:
>>> Thanks for fixing this. By the way, one of the reasons that we don't
>>> expose a virtual PMU to more customers is the conflict with the NMI
>>> watchdog. We aren't willing to give up the NMI watchdog on the host,
>>> and we don't really want to report a reduced number of general purpose

The NMI watchdog is definitely a good feature to me as well.

>>> counters to the guest. (On AMD, we *can't* report a reduced number of
>>> counters to the guest; the architectural specification doesn't allow
>>> it.)

Yes, AMD has more architectural design troubles with the guest PMU,
which I am trying to resolve part of them (such as disabling vpmu or guest IBS 
stuff).

>>
>> FWIW we also generally use the PMU emulation only for debugging of guest
>> performance issues.

More and more customers are interfacing with me since application developers
nowadays cannot access to the host environment for their performance issues.

> 
> We do have quite a few customers who want it, but I'm not sure that
> they really know what it is they will be getting. :-)

My cloud-based customers ask me for almost everything about the host PMU 
capabilities.
For example, the Intel Guest PEBS has been a stable out-of-tree feature for a year.

> 
>>> We can't be the only ones running with the NMI watchdog enabled. How
>>> do others deal with this? Is there any hope of suspending the NMI
>>> watchdog while in VMX non-root mode (or guest mode on AMD)?
>>
>> Like, what do you think?

Suspending the host NMI watchdog for guest code would be easy to implement,
but difficult to get upstream buy-in, especially for security researchers.

For Intel, the enabled NMI watchdog unconditionally seizes a counter
from vCPU usage, causing inaccuracy for guest pmu emulation.

For AMD, we have the dedicated "CPU Watchdog Timer Register",
which may help the issue, but I'm just getting into the AMD world.

I guess Google has a perf scheduler code diff for static allocation counters
and in that case the NMI watchdog occupies a specific counter all the time.

The current upstream status is that we allow guest pmu emulation to fail
temporarily since we expect the vPMU users will run profiling agent repeatedly.

>>
>> Paolo
>>
>>>> This also hid a typo for the force_emulation_prefix module parameter,
>>>> which is part of the kvm module rather than the kvm_intel module,
>>>> so fix that.
>>>>
>>>> Reported-by: Like Xu <like.xu.linux@gmail.com>
>>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> Reviewed-by: Jim Mattson <jmattson@google.com>
>>>
>>

Re: [PATCH kvm-unit-tests] pmu: fix conditions for emulation test

[Jim Mattson]

On Tue, Nov 16, 2021 at 2:50 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
...
> However, the pmu_emulation test does not really need nmi_watchdog=0;
> it is only needed by the PMU counters test because Linux reserves one
> counter if nmi_watchdog=1, but the pmu_emulation test does not
> allocate all counters in the same way.  By removing the counters
> tests from pmu_emulation, the check on nmi_watchdog=0 can be
> removed.

If Linux reserves a counter, shouldn't KVM_GET_SUPPORTED_CPUID remove
that counter from the available set reported in CPUID.0AH? On Intel,
that is. On AMD, we're just SOL.