mpt sets 64bit consistent mask yet requires mapping in same 4GB

mpt sets 64bit consistent mask yet requires mapping in same 4GB

[Alex Williamson]

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

[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?!?

[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

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