linux-kernel - Re: [PATCH v16 45/51] KVM: selftests: Add an MSR test to exercise guest/host and read/write

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aNJUPjdRoqtiXYp+@intel.com>
Date: Tue, 23 Sep 2025 16:03:10 +0800
From: Chao Gao <chao.gao@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Paolo Bonzini <pbonzini@...hat.com>, <kvm@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Tom Lendacky <thomas.lendacky@....com>,
	Mathias Krause <minipli@...ecurity.net>, John Allen <john.allen@....com>,
	Rick Edgecombe <rick.p.edgecombe@...el.com>, Binbin Wu
	<binbin.wu@...ux.intel.com>, Xiaoyao Li <xiaoyao.li@...el.com>, "Maxim
 Levitsky" <mlevitsk@...hat.com>, Zhang Yi Z <yi.z.zhang@...ux.intel.com>,
	"Xin Li" <xin@...or.com>
Subject: Re: [PATCH v16 45/51] KVM: selftests: Add an MSR test to exercise
 guest/host and read/write

On Fri, Sep 19, 2025 at 03:32:52PM -0700, Sean Christopherson wrote:
>Add a selftest to verify reads and writes to various MSRs, from both the
>guest and host, and expect success/failure based on whether or not the
>vCPU supports the MSR according to supported CPUID.
>
>Note, this test is extremely similar to KVM-Unit-Test's "msr" test, but
>provides more coverage with respect to host accesses, and will be extended
>to provide addition testing of CPUID-based features, save/restore lists,
>and KVM_{G,S}ET_ONE_REG, all which are extremely difficult to validate in
>KUT.
>
>If kvm.ignore_msrs=true, skip the unsupported and reserved testcases as
>KVM's ABI is a mess; what exactly is supposed to be ignored, and when,
>varies wildly.
>
>Signed-off-by: Sean Christopherson <seanjc@...gle.com>

Reviewed-by: Chao Gao <chao.gao@...el.com>

<snip>
>+/*
>+ * Note, use a page aligned value for the canonical value so that the value
>+ * is compatible with MSRs that use bits 11:0 for things other than addresses.
>+ */
>+static const u64 canonical_val = 0x123456789000ull;

...

>+{
>+	const struct kvm_msr __msrs[] = {
>+		MSR_TEST_NON_ZERO(MSR_IA32_MISC_ENABLE,
>+				  MISC_ENABLES_RESET_VAL | MSR_IA32_MISC_ENABLE_FAST_STRING,
>+				  MSR_IA32_MISC_ENABLE_FAST_STRING, MISC_ENABLES_RESET_VAL, NONE),
>+		MSR_TEST_NON_ZERO(MSR_IA32_CR_PAT, 0x07070707, 0, 0x7040600070406, NONE),
>+
>+		/*
>+		 * TSC_AUX is supported if RDTSCP *or* RDPID is supported.  Add
>+		 * entries for each features so that TSC_AUX doesn't exists for
>+		 * the "unsupported" vCPU, and obviously to test both cases.
>+		 */
>+		MSR_TEST2(MSR_TSC_AUX, 0x12345678, canonical_val, RDTSCP, RDPID),
>+		MSR_TEST2(MSR_TSC_AUX, 0x12345678, canonical_val, RDPID, RDTSCP),

At first glance, it's unclear to me why canonical_val is invalid for
MSR_TSC_AUX, especially since it is valid for a few other MSRs in this
test. Should we add a note to the above comment? e.g.,

canonical_val is invalid for MSR_TSC_AUX because its high 32 bits must be 0.

>+
>+		MSR_TEST(MSR_IA32_SYSENTER_CS, 0x1234, 0, NONE),
>+		/*
>+		 * SYSENTER_{ESP,EIP} are technically non-canonical on Intel,
>+		 * but KVM doesn't emulate that behavior on emulated writes,
>+		 * i.e. this test will observe different behavior if the MSR
>+		 * writes are handed by hardware vs. KVM.  KVM's behavior is
>+		 * intended (though far from ideal), so don't bother testing
>+		 * non-canonical values.
>+		 */
>+		MSR_TEST(MSR_IA32_SYSENTER_ESP, canonical_val, 0, NONE),
>+		MSR_TEST(MSR_IA32_SYSENTER_EIP, canonical_val, 0, NONE),
>+
>+		MSR_TEST_CANONICAL(MSR_FS_BASE, LM),
>+		MSR_TEST_CANONICAL(MSR_GS_BASE, LM),
>+		MSR_TEST_CANONICAL(MSR_KERNEL_GS_BASE, LM),
>+		MSR_TEST_CANONICAL(MSR_LSTAR, LM),
>+		MSR_TEST_CANONICAL(MSR_CSTAR, LM),
>+		MSR_TEST(MSR_SYSCALL_MASK, 0xffffffff, 0, LM),
>+
>+		MSR_TEST_CANONICAL(MSR_IA32_PL0_SSP, SHSTK),
>+		MSR_TEST(MSR_IA32_PL0_SSP, canonical_val, canonical_val | 1, SHSTK),
>+		MSR_TEST_CANONICAL(MSR_IA32_PL1_SSP, SHSTK),
>+		MSR_TEST(MSR_IA32_PL1_SSP, canonical_val, canonical_val | 1, SHSTK),
>+		MSR_TEST_CANONICAL(MSR_IA32_PL2_SSP, SHSTK),
>+		MSR_TEST(MSR_IA32_PL2_SSP, canonical_val, canonical_val | 1, SHSTK),
>+		MSR_TEST_CANONICAL(MSR_IA32_PL3_SSP, SHSTK),
>+		MSR_TEST(MSR_IA32_PL3_SSP, canonical_val, canonical_val | 1, SHSTK),
>+	};
>+
>+	/*
>+	 * Create two vCPUs, but run them on the same task, to validate KVM's
>+	 * context switching of MSR state.  Don't pin the task to a pCPU to
>+	 * also validate KVM's handling of cross-pCPU migration.
>+	 */
>+	const int NR_VCPUS = 2;
>+	struct kvm_vcpu *vcpus[NR_VCPUS];
>+	struct kvm_vm *vm;
>+
>+	kvm_static_assert(sizeof(__msrs) <= sizeof(msrs));
>+	kvm_static_assert(ARRAY_SIZE(__msrs) <= ARRAY_SIZE(msrs));
>+	memcpy(msrs, __msrs, sizeof(__msrs));
>+
>+	ignore_unsupported_msrs = kvm_is_ignore_msrs();
>+
>+	vm = vm_create_with_vcpus(NR_VCPUS, guest_main, vcpus);
>+
>+	sync_global_to_guest(vm, msrs);
>+	sync_global_to_guest(vm, ignore_unsupported_msrs);
>+
>+	for (idx = 0; idx < ARRAY_SIZE(__msrs); idx++) {
>+		sync_global_to_guest(vm, idx);
>+
>+		vcpus_run(vcpus, NR_VCPUS);
>+		vcpus_run(vcpus, NR_VCPUS);
>+	}
>+
>+	kvm_vm_free(vm);
>+}
>+
>+int main(void)
>+{
>+	test_msrs();
>+}
>-- 
>2.51.0.470.ga7dc726c21-goog
>