On Tue, May 20, 2025 at 05:31:52PM +0900, Alexander Bluhm wrote:
> On Tue, May 20, 2025 at 04:53:51PM +0900, Alexander Bluhm wrote:
> > On Tue, May 20, 2025 at 12:46:24AM -0600, Theo de Raadt wrote:
> > > Yes, the pattern should be more strict.
> > > 
> > > Should it be ^cpp0 ?
> > 
> > I have a machine that has 2 cpp, CPUs on 1 package, 4 cores, 2 smt.
> > There psp0 attaches at ccp0.  I have never seen a psp not at ccp0.
> > 
> > ccp0 at pci4 dev 0 function 2 "AMD 17h Crypto" rev 0x00: msix
> > psp0 at ccp0: vers 1, api 0.17, build 48, SEV, SEV-ES
> > ccp1 at pci5 dev 0 function 1 "AMD 17h Crypto" rev 0x00
> > 
> > So I would say ^cpp0 is better.
> > 
> > Maybe it is also suffcient to check for ^psp0
> > 
> > When we were writing the code, psp(4) driver did not attach reliably
> > on all hardware.  We have fixed that.  We activate it only after
> > recent firmware has been loaded by vmd(8).  That means psp always
> > attaches during autoconf, no matter if it has recent firmware.
> > 
> > After reconsidering, I think it is safe to remove the ccp regex and
> > change the pattern to ^psp0 only.
> > 
> > The lines AMD*Crypto and AMD*PSP are needed when sysupgrading from
> > a kernel without driver.  Also needed with RAMDISK kernel.
> 
> I made a survey in the hackroom.  AMD laptops have ccp, but no psp.
> There SEV cannot not work.  So they should not download firmware.
> 
> Server machine with psp, but without driver writes only
> "AMD 17h Crypto" rev 0x00 at pci19 dev 0 function 1 not configured
> The psp has no PCI Id, it attaches with certain ccp.  Without
> driver you don't see psp.
> 
> Don't remember from what hardware we got the ^\"AMD*PSP\"" pattern.
> But there is some 19h/7xh PSP string in the kernel.
> 
> RAMDISK has neither ccp* nor psp*
> RAMDISK_CD has ccp*, but no psp*
> GENERIC hs ccp* and psp*
> 
> We need ^\"AMD*Crypto\"" for RAMDISK.  Maybe ^\"AMD*PSP\"" for some
> other hardware.
> With ^ccp0 we find psp on RAMDISK_CD.
> ^psp0 is the most specific pattern, but only works with GENERIC.
> 
> To suport all hardware and any install method, we need the pattern
> below.  This means we load amdsev firmware for machines that don't
> support SEV.  But we cannot figure that out before GENERIC with all
> drivers is running.
> 
> Alternative would be to add psp* to RAMDISK_CD and ignore RAMDISK
> installs.  But also in this case rc.firsttime should download the
> firmware before vmd needs it.  Then one line ^psp0 would be sufficient.
> 
> bluhm

Diff below seems to be close enough to what the previous one intended
to match on but strict enough to not match on my qccpucp driver so
ok tobhe@

> 
> Index: patterns.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/fw_update/patterns.c,v
> diff -u -p -r1.17 patterns.c
> --- patterns.c	28 Mar 2025 15:04:30 -0000	1.17
> +++ patterns.c	20 May 2025 08:24:23 -0000
> @@ -96,8 +96,8 @@ main(void)
>  	printf("%s\n", "amdgpu ^vendor \"ATI\", unknown product*class display");
>  	printf("%s\n", "amdsev ^\"AMD*Crypto\"");
>  	printf("%s\n", "amdsev ^\"AMD*PSP\"");
> -	printf("%s\n", "amdsev ccp");
> -	printf("%s\n", "amdsev psp");
> +	printf("%s\n", "amdsev ^ccp0");
> +	printf("%s\n", "amdsev ^psp0");
>  	printf("%s\n", "apple-boot ^cpu0*Apple");
>  	printf("%s\n", "arm64-qcom-dtb ^qcgpio0");
>  	printf("%s\n", "athn");
>