On 2024/02/29 10:54, Bart Van Assche wrote:
> On 2/29/24 10:31, Damien Le Moal wrote:
>> Yes, but I find that a little fragile and given that rw-10 causes problems with
>> ZBC, I prefer to make it very explicit that the 10B command variants should not
>> be used.
>
> Hi Damien,
>
> From commit c6463c651d7a ("sd_zbc: Force use of READ16/WRITE16"; v4.10):
>
> -------------------------------------------------------------------------
> sd_zbc: Force use of READ16/WRITE16
>
> Normally, sd_read_capacity sets sdp->use_16_for_rw to 1 based on the
> disk capacity so that READ16/WRITE16 are used for large drives. However,
> for a zoned disk with RC_BASIS set to 0, the capacity reported through
> READ_CAPACITY may be very small, leading to use_16_for_rw not being
> set and READ10/WRITE10 commands being used, even after the actual zoned
> disk capacity is corrected in sd_zbc_read_zones. This causes LBA offset
> overflow for accesses beyond 2TB.
>
> As the ZBC standard makes it mandatory for ZBC drives to support
> the READ16/WRITE16 commands anyway, make sure that use_16_for_rw is set.
> -------------------------------------------------------------------------
>
> Would this change be sufficient to fix the problems mentioned above?
>
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 997de4daa8c4..71f477e502e9 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -1279,7 +1279,7 @@ static blk_status_t
> sd_setup_read_write_cmnd(struct scsi_cmnd *cmd)
> if (protect && sdkp->protection_type == T10_PI_TYPE2_PROTECTION) {
> ret = sd_setup_rw32_cmnd(cmd, write, lba, nr_blocks,
> protect | fua, dld);
> - } else if (sdp->use_16_for_rw || (nr_blocks > 0xffff)) {
> + } else if (sdp->use_16_for_rw || (nr_blocks > 0xffff) || (lba >> 32)) {
Sure, that works too, but seems useless given that we do have use_16_for_rw set.
Would clearing use_10_for_rw to 0 cause a problem for zoned UFS drives ?
> ret = sd_setup_rw16_cmnd(cmd, write, lba, nr_blocks,
> protect | fua, dld);
> } else if ((nr_blocks > 0xff) || (lba > 0x1fffff) ||
>
> Thanks,
>
> Bart.
--
Damien Le Moal
Western Digital Research
