diff mbox series

[4/4] usb: storage: Implement 64-bit LBA support

Message ID 20231029-usb-fixes-3-v1-4-0153210a7f55@marcan.st
State New
Delegated to: Marek Vasut
Headers show
Series USB fixes: Mass Storage bugs & 64bit support | expand

Commit Message

Hector Martin Oct. 29, 2023, 7:23 a.m. UTC 
This makes things work properly on devices with >= 2 TiB
capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA,
the capacity will be clamped at 2^32 - 1 sectors.

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 114 insertions(+), 18 deletions(-)

Comments

Marek Vasut Oct. 29, 2023, 12:11 p.m. UTC | #1 
On 10/29/23 08:23, Hector Martin wrote:
> This makes things work properly on devices with >= 2 TiB
> capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA,
> the capacity will be clamped at 2^32 - 1 sectors.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++-------
>   1 file changed, 114 insertions(+), 18 deletions(-)
> 
> diff --git a/common/usb_storage.c b/common/usb_storage.c
> index 95507ffbce48..3035f2ee9868 100644
> --- a/common/usb_storage.c
> +++ b/common/usb_storage.c
> @@ -66,7 +66,7 @@
>   static const unsigned char us_direction[256/8] = {
>   	0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77,
>   	0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
> -	0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01,
> +	0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01,

What changed here ?

>   	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>   };
>   #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1)
> @@ -1073,6 +1073,27 @@ static int usb_read_capacity(struct scsi_cmd *srb, struct us_data *ss)
>   	return -1;
>   }

[...]

> +#ifdef CONFIG_SYS_64BIT_LBA

Could you try and use CONFIG_IS_ENABLED() ?

> +	if (capacity == 0x100000000) {
> +		if (usb_read_capacity64(pccb, ss) != 0) {
> +			puts("READ_CAP64 ERROR\n");
> +		} else {
> +			debug("Read Capacity 64 returns: 0x%08x, 0x%08x, 0x%08x\n",
> +			      cap[0], cap[1], cap[2]);
> +			capacity = be64_to_cpu(*(uint64_t *)cap) + 1;
> +			blksz = be32_to_cpu(cap[2]);
> +		}
> +	}
> +#else
> +	/*
> +	 * READ CAPACITY will return 0xffffffff when limited,
> +	 * which wraps to 0 with the +1 above
> +	 */
> +	if (!capacity) {
> +		puts("LBA exceeds 32 bits but 64-bit LBA is disabled.\n");
> +		capacity = ~0;
> +	}
>   #endif
Hector Martin Oct. 29, 2023, 3:54 p.m. UTC | #2 
On 29/10/2023 21.11, Marek Vasut wrote:
> On 10/29/23 08:23, Hector Martin wrote:
>> This makes things work properly on devices with >= 2 TiB
>> capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA,
>> the capacity will be clamped at 2^32 - 1 sectors.
>>
>> Signed-off-by: Hector Martin <marcan@marcan.st>
>> ---
>>   common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++-------
>>   1 file changed, 114 insertions(+), 18 deletions(-)
>>
>> diff --git a/common/usb_storage.c b/common/usb_storage.c
>> index 95507ffbce48..3035f2ee9868 100644
>> --- a/common/usb_storage.c
>> +++ b/common/usb_storage.c
>> @@ -66,7 +66,7 @@
>>   static const unsigned char us_direction[256/8] = {
>>   	0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77,
>>   	0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
>> -	0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01,
>> +	0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01,
> 
> What changed here ?

This is an incomplete bitmap specifying the data transfer direction for
every possible SCSI command ID. I'm adding the new commands I'm now using.

> 
>>   	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
>>   };
>>   #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1)
>> @@ -1073,6 +1073,27 @@ static int usb_read_capacity(struct scsi_cmd *srb, struct us_data *ss)
>>   	return -1;
>>   }
> 
> [...]
> 
>> +#ifdef CONFIG_SYS_64BIT_LBA
> 
> Could you try and use CONFIG_IS_ENABLED() ?

Sure.

> 
>> +	if (capacity == 0x100000000) {
>> +		if (usb_read_capacity64(pccb, ss) != 0) {
>> +			puts("READ_CAP64 ERROR\n");
>> +		} else {
>> +			debug("Read Capacity 64 returns: 0x%08x, 0x%08x, 0x%08x\n",
>> +			      cap[0], cap[1], cap[2]);
>> +			capacity = be64_to_cpu(*(uint64_t *)cap) + 1;
>> +			blksz = be32_to_cpu(cap[2]);
>> +		}
>> +	}
>> +#else
>> +	/*
>> +	 * READ CAPACITY will return 0xffffffff when limited,
>> +	 * which wraps to 0 with the +1 above
>> +	 */
>> +	if (!capacity) {
>> +		puts("LBA exceeds 32 bits but 64-bit LBA is disabled.\n");
>> +		capacity = ~0;
>> +	}
>>   #endif
> 
> 

- Hector
Marek Vasut Oct. 29, 2023, 5:33 p.m. UTC | #3 
On 10/29/23 16:54, Hector Martin wrote:
> On 29/10/2023 21.11, Marek Vasut wrote:
>> On 10/29/23 08:23, Hector Martin wrote:
>>> This makes things work properly on devices with >= 2 TiB
>>> capacity. If u-boot is built without CONFIG_SYS_64BIT_LBA,
>>> the capacity will be clamped at 2^32 - 1 sectors.
>>>
>>> Signed-off-by: Hector Martin <marcan@marcan.st>
>>> ---
>>>    common/usb_storage.c | 132 ++++++++++++++++++++++++++++++++++++++++++++-------
>>>    1 file changed, 114 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/common/usb_storage.c b/common/usb_storage.c
>>> index 95507ffbce48..3035f2ee9868 100644
>>> --- a/common/usb_storage.c
>>> +++ b/common/usb_storage.c
>>> @@ -66,7 +66,7 @@
>>>    static const unsigned char us_direction[256/8] = {
>>>    	0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77,
>>>    	0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
>>> -	0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01,
>>> +	0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01,
>>
>> What changed here ?
> 
> This is an incomplete bitmap specifying the data transfer direction for
> every possible SCSI command ID. I'm adding the new commands I'm now using.

Can you please add a code comment here, so others wouldn't ponder about 
this too ?
diff mbox series

Patch

diff --git a/common/usb_storage.c b/common/usb_storage.c
index 95507ffbce48..3035f2ee9868 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -66,7 +66,7 @@ 
 static const unsigned char us_direction[256/8] = {
 	0x28, 0x81, 0x14, 0x14, 0x20, 0x01, 0x90, 0x77,
 	0x0C, 0x20, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01,
+	0x00, 0x01, 0x00, 0x40, 0x00, 0x01, 0x00, 0x01,
 	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
 };
 #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1)
@@ -1073,6 +1073,27 @@  static int usb_read_capacity(struct scsi_cmd *srb, struct us_data *ss)
 	return -1;
 }
 
+#ifdef CONFIG_SYS_64BIT_LBA
+static int usb_read_capacity64(struct scsi_cmd *srb, struct us_data *ss)
+{
+	int retry;
+	/* XXX retries */
+	retry = 3;
+	do {
+		memset(&srb->cmd[0], 0, 16);
+		srb->cmd[0] = SCSI_SRV_ACTION_IN;
+		srb->cmd[1] = (srb->lun << 5) | SCSI_SAI_RD_CAPAC16;
+		srb->cmd[13] = 32; /* Allocation length */
+		srb->datalen = 32;
+		srb->cmdlen = 16;
+		if (ss->transport(srb, ss) == USB_STOR_TRANSPORT_GOOD)
+			return 0;
+	} while (retry--);
+
+	return -1;
+}
+#endif
+
 static int usb_read_10(struct scsi_cmd *srb, struct us_data *ss,
 		       unsigned long start, unsigned short blocks)
 {
@@ -1107,6 +1128,49 @@  static int usb_write_10(struct scsi_cmd *srb, struct us_data *ss,
 	return ss->transport(srb, ss);
 }
 
+#ifdef CONFIG_SYS_64BIT_LBA
+static int usb_read_16(struct scsi_cmd *srb, struct us_data *ss,
+		       uint64_t start, unsigned short blocks)
+{
+	memset(&srb->cmd[0], 0, 16);
+	srb->cmd[0] = SCSI_READ16;
+	srb->cmd[1] = srb->lun << 5;
+	srb->cmd[2] = ((unsigned char) (start >> 56)) & 0xff;
+	srb->cmd[3] = ((unsigned char) (start >> 48)) & 0xff;
+	srb->cmd[4] = ((unsigned char) (start >> 40)) & 0xff;
+	srb->cmd[5] = ((unsigned char) (start >> 32)) & 0xff;
+	srb->cmd[6] = ((unsigned char) (start >> 24)) & 0xff;
+	srb->cmd[7] = ((unsigned char) (start >> 16)) & 0xff;
+	srb->cmd[8] = ((unsigned char) (start >> 8)) & 0xff;
+	srb->cmd[9] = ((unsigned char) (start)) & 0xff;
+	srb->cmd[12] = ((unsigned char) (blocks >> 8)) & 0xff;
+	srb->cmd[13] = (unsigned char) blocks & 0xff;
+	srb->cmdlen = 16;
+	debug("read16: start %llx blocks %x\n", (long long)start, blocks);
+	return ss->transport(srb, ss);
+}
+
+static int usb_write_16(struct scsi_cmd *srb, struct us_data *ss,
+			uint64_t start, unsigned short blocks)
+{
+	memset(&srb->cmd[0], 0, 16);
+	srb->cmd[0] = SCSI_WRITE16;
+	srb->cmd[1] = srb->lun << 5;
+	srb->cmd[2] = ((unsigned char) (start >> 56)) & 0xff;
+	srb->cmd[3] = ((unsigned char) (start >> 48)) & 0xff;
+	srb->cmd[4] = ((unsigned char) (start >> 40)) & 0xff;
+	srb->cmd[5] = ((unsigned char) (start >> 32)) & 0xff;
+	srb->cmd[6] = ((unsigned char) (start >> 24)) & 0xff;
+	srb->cmd[7] = ((unsigned char) (start >> 16)) & 0xff;
+	srb->cmd[8] = ((unsigned char) (start >> 8)) & 0xff;
+	srb->cmd[9] = ((unsigned char) (start)) & 0xff;
+	srb->cmd[12] = ((unsigned char) (blocks >> 8)) & 0xff;
+	srb->cmd[13] = (unsigned char) blocks & 0xff;
+	srb->cmdlen = 16;
+	debug("write16: start %llx blocks %x\n", (long long)start, blocks);
+	return ss->transport(srb, ss);
+}
+#endif
 
 #ifdef CONFIG_USB_BIN_FIXUP
 /*
@@ -1145,6 +1209,7 @@  static unsigned long usb_stor_read(struct blk_desc *block_dev, lbaint_t blknr,
 	struct usb_device *udev;
 	struct us_data *ss;
 	int retry;
+	int ret;
 	struct scsi_cmd *srb = &usb_ccb;
 #if CONFIG_IS_ENABLED(BLK)
 	struct blk_desc *block_dev;
@@ -1190,7 +1255,13 @@  retry_it:
 			usb_show_progress();
 		srb->datalen = block_dev->blksz * smallblks;
 		srb->pdata = (unsigned char *)buf_addr;
-		if (usb_read_10(srb, ss, start, smallblks)) {
+#ifdef CONFIG_SYS_64BIT_LBA
+		if (block_dev->lba > ((lbaint_t)0x100000000))
+			ret = usb_read_16(srb, ss, start, smallblks);
+		else
+#endif
+		ret = usb_read_10(srb, ss, start, smallblks);
+		if (ret) {
 			debug("Read ERROR\n");
 			ss->flags &= ~USB_READY;
 			usb_request_sense(srb, ss);
@@ -1228,6 +1299,7 @@  static unsigned long usb_stor_write(struct blk_desc *block_dev, lbaint_t blknr,
 	struct usb_device *udev;
 	struct us_data *ss;
 	int retry;
+	int ret;
 	struct scsi_cmd *srb = &usb_ccb;
 #if CONFIG_IS_ENABLED(BLK)
 	struct blk_desc *block_dev;
@@ -1277,7 +1349,13 @@  retry_it:
 			usb_show_progress();
 		srb->datalen = block_dev->blksz * smallblks;
 		srb->pdata = (unsigned char *)buf_addr;
-		if (usb_write_10(srb, ss, start, smallblks)) {
+#ifdef CONFIG_SYS_64BIT_LBA
+		if (block_dev->lba > ((lbaint_t)0x100000000))
+			ret = usb_write_16(srb, ss, start, smallblks);
+		else
+#endif
+		ret = usb_write_10(srb, ss, start, smallblks);
+		if (ret) {
 			debug("Write ERROR\n");
 			ss->flags &= ~USB_READY;
 			usb_request_sense(srb, ss);
@@ -1434,9 +1512,10 @@  int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 		      struct blk_desc *dev_desc)
 {
 	unsigned char perq, modi;
-	ALLOC_CACHE_ALIGN_BUFFER(u32, cap, 2);
+	ALLOC_CACHE_ALIGN_BUFFER(u32, cap, 8);
 	ALLOC_CACHE_ALIGN_BUFFER(u8, usb_stor_buf, 36);
-	u32 capacity, blksz;
+	lbaint_t capacity;
+	u32 blksz;
 	struct scsi_cmd *pccb = &usb_ccb;
 
 	pccb->pdata = usb_stor_buf;
@@ -1487,26 +1566,43 @@  int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 		return 0;
 	}
 	pccb->pdata = (unsigned char *)cap;
-	memset(pccb->pdata, 0, 8);
+	memset(pccb->pdata, 0, 32);
 	if (usb_read_capacity(pccb, ss) != 0) {
 		printf("READ_CAP ERROR\n");
 		ss->flags &= ~USB_READY;
-		cap[0] = 2880;
-		cap[1] = 0x200;
+		capacity = 2880;
+		blksz = 512;
+	} else {
+		debug("Read Capacity returns: 0x%08x, 0x%08x\n",
+		      cap[0], cap[1]);
+		capacity = ((lbaint_t)be32_to_cpu(cap[0])) + 1;
+		blksz = be32_to_cpu(cap[1]);
 	}
-	debug("Read Capacity returns: 0x%08x, 0x%08x\n", cap[0], cap[1]);
-#if 0
-	if (cap[0] > (0x200000 * 10)) /* greater than 10 GByte */
-		cap[0] >>= 16;
 
-	cap[0] = cpu_to_be32(cap[0]);
-	cap[1] = cpu_to_be32(cap[1]);
+#ifdef CONFIG_SYS_64BIT_LBA
+	if (capacity == 0x100000000) {
+		if (usb_read_capacity64(pccb, ss) != 0) {
+			puts("READ_CAP64 ERROR\n");
+		} else {
+			debug("Read Capacity 64 returns: 0x%08x, 0x%08x, 0x%08x\n",
+			      cap[0], cap[1], cap[2]);
+			capacity = be64_to_cpu(*(uint64_t *)cap) + 1;
+			blksz = be32_to_cpu(cap[2]);
+		}
+	}
+#else
+	/*
+	 * READ CAPACITY will return 0xffffffff when limited,
+	 * which wraps to 0 with the +1 above
+	 */
+	if (!capacity) {
+		puts("LBA exceeds 32 bits but 64-bit LBA is disabled.\n");
+		capacity = ~0;
+	}
 #endif
 
-	capacity = be32_to_cpu(cap[0]) + 1;
-	blksz = be32_to_cpu(cap[1]);
-
-	debug("Capacity = 0x%08x, blocksz = 0x%08x\n", capacity, blksz);
+	debug("Capacity = 0x%llx, blocksz = 0x%08x\n",
+	      (long long)capacity, blksz);
 	dev_desc->lba = capacity;
 	dev_desc->blksz = blksz;
 	dev_desc->log2blksz = LOG2(dev_desc->blksz);