On 2/29/24 14:16, Damien Le Moal wrote:
On 2024/02/29 14:07, Bart Van Assche wrote:
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -734,6 +734,8 @@ static int sd_zbc_check_capacity(struct scsi_disk
*sdkp, unsigned char *buf,
(unsigned long long)sdkp->capacity,
(unsigned long long)max_lba + 1);
sdkp->capacity = max_lba + 1;
+ if (sdkp->capacity > 0xffffffff)
+ sdkp->device->use_16_for_rw = 1;
ZBC makes 16B RW mandatory, regardless of the device capacity. So I am not very
keen on playing games like this and think that it is safer to always use RW-16,
regardless of the device capacity. I prefer your first patch, with the added
"use_10_for_rw = 0;" added.
Hi Damien,
I think that we need to combine the two patches otherwise we risk
breaking support for zoned devices that are neither ATA devices nor
UFS devices. How about the patch below?
Thanks,
Bart.
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0a0f483124c3..6d55a01f0263 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -986,8 +986,15 @@ static void ata_gen_ata_sense(struct ata_queued_cmd
*qc)
void ata_scsi_sdev_config(struct scsi_device *sdev)
{
- sdev->use_10_for_rw = 1;
- sdev->use_10_for_ms = 1;
+ if (sdev->type == TYPE_ZBC) {
+ /* READ16/WRITE16/SYNC16 is mandatory for ZBC devices */
+ sdev->use_16_for_rw = 1;
+ sdev->use_10_for_rw = 0;
+ sdev->use_16_for_sync = 1;
+ } else {
+ sdev->use_10_for_rw = 1;
+ sdev->use_10_for_ms = 1;
+ }
sdev->no_write_same = 1;
/* Schedule policy is determined by ->qc_defer() callback and
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 997de4daa8c4..8d3020486095 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -2798,6 +2798,13 @@ sd_read_capacity(struct scsi_disk *sdkp, unsigned
char *buffer)
sdkp->physical_block_size);
sdkp->device->sector_size = sector_size;
+ /*
+ * For zoned block devices, if RC BASIS = 1 in the READ CAPACITY
+ * response, sdkp->capacity does not represent the device capacity but
+ * the highest LBA of the conventional zones at the start of the LBA
+ * space. sd_zbc_check_capacity() sets sdkp->capacity correctly for
+ * zoned devices.
+ */
if (sdkp->capacity > 0xffffffff)
sdp->use_16_for_rw = 1;
diff --git a/drivers/scsi/sd_zbc.c b/drivers/scsi/sd_zbc.c
index 26af5ab7d7c1..d4d6b3056410 100644
--- a/drivers/scsi/sd_zbc.c
+++ b/drivers/scsi/sd_zbc.c
@@ -734,6 +734,8 @@ static int sd_zbc_check_capacity(struct scsi_disk
*sdkp, unsigned char *buf,
(unsigned long long)sdkp->capacity,
(unsigned long long)max_lba + 1);
sdkp->capacity = max_lba + 1;
+ if (sdkp->capacity > 0xffffffff)
+ sdkp->device->use_16_for_rw = 1;
}
}
@@ -924,11 +926,6 @@ int sd_zbc_read_zones(struct scsi_disk *sdkp, u8
buf[SD_BUF_SIZE])
return 0;
}
- /* READ16/WRITE16/SYNC16 is mandatory for ZBC devices */
- sdkp->device->use_16_for_rw = 1;
- sdkp->device->use_10_for_rw = 0;
- sdkp->device->use_16_for_sync = 1;
-
/* Check zoned block device characteristics (unconstrained reads) */
ret = sd_zbc_check_zoned_characteristics(sdkp, buf);
if (ret)
