Re: BUG in drivers/dma/ioat/dma_v2.c:314

[Dan Williams]

On 7/1/2010 1:15 AM, Woodhouse, David wrote:
> On Thu, 2010-07-01 at 08:26 +0100, Williams, Dan J wrote:
> > On 7/1/2010 12:12 AM, Woodhouse, David wrote:
> > > On Thu, 2010-07-01 at 07:51 +0100, Williams, Dan J wrote:
> > > > This version of the device only exists on the 5400 chipset and always
> > > > has its own iommu, but since other platforms get the DMAR entry right I
> > > > think this hammer is too big?  Wouldn't this break VT-d operation on
> > > > non-busted platforms?
> > >
> > > That just means we have to get the quirk right. Does 'this version' of
> > > the device have its own PCI ID? We can always fall back to checking the
> > > ID of the device at 0000:00:00.0 to check which chipset we're on.
> >
> > PCI_DEVICE_ID_INTEL_IOAT_SNB only exists on this chipset
>
> Something like this, then?

Thanks David!

Chris, attached is a combined patch with David's catch for the VT-d 
misconfiguration, and some code to more gracefully handle this init 
failure in the driver.  Can you see if this resolves the problem? 
(remove the prior patches I have sent).

Thanks,
Dan

diff --git a/drivers/dma/ioat/dma.h b/drivers/dma/ioat/dma.h
index 6d3a73b..5216c8a 100644
--- a/drivers/dma/ioat/dma.h
+++ b/drivers/dma/ioat/dma.h
@@ -97,6 +97,7 @@ struct ioat_chan_common {
 	#define IOAT_RESET_PENDING 2
 	#define IOAT_KOBJ_INIT_FAIL 3
 	#define IOAT_RESHAPE_PENDING 4
+	#define IOAT_RUN 5
 	struct timer_list timer;
 	#define COMPLETION_TIMEOUT msecs_to_jiffies(100)
 	#define IDLE_TIMEOUT msecs_to_jiffies(2000)
diff --git a/drivers/dma/ioat/dma_v2.c b/drivers/dma/ioat/dma_v2.c
index 3c8b32a..779f4da 100644
--- a/drivers/dma/ioat/dma_v2.c
+++ b/drivers/dma/ioat/dma_v2.c
@@ -287,7 +287,10 @@ void ioat2_timer_event(unsigned long data)
 			chanerr = readl(chan->reg_base + IOAT_CHANERR_OFFSET);
 			dev_err(to_dev(chan), "%s: Channel halted (%x)\n",
 				__func__, chanerr);
-			BUG_ON(is_ioat_bug(chanerr));
+			if (test_bit(IOAT_RUN, &chan->state))
+				BUG_ON(is_ioat_bug(chanerr));
+			else /* we never got off the ground */
+				return;
 		}
 
 		/* if we haven't made progress and we have already
@@ -492,6 +495,8 @@ static struct ioat_ring_ent **ioat2_alloc_ring(struct dma_chan *c, int order, gf
 	return ring;
 }
 
+void ioat2_free_chan_resources(struct dma_chan *c);
+
 /* ioat2_alloc_chan_resources - allocate/initialize ioat2 descriptor ring
  * @chan: channel to be initialized
  */
@@ -500,6 +505,7 @@ int ioat2_alloc_chan_resources(struct dma_chan *c)
 	struct ioat2_dma_chan *ioat = to_ioat2_chan(c);
 	struct ioat_chan_common *chan = &ioat->base;
 	struct ioat_ring_ent **ring;
+	u64 status;
 	int order;
 
 	/* have we already been set up? */
@@ -540,7 +546,20 @@ int ioat2_alloc_chan_resources(struct dma_chan *c)
 	tasklet_enable(&chan->cleanup_task);
 	ioat2_start_null_desc(ioat);
 
-	return 1 << ioat->alloc_order;
+	/* check that we got off the ground */
+	udelay(5);
+	status = ioat_chansts(chan);
+	if (is_ioat_active(status) || is_ioat_idle(status)) {
+		set_bit(IOAT_RUN, &chan->state);
+		return 1 << ioat->alloc_order;
+	} else {
+		u32 chanerr = readl(chan->reg_base + IOAT_CHANERR_OFFSET);
+
+		dev_err(to_dev(chan), "failed to start channel chanerr: %#x\n",
+			chanerr);
+		ioat2_free_chan_resources(c);
+		return -EFAULT;
+	}
 }
 
 bool reshape_ring(struct ioat2_dma_chan *ioat, int order)
@@ -778,6 +797,7 @@ void ioat2_free_chan_resources(struct dma_chan *c)
 	del_timer_sync(&chan->timer);
 	device->cleanup_fn((unsigned long) c);
 	device->reset_hw(chan);
+	clear_bit(IOAT_RUN, &chan->state);
 
 	spin_lock_bh(&chan->cleanup_lock);
 	spin_lock_bh(&ioat->prep_lock);
diff --git a/drivers/dma/ioat/dma_v3.c b/drivers/dma/ioat/dma_v3.c
index 1cdd22e..d0f4990 100644
--- a/drivers/dma/ioat/dma_v3.c
+++ b/drivers/dma/ioat/dma_v3.c
@@ -361,7 +361,10 @@ static void ioat3_timer_event(unsigned long data)
 			chanerr = readl(chan->reg_base + IOAT_CHANERR_OFFSET);
 			dev_err(to_dev(chan), "%s: Channel halted (%x)\n",
 				__func__, chanerr);
-			BUG_ON(is_ioat_bug(chanerr));
+			if (test_bit(IOAT_RUN, &chan->state))
+				BUG_ON(is_ioat_bug(chanerr));
+			else /* we never got off the ground */
+				return;
 		}
 
 		/* if we haven't made progress and we have already
diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 0a19708..24ac178 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -543,8 +543,20 @@ dmar_find_matched_drhd_unit(struct pci_dev *dev)
 				    header);
 
 		if (dmaru->include_all &&
-		    drhd->segment == pci_domain_nr(dev->bus))
+		    drhd->segment == pci_domain_nr(dev->bus)) {
+			/* We know that this device on this chipset has its own
+			   IOMMU. If we find it under the catch-all IOMMU, then
+			   the BIOS is lying to us. Hope that the IOMMU for
+			   this device is actually disabled, and it needs no
+			   translation... */
+			if (dev->vendor == PCI_VENDOR_ID_INTEL &&
+			    dev->device == PCI_DEVICE_ID_INTEL_IOAT_SNB) {
+				WARN_TAINT_ONCE(1, TAINT_FIRMWARE_WORKAROUND,
+						"BIOS wrongly included I/OAT device under catch-all VT-d unit\n");
+				return NULL;
+			}
 			return dmaru;
+		}
 
 		if (dmar_pci_device_match(dmaru->devices,
 					  dmaru->devices_cnt, dev))