Index | Thread | Search

From:
Mike Larkin <mlarkin@nested.page>
Subject:
Re: SEV-SNP: Prepare for SNP page validation 1/4
To:
tech@openbsd.org
Date:
Sat, 19 Sep 2026 10:58:23 -0700

Download raw body.

Thread
  • Mike Larkin:

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

  • On Tue, Aug 11, 2026 at 11:33:30AM +0200, Hans-Jörg Höxer wrote:
    > Hi,
    >
    > this is patch 1/4 that implemnts the missing pieces for SEV-SNP guest
    > support.
    >
    > One 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
    > accesses that 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 Stage Change" (PSC),
    > that is used alongside with pvalidate.
    >
    > Handling of an AMD EPYC cache coherency CPU bug by Stefan Fritsch
    > and Sebastian Sturm.
    >
    > ok? comments?
    >
    > Take care,
    > HJ.
    >
    
    once you rebase this to -current after the major churn this week lmk and
    I'll take another look.
    
    -ml
    
    > ---
    >  sys/arch/amd64/amd64/ghcb.c         |  58 +++++++
    >  sys/arch/amd64/amd64/snp.c          | 234 ++++++++++++++++++++++++++++
    >  sys/arch/amd64/conf/files.amd64     |   1 +
    >  sys/arch/amd64/include/ghcb.h       |  17 ++
    >  sys/arch/amd64/include/snp.h        |  28 ++++
    >  sys/arch/amd64/include/specialreg.h |   8 +
    >  sys/arch/amd64/include/vmmvar.h     |   3 +-
    >  7 files changed, 348 insertions(+), 1 deletion(-)
    >  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..0083444de72 100644
    > --- a/sys/arch/amd64/amd64/ghcb.c
    > +++ b/sys/arch/amd64/amd64/ghcb.c
    > @@ -434,3 +434,61 @@ _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.
    > + */
    > +int
    > +ghcb_psc_vmgexit(struct ghcb_psc *psc, size_t sz)
    > +{
    > +	struct ghcb_sa	*ghcb;
    > +	uint64_t		 s, sw_exitinfo1, sw_exitinfo2;
    > +	uint8_t		 valid_bm[GHCB_VB_SZ], expected_bm[GHCB_VB_SZ];
    > +	int		 error = 0;
    > +
    > +	if (sz > sizeof(ghcb->v_sharedbuf))
    > +		return (EINVAL);
    > +
    > +	memset(valid_bm, 0, sizeof(valid_bm));
    > +	memset(expected_bm, 0, sizeof(expected_bm));
    > +
    > +	s = intr_disable();
    > +
    > +	ghcb = (struct ghcb_sa *)ghcb_vaddr;
    > +	ghcb_clear(ghcb);
    > +	memcpy(ghcb->v_sharedbuf, psc, sz);
    > +
    > +	ghcb->v_sw_exitcode = SEV_VMGEXIT_PAGE_STATE_CHANGE;
    > +	ghcb->v_sw_exitinfo1 = 0;
    > +	ghcb->v_sw_exitinfo2 = 0;
    > +	ghcb->v_sw_scratch = ghcb_paddr + offsetof(struct ghcb_sa,
    > +	    v_sharedbuf);
    > +	ghcb_valbm_set(valid_bm, GHCB_SW_EXITCODE);
    > +	ghcb_valbm_set(valid_bm, GHCB_SW_EXITINFO1);
    > +	ghcb_valbm_set(valid_bm, GHCB_SW_EXITINFO2);
    > +	ghcb_valbm_set(valid_bm, GHCB_SW_SCRATCH);
    > +	ghcb_valbm_set(expected_bm, GHCB_SW_EXITINFO1);
    > +	ghcb_valbm_set(expected_bm, GHCB_SW_EXITINFO2);
    > +
    > +	memcpy(ghcb->valid_bitmap, valid_bm, sizeof(ghcb->valid_bitmap));
    > +
    > +	vmgexit();
    > +
    > +	memcpy(valid_bm, ghcb->valid_bitmap, sizeof(valid_bm));
    > +	sw_exitinfo1 = ghcb->v_sw_exitinfo1;
    > +	sw_exitinfo2 = ghcb->v_sw_exitinfo2;
    > +	ghcb_clear(ghcb);
    > +
    > +	if (ghcb_verify_bm(valid_bm, expected_bm))
    > +		panic("invalid hypervisor response");
    > +
    > +	if (sw_exitinfo1 != 0)
    > +		panic("page state change failed: 0x%llx 0x%llx", sw_exitinfo1,
    > +		    sw_exitinfo2);
    > +
    > +	intr_restore(s);
    > +
    > +	return (error);
    > +}
    > diff --git a/sys/arch/amd64/amd64/snp.c b/sys/arch/amd64/amd64/snp.c
    > new file mode 100644
    > index 00000000000..64a277ba19a
    > --- /dev/null
    > +++ b/sys/arch/amd64/amd64/snp.c
    > @@ -0,0 +1,234 @@
    > +/*	$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>
    > +
    > +#define SNP_PSC_PRIVATE		0x0001
    > +#define SNP_PSC_SHARED		0x0002
    > +
    > +/*
    > + * 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;
    > +	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 = 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, int early)
    > +{
    > +	paddr_t		pa;
    > +	size_t		left;
    > +	int		error;
    > +
    > +	left = end - start;
    > +	for (pa = start; pa < end; pa += PAGE_SIZE, left -= PAGE_SIZE) {
    > +		if ((error = _pvalidate(PMAP_DIRECT_MAP(pa), state)) != 0) {
    > +			/*
    > +                         * During early bootstrap it is acceptable,
    > +                         * when we initially claim a page, that is
    > +                         * already private.  Therefore the valid
    > +                         * flags was not changed in the RMP.
    > +			 *
    > +			 * However, after bootstrap, this may not happen.
    > +			 */
    > +			if (early && state == SNP_PSC_PRIVATE && error == -1)
    > +				continue;
    > +			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 "assigend" 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 unencryped access.
    > + * 2) Request the hypervisor to set "assigend" 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, int early)
    > +{
    > +	struct ghcb_psc	psc;
    > +	paddr_t		pa;
    > +	int		error;
    > +	size_t		sz, left;
    > +
    > +	left = end - start;
    > +	for (pa = start; pa < end; pa += sz, left -= sz) {
    > +		if ((pa & ~L2_FRAME) == 0 && left >= NBPD_L2)
    > +			sz = NBPD_L2;
    > +		else
    > +			sz = NBPD_L1;
    > +		memset(&psc, 0, sizeof(psc));
    > +		psc.psc_hdr.end_entry = 0;
    > +		psc.psc_entry.cur_page = 0;
    > +		psc.psc_entry.gfn = pa >> PAGE_SHIFT;
    > +		psc.psc_entry.operation = state;
    > +		psc.psc_entry.pagesize = (sz == NBPD_L2);
    > +
    > +		if (state == SNP_PSC_SHARED &&
    > +		    (error = pvalidate(pa, pa + sz, state, early)) != 0) {
    > +			panic("pvalidate failed: %d", error);
    > +		}
    > +
    > +		if ((error = ghcb_psc_vmgexit(&psc, sizeof(psc))) != 0)
    > +			panic("ghcb_psc_vmgexit() failed: %d", error);
    > +
    > +		if (state == SNP_PSC_PRIVATE &&
    > +		    (error =  pvalidate(pa, pa + sz, state, early)) != 0) {
    > +			panic("pvalidate failed: %d", error);
    > +		}
    > +	}
    > +}
    > +
    > +static void
    > +_snp_claim(paddr_t start, paddr_t end, int early)
    > +{
    > +	_snp_page_state_change(start, end, SNP_PSC_PRIVATE, early);
    > +}
    > +
    > +static void
    > +_snp_rescind(paddr_t start, paddr_t end, int early)
    > +{
    > +	_snp_page_state_change(start, end, SNP_PSC_SHARED, early);
    > +}
    > +
    > +
    > +void
    > +snp_claim_early(paddr_t start, paddr_t end)
    > +{
    > +	if (!ISSET(cpu_sev_guestmode, SEV_STAT_SNP_ACTIVE))
    > +		return;
    > +
    > +	return _snp_claim(start, end, 1);
    > +}
    > +
    > +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, 0);
    > +}
    > +
    > +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, 0);
    > +}
    > diff --git a/sys/arch/amd64/conf/files.amd64 b/sys/arch/amd64/conf/files.amd64
    > index 0c1e224f5c2..5474475bb41 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
    >  file	arch/amd64/amd64/sev_bus_space.c
    > +file	arch/amd64/amd64/snp.c
    >  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 55e184186ab..a821abdfb23 100644
    > --- a/sys/arch/amd64/include/ghcb.h
    > +++ b/sys/arch/amd64/include/ghcb.h
    > @@ -111,6 +111,22 @@ struct ghcb_sync {
    >
    >  #ifndef _LOCORE
    >
    > +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;
    >
    > @@ -129,6 +145,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 *);
    > +int	ghcb_psc_vmgexit(struct ghcb_psc *, size_t);
    >
    >  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..07dcea3a813
    > --- /dev/null
    > +++ b/sys/arch/amd64/include/snp.h
    > @@ -0,0 +1,28 @@
    > +/*	$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_
    > +
    > +#include <uvm/uvm.h>
    > +
    > +void	snp_claim_early(paddr_t, paddr_t);
    > +void	snp_claim_pages(struct pglist *);
    > +void	snp_rescind_pages(struct pglist *);
    > +
    > +#endif /* !_MACHINE_SNP_H_ */
    > diff --git a/sys/arch/amd64/include/specialreg.h b/sys/arch/amd64/include/specialreg.h
    > index 17bcc8cb29f..1a32da87ed7 100644
    > --- a/sys/arch/amd64/include/specialreg.h
    > +++ b/sys/arch/amd64/include/specialreg.h
    > @@ -431,6 +431,14 @@
    >       "\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")
    > +
    >  /* 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 25b1618ad1f..215ee20988f 100644
    > --- a/sys/arch/amd64/include/vmmvar.h
    > +++ b/sys/arch/amd64/include/vmmvar.h
    > @@ -267,10 +267,11 @@
    >  #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.53.0
    >
    
    
    
  • Mike Larkin:

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