[PATCH 2.6.16.18 0/4] sbp2: workaround for buggy iPods

[PATCH 2.6.16.18 0/4] sbp2: workaround for buggy iPods

[Stefan Richter]

There is a firmware bug in several Apple iPods which prevents access to
these iPods under certain conditions. The disk size reported by the iPod
is one sector too big. Once access to the end of the disk is attempted,
the iPod becomes inaccessible. This problem has been known for USB iPods
for some time and has recently been discovered to exist with
FireWire/USB combo iPods too.

The following patchset is the fix as it exists in Linux 2.6.17-rc. Alas
it is rather large, therefore it may be unfit for -stable as it is. If
there are objections, I would appreciate suggestions how to better adapt
this fix for -stable.

The necessary workaround is added this way:

patch 1/4: sbp2: consolidate workarounds, part one
patch 2/4: sbp2: consolidate workarounds, part two
    Infrastructure for existing *unrelated* workarounds is refactored.
    This concerns (a) module load parameters to activate workarounds,
    (b) a hardwired blacklist of known buggy devices, (c) detection of
    known buggy devices, (d) activation of the various workarounds.
    Benefits of the refactoring are better readability, extensibility,
    and finer-grained control.
    This is a single patch in 2.6.17-rc; I split it into two due to its
    size (the essential part and a part affecting comments, log messages
    etc.).

patch 3/4: add read_capacity workaround for iPod
    Extends blacklist and device detection, and adds the actual
    workaround.

patch 4/4: sbp2: add ability to override hardwired blacklist
    As we add more workarounds, potential to adversely affect other
    devices increases. This patch adds a simple feature as a safety
    belt: A new flag for the module load parameter from patch 1/4
    tells sbp2 not to use its hardwired blacklist.

Combined diffstat of patches 1/4...4/4:
 Documentation/feature-removal-schedule.txt |    9 +
 drivers/ieee1394/sbp2.c                    |  206 +++++++++++++++++++----------
 drivers/ieee1394/sbp2.h                    |   18 +-
 3 files changed, 159 insertions(+), 74 deletions(-)

So this is much more than is usually acceptable for -stable. Keeping it
this big has the benefit of minimal deviation from 2.6.17+, concerning
the code as well as sbp2's module load parameters. As I said, I could
try to rework this for minimum patch size if so desired by the -stable
team. But I suppose I would still end up with a rather big patch.

The impact of leaving this iPod workaround out of -stable is limited
because there are a few alternatives:
 - Experienced users of affected hardware may compile 2.6.16 and older
   without support for EFI GUID partition tables. This should make
   access to the end of the disk rather unlikely.
 - Experienced users of affected hardware may switch to 2.6.17+.
 - Experienced users or distributors may apply this patchset or a more
   extensive patchset for the 1394 subsystem on their own. (I am
   maintaining a bunch of rediffs of current 1394 patches for released
   kernels.)
-- 
Stefan Richter
-=====-=-==- -=-= ==-==
http://arcgraph.de/sr/

[PATCH 2.6.16.18 1/4] sbp2: consolidate workarounds, part one

[Stefan Richter]

Grand unification of the three types of workarounds we have so far.

The "skip mode page 8" workaround is now limited to devices which
pretend to be of TYPE_DISK instead of TYPE_RBC. This workaround is no
longer enabled for Initio bridges.

Patch update in anticipation of more workarounds:
 - Add module parameter "workarounds".
 - Deprecate parameter "force_inquiry_hack".
 - Compose the blacklist of a compound type for better readability and
   extensibility.
 - Remove a now unused #define.

This patch is required for the subsequent patch "sbp2: add read_capacity
workaround for iPod". For better readability, the patch is split into
the functional part (this one) and a merely cosmetic part (following).

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
rediff for -stable, from commit 24d3bf884e093f9de52d31c97187f4b9b4ad7dcb

 drivers/ieee1394/sbp2.c |   86 +++++++++++++++++++++++++++++++-----------------
 drivers/ieee1394/sbp2.h |    7 +++
 2 files changed, 63 insertions(+), 30 deletions(-)

Index: linux-2.6.16.18/drivers/ieee1394/sbp2.h
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.h	2006-05-27 13:23:24.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.h	2006-05-27 13:24:17.000000000 +0200
@@ -239,6 +239,11 @@ struct sbp2_status_block {
 #define SBP2_MAX_SECTORS		255	/* Max sectors supported */
 #define SBP2_MAX_CMDS			8	/* This should be safe */
 
+/* Flags for detected oddities and brokeness */
+#define SBP2_WORKAROUND_128K_MAX_TRANS	0x1
+#define SBP2_WORKAROUND_INQUIRY_36	0x2
+#define SBP2_WORKAROUND_MODE_SENSE_8	0x4
+
 /* This is the two dma types we use for cmd_dma below */
 enum cmd_dma_types {
 	CMD_DMA_NONE,
@@ -345,7 +350,7 @@ struct scsi_id_instance_data {
 	struct Scsi_Host *scsi_host;
 
 	/* Device specific workarounds/brokeness */
-	u32 workarounds;
+	unsigned workarounds;
 };
 
 /* Sbp2 host data structure (one per IEEE1394 host) */
Index: linux-2.6.16.18/drivers/ieee1394/sbp2.c
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.c	2006-05-27 13:23:50.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.c	2006-05-27 13:24:17.000000000 +0200
@@ -42,6 +42,7 @@
 #include <linux/kernel.h>
 #include <linux/list.h>
 #include <linux/string.h>
+#include <linux/stringify.h>
 #include <linux/slab.h>
 #include <linux/interrupt.h>
 #include <linux/fs.h>
@@ -134,6 +135,14 @@ static int exclusive_login = 1;
 module_param(exclusive_login, int, 0644);
 MODULE_PARM_DESC(exclusive_login, "Exclusive login to sbp2 device (default = 1)");
 
+static int sbp2_default_workarounds;
+module_param_named(workarounds, sbp2_default_workarounds, int, 0644);
+MODULE_PARM_DESC(workarounds, "Work around device bugs (default = 0"
+	", 128kB max transfer = " __stringify(SBP2_WORKAROUND_128K_MAX_TRANS)
+	", 36 byte inquiry = "    __stringify(SBP2_WORKAROUND_INQUIRY_36)
+	", skip mode page 8 = "   __stringify(SBP2_WORKAROUND_MODE_SENSE_8)
+	", or a combination)");
+
 /*
  * SCSI inquiry hack for really badly behaved sbp2 devices. Turn this on
  * if your sbp2 device is not properly handling the SCSI inquiry command.
@@ -268,11 +277,23 @@ static struct hpsb_protocol_driver sbp2_
  * List of device firmwares that require the inquiry hack.
  * Yields a few false positives but did not break other devices so far.
  */
-static u32 sbp2_broken_inquiry_list[] = {
-	0x00002800,	/* Stefan Richter <stefanr@s5r6.in-berlin.de> */
-			/* DViCO Momobay CX-1 */
-	0x00000200	/* Andreas Plesch <plesch@fas.harvard.edu> */
-			/* QPS Fire DVDBurner */
+static const struct {
+	u32 firmware_revision;
+	unsigned workarounds;
+} sbp2_workarounds_table[] = {
+	/* TSB42AA9 */ {
+		.firmware_revision	= 0x002800,
+		.workarounds		= SBP2_WORKAROUND_INQUIRY_36 |
+					  SBP2_WORKAROUND_MODE_SENSE_8,
+	},
+	/* Initio bridges, actually only needed for some older ones */ {
+		.firmware_revision	= 0x000200,
+		.workarounds		= SBP2_WORKAROUND_INQUIRY_36,
+	},
+	/* Symbios bridge */ {
+		.firmware_revision	= 0xa0b800,
+		.workarounds		= SBP2_WORKAROUND_128K_MAX_TRANS,
+	}
 };
 
 /**************************************
@@ -1477,7 +1498,8 @@ static void sbp2_parse_unit_directory(st
 	struct csr1212_dentry *dentry;
 	u64 management_agent_addr;
 	u32 command_set_spec_id, command_set, unit_characteristics,
-	    firmware_revision, workarounds;
+	    firmware_revision;
+	unsigned workarounds;
 	int i;
 
 	SBP2_DEBUG("sbp2_parse_unit_directory");
@@ -1548,7 +1570,7 @@ static void sbp2_parse_unit_directory(st
 
 	/* This is the start of our broken device checking. We try to hack
 	 * around oddities and known defects.  */
-	workarounds = 0x0;
+	workarounds = sbp2_default_workarounds;
 
 	/* If the vendor id is 0xa0b8 (Symbios vendor id), then we have a
 	 * bridge with 128KB max transfer size limitation. For sanity, we
@@ -1559,29 +1581,28 @@ static void sbp2_parse_unit_directory(st
 	 * host gets initialized. That way we can down-force the
 	 * max_sectors to account for it. That is not currently
 	 * possible.  */
-	if ((firmware_revision & 0xffff00) ==
-			SBP2_128KB_BROKEN_FIRMWARE &&
-			(max_sectors * 512) > (128*1024)) {
-		SBP2_WARN("Node " NODE_BUS_FMT ": Bridge only supports 128KB max transfer size.",
-				NODE_BUS_ARGS(ud->ne->host, ud->ne->nodeid));
-		SBP2_WARN("WARNING: Current max_sectors setting is larger than 128KB (%d sectors)!",
-				max_sectors);
-		workarounds |= SBP2_BREAKAGE_128K_MAX_TRANSFER;
-	}
-
 	/* Check for a blacklisted set of devices that require us to force
 	 * a 36 byte host inquiry. This can be overriden as a module param
 	 * (to force all hosts).  */
-	for (i = 0; i < ARRAY_SIZE(sbp2_broken_inquiry_list); i++) {
-		if ((firmware_revision & 0xffff00) ==
-				sbp2_broken_inquiry_list[i]) {
-			SBP2_WARN("Node " NODE_BUS_FMT ": Using 36byte inquiry workaround",
-					NODE_BUS_ARGS(ud->ne->host, ud->ne->nodeid));
-			workarounds |= SBP2_BREAKAGE_INQUIRY_HACK;
-			break; /* No need to continue. */
-		}
+	if (force_inquiry_hack)
+		workarounds |= SBP2_WORKAROUND_INQUIRY_36;
+
+	for (i = 0; i < ARRAY_SIZE(sbp2_workarounds_table); i++) {
+		if (sbp2_workarounds_table[i].firmware_revision !=
+		    (firmware_revision & 0xffff00))
+			continue;
+		workarounds |= sbp2_workarounds_table[i].workarounds;
+		break;
 	}
 
+	if (workarounds & SBP2_WORKAROUND_128K_MAX_TRANS &&
+	    (max_sectors * 512) > (128 * 1024))
+		SBP2_WARN("Node " NODE_BUS_FMT ": Bridge only supports 128KB "
+			  "max transfer size. WARNING: Current max_sectors "
+			  "setting is larger than 128KB (%d sectors)",
+			  NODE_BUS_ARGS(ud->ne->host, ud->ne->nodeid),
+			  max_sectors);
+
 	/* If this is a logical unit directory entry, process the parent
 	 * to get the values. */
 	if (ud->flags & UNIT_DIRECTORY_LUN_DIRECTORY) {
@@ -2481,19 +2502,23 @@ static int sbp2scsi_slave_alloc(struct s
 
 	scsi_id->sdev = sdev;
 
-	if (force_inquiry_hack ||
-	    scsi_id->workarounds & SBP2_BREAKAGE_INQUIRY_HACK) {
+	if (scsi_id->workarounds & SBP2_WORKAROUND_INQUIRY_36)
 		sdev->inquiry_len = 36;
-		sdev->skip_ms_page_8 = 1;
-	}
 	return 0;
 }
 
 static int sbp2scsi_slave_configure(struct scsi_device *sdev)
 {
+	struct scsi_id_instance_data *scsi_id =
+		(struct scsi_id_instance_data *)sdev->host->hostdata[0];
+
 	blk_queue_dma_alignment(sdev->request_queue, (512 - 1));
 	sdev->use_10_for_rw = 1;
 	sdev->use_10_for_ms = 1;
+
+	if (sdev->type == TYPE_DISK &&
+	    scsi_id->workarounds & SBP2_WORKAROUND_MODE_SENSE_8)
+		sdev->skip_ms_page_8 = 1;
 	return 0;
 }
 
@@ -2638,6 +2663,9 @@ static int sbp2_module_init(void)
 	}
 
 	/* Set max sectors (module load option). Default is 255 sectors. */
+	if (sbp2_default_workarounds & SBP2_WORKAROUND_128K_MAX_TRANS &&
+	    (max_sectors * 512) > (128 * 1024))
+		max_sectors = 128 * 1024 / 512;
 	scsi_driver_template.max_sectors = max_sectors;
 
 	/* Register our high level driver with 1394 stack */

[PATCH 2.6.16.18 2/4] sbp2: consolidate workarounds, part two

[Stefan Richter]

rest of the original patch

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
rediff for -stable, from commit 24d3bf884e093f9de52d31c97187f4b9b4ad7dcb

 Documentation/feature-removal-schedule.txt |    9 +++
 drivers/ieee1394/sbp2.c                    |   76 ++++++++++++++---------------
 drivers/ieee1394/sbp2.h                    |    9 ---
 3 files changed, 48 insertions(+), 46 deletions(-)

Index: linux-2.6.16.18/drivers/ieee1394/sbp2.h
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.h	2006-05-27 13:24:17.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.h	2006-05-27 13:26:01.000000000 +0200
@@ -227,11 +227,6 @@ struct sbp2_status_block {
 #define SBP2_SW_VERSION_ENTRY					0x00010483
 
 /*
- * Other misc defines
- */
-#define SBP2_128KB_BROKEN_FIRMWARE				0xa0b800
-
-/*
  * SCSI specific stuff
  */
 
@@ -273,10 +268,6 @@ struct sbp2_command_info {
 
 };
 
-/* A list of flags for detected oddities and brokeness. */
-#define SBP2_BREAKAGE_128K_MAX_TRANSFER		0x1
-#define SBP2_BREAKAGE_INQUIRY_HACK		0x2
-
 struct sbp2scsi_host_info;
 
 /*
Index: linux-2.6.16.18/drivers/ieee1394/sbp2.c
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.c	2006-05-27 13:24:17.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.c	2006-05-27 13:26:01.000000000 +0200
@@ -118,7 +118,8 @@ MODULE_PARM_DESC(serialize_io, "Serializ
  */
 static int max_sectors = SBP2_MAX_SECTORS;
 module_param(max_sectors, int, 0444);
-MODULE_PARM_DESC(max_sectors, "Change max sectors per I/O supported (default = 255)");
+MODULE_PARM_DESC(max_sectors, "Change max sectors per I/O supported (default = "
+		 __stringify(SBP2_MAX_SECTORS) ")");
 
 /*
  * Exclusive login to sbp2 device? In most cases, the sbp2 driver should
@@ -135,6 +136,22 @@ static int exclusive_login = 1;
 module_param(exclusive_login, int, 0644);
 MODULE_PARM_DESC(exclusive_login, "Exclusive login to sbp2 device (default = 1)");
 
+/*
+ * If any of the following workarounds is required for your device to work,
+ * please submit the kernel messages logged by sbp2 to the linux1394-devel
+ * mailing list.
+ *
+ * - 128kB max transfer
+ *   Limit transfer size. Necessary for some old bridges.
+ *
+ * - 36 byte inquiry
+ *   When scsi_mod probes the device, let the inquiry command look like that
+ *   from MS Windows.
+ *
+ * - skip mode page 8
+ *   Suppress sending of mode_sense for mode page 8 if the device pretends to
+ *   support the SCSI Primary Block commands instead of Reduced Block Commands.
+ */
 static int sbp2_default_workarounds;
 module_param_named(workarounds, sbp2_default_workarounds, int, 0644);
 MODULE_PARM_DESC(workarounds, "Work around device bugs (default = 0"
@@ -143,19 +160,10 @@ MODULE_PARM_DESC(workarounds, "Work arou
 	", skip mode page 8 = "   __stringify(SBP2_WORKAROUND_MODE_SENSE_8)
 	", or a combination)");
 
-/*
- * SCSI inquiry hack for really badly behaved sbp2 devices. Turn this on
- * if your sbp2 device is not properly handling the SCSI inquiry command.
- * This hack makes the inquiry look more like a typical MS Windows inquiry
- * by enforcing 36 byte inquiry and avoiding access to mode_sense page 8.
- *
- * If force_inquiry_hack=1 is required for your device to work,
- * please submit the logged sbp2_firmware_revision value of this device to
- * the linux1394-devel mailing list.
- */
+/* legacy parameter */
 static int force_inquiry_hack;
 module_param(force_inquiry_hack, int, 0644);
-MODULE_PARM_DESC(force_inquiry_hack, "Force SCSI inquiry hack (default = 0)");
+MODULE_PARM_DESC(force_inquiry_hack, "Deprecated, use 'workarounds'");
 
 /*
  * Export information about protocols/devices supported by this driver.
@@ -274,8 +282,11 @@ static struct hpsb_protocol_driver sbp2_
 };
 
 /*
- * List of device firmwares that require the inquiry hack.
- * Yields a few false positives but did not break other devices so far.
+ * List of devices with known bugs.
+ *
+ * The firmware_revision field, masked with 0xffff00, is the best indicator
+ * for the type of bridge chip of a device.  It yields a few false positives
+ * but this did not break correctly behaving devices so far.
  */
 static const struct {
 	u32 firmware_revision;
@@ -1555,12 +1566,8 @@ static void sbp2_parse_unit_directory(st
 		case SBP2_FIRMWARE_REVISION_KEY:
 			/* Firmware revision */
 			firmware_revision = kv->value.immediate;
-			if (force_inquiry_hack)
-				SBP2_INFO("sbp2_firmware_revision = %x",
-					  (unsigned int)firmware_revision);
-			else
-				SBP2_DEBUG("sbp2_firmware_revision = %x",
-					   (unsigned int)firmware_revision);
+			SBP2_DEBUG("sbp2_firmware_revision = %x",
+				   (unsigned int)firmware_revision);
 			break;
 
 		default:
@@ -1568,24 +1575,12 @@ static void sbp2_parse_unit_directory(st
 		}
 	}
 
-	/* This is the start of our broken device checking. We try to hack
-	 * around oddities and known defects.  */
 	workarounds = sbp2_default_workarounds;
-
-	/* If the vendor id is 0xa0b8 (Symbios vendor id), then we have a
-	 * bridge with 128KB max transfer size limitation. For sanity, we
-	 * only voice this when the current max_sectors setting
-	 * exceeds the 128k limit. By default, that is not the case.
-	 *
-	 * It would be really nice if we could detect this before the scsi
-	 * host gets initialized. That way we can down-force the
-	 * max_sectors to account for it. That is not currently
-	 * possible.  */
-	/* Check for a blacklisted set of devices that require us to force
-	 * a 36 byte host inquiry. This can be overriden as a module param
-	 * (to force all hosts).  */
-	if (force_inquiry_hack)
+	if (force_inquiry_hack) {
+		SBP2_WARN("force_inquiry_hack is deprecated. "
+			  "Use parameter 'workarounds' instead.");
 		workarounds |= SBP2_WORKAROUND_INQUIRY_36;
+	}
 
 	for (i = 0; i < ARRAY_SIZE(sbp2_workarounds_table); i++) {
 		if (sbp2_workarounds_table[i].firmware_revision !=
@@ -1595,6 +1590,14 @@ static void sbp2_parse_unit_directory(st
 		break;
 	}
 
+	if (workarounds)
+		SBP2_INFO("Workarounds for node " NODE_BUS_FMT ": "
+			  "0x%x (firmware_revision 0x%x)",
+			  NODE_BUS_ARGS(ud->ne->host, ud->ne->nodeid),
+			  workarounds, firmware_revision);
+
+	/* We would need one SCSI host template for each target to adjust
+	 * max_sectors on the fly, therefore warn only. */
 	if (workarounds & SBP2_WORKAROUND_128K_MAX_TRANS &&
 	    (max_sectors * 512) > (128 * 1024))
 		SBP2_WARN("Node " NODE_BUS_FMT ": Bridge only supports 128KB "
@@ -2662,7 +2665,6 @@ static int sbp2_module_init(void)
 		scsi_driver_template.cmd_per_lun = 1;
 	}
 
-	/* Set max sectors (module load option). Default is 255 sectors. */
 	if (sbp2_default_workarounds & SBP2_WORKAROUND_128K_MAX_TRANS &&
 	    (max_sectors * 512) > (128 * 1024))
 		max_sectors = 128 * 1024 / 512;
Index: linux-2.6.16.18/Documentation/feature-removal-schedule.txt
===================================================================
--- linux-2.6.16.18.orig/Documentation/feature-removal-schedule.txt	2006-05-27 13:23:23.000000000 +0200
+++ linux-2.6.16.18/Documentation/feature-removal-schedule.txt	2006-05-27 13:26:50.000000000 +0200
@@ -56,6 +56,15 @@ Who:	Jody McIntyre <scjody@steamballoon.
 
 ---------------------------
 
+What:	sbp2: module parameter "force_inquiry_hack"
+When:	July 2006
+Why:	Superceded by parameter "workarounds". Both parameters are meant to be
+	used ad-hoc and for single devices only, i.e. not in modprobe.conf,
+	therefore the impact of this feature replacement should be low.
+Who:	Stefan Richter <stefanr@s5r6.in-berlin.de>
+
+---------------------------
+
 What:	Video4Linux API 1 ioctls and video_decoder.h from Video devices.
 When:	July 2006
 Why:	V4L1 AP1 was replaced by V4L2 API. during migration from 2.4 to 2.6

[PATCH 2.6.16.18 3/4] sbp2: add read_capacity workaround for iPod

[Stefan Richter]

Apple decided to copy some USB stupidity over to FireWire.
The sector number returned by iPods from read_capacity is one too many.
This may cause I/O errors, especially if the kernel is configured for EFI
partition support. We use the same workaround as usb-storage but have to
check for different model IDs.
http://marc.theaimsgroup.com/?t=114233262300001
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=187409

Acknowledgements: Diagnosis and therapy by Mathieu Chouquet-Stringer
<ml2news@free.fr>, additional data about affected and unaffected Apple
hardware from Vladimir Kotal, Sander De Graaf, Bryan Olmstead, Hugh Dixon.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
rediff for -stable, from commit e9a1c52c7b19d10342226c12f170d7ab644427e2

 drivers/ieee1394/sbp2.c |   49 ++++++++++++++++++++++++++++++++++++++++++++----
 drivers/ieee1394/sbp2.h |    1
 2 files changed, 46 insertions(+), 4 deletions(-)

Index: linux-2.6.16.18/drivers/ieee1394/sbp2.h
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.h	2006-05-27 13:26:01.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.h	2006-05-27 13:27:02.000000000 +0200
@@ -238,6 +238,7 @@ struct sbp2_status_block {
 #define SBP2_WORKAROUND_128K_MAX_TRANS	0x1
 #define SBP2_WORKAROUND_INQUIRY_36	0x2
 #define SBP2_WORKAROUND_MODE_SENSE_8	0x4
+#define SBP2_WORKAROUND_FIX_CAPACITY	0x8
 
 /* This is the two dma types we use for cmd_dma below */
 enum cmd_dma_types {
Index: linux-2.6.16.18/drivers/ieee1394/sbp2.c
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.c	2006-05-27 13:26:01.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.c	2006-05-27 13:27:02.000000000 +0200
@@ -151,6 +151,11 @@ MODULE_PARM_DESC(exclusive_login, "Exclu
  * - skip mode page 8
  *   Suppress sending of mode_sense for mode page 8 if the device pretends to
  *   support the SCSI Primary Block commands instead of Reduced Block Commands.
+ *
+ * - fix capacity
+ *   Tell sd_mod to correct the last sector number reported by read_capacity.
+ *   Avoids access beyond actual disk limits on devices with an off-by-one bug.
+ *   Don't use this with devices which don't have this bug.
  */
 static int sbp2_default_workarounds;
 module_param_named(workarounds, sbp2_default_workarounds, int, 0644);
@@ -158,6 +163,7 @@ MODULE_PARM_DESC(workarounds, "Work arou
 	", 128kB max transfer = " __stringify(SBP2_WORKAROUND_128K_MAX_TRANS)
 	", 36 byte inquiry = "    __stringify(SBP2_WORKAROUND_INQUIRY_36)
 	", skip mode page 8 = "   __stringify(SBP2_WORKAROUND_MODE_SENSE_8)
+	", fix capacity = "       __stringify(SBP2_WORKAROUND_FIX_CAPACITY)
 	", or a combination)");
 
 /* legacy parameter */
@@ -290,6 +296,7 @@ static struct hpsb_protocol_driver sbp2_
  */
 static const struct {
 	u32 firmware_revision;
+	u32 model_id;
 	unsigned workarounds;
 } sbp2_workarounds_table[] = {
 	/* TSB42AA9 */ {
@@ -304,6 +311,31 @@ static const struct {
 	/* Symbios bridge */ {
 		.firmware_revision	= 0xa0b800,
 		.workarounds		= SBP2_WORKAROUND_128K_MAX_TRANS,
+	},
+	/*
+	 * Note about the following Apple iPod blacklist entries:
+	 *
+	 * There are iPods (2nd gen, 3rd gen) with model_id==0.  Since our
+	 * matching logic treats 0 as a wildcard, we cannot match this ID
+	 * without rewriting the matching routine.  Fortunately these iPods
+	 * do not feature the read_capacity bug according to one report.
+	 * Read_capacity behaviour as well as model_id could change due to
+	 * Apple-supplied firmware updates though.
+	 */
+	/* iPod 4th generation */ {
+		.firmware_revision	= 0x0a2700,
+		.model_id		= 0x000021,
+		.workarounds		= SBP2_WORKAROUND_FIX_CAPACITY,
+	},
+	/* iPod mini */ {
+		.firmware_revision	= 0x0a2700,
+		.model_id		= 0x000023,
+		.workarounds		= SBP2_WORKAROUND_FIX_CAPACITY,
+	},
+	/* iPod Photo */ {
+		.firmware_revision	= 0x0a2700,
+		.model_id		= 0x00007e,
+		.workarounds		= SBP2_WORKAROUND_FIX_CAPACITY,
 	}
 };
 
@@ -1583,18 +1615,25 @@ static void sbp2_parse_unit_directory(st
 	}
 
 	for (i = 0; i < ARRAY_SIZE(sbp2_workarounds_table); i++) {
-		if (sbp2_workarounds_table[i].firmware_revision !=
+		if (sbp2_workarounds_table[i].firmware_revision &&
+		    sbp2_workarounds_table[i].firmware_revision !=
 		    (firmware_revision & 0xffff00))
 			continue;
+		if (sbp2_workarounds_table[i].model_id &&
+		    sbp2_workarounds_table[i].model_id != ud->model_id)
+			continue;
 		workarounds |= sbp2_workarounds_table[i].workarounds;
 		break;
 	}
 
 	if (workarounds)
-		SBP2_INFO("Workarounds for node " NODE_BUS_FMT ": "
-			  "0x%x (firmware_revision 0x%x)",
+		SBP2_INFO("Workarounds for node " NODE_BUS_FMT ": 0x%x "
+			  "(firmware_revision 0x%06x, vendor_id 0x%06x,"
+			  " model_id 0x%06x)",
 			  NODE_BUS_ARGS(ud->ne->host, ud->ne->nodeid),
-			  workarounds, firmware_revision);
+			  workarounds, firmware_revision,
+			  ud->vendor_id ? ud->vendor_id : ud->ne->vendor_id,
+			  ud->model_id);
 
 	/* We would need one SCSI host template for each target to adjust
 	 * max_sectors on the fly, therefore warn only. */
@@ -2522,6 +2561,8 @@ static int sbp2scsi_slave_configure(stru
 	if (sdev->type == TYPE_DISK &&
 	    scsi_id->workarounds & SBP2_WORKAROUND_MODE_SENSE_8)
 		sdev->skip_ms_page_8 = 1;
+	if (scsi_id->workarounds & SBP2_WORKAROUND_FIX_CAPACITY)
+		sdev->fix_capacity = 1;
 	return 0;
 }
 

[PATCH 2.6.16.18 4/4] sbp2: add ability to override hardwired blacklist

[Stefan Richter]

In case the blacklist with workarounds for device bugs yields a false
positive, the module load parameter can now also be used as an override
instead of an addition to the blacklist.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
rediff for -stable, from commit 679c0cd2dd61c825ab910fdbf347a8b7d1dddec4

 drivers/ieee1394/sbp2.c |   29 ++++++++++++++++++-----------
 drivers/ieee1394/sbp2.h |    1 +
 2 files changed, 19 insertions(+), 11 deletions(-)

Index: linux-2.6.16.18/drivers/ieee1394/sbp2.h
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.h	2006-05-27 13:27:02.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.h	2006-05-27 13:27:07.000000000 +0200
@@ -239,6 +239,7 @@ struct sbp2_status_block {
 #define SBP2_WORKAROUND_INQUIRY_36	0x2
 #define SBP2_WORKAROUND_MODE_SENSE_8	0x4
 #define SBP2_WORKAROUND_FIX_CAPACITY	0x8
+#define SBP2_WORKAROUND_OVERRIDE	0x100
 
 /* This is the two dma types we use for cmd_dma below */
 enum cmd_dma_types {
Index: linux-2.6.16.18/drivers/ieee1394/sbp2.c
===================================================================
--- linux-2.6.16.18.orig/drivers/ieee1394/sbp2.c	2006-05-27 13:27:02.000000000 +0200
+++ linux-2.6.16.18/drivers/ieee1394/sbp2.c	2006-05-27 13:27:07.000000000 +0200
@@ -156,6 +156,11 @@ MODULE_PARM_DESC(exclusive_login, "Exclu
  *   Tell sd_mod to correct the last sector number reported by read_capacity.
  *   Avoids access beyond actual disk limits on devices with an off-by-one bug.
  *   Don't use this with devices which don't have this bug.
+ *
+ * - override internal blacklist
+ *   Instead of adding to the built-in blacklist, use only the workarounds
+ *   specified in the module load parameter.
+ *   Useful if a blacklist entry interfered with a non-broken device.
  */
 static int sbp2_default_workarounds;
 module_param_named(workarounds, sbp2_default_workarounds, int, 0644);
@@ -164,6 +169,7 @@ MODULE_PARM_DESC(workarounds, "Work arou
 	", 36 byte inquiry = "    __stringify(SBP2_WORKAROUND_INQUIRY_36)
 	", skip mode page 8 = "   __stringify(SBP2_WORKAROUND_MODE_SENSE_8)
 	", fix capacity = "       __stringify(SBP2_WORKAROUND_FIX_CAPACITY)
+	", override internal blacklist = " __stringify(SBP2_WORKAROUND_OVERRIDE)
 	", or a combination)");
 
 /* legacy parameter */
@@ -1614,17 +1620,18 @@ static void sbp2_parse_unit_directory(st
 		workarounds |= SBP2_WORKAROUND_INQUIRY_36;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(sbp2_workarounds_table); i++) {
-		if (sbp2_workarounds_table[i].firmware_revision &&
-		    sbp2_workarounds_table[i].firmware_revision !=
-		    (firmware_revision & 0xffff00))
-			continue;
-		if (sbp2_workarounds_table[i].model_id &&
-		    sbp2_workarounds_table[i].model_id != ud->model_id)
-			continue;
-		workarounds |= sbp2_workarounds_table[i].workarounds;
-		break;
-	}
+	if (!(workarounds & SBP2_WORKAROUND_OVERRIDE))
+		for (i = 0; i < ARRAY_SIZE(sbp2_workarounds_table); i++) {
+			if (sbp2_workarounds_table[i].firmware_revision &&
+			    sbp2_workarounds_table[i].firmware_revision !=
+			    (firmware_revision & 0xffff00))
+				continue;
+			if (sbp2_workarounds_table[i].model_id &&
+			    sbp2_workarounds_table[i].model_id != ud->model_id)
+				continue;
+			workarounds |= sbp2_workarounds_table[i].workarounds;
+			break;
+		}
 
 	if (workarounds)
 		SBP2_INFO("Workarounds for node " NODE_BUS_FMT ": 0x%x "

Re: [stable] [PATCH 2.6.16.18 0/4] sbp2: workaround for buggy iPods

[Chris Wright]

* Stefan Richter (stefanr@s5r6.in-berlin.de) wrote:
> There is a firmware bug in several Apple iPods which prevents access to
> these iPods under certain conditions. The disk size reported by the iPod
> is one sector too big. Once access to the end of the disk is attempted,
> the iPod becomes inaccessible. This problem has been known for USB iPods
> for some time and has recently been discovered to exist with
> FireWire/USB combo iPods too.
> 
> The following patchset is the fix as it exists in Linux 2.6.17-rc. Alas
> it is rather large, therefore it may be unfit for -stable as it is. If
> there are objections, I would appreciate suggestions how to better adapt
> this fix for -stable.

Just to be clear, we're talking about this fix:

+	if (scsi_id->workarounds & SBP2_WORKAROUND_FIX_CAPACITY)
+		sdev->fix_capacity = 1;

with patches 1 and 2 consolidating workaround logic, and 4 adding the
command line override.

I'd feel better to let this bit diverge in -stable to have the two-line
patch rather than all 4 patches.  Could you do detection sbp2_probe(),
set a flag in scsi_id_instance_data, and use that in slave_configure?

thanks,
-chris

Re: [stable] [PATCH 2.6.16.18 0/4] sbp2: workaround for buggy iPods

[Stefan Richter]

Chris Wright wrote:
> I'd feel better to let this bit diverge in -stable to have the two-line
> patch rather than all 4 patches.

I will see how far I can downsize it. (Will of course take a little more 
than 2 lines.)
-- 
Stefan Richter
-=====-=-==- -=-= =====
http://arcgraph.de/sr/

[PATCH 2.6.16.19] sbp2: backport read_capacity workaround for iPod

[Stefan Richter]

sbp2: backport read_capacity workaround for iPod

There is a firmware bug in several Apple iPods which prevents access to
these iPods under certain conditions. The disk size reported by the iPod
is one sector too big. Once access to the end of the disk is attempted,
the iPod becomes inaccessible. This problem has been known for USB iPods
for some time and has recently been discovered to exist with
FireWire/USB combo iPods too.

This patch is derived from the fix in Linux 2.6.17, commit
e9a1c52c7b19d10342226c12f170d7ab644427e2, to be applicable to 2.6.16.x
without prerequisite patches. It hard-wires a workaround for three known
affected model numbers (those of 4th generation iPod, iPod Photo, iPod
mini).

Note: This patch lacks Linux 2.6.17's ability to enable and disable the
workaround via a module parameter.

Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
Index: linux/drivers/ieee1394/sbp2.c
===================================================================
--- linux.orig/drivers/ieee1394/sbp2.c	2006-06-02 18:37:13.000000000 +0200
+++ linux/drivers/ieee1394/sbp2.c	2006-06-02 19:10:05.000000000 +0200
@@ -2491,9 +2491,20 @@ static int sbp2scsi_slave_alloc(struct s
 
 static int sbp2scsi_slave_configure(struct scsi_device *sdev)
 {
+	struct scsi_id_instance_data *scsi_id =
+		(struct scsi_id_instance_data *)sdev->host->hostdata[0];
+
 	blk_queue_dma_alignment(sdev->request_queue, (512 - 1));
 	sdev->use_10_for_rw = 1;
 	sdev->use_10_for_ms = 1;
+
+	if ((scsi_id->sbp2_firmware_revision & 0xffff00) == 0x0a2700 &&
+	    (scsi_id->ud->model_id == 0x000021 /* gen.4 iPod */ ||
+	     scsi_id->ud->model_id == 0x000023 /* iPod mini  */ ||
+	     scsi_id->ud->model_id == 0x00007e /* iPod Photo */ )) {
+		SBP2_INFO("enabling iPod workaround: decrement disk capacity");
+		sdev->fix_capacity = 1;
+	}
 	return 0;
 }
 

Re: [stable] [PATCH 2.6.16.19] sbp2: backport read_capacity workaround for iPod

[Chris Wright]

* Stefan Richter (stefanr@s5r6.in-berlin.de) wrote:
> sbp2: backport read_capacity workaround for iPod

Thank you for the smaller backport Stefan.  Patch queued.
-chris