[PATCH] IBM Power RAID driver (ipr) 2.0.6

[PATCH] IBM Power RAID driver (ipr) 2.0.6

[Brian King]

James,

Here is the latest version of the LLD for the IBM Power RAID family of
adapters. This includes several IBM iSeries and pSeries SCSI adapters.
Please apply.

Changes since last submission include:

1. Don't run BIST at module load time since it is unnecessary and it
    increases module load time dramatically.
2. Adapter dump fixes.
3. Add adapter type and microcode version to the dump.
4. Don't zero DMA address of hostrcb during adapter reset, which was causing
    a DMA to address 0.
5. Change to use the scsi_timeout_mod patch rather than the sd_timeout_mod patch


The driver can be found at:

http://www-124.ibm.com/storageio/ipr/patch-2.6.6-rc3-ipr-2.0.6.gz

The patch to provide pci-ids can be found here:

http://www-124.ibm.com/storageio/ipr/patch-2.6.3-ipr-pci-ids

The now required scsi_timeout_mod patch can be found:

http://www-124.ibm.com/storageio/ipr/patch-2.6.6-rc3-scsi_timeout_mod


Thanks


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center

Re: [PATCH] IBM Power RAID driver (ipr) 2.0.6

[Christoph Hellwig]

On Tue, May 04, 2004 at 12:52:35PM -0500, Brian King wrote:
> James,
> 
> Here is the latest version of the LLD for the IBM Power RAID family of
> adapters. This includes several IBM iSeries and pSeries SCSI adapters.
> Please apply.

BTW, any plans to add support for the older iSeries models supported by
the binary only ibmsis driver?

Re: [PATCH] IBM Power RAID driver (ipr) 2.0.6

[Brian King]

Christoph Hellwig wrote:
> On Tue, May 04, 2004 at 12:52:35PM -0500, Brian King wrote:
> 
>>James,
>>
>>Here is the latest version of the LLD for the IBM Power RAID family of
>>adapters. This includes several IBM iSeries and pSeries SCSI adapters.
>>Please apply.
> 
> 
> BTW, any plans to add support for the older iSeries models supported by
> the binary only ibmsis driver?

No. The older models have several "issues" with them. First of all, while
the firmware interface has similarities, there are enough differences to make
writing a driver to support both somewhat ugly. Yes, ibmsis did both, but it
was also a pretty bulky driver. Secondly, the way RAID is done on the legacy
adapters is completely different. It is done how OS/400 needs it done, which
means data striping is not done in the adapter, but rather by the system, as
each device is exposed to the system. Thirdly, the adapters based on the
Chukar chip have lots of "features" with them. To name a few, BIST does not
work. The only way to reset the card is to assert PCI reset. PCI config space
can come up in either big endian or little endian mode, which requires creative
programming to detect and fixup.

Not supporting the legacy adapters resulted in a *much* cleaner OSS driver.

ibmsis will continue to be supported for bug fixes, etc. for the older adapters,
but all new adapters will be supported only by ipr.

-Brian


-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center

Re: [PATCH] IBM Power RAID driver (ipr) 2.0.6

[James Bottomley]

On Tue, 2004-05-04 at 12:52, Brian King wrote:
> Here is the latest version of the LLD for the IBM Power RAID family of
> adapters. This includes several IBM iSeries and pSeries SCSI adapters.
> Please apply.

OK, I put this in the tree.  However, it fails to compile on parisc:

 CC [M]  drivers/scsi/ipr.o
{standard input}: Assembler messages:
{standard input}:1788: Error: Field not properly aligned [8] (1028).
{standard input}:1788: Error: Invalid operands 
[...]

The problem is your use of list_head pointers in packed structures.  You
can't simply pass a pointer to a misaligned structure to a common kernel
function and expect it to work: the packed attribute is lost doing this
and what you end up with is a misaligned pointer reference.

The attached actually allows me to compile, but all it's doing is hiding
the problem.

James

===== drivers/scsi/ipr.h 1.1 vs edited =====
--- 1.1/drivers/scsi/ipr.h	Mon May 10 11:26:37 2004
+++ edited/drivers/scsi/ipr.h	Mon May 10 14:53:55 2004
@@ -666,7 +666,7 @@
 
 	/* Driver added data */
 	u32 hostrcb_dma;
-	struct list_head queue;
+	struct list_head queue __attribute__((aligned(sizeof(unsigned long))));
 }__attribute__((packed, aligned (4)));
 
 #define IPR_HOSTRCB_SZ offsetof(struct ipr_hostrcb, hostrcb_dma)

Re: [PATCH] IBM Power RAID driver (ipr) 2.0.6

[Matthew Wilcox]

On Mon, May 10, 2004 at 03:38:57PM -0500, James Bottomley wrote:
> On Tue, 2004-05-04 at 12:52, Brian King wrote:
> > Here is the latest version of the LLD for the IBM Power RAID family of
> > adapters. This includes several IBM iSeries and pSeries SCSI adapters.
> > Please apply.
> 
> The problem is your use of list_head pointers in packed structures.  You
> can't simply pass a pointer to a misaligned structure to a common kernel
> function and expect it to work: the packed attribute is lost doing this
> and what you end up with is a misaligned pointer reference.

It also causes gcc to pessimise references to this structure; often
doing byte-loads instead of larger quantities.  Why does this struct
need to be packed?

-- 
"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: [PATCH] IBM Power RAID driver (ipr) 2.0.6

[Brian King]

Matthew Wilcox wrote:
> On Mon, May 10, 2004 at 03:38:57PM -0500, James Bottomley wrote:
> 
>>On Tue, 2004-05-04 at 12:52, Brian King wrote:
>>
>>>Here is the latest version of the LLD for the IBM Power RAID family of
>>>adapters. This includes several IBM iSeries and pSeries SCSI adapters.
>>>Please apply.
>>
>>The problem is your use of list_head pointers in packed structures.  You
>>can't simply pass a pointer to a misaligned structure to a common kernel
>>function and expect it to work: the packed attribute is lost doing this
>>and what you end up with is a misaligned pointer reference.
> 
> 
> It also causes gcc to pessimise references to this structure; often
> doing byte-loads instead of larger quantities.  Why does this struct
> need to be packed? 

It is a shared structure with the adapter with a little bit of added
driver data at the end of it. I'll roll a patch to clean this up. The
better way to do this is probably to break out the data that needs to be
packed into its own structure.

-Brian

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center