Re: [PATCH 5/5] KVM: selftests: Add MONITOR/MWAIT quirk test

[Yuan Yao]

On Wed, Jun 08, 2022 at 10:45:16PM +0000, Sean Christopherson wrote:
> Add a test to verify the "MONITOR/MWAIT never fault" quirk, and as a
> bonus, also verify the related "MISC_ENABLES ignores ENABLE_MWAIT" quirk.
>
> If the "never fault" quirk is enabled, MONITOR/MWAIT should always be
> emulated as NOPs, even if they're reported as disabled in guest CPUID.
> Use the MISC_ENABLES quirk to coerce KVM into toggling the MWAIT CPUID
> enable, as KVM now disallows manually toggling CPUID bits after running
> the vCPU.
>
> Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> ---
>  tools/testing/selftests/kvm/.gitignore        |   1 +
>  tools/testing/selftests/kvm/Makefile          |   1 +
>  .../selftests/kvm/x86_64/monitor_mwait_test.c | 127 ++++++++++++++++++
>  3 files changed, 129 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/x86_64/monitor_mwait_test.c
>
> diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
> index 0ab0e255d292..1a56522f009c 100644
> --- a/tools/testing/selftests/kvm/.gitignore
> +++ b/tools/testing/selftests/kvm/.gitignore
> @@ -27,6 +27,7 @@
>  /x86_64/hyperv_svm_test
>  /x86_64/max_vcpuid_cap_test
>  /x86_64/mmio_warning_test
> +/x86_64/monitor_mwait_test
>  /x86_64/platform_info_test
>  /x86_64/pmu_event_filter_test
>  /x86_64/set_boot_cpu_id
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 9a256c1f1bdf..bbbfdeb7ee9b 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -56,6 +56,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/hyperv_svm_test
>  TEST_GEN_PROGS_x86_64 += x86_64/kvm_clock_test
>  TEST_GEN_PROGS_x86_64 += x86_64/kvm_pv_test
>  TEST_GEN_PROGS_x86_64 += x86_64/mmio_warning_test
> +TEST_GEN_PROGS_x86_64 += x86_64/monitor_mwait_test
>  TEST_GEN_PROGS_x86_64 += x86_64/platform_info_test
>  TEST_GEN_PROGS_x86_64 += x86_64/pmu_event_filter_test
>  TEST_GEN_PROGS_x86_64 += x86_64/set_boot_cpu_id
> diff --git a/tools/testing/selftests/kvm/x86_64/monitor_mwait_test.c b/tools/testing/selftests/kvm/x86_64/monitor_mwait_test.c
> new file mode 100644
> index 000000000000..b9af8e29721e
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86_64/monitor_mwait_test.c
> @@ -0,0 +1,127 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/ioctl.h>
> +
> +#include "kvm_util.h"
> +#include "processor.h"
> +
> +enum monitor_mwait_testcases {
> +	MWAIT_QUIRK_DISABLED = BIT(0),
> +	MISC_ENABLES_QUIRK_DISABLED = BIT(1),
> +	MWAIT_DISABLED = BIT(2),
> +};
> +
> +static void guest_monitor_wait(int testcase)
> +{
> +	/*
> +	 * If both MWAIT and its quirk are disabled, MONITOR/MWAIT should #UD,
> +	 * in all other scenarios KVM should emulate them as nops.
> +	 */
> +	bool fault_wanted = (testcase & MWAIT_QUIRK_DISABLED) &&
> +			    (testcase & MWAIT_DISABLED);
> +	u8 vector;
> +
> +	GUEST_SYNC(testcase);
> +
> +	vector = kvm_asm_safe("monitor");
> +	if (fault_wanted)
> +		GUEST_ASSERT_2(vector == UD_VECTOR, testcase, vector);
> +	else
> +		GUEST_ASSERT_2(!vector, testcase, vector);
> +
> +	vector = kvm_asm_safe("monitor");

emmm... should one of the "monitor" be "mwait" ?

> +	if (fault_wanted)
> +		GUEST_ASSERT_2(vector == UD_VECTOR, testcase, vector);
> +	else
> +		GUEST_ASSERT_2(!vector, testcase, vector);
> +}
> +
> +static void guest_code(void)
> +{
> +	guest_monitor_wait(MWAIT_DISABLED);
> +
> +	guest_monitor_wait(MWAIT_QUIRK_DISABLED | MWAIT_DISABLED);
> +
> +	guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED | MWAIT_DISABLED);
> +	guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED);
> +
> +	guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED | MWAIT_QUIRK_DISABLED | MWAIT_DISABLED);
> +	guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED | MWAIT_QUIRK_DISABLED);
> +
> +	GUEST_DONE();
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	uint64_t disabled_quirks;
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_run *run;
> +	struct kvm_vm *vm;
> +	struct ucall uc;
> +	int testcase;
> +
> +	TEST_REQUIRE(kvm_has_cap(KVM_CAP_DISABLE_QUIRKS2));
> +
> +	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> +	vcpu_clear_cpuid_feature(vcpu, X86_FEATURE_MWAIT);
> +
> +	run = vcpu->run;
> +
> +	vm_init_descriptor_tables(vm);
> +	vcpu_init_descriptor_tables(vcpu);
> +
> +	while (1) {
> +		vcpu_run(vcpu);
> +
> +		TEST_ASSERT(run->exit_reason == KVM_EXIT_IO,
> +			    "Unexpected exit reason: %u (%s),\n",
> +			    run->exit_reason,
> +			    exit_reason_str(run->exit_reason));
> +
> +		switch (get_ucall(vcpu, &uc)) {
> +		case UCALL_SYNC:
> +			testcase = uc.args[1];
> +			break;
> +		case UCALL_ABORT:
> +			TEST_FAIL("%s at %s:%ld, testcase = %lx, vector = %ld",
> +				  (const char *)uc.args[0], __FILE__,
> +				  uc.args[1], uc.args[2], uc.args[3]);
> +			goto done;
> +		case UCALL_DONE:
> +			goto done;
> +		default:
> +			TEST_FAIL("Unknown ucall %lu", uc.cmd);
> +			goto done;
> +		}
> +
> +		disabled_quirks = 0;
> +		if (testcase & MWAIT_QUIRK_DISABLED)
> +			disabled_quirks |= KVM_X86_QUIRK_MWAIT_NEVER_FAULTS;
> +		if (testcase & MISC_ENABLES_QUIRK_DISABLED)
> +			disabled_quirks |= KVM_X86_QUIRK_MISC_ENABLE_NO_MWAIT;
> +		vm_enable_cap(vm, KVM_CAP_DISABLE_QUIRKS2, disabled_quirks);
> +
> +		/*
> +		 * If the MISC_ENABLES quirk (KVM neglects to update CPUID to
> +		 * enable/disable MWAIT) is disabled, toggle the ENABLE_MWAIT
> +		 * bit in MISC_ENABLES accordingly.  If the quirk is enabled,
> +		 * the only valid configuration is MWAIT disabled, as CPUID
> +		 * can't be manually changed after running the vCPU.
> +		 */
> +		if (!(testcase & MISC_ENABLES_QUIRK_DISABLED)) {
> +			TEST_ASSERT(testcase & MWAIT_DISABLED,
> +				    "Can't toggle CPUID features after running vCPU");
> +			continue;
> +		}
> +
> +		vcpu_set_msr(vcpu, MSR_IA32_MISC_ENABLE,
> +			     (testcase & MWAIT_DISABLED) ? 0 : MSR_IA32_MISC_ENABLE_MWAIT);
> +	}
> +
> +done:
> +	kvm_vm_free(vm);
> +	return 0;
> +}
> --
> 2.36.1.255.ge46751e96f-goog
>