From: Tobias Heider Subject: Re: fw_update: fix ccp/qccpucp pattern conflict To: Alexander Bluhm Cc: Theo de Raadt , tech@openbsd.org Date: Tue, 20 May 2025 11:01:33 +0200 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"); >