[PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[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).

Signed-off-by: Robert Hancock <hancockr@shaw.ca>

--- linux-2.6.24-rc3-git1edit/drivers/ata/sata_nv.c.before2	2007-11-22 19:42:28.000000000 -0600
+++ linux-2.6.24-rc3-git1edit/drivers/ata/sata_nv.c	2007-11-22 19:48:25.000000000 -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)

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Mark Lord]

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

RE: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Morrison, Tom]

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: linux-ide-owner@vger.kernel.org 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 majordomo@vger.kernel.org
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]

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

RE: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Morrison, Tom]

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_0000_0000 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:liml@rtr.ca]
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

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Mark Lord]

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_0000_0000 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=0x080000000 (2048MB), size=2048MB: uncachable, count=1  I/O space
reg01: base=0x000000000 (   0MB), size=4096MB: write-back, count=1  first 2GB of RAM + I/O space
reg02: base=0x100000000 (4096MB), size=1024MB: write-back, count=1  third GB of RAM
reg03: base=0x140000000 (5120MB), size= 512MB: write-back, count=1  portion of 4th GB of RAM
reg04: base=0x160000000 (5632MB), size= 256MB: write-back, count=1  portion of 4th GB of RAM
reg05: base=0x170000000 (5888MB), size= 128MB: write-back, count=1  portion of 4th GB of RAM
reg06: base=0x178000000 (6016MB), size=  64MB: write-back, count=1  portion of 4th GB of RAM
reg07: base=0x0af800000 (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

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Robert Hancock]

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 hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Jeff Garzik]

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

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Robert Hancock]

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 hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Mark Lord]

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..

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Tejun Heo]

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 <hancockr@shaw.ca>

Acked-by: Tejun Heo <htejun@gmail.com>

Thanks.

-- 
tejun

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Jeff Garzik]

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 <hancockr@shaw.ca>

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

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Robert Hancock]

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 <hancockr@shaw.ca>
> 
> 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 hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Robert Hancock]

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 <hancockr@shaw.ca>
> 
> 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 hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/

Re: [PATCH] sata_nv: fix ADMA ATAPI issues with memory over 4GB (v3)

[Robert Hancock]

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 <hancockr@shaw.ca>
>>
>> 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 hancockr@nospamshaw.ca
Home Page: http://www.roberthancock.com/