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

Chao Gao
2025-09-23T08:03:32Z

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@google.com>

Reviewed-by: Chao Gao <chao.gao@intel.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
>