Message-ID: <20250923050942.206116-31-Neeraj.Upadhyay@amd.com>
Date: Tue, 23 Sep 2025 10:39:37 +0530
From: Neeraj Upadhyay <Neeraj.Upadhyay@....com>
To: <kvm@...r.kernel.org>, <seanjc@...gle.com>, <pbonzini@...hat.com>
CC: <linux-kernel@...r.kernel.org>, <Thomas.Lendacky@....com>,
	<nikunj@....com>, <Santosh.Shukla@....com>, <Vasant.Hegde@....com>,
	<Suravee.Suthikulpanit@....com>, <bp@...en8.de>, <David.Kaplan@....com>,
	<huibo.wang@....com>, <naveen.rao@....com>, <pgonda@...gle.com>,
	<linux-kselftest@...r.kernel.org>, <shuah@...nel.org>, <tiala@...rosoft.com>
Subject: [RFC PATCH v2 30/35] KVM: selftests: Add test to verify APIC MSR accesses for SAVIC guest

When Secure AVIC (SAVIC) is enabled in an SEV-SNP guest, a hybrid model
for APIC register access is used. Some registers are accelerated by the
hardware, while others trigger a #VC exception for emulation by the
guest's #VC handler. This complex interaction requires dedicated testing
to ensure correctness.

Add savic_test.c, a new selftest designed to validate APIC MSR accesses
in SAVIC mode. The test creates an SEV-SNP VM with SAVIC enabled and
systematically performs rdmsr/wrmsr operations on a comprehensive set
of APIC registers.

The test verifies several key behaviors:

* Ensure that accesses to unaccelerated MSRs (e.g., APIC_TMICT, APIC_LVTT)
  correctly trigger a #VC exception and are properly emulated by the
  savic_vc_handler().

* Confirm that architecturally invalid operations, such as writing to a
  read-only register (APIC_LVR) or reading a write-only register
  (APIC_EOI), generate the expected #GP fault.

* Verify that the state in the guest's APIC backing page is consistent
  with values read via RDMSR after a write.

* Validate that register writes are correctly propagated (or intentionally
  not propagated) to the hypervisor's internal state, ensuring proper
  synchronization for registers like APIC_SPIV versus accelerated
  registers like APIC_TASKPRI.

This test provides essential coverage for the SAVIC feature, ensuring its
MSR handling mechanism is implemented correctly.

Signed-off-by: Neeraj Upadhyay <Neeraj.Upadhyay@....com>
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../testing/selftests/kvm/include/x86/apic.h  |   1 +
 .../testing/selftests/kvm/include/x86/savic.h |   4 +
 tools/testing/selftests/kvm/lib/x86/savic.c   |  20 +-
 tools/testing/selftests/kvm/x86/savic_test.c  | 291 ++++++++++++++++++
 5 files changed, 312 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/savic_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index b94ac1caa514..1b0281e6bbe1 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -96,6 +96,7 @@ TEST_GEN_PROGS_x86 += x86/pmu_counters_test
 TEST_GEN_PROGS_x86 += x86/pmu_event_filter_test
 TEST_GEN_PROGS_x86 += x86/private_mem_conversions_test
 TEST_GEN_PROGS_x86 += x86/private_mem_kvm_exits_test
+TEST_GEN_PROGS_x86 += x86/savic_test
 TEST_GEN_PROGS_x86 += x86/set_boot_cpu_id
 TEST_GEN_PROGS_x86 += x86/set_sregs_test
 TEST_GEN_PROGS_x86 += x86/smaller_maxphyaddr_emulation_test
diff --git a/tools/testing/selftests/kvm/include/x86/apic.h b/tools/testing/selftests/kvm/include/x86/apic.h
index aa3a5d54c404..af555638086f 100644
--- a/tools/testing/selftests/kvm/include/x86/apic.h
+++ b/tools/testing/selftests/kvm/include/x86/apic.h
@@ -33,6 +33,7 @@
 #define	APIC_SPIV	0xF0
 #define		APIC_SPIV_FOCUS_DISABLED	(1 << 9)
 #define		APIC_SPIV_APIC_ENABLED		(1 << 8)
+#define APIC_ISR	0x100
 #define APIC_TMR        0x180
 #define APIC_IRR	0x200
 #define	APIC_ICR	0x300
diff --git a/tools/testing/selftests/kvm/include/x86/savic.h b/tools/testing/selftests/kvm/include/x86/savic.h
index 238d7450ab6e..33f19f5e39b3 100644
--- a/tools/testing/selftests/kvm/include/x86/savic.h
+++ b/tools/testing/selftests/kvm/include/x86/savic.h
@@ -6,6 +6,9 @@
 #ifndef SELFTEST_KVM_SAVIC_H
 #define SELFTEST_KVM_SAVIC_H
 
+#define APIC_REG_OFF(VEC)		(VEC / 32 * 16)
+#define APIC_VEC_POS(VEC)		(VEC % 32)
+
 struct guest_apic_page;
 
 void guest_apic_pages_init(struct kvm_vm *vm);
@@ -17,4 +20,5 @@ uint64_t savic_hv_read_reg(uint32_t reg);
 void savic_enable(void);
 int savic_nr_pages_required(uint64_t page_size);
 void savic_vc_handler(struct ex_regs *regs);
+struct guest_apic_page *get_guest_apic_page(void);
 #endif
diff --git a/tools/testing/selftests/kvm/lib/x86/savic.c b/tools/testing/selftests/kvm/lib/x86/savic.c
index 24ee15cc5603..da01bb5deae1 100644
--- a/tools/testing/selftests/kvm/lib/x86/savic.c
+++ b/tools/testing/selftests/kvm/lib/x86/savic.c
@@ -8,6 +8,7 @@
 #include "apic.h"
 #include "kvm_util.h"
 #include "sev.h"
+#include "savic.h"
 
 struct apic_page {
 	u8 apic_regs[PAGE_SIZE];
@@ -44,9 +45,6 @@ enum lapic_lvt_entry {
 #define SVM_EXIT_AVIC_UNACCELERATED_ACCESS      0x402
 #define SVM_EXIT_AVIC_INCOMPLETE_IPI            0x401
 
-#define REG_OFF(VEC)		(VEC / 32 * 16)
-#define VEC_POS(VEC)		(VEC % 32)
-
 #define SAVIC_NMI_REQ_OFFSET            0x278
 
 /*
@@ -104,6 +102,11 @@ void set_savic_control_msr(struct guest_apic_page *apic_page, bool enable, bool
 	wrmsr(MSR_AMD64_SECURE_AVIC_CONTROL, val);
 }
 
+struct guest_apic_page *get_guest_apic_page(void)
+{
+	return &apic_page_pool->guest_apic_page[x2apic_read_reg(APIC_ID)];
+}
+
 /*
  * Write APIC reg offset in the guest APIC backing page.
  *
@@ -175,11 +178,17 @@ static void savic_init_backing_page(struct guest_apic_page *apic_page, uint32_t
 	regval = savic_hv_read_reg(APIC_LDR);
 	savic_write_reg(apic_page, APIC_LDR, regval);
 
-	for (i = LVT_THERMAL_MONITOR; i < APIC_MAX_NR_LVT_ENTRIES; i++) {
+	for (i = LVT_TIMER; i < APIC_MAX_NR_LVT_ENTRIES; i++) {
 		regval = savic_hv_read_reg(APIC_LVTx(i));
 		savic_write_reg(apic_page, APIC_LVTx(i), regval);
 	}
 
+	regval = savic_hv_read_reg(APIC_TMICT);
+	savic_write_reg(apic_page, APIC_TMICT, regval);
+
+	regval = savic_hv_read_reg(APIC_TDCR);
+	savic_write_reg(apic_page, APIC_TDCR, regval);
+
 	regval = savic_hv_read_reg(APIC_LVT0);
 	savic_write_reg(apic_page, APIC_LVT0, regval);
 
@@ -351,7 +360,8 @@ static void send_ipi(int cpu, int vector, bool nmi)
 	if (nmi)
 		savic_write_reg(apic_page, SAVIC_NMI_REQ_OFFSET, 1);
 	else
-		savic_write_reg(apic_page, APIC_IRR + REG_OFF(vector), BIT(VEC_POS(vector)));
+		savic_write_reg(apic_page, APIC_IRR + APIC_REG_OFF(vector),
+				BIT(APIC_VEC_POS(vector)));
 }
 
 static bool is_cpu_present(int cpu)
diff --git a/tools/testing/selftests/kvm/x86/savic_test.c b/tools/testing/selftests/kvm/x86/savic_test.c
new file mode 100644
index 000000000000..bac56f85caea
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/savic_test.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+/*
+ *  Copyright (C) 2024 Advanced Micro Devices, Inc.
+ *
+ */
+#include <pthread.h>
+
+#include "processor.h"
+#include "apic.h"
+#include "kvm_util.h"
+#include "sev.h"
+#include "test_util.h"
+#include "savic.h"
+
+#define NR_SAVIC_VCPUS	1
+
+static struct kvm_vcpu *vcpus[NR_SAVIC_VCPUS];
+static pthread_t threads[NR_SAVIC_VCPUS];
+
+#define SAVIC_TEST_STATE(STATE) \
+	STATE ## _START, \
+	STATE ## _END
+
+enum savic_test_state {
+	SAVIC_TEST_STATE(SAVIC_APIC_MSR_ACCESSES),
+};
+
+#define SAVIC_GUEST_SYNC(sync, func) ({\
+	GUEST_SYNC(sync ## _START); \
+	func(id); \
+	GUEST_SYNC(sync ## _END); \
+})
+
+static int savic_wrmsr(uint32_t reg, uint64_t val)
+{
+	switch (reg) {
+	case APIC_LVR:
+	case APIC_LDR:
+	case APIC_ISR:
+	case APIC_TMR:
+	case APIC_IRR:
+	case APIC_TMCCT:
+		x2apic_write_reg_fault(reg, val);
+		return -1;
+	default:
+		x2apic_write_reg(reg, val);
+		break;
+	}
+
+	return 0;
+}
+
+static uint64_t savic_rdmsr(uint32_t reg)
+{
+	uint64_t val;
+	uint32_t msr = APIC_BASE_MSR + (reg >> 4);
+
+	switch (reg) {
+	case APIC_EOI:
+		uint8_t fault = rdmsr_safe(msr, &val);
+
+		__GUEST_ASSERT(fault == GP_VECTOR,
+				"Wanted #GP on RDMSR(%x) = %x, got 0x%x\n",
+				msr, GP_VECTOR, fault);
+		return val;
+	default:
+		return x2apic_read_reg(reg);
+	}
+}
+
+static void guest_verify_host_guest_reg(struct guest_apic_page *apage, uint32_t reg,
+		uint64_t val, char *regname)
+{
+	uint64_t hval, gval, gval2;
+
+	if (savic_wrmsr(reg, val) == -1) {
+		savic_write_reg(apage, reg, val);
+		/*
+		 * Write using PV interface if wrmsr fails. Skip for
+		 * regs which trigger GP
+		 */
+		if (reg != APIC_LVR && reg != APIC_TMR && reg != APIC_IRR)
+			savic_hv_write_reg(reg, val);
+	}
+
+	gval = savic_read_reg(apage, reg);
+	gval2 = savic_rdmsr(reg);
+	hval = savic_hv_read_reg(reg);
+	__GUEST_ASSERT(gval == val, "Unexpected Guest %s 0x%lx, expected val:0x%lx\n",
+			regname, gval, val);
+	__GUEST_ASSERT(gval == gval2, "Unexpected Guest %s backing page value : 0x%lx, msr read val:0x%lx\n",
+			regname, gval, gval2);
+
+	switch (reg) {
+	case APIC_LVR:
+	case APIC_LDR:
+	case APIC_ISR:
+	case APIC_TMICT:
+	case APIC_TDCR:
+	case APIC_LVTT:
+	case APIC_LVTTHMR:
+	case APIC_LVTPC:
+	case APIC_LVT0:
+	case APIC_LVT1:
+	case APIC_LVTERR:
+	case APIC_SPIV:
+		__GUEST_ASSERT(hval == gval, "Guest 0x%lx host 0x%lx %s mismatch\n",
+			gval, hval, regname);
+		break;
+	case APIC_TASKPRI:
+	case APIC_ICR:
+	case APIC_TMR:
+	case APIC_IRR:
+		__GUEST_ASSERT(hval != gval, "Guest 0x%lx host 0x%lx reg: %x %s must not match\n",
+			gval, hval, reg, regname);
+		break;
+	default:
+		break;
+	}
+}
+
+static inline uint32_t x2apic_ldr(uint32_t id)
+{
+	return ((id >> 4) << 16) | (1 << (id & 0xf));
+}
+
+static void guest_savic_apic_msr_accesses(int id)
+{
+	struct guest_apic_page *apage = get_guest_apic_page();
+	uint64_t val, hval;
+	uint32_t reg;
+	int vec;
+	int i;
+	uint32_t lvt_regs[] = {
+		APIC_LVTT, APIC_LVTTHMR, APIC_LVTPC,
+		APIC_LVT0, APIC_LVT1, APIC_LVTERR
+	};
+
+	reg = APIC_LVR;
+	val = savic_hv_read_reg(reg);
+	/* APIC_LVR state is in sync between host and guest. */
+	guest_verify_host_guest_reg(apage, reg, val, "APIC_LVR");
+
+	reg = APIC_TASKPRI;
+	val = 0x30;
+	/* Write new TASKPRI to host using PV interface. */
+	savic_hv_write_reg(reg, val);
+	val = 0x40;
+	/* TASKPRI is accelerated and state is not up-to-date in host. */
+	guest_verify_host_guest_reg(apage, reg, val, "APIC_TASKPRI");
+
+	reg = APIC_PROCPRI;
+	val = x2apic_read_reg(reg);
+	/* APIC_PROCPRI is updated with the APIC_TASKPRI update above. */
+	GUEST_ASSERT((val & 0xf0) == (x2apic_read_reg(APIC_TASKPRI) & 0xf0));
+	GUEST_ASSERT((val & 0xf0) == 0x40);
+	vec = 0x20;
+	x2apic_write_reg(APIC_ICR, APIC_DEST_SELF | APIC_INT_ASSERT | vec);
+	/* Interrupt remains pending in APIC_IRR. */
+	val = savic_read_reg(apage, APIC_IRR + APIC_REG_OFF(vec));
+	GUEST_ASSERT((val & BIT_ULL(APIC_VEC_POS(vec))) == BIT_ULL(APIC_VEC_POS(vec)));
+	savic_wrmsr(APIC_TASKPRI, 0x0);
+
+	/* Triggers GP fault */
+	savic_rdmsr(APIC_EOI);
+
+	reg = APIC_LDR;
+	val = x2apic_ldr(savic_rdmsr(APIC_ID));
+	hval = savic_hv_read_reg(APIC_LDR);
+	__GUEST_ASSERT(val == hval, "APIC_LDR mismatch between host %lx and guest %lx",
+			hval, val);
+
+	/* APIC_SPIV state is not visible to host. */
+	reg = APIC_SPIV;
+	val = savic_rdmsr(APIC_SPIV) & ~APIC_SPIV_APIC_ENABLED;
+	savic_hv_write_reg(reg, val);
+	val = savic_rdmsr(APIC_SPIV) | APIC_SPIV_APIC_ENABLED;
+	guest_verify_host_guest_reg(apage, reg, val, "APIC_SPIV");
+
+	reg = APIC_ISR;
+	(void) savic_rdmsr(reg);
+	/* Triggers GP fault */
+	savic_wrmsr(reg, 0x10);
+
+	/* APIC_TMR is not synced to host. */
+	reg = APIC_TMR;
+	val = 0x10000;
+	guest_verify_host_guest_reg(apage, reg, val, "APIC_TMR");
+	vec = 0x20;
+	savic_write_reg(apage, reg + APIC_REG_OFF(vec),  BIT_ULL(APIC_VEC_POS(vec)));
+	GUEST_ASSERT(x2apic_read_reg(reg + APIC_REG_OFF(vec)) & BIT_ULL(APIC_VEC_POS(vec)));
+
+	reg = APIC_IRR;
+	val = 0x10000;
+	guest_verify_host_guest_reg(apage, reg, val, "APIC_IRR");
+	savic_write_reg(apage, reg, 0x0);
+
+	reg = APIC_TMICT;
+	val = 0x555;
+	guest_verify_host_guest_reg(apage, reg, val, "APIC_TMICT");
+
+	reg = APIC_TMCCT;
+	savic_rdmsr(reg);
+	savic_wrmsr(reg, 0xf);
+
+	reg = APIC_TDCR;
+	val = 0x1;
+	savic_hv_write_reg(reg, val);
+	val = 0x3;
+	guest_verify_host_guest_reg(apage, reg, val, "APIC_TDCR");
+
+	for (i = 0; i < ARRAY_SIZE(lvt_regs); i++) {
+		reg = lvt_regs[i];
+		val = 0x41;
+		savic_hv_write_reg(reg, val);
+		val = 0x42;
+		guest_verify_host_guest_reg(apage, reg, val, "APIC_LVTx");
+	}
+}
+
+static void guest_code(int id)
+{
+	GUEST_ASSERT(rdmsr(MSR_AMD64_SEV) & MSR_AMD64_SNP_SECURE_AVIC);
+
+	x2apic_enable();
+
+	savic_enable();
+
+	SAVIC_GUEST_SYNC(SAVIC_APIC_MSR_ACCESSES, guest_savic_apic_msr_accesses);
+
+	GUEST_DONE();
+}
+
+static void *vcpu_thread(void *arg)
+{
+	struct kvm_vcpu *vcpu = (struct kvm_vcpu *)arg;
+	struct ucall uc;
+
+	fprintf(stderr, "vCPU thread running vCPU %u\n", vcpu->id);
+
+	while (true) {
+		vcpu_run(vcpu);
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_SYNC:
+			break;
+		case UCALL_DONE:
+			return NULL;
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+			break;
+		case UCALL_NONE:
+			continue;
+		default:
+			TEST_FAIL("Unknown ucall 0x%lx.", uc.cmd);
+		}
+
+	}
+
+	return NULL;
+}
+
+int main(int argc, char *argv[])
+{
+	struct kvm_sev_init args = {
+		.vmsa_features = BIT_ULL(SVM_FEAT_SECURE_AVIC)
+	};
+	struct kvm_vm *vm;
+	int r;
+
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SEV_SNP));
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SECURE_AVIC));
+
+	vm = _vm_sev_create_with_one_vcpu(KVM_X86_SNP_VM, guest_code, &vcpus[0], &args);
+
+	virt_pg_map(vm, APIC_DEFAULT_GPA, APIC_DEFAULT_GPA);
+
+	vcpu_args_set(vcpus[0], 1, vcpus[0]->id);
+
+	vm_install_exception_handler(vm, 29, savic_vc_handler);
+	vm_sev_launch(vm, snp_default_policy(), NULL);
+
+	r = pthread_create(&threads[0], NULL, vcpu_thread, vcpus[0]);
+	TEST_ASSERT(r == 0, "pthread_create failed errno=%d", errno);
+
+	pthread_join(threads[0], NULL);
+
+	kvm_vm_free(vm);
+
+	return 0;
+}
-- 
2.34.1

Powered by blists - more mailing lists