dma: override "dma_flags_set_dmaflush" for sn-ia64
PATCH 2/3 dma: override "dma_flags_set_dmaflush" for sn-ia64
Define ARCH_DOES_POSTED_DMA, and override the dma_flags_set_dmaflush function. Also define dma_flags_get_direction, dma_flags_get_dmaflush to retrieve the direction and "dmaflush attribute" from flags passed to the sn_dma_map_* functions.
--
arch/ia64/sn/pci/pci_dma.c | 35 ++++++++++++++++++++++++++--------- include/asm-ia64/sn/io.h | 26 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 9 deletions(-)
arch/ia64/sn/pci/pci_dma.c +26 -9
@@ -153,7 +153,7 @@ EXPORT_SYMBOL(sn_dma_free_coherent);* @dev: device to map for * @cpu_addr: kernel virtual address of the region to map * @size: size of the region -* @direction: DMA direction +* @flags: DMA direction, and arch-specific attributes * * Map the region pointed to by @cpu_addr for DMA and return the * DMA address.
@@ -167,17 +167,23 @@ EXPORT_SYMBOL(sn_dma_free_coherent);* figure out how to save dmamap handle so can use two step. */ dma_addr_t sn_dma_map_single (struct device *dev,void *cpu_addr,size_t size, -int direction) +int flags) {dma_addr_t dma_addr;unsigned long phys_addr;struct pci_dev *pdev =to_pci_dev (dev);struct sn_pcibus_provider *provider =SN_PCIDEV_BUSPROVIDER (pdev); +int dmaflush =dma_flags_get_dmaflush (flags);BUG_ON (dev->bus != &pci_bus_type); phys_addr =__pa (cpu_addr); - dma_addr = provider->dma_map (pdev, phys_addr, size,SN_DMA_ADDR_PHYS ); +if (dmaflush) + dma_addr = provider->dma_map_consistent (pdev, phys_addr, size, +SN_DMA_ADDR_PHYS ); +else + dma_addr = provider->dma_map (pdev, phys_addr, size, +SN_DMA_ADDR_PHYS );if (!dma_addr) {printk (KERN_ERR "%s: out of ATEs\n" , __FUNCTION__);return 0 ;
@@ -240,18 +246,20 @@ EXPORT_SYMBOL(sn_dma_unmap_sg);* @dev: device to map for * @sg: scatterlist to map * @nhwentries: number of entries -* @direction: direction of the DMA transaction +* @flags: direction of the DMA transaction, and arch-specific attributes * * Maps each entry of @sg for DMA. */ int sn_dma_map_sg (struct device *dev,struct scatterlist *sg,int nhwentries, -int direction) +int flags) {unsigned long phys_addr;struct scatterlist *saved_sg = sg;struct pci_dev *pdev =to_pci_dev (dev);struct sn_pcibus_provider *provider =SN_PCIDEV_BUSPROVIDER (pdev);int i; +int dmaflush =dma_flags_get_dmaflush (flags); +int direction =dma_flags_get_direction (flags);BUG_ON (dev->bus != &pci_bus_type);
@@ -259,12 +267,21 @@ int sn_dma_map_sg(struct device *dev, struct scatterlist *sg, int nhwentries,* Setup a DMA address for each entry in the scatterlist. */ for (i =0 ; i < nhwentries; i++, sg++) { +dma_addr_t dma_addr; phys_addr =SG_ENT_PHYS_ADDRESS (sg); - sg->dma_address = provider->dma_map (pdev, - phys_addr, sg->length, -SN_DMA_ADDR_PHYS ); -if (!sg->dma_address) { +if (dmaflush) { + dma_addr = provider->dma_map_consistent (pdev, + phys_addr, + sg->length, +SN_DMA_ADDR_PHYS ); + }else { + dma_addr = provider->dma_map (pdev, + phys_addr, sg->length, +SN_DMA_ADDR_PHYS ); + } + +if (!(sg->dma_address = dma_addr)) {printk (KERN_ERR "%s: out of ATEs\n" , __FUNCTION__);/*
include/asm-ia64/sn/io.h +26 -0
@@ -271,4 +271,30 @@ sn_pci_set_vchan(struct pci_dev *pci_dev, unsigned long *addr, int vchan) return 0; } +#define ARCH_DOES_POSTED_DMA +/* here we steal some upper bits from the "direction" argument to the +* dma_map_* routines */ +#define DMA_ATTR_SHIFT 8 +/* bottom 8 bits for direction, remaining bits for additional "attributes" */ +#define DMA_FLUSH_ATTR 0x1 +/* For now the only attribute is "flush in-flight dma when writing to +* this DMA mapped memory" */ +#define DMA_DIR_MASK ((1 << DMA_ATTR_SHIFT) - 1) +#define DMA_ATTR_MASK ~DMA_DIR_MASK + +static inline int +dma_flags_set_dmaflush (int dir) { +return (dir | (DMA_FLUSH_ATTR <<DMA_ATTR_SHIFT )); +} + +static inline int +dma_flags_get_direction (int dir) { +return (dir &DMA_DIR_MASK ); +} + +static inline int +dma_flags_get_dmaflush (int dir) { +return (((dir &DMA_ATTR_MASK ) >>DMA_ATTR_SHIFT ) &DMA_FLUSH_ATTR ); +} +#endif /* _ASM_SN_IO_H */
Signature · 1 line
Arthur
Quoted 5 lines · [email protected] wrote:
Define
ARCH_DOES_POSTED_DMA, and override thedma_flags_set_dmaflushfunction. Also definedma_flags_get_direction,dma_flags_get_dmaflushto retrieve the direction and "dmaflush attribute" from flags passed to the sn_dma_map_* functions.
Hi Arthur,
I'm a little concerned about changing the API for the dma_ foo functions, which are defined cross platform. If you want to change that, I think it will require updating the documentation explaining it.
Regarding ARCH_DOES_POSTED_DMA, is that sufficient as a #define or does it have to be run-time tested? (does it do anything at this stage). I ask since not all ia64 platforms do posted DMA.
Cheers,
Jes
Quoted 4 lines · Jes Sorensen
I'm a little concerned about changing the API for the
dma_foo functions, which are defined cross platform. If you want to change that, I think it will require updating the documentation explaining it.
Good idea. I'll post a documentation patchette.
Quoted 5 lines
Regarding
ARCH_DOES_POSTED_DMA, is that sufficient as a #define or does it have to be run-time tested? (does it do anything at this stage). I ask since not all ia64 platforms do posted DMA.
I think a #define is exactly what we want here.
The newly #define-ed function (for now, and maybe forever) does nothing except on IA64_SGI_SN2, which does posted DMA.
Signature · 1 line
Arthur
Quoted 5 lines
I'm a little concerned about changing the API for the
dma_foo functions, which are defined cross platform. If you want to change that, I think it will require updating the documentation explaining it.....
What do you think of the following? (And is there anyone else I should be cc-ing for review?)
Document semantics of dma_flags_set_dmaflush()
--
DMA-API.txt | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+)
Documentation/DMA-API.txt +22 -0
@@ -392,6 +392,28 @@ Notes: You must do this: See also dma_map_single(). +int +dma_flags_set_dmaflush(int dir) + +Amend dir (one of the enum dma_data_direction values), with a platform- +specific "dmaflush" attribute. Unless the platform supports "posted DMA" +this is a no-op. + +On platforms that support posted DMA, dma_flags_set_dmaflush() causes +device writes to the associated memory region to flush in-flight DMA. +This can be important, for example, when (DMA) writes to the memory +region indicate that DMA of data is complete. If DMA of data and DMA of +the completion indication race, as they can do when the platform supports +posted DMA, then the completion indication may arrive in host memory +ahead of some data. + +To prevent this, you might map the memory region used for completion +indications as follows: + + int count, flags = dma_flags_set_dmaflush(DMA_BIDIRECTIONAL); + ..... + count = dma_map_sg(dev, sglist, nents, flags); + Part II - Advanced dma_ usage -----------------------------
Signature · 1 line
Arthur
Quoted 8 lines · On Tue, 21 Aug 2007 12:35:22 -0700 [email protected] wrote:
Quoted 5 lines
I'm a little concerned about changing the API for the
dma_foo functions, which are defined cross platform. If you want to change that, I think it will require updating the documentation explaining it.....What do you think of the following? (And is there anyone else I should be cc-ing for review?)
probably the document's author (cc added)
Quoted 12 lines
Document semantics of
dma_flags_set_dmaflush()Signed-off-by Arthur Kepner <[email protected]>--
DMA-API.txt | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+)Documentation/DMA-API.txt +0
-0
Documentation/DMA-API.txt Notes: You must do this: · line 398
Quoted 7 lines
Documentation/DMA-API.txt +4
-0@@ -392,6 +392,28 @@ Notes: You must do this: See also dma_map_single(). +int +dma_flags_set_dmaflush(int dir) + +Amend dir (one of the enum dma_data_direction values), with a platform-
no comma.
Documentation/DMA-API.txt Notes: You must do this: · line 399
Quoted 1 line
Documentation/DMA-API.txt +1
-0+specific "dmaflush" attribute. Unless the platform supports "posted DMA"
add comma after "posted DMA" and drop lots of trailing spaces.
Quoted 22 lines
Documentation/DMA-API.txt +17
-0+this is a no-op. + +On platforms that support posted DMA, dma_flags_set_dmaflush() causes +device writes to the associated memory region to flush in-flight DMA. +This can be important, for example, when (DMA) writes to the memory +region indicate that DMA of data is complete. If DMA of data and DMA of +the completion indication race, as they can do when the platform supports +posted DMA, then the completion indication may arrive in host memory +ahead of some data. + +To prevent this, you might map the memory region used for completion +indications as follows: + + int count, flags = dma_flags_set_dmaflush(DMA_BIDIRECTIONAL); + ..... + count = dma_map_sg(dev, sglist, nents, flags); + Part II - Advanced dma_ usage -----------------------------Signature · 1 line
Arthur
---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
Quoted 15 lines · Randy Dunlap
Documentation/DMA-API.txt Notes: You must do this: · line 398
Quoted 12 lines
Documentation/DMA-API.txt +4
-0@@ -392,6 +392,28 @@ Notes: You must do this: See also dma_map_single(). +int +dma_flags_set_dmaflush(int dir) + +Amend dir (one of the enum dma_data_direction values), with a platform-no comma.
Quoted 5 lines
Documentation/DMA-API.txt Notes: You must do this: · line 399
Quoted 1 line
Documentation/DMA-API.txt +1
-0+specific "dmaflush" attribute. Unless the platform supports "posted DMA"add comma after "posted DMA" and drop lots of trailing spaces.
Quoted 1 line
Documentation/DMA-API.txt +1
-0+this is a no-op
Almost every platform supports posted DMA ... its a property of most PCI bridge chips.
Quoted 7 lines
Documentation/DMA-API.txt Notes: You must do this: · line 408
Quoted 7 lines
Documentation/DMA-API.txt +7
-0+On platforms that support posted DMA, dma_flags_set_dmaflush() causes +device writes to the associated memory region to flush in-flight DMA. +This can be important, for example, when (DMA) writes to the memory +region indicate that DMA of data is complete. If DMA of data and DMA of +the completion indication race, as they can do when the platform supports +posted DMA, then the completion indication may arrive in host memory +ahead of some data.
This isn't possible on most platforms. PCI write posting can only be flushed by a read transaction on the device (or sometimes any device on the bridge). Either this interface is misnamed and misdescribed, or it can't work for most systems.
Quoted 6 lines
Documentation/DMA-API.txt Notes: You must do this: · line 415
Quoted 6 lines
Documentation/DMA-API.txt +6
-0+To prevent this, you might map the memory region used for completion +indications as follows: + + int count, flags = dma_flags_set_dmaflush(DMA_BIDIRECTIONAL); + ..... + count = dma_map_sg(dev, sglist, nents, flags);
James
Quoted 4 lines · James Bottomley
.....
Almost every platform supports posted DMA ... its a property of most PCI bridge chips.
The term "posted DMA" is used to describe this behavior in the Altix Device Driver Writer's Guide, but it may be confusing things here. Maybe a better term will suggest itself if I can clarify....
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
Quoted 6 lines
......
This isn't possible on most platforms. PCI write posting can only be flushed by a read transaction on the device (or sometimes any device on the bridge). Either this interface is misnamed and misdescribed, or it can't work for most systems.
Clearly it wasn't described adequately...
A read transaction on the device will flush pending writes to the device. But I'm worried about DMA from the device to host memory. On Altix, there are two mechanisms that flush all in-flight DMA to host memory: 1) an interrupt, and 2) a write to a memory region which has a "barrier" attribute set. Obviously option 1 isn't viable for performance reasons. This new interface is about making "option 2" generally available. (As it is now, the only way to get memory with the "barrier" attribute is to allocate it with dma_alloc_coherent().)
Signature · 1 line
Arthur
Quoted 10 lines · On Tue, 2007-08-21 at 17:34 -0700, [email protected] wrote:
Quoted 4 lines · James Bottomley
.....
Almost every platform supports posted DMA ... its a property of most PCI bridge chips.The term "posted DMA" is used to describe this behavior in the Altix Device Driver Writer's Guide, but it may be confusing things here. Maybe a better term will suggest itself if I can clarify....
OK, but posted DMA has a pretty specific meaning in terms of PCI, hence the confusion.
Quoted 3 lines
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
This is mmiowb and read_relaxed() again, isn't it?
Quoted 9 lines
Quoted 6 lines
......
This isn't possible on most platforms. PCI write posting can only be flushed by a read transaction on the device (or sometimes any device on the bridge). Either this interface is misnamed and misdescribed, or it can't work for most systems.Clearly it wasn't described adequately...
Quoted 9 lines
A read transaction on the device will flush pending writes to the device. But I'm worried about DMA from the device to host memory. On Altix, there are two mechanisms that flush all in-flight DMA to host memory: 1) an interrupt, and 2) a write to a memory region which has a "barrier" attribute set. Obviously option 1 isn't viable for performance reasons. This new interface is about making "option 2" generally available. (As it is now, the only way to get memory with the "barrier" attribute is to allocate it with
dma_alloc_coherent().)
Which sounds exactly what mmiowb does ... is there a need for a new API; can't you just use mmiowb()?
James
Quoted 7 lines · James Bottomley
Quoted 3 lines · On Tue, 2007-08-21 at 17:34 -0700, [email protected] wrote:
The term "posted DMA" is used to describe this behavior in the Altix Device Driver Writer's Guide, but it may be confusing things here. Maybe a better term will suggest itself if I can clarify....
OK, but posted DMA has a pretty specific meaning in terms of PCI, hence the confusion.
Maybe it would be more better to refer to this as 'out of order DMA'?
Quoted 5 lines
Quoted 3 lines
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
This is mmiowb and
read_relaxed()again, isn't it?
I believe it's the same problem, except this time it's when exposing structures to userland.
Cheers,
Jes
Quoted 10 lines · Jes Sorensen
Quoted 7 lines · James Bottomley
Quoted 3 lines · On Tue, 2007-08-21 at 17:34 -0700, [email protected] wrote:
The term "posted DMA" is used to describe this behavior in the Altix Device Driver Writer's Guide, but it may be confusing things here. Maybe a better term will suggest itself if I can clarify....
OK, but posted DMA has a pretty specific meaning in terms of PCI, hence the confusion.
Maybe it would be more better to refer to this as 'out of order DMA'?
Or Relaxed ordering DMA ... that's why the readX_relaxed()?
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
This is mmiowb and
read_relaxed()again, isn't it?I believe it's the same problem, except this time it's when exposing structures to userland.
Hmm, so how does another kernel API exposing mmiowb in a different way help with this? Surely, if the driver is exporting something to user space, it's simply up to the driver to call mmiowb when the user says it's done?
James
Quoted 20 lines · James Bottomley
Quoted 10 lines · Jes Sorensen
Quoted 7 lines · James Bottomley
Quoted 3 lines · On Tue, 2007-08-21 at 17:34 -0700, [email protected] wrote:
The term "posted DMA" is used to describe this behavior in the Altix Device Driver Writer's Guide, but it may be confusing things here. Maybe a better term will suggest itself if I can clarify....
OK, but posted DMA has a pretty specific meaning in terms of PCI, hence the confusion.
Maybe it would be more better to refer to this as 'out of order DMA'?
Or Relaxed ordering DMA ... that's why the
readX_relaxed()?Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
This is mmiowb and
read_relaxed()again, isn't it?
Quoted 7 lines
Quoted 2 lines
I believe it's the same problem, except this time it's when exposing structures to userland.
Hmm, so how does another kernel API exposing mmiowb in a different way help with this? Surely, if the driver is exporting something to user space, it's simply up to the driver to call mmiowb when the user says it's done?
mmiowb() is for PIO->device. This interface is for DMA->memory (see akepner's other mail).
The problem is a DMA write (say to a completion queue) from a device may imply something about another DMA write from the same device (say the actual data). If the completion queue write arrives first (which can happen on sn2), the driver must ensure that the rest of the outstanding DMA is complete prior to looking at the completion queue status. It can either use a regular PIO read to do this (i.e. a non-relaxed one) or set a flag on the completion queue DMA address that makes it act as a barrier wrt other DMA, which is what akepner's patch does (which should be much more efficient that using a PIO read to guarantee DMA writes have completed).
Jesse
Quoted 32 lines · Jesse Barnes
Quoted 20 lines · James Bottomley
Quoted 10 lines · Jes Sorensen
Quoted 7 lines · James Bottomley
Quoted 3 lines · On Tue, 2007-08-21 at 17:34 -0700, [email protected] wrote:
The term "posted DMA" is used to describe this behavior in the Altix Device Driver Writer's Guide, but it may be confusing things here. Maybe a better term will suggest itself if I can clarify....
OK, but posted DMA has a pretty specific meaning in terms of PCI, hence the confusion.
Maybe it would be more better to refer to this as 'out of order DMA'?
Or Relaxed ordering DMA ... that's why the
readX_relaxed()?Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
This is mmiowb and
read_relaxed()again, isn't it?Quoted 7 lines
Quoted 2 lines
I believe it's the same problem, except this time it's when exposing structures to userland.
Hmm, so how does another kernel API exposing mmiowb in a different way help with this? Surely, if the driver is exporting something to user space, it's simply up to the driver to call mmiowb when the user says it's done?
mmiowb()is forPIO->device. This interface is forDMA->memory(see akepner's other mail).
Quoted 9 lines
The problem is a DMA write (say to a completion queue) from a device may imply something about another DMA write from the same device (say the actual data). If the completion queue write arrives first (which can happen on sn2), the driver must ensure that the rest of the outstanding DMA is complete prior to looking at the completion queue status. It can either use a regular PIO read to do this (i.e. a non-relaxed one) or set a flag on the completion queue DMA address that makes it act as a barrier wrt other DMA, which is what akepner's patch does (which should be much more efficient that using a PIO read to guarantee DMA writes have completed).
This is a violation of the PCI spec, isn't it, like Matthew pointed out? The only time a device->host DMA transaction shouldn't follow strict ordering is when the device sets the relaxed hint in its PCI registers.
James
Quoted 11 lines · James Bottomley
Quoted 10 lines
The problem is a DMA write (say to a completion queue) from a device may imply something about another DMA write from the same device (say the actual data). If the completion queue write arrives first (which can happen on sn2), the driver must ensure that the rest of the outstanding DMA is complete prior to looking at the completion queue status. It can either use a regular PIO read to do this (i.e. a non-relaxed one) or set a flag on the completion queue DMA address that makes it act as a barrier wrt other DMA, which is what akepner's patch does (which should be much more efficient that using a PIO read to guarantee DMA writes have completed).
Quoted 3 lines
This is a violation of the PCI spec, isn't it, like Matthew pointed out? The only time a
device->hostDMA transaction shouldn't follow strict ordering is when the device sets the relaxed hint in its PCI registers.
Yeah, it is. Whether its allowed in PCIe depends on how you read the spec (but either way it would need to be explicitly enabled).
For better or for worse, Altix hardware always behaves this way (well mostly for the better, since most device protocols don't care as they involve PIO, and out of order completion is *much* faster on Altix than strict ordering).
Arthur's patch is pretty straightfoward though, so unless someone can think of a better way of hiding this architectural detail in lower level code it's probably a good thing to add (especially given that future revs of PCIe will probably allow this behavior, and hopefully less ambiguously than the current spec).
Jesse
Quoted 23 lines · Jesse Barnes
Quoted 11 lines · James Bottomley
Quoted 10 lines
The problem is a DMA write (say to a completion queue) from a device may imply something about another DMA write from the same device (say the actual data). If the completion queue write arrives first (which can happen on sn2), the driver must ensure that the rest of the outstanding DMA is complete prior to looking at the completion queue status. It can either use a regular PIO read to do this (i.e. a non-relaxed one) or set a flag on the completion queue DMA address that makes it act as a barrier wrt other DMA, which is what akepner's patch does (which should be much more efficient that using a PIO read to guarantee DMA writes have completed).
Quoted 3 lines
This is a violation of the PCI spec, isn't it, like Matthew pointed out? The only time a
device->hostDMA transaction shouldn't follow strict ordering is when the device sets the relaxed hint in its PCI registers.Yeah, it is. Whether its allowed in PCIe depends on how you read the spec (but either way it would need to be explicitly enabled).
For better or for worse, Altix hardware always behaves this way (well mostly for the better, since most device protocols don't care as they involve PIO, and out of order completion is *much* faster on Altix than strict ordering).
Quoted 5 lines
Arthur's patch is pretty straightfoward though, so unless someone can think of a better way of hiding this architectural detail in lower level code it's probably a good thing to add (especially given that future revs of PCIe will probably allow this behavior, and hopefully less ambiguously than the current spec).
The spec isn't ambiguous ... it says if the device and bridge agree on relaxed ordering, then it *may* be observed in the transaction. If there's a disagreement or neither wishes to support relaxed ordering then the transaction *must* be strict.
I really don't think a work around for a PCI spec violation belongs in the generic DMA code, do you? The correct fix for this should be to set the device hints to strict ordering, which presumably altix respects? In which case, it sounds like what needs exposing are access to the PCI device hints. I believe both PCI-X and PCIe have these hints as optional specifiers in the bridges, so it should be in a current Rev of the PCI spec. Or are you proposing adding an additional PCI API that allows transaction flushes to be inserted into the stream for devices and bridges that have already negotiated relaxed ordering? ... in which case we need to take this to the PCI list.
James
Quoted 10 lines · James Bottomley
I really don't think a work around for a PCI spec violation belongs in the generic DMA code, do you? The correct fix for this should be to set the device hints to strict ordering, which presumably altix respects? In which case, it sounds like what needs exposing are access to the PCI device hints. I believe both PCI-X and PCIe have these hints as optional specifiers in the bridges, so it should be in a current Rev of the PCI spec. Or are you proposing adding an additional PCI API that allows transaction flushes to be inserted into the stream for devices and bridges that have already negotiated relaxed ordering? ... in which case we need to take this to the PCI list.
James,
I don't believe it respects those hints - I agree, it's a pita, but thats the state of the situation. Even if it did, it would make performance suck as Jesse also pointed out.
As I pointed out in my email to Willy is that the NUMA fabric is routed, there's not one path through the system, which is what makes this happen.
Jes
Quoted 14 lines · Jes Sorensen
Quoted 10 lines · James Bottomley
I really don't think a work around for a PCI spec violation belongs in the generic DMA code, do you? The correct fix for this should be to set the device hints to strict ordering, which presumably altix respects? In which case, it sounds like what needs exposing are access to the PCI device hints. I believe both PCI-X and PCIe have these hints as optional specifiers in the bridges, so it should be in a current Rev of the PCI spec. Or are you proposing adding an additional PCI API that allows transaction flushes to be inserted into the stream for devices and bridges that have already negotiated relaxed ordering? ... in which case we need to take this to the PCI list.
James,
Quoted 7 lines
I don't believe it respects those hints - I agree, it's a pita, but thats the state of the situation. Even if it did, it would make performance suck as Jesse also pointed out.
As I pointed out in my email to Willy is that the NUMA fabric is routed, there's not one path through the system, which is what makes this happen.
Hmm, didn't see the email ... but I'm probably not cc'd on all the thread. However ... it isn't that you couldn't do it ... it's that you don't want to do it because it's faster to violate the spec ... like all those nice ATA devices that lie about having a cache and then let you power down with uncommitted data still in it ... they work much faster for HDIO tests ... and who ever switches their box off?
James
Quoted 6 lines · James Bottomley
Hmm, didn't see the email ... but I'm probably not cc'd on all the thread. However ... it isn't that you couldn't do it ... it's that you don't want to do it because it's faster to violate the spec ... like all those nice ATA devices that lie about having a cache and then let you power down with uncommitted data still in it ... they work much faster for HDIO tests ... and who ever switches their box off?
James,
I didn't do it, I don't know who did it, but sure we can try and track them down and line them up outside .....
Point is that this is how the chips were done and they are out there in numbers. It makes things a *lot* faster to violate the spec in such a system, yes it sucks, but thats how things are. It wouldn't be the first time someone violated the PCI spec in their implementation and I am pretty sure it won't be the last.
Cheers,
Jes
Quoted 4 lines · James Bottomley
The spec isn't ambiguous ... it says if the device and bridge agree on relaxed ordering, then it *may* be observed in the transaction. If there's a disagreement or neither wishes to support relaxed ordering then the transaction *must* be strict.
Arg, don't make me dig out my spec, I don't have it handy...
Quoted 3 lines
I really don't think a work around for a PCI spec violation belongs in the generic DMA code, do you? The correct fix for this should be to set the device hints to strict ordering, which presumably altix respects?
Well, the Altix hw by itself won't honor device hints (I'm not even sure if there are devices that honor order hints like you outline above). However, Altix could track per-device ordering as long as arch code was called from such a hook.
Quoted 7 lines
In which case, it sounds like what needs exposing are access to the PCI device hints. I believe both PCI-X and PCIe have these hints as optional specifiers in the bridges, so it should be in a current Rev of the PCI spec. Or are you proposing adding an additional PCI API that allows transaction flushes to be inserted into the stream for devices and bridges that have already negotiated relaxed ordering? ... in which case we need to take this to the PCI list.
I think it would have to be the latter, since otherwise it would be hard to setup just completion queue requests to be ordered.
Jesse
Quoted 7 lines · Jesse Barnes
Quoted 4 lines · James Bottomley
The spec isn't ambiguous ... it says if the device and bridge agree on relaxed ordering, then it *may* be observed in the transaction. If there's a disagreement or neither wishes to support relaxed ordering then the transaction *must* be strict.
Arg, don't make me dig out my spec, I don't have it handy...
Well ... I don't have one either. However, Grant Grundler did a presentation to OLS about relaxed ordering, and I went over it pretty thoroughly with him a while ago. The bottom line is that the default is always strict unless both the bridge and the device agree otherwise.
Quoted 17 lines
Quoted 3 lines
I really don't think a work around for a PCI spec violation belongs in the generic DMA code, do you? The correct fix for this should be to set the device hints to strict ordering, which presumably altix respects?
Well, the Altix hw by itself won't honor device hints (I'm not even sure if there are devices that honor order hints like you outline above). However, Altix could track per-device ordering as long as arch code was called from such a hook.
Quoted 7 lines
In which case, it sounds like what needs exposing are access to the PCI device hints. I believe both PCI-X and PCIe have these hints as optional specifiers in the bridges, so it should be in a current Rev of the PCI spec. Or are you proposing adding an additional PCI API that allows transaction flushes to be inserted into the stream for devices and bridges that have already negotiated relaxed ordering? ... in which case we need to take this to the PCI list.
Quoted 2 lines
I think it would have to be the latter, since otherwise it would be hard to setup just completion queue requests to be ordered.
OK ... I think this is definitely a PCI specific API ... and probably a generic one for requesting order flushes in devices that have negotiated relaxed ordering. Do you want to start a new thread on linux-pci and cc me?
James
Quoted 6 lines · James Bottomley
......
OK ... I think this is definitely a PCI specific API ... and probably a generic one for requesting order flushes in devices that have negotiated relaxed ordering. Do you want to start a new thread on linux-pci and cc me?
I'll do this.
Signature · 1 line
Arthur
Quoted 11 lines · Jes Sorensen
Quoted 7 lines · James Bottomley
Quoted 3 lines · On Tue, 2007-08-21 at 17:34 -0700, [email protected] wrote:
The term "posted DMA" is used to describe this behavior in the Altix Device Driver Writer's Guide, but it may be confusing things here. Maybe a better term will suggest itself if I can clarify....
OK, but posted DMA has a pretty specific meaning in terms of PCI, hence the confusion.
Maybe it would be more better to refer to this as 'out of order DMA'?
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
This is mmiowb and
read_relaxed()again, isn't it?I believe it's the same problem, except this time it's when exposing structures to userland.
Actually, it's a different problem, but with a similar cause.
mmiowb() is for the case PIO (or MMIO) write order from two different CPUs can invert somewhere in the NL fabric.
read_relaxed() is a performance optimization to avoid the flush that's necessary to avoid inversion in order between a PIO (or MMIO) read and a DMA write.
What Arthur's trying to do here is avoid inversion in the order of two DMA writes.
jeremy
Quoted 8 lines · James Bottomley
On Tue, 2007-08-21 at 17:34 -0700, [email protected] wrote:
.....Quoted 3 lines
On Altix, DMA from a device isn't guaranteed to arrive in host memory in the order it was sent from the device. This reordering can happen in the NUMA interconnect (it's specifically not a PCI reordering.)
This is mmiowb and
read_relaxed()again, isn't it?
.....
No, this is different.
This problem here has do with ordering writes from the device to host memory. Specifically this problem can be manifested with Infiniband - when a Completion Queue Entry is written by the IB device, it indicates that data is available in host memory. But the write to the Completion Queue can race with DMA of data.
(Completion Queues can be allocated by the kernel or in userspace. The race described above can only happen when they are allocated in userspace - kernel allocations of CQs use dma_alloc_coherent() and so get the "barrier" attribute that's needed to prevent the race. The proposed new interface would allow CQs, or anything else, allocated with plain old malloc(), to set the barrier attribute.)
Signature · 1 line
Arthur
Quoted 4 lines · On Tue, Aug 21, 2007 at 12:35:22PM -0700, [email protected] wrote:
Documentation/DMA-API.txt +4
-0+int +dma_flags_set_dmaflush(int dir) + +Amend dir (one of the enum dma_data_direction values), with a platform-
Documentation/DMA-API.txt Notes: You must do this: · line 408
Quoted 10 lines
Documentation/DMA-API.txt +10
-0+specific "dmaflush" attribute. Unless the platform supports "posted DMA" +this is a no-op. + +On platforms that support posted DMA, dma_flags_set_dmaflush() causes +device writes to the associated memory region to flush in-flight DMA. +This can be important, for example, when (DMA) writes to the memory +region indicate that DMA of data is complete. If DMA of data and DMA of +the completion indication race, as they can do when the platform supports +posted DMA, then the completion indication may arrive in host memory +ahead of some data.
So, let me try to understand ... your hardware allows writes from the device to pass other writes from the device? Doesn't that violate the PCI spec? I'm thinking about this (page 43 of PCI 2.3):
Posted memory writes moving in the same direction through a bridge will complete on the destination bus in the same order they complete on the originating bus. Even if a single burst on the originating bus is terminated with Disconnect on the destination bus so that it is broken into multiple transactions, those transactions must not allow the data phases to complete on the destination bus in any order other than their order on the originating bus.
Documentation/DMA-API.txt Notes: You must do this: · line 415
Quoted 6 lines
Documentation/DMA-API.txt +6
-0+To prevent this, you might map the memory region used for completion +indications as follows: + + int count, flags = dma_flags_set_dmaflush(DMA_BIDIRECTIONAL); + ..... + count = dma_map_sg(dev, sglist, nents, flags);
So any device driver used on your hardware has to add a call to this new function, or it'll see data corruption? Not acceptable, IMO.
If this is a performance optimisation, then by all means add a function drivers can call to say "it's OK, I know about this brokenness, and I don't depend on it", but safety first.
Signature · 3 lines
"Bill, look, we understand that you're interested in selling us this operating system, but compare it to ours. We can't possibly take such a retrograde step."
Quoted 5 lines · Matthew Wilcox
So, let me try to understand ... your hardware allows writes from the device to pass other writes from the device? Doesn't that violate the PCI spec? I'm thinking about this (page 43 of PCI 2.3):
....
I should have stated this more carefully, but it's not a PCI reordering that's being addressed here, it's a reordering that can occur within the NUMA-interconnect.
Signature · 1 line
Arthur
Quoted 3 lines · Matthew Wilcox
So, let me try to understand ... your hardware allows writes from the device to pass other writes from the device? Doesn't that violate the PCI spec? I'm thinking about this (page 43 of PCI 2.3):
Hi Matthew,
Yes I believe this behavior on sn2 is 'bending' the PCI spec a bit. The problem is that the NUMA fabric is a routed network in itself so there can be multiple paths between the device and the physical memory it DMAs to/from.
Cheers,
Jes