From: Jonathan Gray <jsg@jsg.id.au>
Subject: Re: [EXT] make psp attach to ccp
To: Hans-Jörg Höxer <Hans-Joerg_Hoexer@genua.de>
Cc: tech@openbsd.org, bluhm@openbsd.org
Date: Tue, 3 Sep 2024 21:51:18 +1000

On Tue, Sep 03, 2024 at 01:26:09PM +0200, Hans-Jörg Höxer wrote:
> Hi,
> 
> I like the diff.  And with it SEV/psp/ccp are still working for me.
> 
> One issue with dmesg and a minor comment inline.
> 
> On my machine with ccp and SEV I get this output with your diff:
> 
> ...
> vendor "AMD", unknown product 0x1485 (class instrumentation unknown subclass 0x00, rev 0x00) at pci19 dev 0 function 0 not configured
> ccp0 at pci19 dev 0 function 1 "AMD 17h Crypto" rev 0x00: msixpsp0 at ccp0: SEV, SEV-ES
> 
> vendor "AMD", unknown product 0x1498 (class crypto subclass miscellaneous, rev 0x00) at pci19 dev 0 function 2 not configured
> ...
> 
> ccp0 and psp0 are printed on the same line.  And we have an extra "\n".
> That one is the printf in ccp_attach().
> 
> On a machine without psp (but two ccp) the output is sane:
> 
> ...
> vendor "AMD", unknown product 0x145a (class instrumentation unknown subclass 0x00, rev 0x00) at pci4 dev 0 function 0 not configured
> ccp0 at pci4 dev 0 function 2 "AMD 17h Crypto" rev 0x00: msix
> xhci0 at pci4 dev 0 function 3 "AMD 17h xHCI" rev 0x00: msix, xHCI 1.0
> ...
> vendor "AMD", unknown product 0x1455 (class instrumentation unknown subclass 0x00, rev 0x00) at pci5 dev 0 function 0 not configured
> ccp1 at pci5 dev 0 function 1 "AMD 17h Crypto" rev 0x00: msix
> ahci0 at pci5 dev 0 function 2 "AMD FCH AHCI" rev 0x51: msi, AHCI 1.3.1
> ...
> 
> 
> One fix might be, to add a "\n" in psp_pci_attach() after the conditional
> printf for intrstr.  And removing the "\n" ccp_attach().  However,
> this would require an printf("\n") in psp_pci_attach() when NPSP == 0.
> Maybe there is a better way to solve this.

does calling psp_pci_attach() after ccp_attach() give the right output?

diff --git sys/dev/ic/psp.c sys/dev/ic/psp.c
index e2430e17413..5448bab2f1c 100644
--- sys/dev/ic/psp.c
+++ sys/dev/ic/psp.c
@@ -109,7 +109,7 @@ psp_attach(struct device *parent, struct device *self, void *aux)
 	sc->sc_dmat = arg->dmat;
 	sc->sc_capabilities = arg->capabilities;
 
-	rw_init(&sc->sc_lock, "ccp_lock");
+	rw_init(&sc->sc_lock, "psp_lock");
 
 	/* create and map SEV command buffer */
 	sc->sc_cmd_size = size = PAGE_SIZE;
diff --git sys/dev/pci/ccp_pci.c sys/dev/pci/ccp_pci.c
index a81f455f203..f7577529a7c 100644
--- sys/dev/pci/ccp_pci.c
+++ sys/dev/pci/ccp_pci.c
@@ -79,9 +79,9 @@ ccp_pci_attach(struct device *parent, struct device *self, void *aux)
 		return;
 	}
 
-	psp_pci_attach(parent, self, aux);
-
 	ccp_attach(sc);
+
+	psp_pci_attach(parent, self, aux);
 }
 
 void