Index | Thread | Search

From:
Hans-Jörg Höxer <hshoexer@genua.de>
Subject:
psp(4): Avoid sleep/wakeup race by raising SPL
To:
<tech@openbsd.org>
Date:
Mon, 4 Nov 2024 17:25:22 +0100

Download raw body.

Thread
Hi,

I think we have a race in psp(4):  Writing the command register triggers
the PSP to execute the current command.  If the PSP is fast, it might
raise the interrupt before we actually go to sleep.  Thus, we miss
the wakeup(9) from the interrupt handler and tsleep(9) just times out.
This is not really a problem, nonetheless we should avoid this.

Raising the IPL just before writing the command register should ensure,
that we do not handle the interrupt before actually calling tsleep(9).
As the interrupt handle is not marked as MP safe, it runs with the kernel
lock; so raising SPL should be enough.

Take care,
HJ.
-----------------------------------------------------------------------
commit 0dc493a68ab77ae36e83f8e4ff15b3ee523b5b07
Author: Hans-Joerg Hoexer <hshoexer@genua.de>
Date:   Mon Nov 4 17:09:24 2024 +0100

    psp(4): Avoid sleep/wakeup race by raising SPL

diff --git a/sys/dev/ic/psp.c b/sys/dev/ic/psp.c
index e697b307236..a5840e03d93 100644
--- a/sys/dev/ic/psp.c
+++ b/sys/dev/ic/psp.c
@@ -223,6 +223,7 @@ static int
 ccp_docmd(struct psp_softc *sc, int cmd, uint64_t paddr)
 {
 	uint32_t	plo, phi, cmdword, status;
+	int		ret, s;
 
 	plo = ((paddr >> 0) & 0xffffffff);
 	phi = ((paddr >> 32) & 0xffffffff);
@@ -232,9 +233,12 @@ ccp_docmd(struct psp_softc *sc, int cmd, uint64_t paddr)
 
 	bus_space_write_4(sc->sc_iot, sc->sc_ioh, sc->sc_reg_addrlo, plo);
 	bus_space_write_4(sc->sc_iot, sc->sc_ioh, sc->sc_reg_addrhi, phi);
+	s = splbio();
 	bus_space_write_4(sc->sc_iot, sc->sc_ioh, sc->sc_reg_cmdresp, cmdword);
 
-	if (ccp_wait(sc, &status, cold))
+	ret = ccp_wait(sc, &status, cold);
+	splx(s);
+	if (ret)
 		return (1);
 
 	/* Did PSP sent a response code? */