On Mon, Mar 31, 2025 at 04:47:25PM +0200, Mark Kettenis wrote:
> > Date: Mon, 31 Mar 2025 15:13:27 +0200 (CEST)
> > From: Stefan Fritsch <sf@openbsd.org>
> > 
> > Hi,
> > 
> > if SEV is enabled, we need to map the pvclock page as unencrypted / shared 
> > with the hypervisor. Otherwise, the pvclock_attach() code may hang during 
> > boot due to garbage in the page.
> > 
> > Found and fix tested by bluhm@
> > 
> > ok?
> > 
> > Cheers,
> > Stefan
> > 
> > 
> > diff --git a/sys/dev/pv/pvclock.c b/sys/dev/pv/pvclock.c
> > index 994fc4a337c..f04d0e87c6b 100644
> > --- a/sys/dev/pv/pvclock.c
> > +++ b/sys/dev/pv/pvclock.c
> > @@ -31,6 +31,10 @@
> >  #include <dev/pv/pvvar.h>
> >  #include <dev/pv/pvreg.h>
> >  
> > +#ifndef PMAP_NOCRYPT
> > +#define	PMAP_NOCRYPT	0
> > +#endif
> > +
> >  uint pvclock_lastcount;
> >  
> >  struct pvclock_softc {
> > @@ -123,17 +127,19 @@ pvclock_attach(struct device *parent, struct device *self, void *aux)
> >  	paddr_t			 	 pa;
> >  	uint32_t			 version;
> >  	uint8_t				 flags;
> > +	struct vm_page			*page;
> >  
> > -	if ((sc->sc_time = km_alloc(PAGE_SIZE,
> > -	    &kv_any, &kp_zero, &kd_nowait)) == NULL) {
> > -		printf(": time page allocation failed\n");
> > -		return;
> > -	}
> > -	if (!pmap_extract(pmap_kernel(), (vaddr_t)sc->sc_time, &pa)) {
> > -		printf(": time page PA extraction failed\n");
> > -		km_free(sc->sc_time, PAGE_SIZE, &kv_any, &kp_zero);
> > -		return;
> > -	}
> > +	page = uvm_pagealloc(NULL, 0, NULL, 0);
> > +	if (page == NULL)
> > +		goto err;
> > +	sc->sc_time = km_alloc(PAGE_SIZE, &kv_any, &kp_none, &kd_nowait);
> > +	if (sc->sc_time == NULL)
> > +		goto err;
> > +
> > +	pa = page->phys_addr;
> 
> You should use VM_PAGE_TO_PHYS here.
> 
> > +	pmap_kenter_pa((vaddr_t)sc->sc_time, pa | PMAP_NOCRYPT,
> > +		PROT_READ | PROT_WRITE);
> > +	memset(sc->sc_time, 0, PAGE_SIZE);
> 
> Is that because using UVM_PGA_ZERO in uvm_pagealloc(9) would allocate
> a page that contain encrypted zeroes and therefore still contains
> garbage when viewed from the hypervisor?

When it was mapped without PMAP_NOCRYPT while zeroed, it will look
like garbarge for us when we turn off encryption.

Per default, all pages we write to, run through AES and write garbage
to memory.  That is what the hypervisor sees.  When we set PMAP_NOCRYPT,
we turn off AES encryption for the guest.  Now both guest and
hypervisor see garbage.  We have to zero it after that, so both
guest and hypervisor see zeros.

> >  	wrmsr(KVM_MSR_SYSTEM_TIME, pa | PVCLOCK_SYSTEM_TIME_ENABLE);
> >  	sc->sc_paddr = pa;
> > @@ -163,6 +169,11 @@ pvclock_attach(struct device *parent, struct device *self, void *aux)
> >  	tc_init(sc->sc_tc);
> >  
> >  	printf("\n");
> > +	return;
> > +err:
> > +	if (page)
> > +		uvm_pagefree(page);
> > +	printf(": time page allocation failed\n");
> >  }
> >  
> >  int
> > 
> >