Re: [PATCH] ppc64 iommu rewrite part 4/5

Re: [PATCH] ppc64 iommu rewrite part 4/5

[Andi Kleen]

Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:

> +#if !defined(CONFIG_PCI) || PCI_DMA_BUS_IS_PHYS

Can you make that a run time test? On x86-64 PCI_DMA_BUS_IS_PHYS is 
defined to a variable.

Thanks,
-Andi

Re: [PATCH] ppc64 iommu rewrite part 4/5

[Benjamin Herrenschmidt]

On Fri, 2004-02-27 at 23:51, Andi Kleen wrote:
> Benjamin Herrenschmidt <benh@kernel.crashing.org> writes:
> 
> > +#if !defined(CONFIG_PCI) || PCI_DMA_BUS_IS_PHYS
> 
> Can you make that a run time test? On x86-64 PCI_DMA_BUS_IS_PHYS is 
> defined to a variable.

Sure, here's a revised version:

diff -urN linux-2.5/drivers/ide/ide-probe.c linux-iommu/drivers/ide/ide-probe.c
--- linux-2.5/drivers/ide/ide-probe.c	2004-02-28 00:02:30.971890984 +1100
+++ linux-iommu/drivers/ide/ide-probe.c	2004-02-28 00:02:07.145513144 +1100
@@ -50,6 +50,7 @@
 #include <linux/spinlock.h>
 #include <linux/pci.h>
 #include <linux/kmod.h>
+#include <linux/pci.h>
 
 #include <asm/byteorder.h>
 #include <asm/irq.h>
@@ -904,6 +905,7 @@
 	request_queue_t *q;
 	ide_hwif_t *hwif = HWIF(drive);
 	int max_sectors = 256;
+	int max_sg_entries = PRD_ENTRIES;
 
 	/*
 	 *	Our default set up assumes the normal IDE case,
@@ -926,11 +928,22 @@
 		max_sectors = hwif->rqsize;
 	blk_queue_max_sectors(q, max_sectors);
 
-	/* IDE DMA can do PRD_ENTRIES number of segments. */
-	blk_queue_max_hw_segments(q, PRD_ENTRIES);
+#ifdef CONFIG_PCI
+	/* When we have an IOMMU, we may have a problem where pci_map_sg()
+	 * creates segments that don't completely match our boundary
+	 * requirements and thus need to be broken up again. Because it
+	 * doesn't align properly neither, we may actually have to break up
+	 * to more segments than what was we got in the first place, a max
+	 * worst case is twice as many.
+	 * This will be fixed once we teach pci_map_sg() about our boundary
+	 * requirements, hopefully soon
+	 */
+	if (!PCI_DMA_BUS_IS_PHYS)
+		max_sg_entries >>= 1;
+#endif /* CONFIG_PCI */
 
-	/* This is a driver limit and could be eliminated. */
-	blk_queue_max_phys_segments(q, PRD_ENTRIES);
+	blk_queue_max_hw_segments(q, max_sg_entries);
+	blk_queue_max_phys_segments(q, max_sg_entries);
 
 	/* assign drive and gendisk queue */
 	drive->queue = q;

[PATCH] ppc64 iommu rewrite part 4/5

[Benjamin Herrenschmidt]

Fix drivers/ide when using an IOMMU

# This is a BitKeeper generated diff -Nru style patch.
#
# ChangeSet
#   2004/02/27 22:28:18+11:00 benh@kernel.crashing.org 
#   Make IDE advertise only 128 entries of SG table on archs with an IOMMU.
#   The current IOMMU implementations of pci_map_sg() may produce segments
#   that don't match the boundary requirements of IDE, thus causing the
#   driver to break them up. The BIO is supposed to account for that,
#   however, it cannot account for a pci_map_sg producing a segment of
#   the requested size, but with incorrect alignement, thus we may still
#   try to break up the list in more entries than is supported by the HW.
#   A similar fix already went in libata. The "real" long term fix will be
#   to move the boundary requirements to struct device so that pci_map_sg()
#   can respect them when producing the sglist. In the meantime, this
#   band-aid works around the problem.
# 
# drivers/ide/ide-probe.c
#   2004/02/27 22:28:06+11:00 benh@kernel.crashing.org +16 -1
#   Make IDE advertise only 128 entries of SG table on archs with an IOMMU.
#   The current IOMMU implementations of pci_map_sg() may produce segments
#   that don't match the boundary requirements of IDE, thus causing the
#   driver to break them up. The BIO is supposed to account for that,
#   however, it cannot account for a pci_map_sg producing a segment of
#   the requested size, but with incorrect alignement, thus we may still
#   try to break up the list in more entries than is supported by the HW.
#   A similar fix already went in libata. The "real" long term fix will be
#   to move the boundary requirements to struct device so that pci_map_sg()
#   can respect them when producing the sglist. In the meantime, this
#   band-aid works around the problem.
# 
diff -Nru a/drivers/ide/ide-probe.c b/drivers/ide/ide-probe.c
--- a/drivers/ide/ide-probe.c	Fri Feb 27 22:44:36 2004
+++ b/drivers/ide/ide-probe.c	Fri Feb 27 22:44:36 2004
@@ -50,6 +50,7 @@
 #include <linux/spinlock.h>
 #include <linux/pci.h>
 #include <linux/kmod.h>
+#include <linux/pci.h>
 
 #include <asm/byteorder.h>
 #include <asm/irq.h>
@@ -926,11 +927,25 @@
 		max_sectors = hwif->rqsize;
 	blk_queue_max_sectors(q, max_sectors);
 
-	/* IDE DMA can do PRD_ENTRIES number of segments. */
+#if !defined(CONFIG_PCI) || PCI_DMA_BUS_IS_PHYS
+	/* IDE DMA can do PRD_ENTRIES number of segments. */	
 	blk_queue_max_hw_segments(q, PRD_ENTRIES);
 
 	/* This is a driver limit and could be eliminated. */
 	blk_queue_max_phys_segments(q, PRD_ENTRIES);
+#else
+	/* When we have an IOMMU, we may have a problem where pci_map_sg()
+	 * creates segments that don't completely match our boundary
+	 * requirements and thus need to be broken up again. Because it
+	 * doesn't align properly neither, we may actually have to break up
+	 * to more segments than what was we got in the first place, a max
+	 * worst case is twice as many.
+	 * This will be fixed once we teach pci_map_sg() about our boundary
+	 * requirements, hopefully soon
+	 */
+	blk_queue_max_hw_segments(q, PRD_ENTRIES / 2);
+	blk_queue_max_phys_segments(q, PRD_ENTRIES / 2);
+#endif
 
 	/* assign drive and gendisk queue */
 	drive->queue = q;