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.

bluhm

> > Due to an unfortunate driver name collision the current fw_update
> > matching pattern for amdsev checks for ccpX devices using just
> > ccp as pattern. This only matches on Qualcomm machines with
> > qccpucpX lines in their dmesg.
> > 
> > Maybe it would make more sense to only consider lines starting
> > with ccp?  Tested on my T14s and confirmed this make amdsev go away,
> > I'd appreciate if someone could test on an amd machine to see if it
> > still works as expected.
> > 
> > diff --git a/usr.sbin/fw_update/patterns.c b/usr.sbin/fw_update/patterns.c
> > index aa2ec49f4a2..070a994f7da 100644
> > --- a/usr.sbin/fw_update/patterns.c
> > +++ b/usr.sbin/fw_update/patterns.c
> > @@ -96,7 +96,7 @@ 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 ^ccp");
> >  	printf("%s\n", "amdsev psp");
> >  	printf("%s\n", "apple-boot ^cpu0*Apple");
> >  	printf("%s\n", "arm64-qcom-dtb ^qcgpio0");
> >