SCSI: improve endian handling in LSI fusion firmware mpt_downloadboot

PATCH In review Erik Habbinga 2 participants 2 messages Aug 30, 2006 – Aug 31, 2006 LSI Logic MPT fusion drivers (FC/SAS/SPI)

lore

Share & tools

lore

Erik Habbinga

author

2006-08-30 17:33 UTC #0 lore raw

PATCH 1/1 SCSI: improve endian handling in LSI fusion firmware mpt_downloadboot

The mpt_downloadboot function in drivers/message/fusion/mptbase.c doesn't work correctly on big endian systems (powerpc in my case). I've added appropriate le32_to_cpu calls to correctly translate little endian firmware file data into cpu endian format before getting written to little endian PCI registers. This patch has been tested successfully on a powerpc target and an Intel target.

Signed-off-by Erik Habbinga <[email protected]>

drivers/message/fusion/mptbase.c renamed from drivers/message/fusion/mptbase.c.orig +14 ~~-14~~

@@ -2915,7 +2915,7 @@
 	u32 			 ioc_state=0;
 
 	ddlprintk((MYIOC_s_INFO_FMT "downloadboot: fw size 0x%x (%d), FW Ptr %p\n",
-				ioc->name, pFwHeader->ImageSize, pFwHeader->ImageSize, pFwHeader));
+				ioc->name, le32_to_cpu(pFwHeader->ImageSize), le32_to_cpu(pFwHeader->ImageSize), pFwHeader));
 
 	CHIPREG_WRITE32(&ioc->chip->WriteSequence, 0xFF);
 	CHIPREG_WRITE32(&ioc->chip->WriteSequence, MPI_WRSEQ_1ST_KEY_VALUE);

@@ -2968,7 +2968,7 @@
 	/* Set the DiagRwEn and Disable ARM bits */
 	CHIPREG_WRITE32(&ioc->chip->Diagnostic, (MPI_DIAG_RW_ENABLE | MPI_DIAG_DISABLE_ARM));
 
-	fwSize = (pFwHeader->ImageSize + 3)/4;
+	fwSize = (le32_to_cpu(pFwHeader->ImageSize) + 3)/4;
 	ptrFw = (u32 *) pFwHeader;
 
 	/* Write the LoadStartAddress to the DiagRw Address Register

@@ -2977,23 +2977,23 @@
 	if (ioc->errata_flag_1064)
 		pci_enable_io_access(ioc->pcidev);
 
-	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, pFwHeader->LoadStartAddress);
+	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, le32_to_cpu(pFwHeader->LoadStartAddress));
 	ddlprintk((MYIOC_s_INFO_FMT "LoadStart addr written 0x%x \n",
-		ioc->name, pFwHeader->LoadStartAddress));
+		ioc->name, le32_to_cpu(pFwHeader->LoadStartAddress)));
 
 	ddlprintk((MYIOC_s_INFO_FMT "Write FW Image: 0x%x bytes @ %p\n",
 				ioc->name, fwSize*4, ptrFw));
 	while (fwSize--) {
-		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, *ptrFw++);
+		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(*ptrFw++));
 	}
 
-	nextImage = pFwHeader->NextImageHeaderOffset;
+	nextImage = le32_to_cpu(pFwHeader->NextImageHeaderOffset);
 	while (nextImage) {
 		pExtImage = (MpiExtImageHeader_t *) ((char *)pFwHeader + nextImage);
 
-		load_addr = pExtImage->LoadStartAddress;
+		load_addr = le32_to_cpu(pExtImage->LoadStartAddress);
 
-		fwSize = (pExtImage->ImageSize + 3) >> 2;
+		fwSize = (le32_to_cpu(pExtImage->ImageSize) + 3) >> 2;
 		ptrFw = (u32 *)pExtImage;
 
 		ddlprintk((MYIOC_s_INFO_FMT "Write Ext Image: 0x%x (%d) bytes @ %p load_addr=%x\n",

@@ -3001,18 +3001,18 @@
 		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, load_addr);
 
 		while (fwSize--) {
-			CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, *ptrFw++);
+			CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(*ptrFw++));
 		}
-		nextImage = pExtImage->NextImageHeaderOffset;
+		nextImage = le32_to_cpu(pExtImage->NextImageHeaderOffset);
 	}
 
 	/* Write the IopResetVectorRegAddr */
-	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Addr=%x! \n", ioc->name, 	pFwHeader->IopResetRegAddr));
-	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, pFwHeader->IopResetRegAddr);
+	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Addr=%x! \n", ioc->name, 	le32_to_cpu(pFwHeader->IopResetRegAddr)));
+	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, le32_to_cpu(pFwHeader->IopResetRegAddr));
 
 	/* Write the IopResetVectorValue */
-	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Value=%x! \n", ioc->name, pFwHeader->IopResetVectorValue));
-	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, pFwHeader->IopResetVectorValue);
+	ddlprintk((MYIOC_s_INFO_FMT "Write IopResetVector Value=%x! \n", ioc->name, le32_to_cpu(pFwHeader->IopResetVectorValue)));
+	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(pFwHeader->IopResetVectorValue));
 
 	/* Clear the internal flash bad bit - autoincrementing register,
 	 * so must do two writes.

Andrew Morton

2006-08-31 20:09 UTC #1 lore raw

Quoted 31 lines · On Wed, 30 Aug 2006 11:32:47 -0600 "Erik Habbinga"…

The mpt_downloadboot function in drivers/message/fusion/mptbase.c doesn't work correctly on big endian systems (powerpc in my case). I've added appropriate le32_to_cpu calls to correctly translate little endian firmware file data into cpu endian format before getting written to little endian PCI registers. This patch has been tested successfully on a powerpc target and an Intel target.
Signed-off-by Erik Habbinga <[email protected]>
drivers/message/fusion/mptbase.c renamed from drivers/message/fusion/mptbase.c.orig +2 -3
@@ -2915,7 +2915,7 @@
 	u32 			 ioc_state=0;
 
 	ddlprintk((MYIOC_s_INFO_FMT "downloadboot: fw size 0x%x (%d), FW Ptr %p\n",
-				ioc->name, pFwHeader->ImageSize, pFwHeader->ImageSize, pFwHeader));
+				ioc->name, le32_to_cpu(pFwHeader->ImageSize), le32_to_cpu(pFwHeader->ImageSize), pFwHeader));
 
 	CHIPREG_WRITE32(&ioc->chip->WriteSequence, 0xFF);
 	CHIPREG_WRITE32(&ioc->chip->WriteSequence, MPI_WRSEQ_1ST_KEY_VALUE);
@@ -2968,7 +2968,7 @@
 	/* Set the DiagRwEn and Disable ARM bits */
 	CHIPREG_WRITE32(&ioc->chip->Diagnostic, (MPI_DIAG_RW_ENABLE | MPI_DIAG_DISABLE_ARM));
 
-	fwSize = (pFwHeader->ImageSize + 3)/4;
+	fwSize = (le32_to_cpu(pFwHeader->ImageSize) + 3)/4;
 	ptrFw = (u32 *) pFwHeader;
 
 	/* Write the LoadStartAddress to the DiagRw Address Register
@@ -2977,23 +2977,23 @@
 	if (ioc->errata_flag_1064)
 		pci_enable_io_access(ioc->pcidev);
 
-	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, pFwHeader->LoadStartAddress);

drivers/message/fusion/mptbase.c line 2987

Quoted 10 lines

drivers/message/fusion/mptbase.c renamed from drivers/message/fusion/mptbase.c.orig +3 -2

+	CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwAddress, le32_to_cpu(pFwHeader->LoadStartAddress));
 	ddlprintk((MYIOC_s_INFO_FMT "LoadStart addr written 0x%x \n",
-		ioc->name, pFwHeader->LoadStartAddress));
+		ioc->name, le32_to_cpu(pFwHeader->LoadStartAddress)));
 
 	ddlprintk((MYIOC_s_INFO_FMT "Write FW Image: 0x%x bytes @ %p\n",
 				ioc->name, fwSize*4, ptrFw));
 	while (fwSize--) {
-		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, *ptrFw++);
+		CHIPREG_PIO_WRITE32(&ioc->pio_chip->DiagRwData, le32_to_cpu(*ptrFw++));

hm, I'd be a bit more comfortable if this didn't require that le32_to_cpu()
was sanely implemented. include/linux/byteorder/swab.h has

# define __swab32(x) \
(__builtin_constant_p((__u32)(x)) ? \
___swab32((x)) : \
__fswab32((x)))

so if x' is foo++, what value does foo' end up with? I guess it would be pretty dumb if __builtin_constant_p(foo++) were to increment foo, but is that guaranteed?

Anyway. This is a moderately important fix, isn't it? Eric (Moore), do you want to proceed with this patch?

Thanks.