Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Robert Hancock wrote: Jeff Garzik wrote: Robert Hancock wrote: 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). Signed-off-by: Robert Hancock [EMAIL PROTECTED] This is a bit nasty :/ I would consider setting the consistent DMA mask to 32-bit, and setting the overall mask to 64-bit. Seems like that would solve the problem? The issue with that is that it would also constrain the ADMA CPB/PRD table allocation to 32-bit, which I'd rather avoid having to do. There are dual-socket Opteron boxes like HP xw9300 that use this controller, and limiting the allocation to 32-bit could force a non-optimal node allocation for the table memory. These type of devices really want a version of dma_alloc_coherent that allows overriding the DMA mask for specific allocations to make this cleaner. I'm sure this isn't the only device that has different DMA mask requirements for different consistent memory allocations.. This patch does has the advantage of being confirmed to fix the reporter's problem (https://bugzilla.redhat.com/show_bug.cgi?id=351451) which there's something to be said for this late in the .24-rc series.. Also, does this need to be rebased on top of what I just pushed upstream? It don't think so.. this change is independent from the sata_nv: don't use legacy DMA in ADMA mode (v3) patch you just merged. This bug fix is still outstanding. I haven't heard any more comments on this one recently.. -- Robert Hancock Saskatoon, SK, Canada To email, remove nospam from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Jeff Garzik wrote: Robert Hancock wrote: 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). Signed-off-by: Robert Hancock [EMAIL PROTECTED] This is a bit nasty :/ I would consider setting the consistent DMA mask to 32-bit, and setting the overall mask to 64-bit. Seems like that would solve the problem? Also, does this need to be rebased on top of what I just pushed upstream? Jeff Jeff, ping on this one? This (or, one like it) really should make it into 2.6.24.. -- Robert Hancock Saskatoon, SK, Canada To email, remove nospam from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Robert Hancock wrote: 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). Signed-off-by: Robert Hancock [EMAIL PROTECTED] This is a bit nasty :/ I would consider setting the consistent DMA mask to 32-bit, and setting the overall mask to 64-bit. Seems like that would solve the problem? Also, does this need to be rebased on top of what I just pushed upstream? Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Jeff Garzik wrote: Robert Hancock wrote: 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). Signed-off-by: Robert Hancock [EMAIL PROTECTED] This is a bit nasty :/ I would consider setting the consistent DMA mask to 32-bit, and setting the overall mask to 64-bit. Seems like that would solve the problem? The issue with that is that it would also constrain the ADMA CPB/PRD table allocation to 32-bit, which I'd rather avoid having to do. There are dual-socket Opteron boxes like HP xw9300 that use this controller, and limiting the allocation to 32-bit could force a non-optimal node allocation for the table memory. These type of devices really want a version of dma_alloc_coherent that allows overriding the DMA mask for specific allocations to make this cleaner. I'm sure this isn't the only device that has different DMA mask requirements for different consistent memory allocations.. This patch does has the advantage of being confirmed to fix the reporter's problem (https://bugzilla.redhat.com/show_bug.cgi?id=351451) which there's something to be said for this late in the .24-rc series.. Also, does this need to be rebased on top of what I just pushed upstream? It don't think so.. this change is independent from the sata_nv: don't use legacy DMA in ADMA mode (v3) patch you just merged. -- Robert Hancock Saskatoon, SK, Canada To email, remove nospam from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Robert Hancock wrote: 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). ... Mmm.. I wonder how many other libata drivers have this exact same bug, whether noticed yet or not ? Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Mark Lord wrote: Morrison, Tom wrote: I am hopeful that the sata_mv has this bug (I proved that the problem I was experiencing was due to the sata_mv driver with 3.75Gig or more of memory)... I am on vacation for a week or more ...or I'd tell you today if it did have this bug! .. Yeah, I kind of had your reports in mind when I asked that. :) On a related note, I now have lots of Marvell (sata_mv) hardware here, and an Intel CPU/chipset box with physical RAM above the 4GB boundary. Based on a quick look at sata_mv it appears it sets a 64-bit DMA mask unconditionally, but for non-ATA_PROT_DMA commands (which includes all ATAPI), it just falls back to ata_qc_issue_prot which issues via the legacy SFF interface and can only handle 32-bit addressing. So yes, it appears to have a similar bug as sata_nv had. Likely it needs a similar slave_config trick to change bounce limit depending on the connected device, unless there is really a way to issue ATAPI commands with this EDMA interface, as the TODO list in sata_mv.c suggests may be possible.. -- Robert Hancock Saskatoon, SK, Canada To email, remove nospam from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Jeff Garzik wrote: Robert Hancock wrote: Based on a quick look at sata_mv it appears it sets a 64-bit DMA mask unconditionally, but for non-ATA_PROT_DMA commands (which includes all ATAPI), it just falls back to ata_qc_issue_prot which issues via the legacy SFF interface and can only handle 32-bit addressing. So yes, it appears to have a similar bug as sata_nv had. sata_mv doesn't do ATAPI at all... .. Not yet, anyway. Stay tuned.. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Robert Hancock wrote: 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). Signed-off-by: Robert Hancock [EMAIL PROTECTED] Acked-by: Tejun Heo [EMAIL PROTECTED] Thanks. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Robert Hancock wrote: Based on a quick look at sata_mv it appears it sets a 64-bit DMA mask unconditionally, but for non-ATA_PROT_DMA commands (which includes all ATAPI), it just falls back to ata_qc_issue_prot which issues via the legacy SFF interface and can only handle 32-bit addressing. So yes, it appears to have a similar bug as sata_nv had. sata_mv doesn't do ATAPI at all... Jeff - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Morrison, Tom wrote: I am hopeful that the sata_mv has this bug (I proved that the problem I was experiencing was due to the sata_mv driver with 3.75Gig or more of memory)... I am on vacation for a week or more ...or I'd tell you today if it did have this bug! .. Yeah, I kind of had your reports in mind when I asked that. :) On a related note, I now have lots of Marvell (sata_mv) hardware here, and an Intel CPU/chipset box with physical RAM above the 4GB boundary. Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Jeff Garzik wrote: Robert Hancock wrote: Based on a quick look at sata_mv it appears it sets a 64-bit DMA mask unconditionally, but for non-ATA_PROT_DMA commands (which includes all ATAPI), it just falls back to ata_qc_issue_prot which issues via the legacy SFF interface and can only handle 32-bit addressing. So yes, it appears to have a similar bug as sata_nv had. sata_mv doesn't do ATAPI at all... Right.. missed that ATA_FLAG_NO_ATAPI. So these issues Tom is reporting are just with a normal SATA hard drive? -- Robert Hancock Saskatoon, SK, Canada To email, remove nospam from [EMAIL PROTECTED] Home Page: http://www.roberthancock.com/ - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
I am hopeful that the sata_mv has this bug (I proved that the problem I was experiencing was due to the sata_mv driver with 3.75Gig or more of memory)... I am on vacation for a week or more ...or I'd tell you today if it did have this bug! From: [EMAIL PROTECTED] on behalf of Mark Lord Sent: Fri 11/23/2007 10:22 AM To: Robert Hancock Cc: linux-kernel; ide; Jeff Garzik; Tejun Heo Subject: Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3) Robert Hancock wrote: 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). ... Mmm.. I wonder how many other libata drivers have this exact same bug, whether noticed yet or not ? Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Yes, I believe that - otherwise, this problem would have been a crisis a LONG time ago...:-) But I do have some more questions in relationship to how things are mapped in your environment. I have a flat memory map (i.e.: the full 0x0 -- 0x1__ is passed to the 32bit Linux kernel without any 'holes' and/or reserved areas). Does your Intel memory map have this same type of flat memory model (and thus allow use of the FULL lower 4Gig) - or does it reserve areas of lower 4Gig for devices and such - if not - where are these reserved areas - and how do the relate to the I/O memory map for the device? In other words, I would be very interested in seeing the memory map the PCI memory mapping to see if any overlap/correspond to reserve areas of lower 4 Gig (in a linux 32bit mode)... Tom From: Mark Lord [mailto:[EMAIL PROTECTED] Sent: Fri 11/23/2007 12:46 PM To: Morrison, Tom Cc: Robert Hancock; linux-kernel; ide; Jeff Garzik; Tejun Heo Subject: Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3) Morrison, Tom wrote: I am hopeful that the sata_mv has this bug (I proved that the problem I was experiencing was due to the sata_mv driver with 3.75Gig or more of memory)... I am on vacation for a week or more ...or I'd tell you today if it did have this bug! .. Yeah, I kind of had your reports in mind when I asked that. :) On a related note, I now have lots of Marvell (sata_mv) hardware here, and an Intel CPU/chipset box with physical RAM above the 4GB boundary. Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
Mark wrote: Yeah, I kind of had your reports in mind when I asked that. :) On a related note, I now have lots of Marvell (sata_mv) hardware here, and an Intel CPU/chipset box with physical RAM above the 4GB boundary. Morrison, Tom wrote: Yes, I believe that - otherwise, this problem would have been a crisis a LONG time ago...:-) But I do have some more questions in relationship to how things are mapped in your environment. I have a flat memory map (i.e.: the full 0x0 -- 0x1__ is passed to the 32bit Linux kernel without any 'holes' and/or reserved areas). Does your Intel memory map have this same type of flat memory model (and thus allow use of the FULL lower 4Gig) - or does it reserve areas of lower 4Gig for devices and such - if not - where are these reserved areas - and how do the relate to the I/O memory map for the device? In other words, I would be very interested in seeing the memory map the PCI memory mapping to see if any overlap/correspond to reserve areas of lower 4 Gig (in a linux 32bit mode)... ... I believe that only 2GB or so of the 4GB RAM appears below the 4GB boundary. The rest is accessed above 4GB, using Intel's 36-bit PAE functionality. I think what you want to see is /proc/mtrr, annotated below by me: reg00: base=0x08000 (2048MB), size=2048MB: uncachable, count=1 I/O space reg01: base=0x0 ( 0MB), size=4096MB: write-back, count=1 first 2GB of RAM + I/O space reg02: base=0x1 (4096MB), size=1024MB: write-back, count=1 third GB of RAM reg03: base=0x14000 (5120MB), size= 512MB: write-back, count=1 portion of 4th GB of RAM reg04: base=0x16000 (5632MB), size= 256MB: write-back, count=1 portion of 4th GB of RAM reg05: base=0x17000 (5888MB), size= 128MB: write-back, count=1 portion of 4th GB of RAM reg06: base=0x17800 (6016MB), size= 64MB: write-back, count=1 portion of 4th GB of RAM reg07: base=0x0af80 (2808MB), size= 8MB: uncachable, count=1 (?) dunno From that, the visible RAM should be 2048 + 1024 + 512 + 256 + 128 + 64 = 3968MB. In /proc/meminfo, it reports MemTotal of 4067260kB, which divided by 1024 gives 3971MB. The BIOS reports 4024MB. But the MTRR values above do make it rather clear that nearly half the RAM requires 33-bit physical addressing for access. Cheers - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)
Tejun Heo wrote: Hello, Robert. Robert Hancock wrote: 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] + /* 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--] + 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.. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)
Robert Hancock wrote: +/* 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--] +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.. 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. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)
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).
Signed-off-by: Robert Hancock [EMAIL PROTECTED]
--- linux-2.6.24-rc3-git1edit/drivers/ata/sata_nv.c.before2 2007-11-22
19:42:28.0 -0600
+++ linux-2.6.24-rc3-git1edit/drivers/ata/sata_nv.c 2007-11-22
19:48:25.0 -0600
@@ -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,20 @@
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 */
+ rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (rc)
+ return rc;
+ rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
+ if (rc)
+ return rc;
+
rc = ata_port_start(ap);
if (rc)
return rc;
@@ -1153,6 +1164,15 @@
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.
+ These are allowed to fail since we store the value that ends up
+ being used to set as the bounce limit in slave_config later if
+ needed. */
+ pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
+ pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
+ pp-adma_dma_mask = *dev-dma_mask;
+
mem = dmam_alloc_coherent(dev, NV_ADMA_PORT_PRIV_DMA_SZ,
mem_dma, GFP_KERNEL);
if (!mem)
@@ -2414,12 +2434,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)
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)
Le mardi 20 novembre 2007 à 18:56 -0600, Robert Hancock a écrit : 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 - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)
Vincent Fortier wrote: Le mardi 20 novembre 2007 à 18:56 -0600, Robert Hancock a écrit : 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 ? Likely (after it gets merged), those versions would have the same bug.. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v2)
Hello, Robert. Robert Hancock wrote: 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] + /* 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--] + 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. -- tejun - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] 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).
Signed-off-by: Robert Hancock [EMAIL PROTECTED]
--- linux-2.6.24-rc3-git1edit/drivers/ata/sata_nv.c.before2 2007-11-20
17:47:46.0 -0600
+++ linux-2.6.24-rc3-git1edit/drivers/ata/sata_nv.c 2007-11-20
17:50:30.0 -0600
@@ -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)
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB
Tejun Heo wrote:
Robert Hancock wrote:
Tejun Heo wrote:
..
Yes, it should likely do something with these return values. Though
theoretically it shouldn't fail, since the DMA mask is either 32-bit,
which shouldn't fail, or one that was successfully set before. Also I
don't think the SCSI layer actually checks the slave_config return
value.. sigh.
Then please at least add WARN_ON() another reason why allocating /
deallocating resources from -slave_config isn't such a good idea.
..
The entire point of slave_configure is to provide a point for the LLD
to do per-device data structure allocation/init.
And yes, SCSI does check the return code. Whether the code around that check
is buggy or not is another question, but it's always worked for me.
if (sdev-host-hostt-slave_configure) {
int ret = sdev-host-hostt-slave_configure(sdev);
if (ret) {
/*
* if LLDD reports slave not present, don't clutter
* console with alloc failure messages
*/
if (ret != -ENXIO) {
sdev_printk(KERN_ERR, sdev,
failed to configure device\n);
}
return SCSI_SCAN_NO_RESPONSE;
}
}
Cheers
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB
Tejun Heo wrote: Could be done.. but, I don't want to constrain the ADMA APRD/CPB area in that way (there are some dual-socket Opteron boxes with this controller, forcing an allocation below 4GB for this could force a non-optimal node allocation I think..) To do this I'd have to raise the mask for the APRD allocation, drop it again, then raise it again in ADMA mode, which is kind of ugly. I don't think it really matters. The table isn't too big and it's not like access to the table has any processor locality. Maybe it's better to allocate to the same node as the irq but raising DMA mask doesn't help at all. It's quite possible that restricting the DMA mask will also restrict what node that can get allocated on. I'm not so much thinking of the CPU access to the table but the controller's banging on the thing several times for each command.. I think performance impact is nil either way but even in highly unlikely case it has any impact, allocating PRDs under 4G should be better as it avoids DAC cycles on the bus. But again, this is just irrelevant. I'd say just allocate everything under 4G. The DAC issue shouldn't matter as these controllers are integrated into the chipset so it will be using all HT bus transactions, not PCI. We can do it without all that mess in slave_config though, just by delaying raising the DMA mask until after the PRD/pad buffers are allocated. Also, I'd rather not allocate the legacy PRD at all if we're in ADMA mode. That way, if some bug causes us to try and do legacy DMA in ADMA mode, we'll crash from null pointer dereference instead of potentially transferring incorrect data (as we had in this case) and corrupting things. Yeap, I can agree with this. But can you add BUG_ON()/WARN_ON() at places instead? I know blanking pointers feel safer but I think it's best to keep resource allocation / release in -port_start/stop(). Yeah, I've got rid of that stuff now and added some BUG_ONs for this. Will submit the patches shortly. - To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
