Re: [PATCH v3 0/5] Add support for the Idle HLT intercept feature

Paolo Bonzini
2024-05-28T10:23:08Z

On Tue, May 28, 2024 at 6:19 AM Manali Shukla <manali.shukla@amd.com> wrote:
>
> The upcoming new Idle HLT Intercept feature allows for the HLT
> instruction execution by a vCPU to be intercepted by the hypervisor
> only if there are no pending V_INTR and V_NMI events for the vCPU.
> When the vCPU is expected to service the pending V_INTR and V_NMI
> events, the Idle HLT intercept won’t trigger. The feature allows the
> hypervisor to determine if the vCPU is actually idle and reduces
> wasteful VMEXITs.

Does this have an effect on the number of vmexits for KVM, unless AVIC
is enabled? Can you write a testcase for kvm-unit-tests' vmexit.flat
that shows an improvement?

The reason I am wondering is because KVM does not really use V_INTR
injection. The "idle HLT" intercept basically differs from the basic
HLT trigger only in how it handles an STI;HLT sequence, as in that
case the interrupt can be injected directly and the HLT vmexit is
suppressed. But in that circumstance KVM would anyway use a V_INTR
intercept to detect the opening of the interrupt injection window (and
then the interrupt uses event injection rather than V_INTR). Again,
this is only true if AVIC is disabled, but that is the default.

So unless I'm wrong in my analysis above, I'm not sure this series,
albeit small, is really worth it. As things stand, it would be more
interesting to enable this for nested VMs, especially Hyper-V which
does use V_INTR and V_TPL; even better, _emulating_ it on older
processors would reduce the L2->L0->L1->L0->L2 path to a
less-expensive L2->L0->L2 vmexit.

Paolo

> Presence of the Idle HLT Intercept feature is indicated via CPUID
> function Fn8000_000A_EDX[30].
>
> Document for the Idle HLT intercept feature is available at [1].
>
> [1]: AMD64 Architecture Programmer's Manual Pub. 24593, April 2024,
> Vol 2, 15.9 Instruction Intercepts (Table 15-7: IDLE_HLT).
> https://bugzilla.kernel.org/attachment.cgi?id=306250
>
> Testing Done:
> - Added a selftest to test the Idle HLT intercept functionality.
> - Compile and functionality testing for the Idle HLT intercept selftest
> are only done for x86_64.
> - Tested SEV and SEV-ES guest for the Idle HLT intercept functionality.
>
> v2 -> v3
> - Incorporated Andrew's suggestion to structure vcpu_stat_types in
> a way that each architecture can share the generic types and also
> provide its own.
>
> v1 -> v2
> - Done changes in svm_idle_hlt_test based on the review comments from Sean.
> - Added an enum based approach to get binary stats in vcpu_get_stat() which
> doesn't use string to get stat data based on the comments from Sean.
> - Added self_halt() and cli() helpers based on the comments from Sean.
>
> Manali Shukla (5):
> x86/cpufeatures: Add CPUID feature bit for Idle HLT intercept
> KVM: SVM: Add Idle HLT intercept support
> KVM: selftests: Add safe_halt() and cli() helpers to common code
> KVM: selftests: Add an interface to read the data of named vcpu stat
> KVM: selftests: KVM: SVM: Add Idle HLT intercept test
>
> arch/x86/include/asm/cpufeatures.h | 1 +
> arch/x86/include/asm/svm.h | 1 +
> arch/x86/include/uapi/asm/svm.h | 2 +
> arch/x86/kvm/svm/svm.c | 11 ++-
> tools/testing/selftests/kvm/Makefile | 1 +
> .../testing/selftests/kvm/include/kvm_util.h | 44 +++++++++
> .../kvm/include/x86_64/kvm_util_arch.h | 40 +++++++++
> .../selftests/kvm/include/x86_64/processor.h | 18 ++++
> tools/testing/selftests/kvm/lib/kvm_util.c | 32 +++++++
> .../selftests/kvm/x86_64/svm_idle_hlt_test.c | 89 +++++++++++++++++++
> 10 files changed, 236 insertions(+), 3 deletions(-)
> create mode 100644 tools/testing/selftests/kvm/x86_64/svm_idle_hlt_test.c
>
>
> base-commit: d91a9cc16417b8247213a0144a1f0fd61dc855dd
> --
> 2.34.1
>