sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)
PATCH 1/1 sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)
This fixes some problems with ATAPI devices on nForce4 controllers in ADMA mode on systems with memory located above 4GB. We need to delay setting the 64-bit DMA mask until the PRD table and padding buffer are allocated so that they don't get allocated above 4GB and break legacy mode (which is needed for ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the legacy buffers since setting the DMA mask affects both ports and we need to ensure the second port's buffers are allocated properly (fixes a problem with the previous version of this patch).
drivers/ata/sata_nv.c renamed from drivers/ata/sata_nv.c.before2 +16 -7
@@ -247,6 +247,7 @@void __iomem *ctl_block;void __iomem *gen_block;void __iomem *notifier_clear_block; +u64 adma_dma_mask;u8 flags;int last_issue_ncq; };
@@ -748,7 +749,7 @@ adma_enable =0 ;nv_adma_register_mode (ap); }else { - bounce_limit = *ap->dev->dma_mask; + bounce_limit = pp->adma_dma_mask; segment_boundary =NV_ADMA_DMA_BOUNDARY ; sg_tablesize =NV_ADMA_SGTBL_TOTAL_LEN ; adma_enable =1 ;
@@ -1134,10 +1135,16 @@void *mem;dma_addr_t mem_dma;void __iomem *mmio; +struct pci_dev *pdev =to_pci_dev (dev);u16 tmp;VPRINTK ("ENTER\n" ); +/* Ensure DMA mask is set to 32-bit before allocating legacy PRD and +pad buffers */ +pci_set_dma_mask (pdev,DMA_BIT_MASK (32 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (32 )); + rc =ata_port_start (ap);if (rc)return rc;
@@ -1153,6 +1160,14 @@ pp->notifier_clear_block = pp->gen_block +NV_ADMA_NOTIFIER_CLEAR + (4 * ap->port_no); +/* Now that the legacy PRD and padding buffer are allocated we can +safely raise the DMA mask to allocate the CPB/APRD table. */ +pci_set_dma_mask (pdev,DMA_BIT_MASK (64 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (64 )); +/* Store the mask that was actually used so we can restore it as +the bounce limit later if needed */ + pp->adma_dma_mask = *dev->dma_mask; + mem =dmam_alloc_coherent (dev,NV_ADMA_PORT_PRIV_DMA_SZ , &mem_dma,GFP_KERNEL );if (!mem)
@@ -2408,12 +2423,6 @@ hpriv->type = type; host->private_data = hpriv; -/* set 64bit dma masks, may fail */ -if (type ==ADMA ) { -if (pci_set_dma_mask (pdev,DMA_64BIT_MASK ) ==0 ) -pci_set_consistent_dma_mask (pdev,DMA_64BIT_MASK ); - } -/* request and iomap NV_MMIO_BAR */ rc =pcim_iomap_regions (pdev,1 <<NV_MMIO_BAR ,DRV_NAME );if (rc)
Quoted 11 lines · Robert Hancock
This fixes some problems with ATAPI devices on nForce4 controllers in ADMA mode on systems with memory located above 4GB. We need to delay setting the 64-bit DMA mask until the PRD table and padding buffer are allocated so that they don't get allocated above 4GB and break legacy mode (which is needed for ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the legacy buffers since setting the DMA mask affects both ports and we need to ensure the second port's buffers are allocated properly (fixes a problem with the previous version of this patch).
Signed-off-by Robert Hancock <[email protected]>
Would this be worth sending to stable team for 2.6.22 & 2.6.23 ?
Regards,
- vin
Quoted 10 lines · Vincent Fortier
Quoted 9 lines · Robert Hancock
This fixes some problems with ATAPI devices on nForce4 controllers in ADMA mode on systems with memory located above 4GB. We need to delay setting the 64-bit DMA mask until the PRD table and padding buffer are allocated so that they don't get allocated above 4GB and break legacy mode (which is needed for ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the legacy buffers since setting the DMA mask affects both ports and we need to ensure the second port's buffers are allocated properly (fixes a problem with the previous version of this patch).
Quoted 4 lines
Quoted 2 lines
Signed-off-by Robert Hancock <[email protected]>Would this be worth sending to stable team for 2.6.22 & 2.6.23 ?
Likely (after it gets merged), those versions would have the same bug..
Hello, Robert.
Quoted 11 lines · Robert Hancock
This fixes some problems with ATAPI devices on nForce4 controllers in ADMA mode on systems with memory located above 4GB. We need to delay setting the 64-bit DMA mask until the PRD table and padding buffer are allocated so that they don't get allocated above 4GB and break legacy mode (which is needed for ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the legacy buffers since setting the DMA mask affects both ports and we need to ensure the second port's buffers are allocated properly (fixes a problem with the previous version of this patch).
Signed-off-by Robert Hancock <[email protected]>
drivers/ata/sata_nv.c line 1146
Quoted 4 lines
drivers/ata/sata_nv.c renamed from drivers/ata/sata_nv.c.before2 +4
-0+/* Ensure DMA mask is set to 32-bit before allocating legacy PRD and +pad buffers */ +pci_set_dma_mask (pdev,DMA_BIT_MASK (32 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (32 ));
[--snip--]
drivers/ata/sata_nv.c line 1166
Quoted 2 lines
drivers/ata/sata_nv.c renamed from drivers/ata/sata_nv.c.before2 +2
-0+pci_set_dma_mask (pdev,DMA_BIT_MASK (64 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (64 ));
I'm probably being paranoid here but please add error checks. Just checking return value and returning error suffices.
Thanks.
Signature · 1 line
tejun
Quoted 19 lines · Tejun Heo
Hello, Robert.
Quoted 11 lines · Robert Hancock
This fixes some problems with ATAPI devices on nForce4 controllers in ADMA mode on systems with memory located above 4GB. We need to delay setting the 64-bit DMA mask until the PRD table and padding buffer are allocated so that they don't get allocated above 4GB and break legacy mode (which is needed for ATAPI devices). Also, explicitly set a 32-bit DMA mask before allocating the legacy buffers since setting the DMA mask affects both ports and we need to ensure the second port's buffers are allocated properly (fixes a problem with the previous version of this patch).
Signed-off-by Robert Hancock <[email protected]>drivers/ata/sata_nv.c line 1146
Quoted 4 lines
drivers/ata/sata_nv.c renamed from drivers/ata/sata_nv.c.before2 +4
-0+/* Ensure DMA mask is set to 32-bit before allocating legacy PRD and +pad buffers */ +pci_set_dma_mask (pdev,DMA_BIT_MASK (32 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (32 ));[--snip--]
Quoted 5 lines
drivers/ata/sata_nv.c line 1166
Quoted 2 lines
drivers/ata/sata_nv.c renamed from drivers/ata/sata_nv.c.before2 +2
-0+pci_set_dma_mask (pdev,DMA_BIT_MASK (64 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (64 ));I'm probably being paranoid here but please add error checks. Just checking return value and returning error suffices.
In the 32-bit case, I'm pretty sure those are guaranteed not to fail because 32-bit is the default. For the 64-bit ones, we don't care if they fail, because then we'll just use whatever mask ends up being set (we store the actual set DMA mask in adma_dma_mask for use when we need to reconfigure the bounce limit). We definitely don't want to fail initialization if the 64-bit set doesn't succeed..
Quoted 12 lines · Robert Hancock
Quoted 11 lines
drivers/ata/sata_nv.c line 1146
Quoted 5 lines
+ /* Ensure DMA mask is set to 32-bit before allocating legacy PRD
anddrivers/ata/sata_nv.c renamed from drivers/ata/sata_nv.c.before2 +3
-0+pad buffers */ +pci_set_dma_mask (pdev,DMA_BIT_MASK (32 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (32 ));[--snip--]
drivers/ata/sata_nv.c line 1166
Quoted 2 lines
drivers/ata/sata_nv.c renamed from drivers/ata/sata_nv.c.before2 +2
-0+pci_set_dma_mask (pdev,DMA_BIT_MASK (64 )); +pci_set_consistent_dma_mask (pdev,DMA_BIT_MASK (64 ));I'm probably being paranoid here but please add error checks. Just checking return value and returning error suffices.
Quoted 6 lines
In the 32-bit case, I'm pretty sure those are guaranteed not to fail because 32-bit is the default. For the 64-bit ones, we don't care if they fail, because then we'll just use whatever mask ends up being set (we store the actual set DMA mask in
adma_dma_maskfor use when we need to reconfigure the bounce limit). We definitely don't want to fail initialization if the 64-bit set doesn't succeed..
Then please add BUG or WARN_ON after 32bit switching (but then again if you're gonna do that why not just add if (rc) return rc?) and add comment stating setting 64 bit dma masks is allowed to fail.
Thanks.
Signature · 1 line
tejun