Re: New "ahcisata0 port 1: device present" messages with NetBSD 9

[Simon Burge <simonb%NetBSD.org@localhost>]

matthew green wrote (in kern/54855):

> most people with 860 EVOs are successful with NCQ, i'm one
> of them with a 500GB model, with netbsd-9 for months now,
> and they're a wildly successful line i'm sure many others
> also have working happily with NCQ.
> 
> we don't know what the real issue is.  perhaps it is a
> problem that the controller had that these drives trigger,
> rather than a problem with them, so perhaps we should
> disable it on the controller instead of the drive, if
> we're going to pick one of the two triggers here.

Izumi Tsutsui wrote:

> Note mine is:
> >> ahcisata0 at pci0 dev 18 function 0: vendor 1002 product 4380 (rev. 0x00)
>  [ ... ]
> >> 000:18:0: ATI Technologies SB600 SATA Controller (SATA mass storage, AHCI 1.0)
>
> Note it already has AHCI_QUIRK_BADPM, but the Samsung EVO 860 NCQ problem
> should be an independent quirk?
>  https://nxr.netbsd.org/xref/src/sys/dev/ic/ahcisatavar.h?r=1.23#57
>  https://nxr.netbsd.org/xref/src/sys/dev/pci/ahcisata_pci.c?r=1.56#59

Using the approach suggested by mrg, where we "penalise" the (older)
AMD chipsets that exhibit the problem rather than the more recent and
popular Samsung EVO 860 disks, I've added a new AHCI_QUIRK_BADNCQ quirk
and enabled it for the same AMD chipsets that had the AHCI_QUIRK_BADPM
quirk.  I do wonder, out of caution, if maybe this quirk should be
enabled for PCI_PRODUCT_ATI_SB600_SATA_1 as well?

Before patch:

wd0(ahcisata0:0:0): using PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133) (using DMA), NCQ (31 tags)

After patch:

wd0(ahcisata0:0:0): using PIO mode 4, DMA mode 2, Ultra-DMA mode 6 (Ultra/133) (using DMA), WRITE DMA FUA EXT

Jaromir - are you happy with this alternate approach?  If you are
happy with this I'll commit this patch and revert your EVO 860 quirk
and request a pull-up to the netbsd-9 branch.

tsutsui - are you able to test the patch too?

Cheers,
Simon.

--
Index: sys/dev/ic/ahcisata_core.c
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/ahcisata_core.c,v
retrieving revision 1.75.4.2
diff -d -p -u -r1.75.4.2 ahcisata_core.c
--- sys/dev/ic/ahcisata_core.c	24 Dec 2019 17:34:33 -0000	1.75.4.2
+++ sys/dev/ic/ahcisata_core.c	17 Jan 2020 08:45:41 -0000
@@ -276,6 +276,11 @@ ahci_attach(struct ahci_softc *sc)
 		    "ignoring broken port multiplier support\n");
 		sc->sc_ahci_cap &= ~AHCI_CAP_SPM;
 	}
+	if (sc->sc_ahci_quirks & AHCI_QUIRK_BADNCQ) {
+		aprint_verbose_dev(sc->sc_atac.atac_dev,
+		    "ignoring broken NCQ support\n");
+		sc->sc_ahci_cap &= ~AHCI_CAP_NCQ;
+	}
 	sc->sc_atac.atac_nchannels = (sc->sc_ahci_cap & AHCI_CAP_NPMASK) + 1;
 	sc->sc_ncmds = ((sc->sc_ahci_cap & AHCI_CAP_NCS) >> 8) + 1;
 	ahci_rev = AHCI_READ(sc, AHCI_VS);
Index: sys/dev/ic/ahcisatavar.h
===================================================================
RCS file: /cvsroot/src/sys/dev/ic/ahcisatavar.h,v
retrieving revision 1.22
diff -d -p -u -r1.22 ahcisatavar.h
--- sys/dev/ic/ahcisatavar.h	14 Jan 2019 21:29:56 -0000	1.22
+++ sys/dev/ic/ahcisatavar.h	17 Jan 2020 08:45:41 -0000
@@ -59,6 +59,7 @@ struct ahci_softc {
 #define AHCI_PCI_QUIRK_BAD64	__BIT(1)  /* broken 64-bit DMA */
 #define AHCI_QUIRK_BADPMP	__BIT(2)  /* broken PMP support, ignore */
 #define AHCI_QUIRK_SKIP_RESET	__BIT(4)  /* skip drive reset sequence */
+#define AHCI_QUIRK_BADNCQ	__BIT(5)  /* possibly broken NCQ support, ignore */
 
 	uint32_t sc_ahci_cap;	/* copy of AHCI_CAP */
 	int sc_ncmds; /* number of command slots */
Index: sys/dev/pci/ahcisata_pci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/pci/ahcisata_pci.c,v
retrieving revision 1.55.4.1
diff -d -p -u -r1.55.4.1 ahcisata_pci.c
--- sys/dev/pci/ahcisata_pci.c	23 Oct 2019 18:09:18 -0000	1.55.4.1
+++ sys/dev/pci/ahcisata_pci.c	17 Jan 2020 08:45:41 -0000
@@ -179,17 +179,17 @@ static const struct ahci_pci_quirk ahci_
 	    AHCI_PCI_QUIRK_FORCE },
 	/* ATI SB600 AHCI 64-bit DMA only works on some boards/BIOSes */
 	{ PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB600_SATA_1,
-	    AHCI_PCI_QUIRK_BAD64 | AHCI_QUIRK_BADPMP },
+	    AHCI_PCI_QUIRK_BAD64 | AHCI_QUIRK_BADPMP | AHCI_QUIRK_BADNCQ },
 	{ PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB700_SATA_AHCI,
-	    AHCI_QUIRK_BADPMP },
+	    AHCI_QUIRK_BADPMP | AHCI_QUIRK_BADNCQ },
 	{ PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB700_SATA_RAID,
-	    AHCI_QUIRK_BADPMP },
+	    AHCI_QUIRK_BADPMP | AHCI_QUIRK_BADNCQ },
 	{ PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB700_SATA_RAID5,
-	    AHCI_QUIRK_BADPMP },
+	    AHCI_QUIRK_BADPMP | AHCI_QUIRK_BADNCQ },
 	{ PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB700_SATA_AHCI2,
-	    AHCI_QUIRK_BADPMP },
+	    AHCI_QUIRK_BADPMP | AHCI_QUIRK_BADNCQ },
 	{ PCI_VENDOR_ATI, PCI_PRODUCT_ATI_SB700_SATA_STORAGE,
-	    AHCI_QUIRK_BADPMP },
+	    AHCI_QUIRK_BADPMP | AHCI_QUIRK_BADNCQ },
 	{ PCI_VENDOR_VIATECH, PCI_PRODUCT_VIATECH_VT8237R_SATA,
 	    AHCI_QUIRK_BADPMP },
 	{ PCI_VENDOR_VIATECH, PCI_PRODUCT_VIATECH_VT8251_SATA,