From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: fw_update: fix ccp/qccpucp pattern conflict
To: Theo de Raadt <deraadt@openbsd.org>
Cc: Tobias Heider <tobias.heider@stusta.de>, tech@openbsd.org
Date: Tue, 20 May 2025 17:31:52 +0900

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

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");