Skip to contentSkip to conversation

support 64 bit pci_alloc_consistent

PATCH In review Jes Sorensen 6 participants 14 messages May 18, 2003 – May 19, 2003

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 support 64 bit pci_alloc_consistent

Hi Linus,

This is patch which provides support for 64 bit address allocations from pci_alloc_consistent(), based on the address mask set through pci_set_consistent_dma_mask(). This is necessary on some platforms which are unable to provide physical memory in the lower 4GB block and do not provide IOMMU support for cards operating in certain bus modes, such as PCI-X on the SGI SN2.

The default mask for pci_alloc_consistent() is still 32 bit as there are 64 bit capable hardware out there that doesn't support 64 bit addresses for descripters etc. Likewise, platforms which provide IOMMU support in all bus modes can ignore struct pci_dev->consistent_dma_mask and just return a 32 bit address as before.

The patch also includes changes to tg3.c to make it use the new api as well as a documentation update. I have done my best on the documentation part, if anyone feel the can make my scribbles clearer, please do.

Thanks to Dave Miller, Grant Grundler, James Bottomley, Colin Ngam, and Jeremy Higdon for input and code/documentation portions.

This patch should apply cleanly to 2.5.69-bk12.

Thanks,
Jes

Documentation/DMA-mapping.txt +51 -13
diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/Documentation/DMA-mapping.txt linux-2.5.69-pci/Documentation/DMA-mapping.txt
@@ -83,6 +83,15 @@
 to be increased.  And for a device with limitations, as discussed in
 the previous paragraph, it needs to be decreased.
 
+pci_alloc_consistent() by default will return 32-bit DMA addresses.
+PCI-X specification requires PCI-X devices to support 64-bit
+addressing (DAC) for all transactions. And at least one platform (SGI
+SN2) requires 64-bit consistent allocations to operate correctly when
+the IO bus is in PCI-X mode. Therefore, like with pci_set_dma_mask(),
+it's good practice to call pci_set_consistent_dma_mask() to set the
+appropriate mask even if your device only supports 32-bit DMA
+(default) and especially if it's a PCI-X device.
+
 For correct operation, you must interrogate the PCI layer in your
 device probe routine to see if the PCI controller on the machine can
 properly support the DMA addressing limitation your device has.  It is
@@ -94,6 +103,11 @@
 
 	int pci_set_dma_mask(struct pci_dev *pdev, u64 device_mask);
 
+The query for consistent allocations is performed via a a call to
+pci_set_consistent_dma_mask():
+
+	int pci_set_consistent_dma_mask(struct pci_dev *pdev, u64 device_mask);
+
 Here, pdev is a pointer to the PCI device struct of your device, and
 device_mask is a bit mask describing which bits of a PCI address your
 device supports.  It returns zero if your card can perform DMA
@@ -133,7 +147,7 @@
 Sparc64 is one platform which behaves in this way.
 
 Here is how you would handle a 64-bit capable device which can drive
-all 64-bits during a DAC cycle:
+all 64-bits when accessing streaming DMA:
 
 	int using_dac;
 
@@ -147,6 +161,30 @@
 		goto ignore_this_device;
 	}
 
+If a card is capable of using 64-bit consistent allocations as well,
+the case would look like this:
+
+	int using_dac, consistent_using_dac;
+
+	if (!pci_set_dma_mask(pdev, 0xffffffffffffffff)) {
+		using_dac = 1;
+	   	consistent_using_dac = 1;
+		pci_set_consistent_dma_mask(pdev, 0xffffffffffffffff)
+	} else if (!pci_set_dma_mask(pdev, 0xffffffff)) {
+		using_dac = 0;
+		consistent_using_dac = 0;
+		pci_set_consistent_dma_mask(pdev, 0xffffffff)
+	} else {
+		printk(KERN_WARNING
+		       "mydev: No suitable DMA available.\n");
+		goto ignore_this_device;
+	}
+
+pci_set_consistent_dma_mask() will always be able to set the same or a
+smaller mask as pci_set_dma_mask(). However for the rare case that a
+device driver only uses consistent allocations, one would have to
+check the return value from pci_set_consistent().
+
 If your 64-bit device is going to be an enormous consumer of DMA
 mappings, this can be problematic since the DMA mappings are a
 finite resource on many platforms.  Please see the "DAC Addressing
@@ -215,9 +253,10 @@
 
   Think of "consistent" as "synchronous" or "coherent".
 
-  Consistent DMA mappings are always SAC addressable.  That is
-  to say, consistent DMA addresses given to the driver will always
-  be in the low 32-bits of the PCI bus space.
+  The current default is to return consistent memory in the low 32
+  bits of the PCI bus space.  However, for future compatibility you
+  should set the consistent mask even if this default is fine for your
+  driver.
 
   Good examples of what to use consistent mappings for are:
 
@@ -287,15 +326,14 @@
 driver needs regions sized smaller than a page, you may prefer using
 the pci_pool interface, described below.
 
-The consistent DMA mapping interfaces, for non-NULL dev, will always
-return a DMA address which is SAC (Single Address Cycle) addressable.
-Even if the device indicates (via PCI dma mask) that it may address
-the upper 32-bits and thus perform DAC cycles, consistent allocation
-will still only return 32-bit PCI addresses for DMA.  This is true
-of the pci_pool interface as well.
-
-In fact, as mentioned above, all consistent memory provided by the
-kernel DMA APIs are always SAC addressable.
+The consistent DMA mapping interfaces, for non-NULL dev, will by
+default return a DMA address which is SAC (Single Address Cycle)
+addressable.  Even if the device indicates (via PCI dma mask) that it
+may address the upper 32-bits and thus perform DAC cycles, consistent
+allocation will only return > 32-bit PCI addresses for DMA if the
+consistent dma mask has been explicitly changed via
+pci_set_consistent_dma_mask().  This is true of the pci_pool interface
+as well.
 
 pci_alloc_consistent returns two values: the virtual address which you
 can use to access it from the CPU and dma_handle which you pass to the
drivers/net/tg3.c +7 -2
diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/drivers/net/tg3.c linux-2.5.69-pci/drivers/net/tg3.c
@@ -6400,8 +6400,7 @@
 	tw32(BUFMGR_MODE, 0);
 	tw32(FTQ_RESET, 0);
 
-	/* pci_alloc_consistent gives only non-DAC addresses */
-	test_desc.addr_hi = 0;
+	test_desc.addr_hi = ((u64) buf_dma) >> 32;
 	test_desc.addr_lo = buf_dma & 0xffffffff;
 	test_desc.nic_mbuf = 0x00002100;
 	test_desc.len = size;
@@ -6743,6 +6742,12 @@
 	/* Configure DMA attributes. */
 	if (!pci_set_dma_mask(pdev, (u64) 0xffffffffffffffff)) {
 		pci_using_dac = 1;
+		if (pci_set_consistent_dma_mask(pdev,
+						(u64) 0xffffffffffffffff)) {
+			printk(KERN_ERR PFX "Unable to obtain 64 bit DMA "
+			       "for consistent allocations\n");
+			goto err_out_free_res;
+		}
 	} else {
 		err = pci_set_dma_mask(pdev, (u64) 0xffffffff);
 		if (err) {
drivers/pci/pci.c +12 -0
diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/drivers/pci/pci.c linux-2.5.69-pci/drivers/pci/pci.c
@@ -701,6 +701,17 @@
 	return 0;
 }
 
+int
+pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask)
+{
+	if (!pci_dma_supported(dev, mask))
+		return -EIO;
+
+	dev->consistent_dma_mask = mask;
+
+	return 0;
+}
+     
 static int __devinit pci_init(void)
 {
 	struct pci_dev *dev;
@@ -751,6 +762,7 @@
 EXPORT_SYMBOL(pci_clear_mwi);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_dac_set_dma_mask);
+EXPORT_SYMBOL(pci_set_consistent_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
 EXPORT_SYMBOL(pci_find_parent_resource);
 
drivers/pci/probe.c +1 -0
diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/drivers/pci/probe.c linux-2.5.69-pci/drivers/pci/probe.c
@@ -502,6 +502,7 @@
 	/* Assume 32-bit PCI; let 64-bit PCI cards (which are far rarer)
 	   set this higher, assuming the system even supports it.  */
 	dev->dma_mask = 0xffffffff;
+	dev->consistent_dma_mask = 0xffffffff;
 	if (pci_setup_device(dev) < 0) {
 		kfree(dev);
 		return NULL;
include/linux/pci.h +6 -0
diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/include/linux/pci.h linux-2.5.69-pci/include/linux/pci.h
@@ -390,6 +390,11 @@
 					   or supports 64-bit transfers.  */
 	struct list_head pools;		/* pci_pools tied to this device */
 
+	u64		consistent_dma_mask;/* Like dma_mask, but for
+					       pci_alloc_consistent mappings as
+					       not all hardware supports
+					       64 bit addresses for consistent
+					       allocations such descriptors. */
 	u32             current_state;  /* Current operating state. In ACPI-speak,
 					   this is D0-D3, D0 being fully functional,
 					   and D3 being off. */
@@ -614,6 +619,7 @@
 void pci_clear_mwi(struct pci_dev *dev);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_dac_set_dma_mask(struct pci_dev *dev, u64 mask);
+int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_assign_resource(struct pci_dev *dev, int i);
 
 /* Power management related routines */
#1 lore raw
Quoted 3 lines · Jes Sorensen

This is patch which provides support for 64 bit address allocations from pci_alloc_consistent(), based on the address mask set through pci_set_consistent_dma_mask().

I fully support these changes.

#2 lore raw
In reply to Jes Sorensen

Jes,

Thumbs up here, as I will need this for 47bit DMA mapping for SATA/SAS.

Please adopt!

Andre Hedrick
LAD Storage Consulting Group

Quoted 232 lines · Jes Sorensen

Hi Linus,

This is patch which provides support for 64 bit address allocations from pci_alloc_consistent(), based on the address mask set through pci_set_consistent_dma_mask(). This is necessary on some platforms which are unable to provide physical memory in the lower 4GB block and do not provide IOMMU support for cards operating in certain bus modes, such as PCI-X on the SGI SN2.

The default mask for pci_alloc_consistent() is still 32 bit as there are 64 bit capable hardware out there that doesn't support 64 bit addresses for descripters etc. Likewise, platforms which provide IOMMU support in all bus modes can ignore struct pci_dev->consistent_dma_mask and just return a 32 bit address as before.

The patch also includes changes to tg3.c to make it use the new api as well as a documentation update. I have done my best on the documentation part, if anyone feel the can make my scribbles clearer, please do.

Thanks to Dave Miller, Grant Grundler, James Bottomley, Colin Ngam, and Jeremy Higdon for input and code/documentation portions.

This patch should apply cleanly to 2.5.69-bk12.

Thanks,
Jes

diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/Documentation/DMA-mapping.txt linux-2.5.69-pci/Documentation/DMA-mapping.txt

Documentation/DMA-mapping.txt +51 -13
@@ -83,6 +83,15 @@
 to be increased.  And for a device with limitations, as discussed in
 the previous paragraph, it needs to be decreased.
 
+pci_alloc_consistent() by default will return 32-bit DMA addresses.
+PCI-X specification requires PCI-X devices to support 64-bit
+addressing (DAC) for all transactions. And at least one platform (SGI
+SN2) requires 64-bit consistent allocations to operate correctly when
+the IO bus is in PCI-X mode. Therefore, like with pci_set_dma_mask(),
+it's good practice to call pci_set_consistent_dma_mask() to set the
+appropriate mask even if your device only supports 32-bit DMA
+(default) and especially if it's a PCI-X device.
+
 For correct operation, you must interrogate the PCI layer in your
 device probe routine to see if the PCI controller on the machine can
 properly support the DMA addressing limitation your device has.  It is
@@ -94,6 +103,11 @@
 
 	int pci_set_dma_mask(struct pci_dev *pdev, u64 device_mask);
 
+The query for consistent allocations is performed via a a call to
+pci_set_consistent_dma_mask():
+
+	int pci_set_consistent_dma_mask(struct pci_dev *pdev, u64 device_mask);
+
 Here, pdev is a pointer to the PCI device struct of your device, and
 device_mask is a bit mask describing which bits of a PCI address your
 device supports.  It returns zero if your card can perform DMA
@@ -133,7 +147,7 @@
 Sparc64 is one platform which behaves in this way.
 
 Here is how you would handle a 64-bit capable device which can drive
-all 64-bits during a DAC cycle:
+all 64-bits when accessing streaming DMA:
 
 	int using_dac;
 
@@ -147,6 +161,30 @@
 		goto ignore_this_device;
 	}
 
+If a card is capable of using 64-bit consistent allocations as well,
+the case would look like this:
+
+	int using_dac, consistent_using_dac;
+
+	if (!pci_set_dma_mask(pdev, 0xffffffffffffffff)) {
+		using_dac = 1;
+	   	consistent_using_dac = 1;
+		pci_set_consistent_dma_mask(pdev, 0xffffffffffffffff)
+	} else if (!pci_set_dma_mask(pdev, 0xffffffff)) {
+		using_dac = 0;
+		consistent_using_dac = 0;
+		pci_set_consistent_dma_mask(pdev, 0xffffffff)
+	} else {
+		printk(KERN_WARNING
+		       "mydev: No suitable DMA available.\n");
+		goto ignore_this_device;
+	}
+
+pci_set_consistent_dma_mask() will always be able to set the same or a
+smaller mask as pci_set_dma_mask(). However for the rare case that a
+device driver only uses consistent allocations, one would have to
+check the return value from pci_set_consistent().
+
 If your 64-bit device is going to be an enormous consumer of DMA
 mappings, this can be problematic since the DMA mappings are a
 finite resource on many platforms.  Please see the "DAC Addressing
@@ -215,9 +253,10 @@
 
   Think of "consistent" as "synchronous" or "coherent".
 
-  Consistent DMA mappings are always SAC addressable.  That is
-  to say, consistent DMA addresses given to the driver will always
-  be in the low 32-bits of the PCI bus space.
+  The current default is to return consistent memory in the low 32
+  bits of the PCI bus space.  However, for future compatibility you
+  should set the consistent mask even if this default is fine for your
+  driver.
 
   Good examples of what to use consistent mappings for are:
 
@@ -287,15 +326,14 @@
 driver needs regions sized smaller than a page, you may prefer using
 the pci_pool interface, described below.
 
-The consistent DMA mapping interfaces, for non-NULL dev, will always
-return a DMA address which is SAC (Single Address Cycle) addressable.
-Even if the device indicates (via PCI dma mask) that it may address
-the upper 32-bits and thus perform DAC cycles, consistent allocation
-will still only return 32-bit PCI addresses for DMA.  This is true
-of the pci_pool interface as well.
-
-In fact, as mentioned above, all consistent memory provided by the
-kernel DMA APIs are always SAC addressable.
+The consistent DMA mapping interfaces, for non-NULL dev, will by
+default return a DMA address which is SAC (Single Address Cycle)
+addressable.  Even if the device indicates (via PCI dma mask) that it
+may address the upper 32-bits and thus perform DAC cycles, consistent
+allocation will only return > 32-bit PCI addresses for DMA if the
+consistent dma mask has been explicitly changed via
+pci_set_consistent_dma_mask().  This is true of the pci_pool interface
+as well.
 
 pci_alloc_consistent returns two values: the virtual address which you
 can use to access it from the CPU and dma_handle which you pass to the

diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/drivers/net/tg3.c linux-2.5.69-pci/drivers/net/tg3.c

drivers/net/tg3.c +7 -2
@@ -6400,8 +6400,7 @@
 	tw32(BUFMGR_MODE, 0);
 	tw32(FTQ_RESET, 0);
 
-	/* pci_alloc_consistent gives only non-DAC addresses */
-	test_desc.addr_hi = 0;
+	test_desc.addr_hi = ((u64) buf_dma) >> 32;
 	test_desc.addr_lo = buf_dma & 0xffffffff;
 	test_desc.nic_mbuf = 0x00002100;
 	test_desc.len = size;
@@ -6743,6 +6742,12 @@
 	/* Configure DMA attributes. */
 	if (!pci_set_dma_mask(pdev, (u64) 0xffffffffffffffff)) {
 		pci_using_dac = 1;
+		if (pci_set_consistent_dma_mask(pdev,
+						(u64) 0xffffffffffffffff)) {
+			printk(KERN_ERR PFX "Unable to obtain 64 bit DMA "
+			       "for consistent allocations\n");
+			goto err_out_free_res;
+		}
 	} else {
 		err = pci_set_dma_mask(pdev, (u64) 0xffffffff);
 		if (err) {

diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/drivers/pci/pci.c linux-2.5.69-pci/drivers/pci/pci.c

drivers/pci/pci.c +12 -0
@@ -701,6 +701,17 @@
 	return 0;
 }
 
+int
+pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask)
+{
+	if (!pci_dma_supported(dev, mask))
+		return -EIO;
+
+	dev->consistent_dma_mask = mask;
+
+	return 0;
+}
+     
 static int __devinit pci_init(void)
 {
 	struct pci_dev *dev;
@@ -751,6 +762,7 @@
 EXPORT_SYMBOL(pci_clear_mwi);
 EXPORT_SYMBOL(pci_set_dma_mask);
 EXPORT_SYMBOL(pci_dac_set_dma_mask);
+EXPORT_SYMBOL(pci_set_consistent_dma_mask);
 EXPORT_SYMBOL(pci_assign_resource);
 EXPORT_SYMBOL(pci_find_parent_resource);
 

diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/drivers/pci/probe.c linux-2.5.69-pci/drivers/pci/probe.c

drivers/pci/probe.c +1 -0
@@ -502,6 +502,7 @@
 	/* Assume 32-bit PCI; let 64-bit PCI cards (which are far rarer)
 	   set this higher, assuming the system even supports it.  */
 	dev->dma_mask = 0xffffffff;
+	dev->consistent_dma_mask = 0xffffffff;
 	if (pci_setup_device(dev) < 0) {
 		kfree(dev);
 		return NULL;

diff -urN -X /home/jes/exclude-linux linux-2.5.69-030509-vanilla/include/linux/pci.h linux-2.5.69-pci/include/linux/pci.h

include/linux/pci.h +6 -1
@@ -390,6 +390,11 @@
 					   or supports 64-bit transfers.  */
 	struct list_head pools;		/* pci_pools tied to this device */
 
+	u64		consistent_dma_mask;/* Like dma_mask, but for
+					       pci_alloc_consistent mappings as
+					       not all hardware supports
+					       64 bit addresses for consistent
+					       allocations such descriptors. */
 	u32             current_state;  /* Current operating state. In ACPI-speak,
 					   this is D0-D3, D0 being fully functional,
 					   and D3 being off. */
@@ -614,6 +619,7 @@
 void pci_clear_mwi(struct pci_dev *dev);
 int pci_set_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_dac_set_dma_mask(struct pci_dev *dev, u64 mask);
+int pci_set_consistent_dma_mask(struct pci_dev *dev, u64 mask);
 int pci_assign_resource(struct pci_dev *dev, int i);
 
 /* Power management related routines */
 
-

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#3 lore raw
In reply to Jes Sorensen
Quoted 8 lines · Jes Sorensen

Hi Linus,

This is patch which provides support for 64 bit address allocations from pci_alloc_consistent(), based on the address mask set through pci_set_consistent_dma_mask(). This is necessary on some platforms which are unable to provide physical memory in the lower 4GB block and do not provide IOMMU support for cards operating in certain bus modes, such as PCI-X on the SGI SN2.

I rather see a slightly different interface for this all together. Right now the template for doing this is that the driver needs to check the return value of the "set to 64 bit" operation and itself fall back to 32 bit etc. What the driver really wants to achieve is to announce device capabilities. The current interface is sort of also used to retrieve the capability as well, which is a whole different thing.

An interface like

#define PCI_DMA_64BIT 0xffffffffffffffffULL
#define PCI_DMA_32BIT 0xffffffffULL

void pci_set_dma_capabilities(device,

               u64 streaming_mask, u64 persistent_mask);

u64 pci_get_effective_streaming_mask(device);
u64 pci_get_effective_persistent_mask(device);

if for some reason the architecture PCI code needs or wants to reduce the DMA mask (for example on non-PAE36 x86 kernels) it now doesn't need to return failure for the 64 bit mask (and maybe even the 32 bit one) but it can just do it. All places in drivers that actually care about the resulting, effective DMA mask now have an interface to get this. Why this interface? I think it fits closer to what drivers use it for; uncomplicated announcing of the hardware's capabilities and independent checking of the effective mask, for example for the decision about what DMA descriptor model to use in some communications ringbuffer.

Greetings,
Arjan van de Ven

#4 lore raw

From: Arjan van de Ven <[email protected]>
Date: 18 May 2003 11:29:02 +0200

   #define PCI_DMA_64BIT 0xffffffffffffffffULL
   #define PCI_DMA_32BIT 0xffffffffULL
   
   void pci_set_dma_capabilities(device, 
                  u64 streaming_mask, u64 persistent_mask);
   u64 pci_get_effective_streaming_mask(device);
   u64 pci_get_effective_persistent_mask(device);

if for some reason the architecture PCI code needs or wants to reduce the DMA mask

WHile logically you are correct, the probing code is going to look basically identical.

Instead of frobbing around with pci_set_dma() calls, you're going to be frobbing around with pci_get_effective*() calls and branching based upon that.

I really see no value in this. Both are effectively equivalent and the present setup has the advantage that it has existed for some time and driver authors (at least some :-) know how to use it already.

#5 lore raw
Quoted 2 lines · David S. Miller

WHile logically you are correct, the probing code is going to look basically identical.

For most drivers I don't think it will. Most drivers will just say "look I can do THIS much. I don't give a flying fish about how much of that you actually use". At least in the probing code.

In code of say a scsi driver that has to pick some dma descriptor format it's a different matter of course, but there you have to check SOMETHING, either a variable you stored during probing, or now the effective mask....

#6 lore raw
Quoted 3 lines · Arjan van de Ven

Most drivers will just say "look I
can do THIS much. I don't give a flying fish about how much of that you actually use". At least in the probing code.

The platform code needs a way to indicate the given mask will not work. Rejecting proposals by the driver seems reasonable if the driver only supports two different masks anyway (eg 64 and 32-bit).

In the case of a platform requiring 64-bit masks for consistent mappings, the platform DMA code must reject proposals for non-64-bit DMA masks. (eg PCI-X device implementing less than 64-bits)

In both cases the driver will care because it will crash the box otherwise.

grant

#7 lore raw
Quoted 9 lines · Grant Grundler
Quoted 3 lines · Arjan van de Ven

Most drivers will just say "look I
can do THIS much. I don't give a flying fish about how much of that you actually use". At least in the probing code.

The platform code needs a way to indicate the given mask will not work. Rejecting proposals by the driver seems reasonable if the driver only supports two different masks anyway (eg 64 and 32-bit).

Quoted 5 lines

In the case of a platform requiring 64-bit masks for consistent mappings, the platform DMA code must reject proposals for non-64-bit DMA masks. (eg PCI-X device implementing less than 64-bits)

In both cases the driver will care because it will crash the box otherwise.

In that case, the platform returns zero to "this much" being less than the full 64 bits implying there's no mask the platform and driver can agree on.

James

#8 lore raw
Quoted 3 lines · James Bottomley

In that case, the platform returns zero to "this much" being less than the full 64 bits implying there's no mask the platform and driver can agree on.

My point was it's better if the driver always check the return value regardless of which interface is ultimately agreed upon. (in reference to whether "no one cares a flying fish".)

If one accepts that requirement, the only improvement in Arjen's proposal is the platform DMA support can guess what might be better and make that the "effective" mask. The driver still needs to check the effective mask. I happen to agree with davem : redefining this interface in 2.5 for a trivial improvement doesn't seem reasonable at this point.

a swimming fish,
grant

#9 lore raw
Quoted 9 lines · Grant Grundler
Quoted 3 lines · James Bottomley

In that case, the platform returns zero to "this much" being less than the full 64 bits implying there's no mask the platform and driver can agree on.

My point was it's better if the driver always check the return value regardless of which interface is ultimately agreed upon. (in reference to whether "no one cares a flying fish".)

Quoted 5 lines

If one accepts that requirement, the only improvement in Arjen's proposal is the platform DMA support can guess what might be better and make that the "effective" mask. The driver still needs to check the effective mask. I happen to agree with davem : redefining this interface in 2.5 for a trivial improvement doesn't seem reasonable at this point.

Yes and no. A full bit u64 mask should never fail, so the *majority* of drivers will just set the full mask and see what they get back, not expecting a zero. Any driver setting less than the full mask would have to check the return. That would be better for most drivers (also, the if 64 bit mask else if 32 bit mask else error would be removed).

I agree its not a 2.5 must have. However, it is easy enough to thread into the dma_ interface (and that has currently few enough implementing platforms and driver users to make such a change small and fairly easy).

Also, knowing the effective mask (and it would have to be set properly on return) would be extremely useful for drivers that have weird width modes (like aic with 64 vs 39 vs 32 bit addressing in the descriptors)...it would allow me to eliminate the memory size checks in those drivers.

James

#10 lore raw
Quoted 1 line · James Bottomley

A full bit u64 mask should never fail...

OK. "never fail" in the sense that the driver has advertised a mask which equals or exceeds the platform capabilities.

Bottom line is the driver has to check the platform DMA support likes the proposed mask and adjust it's behavior accordingly. Existing API and Arjen's proposal both require that.

Quoted 4 lines

Also, knowing the effective mask (and it would have to be set properly on return) would be extremely useful for drivers that have weird width modes (like aic with 64 vs 39 vs 32 bit addressing in the descriptors)

aic driver could try all three in order of preference? "extremely useful" seems like a stretch to me.

Quoted 2 lines

...it would allow me to eliminate the memory size checks in those drivers.

I expect DMA support to determine how many bits are needed to address all of physical RAM and accept/reject the 64/39/32-bit DMA mask as appropriate. I haven't studied x86 DMA support recently. There might be valid reasons it doesn't work that way.

grant

#11 lore raw
In reply to Arjan van de Ven
Quoted 4 lines · Arjan van de Ven

An interface like

#define PCI_DMA_64BIT 0xffffffffffffffffULL
#define PCI_DMA_32BIT 0xffffffffULL

This we can add now.

Quoted 5 lines

void pci_set_dma_capabilities(device,

               u64 streaming_mask, u64 persistent_mask);

u64 pci_get_effective_streaming_mask(device);
u64 pci_get_effective_persistent_mask(device);

I see the value in this for weird mask devices, but I don't think it's a "must fix" for 2.6 since the weird mask devices can advertise a lower standard supported mask.

The aic driver, for instance, seems to have a 39 bit addressing range (it uses 32 bit addr and 32 bit len descriptors, but reduces len to 24 bits to steal the extra byte for 7 bits extra addressing). However, it is forced to request the full 64 bit address mask---I've never worked out what will happen to it on a machine with more than 512GB memory.

The lack of a coherent_mask is causing breakage on some platforms, so putting it in is a 2.6 must fix thing.

James

#12 lore raw
Quoted 4 lines · James Bottomley

(it uses 32 bit addr and 32 bit len descriptors, but reduces len to 24 bits to steal the extra byte for 7 bits extra addressing). However, it is forced to request the full 64 bit address mask---I've never worked out what will happen to it on a machine with more than 512GB memory.

Uh??? right now even in 2.4 arbitrary bitmasks are supported for non-coherent memory.

#13 lore raw
Quoted 8 lines · Arjan van de Ven
Quoted 4 lines · James Bottomley

(it uses 32 bit addr and 32 bit len descriptors, but reduces len to 24 bits to steal the extra byte for 7 bits extra addressing). However, it is forced to request the full 64 bit address mask---I've never worked out what will happen to it on a machine with more than 512GB memory.

Uh??? right now even in 2.4 arbitrary bitmasks are supported for non-coherent memory.

Ah, my mistake. I keep forgetting there are two of these drivers in the aic7xxx directory. The 79xx does have a 64 bit descriptor and sets the full width. The 7xxx doesn't and sets a partial width.

Sorry,

James