RE: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

RE: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Moore, Eric Dean]

[-- Attachment #1: Type: text/plain, Size: 2282 bytes --]

The reply and request FIFO's have to be within
the same 4GB boundary, as the upper 32 bits
are provided in the IOC Init HostMfaHighAddr field. I
see it crossed boundaries in your post.

I added pci_set_consistent_dma_mask() per request from
Jeremy Higdon<jeremy@sgi.com> back on 12/03/03. He
was having problems on a Altix (IA64) system. See
attached email.  What is your recommendation? I have a mpt 
patch in the queue which doesn't address this.


Eric Moore


On Friday, February 13, 2004 9:12 AM, Alex Williamson wrote:
> 
> 
> Eric,
> 
>    I think I found a bug in the version 3.00.02 mpt driver that's
> currently in Linus' 2.6 tree (latest linux-2.5).  Here's the code
> snippet:
> 
> message/fusion/mptbase.c: PrimeIocFifos()
> ...
>     if (sizeof(dma_addr_t) == sizeof(u64)) {
>         /* Check: upper 32-bits of the request and reply frame
>          * physical addresses must be the same.
>          */
>         if (((u64)ioc->req_frames_dma >> 32) != 
> ((u64)ioc->reply_frames_dma >> 32)){
>             goto out_fail;
>     }
> 
> req_frames_dma and reply_frames_dma are aligned version of 
> dma addresses
> retrieved via two separate pci_alloc_consistent() calls.  The 
> mpt driver
> sets a 64bit consistent dma mask.  How is it reasonable to assume two
> separate mappings will fall within the same 4GB chunk of memory?  I
> recently added support for consistent mapping bypass in the sba_iommu
> driver for the ia64 zx1 chipset.  Randomly on bootup, the MPT driver
> fails.  Checking the mappings, I get a things like this:
> 
> mptbase: Initiating ioc0 bringup
> ioc0: 53C1030: Capabilities={Initiator}
> sba_alloc_coherent() bypassing, dma_handle: 0x1fffb8000, 
> addr: 0xe0000001fffb8000
> sba_alloc_coherent() bypassing, dma_handle: 0x40ffcd8000, 
> addr: 0xe0000040ffcd8000
> PrimeIocFifos() req_frames_dma: 0x40ffcd8000, 
> reply_frames_dma: 0x1fffb8000
> mptbase: WARNING - ioc0 did not initialize properly! (-3)
> 
> These are two perfectly valid consistent mappings as far as 
> the platform
> code is concerned.  I think mpt really needs to either set a 32bit
> consistent map of do a single allocation when it requires this type of
> thing.  Thanks,
> 
> 	Alex
> 
> -- 
> Alex Williamson                             HP Linux & Open Source Lab
> 


[-- Attachment #2: Re: [PATCH] 2.6.0-test9 - MPT Fusion driver 2.05.00.05 update --]
[-- Type: message/rfc822, Size: 2112 bytes --]

From: Jeremy Higdon <jeremy@sgi.com>
To: "Moore, Eric Dean" <Emoore@lsil.com>
Cc: audy@sgi.com, wnunn@sgi.com, gwh@sgi.com
Subject: Re: [PATCH] 2.6.0-test9 - MPT Fusion driver 2.05.00.05 update
Date: Wed, 3 Dec 2003 02:49:59 -0500
Message-ID: <20031203074959.GA178136@sgi.com>

On Mon, Nov 17, 2003 at 11:33:49AM -0500, Moore, Eric Dean wrote:
> James: Please apply the patch below to the 2.6 tree.
> 
> If you find this inline patch malformed,
> please kindly download this patch from this URL:
> ftp://ftp.lsil.com/HostAdapterDrivers/linux/Fusion-MPT/2.6-patches/
> 
> 
> 2.05.00.05 changes
> * error handling fixes, e.g. use of host_lock


Hi Eric,

I applied your patch to my LSI driver.  If the card sees all the devices,
it seems to work.  However, often times the card won't see the devices until
I manually reset it using lsiutil.

Here are the vital statistics:

Firmware image's version is LSIFC929X-1.00.00 (2003.02.28)
BIOS image's version is FC MPTBIOS-2.01.02.00 (2003.03.24)
FCode image's version is MPT FC FCode Version 1.00.29 (2002.10.21)


I'm testing on an Altix (IA64) system.  By the way, here is a patch that
is necessary for it to work.  This applies on top of your 2.05.00.05 patch.

Also, once I've run lsiutil to reset and probe for devices, how do you
get the SCSI subsystem to probe the driver?
I thought we could
	
	echo 1 > /sys/class/scsi_host/host17/scan

but that doesn't seem to work.

thanks

jeremy


===== drivers/message/fusion/mptbase.c 1.14 vs edited =====
--- 1.14/drivers/message/fusion/mptbase.c	Wed Aug 13 23:10:03 2003
+++ edited/drivers/message/fusion/mptbase.c	Tue Dec  2 22:54:31 2003
@@ -1292,6 +1292,11 @@
 		return r;
 	}
 
+	if (pci_set_consistent_dma_mask(pdev, mask))
+		printk(KERN_INFO "Not using 64 bit consistent mask\n");
+	else
+		printk(KERN_INFO "LSI: Using 64 bit consistent mask\n");
+
 	ioc = kmalloc(sizeof(MPT_ADAPTER), GFP_ATOMIC);
 	if (ioc == NULL) {
 		printk(KERN_ERR MYNAM ": ERROR - Insufficient memory to add
adapter!\n");

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Jesse Barnes]

On Fri, Feb 13, 2004 at 04:20:35PM -0500, Moore, Eric Dean wrote:
> I added pci_set_consistent_dma_mask() per request from
> Jeremy Higdon<jeremy@sgi.com> back on 12/03/03. He
> was having problems on a Altix (IA64) system. See
> attached email.  What is your recommendation? I have a mpt 
> patch in the queue which doesn't address this.
> 
> On Friday, February 13, 2004 9:12 AM, Alex Williamson wrote:
> > These are two perfectly valid consistent mappings as far as the
> > platform code is concerned.  I think mpt really needs to either set
> > a 32bit consistent map of do a single allocation when it requires
> > this type of thing.  Thanks,

I think Alex's second suggestion would be the best--do a single
allocation rather than falling back to a 32 bit consistent mask.

Jesse

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Bjorn Helgaas]

On Friday 13 February 2004 2:30 pm, Jesse Barnes wrote:
> On Fri, Feb 13, 2004 at 04:20:35PM -0500, Moore, Eric Dean wrote:
> > I added pci_set_consistent_dma_mask() per request from
> > Jeremy Higdon<jeremy@sgi.com> back on 12/03/03. He
> > was having problems on a Altix (IA64) system. See
> > attached email.  What is your recommendation? I have a mpt 
> > patch in the queue which doesn't address this.
> > 
> > On Friday, February 13, 2004 9:12 AM, Alex Williamson wrote:
> > > These are two perfectly valid consistent mappings as far as the
> > > platform code is concerned.  I think mpt really needs to either set
> > > a 32bit consistent map of do a single allocation when it requires
> > > this type of thing.  Thanks,
> 
> I think Alex's second suggestion would be the best--do a single
> allocation rather than falling back to a 32 bit consistent mask.

But even with a single allocation, there's nothing to prevent it
from crossing a 4GB boundary, is there?

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Jeremy Higdon]

On Fri, Feb 13, 2004 at 03:52:43PM -0700, Bjorn Helgaas wrote:
> On Friday 13 February 2004 2:30 pm, Jesse Barnes wrote:
> > On Fri, Feb 13, 2004 at 04:20:35PM -0500, Moore, Eric Dean wrote:
> > > I added pci_set_consistent_dma_mask() per request from
> > > Jeremy Higdon<jeremy@sgi.com> back on 12/03/03. He
> > > was having problems on a Altix (IA64) system. See
> > > attached email.  What is your recommendation? I have a mpt 
> > > patch in the queue which doesn't address this.
> > > 
> > > On Friday, February 13, 2004 9:12 AM, Alex Williamson wrote:
> > > > These are two perfectly valid consistent mappings as far as the
> > > > platform code is concerned.  I think mpt really needs to either set
> > > > a 32bit consistent map of do a single allocation when it requires
> > > > this type of thing.  Thanks,
> > 
> > I think Alex's second suggestion would be the best--do a single
> > allocation rather than falling back to a 32 bit consistent mask.
> 
> But even with a single allocation, there's nothing to prevent it
> from crossing a 4GB boundary, is there?

I don't think the Linux memory allocator will give back memory that
crosses a 32 bit boundary.

But I'm not an expert.  If I'm wrong, hopefully an expert will chime
in  :-)

jeremy

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Matthew Wilcox]

On Fri, Feb 13, 2004 at 03:52:43PM -0700, Bjorn Helgaas wrote:
> > I think Alex's second suggestion would be the best--do a single
> > allocation rather than falling back to a 32 bit consistent mask.
> 
> But even with a single allocation, there's nothing to prevent it
> from crossing a 4GB boundary, is there?

Since we use buddy allocators everywhere, we'd need to be doing 8GB
allocations to cross a 4GB barrier.  Of course if we change allocators ...

-- 
"Next the statesmen will invent cheap lies, putting the blame upon 
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince 
himself that the war is just, and will thank God for the better sleep 
he enjoys after this process of grotesque self-deception." -- Mark Twain

RE: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Moore, Eric Dean]

I will investigate, and see what can be done.
Thanks for your feedback.

Eric

> 
> 
> On Fri, Feb 13, 2004 at 03:52:43PM -0700, Bjorn Helgaas wrote:
> > > I think Alex's second suggestion would be the best--do a single
> > > allocation rather than falling back to a 32 bit consistent mask.
> > 
> > But even with a single allocation, there's nothing to prevent it
> > from crossing a 4GB boundary, is there?
> 
> Since we use buddy allocators everywhere, we'd need to be doing 8GB
> allocations to cross a 4GB barrier.  Of course if we change 
> allocators ...
> 
> -- 
> "Next the statesmen will invent cheap lies, putting the blame upon 
> the nation that is attacked, and every man will be glad of those
> conscience-soothing falsities, and will diligently study 
> them, and refuse
> to examine any refutations of them; and thus he will by and 
> by convince 
> himself that the war is just, and will thank God for the better sleep 
> he enjoys after this process of grotesque self-deception." -- 
> Mark Twain
> 

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Jeremy Higdon]

On Fri, Feb 13, 2004 at 11:13:08PM +0000, Matthew Wilcox wrote:
> On Fri, Feb 13, 2004 at 03:52:43PM -0700, Bjorn Helgaas wrote:
> > > I think Alex's second suggestion would be the best--do a single
> > > allocation rather than falling back to a 32 bit consistent mask.
> > 
> > But even with a single allocation, there's nothing to prevent it
> > from crossing a 4GB boundary, is there?
> 
> Since we use buddy allocators everywhere, we'd need to be doing 8GB
> allocations to cross a 4GB barrier.  Of course if we change allocators ...

... and this particular behavior, then I think lots of hardware will
stop working, because LSI is not the only vendor with this sort of
limitation  :-)

jeremy

RE: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Moore, Eric Dean]

Regarding last weeks thread
on mpt fusion driver(2.6 kernel), and PrimeIocFifos:

The problem we face in combining two pci_alloc_consistent
for reply and request fifo into one memory allocation is 
there is a third pci_alloc_consistent call from mptscsih, 
which creates a chain buffer pool. The chain buffer pool
is a pool of extra memory which links additional message
frames to the request fifo.  The original author of these
drivers thought it was best to separate this memory allocation
into a separate device driver to conserve memory in mptbase. 
It will be a lot of work/testing at this time to combine 
three alloc's to one memory allocation, as code is being 
located in two separate device drivers.

If this fix is urgent, I recommend providing a quick 
fix which is creating 32bit pci_dev mask just for these 
three pci_alloc_consistent calls. 

Eric Moore
LSI Logic



On Friday, February 13, 2004 9:12 AM, Alex Williamson wrote:
> 
> 
> Eric,
> 
>    I think I found a bug in the version 3.00.02 mpt driver that's
> currently in Linus' 2.6 tree (latest linux-2.5).  Here's the code
> snippet:
> 
> message/fusion/mptbase.c: PrimeIocFifos()
> ...
>     if (sizeof(dma_addr_t) == sizeof(u64)) {
>         /* Check: upper 32-bits of the request and reply frame
>          * physical addresses must be the same.
>          */
>         if (((u64)ioc->req_frames_dma >> 32) != 
> ((u64)ioc->reply_frames_dma >> 32)){
>             goto out_fail;
>     }
> 
> req_frames_dma and reply_frames_dma are aligned version of 
> dma addresses
> retrieved via two separate pci_alloc_consistent() calls.  The 
> mpt driver
> sets a 64bit consistent dma mask.  How is it reasonable to assume two
> separate mappings will fall within the same 4GB chunk of memory?  I
> recently added support for consistent mapping bypass in the sba_iommu
> driver for the ia64 zx1 chipset.  Randomly on bootup, the MPT driver
> fails.  Checking the mappings, I get a things like this:
> 
> mptbase: Initiating ioc0 bringup
> ioc0: 53C1030: Capabilities={Initiator}
> sba_alloc_coherent() bypassing, dma_handle: 0x1fffb8000, 
> addr: 0xe0000001fffb8000
> sba_alloc_coherent() bypassing, dma_handle: 0x40ffcd8000, 
> addr: 0xe0000040ffcd8000
> PrimeIocFifos() req_frames_dma: 0x40ffcd8000, 
> reply_frames_dma: 0x1fffb8000
> mptbase: WARNING - ioc0 did not initialize properly! (-3)
> 
> These are two perfectly valid consistent mappings as far as 
> the platform
> code is concerned.  I think mpt really needs to either set a 32bit
> consistent map of do a single allocation when it requires this type of
> thing.  Thanks,
> 
> 	Alex
> 
> -- 
> Alex Williamson                             HP Linux & Open Source Lab
> 

RE: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Alex Williamson]

On Wed, 2004-02-18 at 11:41, Moore, Eric Dean wrote:
> Regarding last weeks thread
> on mpt fusion driver(2.6 kernel), and PrimeIocFifos:
> 
[snip... a third allocation dependency]

> If this fix is urgent, I recommend providing a quick 
> fix which is creating 32bit pci_dev mask just for these 
> three pci_alloc_consistent calls. 
> 

   The current failure mode is pretty ugly and appears fairly random.  I
would consider getting something that works reliably to be urgent.  It
sounds easy and safe to twiddle the consistent device mask in the short
term and combine these allocations in a future rev.  Thanks,

	Alex

-- 
Alex Williamson                             HP Linux & Open Source Lab

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Jeremy Higdon]

On Wed, Feb 18, 2004 at 11:52:57AM -0700, Alex Williamson wrote:
> On Wed, 2004-02-18 at 11:41, Moore, Eric Dean wrote:
> > Regarding last weeks thread
> > on mpt fusion driver(2.6 kernel), and PrimeIocFifos:
> > 
> [snip... a third allocation dependency]
> 
> > If this fix is urgent, I recommend providing a quick 
> > fix which is creating 32bit pci_dev mask just for these 
> > three pci_alloc_consistent calls. 
> > 
> 
>    The current failure mode is pretty ugly and appears fairly random.  I
> would consider getting something that works reliably to be urgent.  It
> sounds easy and safe to twiddle the consistent device mask in the short
> term and combine these allocations in a future rev.  Thanks,
> 
> 	Alex


Unfortunately, the SGI platform needs 64 bit addresses for consistent
allocations.  However, since that's not reliable, I guess we'll just
need to wait for a fix that combines the allocations.

Eric, do you have any idea how long this will take?

thanks

jeremy

RE: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Moore, Eric Dean]

Jeremy - thanks for your reply. Just wanted to clarify
if this would work on your system:


mptbase_probe(pdev,id) {

u64 mask64 = 0xFFFFFFFFFFFFFFFF;
u32 mask32 = 0xFFFFFFFF;

pci_enable_device(pdev);

ioc->pcidev32 = pdev;
ioc->pcidev64 = pdev;

r = pci_set_dma_mask(ioc->pcidev64, mask64 );
r = pci_set_consistent_dma_mask(ioc->pcidev64, mask64 );
r = pci_set_dma_mask(ioc->pcidev32, mask32 );
r = pci_set_consistent_dma_mask(ioc->pcidev32, mask32 );

/* use 
 * ioc->pcidev32 
 * for the three fifo memory allocations for < 4gb  
 *
 * use
 * ioc->pcidev64 
 * for all other allocations
 */

}


On Wednesday, February 18, 2004 4:21 PM, Jeremy Higdon wrote:
> 
> On Wed, Feb 18, 2004 at 11:52:57AM -0700, Alex Williamson wrote:
> > On Wed, 2004-02-18 at 11:41, Moore, Eric Dean wrote:
> > > Regarding last weeks thread
> > > on mpt fusion driver(2.6 kernel), and PrimeIocFifos:
> > > 
> > [snip... a third allocation dependency]
> > 
> > > If this fix is urgent, I recommend providing a quick 
> > > fix which is creating 32bit pci_dev mask just for these 
> > > three pci_alloc_consistent calls. 
> > > 
> > 
> >    The current failure mode is pretty ugly and appears 
> fairly random.  I
> > would consider getting something that works reliably to be 
> urgent.  It
> > sounds easy and safe to twiddle the consistent device mask 
> in the short
> > term and combine these allocations in a future rev.  Thanks,
> > 
> > 	Alex
> 
> 
> Unfortunately, the SGI platform needs 64 bit addresses for consistent
> allocations.  However, since that's not reliable, I guess we'll just
> need to wait for a fix that combines the allocations.
> 
> Eric, do you have any idea how long this will take?
> 
> thanks
> 
> jeremy
> 

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Jeremy Higdon]

On Wed, Feb 18, 2004 at 07:03:47PM -0500, Moore, Eric Dean wrote:
> Jeremy - thanks for your reply. Just wanted to clarify
> if this would work on your system:
> 
> 
> mptbase_probe(pdev,id) {
> 
> u64 mask64 = 0xFFFFFFFFFFFFFFFF;
> u32 mask32 = 0xFFFFFFFF;
> 
> pci_enable_device(pdev);
> 
> ioc->pcidev32 = pdev;
> ioc->pcidev64 = pdev;
> 
> r = pci_set_dma_mask(ioc->pcidev64, mask64 );
> r = pci_set_consistent_dma_mask(ioc->pcidev64, mask64 );
> r = pci_set_dma_mask(ioc->pcidev32, mask32 );
> r = pci_set_consistent_dma_mask(ioc->pcidev32, mask32 );
> 
> /* use 
>  * ioc->pcidev32 
>  * for the three fifo memory allocations for < 4gb  
>  *
>  * use
>  * ioc->pcidev64 
>  * for all other allocations
>  */
> 
> }

I don't think so, for two reasons.

The first is that we require 64 bit PCI addresses for all PCI-X devices.

The second is that if I understand your pseudo-code correctly, the pcidev64
and pcidev32 pointers are both pointing to the same struct pci_dev, which
would mean that the second pci_set_consistent_dma_mask call would override
the first.

Hopefully it's not too much effort to change it to a single consistent
allocation for all three FIFO chunks.  I don't see another way around
this.

jeremy

RE: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Moore, Eric Dean]

My comments <EDM> are embedded below.

On Wednesday, February 18, 2004 6:10 PM,Jeremy Higdon wrote: 
> 
> On Wed, Feb 18, 2004 at 07:03:47PM -0500, Moore, Eric Dean wrote:
> > Jeremy - thanks for your reply. Just wanted to clarify
> > if this would work on your system:
> > 
> > 
> > mptbase_probe(pdev,id) {
> > 
> > u64 mask64 = 0xFFFFFFFFFFFFFFFF;
> > u32 mask32 = 0xFFFFFFFF;
> > 
> > pci_enable_device(pdev);
> > 
> > ioc->pcidev32 = pdev;
> > ioc->pcidev64 = pdev;
> > 
> > r = pci_set_dma_mask(ioc->pcidev64, mask64 );
> > r = pci_set_consistent_dma_mask(ioc->pcidev64, mask64 );
> > r = pci_set_dma_mask(ioc->pcidev32, mask32 );
> > r = pci_set_consistent_dma_mask(ioc->pcidev32, mask32 );
> > 
> > /* use 
> >  * ioc->pcidev32 
> >  * for the three fifo memory allocations for < 4gb  
> >  *
> >  * use
> >  * ioc->pcidev64 
> >  * for all other allocations
> >  */
> > 
> > }
> 
> I don't think so, for two reasons.
> 
> The first is that we require 64 bit PCI addresses for all 
> PCI-X devices.

<EDM> 
I'm not sure what you mean? 
For PCI-X devices - are you saying passing something other than
0xFFFFFFFFFFFFFFFF for the mask in pci_set_consistent_dma_mask()
fails? 

Does your system have restrictions below 4GB?

I would believe pci_alloc_consistent() call would work 
if memory allocation is restricted to below < 4GB.


> 
> The second is that if I understand your pseudo-code 
> correctly, the pcidev64
> and pcidev32 pointers are both pointing to the same struct 
> pci_dev, which
> would mean that the second pci_set_consistent_dma_mask call 
> would override
> the first.

<EDM> 
Your right. My mistake in the example. I would be allocating
seperate memory for the 32bit pcidev.

> 
> Hopefully it's not too much effort to change it to a single consistent
> allocation for all three FIFO chunks.  I don't see another way around
> this.

<EDM> 
This driver is in current code freeze as LSI as I have up-ported 
bug fixes/enhances back from the 2.4 kernel version of the mpt driver.
The 2.4 kernel version of the mpt driver is in current test cycle
for the next couple weeks.  

Adding 32bit dma handles is much less impact to the driver, and
I could go ahead and send you a test driver to see if this would work
later today.

However merging the 3 fifo allocations would need to wait till
after this release due to the nature of the change.  I need
to wait till something like the first week of March.

Eric

  

Re: mpt sets 64bit consistent mask yet requires mapping in same 4 GB chunk?!?

[Jeremy Higdon]

On Thu, Feb 19, 2004 at 12:18:07PM -0500, Moore, Eric Dean wrote:
> My comments <EDM> are embedded below.
> 
> > I don't think so, for two reasons.
> > 
> > The first is that we require 64 bit PCI addresses for all 
> > PCI-X devices.
> 
> <EDM> 
> I'm not sure what you mean? 
> For PCI-X devices - are you saying passing something other than
> 0xFFFFFFFFFFFFFFFF for the mask in pci_set_consistent_dma_mask()
> fails? 

Correct.  We require PCI-X devices to use the entire 64 address
bits.

jeremy