Re: Handling erroneous READ CAPACITY response in sd.c

[Luben Tuikov <luben_tuikov@adaptec.com>]

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

Alan Stern wrote:
> A number of USB mass storage devices incorrectly return the total number
> of blocks rather than the largest block number in response to READ
> CAPACITY.  They then go on to compound the problem by not returning
> "Logical block address out of range" sense when asked to read the "last"
> block; instead they mess up the protocol and don't send any status phase
> information.
> 
> The usb-storage driver has been dealing with this by adding blacklist
> entries for these devices, but this isn't a very good solution.  After
> all, it's not a transport problem -- and it's not a good idea for a
> low-level driver to change the data being sent by a device.
> 
> Instead the adjustment should be made in sd.c, in the sd_read_capacity()
> routine.  One possibility is to have a SCSI blacklist flag for the bad
> devices.  A better choice might be to correct the mistake at runtime. 
> Using the heuristic that the total number of blocks is almost always even
> (as far as I know it is _always_ even for USB disk-like devices), we could
> try to read the last block whenever READ CAPACITY reports an odd number of
> blocks.  If the read fails then we would know to decrement the number.
> 
> Does either of these sound like a good idea?  And if the second choice
> sounds better, is there anyone who could help me to write such a patch?

Alan, do you have such a USB device handy?

A third possibility is to use the PMI bit to get the proper value
of the LBA of the last logical block.

If you want to move this into sd, then do you know if those
devices support the use of PMI bit in READ CAPACITY CDB?
As block devices they should.

See the appended/attached patch.  Can you test it against such a USB
device?

Signed-off-by: Luben Tuikov <luben_tuikov@adaptec.com>

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-19 16:51:46 -04:00
@@ -987,6 +987,112 @@
  	}
  }

+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+#define GET_RLBA_READ_CAP16(_buffer) (((u64)(_buffer)[0] << 56) | \
+				     ((u64)(_buffer)[1] << 48) |  \
+				     ((u64)(_buffer)[2] << 40) |  \
+				     ((u64)(_buffer)[3] << 32) |  \
+				     ((sector_t)(_buffer)[4] << 24) | \
+				     ((sector_t)(_buffer)[5] << 16) | \
+				     ((sector_t)(_buffer)[6] << 8)  | \
+				     (sector_t)(_buffer)[7])
+#define GET_RLBA_READ_CAP10(_buffer) (((sector_t)(_buffer)[0] << 24) | \
+				      ((_buffer)[1] << 16) |           \
+				      ((_buffer)[2] << 8) |            \
+				      (_buffer)[3])
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = GET_RLBA_READ_CAP16(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = GET_RLBA_READ_CAP10(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = GET_RLBA_READ_CAP16(buffer);
+		u64 x    = GET_RLBA_READ_CAP16(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = GET_RLBA_READ_CAP10(buffer);
+		u32 x    = GET_RLBA_READ_CAP10(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+	
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
  /*
   * read disk capacity
   */
@@ -1070,7 +1176,12 @@
  		
  		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
  		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
  	
  	if (!longrc) {
  		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1189,14 @@
  		if (buffer[0] == 0xff && buffer[1] == 0xff &&
  		    buffer[2] == 0xff && buffer[3] == 0xff) {
  			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
  				longrc = 1;
  				goto repeat;
  			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
  			}
  		}
  		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1215,7 @@
  			
  		sector_size = (buffer[8] << 24) |
  			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}

  got_data:
  	if (sector_size == 0) {

[-- Attachment #2: sd_read_cap.patch --]
[-- Type: application/octet-stream, Size: 4565 bytes --]

===== drivers/scsi/sd.c 1.161 vs edited =====
--- 1.161/drivers/scsi/sd.c	2004-10-15 10:46:07 -04:00
+++ edited/drivers/scsi/sd.c	2004-10-19 16:51:46 -04:00
@@ -987,6 +987,112 @@
 	}
 }
 
+/* sd_read_true_cap: Some device servers incorrectly return the
+ * capacity as opposed to the LBA of the last logical block of the
+ * block device.
+ *
+ * We try to fix this as follows: Let x = Returned LBA from the last
+ * READ CAPACITY command issued (result in "buffer").  Reissue the
+ * READ CAPACITY command as follows: set the partial medium indicator
+ * (PMI) bit to one; set the LBA to x - 1. Fire off that READ CAPACITY
+ * command.
+ *
+ * If we get success,
+ *       If Returned LBA > x - 1, then capacity is x+1, spec behavior.
+ *       Else Returned LBA <= x - 1, then capacity is x, broken device server.
+ * Else error, nothing can be assumed, capacity is x+1.
+ */
+#define GET_RLBA_READ_CAP16(_buffer) (((u64)(_buffer)[0] << 56) | \
+				     ((u64)(_buffer)[1] << 48) |  \
+				     ((u64)(_buffer)[2] << 40) |  \
+				     ((u64)(_buffer)[3] << 32) |  \
+				     ((sector_t)(_buffer)[4] << 24) | \
+				     ((sector_t)(_buffer)[5] << 16) | \
+				     ((sector_t)(_buffer)[6] << 8)  | \
+				     (sector_t)(_buffer)[7])
+#define GET_RLBA_READ_CAP10(_buffer) (((sector_t)(_buffer)[0] << 24) | \
+				      ((_buffer)[1] << 16) |           \
+				      ((_buffer)[2] << 8) |            \
+				      (_buffer)[3])
+static void sd_read_true_cap(struct scsi_disk *sd, char *diskname,
+			     struct scsi_request *SRpnt, unsigned char *buffer,
+			     int longrc)
+{
+	unsigned char cmd[16];
+	unsigned char buf[12];
+
+	/* save the old buffer contents here */
+	memcpy(buf, buffer, 12);
+
+	if (longrc) {
+		u64 *lba = (u64 *) (cmd+2);
+		u64 rlba;
+
+		memset((void *) cmd, 0, 16);
+		cmd[0] = SERVICE_ACTION_IN;
+		cmd[1] = SAI_READ_CAPACITY_16;
+		cmd[13] = 12;
+
+		rlba = GET_RLBA_READ_CAP16(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be64(rlba);
+		/* turn on the PMI bit */
+		cmd[14] |= 1;
+		memset((void *) buffer, 0, 12);
+	} else {
+		u32 *lba = (u32 *) (cmd+2);
+		u32 rlba;
+
+		cmd[0] = READ_CAPACITY;
+		memset((void *) &cmd[1], 0, 9);
+		
+		rlba = GET_RLBA_READ_CAP10(buffer);
+		rlba -= 1;
+		*lba = cpu_to_be32(rlba);
+		/* turn on the PMI bit */
+		cmd[8] |= 1;
+		memset((void *) buffer, 0, 8);
+	}
+
+	SRpnt->sr_cmd_len = 0;
+	SRpnt->sr_sense_buffer[0] = 0;
+	SRpnt->sr_sense_buffer[2] = 0;
+	SRpnt->sr_data_direction = DMA_FROM_DEVICE;
+	
+	scsi_wait_req(SRpnt, (void *) cmd, (void *) buffer,
+		      longrc ? 12 : 8, SD_TIMEOUT, SD_MAX_RETRIES);
+
+	if (SRpnt->sr_result) {
+		/* Nothing can be assumed. */
+		printk(KERN_NOTICE "%s: %s: PMI not supported\n",
+		       __FUNCTION__, diskname);
+		memcpy(buffer, buf, 12);
+		return;
+	}
+
+	if (longrc) {
+		u64 rlba = GET_RLBA_READ_CAP16(buffer);
+		u64 x    = GET_RLBA_READ_CAP16(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	} else {
+		u32 rlba = GET_RLBA_READ_CAP10(buffer);
+		u32 x    = GET_RLBA_READ_CAP10(buf);
+		if (rlba > x - 1) {
+			goto out_spec;
+		}
+	}
+	printk(KERN_NOTICE "%s: %s: broken device server\n", __FUNCTION__,
+	       diskname);
+	return;
+	
+ out_spec:
+	/* Capacity is x+1, spec behavior. */
+	printk(KERN_NOTICE "%s: %s: spec behavior\n", __FUNCTION__, diskname);
+	memcpy(buffer, buf, 12);
+} /* end sd_read_true_cap() */
+
 /*
  * read disk capacity
  */
@@ -1070,7 +1176,12 @@
 		
 		sdkp->capacity = 1 + (sector_t) 0xffffffff;		
 		goto got_data;
-	}	
+	}
+
+	/* Check if the device reported CAPACITY as opposed to
+	 * the maxumum LBA (as per the SBC spec).
+	 */
+	sd_read_true_cap(sdkp, diskname, SRpnt, buffer, longrc);
 	
 	if (!longrc) {
 		sector_size = (buffer[4] << 24) |
@@ -1078,12 +1189,14 @@
 		if (buffer[0] == 0xff && buffer[1] == 0xff &&
 		    buffer[2] == 0xff && buffer[3] == 0xff) {
 			if(sizeof(sdkp->capacity) > 4) {
-				printk(KERN_NOTICE "%s : very big device. try to use"
-				       " READ CAPACITY(16).\n", diskname);
+				printk(KERN_NOTICE "%s : very big device. "
+				       "try to use READ CAPACITY(16).\n",
+				       diskname);
 				longrc = 1;
 				goto repeat;
 			} else {
-				printk(KERN_ERR "%s: too big for kernel.  Assuming maximum 2Tb\n", diskname);
+				printk(KERN_ERR "%s: too big for kernel. "
+				       "Assuming maximum 2Tb\n", diskname);
 			}
 		}
 		sdkp->capacity = 1 + (((sector_t)buffer[0] << 24) |
@@ -1102,7 +1215,7 @@
 			
 		sector_size = (buffer[8] << 24) |
 			(buffer[9] << 16) | (buffer[10] << 8) | buffer[11];
-	}	
+	}
 
 got_data:
 	if (sector_size == 0) {