On 2020/04/23 1:46, Douglas Gilbert wrote:
> On 2020-04-22 6:42 a.m., Damien Le Moal wrote:
>> Implement ZBC host-aware device model emulation. The main changes from
>> the host-managed emulation are the device type (TYPE_DISK is used),
>> relaxation of access checks for read and write operations and different
>> handling of a sequential write preferred zone write pointer as mandated
>> by the ZBC r05 specifications.
>>
>> To facilitate the implementation and avoid a lot of "if" statement, the
>> zmodel field is added to the device information and the z_type field to
>> the zone state data structure.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxx>
>
> This is an unexpected surprise and a chance to exercise previously untested
> parts of my user space packages (sg3_utils and sdparm).
Yes, I decided yesterday to add this on top of the series we worked on together.
The main reason iss to allow for writing blktests to test partition handling
with host-aware drives as some special handling was added for these recently. If
an HA drive has partition, its zoned model is automatically changed to "NONE"
and restored to "host-aware" only when the partitions are deleted. Having
blktests cases to do regression testing on that iss I think important as this is
an unusal behavior. And you already had added the host-aware paramter cases so
everything was mostly already wired for this emulation case.
> A small nit: draft ZBC-2 has added the Zoned Block Device Extension field
> which should be set to 1 for host-aware (and 0 for host-managed). It is 0
> in both cases at the moment which is strictly speaking correct as 0 is
> defined as "not reported". IMO it is more correct to set it to 1 in the
> host-aware case :-)
>
> Ralph Weber failed in his attempt to get version strings thrown out at T10.
> So we might consider adding a ZBC version descriptor in the host-aware and
> host-managed case. [Version descriptors are in the standard INQUIRY
> response.]
ZBC-2 is still in draft state. We should not yet implement against it. Last I
checked with Ralph, the outlook for a stable spec is year end. I am monitoring
that since yhere are a lot more changes that will need to be taken care of on
the kernel side. ZBC-2 (and ZAC-2) extend reset/open/close/finish operations to
operate on a range of zones. At the very least, we will need that support in the
kernel SAT (drivers/ata/libata-scsi.c).
>
> Wish list: it would be good if lsscsi could indicate in its one line per
> device mode that a disk was actually a ZBC host-aware disk. lsscsi is
> restricted to datamining in sysfs and some other locations that don't
> need root permissions. So it does not issue any SCSI commands. To see a
> disk is ZBC host-aware it needs access to the Block Device Characteristics
> VPD page, but as far as I can see that is not loaded into sysfs at this
> time. Hannes?
>
> Tested-by: Douglas Gilbert <dgilbert@xxxxxxxxxxxx>
>
>> ---
>> drivers/scsi/scsi_debug.c | 148 ++++++++++++++++++++++++++------------
>> 1 file changed, 103 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
>> index 9279ac9bb98d..46cd4e64bb9c 100644
>> --- a/drivers/scsi/scsi_debug.c
>> +++ b/drivers/scsi/scsi_debug.c
>> @@ -262,6 +262,13 @@ static const char *sdebug_version_date = "20200421";
>>
>> #define SDEB_XA_NOT_IN_USE XA_MARK_1
>>
>> +/* Zone types (zbcr05 table 25) */
>> +enum sdebug_z_type {
>> + ZBC_ZONE_TYPE_CNV = 0x1,
>> + ZBC_ZONE_TYPE_SWR = 0x2,
>> + ZBC_ZONE_TYPE_SWP = 0x3,
>> +};
>> +
>> /* enumeration names taken from table 26, zbcr05 */
>> enum sdebug_z_cond {
>> ZBC_NOT_WRITE_POINTER = 0x0,
>> @@ -275,7 +282,9 @@ enum sdebug_z_cond {
>> };
>>
>> struct sdeb_zone_state { /* ZBC: per zone state */
>> + enum sdebug_z_type z_type;
>> enum sdebug_z_cond z_cond;
>> + bool z_non_seq_resource;
>> unsigned int z_size;
>> sector_t z_start;
>> sector_t z_wp;
>> @@ -294,6 +303,7 @@ struct sdebug_dev_info {
>> bool used;
>>
>> /* For ZBC devices */
>> + enum blk_zoned_model zmodel;
>> unsigned int zsize;
>> unsigned int zsize_shift;
>> unsigned int nr_zones;
>> @@ -822,7 +832,7 @@ static int dix_reads;
>> static int dif_errors;
>>
>> /* ZBC global data */
>> -static bool sdeb_zbc_in_use; /* true when ptype=TYPE_ZBC [0x14] */
>> +static bool sdeb_zbc_in_use; /* true for host-aware and host-managed disks */
>> static int sdeb_zbc_zone_size_mb;
>> static int sdeb_zbc_max_open = DEF_ZBC_MAX_OPEN_ZONES;
>> static int sdeb_zbc_nr_conv = DEF_ZBC_NR_CONV_ZONES;
>> @@ -1500,13 +1510,15 @@ static int inquiry_vpd_b0(unsigned char *arr)
>> }
>>
>> /* Block device characteristics VPD page (SBC-3) */
>> -static int inquiry_vpd_b1(unsigned char *arr)
>> +static int inquiry_vpd_b1(struct sdebug_dev_info *devip, unsigned char *arr)
>> {
>> memset(arr, 0, 0x3c);
>> arr[0] = 0;
>> arr[1] = 1; /* non rotating medium (e.g. solid state) */
>> arr[2] = 0;
>> arr[3] = 5; /* less than 1.8" */
>> + if (devip->zmodel == BLK_ZONED_HA)
>> + arr[4] = 1 << 4; /* zoned field = 01b */
>>
>> return 0x3c;
>> }
>> @@ -1543,7 +1555,7 @@ static int inquiry_vpd_b6(struct sdebug_dev_info *devip, unsigned char *arr)
>> */
>> put_unaligned_be32(0xffffffff, &arr[4]);
>> put_unaligned_be32(0xffffffff, &arr[8]);
>> - if (devip->max_open)
>> + if (sdeb_zbc_model == BLK_ZONED_HM && devip->max_open)
>> put_unaligned_be32(devip->max_open, &arr[12]);
>> else
>> put_unaligned_be32(0xffffffff, &arr[12]);
>> @@ -1566,7 +1578,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>> if (! arr)
>> return DID_REQUEUE << 16;
>> is_disk = (sdebug_ptype == TYPE_DISK);
>> - is_zbc = (sdebug_ptype == TYPE_ZBC);
>> + is_zbc = (devip->zmodel != BLK_ZONED_NONE);
>> is_disk_zbc = (is_disk || is_zbc);
>> have_wlun = scsi_is_wlun(scp->device->lun);
>> if (have_wlun)
>> @@ -1611,7 +1623,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>> arr[n++] = 0xb1; /* Block characteristics */
>> if (is_disk)
>> arr[n++] = 0xb2; /* LB Provisioning */
>> - else if (is_zbc)
>> + if (is_zbc)
>> arr[n++] = 0xb6; /* ZB dev. char. */
>> }
>> arr[3] = n - 4; /* number of supported VPD pages */
>> @@ -1660,7 +1672,7 @@ static int resp_inquiry(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
>> arr[3] = inquiry_vpd_b0(&arr[4]);
>> } else if (is_disk_zbc && 0xb1 == cmd[2]) { /* Block char. */
>> arr[1] = cmd[2]; /*sanity */
>> - arr[3] = inquiry_vpd_b1(&arr[4]);
>> + arr[3] = inquiry_vpd_b1(devip, &arr[4]);
>> } else if (is_disk && 0xb2 == cmd[2]) { /* LB Prov. */
>> arr[1] = cmd[2]; /*sanity */
>> arr[3] = inquiry_vpd_b2(&arr[4]);
>> @@ -2305,7 +2317,7 @@ static int resp_mode_sense(struct scsi_cmnd *scp,
>> msense_6 = (MODE_SENSE == cmd[0]);
>> llbaa = msense_6 ? false : !!(cmd[1] & 0x10);
>> is_disk = (sdebug_ptype == TYPE_DISK);
>> - is_zbc = (sdebug_ptype == TYPE_ZBC);
>> + is_zbc = (devip->zmodel != BLK_ZONED_NONE);
>> if ((is_disk || is_zbc) && !dbd)
>> bd_len = llbaa ? 16 : 8;
>> else
>> @@ -2656,7 +2668,7 @@ static struct sdeb_zone_state *zbc_zone(struct sdebug_dev_info *devip,
>>
>> static inline bool zbc_zone_is_conv(struct sdeb_zone_state *zsp)
>> {
>> - return zsp->z_cond == ZBC_NOT_WRITE_POINTER;
>> + return zsp->z_type == ZBC_ZONE_TYPE_CNV;
>> }
>>
>> static void zbc_close_zone(struct sdebug_dev_info *devip,
>> @@ -2732,13 +2744,42 @@ static void zbc_inc_wp(struct sdebug_dev_info *devip,
>> unsigned long long lba, unsigned int num)
>> {
>> struct sdeb_zone_state *zsp = zbc_zone(devip, lba);
>> + unsigned long long n, end, zend = zsp->z_start + zsp->z_size;
>>
>> if (zbc_zone_is_conv(zsp))
>> return;
>>
>> - zsp->z_wp += num;
>> - if (zsp->z_wp >= zsp->z_start + zsp->z_size)
>> - zsp->z_cond = ZC5_FULL;
>> + if (zsp->z_type == ZBC_ZONE_TYPE_SWR) {
>> + zsp->z_wp += num;
>> + if (zsp->z_wp >= zend)
>> + zsp->z_cond = ZC5_FULL;
>> + return;
>> + }
>> +
>> + while (num) {
>> + if (lba != zsp->z_wp)
>> + zsp->z_non_seq_resource = true;
>> +
>> + end = lba + num;
>> + if (end >= zend) {
>> + n = zend - lba;
>> + zsp->z_wp = zend;
>> + } else if (end > zsp->z_wp) {
>> + n = num;
>> + zsp->z_wp = end;
>> + } else {
>> + n = num;
>> + }
>> + if (zsp->z_wp >= zend)
>> + zsp->z_cond = ZC5_FULL;
>> +
>> + num -= n;
>> + lba += n;
>> + if (num) {
>> + zsp++;
>> + zend = zsp->z_start + zsp->z_size;
>> + }
>> + }
>> }
>>
>> static int check_zbc_access_params(struct scsi_cmnd *scp,
>> @@ -2750,7 +2791,9 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
>> struct sdeb_zone_state *zsp_end = zbc_zone(devip, lba + num - 1);
>>
>> if (!write) {
>> - /* Reads cannot cross zone types boundaries */
>> + if (devip->zmodel == BLK_ZONED_HA)
>> + return 0;
>> + /* For host-managed, reads cannot cross zone types boundaries */
>> if (zsp_end != zsp &&
>> zbc_zone_is_conv(zsp) &&
>> !zbc_zone_is_conv(zsp_end)) {
>> @@ -2773,25 +2816,27 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
>> return 0;
>> }
>>
>> - /* Writes cannot cross sequential zone boundaries */
>> - if (zsp_end != zsp) {
>> - mk_sense_buffer(scp, ILLEGAL_REQUEST,
>> - LBA_OUT_OF_RANGE,
>> - WRITE_BOUNDARY_ASCQ);
>> - return check_condition_result;
>> - }
>> - /* Cannot write full zones */
>> - if (zsp->z_cond == ZC5_FULL) {
>> - mk_sense_buffer(scp, ILLEGAL_REQUEST,
>> - INVALID_FIELD_IN_CDB, 0);
>> - return check_condition_result;
>> - }
>> - /* Writes must be aligned to the zone WP */
>> - if (lba != zsp->z_wp) {
>> - mk_sense_buffer(scp, ILLEGAL_REQUEST,
>> - LBA_OUT_OF_RANGE,
>> - UNALIGNED_WRITE_ASCQ);
>> - return check_condition_result;
>> + if (zsp->z_type == ZBC_ZONE_TYPE_SWR) {
>> + /* Writes cannot cross sequential zone boundaries */
>> + if (zsp_end != zsp) {
>> + mk_sense_buffer(scp, ILLEGAL_REQUEST,
>> + LBA_OUT_OF_RANGE,
>> + WRITE_BOUNDARY_ASCQ);
>> + return check_condition_result;
>> + }
>> + /* Cannot write full zones */
>> + if (zsp->z_cond == ZC5_FULL) {
>> + mk_sense_buffer(scp, ILLEGAL_REQUEST,
>> + INVALID_FIELD_IN_CDB, 0);
>> + return check_condition_result;
>> + }
>> + /* Writes must be aligned to the zone WP */
>> + if (lba != zsp->z_wp) {
>> + mk_sense_buffer(scp, ILLEGAL_REQUEST,
>> + LBA_OUT_OF_RANGE,
>> + UNALIGNED_WRITE_ASCQ);
>> + return check_condition_result;
>> + }
>> }
>>
>> /* Handle implicit open of closed and empty zones */
>> @@ -4312,13 +4357,16 @@ static int resp_report_zones(struct scsi_cmnd *scp,
>> case 0x06:
>> case 0x07:
>> case 0x10:
>> - case 0x11:
>> /*
>> - * Read-only, offline, reset WP recommended and
>> - * non-seq-resource-used are not emulated: no zones
>> - * to report;
>> + * Read-only, offline, reset WP recommended are
>> + * not emulated: no zones to report;
>> */
>> continue;
>> + case 0x11:
>> + /* non-seq-resource set */
>> + if (!zsp->z_non_seq_resource)
>> + continue;
>> + break;
>> case 0x3f:
>> /* Not write pointer (conventional) zones */
>> if (!zbc_zone_is_conv(zsp))
>> @@ -4333,11 +4381,10 @@ static int resp_report_zones(struct scsi_cmnd *scp,
>>
>> if (nrz < rep_max_zones) {
>> /* Fill zone descriptor */
>> - if (zbc_zone_is_conv(zsp))
>> - desc[0] = 0x1;
>> - else
>> - desc[0] = 0x2;
>> + desc[0] = zsp->z_type;
>> desc[1] = zsp->z_cond << 4;
>> + if (zsp->z_non_seq_resource)
>> + desc[1] |= 1 << 1;
>> put_unaligned_be64((u64)zsp->z_size, desc + 8);
>> put_unaligned_be64((u64)zsp->z_start, desc + 16);
>> put_unaligned_be64((u64)zsp->z_wp, desc + 24);
>> @@ -4591,6 +4638,7 @@ static void zbc_rwp_zone(struct sdebug_dev_info *devip,
>> if (zsp->z_cond == ZC4_CLOSED)
>> devip->nr_closed--;
>>
>> + zsp->z_non_seq_resource = false;
>> zsp->z_wp = zsp->z_start;
>> zsp->z_cond = ZC1_EMPTY;
>> }
>> @@ -4796,11 +4844,13 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
>> }
>> devip->nr_conv_zones = sdeb_zbc_nr_conv;
>>
>> - /* zbc_max_open_zones can be 0, meaning "not reported" (no limit) */
>> - if (sdeb_zbc_max_open >= devip->nr_zones - 1)
>> - devip->max_open = (devip->nr_zones - 1) / 2;
>> - else
>> - devip->max_open = sdeb_zbc_max_open;
>> + if (devip->zmodel == BLK_ZONED_HM) {
>> + /* zbc_max_open_zones can be 0, meaning "not reported" */
>> + if (sdeb_zbc_max_open >= devip->nr_zones - 1)
>> + devip->max_open = (devip->nr_zones - 1) / 2;
>> + else
>> + devip->max_open = sdeb_zbc_max_open;
>> + }
>>
>> devip->zstate = kcalloc(devip->nr_zones,
>> sizeof(struct sdeb_zone_state), GFP_KERNEL);
>> @@ -4813,9 +4863,14 @@ static int sdebug_device_create_zones(struct sdebug_dev_info *devip)
>> zsp->z_start = zstart;
>>
>> if (i < devip->nr_conv_zones) {
>> + zsp->z_type = ZBC_ZONE_TYPE_CNV;
>> zsp->z_cond = ZBC_NOT_WRITE_POINTER;
>> zsp->z_wp = (sector_t)-1;
>> } else {
>> + if (devip->zmodel == BLK_ZONED_HM)
>> + zsp->z_type = ZBC_ZONE_TYPE_SWR;
>> + else
>> + zsp->z_type = ZBC_ZONE_TYPE_SWP;
>> zsp->z_cond = ZC1_EMPTY;
>> zsp->z_wp = zsp->z_start;
>> }
>> @@ -4851,10 +4906,13 @@ static struct sdebug_dev_info *sdebug_device_create(
>> }
>> devip->sdbg_host = sdbg_host;
>> if (sdeb_zbc_in_use) {
>> + devip->zmodel = sdeb_zbc_model;
>> if (sdebug_device_create_zones(devip)) {
>> kfree(devip);
>> return NULL;
>> }
>> + } else {
>> + devip->zmodel = BLK_ZONED_NONE;
>> }
>> devip->sdbg_host = sdbg_host;
>> list_add_tail(&devip->dev_list, &sdbg_host->dev_info_list);
>> @@ -6566,12 +6624,12 @@ static int __init scsi_debug_init(void)
>> sdeb_zbc_model = k;
>> switch (sdeb_zbc_model) {
>> case BLK_ZONED_NONE:
>> + case BLK_ZONED_HA:
>> sdebug_ptype = TYPE_DISK;
>> break;
>> case BLK_ZONED_HM:
>> sdebug_ptype = TYPE_ZBC;
>> break;
>> - case BLK_ZONED_HA:
>> default:
>> pr_err("Invalid ZBC model\n");
>> return -EINVAL;
>>
>
>
--
Damien Le Moal
Western Digital Research
