| Message ID | 20250128124812.7324-4-manali.shukla@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v6,1/3] x86/cpufeatures: Add CPUID feature bit for Idle HLT intercept | expand |
On Tue, Jan 28, 2025, Manali Shukla wrote:
> + if (kvm_cpu_has(X86_FEATURE_IDLE_HLT))
Well, shoot. I gave you bad input, and we're stuck.
this_cpu_has() isn't correct, because the part of my previous feedback about
needing to check *KVM* support was 100% correct. But kvm_cpu_has() isn't right
either, because that checks what KVM supports exposing to the guest, not what
KVM itself supports/uses. E.g. even if we add full nested support, the test would
fail if nested=0 due to KVM not "supporting" Idle HLT despite using it under the
hood.
The lack of a way for KVM to communicate support to the user has come up in the
past, e.g. in discussion around /proc/cpuinfo. Sadly, AFAIK there are no (good)
ideas on what that should look like.
For now, I'll just skip this patch, even though doing so makes me quite sad.
Hi Sean, Thank you for reviewing my patches. On 2/26/2025 6:08 AM, Sean Christopherson wrote: > On Tue, Jan 28, 2025, Manali Shukla wrote: >> + if (kvm_cpu_has(X86_FEATURE_IDLE_HLT)) > > Well, shoot. I gave you bad input, and we're stuck. > > this_cpu_has() isn't correct, because the part of my previous feedback about > needing to check *KVM* support was 100% correct. But kvm_cpu_has() isn't right > either, because that checks what KVM supports exposing to the guest, not what > KVM itself supports/uses. E.g. even if we add full nested support, the test would > fail if nested=0 due to KVM not "supporting" Idle HLT despite using it under the > hood. > > The lack of a way for KVM to communicate support to the user has come up in the > past, e.g. in discussion around /proc/cpuinfo. Sadly, AFAIK there are no (good) > ideas on what that should look like. > > For now, I'll just skip this patch, even though doing so makes me quite sad. I understand your point. -Manali
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm index 4277b983cace..d6eda8c19fed 100644 --- a/tools/testing/selftests/kvm/Makefile.kvm +++ b/tools/testing/selftests/kvm/Makefile.kvm @@ -135,6 +135,7 @@ TEST_GEN_PROGS_x86 += steal_time TEST_GEN_PROGS_x86 += kvm_binary_stats_test TEST_GEN_PROGS_x86 += system_counter_offset_test TEST_GEN_PROGS_x86 += pre_fault_memory_test +TEST_GEN_PROGS_x86 += ipi_hlt_test # Compiled outputs used by test targets TEST_GEN_PROGS_EXTENDED_x86 += x86/nx_huge_pages_test diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h index e28c3e462fa7..fe0ed8d33e37 100644 --- a/tools/testing/selftests/kvm/include/x86/processor.h +++ b/tools/testing/selftests/kvm/include/x86/processor.h @@ -200,6 +200,7 @@ struct kvm_x86_cpu_feature { #define X86_FEATURE_PAUSEFILTER KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 10) #define X86_FEATURE_PFTHRESHOLD KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 12) #define X86_FEATURE_VGIF KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 16) +#define X86_FEATURE_IDLE_HLT KVM_X86_CPU_FEATURE(0x8000000A, 0, EDX, 30) #define X86_FEATURE_SEV KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 1) #define X86_FEATURE_SEV_ES KVM_X86_CPU_FEATURE(0x8000001F, 0, EAX, 3) #define X86_FEATURE_PERFMON_V2 KVM_X86_CPU_FEATURE(0x80000022, 0, EAX, 0) diff --git a/tools/testing/selftests/kvm/ipi_hlt_test.c b/tools/testing/selftests/kvm/ipi_hlt_test.c new file mode 100644 index 000000000000..449845fa2a82 --- /dev/null +++ b/tools/testing/selftests/kvm/ipi_hlt_test.c @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (C) 2024 Advanced Micro Devices, Inc. + * + */ +#include <kvm_util.h> +#include <processor.h> +#include <test_util.h> +#include "apic.h" + +#define INTR_VECTOR 0x30 +#define NUM_ITERATIONS 1000 + +static bool irq_received; + +/* + * The guest code instruments the scenario where there is a V_INTR pending + * event available while hlt instruction is executed. + */ + +static void guest_code(void) +{ + uint64_t icr_val; + int i; + + x2apic_enable(); + + icr_val = (APIC_DEST_SELF | APIC_INT_ASSERT | INTR_VECTOR); + + for (i = 0; i < NUM_ITERATIONS; i++) { + cli(); + x2apic_write_reg(APIC_ICR, icr_val); + safe_halt(); + GUEST_ASSERT(READ_ONCE(irq_received)); + WRITE_ONCE(irq_received, false); + } + GUEST_DONE(); +} + +static void guest_intr_handler(struct ex_regs *regs) +{ + WRITE_ONCE(irq_received, true); + x2apic_write_reg(APIC_EOI, 0x00); +} + +int main(int argc, char *argv[]) +{ + struct kvm_vm *vm; + struct kvm_vcpu *vcpu; + struct ucall uc; + uint64_t halt_exits; + + TEST_REQUIRE(kvm_has_cap(KVM_CAP_BINARY_STATS_FD)); + + vm = vm_create_with_one_vcpu(&vcpu, guest_code); + + vm_install_exception_handler(vm, INTR_VECTOR, guest_intr_handler); + + vcpu_run(vcpu); + TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); + + halt_exits = vcpu_get_stat(vcpu, halt_exits); + + switch (get_ucall(vcpu, &uc)) { + case UCALL_ABORT: + REPORT_GUEST_ASSERT(uc); + /* NOT REACHED */ + case UCALL_DONE: + break; + default: + TEST_FAIL("Unknown ucall 0x%lx.", uc.cmd); + } + + if (kvm_cpu_has(X86_FEATURE_IDLE_HLT)) + TEST_ASSERT_EQ(halt_exits, 0); + else + TEST_ASSERT_EQ(halt_exits, NUM_ITERATIONS); + + kvm_vm_free(vm); + return 0; +}