Index | Thread | Search

From:
hshoexer <hshoexer@yerbouti.franken.de>
Subject:
SEV-SNP: Prepare for SNP page validation 1/5
To:
tech@openbsd.org
Date:
Thu, 24 Sep 2026 18:10:50 +0200

Download raw body.

Thread
  • hshoexer:

    SEV-SNP: Prepare for SNP page validation 1/5

Hi,

this is a series of five diffs, that implement SEV-SNP guest support
for qemu/KVM.

An essential functionality of SEV-SNP is page validation: An SNP enabled
guest claims pages using the pvalidate instruction.  After successfully
claiming a page, the CPU ensures that only that guest can access the page.
If the hypervisor or another guest accesses that page, they will receive
a protection fault trap.

For bounce buffers, the guest explicitly rescinds the page to the
hypervisor.

This change implements claiming and rescinding pages for SEV-SNP.
It also provides a new GHCB request, "Page State Change" (PSC), that is
used together with pvalidate.

A PSC request may be interrupted by the hypervisor and has to be repeated
in that case.

Handling of an AMD EPYC cache coherency CPU bug by Stefan Fritsch and
Sebastian Sturm.

ok?

Take care,
HJ.
---
 sys/arch/amd64/amd64/ghcb.c         | 118 +++++++++++++---
 sys/arch/amd64/amd64/snp.c          | 202 ++++++++++++++++++++++++++++
 sys/arch/amd64/conf/files.amd64     |   1 +
 sys/arch/amd64/include/ghcb.h       |  22 +++
 sys/arch/amd64/include/snp.h        |  36 +++++
 sys/arch/amd64/include/specialreg.h |   5 +
 sys/arch/amd64/include/vmmvar.h     |   3 +-
 7 files changed, 369 insertions(+), 18 deletions(-)
 create mode 100644 sys/arch/amd64/amd64/snp.c
 create mode 100644 sys/arch/amd64/include/snp.h

diff --git a/sys/arch/amd64/amd64/ghcb.c b/sys/arch/amd64/amd64/ghcb.c
index a0539c76eb4..764010dccec 100644
--- a/sys/arch/amd64/amd64/ghcb.c
+++ b/sys/arch/amd64/amd64/ghcb.c
@@ -225,19 +225,20 @@ ghcb_sync_out(struct trapframe *frame, const struct ghcb_extra_regs *regs,
 	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_RDX))
 		ghcb->v_rdx = frame->tf_rdx & ghcb_sz_masks[gsout->sz_d];
 
-	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_EXITCODE))
-		ghcb->v_sw_exitcode = regs->exitcode;
-	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_EXITINFO1))
-		ghcb->v_sw_exitinfo1 = regs->exitinfo1;
-	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_EXITINFO2))
-		ghcb->v_sw_exitinfo2 = regs->exitinfo2;
-	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_SCRATCH))
-		ghcb->v_sw_scratch = regs->scratch;
-
-	if (regs && regs->data) {
-		data_sz = regs->data_sz;
-		KASSERT(data_sz <= sizeof(ghcb->v_sharedbuf));
-		memcpy(ghcb->v_sharedbuf, regs->data, data_sz);
+	if (regs) {
+		if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_EXITCODE))
+			ghcb->v_sw_exitcode = regs->exitcode;
+		if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_EXITINFO1))
+			ghcb->v_sw_exitinfo1 = regs->exitinfo1;
+		if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_EXITINFO2))
+			ghcb->v_sw_exitinfo2 = regs->exitinfo2;
+		if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_SCRATCH))
+			ghcb->v_sw_scratch = regs->scratch;
+		if (regs->data) {
+			data_sz = regs->data_sz;
+			KASSERT(data_sz <= sizeof(ghcb->v_sharedbuf));
+			memcpy(ghcb->v_sharedbuf, regs->data, data_sz);
+		}
 	}
 }
 
@@ -270,10 +271,20 @@ ghcb_sync_in(struct trapframe *frame, struct ghcb_extra_regs *regs,
 		frame->tf_rdx |= (ghcb->v_rdx & ghcb_sz_masks[gsin->sz_d]);
 	}
 
-	if (regs && regs->data) {
-		data_sz = regs->data_sz;
-		KASSERT(data_sz <= sizeof(ghcb->v_sharedbuf));
-		memcpy(regs->data, ghcb->v_sharedbuf, data_sz);
+	if (regs) {
+		if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_SW_EXITCODE))
+			regs->exitcode = ghcb->v_sw_exitcode;
+		if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_SW_EXITINFO1))
+			regs->exitinfo1 = ghcb->v_sw_exitinfo1;
+		if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_SW_EXITINFO2))
+			regs->exitinfo2 = ghcb->v_sw_exitinfo2;
+		if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_SW_SCRATCH))
+			regs->scratch = ghcb->v_sw_scratch;
+		if (regs->data) {
+			data_sz = regs->data_sz;
+			KASSERT(data_sz <= sizeof(ghcb->v_sharedbuf));
+			memcpy(regs->data, ghcb->v_sharedbuf, data_sz);
+		}
 	}
 
 	ghcb_clear(ghcb);
@@ -434,3 +445,76 @@ _ghcb_io_rw(uint16_t port, int valsz, uint32_t *val, bool read)
 	if (read)
 		*val = frame.tf_rax;
 }
+
+/*
+ * ghcb_psc_vmgexit
+ *
+ * Request a page state change from the hypervisor.
+ */
+void
+ghcb_psc_vmgexit(struct ghcb_psc *psc)
+{
+	struct ghcb_sync	 syncout, syncin;
+	struct ghcb_sa		*ghcb;
+	struct ghcb_psc		 rpsc;
+	unsigned long		 s;
+	struct ghcb_extra_regs	 ghcb_regs, rghcb_regs;
+	int			 cnt;
+
+	memset(&syncout, 0, sizeof(syncout));
+	memset(&syncin, 0, sizeof(syncin));
+	memset(&ghcb_regs, 0, sizeof(ghcb_regs));
+
+	ghcb_regs.exitcode = SEV_VMGEXIT_PAGE_STATE_CHANGE;
+	ghcb_regs.scratch = ghcb_paddr + offsetof(struct ghcb_sa, v_sharedbuf);
+	ghcb_regs.data = psc;
+	ghcb_regs.data_sz = sizeof(*psc);
+
+	ghcb_sync_val(GHCB_SW_EXITCODE, GHCB_SZ64, &syncout);
+	ghcb_sync_val(GHCB_SW_EXITINFO1, GHCB_SZ64, &syncout);
+	ghcb_sync_val(GHCB_SW_EXITINFO2, GHCB_SZ64, &syncout);
+	ghcb_sync_val(GHCB_SW_SCRATCH, GHCB_SZ64, &syncout);
+
+	ghcb_sync_val(GHCB_SW_EXITINFO1, GHCB_SZ64, &syncin);
+	ghcb_sync_val(GHCB_SW_EXITINFO2, GHCB_SZ64, &syncin);
+
+	s = intr_disable();
+
+	for (cnt = 0; cnt < GHCB_MAX_RETRIES; cnt++) {
+		ghcb = (struct ghcb_sa *)ghcb_vaddr;
+		ghcb_sync_out(NULL, &ghcb_regs, ghcb, &syncout);
+
+		wrmsr(MSR_SEV_GHCB, ghcb_paddr);
+
+		vmgexit();
+
+		if (ghcb_verify_bm(ghcb->valid_bitmap, syncin.valid_bitmap)) {
+			ghcb_clear(ghcb);
+			panic("invalid hypervisor response");
+		}
+
+		memset(&rghcb_regs, 0, sizeof(rghcb_regs));
+		memset(&rpsc, 0, sizeof(rpsc));
+		rghcb_regs.data = &rpsc;
+		rghcb_regs.data_sz = sizeof(rpsc);
+
+		ghcb_sync_in(NULL, &rghcb_regs, ghcb, &syncin);
+
+		if (rghcb_regs.exitinfo1 != 0 || rpsc.psc_hdr.reserved != 0)
+			panic("page state change failed: 0x%llx 0x%x",
+			    rghcb_regs.exitinfo1, rpsc.psc_hdr.reserved);
+
+		/* All entries processed. */
+		if (rpsc.psc_hdr.cur_entry > psc->psc_hdr.end_entry)
+			break;
+
+		/* Zero means interrupted, retry.  Anything else is fatal. */
+		if (rghcb_regs.exitinfo2 != 0)
+			panic("page state change failed: 0x%llx",
+			    rghcb_regs.exitinfo2);
+	}
+	if (cnt == GHCB_MAX_RETRIES)
+		panic("page state change did not finish");
+
+	intr_restore(s);
+}
diff --git a/sys/arch/amd64/amd64/snp.c b/sys/arch/amd64/amd64/snp.c
new file mode 100644
index 00000000000..6789a7421d1
--- /dev/null
+++ b/sys/arch/amd64/amd64/snp.c
@@ -0,0 +1,202 @@
+/*	$OpenBSD$ */
+
+/*
+ * Copyright (c) 2025 Hans-Joerg Hoexer <hshoexer@genua.de>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <sys/param.h>
+#include <sys/types.h>
+#include <sys/systm.h>
+
+#include <machine/ghcb.h>
+#include <machine/snp.h>
+
+/*
+ * Workaround for cache coherency CPU bug named CVE-2025-38560 in linux and
+ * CVE-2024-36331 in OVMF.  It seems this can cause memory corruption even
+ * without any attacker, corrupting page tables and causing panics while
+ * booting.
+ */
+static void
+snp_evict_cache(vaddr_t va, unsigned int npages)
+{
+	volatile uint8_t dummy;	/* Needs to be volatile. */
+	uint8_t *ptr = (uint8_t *)va;
+	unsigned int i;
+
+	if (curcpu()->ci_feature_amdsev_ebx & CPUIDEBX_COHERENCY_SFW_NO)
+		return;
+
+	for (i = 0; i < npages; i++) {
+		/*
+		 * Reading from the first and last cache line of the page from
+		 * the guest causes a flush of all cache-lines of that page
+		 * and is cheaper than a complete clflush.
+		 *
+		 * dummy must be volatile to ensure the loads are not
+		 * optimized away.
+		 */
+		dummy = ptr[i * PAGE_SIZE];
+		dummy = ptr[i * PAGE_SIZE + PAGE_SIZE - 1];
+	}
+}
+
+/*
+ * _pvalidate
+ *
+ * pvalidate single 4K page.
+ */
+static __inline int
+_pvalidate(vaddr_t va, uint32_t state)
+{
+	uint64_t	rax, rv;
+	uint32_t	ecx, edx;
+	uint8_t		unchanged = 0;
+
+	rax = va;
+	ecx = 0;	/* 4K page */
+	if (state == SNP_PSC_PRIVATE)
+		edx = 1;	/* set "validated" in RMP */
+	else
+		edx = 0;	/* clear "validated" RMP */
+
+	__asm volatile(
+		"	pvalidate				;"
+		"	setc	%b0				;"
+	    : "=qm" (unchanged), "=a" (rv)
+	    : "a" (rax), "c" (ecx), "d" (edx)
+	    : "memory", "cc");
+
+	/*
+	 * pvalidate was successful, however, the "validated" entry
+	 * was already set to "state" (ie. no change).
+	 */
+	if (rv == 0 && unchanged)
+		return (-1);
+
+	if (state == SNP_PSC_PRIVATE)
+		snp_evict_cache(va, 1);
+
+	return (rv);
+}
+
+/*
+ * pvalidate
+ *
+ * pvalidate range in 4K chunks.
+ */
+static int
+pvalidate(paddr_t start, paddr_t end, uint32_t state)
+{
+	paddr_t		pa;
+	int		error;
+
+	for (pa = start; pa < end; pa += PAGE_SIZE) {
+		if ((error = _pvalidate(PMAP_DIRECT_MAP(pa), state)) != 0)
+			return (error);
+	}
+
+	return (0);
+}
+
+/*
+ * snp_page_state_change
+ *
+ * Set the host physical page, that is assigned to our VM as guest
+ * physical page to either "private" (ie. encrypted) or "shared" (ie.
+ * unencrypted).
+ *
+ * When sharing a page with hypervisor, ie. a page shall be used
+ * as bounce buffer, we require the following order of events:
+ *
+ * 1) Set "validate" in the RMP to 0 using pvalidate.
+ * 2) Request the hypervisor to set "assigned" in the RMP to
+ *    "hypervisor owned".
+ * 3) Establish a mapping with the C-bit cleared; with this mapping
+ *    the page can be accessed unencrypted.
+ *
+ * When claiming a page, ie. when freeing a bounce buffer page, we
+ * require the following order:
+ *
+ * 1) Remove the mapping for unencrypted access.
+ * 2) Request the hypervisor to set "assigned" to "guest owned"
+ * 3) Set "validated" in the RMP to 1; this requires a mapping with
+ *    C-bit set; we use the PMAP DIRECT mapping of that page, as
+ *    these are always encrypted.
+ */
+static void
+snp_page_state_change(paddr_t start, paddr_t end, int state)
+{
+	struct ghcb_psc	psc;
+	paddr_t		pa;
+	int		error;
+
+	for (pa = start; pa < end; pa += PAGE_SIZE) {
+		memset(&psc, 0, sizeof(psc));
+		psc.psc_entry.gfn = pa >> PAGE_SHIFT;
+		psc.psc_entry.operation = state;
+		psc.psc_entry.pagesize = 0;	/* 4K page */
+
+		if (state == SNP_PSC_SHARED &&
+		    (error = pvalidate(pa, pa + PAGE_SIZE, state)) != 0)
+			panic("pvalidate failed: %d", error);
+
+		ghcb_psc_vmgexit(&psc);
+
+		if (state == SNP_PSC_PRIVATE &&
+		    (error = pvalidate(pa, pa + PAGE_SIZE, state)) != 0)
+			panic("pvalidate failed: %d", error);
+	}
+}
+
+static void
+snp_claim(paddr_t start, paddr_t end)
+{
+	snp_page_state_change(start, end, SNP_PSC_PRIVATE);
+}
+
+static void
+snp_rescind(paddr_t start, paddr_t end)
+{
+	snp_page_state_change(start, end, SNP_PSC_SHARED);
+}
+
+void
+snp_claim_pages(struct pglist *mlist)
+{
+	struct vm_page *pg;
+
+	if (!ISSET(cpu_sev_guestmode, SEV_STAT_SNP_ACTIVE))
+		return;
+
+	TAILQ_FOREACH(pg, mlist, pageq) {
+		snp_claim(VM_PAGE_TO_PHYS(pg),
+		    VM_PAGE_TO_PHYS(pg) + PAGE_SIZE);
+	}
+}
+
+void
+snp_rescind_pages(struct pglist *mlist)
+{
+	struct vm_page *pg;
+
+	if (!ISSET(cpu_sev_guestmode, SEV_STAT_SNP_ACTIVE))
+		return;
+
+	TAILQ_FOREACH(pg, mlist, pageq) {
+		snp_rescind(VM_PAGE_TO_PHYS(pg),
+		    VM_PAGE_TO_PHYS(pg) + PAGE_SIZE);
+	}
+}
diff --git a/sys/arch/amd64/conf/files.amd64 b/sys/arch/amd64/conf/files.amd64
index d575e835795..93579921c09 100644
--- a/sys/arch/amd64/conf/files.amd64
+++ b/sys/arch/amd64/conf/files.amd64
@@ -30,6 +30,7 @@ file	arch/amd64/amd64/i8259.c
 file	arch/amd64/amd64/cacheinfo.c
 file	arch/amd64/amd64/ghcb.c			amdsev
 file	arch/amd64/amd64/sev_bus_space.c	amdsev
+file	arch/amd64/amd64/snp.c			amdsev
 file	arch/amd64/amd64/vector.S
 file	arch/amd64/amd64/copy.S
 file	arch/amd64/amd64/spl.S
diff --git a/sys/arch/amd64/include/ghcb.h b/sys/arch/amd64/include/ghcb.h
index 7d98e2c089b..21062867204 100644
--- a/sys/arch/amd64/include/ghcb.h
+++ b/sys/arch/amd64/include/ghcb.h
@@ -44,6 +44,8 @@
 
 #define GHCB_MAX			0xFFF
 
+#define GHCB_MAX_RETRIES		8
+
 #endif	/* !_LOCORE */
 
 /* Definitions used with the MSR protocol */
@@ -118,6 +120,25 @@ struct ghcb_sync {
 	int			sz_d;
 };
 
+#define SNP_PSC_PRIVATE		0x0001
+#define SNP_PSC_SHARED		0x0002
+
+struct ghcb_psc {
+	struct {
+		uint16_t	cur_entry;
+		uint16_t	end_entry;
+		uint32_t	reserved;
+	} psc_hdr;
+
+	struct {
+		uint64_t	cur_page:12;
+		uint64_t	gfn:40;
+		uint64_t	operation:4;
+		uint64_t	pagesize:1;
+		uint64_t	reserved:7;
+	} psc_entry;
+};
+
 extern vaddr_t ghcb_vaddr;
 extern paddr_t ghcb_paddr;
 
@@ -136,6 +157,7 @@ int	ghcb_valbm_isset(uint8_t *, int);
 int	ghcb_verify_bm(uint8_t *, uint8_t *);
 int	ghcb_valid(struct ghcb_sa *);
 int	ghcb_empty(struct ghcb_sa *);
+void	ghcb_psc_vmgexit(struct ghcb_psc *);
 
 void	ghcb_sync_val(int, int, struct ghcb_sync *);
 void	ghcb_sync_out(struct trapframe *, const struct ghcb_extra_regs *,
diff --git a/sys/arch/amd64/include/snp.h b/sys/arch/amd64/include/snp.h
new file mode 100644
index 00000000000..fd6d4bf6a44
--- /dev/null
+++ b/sys/arch/amd64/include/snp.h
@@ -0,0 +1,36 @@
+/*	$OpenBSD$ */
+
+/*
+ * Copyright (c) 2025 Hans-Joerg Hoexer <hshoexer@genua.de>
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef _MACHINE_SNP_H_
+#define _MACHINE_SNP_H_
+
+#ifdef AMDSEV
+
+#include <uvm/uvm.h>
+
+void	snp_claim_pages(struct pglist *);
+void	snp_rescind_pages(struct pglist *);
+
+#else	/* !AMDSEV */
+
+#define	snp_claim_pages(l)	/* nothing */
+#define	snp_rescind_pages(l)	/* nothing */
+
+#endif	/* AMDSEV */
+
+#endif /* !_MACHINE_SNP_H_ */
diff --git a/sys/arch/amd64/include/specialreg.h b/sys/arch/amd64/include/specialreg.h
index 32b156b78a7..11b7b594db7 100644
--- a/sys/arch/amd64/include/specialreg.h
+++ b/sys/arch/amd64/include/specialreg.h
@@ -436,6 +436,11 @@
      "\023VTOMMSR" "\024IBSVIRT" "\031VMSARPROT" "\032SMTPROT" \
      "\035SVSMPAGEMSR" "\036NVSMSR" )
 
+/*
+ * AMD CPUID function 0x8000001F EBX bits
+ */
+#define CPUIDEBX_COHERENCY_SFW_NO	(1ULL << 31) /* no coherency cpu bug */
+
 /* Number of encrypted guests */
 #define CPUID_AMDSEV_ECX_BITS ("\20")
 
diff --git a/sys/arch/amd64/include/vmmvar.h b/sys/arch/amd64/include/vmmvar.h
index 8b670c5fcf0..287632bb574 100644
--- a/sys/arch/amd64/include/vmmvar.h
+++ b/sys/arch/amd64/include/vmmvar.h
@@ -271,10 +271,11 @@ struct vm;
 #define SVM_VMEXIT_INVALID			-1
 
 /*
- *  Additional VMEXIT codes used in SEV-ES/SNP in the GHCB
+ * Additional VMEXIT codes used in SEV-ES/SNP in the GHCB
  */
 #define SEV_VMGEXIT_MMIO_READ			0x80000001
 #define SEV_VMGEXIT_MMIO_WRITE			0x80000002
+#define SEV_VMGEXIT_PAGE_STATE_CHANGE		0x80000010
 
 #ifndef _LOCORE
 
-- 
2.47.3