From: Mike Larkin <mlarkin@nested.page>
Subject: Re: pvclock: Map shared on SEV
To: Stefan Fritsch <sf@openbsd.org>
Cc: tech@openbsd.org, Hans-Joerg Hoexer <hshoexer@genua.de>, bluhm@openbsd.org
Date: Tue, 1 Apr 2025 17:17:47 +0000

On Mon, Mar 31, 2025 at 03:13:27PM +0200, Stefan Fritsch wrote:
> 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
>


a bit late to the party (and I see this was already committed with kettenis'
suggestion), but ok mlarkin for posterity :)

>
> 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;
> +	pmap_kenter_pa((vaddr_t)sc->sc_time, pa | PMAP_NOCRYPT,
> +		PROT_READ | PROT_WRITE);
> +	memset(sc->sc_time, 0, PAGE_SIZE);
>
>  	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
>