AMD: Add Secure AVIC Guest Support

[RFC 04/14] x86/apic: Initialize APIC backing page for Secure AVIC

Posted by Neeraj Upadhyay 2 months, 2 weeks ago

From: Kishon Vijay Abraham I <kvijayab@amd.com>

Secure AVIC lets guest manage the APIC backing page (unlike emulated
x2APIC or x2AVIC where the hypervisor manages the APIC backing page).

However the introduced Secure AVIC Linux design still maintains the
APIC backing page in the hypervisor to shadow the APIC backing page
maintained by guest (It should be noted only subset of the registers
are shadowed for specific usecases and registers like APIC_IRR,
APIC_ISR are not shadowed).

Add sev_ghcb_msr_read() to invoke "SVM_EXIT_MSR" VMGEXIT to read
MSRs from hypervisor. Initialize the Secure AVIC's APIC backing
page by copying the initial state of shadow APIC backing page in
the hypervisor to the guest APIC backing page. Specifically copy
APIC_LVR, APIC_LDR, and APIC_LVT MSRs from the shadow APIC backing
page.

Signed-off-by: Kishon Vijay Abraham I <kvijayab@amd.com>
Co-developed-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
Signed-off-by: Neeraj Upadhyay <Neeraj.Upadhyay@amd.com>
---
 arch/x86/coco/sev/core.c            | 41 ++++++++++++++++-----
 arch/x86/include/asm/sev.h          |  2 ++
 arch/x86/kernel/apic/x2apic_savic.c | 55 +++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
index 93470538af5e..0e140f92cfef 100644
--- a/arch/x86/coco/sev/core.c
+++ b/arch/x86/coco/sev/core.c
@@ -1331,18 +1331,15 @@ int __init sev_es_efi_map_ghcbs(pgd_t *pgd)
 	return 0;
 }
 
-static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
+static enum es_result __vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt, bool write)
 {
 	struct pt_regs *regs = ctxt->regs;
+	u64 exit_info_1 = write ? 1 : 0;
 	enum es_result ret;
-	u64 exit_info_1;
-
-	/* Is it a WRMSR? */
-	exit_info_1 = (ctxt->insn.opcode.bytes[1] == 0x30) ? 1 : 0;
 
 	if (regs->cx == MSR_SVSM_CAA) {
 		/* Writes to the SVSM CAA msr are ignored */
-		if (exit_info_1)
+		if (write)
 			return ES_OK;
 
 		regs->ax = lower_32_bits(this_cpu_read(svsm_caa_pa));
@@ -1352,14 +1349,14 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 	}
 
 	ghcb_set_rcx(ghcb, regs->cx);
-	if (exit_info_1) {
+	if (write) {
 		ghcb_set_rax(ghcb, regs->ax);
 		ghcb_set_rdx(ghcb, regs->dx);
 	}
 
 	ret = sev_es_ghcb_hv_call(ghcb, ctxt, SVM_EXIT_MSR, exit_info_1, 0);
 
-	if ((ret == ES_OK) && (!exit_info_1)) {
+	if (ret == ES_OK && !write) {
 		regs->ax = ghcb->save.rax;
 		regs->dx = ghcb->save.rdx;
 	}
@@ -1367,6 +1364,34 @@ static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
 	return ret;
 }
 
+static enum es_result vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt)
+{
+	return __vc_handle_msr(ghcb, ctxt, ctxt->insn.opcode.bytes[1] == 0x30);
+}
+
+enum es_result sev_ghcb_msr_read(u64 msr, u64 *value)
+{
+	struct pt_regs regs = { .cx = msr };
+	struct es_em_ctxt ctxt = { .regs = &regs };
+	struct ghcb_state state;
+	unsigned long flags;
+	enum es_result ret;
+	struct ghcb *ghcb;
+
+	local_irq_save(flags);
+	ghcb = __sev_get_ghcb(&state);
+	vc_ghcb_invalidate(ghcb);
+
+	ret = __vc_handle_msr(ghcb, &ctxt, false);
+	if (ret == ES_OK)
+		*value = regs.ax | regs.dx << 32;
+
+	__sev_put_ghcb(&state);
+	local_irq_restore(flags);
+
+	return ret;
+}
+
 enum es_result sev_notify_savic_gpa(u64 gpa)
 {
 	struct ghcb_state state;
diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index e84fc7fcc32a..5e6385bfb85a 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -400,6 +400,7 @@ u64 sev_get_status(void);
 void sev_show_status(void);
 void snp_update_svsm_ca(void);
 enum es_result sev_notify_savic_gpa(u64 gpa);
+enum es_result sev_ghcb_msr_read(u64 msr, u64 *value);
 
 #else	/* !CONFIG_AMD_MEM_ENCRYPT */
 
@@ -437,6 +438,7 @@ static inline u64 sev_get_status(void) { return 0; }
 static inline void sev_show_status(void) { }
 static inline void snp_update_svsm_ca(void) { }
 static inline enum es_result sev_notify_savic_gpa(u64 gpa) { return ES_UNSUPPORTED; }
+static inline enum es_result sev_ghcb_msr_read(u64 msr, u64 *value) { return ES_UNSUPPORTED; }
 
 #endif	/* CONFIG_AMD_MEM_ENCRYPT */
 
diff --git a/arch/x86/kernel/apic/x2apic_savic.c b/arch/x86/kernel/apic/x2apic_savic.c
index 6a471bbc3dba..99151be4e173 100644
--- a/arch/x86/kernel/apic/x2apic_savic.c
+++ b/arch/x86/kernel/apic/x2apic_savic.c
@@ -11,6 +11,7 @@
 #include <linux/cc_platform.h>
 #include <linux/percpu-defs.h>
 #include <linux/align.h>
+#include <linux/sizes.h>
 
 #include <asm/apic.h>
 #include <asm/sev.h>
@@ -20,6 +21,19 @@
 static DEFINE_PER_CPU(void *, apic_backing_page);
 static DEFINE_PER_CPU(bool, savic_setup_done);
 
+enum lapic_lvt_entry {
+	LVT_TIMER,
+	LVT_THERMAL_MONITOR,
+	LVT_PERFORMANCE_COUNTER,
+	LVT_LINT0,
+	LVT_LINT1,
+	LVT_ERROR,
+
+	APIC_MAX_NR_LVT_ENTRIES,
+};
+
+#define APIC_LVTx(x) (APIC_LVTT + 0x10 * (x))
+
 static int x2apic_savic_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
 {
 	return x2apic_enabled() && cc_platform_has(CC_ATTR_SNP_SECURE_AVIC);
@@ -35,6 +49,22 @@ static inline void set_reg(char *page, int reg_off, u32 val)
 	WRITE_ONCE(*((u32 *)(page + reg_off)), val);
 }
 
+static u32 read_msr_from_hv(u32 reg)
+{
+	u64 data, msr;
+	int ret;
+
+	msr = APIC_BASE_MSR + (reg >> 4);
+	ret = sev_ghcb_msr_read(msr, &data);
+	if (ret != ES_OK) {
+		pr_err("Secure AVIC msr (%#llx) read returned error (%d)\n", msr, ret);
+		/* MSR read failures are treated as fatal errors */
+		snp_abort();
+	}
+
+	return lower_32_bits(data);
+}
+
 #define SAVIC_ALLOWED_IRR_OFFSET	0x204
 
 static u32 x2apic_savic_read(u32 reg)
@@ -168,6 +198,30 @@ static void x2apic_savic_send_IPI_mask_allbutself(const struct cpumask *mask, in
 	__send_IPI_mask(mask, vector, APIC_DEST_ALLBUT);
 }
 
+static void init_backing_page(void *backing_page)
+{
+	u32 val;
+	int i;
+
+	val = read_msr_from_hv(APIC_LVR);
+	set_reg(backing_page, APIC_LVR, val);
+
+	/*
+	 * Hypervisor is used for all timer related functions,
+	 * so don't copy those values.
+	 */
+	for (i = LVT_THERMAL_MONITOR; i < APIC_MAX_NR_LVT_ENTRIES; i++) {
+		val = read_msr_from_hv(APIC_LVTx(i));
+		set_reg(backing_page, APIC_LVTx(i), val);
+	}
+
+	val = read_msr_from_hv(APIC_LVT0);
+	set_reg(backing_page, APIC_LVT0, val);
+
+	val = read_msr_from_hv(APIC_LDR);
+	set_reg(backing_page, APIC_LDR, val);
+}
+
 static void x2apic_savic_setup(void)
 {
 	void *backing_page;
@@ -178,6 +232,7 @@ static void x2apic_savic_setup(void)
 		return;
 
 	backing_page = this_cpu_read(apic_backing_page);
+	init_backing_page(backing_page);
 	gpa = __pa(backing_page);
 	ret = sev_notify_savic_gpa(gpa);
 	if (ret != ES_OK)
-- 
2.34.1

Re: [sos-linux-ext-patches] [RFC 04/14] x86/apic: Initialize APIC backing page for Secure AVIC

Posted by Melody (Huibo) Wang 2 weeks, 4 days ago

Hi Neeraj,

On 9/13/2024 4:36 AM, Neeraj Upadhyay wrote:

> +static void init_backing_page(void *backing_page)
> +{
> +	u32 val;
> +	int i;
> +
> +	val = read_msr_from_hv(APIC_LVR);
> +	set_reg(backing_page, APIC_LVR, val);
> +

When you read the register from hypervisor, there is certain value defined in APM Table 16-2. APIC Registers, says APIC_LVR has value 80??0010h out of reset.

More specifically, Bit 31 is set which means the presence of extended APIC registers, and Bit 4 is set which is part of version number: "The local APIC implementation is identified with a value=1Xh (20h-FFh are
reserved)".

I think you should verify those values instead of just reading from the hypervisor. Also, I think you probably should verify all of registers you read from the hypervisor before you use them in the guest. In other words, sanitize the inputs from the hypervisor. 

Thanks,
Melody