kern/49926: [PATCH] Avoid deadlock condition in auich_read_codec() and auich_write_codec()

[rmh%freebsd.org@localhost]

>Number:         49926
>Category:       kern
>Synopsis:       [PATCH] Avoid deadlock condition in auich_read_codec() and auich_write_codec()
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun May 24 14:15:00 +0000 2015
>Originator:     Robert Millan
>Release:        HEAD
>Organization:
>Environment:
RUMP environment (on top of GNU/Linux host)
>Description:
Please consider the following fix for auich(4):

    Avoid deadlock condition in auich_read_codec() and auich_write_codec()

    When running this code in userspace (e.g. RUMP), it can be permanently interrupted at any time if the process which contains it is killed.

    If this happens in certain parts of auich_read_codec() or auich_write_codec() routines, ICH_CAS register may be left in an inconsistent state which leads to deadlock:

      - a command is expected by ICH_CAS before granting access to the driver
      - the driver has requested access and is polling ICH_CAS before issuing a command

    This commit makes these routines more permissive when run for first time, so that they can recover from this inconsistent state.

    For details on ICH_CAS semaphore, refer to Intel datasheet (page 328):

    http://www.intel.com/design/chipsets/datashts/290655.htm 


>How-To-Repeat:

>Fix:
diff --git a/sys/dev/pci/auich.c b/sys/dev/pci/auich.c
index 3038091..b2adad1 100644
--- a/sys/dev/pci/auich.c
+++ b/sys/dev/pci/auich.c
@@ -789,6 +789,19 @@ auich_read_codec(void *v, uint8_t reg, uint16_t *val)
 		ICH_CAS + sc->sc_modem_offset) & 1;
 	    DELAY(ICH_CODECIO_INTERVAL));
 
+	/*
+	 * Be permissive in first attempt. If previous instances of
+	 * this routine were interrupted precisely at this point (after
+	 * access is granted by CAS but before a command is sent),
+	 * they could have left hardware in an inconsistent state where
+	 * a command is expected and therefore semaphore wait would hit
+	 * the timeout.
+	 */
+	static int first = 1;
+	if (i <= 0 && first)
+		i = 1;
+	first = 0;
+
 	if (i > 0) {
 		*val = bus_space_read_2(sc->iot, sc->mix_ioh,
 		    reg + (sc->sc_codecnum * ICH_CODEC_OFFSET));
@@ -832,6 +845,12 @@ auich_write_codec(void *v, uint8_t reg, uint16_t val)
 		ICH_CAS + sc->sc_modem_offset) & 1;
 	    DELAY(ICH_CODECIO_INTERVAL));
 
+	/* Be permissive in first attempt (see comments in auich_read_codec) */
+	static int first = 1;
+	if (i <= 0 && first)
+		i = 1;
+	first = 0;
+
 	if (i > 0) {
 		bus_space_write_2(sc->iot, sc->mix_ioh,
 		    reg + (sc->sc_codecnum * ICH_CODEC_OFFSET), val);