From: Mike Larkin <mlarkin@nested.page>
Subject: Re: psp(4): Automatically load SEV firmware 3/3
To: tech@openbsd.org
Date: Fri, 25 Oct 2024 09:28:59 -0700

On Fri, Oct 25, 2024 at 03:51:11PM +0200, Hans-Jörg Höxer wrote:
> Hi,
>
> and the actual firmware loading.
>

See 1 comment below, otherwise ok mlarkin.

> Take care,
> HJ.
> ---------------------------------------------------------------
> commit 9c4c3b5c92581546d9d0c1dda8e463715eeb5306
> Author: Hans-Joerg Hoexer <hshoexer@genua.de>
> Date:   Tue Oct 1 18:49:01 2024 +0200
>
>     psp(4): Automatically load SEV firmware
>
>     When opening /dev/psp for the first time, load the SEV firmware.
>     The firmware files will be provided as ports package.  The relevant
>     firmware file is determined by CPU family and model (ci_family and
>     ci_model).
>
>     If no firmware files are installed or the right file could be
>     determined, the PSP will be initialized using the on-chip firmware.
>
> diff --git a/sys/dev/ic/psp.c b/sys/dev/ic/psp.c
> index 92459c9e06f..4836f3f69a5 100644
> --- a/sys/dev/ic/psp.c
> +++ b/sys/dev/ic/psp.c
> @@ -21,6 +21,7 @@
>  #include <sys/device.h>
>  #include <sys/pledge.h>
>  #include <sys/rwlock.h>
> +#include <sys/malloc.h>
>
>  #include <machine/bus.h>
>
> @@ -58,6 +59,11 @@ struct psp_softc {
>
>  	uint32_t		sc_flags;
>  #define PSPF_INITIALIZED	0x1
> +#define PSPF_UCODELOADED	0x2
> +#define PSPF_NOUCODE		0x4
> +
> +	u_char			*sc_ucodebuf;
> +	size_t			sc_ucodelen;
>  };
>
>  int	psp_get_pstatus(struct psp_softc *, struct psp_platform_status *);
> @@ -65,6 +71,7 @@ int	psp_init(struct psp_softc *, struct psp_init *);
>  int	psp_reinit(struct psp_softc *);
>  int	psp_match(struct device *, void *, void *);
>  void	psp_attach(struct device *, struct device *, void *);
> +void	psp_load_ucode(struct psp_softc *);
>
>  struct cfdriver psp_cd = {
>  	NULL, "psp", DV_DULL
> @@ -705,6 +712,10 @@ pspopen(dev_t dev, int flag, int mode, struct proc *p)
>  	if (sc == NULL)
>  		return (ENXIO);
>
> +#ifndef SMALL_KERNEL
> +	psp_load_ucode(sc);
> +#endif
> +
>  	if (!(sc->sc_flags & PSPF_INITIALIZED))
>  		return (psp_reinit(sc));
>
> @@ -825,3 +836,65 @@ pspsubmatch(struct device *parent, void *match, void *aux)
>  		return (0);
>  	return ((*cf->cf_attach->ca_match)(parent, cf, aux));
>  }
> +
> +#ifndef SMALL_KERNEL
> +struct ucode {
> +	uint8_t		 family;
> +	uint8_t	 	 model;
> +	const char	*uname;
> +} const psp_ucode_table[] = {
> +	{ 0x17, 0x0, "amdsev/amd_sev_fam17h_model0xh.sbin" },
> +	{ 0x17, 0x3, "amdsev/amd_sev_fam17h_model3xh.sbin" },
> +	{ 0x19, 0x0, "amdsev/amd_sev_fam19h_model0xh.sbin" },
> +	{ 0x19, 0x1, "amdsev/amd_sev_fam19h_model1xh.sbin" },
> +	{ 0, 0, NULL }
> +};
> +
> +void
> +psp_load_ucode(struct psp_softc *sc)
> +{
> +	struct psp_downloadfirmware dlfw;
> +	struct cpu_info		*ci = &cpu_info_primary;
> +	const struct ucode	*uc;
> +	int			 error;
> +
> +	if (sc->sc_flags & PSPF_UCODELOADED || sc->sc_flags & PSPF_NOUCODE ||
> +	    sc->sc_flags & PSPF_INITIALIZED)
> +		return;
> +
> +	for (uc = psp_ucode_table; uc->uname; uc++)
> +		if (ci->ci_family == uc->family &&
> +		    ((ci->ci_model & 0xf0) >> 4) == uc->model)
> +			break;
> +
> +	if (uc->uname == NULL) {
> +		sc->sc_flags |= PSPF_NOUCODE;

Do you think it would be useful to print a message about not knowing about,
and/or having firmware for this family/model? We do that for example, for some
wireless devices.

> +		return;	/* no firmware found */
> +	}
> +
> +	error = loadfirmware(uc->uname, &sc->sc_ucodebuf, &sc->sc_ucodelen);
> +	if (error) {
> +		if (error != ENOENT) {
> +			printf("%s: error %d, could not read firmware %s\n",
> +			    sc->sc_dev.dv_xname, error, uc->uname);
> +		}
> +		sc->sc_flags |= PSPF_NOUCODE;
> +		return;
> +	}
> +
> +	bzero(&dlfw, sizeof(dlfw));
> +	dlfw.fw_len = sc->sc_ucodelen;
> +	dlfw.fw_paddr = (uint64_t)sc->sc_ucodebuf;
> +
> +	if (psp_downloadfirmware(sc, &dlfw) < 0)
> +		goto out;
> +
> +	sc->sc_flags |= PSPF_UCODELOADED;
> +out:
> +	if (sc->sc_ucodebuf) {
> +		free(sc->sc_ucodebuf, M_DEVBUF, sc->sc_ucodelen);
> +		sc->sc_ucodebuf = NULL;
> +		sc->sc_ucodelen = 0;
> +	}
> +}
> +#endif	/* !SMALL_KERNEL */