[PATCHv2 0/4] Initial SMR drive support

[PATCHv2 0/4] Initial SMR drive support

[Hannes Reinecke]

this is a first stab at implementing SMR support.
The powers that be decided to call the ATA implementation
'ZAC' (zoned access commands), and the SCSI implementation
'ZBC' (zoned block commands).

This is just basic enablement to get ZAC and ZBC drives
handled correctly.
The first three patches update the libata SATL to handle
ZAC devices correctly, and the last patch updates the 'sd'
to work correctly with ZBC devices.
ZBC devices will _not_ be handled with the 'sd' driver
automatically ss of now; however, the sd device can
be bound manually to the device by using the sysfs
'bind' attribute.

None of the specific commands like 'report zones' or
'reset write pointer' have been implemented yet as the
actual format is still not finalized.

This patch is made against the core-for-3.17 tree from hch.

Hannes Reinecke (4):
  libata: consolidate ata_dev_classify()
  libata: Implement ATA_DEV_ZAC
  libata-scsi: Update SATL for ZAC drives
  sd: Handle ZBC drives correctly

 drivers/ata/libahci.c               | 11 +++----
 drivers/ata/libata-core.c           | 34 +++++++++++++-------
 drivers/ata/libata-eh.c             |  7 +++--
 drivers/ata/libata-scsi.c           | 34 +++++++++++++++++---
 drivers/ata/libata-sff.c            |  2 +-
 drivers/ata/libata-transport.c      |  1 +
 drivers/ata/sata_fsl.c              | 11 +++----
 drivers/ata/sata_inic162x.c         |  2 +-
 drivers/ata/sata_sil24.c            |  2 +-
 drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
 drivers/scsi/isci/request.c         |  4 +--
 drivers/scsi/libsas/sas_ata.c       | 63 +++++--------------------------------
 drivers/scsi/mvsas/mv_sas.c         |  4 +--
 drivers/scsi/pm8001/pm8001_hwi.c    |  2 +-
 drivers/scsi/pm8001/pm80xx_hwi.c    |  2 +-
 drivers/scsi/sd.c                   | 10 +++---
 include/linux/libata.h              |  8 +++--
 include/scsi/libsas.h               | 11 ++-----
 18 files changed, 103 insertions(+), 115 deletions(-)

-- 
1.7.12.4

[PATCH 1/4] libata: consolidate ata_dev_classify()

[Hannes Reinecke]

ata_dev_classify() just uses the 'lbah' and 'lbam'
fields from the taskfile, so we can as well use those
as arguments and rip out the custom code from sas_ata.

Cc: Tejun Heo <tj@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libahci.c               | 11 +++----
 drivers/ata/libata-core.c           | 14 +++++----
 drivers/ata/libata-sff.c            |  2 +-
 drivers/ata/sata_fsl.c              | 11 +++----
 drivers/ata/sata_inic162x.c         |  2 +-
 drivers/ata/sata_sil24.c            |  2 +-
 drivers/scsi/aic94xx/aic94xx_task.c | 10 +++---
 drivers/scsi/isci/request.c         |  4 +--
 drivers/scsi/libsas/sas_ata.c       | 63 +++++--------------------------------
 drivers/scsi/mvsas/mv_sas.c         |  4 +--
 drivers/scsi/pm8001/pm8001_hwi.c    |  2 +-
 drivers/scsi/pm8001/pm80xx_hwi.c    |  2 +-
 include/linux/libata.h              |  2 +-
 include/scsi/libsas.h               | 11 ++-----
 14 files changed, 44 insertions(+), 96 deletions(-)

diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
index d72ce04..09f9fcb 100644
--- a/drivers/ata/libahci.c
+++ b/drivers/ata/libahci.c
@@ -1185,16 +1185,15 @@ static void ahci_dev_config(struct ata_device *dev)
 unsigned int ahci_dev_classify(struct ata_port *ap)
 {
 	void __iomem *port_mmio = ahci_port_base(ap);
-	struct ata_taskfile tf;
+	u8 lbah, lbam, lbal;
 	u32 tmp;
 
 	tmp = readl(port_mmio + PORT_SIG);
-	tf.lbah		= (tmp >> 24)	& 0xff;
-	tf.lbam		= (tmp >> 16)	& 0xff;
-	tf.lbal		= (tmp >> 8)	& 0xff;
-	tf.nsect	= (tmp)		& 0xff;
+	lbah		= (tmp >> 24)	& 0xff;
+	lbam		= (tmp >> 16)	& 0xff;
+	lbal		= (tmp >> 8)	& 0xff;
 
-	return ata_dev_classify(&tf);
+	return ata_dev_classify(lbah, lbam, lbal);
 }
 EXPORT_SYMBOL_GPL(ahci_dev_classify);
 
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 18d97d5..2c4d742 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1032,7 +1032,9 @@ const char *sata_spd_string(unsigned int spd)
 
 /**
  *	ata_dev_classify - determine device type based on ATA-spec signature
- *	@tf: ATA taskfile register set for device to be identified
+ *	@lbah: LBA high byte (LBA bits 23:16)
+ *	@lbam: LBA mid byte (LBA bits 15:8)
+ *	@lbal: LBA low byte (LBA bits 7:0)
  *
  *	Determine from taskfile register contents whether a device is
  *	ATA or ATAPI, as per "Signature and persistence" section
@@ -1045,7 +1047,7 @@ const char *sata_spd_string(unsigned int spd)
  *	Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
  *	%ATA_DEV_UNKNOWN the event of failure.
  */
-unsigned int ata_dev_classify(const struct ata_taskfile *tf)
+unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
 {
 	/* Apple's open source Darwin code hints that some devices only
 	 * put a proper signature into the LBA mid/high registers,
@@ -1068,22 +1070,22 @@ unsigned int ata_dev_classify(const struct ata_taskfile *tf)
 	 * SEMB signature.  This is worked around in
 	 * ata_dev_read_id().
 	 */
-	if ((tf->lbam == 0) && (tf->lbah == 0)) {
+	if ((lbam == 0) && (lbah == 0)) {
 		DPRINTK("found ATA device by sig\n");
 		return ATA_DEV_ATA;
 	}
 
-	if ((tf->lbam == 0x14) && (tf->lbah == 0xeb)) {
+	if ((lbam == 0x14) && (lbah == 0xeb)) {
 		DPRINTK("found ATAPI device by sig\n");
 		return ATA_DEV_ATAPI;
 	}
 
-	if ((tf->lbam == 0x69) && (tf->lbah == 0x96)) {
+	if ((lbam == 0x69) && (lbah == 0x96)) {
 		DPRINTK("found PMP device by sig\n");
 		return ATA_DEV_PMP;
 	}
 
-	if ((tf->lbam == 0x3c) && (tf->lbah == 0xc3)) {
+	if ((lbam == 0x3c) && (lbah == 0xc3)) {
 		DPRINTK("found SEMB device by sig (could be ATA device)\n");
 		return ATA_DEV_SEMB;
 	}
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 1121153..ec9b5db 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1903,7 +1903,7 @@ unsigned int ata_sff_dev_classify(struct ata_device *dev, int present,
 		return ATA_DEV_NONE;
 
 	/* determine if device is ATA or ATAPI */
-	class = ata_dev_classify(&tf);
+	class = ata_dev_classify(tf.lbah, tf.lbam, tf.lbal);
 
 	if (class == ATA_DEV_UNKNOWN) {
 		/* If the device failed diagnostic, it's likely to
diff --git a/drivers/ata/sata_fsl.c b/drivers/ata/sata_fsl.c
index 616a6d2..2194fa0 100644
--- a/drivers/ata/sata_fsl.c
+++ b/drivers/ata/sata_fsl.c
@@ -807,8 +807,8 @@ static unsigned int sata_fsl_dev_classify(struct ata_port *ap)
 {
 	struct sata_fsl_host_priv *host_priv = ap->host->private_data;
 	void __iomem *hcr_base = host_priv->hcr_base;
-	struct ata_taskfile tf;
 	u32 temp;
+	u8 lbah, lbam, lbal;
 
 	temp = ioread32(hcr_base + SIGNATURE);
 
@@ -816,12 +816,11 @@ static unsigned int sata_fsl_dev_classify(struct ata_port *ap)
 	VPRINTK("HStatus = 0x%x\n", ioread32(hcr_base + HSTATUS));
 	VPRINTK("HControl = 0x%x\n", ioread32(hcr_base + HCONTROL));
 
-	tf.lbah = (temp >> 24) & 0xff;
-	tf.lbam = (temp >> 16) & 0xff;
-	tf.lbal = (temp >> 8) & 0xff;
-	tf.nsect = temp & 0xff;
+	lbah = (temp >> 24) & 0xff;
+	lbam = (temp >> 16) & 0xff;
+	lbal = (temp >>  8) & 0xff;
 
-	return ata_dev_classify(&tf);
+	return ata_dev_classify(lbah, lbam, lbal);
 }
 
 static int sata_fsl_hardreset(struct ata_link *link, unsigned int *class,
diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index 0698278..7d95cc4 100644
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -649,7 +649,7 @@ static int inic_hardreset(struct ata_link *link, unsigned int *class,
 		}
 
 		inic_tf_read(ap, &tf);
-		*class = ata_dev_classify(&tf);
+		*class = ata_dev_classify(tf. lbah, tf.lbam, tf.lbal);
 	}
 
 	return 0;
diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index 0534890..c69ebaf 100644
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -686,7 +686,7 @@ static int sil24_softreset(struct ata_link *link, unsigned int *class,
 	}
 
 	sil24_read_tf(ap, 0, &tf);
-	*class = ata_dev_classify(&tf);
+	*class = ata_dev_classify(tf. lbah, tf.lbam, tf.lbal);
 
 	DPRINTK("EXIT, class=%u\n", *class);
 	return 0;
diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
index 59b86e2..7abbe42 100644
--- a/drivers/scsi/aic94xx/aic94xx_task.c
+++ b/drivers/scsi/aic94xx/aic94xx_task.c
@@ -373,10 +373,10 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct sas_task *task,
 
 	if (unlikely(task->ata_task.device_control_reg_update))
 		scb->header.opcode = CONTROL_ATA_DEV;
-	else if (dev->sata_dev.command_set == ATA_COMMAND_SET)
-		scb->header.opcode = INITIATE_ATA_TASK;
-	else
+	else if (dev->sata_dev.class == ATA_DEV_ATAPI)
 		scb->header.opcode = INITIATE_ATAPI_TASK;
+	else
+		scb->header.opcode = INITIATE_ATA_TASK;
 
 	scb->ata_task.proto_conn_rate = (1 << 5); /* STP */
 	if (dev->port->oob_mode == SAS_OOB_MODE)
@@ -387,7 +387,7 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct sas_task *task,
 	if (likely(!task->ata_task.device_control_reg_update))
 		scb->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
 	scb->ata_task.fis.flags &= 0xF0; /* PM_PORT field shall be 0 */
-	if (dev->sata_dev.command_set == ATAPI_COMMAND_SET)
+	if (dev->sata_dev.class == ATA_DEV_ATAPI)
 		memcpy(scb->ata_task.atapi_packet, task->ata_task.atapi_packet,
 		       16);
 	scb->ata_task.sister_scb = cpu_to_le16(0xFFFF);
@@ -399,7 +399,7 @@ static int asd_build_ata_ascb(struct asd_ascb *ascb, struct sas_task *task,
 		if (task->ata_task.dma_xfer)
 			flags |= DATA_XFER_MODE_DMA;
 		if (task->ata_task.use_ncq &&
-		    dev->sata_dev.command_set != ATAPI_COMMAND_SET)
+		    dev->sata_dev.class != ATA_DEV_ATAPI)
 			flags |= ATA_Q_TYPE_NCQ;
 		flags |= data_dir_flags[task->data_dir];
 		scb->ata_task.ata_flags = flags;
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index 56e3809..cfd0084 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -694,7 +694,7 @@ sci_io_request_construct_sata(struct isci_request *ireq,
 	}
 
 	/* ATAPI */
-	if (dev->sata_dev.command_set == ATAPI_COMMAND_SET &&
+	if (dev->sata_dev.class == ATA_DEV_ATAPI &&
 	    task->ata_task.fis.command == ATA_CMD_PACKET) {
 		sci_atapi_construct(ireq);
 		return SCI_SUCCESS;
@@ -2980,7 +2980,7 @@ static void sci_request_started_state_enter(struct sci_base_state_machine *sm)
 		state = SCI_REQ_SMP_WAIT_RESP;
 	} else if (task && sas_protocol_ata(task->task_proto) &&
 		   !task->ata_task.use_ncq) {
-		if (dev->sata_dev.command_set == ATAPI_COMMAND_SET &&
+		if (dev->sata_dev.class == ATA_DEV_ATAPI &&
 			task->ata_task.fis.command == ATA_CMD_PACKET) {
 			state = SCI_REQ_ATAPI_WAIT_H2D;
 		} else if (task->data_dir == DMA_NONE) {
diff --git a/drivers/scsi/libsas/sas_ata.c b/drivers/scsi/libsas/sas_ata.c
index 766098a..865de86 100644
--- a/drivers/scsi/libsas/sas_ata.c
+++ b/drivers/scsi/libsas/sas_ata.c
@@ -138,7 +138,7 @@ static void sas_ata_task_done(struct sas_task *task)
 
 	if (stat->stat == SAS_PROTO_RESPONSE || stat->stat == SAM_STAT_GOOD ||
 	    ((stat->stat == SAM_STAT_CHECK_CONDITION &&
-	      dev->sata_dev.command_set == ATAPI_COMMAND_SET))) {
+	      dev->sata_dev.class == ATA_DEV_ATAPI))) {
 		memcpy(dev->sata_dev.fis, resp->ending_fis, ATA_RESP_FIS_SIZE);
 
 		if (!link->sactive) {
@@ -278,7 +278,7 @@ static struct sas_internal *dev_to_sas_internal(struct domain_device *dev)
 	return to_sas_internal(dev->port->ha->core.shost->transportt);
 }
 
-static void sas_get_ata_command_set(struct domain_device *dev);
+static int sas_get_ata_command_set(struct domain_device *dev);
 
 int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
 {
@@ -303,8 +303,7 @@ int sas_get_ata_info(struct domain_device *dev, struct ex_phy *phy)
 		}
 		memcpy(dev->frame_rcvd, &dev->sata_dev.rps_resp.rps.fis,
 		       sizeof(struct dev_to_host_fis));
-		/* TODO switch to ata_dev_classify() */
-		sas_get_ata_command_set(dev);
+		dev->sata_dev.class = sas_get_ata_command_set(dev);
 	}
 	return 0;
 }
@@ -425,18 +424,7 @@ static int sas_ata_hard_reset(struct ata_link *link, unsigned int *class,
 	if (ret && ret != -EAGAIN)
 		sas_ata_printk(KERN_ERR, dev, "reset failed (errno=%d)\n", ret);
 
-	/* XXX: if the class changes during the reset the upper layer
-	 * should be informed, if the device has gone away we assume
-	 * libsas will eventually delete it
-	 */
-	switch (dev->sata_dev.command_set) {
-	case ATA_COMMAND_SET:
-		*class = ATA_DEV_ATA;
-		break;
-	case ATAPI_COMMAND_SET:
-		*class = ATA_DEV_ATAPI;
-		break;
-	}
+	*class = dev->sata_dev.class;
 
 	ap->cbl = ATA_CBL_SATA;
 	return ret;
@@ -626,50 +614,15 @@ void sas_ata_task_abort(struct sas_task *task)
 	complete(waiting);
 }
 
-static void sas_get_ata_command_set(struct domain_device *dev)
+static int sas_get_ata_command_set(struct domain_device *dev)
 {
 	struct dev_to_host_fis *fis =
 		(struct dev_to_host_fis *) dev->frame_rcvd;
 
 	if (dev->dev_type == SAS_SATA_PENDING)
-		return;
+		return ATA_DEV_UNKNOWN;
 
-	if ((fis->sector_count == 1 && /* ATA */
-	     fis->lbal         == 1 &&
-	     fis->lbam         == 0 &&
-	     fis->lbah         == 0 &&
-	     fis->device       == 0)
-	    ||
-	    (fis->sector_count == 0 && /* CE-ATA (mATA) */
-	     fis->lbal         == 0 &&
-	     fis->lbam         == 0xCE &&
-	     fis->lbah         == 0xAA &&
-	     (fis->device & ~0x10) == 0))
-
-		dev->sata_dev.command_set = ATA_COMMAND_SET;
-
-	else if ((fis->interrupt_reason == 1 &&	/* ATAPI */
-		  fis->lbal             == 1 &&
-		  fis->byte_count_low   == 0x14 &&
-		  fis->byte_count_high  == 0xEB &&
-		  (fis->device & ~0x10) == 0))
-
-		dev->sata_dev.command_set = ATAPI_COMMAND_SET;
-
-	else if ((fis->sector_count == 1 && /* SEMB */
-		  fis->lbal         == 1 &&
-		  fis->lbam         == 0x3C &&
-		  fis->lbah         == 0xC3 &&
-		  fis->device       == 0)
-		||
-		 (fis->interrupt_reason == 1 &&	/* SATA PM */
-		  fis->lbal             == 1 &&
-		  fis->byte_count_low   == 0x69 &&
-		  fis->byte_count_high  == 0x96 &&
-		  (fis->device & ~0x10) == 0))
-
-		/* Treat it as a superset? */
-		dev->sata_dev.command_set = ATAPI_COMMAND_SET;
+	return ata_dev_classify(fis->lbah, fis->lbam, fis->lbal);
 }
 
 void sas_probe_sata(struct asd_sas_port *port)
@@ -775,7 +728,7 @@ int sas_discover_sata(struct domain_device *dev)
 	if (dev->dev_type == SAS_SATA_PM)
 		return -ENODEV;
 
-	sas_get_ata_command_set(dev);
+	dev->sata_dev.class = sas_get_ata_command_set(dev);
 	sas_fill_in_rphy(dev, dev->rphy);
 
 	res = sas_notify_lldd_dev_found(dev);
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 6c1f223..44625b8 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -479,7 +479,7 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,
 
 	if (task->ata_task.use_ncq)
 		flags |= MCH_FPDMA;
-	if (dev->sata_dev.command_set == ATAPI_COMMAND_SET) {
+	if (dev->sata_dev.class == ATA_DEV_ATAPI) {
 		if (task->ata_task.fis.command != ATA_CMD_ID_ATAPI)
 			flags |= MCH_ATAPI;
 	}
@@ -546,7 +546,7 @@ static int mvs_task_prep_ata(struct mvs_info *mvi,
 		task->ata_task.fis.flags |= 0x80; /* C=1: update ATA cmd reg */
 	/* fill in command FIS and ATAPI CDB */
 	memcpy(buf_cmd, &task->ata_task.fis, sizeof(struct host_to_dev_fis));
-	if (dev->sata_dev.command_set == ATAPI_COMMAND_SET)
+	if (dev->sata_dev.class == ATA_DEV_ATAPI)
 		memcpy(buf_cmd + STP_ATAPI_CMD,
 			task->ata_task.atapi_packet, 16);
 
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index a97be01..70f9b45 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -4356,7 +4356,7 @@ static int pm8001_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
 			PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO\n"));
 		}
 		if (task->ata_task.use_ncq &&
-			dev->sata_dev.command_set != ATAPI_COMMAND_SET) {
+			dev->sata_dev.class != ATA_DEV_ATAPI) {
 			ATAP = 0x07; /* FPDMA */
 			PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA\n"));
 		}
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index d70587f..b1f47f7 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -4059,7 +4059,7 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
 			PM8001_IO_DBG(pm8001_ha, pm8001_printk("PIO\n"));
 		}
 		if (task->ata_task.use_ncq &&
-			dev->sata_dev.command_set != ATAPI_COMMAND_SET) {
+		    dev->sata_dev.class != ATA_DEV_ATAPI) {
 			ATAP = 0x07; /* FPDMA */
 			PM8001_IO_DBG(pm8001_ha, pm8001_printk("FPDMA\n"));
 		}
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 5ab4e3a..3412aee 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -1172,7 +1172,7 @@ extern int ata_std_qc_defer(struct ata_queued_cmd *qc);
 extern void ata_noop_qc_prep(struct ata_queued_cmd *qc);
 extern void ata_sg_init(struct ata_queued_cmd *qc, struct scatterlist *sg,
 		 unsigned int n_elem);
-extern unsigned int ata_dev_classify(const struct ata_taskfile *tf);
+extern unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal);
 extern void ata_dev_disable(struct ata_device *adev);
 extern void ata_id_string(const u16 *id, unsigned char *s,
 			  unsigned int ofs, unsigned int len);
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index ef7872c..8528a09 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -161,17 +161,12 @@ struct expander_device {
 };
 
 /* ---------- SATA device ---------- */
-enum ata_command_set {
-        ATA_COMMAND_SET   = 0,
-        ATAPI_COMMAND_SET = 1,
-};
-
 #define ATA_RESP_FIS_SIZE 24
 
 struct sata_device {
-        enum   ata_command_set command_set;
-        struct smp_resp        rps_resp; /* report_phy_sata_resp */
-        u8     port_no;        /* port number, if this is a PM (Port) */
+	unsigned int class;
+	struct smp_resp        rps_resp; /* report_phy_sata_resp */
+	u8     port_no;        /* port number, if this is a PM (Port) */
 
 	struct ata_port *ap;
 	struct ata_host ata_host;
-- 
1.7.12.4

[PATCH 2/4] libata: Implement ATA_DEV_ZAC

[Hannes Reinecke]

Add new ATA device type for ZAC devices.

Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-core.c      | 20 ++++++++++++++------
 drivers/ata/libata-eh.c        |  7 +++++--
 drivers/ata/libata-scsi.c      |  5 +++--
 drivers/ata/libata-transport.c |  1 +
 include/linux/libata.h         |  6 ++++--
 5 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2c4d742..5f23804 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1044,8 +1044,8 @@ const char *sata_spd_string(unsigned int spd)
  *	None.
  *
  *	RETURNS:
- *	Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP or
- *	%ATA_DEV_UNKNOWN the event of failure.
+ *	Device type, %ATA_DEV_ATA, %ATA_DEV_ATAPI, %ATA_DEV_PMP,
+ *	%ATA_DEV_ZAC, or %ATA_DEV_UNKNOWN the event of failure.
  */
 unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
 {
@@ -1090,6 +1090,11 @@ unsigned int ata_dev_classify(u8 lbah, u8 lbam, u8 lbal)
 		return ATA_DEV_SEMB;
 	}
 
+	if ((lbam == 0xcd) && (lbah == 0xab)) {
+		DPRINTK("found ZAC device by sig\n");
+		return ATA_DEV_ZAC;
+	}
+
 	DPRINTK("unknown device\n");
 	return ATA_DEV_UNKNOWN;
 }
@@ -1330,7 +1335,7 @@ static int ata_hpa_resize(struct ata_device *dev)
 	int rc;
 
 	/* do we need to do it? */
-	if (dev->class != ATA_DEV_ATA ||
+	if ((dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ZAC) ||
 	    !ata_id_has_lba(dev->id) || !ata_id_hpa_enabled(dev->id) ||
 	    (dev->horkage & ATA_HORKAGE_BROKEN_HPA))
 		return 0;
@@ -1890,6 +1895,7 @@ retry:
 	case ATA_DEV_SEMB:
 		class = ATA_DEV_ATA;	/* some hard drives report SEMB sig */
 	case ATA_DEV_ATA:
+	case ATA_DEV_ZAC:
 		tf.command = ATA_CMD_ID_ATA;
 		break;
 	case ATA_DEV_ATAPI:
@@ -1981,7 +1987,7 @@ retry:
 	rc = -EINVAL;
 	reason = "device reports invalid type";
 
-	if (class == ATA_DEV_ATA) {
+	if (class == ATA_DEV_ATA || class == ATA_DEV_ZAC) {
 		if (!ata_id_is_ata(id) && !ata_id_is_cfa(id))
 			goto err_out;
 		if (ap->host->flags & ATA_HOST_IGNORE_ATA &&
@@ -2016,7 +2022,8 @@ retry:
 			goto retry;
 	}
 
-	if ((flags & ATA_READID_POSTRESET) && class == ATA_DEV_ATA) {
+	if ((flags & ATA_READID_POSTRESET) &&
+	    (class == ATA_DEV_ATA || class == ATA_DEV_ZAC)) {
 		/*
 		 * The exact sequence expected by certain pre-ATA4 drives is:
 		 * SRST RESET
@@ -2281,7 +2288,7 @@ int ata_dev_configure(struct ata_device *dev)
 			sizeof(modelbuf));
 
 	/* ATA-specific feature tests */
-	if (dev->class == ATA_DEV_ATA) {
+	if (dev->class == ATA_DEV_ATA || dev->class == ATA_DEV_ZAC) {
 		if (ata_id_is_cfa(id)) {
 			/* CPRM may make this media unusable */
 			if (id[ATA_ID_CFA_KEY_MGMT] & 1)
@@ -4034,6 +4041,7 @@ int ata_dev_revalidate(struct ata_device *dev, unsigned int new_class,
 	if (ata_class_enabled(new_class) &&
 	    new_class != ATA_DEV_ATA &&
 	    new_class != ATA_DEV_ATAPI &&
+	    new_class != ATA_DEV_ZAC &&
 	    new_class != ATA_DEV_SEMB) {
 		ata_dev_info(dev, "class mismatch %u != %u\n",
 			     dev->class, new_class);
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 6760fc4..f620c4a 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1809,6 +1809,7 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,
 
 	switch (qc->dev->class) {
 	case ATA_DEV_ATA:
+	case ATA_DEV_ZAC:
 		if (err & ATA_ICRC)
 			qc->err_mask |= AC_ERR_ATA_BUS;
 		if (err & ATA_UNC)
@@ -3791,7 +3792,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 				struct ata_eh_context *ehc = &link->eh_context;
 				unsigned long tmp;
 
-				if (dev->class != ATA_DEV_ATA)
+				if (dev->class != ATA_DEV_ATA &&
+				    dev->class != ATA_DEV_ZAC)
 					continue;
 				if (!(ehc->i.dev_action[dev->devno] &
 				      ATA_EH_PARK))
@@ -3872,7 +3874,8 @@ int ata_eh_recover(struct ata_port *ap, ata_prereset_fn_t prereset,
 
 		/* retry flush if necessary */
 		ata_for_each_dev(dev, link, ALL) {
-			if (dev->class != ATA_DEV_ATA)
+			if (dev->class != ATA_DEV_ATA &&
+			    dev->class != ATA_DEV_ZAC)
 				continue;
 			rc = ata_eh_maybe_retry_flush(dev);
 			if (rc)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0586f66..bea6e7f 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -235,7 +235,8 @@ static ssize_t ata_scsi_park_store(struct device *device,
 		rc = -ENODEV;
 		goto unlock;
 	}
-	if (dev->class != ATA_DEV_ATA) {
+	if (dev->class != ATA_DEV_ATA &&
+	    dev->class != ATA_DEV_ZAC) {
 		rc = -EOPNOTSUPP;
 		goto unlock;
 	}
@@ -3412,7 +3413,7 @@ static inline int __ata_scsi_queuecmd(struct scsi_cmnd *scmd,
 	ata_xlat_func_t xlat_func;
 	int rc = 0;
 
-	if (dev->class == ATA_DEV_ATA) {
+	if (dev->class == ATA_DEV_ATA || dev->class == ATA_DEV_ZAC) {
 		if (unlikely(!scmd->cmd_len || scmd->cmd_len > dev->cdb_len))
 			goto bad_cdb_len;
 
diff --git a/drivers/ata/libata-transport.c b/drivers/ata/libata-transport.c
index e37413228..3227b7c 100644
--- a/drivers/ata/libata-transport.c
+++ b/drivers/ata/libata-transport.c
@@ -143,6 +143,7 @@ static struct {
 	{ ATA_DEV_PMP_UNSUP,		"pmp" },
 	{ ATA_DEV_SEMB,			"semb" },
 	{ ATA_DEV_SEMB_UNSUP,		"semb" },
+	{ ATA_DEV_ZAC,			"zac" },
 	{ ATA_DEV_NONE,			"none" }
 };
 ata_bitfield_name_search(class, ata_class_names)
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 3412aee..37f482b 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -191,7 +191,8 @@ enum {
 	ATA_DEV_PMP_UNSUP	= 6,	/* SATA port multiplier (unsupported) */
 	ATA_DEV_SEMB		= 7,	/* SEMB */
 	ATA_DEV_SEMB_UNSUP	= 8,	/* SEMB (unsupported) */
-	ATA_DEV_NONE		= 9,	/* no device */
+	ATA_DEV_ZAC		= 9,	/* ZAC device */
+	ATA_DEV_NONE		= 10,	/* no device */
 
 	/* struct ata_link flags */
 	ATA_LFLAG_NO_HRST	= (1 << 1), /* avoid hardreset */
@@ -1490,7 +1491,8 @@ static inline unsigned int ata_tag_internal(unsigned int tag)
 static inline unsigned int ata_class_enabled(unsigned int class)
 {
 	return class == ATA_DEV_ATA || class == ATA_DEV_ATAPI ||
-		class == ATA_DEV_PMP || class == ATA_DEV_SEMB;
+		class == ATA_DEV_PMP || class == ATA_DEV_SEMB ||
+		class == ATA_DEV_ZAC;
 }
 
 static inline unsigned int ata_class_disabled(unsigned int class)
-- 
1.7.12.4

[PATCH 3/4] libata-scsi: Update SATL for ZAC drives

[Hannes Reinecke]

ZAC (zoned-access command) drives translate into ZBC (Zoned block
command) device type for SCSI. So implement the correct mappings
into libata-scsi and update the SCSI command set versions.

Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/ata/libata-scsi.c | 29 +++++++++++++++++++++++++++--
 1 file changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index bea6e7f..500b721 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1969,6 +1969,7 @@ static void ata_scsi_rbuf_fill(struct ata_scsi_args *args,
 static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
 {
 	const u8 versions[] = {
+		0x00,
 		0x60,	/* SAM-3 (no version claimed) */
 
 		0x03,
@@ -1977,6 +1978,20 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
 		0x02,
 		0x60	/* SPC-3 (no version claimed) */
 	};
+	const u8 versions_zbc[] = {
+		0x00,
+		0xA0,	/* SAM-5 (no version claimed) */
+
+		0x04,
+		0xC0,	/* SBC-3 (no version claimed) */
+
+		0x04,
+		0x60,	/* SPC-4 (no version claimed) */
+
+		0x60,
+		0x20,   /* ZBC (no version claimed) */
+	};
+
 	u8 hdr[] = {
 		TYPE_DISK,
 		0,
@@ -1991,6 +2006,11 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
 	if (ata_id_removeable(args->id))
 		hdr[1] |= (1 << 7);
 
+	if (args->dev->class == ATA_DEV_ZAC) {
+		hdr[0] = TYPE_ZBC;
+		hdr[2] = 0x6; /* ZBC is defined in SPC-4 */
+	}
+
 	memcpy(rbuf, hdr, sizeof(hdr));
 	memcpy(&rbuf[8], "ATA     ", 8);
 	ata_id_string(args->id, &rbuf[16], ATA_ID_PROD, 16);
@@ -2003,7 +2023,10 @@ static unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf)
 	if (rbuf[32] == 0 || rbuf[32] == ' ')
 		memcpy(&rbuf[32], "n/a ", 4);
 
-	memcpy(rbuf + 59, versions, sizeof(versions));
+	if (args->dev->class == ATA_DEV_ZAC)
+		memcpy(rbuf + 58, versions_zbc, sizeof(versions_zbc));
+	else
+		memcpy(rbuf + 58, versions, sizeof(versions));
 
 	return 0;
 }
@@ -2202,7 +2225,9 @@ static unsigned int ata_scsiop_inq_b1(struct ata_scsi_args *args, u8 *rbuf)
 	rbuf[4] = media_rotation_rate >> 8;
 	rbuf[5] = media_rotation_rate;
 	rbuf[7] = form_factor;
-
+	if (args->dev->class == ATA_DEV_ZAC)
+		/* SBC-4 HAW_ZBC bit */
+		rbuf[8] |= 0x10;
 	return 0;
 }
 
-- 
1.7.12.4

[PATCH 4/4] sd: Handle ZBC drives correctly

[Hannes Reinecke]

ZBC drives are close to disk devices, so update sd.c to handle
ZBC drives correctly.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/sd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 377a520..29c981b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -161,7 +161,7 @@ cache_type_store(struct device *dev, struct device_attribute *attr,
 	static const char temp[] = "temporary ";
 	int len;
 
-	if (sdp->type != TYPE_DISK)
+	if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
 		/* no cache control on RBC devices; theoretically they
 		 * can do it, but there's probably so many exceptions
 		 * it's not worth the risk */
@@ -259,7 +259,7 @@ allow_restart_store(struct device *dev, struct device_attribute *attr,
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
 
-	if (sdp->type != TYPE_DISK)
+	if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
 		return -EINVAL;
 
 	sdp->allow_restart = simple_strtoul(buf, NULL, 10);
@@ -389,7 +389,7 @@ provisioning_mode_store(struct device *dev, struct device_attribute *attr,
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
 
-	if (sdp->type != TYPE_DISK)
+	if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
 		return -EINVAL;
 
 	if (!strncmp(buf, lbp_mode[SD_LBP_UNMAP], 20))
@@ -456,7 +456,7 @@ max_write_same_blocks_store(struct device *dev, struct device_attribute *attr,
 	if (!capable(CAP_SYS_ADMIN))
 		return -EACCES;
 
-	if (sdp->type != TYPE_DISK)
+	if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
 		return -EINVAL;
 
 	err = kstrtoul(buf, 10, &max);
@@ -2537,7 +2537,7 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
 	struct scsi_mode_data data;
 	struct scsi_sense_hdr sshdr;
 
-	if (sdp->type != TYPE_DISK)
+	if (sdp->type != TYPE_DISK && sdp->type != TYPE_ZBC)
 		return;
 
 	if (sdkp->protection_type == 0)
-- 
1.7.12.4

Re: [PATCH 4/4] sd: Handle ZBC drives correctly

[Christoph Hellwig]

On Mon, Jul 21, 2014 at 10:27:21AM +0200, Hannes Reinecke wrote:
> ZBC drives are close to disk devices, so update sd.c to handle
> ZBC drives correctly.

While the other patches all look fine with me I strongly disagree with
this one.  Devices reporting the ZBC device type must have sequential
required zones, so they are _not_ anywhere close to disk devices,
and writes to them will cause errors that we aren't evenable able to
handle yet.

If it makes some sort of experiment easier for you we can add a way to
force ZBC devices to use sd.c conditionally.  Do you have a test device
that this works with?

Re: [PATCH 4/4] sd: Handle ZBC drives correctly

[Hannes Reinecke]

On 07/21/2014 01:29 PM, Christoph Hellwig wrote:
> On Mon, Jul 21, 2014 at 10:27:21AM +0200, Hannes Reinecke wrote:
>> ZBC drives are close to disk devices, so update sd.c to handle
>> ZBC drives correctly.
>
> While the other patches all look fine with me I strongly disagree with
> this one.  Devices reporting the ZBC device type must have sequential
> required zones, so they are _not_ anywhere close to disk devices,
> and writes to them will cause errors that we aren't evenable able to
> handle yet.
>
> If it makes some sort of experiment easier for you we can add a way to
> force ZBC devices to use sd.c conditionally.  Do you have a test device
> that this works with?
>
Yes, I have.

I was actually considering implementing a module option for sd
(eg attach_zbc) to allow zac/zbc devices to be attached to the sd 
driver.
Which will be off per default, so your concern should be addressed.

However, zbc devices _are_ similar to normal 'sd' devices; it's only 
that some commands might fail unexpectedly.
But all the usual commands from sbc are supported, so I found it a 
bit odd having to implement my own device driver (which would be a 
clone of 'sd' anyway).

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 4/4] sd: Handle ZBC drives correctly

[Christoph Hellwig]

On Mon, Jul 21, 2014 at 01:51:26PM +0200, Hannes Reinecke wrote:
> Yes, I have.

How well does the SCSI layer recover when you write outside the write
pointer?  ZBC defeinds new sense codes that we don't handle, so I'd
expect we'd get long loops of unrecovered errors.

> I was actually considering implementing a module option for sd
> (eg attach_zbc) to allow zac/zbc devices to be attached to the sd driver.
> Which will be off per default, so your concern should be addressed.

That sounds fine to.

> However, zbc devices _are_ similar to normal 'sd' devices; it's only that
> some commands might fail unexpectedly.
> But all the usual commands from sbc are supported, so I found it a bit odd
> having to implement my own device driver (which would be a clone of 'sd'
> anyway).

They are similar in some ways and very different in others.  I
defintively agree we should share as much code as possible, but I'm not
sure exposing them as plain sd devices is a good idea.  I'm defintively
fine with an optional attachment to sd to start playing around with
different ways to make use of them.

RE: [PATCH 4/4] sd: Handle ZBC drives correctly

[John Utz]

If I read Hanne's patch correctly, he is checking for the ZBC signature when the drives are probed.

Thus, the only circumstance where adding zbc handling would lead to unexpected bad behavior in sd would be if a ZBC drive reported itself as a standard ATA drive or an ATA drive reported itself as a ZBC drive.
________________________________________
From: linux-ide-owner@vger.kernel.org [linux-ide-owner@vger.kernel.org] on behalf of Hannes Reinecke [hare@suse.de]
Sent: Monday, July 21, 2014 4:51 AM
To: Christoph Hellwig
Cc: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org; James Bottomely
Subject: Re: [PATCH 4/4] sd: Handle ZBC drives correctly

On 07/21/2014 01:29 PM, Christoph Hellwig wrote:
> On Mon, Jul 21, 2014 at 10:27:21AM +0200, Hannes Reinecke wrote:
>> ZBC drives are close to disk devices, so update sd.c to handle
>> ZBC drives correctly.
>
> While the other patches all look fine with me I strongly disagree with
> this one.  Devices reporting the ZBC device type must have sequential
> required zones, so they are _not_ anywhere close to disk devices,
> and writes to them will cause errors that we aren't evenable able to
> handle yet.
>
> If it makes some sort of experiment easier for you we can add a way to
> force ZBC devices to use sd.c conditionally.  Do you have a test device
> that this works with?
>
Yes, I have.

I was actually considering implementing a module option for sd
(eg attach_zbc) to allow zac/zbc devices to be attached to the sd
driver.
Which will be off per default, so your concern should be addressed.

However, zbc devices _are_ similar to normal 'sd' devices; it's only
that some commands might fail unexpectedly.
But all the usual commands from sbc are supported, so I found it a
bit odd having to implement my own device driver (which would be a
clone of 'sd' anyway).

Cheers,

Hannes
--
Dr. Hannes Reinecke                   zSeries & Storage
hare@suse.de                          +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 4/4] sd: Handle ZBC drives correctly

[John Utz]

BAH! I get this ZBC/ZAC thing wrong still. mentally replace ATA with SCSI in my comments below....
________________________________________
From: linux-ide-owner@vger.kernel.org [linux-ide-owner@vger.kernel.org] on behalf of John Utz [John.Utz@wdc.com]
Sent: Monday, July 21, 2014 9:18 AM
To: Hannes Reinecke; Christoph Hellwig
Cc: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org; James Bottomely
Subject: RE: [PATCH 4/4] sd: Handle ZBC drives correctly

If I read Hanne's patch correctly, he is checking for the ZBC signature when the drives are probed.

Thus, the only circumstance where adding zbc handling would lead to unexpected bad behavior in sd would be if a ZBC drive reported itself as a standard ATA drive or an ATA drive reported itself as a ZBC drive.
________________________________________
From: linux-ide-owner@vger.kernel.org [linux-ide-owner@vger.kernel.org] on behalf of Hannes Reinecke [hare@suse.de]
Sent: Monday, July 21, 2014 4:51 AM
To: Christoph Hellwig
Cc: linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org; James Bottomely
Subject: Re: [PATCH 4/4] sd: Handle ZBC drives correctly

On 07/21/2014 01:29 PM, Christoph Hellwig wrote:
> On Mon, Jul 21, 2014 at 10:27:21AM +0200, Hannes Reinecke wrote:
>> ZBC drives are close to disk devices, so update sd.c to handle
>> ZBC drives correctly.
>
> While the other patches all look fine with me I strongly disagree with
> this one.  Devices reporting the ZBC device type must have sequential
> required zones, so they are _not_ anywhere close to disk devices,
> and writes to them will cause errors that we aren't evenable able to
> handle yet.
>
> If it makes some sort of experiment easier for you we can add a way to
> force ZBC devices to use sd.c conditionally.  Do you have a test device
> that this works with?
>
Yes, I have.

I was actually considering implementing a module option for sd
(eg attach_zbc) to allow zac/zbc devices to be attached to the sd
driver.
Which will be off per default, so your concern should be addressed.

However, zbc devices _are_ similar to normal 'sd' devices; it's only
that some commands might fail unexpectedly.
But all the usual commands from sbc are supported, so I found it a
bit odd having to implement my own device driver (which would be a
clone of 'sd' anyway).

Cheers,

Hannes
--
Dr. Hannes Reinecke                   zSeries & Storage
hare@suse.de                          +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] sd: Handle ZBC drives correctly

[Hannes Reinecke]

On 07/21/2014 06:18 PM, John Utz wrote:
> If I read Hanne's patch correctly, he is checking for the ZBC signature
 > when the drives are probed.
>
Yep.

> Thus, the only circumstance where adding zbc handling would lead to
 > unexpected bad behavior in sd would be if a ZBC drive reported itself
 > as a standard ATA drive or an ATA drive reported itself as a ZBC drive.

It's actually worse than that:
Currently libata will happily attach to a ZAC device, and passing them 
on to the SCSI stack as a normal disk device.
Which most definitely will lead to unexpected results :-)

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 4/4] sd: Handle ZBC drives correctly

[Christoph Hellwig]

Hi John,

I can't really comment much on the ATA side as I'm not overly uptodate
on the ZAC spec.

But a device with the 0x14 device type must have sequential required
zones, which will generate new sense codes when writes outside the
write pointer happens.  I'm very concerned about

 a) how the SCSI midlayer handles those sense codes

and

 b) how we can communicate them to the user of the device, e.g. the
    filesystem or user application

Just attaching the device by default without a good solution for these
seems dangerous to me.

RE: [PATCH 4/4] sd: Handle ZBC drives correctly

[John Utz]

(hand citing in outlook web access, sorry! i also took the liberty of reformatting your response a teeny bit.)
> ________________________________________
> From: Christoph Hellwig [hch@infradead.org]
> Sent: Tuesday, July 22, 2014 9:19 AM
> To: John Utz
> Cc: Hannes Reinecke; linux-ide@vger.kernel.org; linux-scsi@vger.kernel.org; James Bottomely
> Subject: Re: [PATCH 4/4] sd: Handle ZBC drives correctly
>
> Hi John,

Hi Christoph!

> I can't really comment much on the ATA side as I'm not overly uptodate
> on the ZAC spec.

It's OK, I have to chew on it because it's my day job, it's not reasonable to expect other folks to pay too much attention to it yet. :-)

I will say that it's helpful to consider it to be largely the same as the exiting ATA and SCSI DISK specs with a few extra new commands to make it adaptable to newer storage media designs.

>But a device with the 0x14 device type must have sequential required zones, 

Yes. 

> which will generate new sense codes when writes outside the write pointer happens.

Yes. and reads that will start or finish past the write pointer will also do something similar too.

> I'm very concerned about
>
>  a) how the SCSI midlayer handles those sense codes
>
> and
>
>  b) how we can communicate them to the user of the device, e.g. the filesystem or user application
>
> Just attaching the device by default without a good solution for these seems dangerous to me.

Your concerns are eminently practical and reasonable and you are not alone in possessing them.

But we gotta start somewhere. :-)

Actually, a couple of 'somewheres' and those 2 somewheres  map directly to your a and b concerns.

i am working on addressing 'b'.  i am using device-mapper to create a simulated ZAC target and this simulator will be used by hardworking file system vict.....um hardy pioneers.... to adapt their favorite file systems to do the right thing with respect to ZAC and ZBC.

This will not be a small job, and it will probably be easier for some file systems than others. 

Ultimately, i dont think that there will be much user space impact unless your userspace app is a database that manages its own disk access.

So let's talk about your concern 'a'. 

We are all blessed to be working at the cutting edge of linux.

IMHO, things like this are *supposed* to go into the tree here at the leading edge, because folks know not to be sticking bleeding edge kernels on to boxes that they run their lives and businesses on.

There have been many discussions/personal ruminations/guesses about how ZAC/ZBC is going to work and these things have progressed to the point where folks now have to check their hypotheticals into the tree so that we can all see what will happen.

So I would argue that this is actually a good thing because it's inspiring smart folks such as yourself to point out the snakepits that have jumped out at *you* when you saw Hannes's proposed checkin. :-)

Perhaps those of us who will be in Chicago in August who are interested in the topic should go find a hallway to hog up for a while and talk about this? :-)

just my us$0.02!

tnx!

johnu

Really,  alot of progress has been made since