Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 22 Jul 2011 16:37:04 +0000 (UTC)
From:      Alexander Motin <mav@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r224270 - in head/sys/dev/ata: . chipsets
Message-ID:  <201107221637.p6MGb4GQ024422@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mav
Date: Fri Jul 22 16:37:04 2011
New Revision: 224270
URL: http://svn.freebsd.org/changeset/base/224270

Log:
   - Use mutex to serialize index/data register pair usage, when
  accessing SATA registers. Unserialized access under heavy load caused
  wrong speed reporting and potentially could cause device loss.
   - To free memory and other resources (including above), allocated
  during chipinit() method call on attach, add new chipdeinit() method,
  called during driver detach.
  
  Submitted by:   Andrew Boyer <aboyer@averesystems.com> (initial version)
  Approved by:	re (kib)
  MFC after:	1 week

Modified:
  head/sys/dev/ata/ata-pci.c
  head/sys/dev/ata/ata-pci.h
  head/sys/dev/ata/chipsets/ata-acard.c
  head/sys/dev/ata/chipsets/ata-acerlabs.c
  head/sys/dev/ata/chipsets/ata-intel.c
  head/sys/dev/ata/chipsets/ata-promise.c

Modified: head/sys/dev/ata/ata-pci.c
==============================================================================
--- head/sys/dev/ata/ata-pci.c	Fri Jul 22 15:37:23 2011	(r224269)
+++ head/sys/dev/ata/ata-pci.c	Fri Jul 22 16:37:04 2011	(r224270)
@@ -49,8 +49,7 @@ __FBSDID("$FreeBSD$");
 #include <dev/ata/ata-pci.h>
 #include <ata_if.h>
 
-/* local vars */
-static MALLOC_DEFINE(M_ATAPCI, "ata_pci", "ATA driver PCI");
+MALLOC_DEFINE(M_ATAPCI, "ata_pci", "ATA driver PCI");
 
 /* misc defines */
 #define IOMASK                  0xfffffffc
@@ -146,13 +145,14 @@ ata_pci_detach(device_t dev)
 	    device_delete_child(dev, children[i]);
 	free(children, M_TEMP);
     }
-
     if (ctlr->r_irq) {
 	bus_teardown_intr(dev, ctlr->r_irq, ctlr->handle);
 	bus_release_resource(dev, SYS_RES_IRQ, ctlr->r_irq_rid, ctlr->r_irq);
 	if (ctlr->r_irq_rid != ATA_IRQ_RID)
 	    pci_release_msi(dev);
     }
+    if (ctlr->chipdeinit != NULL)
+	ctlr->chipdeinit(dev);
     if (ctlr->r_res2)
 	bus_release_resource(dev, ctlr->r_type2, ctlr->r_rid2, ctlr->r_res2);
     if (ctlr->r_res1)

Modified: head/sys/dev/ata/ata-pci.h
==============================================================================
--- head/sys/dev/ata/ata-pci.h	Fri Jul 22 15:37:23 2011	(r224269)
+++ head/sys/dev/ata/ata-pci.h	Fri Jul 22 16:37:04 2011	(r224270)
@@ -55,6 +55,7 @@ struct ata_pci_controller {
     int                 channels;
     int			ichannels;
     int                 (*chipinit)(device_t);
+    int                 (*chipdeinit)(device_t);
     int                 (*suspend)(device_t);
     int                 (*resume)(device_t);
     int                 (*ch_attach)(device_t);
@@ -579,6 +580,8 @@ int ata_sii_chipinit(device_t);
 /* externs */
 extern devclass_t ata_pci_devclass;
 
+MALLOC_DECLARE(M_ATAPCI);
+
 /* macro for easy definition of all driver module stuff */
 #define ATA_DECLARE_DRIVER(dname) \
 static device_method_t __CONCAT(dname,_methods)[] = { \

Modified: head/sys/dev/ata/chipsets/ata-acard.c
==============================================================================
--- head/sys/dev/ata/chipsets/ata-acard.c	Fri Jul 22 15:37:23 2011	(r224269)
+++ head/sys/dev/ata/chipsets/ata-acard.c	Fri Jul 22 16:37:04 2011	(r224270)
@@ -59,6 +59,7 @@ struct ata_serialize {
 
 /* local prototypes */
 static int ata_acard_chipinit(device_t dev);
+static int ata_acard_chipdeinit(device_t dev);
 static int ata_acard_ch_attach(device_t dev);
 static int ata_acard_status(device_t dev);
 static int ata_acard_850_setmode(device_t dev, int target, int mode);
@@ -93,6 +94,7 @@ ata_acard_probe(device_t dev)
 
     ata_set_desc(dev);
     ctlr->chipinit = ata_acard_chipinit;
+    ctlr->chipdeinit = ata_acard_chipdeinit;
     return (BUS_PROBE_DEFAULT);
 }
 
@@ -111,7 +113,7 @@ ata_acard_chipinit(device_t dev)
 	ctlr->setmode = ata_acard_850_setmode;
 	ctlr->locking = ata_serialize;
 	serial = malloc(sizeof(struct ata_serialize),
-			      M_TEMP, M_WAITOK | M_ZERO);
+			      M_ATAPCI, M_WAITOK | M_ZERO);
 	ata_serialize_init(serial);
 	ctlr->chipset_data = serial;
     }
@@ -121,6 +123,21 @@ ata_acard_chipinit(device_t dev)
 }
 
 static int
+ata_acard_chipdeinit(device_t dev)
+{
+	struct ata_pci_controller *ctlr = device_get_softc(dev);
+	struct ata_serialize *serial;
+
+	if (ctlr->chip->cfg1 == ATP_OLD) {
+		serial = ctlr->chipset_data;
+		mtx_destroy(&serial->locked_mtx);
+		free(serial, M_ATAPCI);
+		ctlr->chipset_data = NULL;
+	}
+	return (0);
+}
+
+static int
 ata_acard_ch_attach(device_t dev)
 {
     struct ata_channel *ch = device_get_softc(dev);

Modified: head/sys/dev/ata/chipsets/ata-acerlabs.c
==============================================================================
--- head/sys/dev/ata/chipsets/ata-acerlabs.c	Fri Jul 22 15:37:23 2011	(r224269)
+++ head/sys/dev/ata/chipsets/ata-acerlabs.c	Fri Jul 22 16:37:04 2011	(r224270)
@@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$");
 
 /* local prototypes */
 static int ata_ali_chipinit(device_t dev);
+static int ata_ali_chipdeinit(device_t dev);
 static int ata_ali_ch_attach(device_t dev);
 static int ata_ali_sata_ch_attach(device_t dev);
 static void ata_ali_reset(device_t dev);
@@ -95,6 +96,7 @@ ata_ali_probe(device_t dev)
 
     ata_set_desc(dev);
     ctlr->chipinit = ata_ali_chipinit;
+    ctlr->chipdeinit = ata_ali_chipdeinit;
     return (BUS_PROBE_DEFAULT);
 }
 
@@ -122,7 +124,7 @@ ata_ali_chipinit(device_t dev)
             return 0;
 
 	/* Allocate resources for later use by channel attach routines. */
-	res = malloc(sizeof(struct ali_sata_resources), M_TEMP, M_WAITOK);
+	res = malloc(sizeof(struct ali_sata_resources), M_ATAPCI, M_WAITOK);
 	for (i = 0; i < 4; i++) {
 		rid = PCIR_BAR(i);
 		res->bars[i] = bus_alloc_resource_any(dev, SYS_RES_IOPORT, &rid,
@@ -173,6 +175,27 @@ ata_ali_chipinit(device_t dev)
 }
 
 static int
+ata_ali_chipdeinit(device_t dev)
+{
+	struct ata_pci_controller *ctlr = device_get_softc(dev);
+	struct ali_sata_resources *res;
+	int i;
+
+	if (ctlr->chip->cfg2 == ALI_SATA) {
+		res = ctlr->chipset_data;
+		for (i = 0; i < 4; i++) {
+			if (res->bars[i] != NULL) {
+				bus_release_resource(dev, SYS_RES_IOPORT,
+				    PCIR_BAR(i), res->bars[i]);
+			}
+		}
+		free(res, M_ATAPCI);
+		ctlr->chipset_data = NULL;
+	}
+	return (0);
+}
+
+static int
 ata_ali_ch_attach(device_t dev)
 {
     struct ata_pci_controller *ctlr = device_get_softc(device_get_parent(dev));

Modified: head/sys/dev/ata/chipsets/ata-intel.c
==============================================================================
--- head/sys/dev/ata/chipsets/ata-intel.c	Fri Jul 22 15:37:23 2011	(r224269)
+++ head/sys/dev/ata/chipsets/ata-intel.c	Fri Jul 22 16:37:04 2011	(r224270)
@@ -53,6 +53,7 @@ __FBSDID("$FreeBSD$");
 
 /* local prototypes */
 static int ata_intel_chipinit(device_t dev);
+static int ata_intel_chipdeinit(device_t dev);
 static int ata_intel_ch_attach(device_t dev);
 static void ata_intel_reset(device_t dev);
 static int ata_intel_old_setmode(device_t dev, int target, int mode);
@@ -85,6 +86,18 @@ static void ata_intel_31244_reset(device
 #define INTEL_6CH2	8
 #define INTEL_ICH7	16
 
+struct ata_intel_data {
+	struct mtx	lock;
+	u_char		smap[4];
+};
+
+#define ATA_INTEL_SMAP(ctlr, ch) \
+    &((struct ata_intel_data *)((ctlr)->chipset_data))->smap[(ch)->unit * 2]
+#define ATA_INTEL_LOCK(ctlr) \
+    mtx_lock(&((struct ata_intel_data *)((ctlr)->chipset_data))->lock)
+#define ATA_INTEL_UNLOCK(ctlr) \
+    mtx_unlock(&((struct ata_intel_data *)((ctlr)->chipset_data))->lock)
+
 /*
  * Intel chipset support functions
  */
@@ -206,6 +219,7 @@ ata_intel_probe(device_t dev)
 
     ata_set_desc(dev);
     ctlr->chipinit = ata_intel_chipinit;
+    ctlr->chipdeinit = ata_intel_chipdeinit;
     return (BUS_PROBE_DEFAULT);
 }
 
@@ -213,11 +227,14 @@ static int
 ata_intel_chipinit(device_t dev)
 {
     struct ata_pci_controller *ctlr = device_get_softc(dev);
+    struct ata_intel_data *data;
 
     if (ata_setup_interrupt(dev, ata_generic_intr))
 	return ENXIO;
 
-    ctlr->chipset_data = NULL;
+    data = malloc(sizeof(struct ata_intel_data), M_ATAPCI, M_WAITOK | M_ZERO);
+    mtx_init(&data->lock, "Intel SATA lock", NULL, MTX_DEF);
+    ctlr->chipset_data = (void *)data;
 
     /* good old PIIX needs special treatment (not implemented) */
     if (ctlr->chip->chipid == ATA_I82371FB) {
@@ -305,6 +322,19 @@ ata_intel_chipinit(device_t dev)
 }
 
 static int
+ata_intel_chipdeinit(device_t dev)
+{
+	struct ata_pci_controller *ctlr = device_get_softc(dev);
+	struct ata_intel_data *data;
+
+	data = ctlr->chipset_data;
+	mtx_destroy(&data->lock);
+	free(data, M_ATAPCI);
+	ctlr->chipset_data = NULL;
+	return (0);
+}
+
+static int
 ata_intel_ch_attach(device_t dev)
 {
 	struct ata_pci_controller *ctlr;
@@ -329,7 +359,7 @@ ata_intel_ch_attach(device_t dev)
 
 	ch->flags |= ATA_ALWAYS_DMASTAT;
 	if (ctlr->chip->max_dma >= ATA_SA150) {
-		smap = (u_char *)&ctlr->chipset_data + ch->unit * 2;
+		smap = ATA_INTEL_SMAP(ctlr, ch);
 		map = pci_read_config(device_get_parent(dev), 0x90, 1);
 		if (ctlr->chip->cfg1 & INTEL_ICH5) {
 			map &= 0x07;
@@ -415,7 +445,7 @@ ata_intel_reset(device_t dev)
 		return (ata_generic_reset(dev));
 
 	/* Do hard-reset on respective SATA ports. */
-	smap = (u_char *)&ctlr->chipset_data + ch->unit * 2;
+	smap = ATA_INTEL_SMAP(ctlr, ch);
 	mask = 1 << smap[0];
 	if ((ch->flags & ATA_NO_SLAVE) == 0)
 		mask |= (1 << smap[1]);
@@ -605,7 +635,7 @@ ata_intel_sata_ahci_read(device_t dev, i
 	ctlr = device_get_softc(parent);
 	ch = device_get_softc(dev);
 	port = (port == 1) ? 1 : 0;
-	smap = (u_char *)&ctlr->chipset_data + ch->unit * 2;
+	smap = ATA_INTEL_SMAP(ctlr, ch);
 	offset = 0x100 + smap[port] * 0x80;
 	switch (reg) {
 	case ATA_SSTATUS:
@@ -635,7 +665,7 @@ ata_intel_sata_cscr_read(device_t dev, i
 	parent = device_get_parent(dev);
 	ctlr = device_get_softc(parent);
 	ch = device_get_softc(dev);
-	smap = (u_char *)&ctlr->chipset_data + ch->unit * 2;
+	smap = ATA_INTEL_SMAP(ctlr, ch);
 	port = (port == 1) ? 1 : 0;
 	switch (reg) {
 	case ATA_SSTATUS:
@@ -650,9 +680,11 @@ ata_intel_sata_cscr_read(device_t dev, i
 	default:
 	    return (EINVAL);
 	}
+	ATA_INTEL_LOCK(ctlr);
 	pci_write_config(parent, 0xa0,
 	    0x50 + smap[port] * 0x10 + reg * 4, 4);
 	*result = pci_read_config(parent, 0xa4, 4);
+	ATA_INTEL_UNLOCK(ctlr);
 	return (0);
 }
 
@@ -680,8 +712,10 @@ ata_intel_sata_sidpr_read(device_t dev, 
 	default:
 	    return (EINVAL);
 	}
+	ATA_INTEL_LOCK(ctlr);
 	ATA_IDX_OUTL(ch, ATA_IDX_ADDR, ((ch->unit * 2 + port) << 8) + reg);
 	*result = ATA_IDX_INL(ch, ATA_IDX_DATA);
+	ATA_INTEL_UNLOCK(ctlr);
 	return (0);
 }
 
@@ -698,7 +732,7 @@ ata_intel_sata_ahci_write(device_t dev, 
 	ctlr = device_get_softc(parent);
 	ch = device_get_softc(dev);
 	port = (port == 1) ? 1 : 0;
-	smap = (u_char *)&ctlr->chipset_data + ch->unit * 2;
+	smap = ATA_INTEL_SMAP(ctlr, ch);
 	offset = 0x100 + smap[port] * 0x80;
 	switch (reg) {
 	case ATA_SSTATUS:
@@ -728,7 +762,7 @@ ata_intel_sata_cscr_write(device_t dev, 
 	parent = device_get_parent(dev);
 	ctlr = device_get_softc(parent);
 	ch = device_get_softc(dev);
-	smap = (u_char *)&ctlr->chipset_data + ch->unit * 2;
+	smap = ATA_INTEL_SMAP(ctlr, ch);
 	port = (port == 1) ? 1 : 0;
 	switch (reg) {
 	case ATA_SSTATUS:
@@ -743,9 +777,11 @@ ata_intel_sata_cscr_write(device_t dev, 
 	default:
 	    return (EINVAL);
 	}
+	ATA_INTEL_LOCK(ctlr);
 	pci_write_config(parent, 0xa0,
 	    0x50 + smap[port] * 0x10 + reg * 4, 4);
 	pci_write_config(parent, 0xa4, value, 4);
+	ATA_INTEL_UNLOCK(ctlr);
 	return (0);
 }
 
@@ -773,8 +809,10 @@ ata_intel_sata_sidpr_write(device_t dev,
 	default:
 	    return (EINVAL);
 	}
+	ATA_INTEL_LOCK(ctlr);
 	ATA_IDX_OUTL(ch, ATA_IDX_ADDR, ((ch->unit * 2 + port) << 8) + reg);
 	ATA_IDX_OUTL(ch, ATA_IDX_DATA, value);
+	ATA_INTEL_UNLOCK(ctlr);
 	return (0);
 }
 

Modified: head/sys/dev/ata/chipsets/ata-promise.c
==============================================================================
--- head/sys/dev/ata/chipsets/ata-promise.c	Fri Jul 22 15:37:23 2011	(r224269)
+++ head/sys/dev/ata/chipsets/ata-promise.c	Fri Jul 22 16:37:04 2011	(r224270)
@@ -280,7 +280,7 @@ ata_promise_chipinit(device_t dev)
 
 	    /* setup host packet controls */
 	    hpkt = malloc(sizeof(struct ata_promise_sx4),
-			  M_TEMP, M_NOWAIT | M_ZERO);
+			  M_ATAPCI, M_NOWAIT | M_ZERO);
 	    mtx_init(&hpkt->mtx, "ATA promise HPKT lock", NULL, MTX_DEF);
 	    TAILQ_INIT(&hpkt->queue);
 	    hpkt->busy = 0;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201107221637.p6MGb4GQ024422>