TCG Opal support for libata

TCG Opal support for libata

[Christoph Hellwig]

Hi all,

this series adds support for using our new generic TCG OPAL code with
SATA disks, and as side effect for SCSI disks (although so far it doesn't
seem like none of those actually exist).

[PATCH 1/6] libata: move ata_read_log_page to libata-core.c

[Christoph Hellwig]

It is core functionality, and only one of the users is in the EH code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-core.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 drivers/ata/libata-eh.c   | 64 -----------------------------------------------
 drivers/ata/libata.h      |  4 +--
 3 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 2d83b8c75965..d4bab5052268 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2047,6 +2047,70 @@ int ata_dev_read_id(struct ata_device *dev, unsigned int *p_class,
 	return rc;
 }
 
+/**
+ *	ata_read_log_page - read a specific log page
+ *	@dev: target device
+ *	@log: log to read
+ *	@page: page to read
+ *	@buf: buffer to store read page
+ *	@sectors: number of sectors to read
+ *
+ *	Read log page using READ_LOG_EXT command.
+ *
+ *	LOCKING:
+ *	Kernel thread context (may sleep).
+ *
+ *	RETURNS:
+ *	0 on success, AC_ERR_* mask otherwise.
+ */
+unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
+			       u8 page, void *buf, unsigned int sectors)
+{
+	unsigned long ap_flags = dev->link->ap->flags;
+	struct ata_taskfile tf;
+	unsigned int err_mask;
+	bool dma = false;
+
+	DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page);
+
+	/*
+	 * Return error without actually issuing the command on controllers
+	 * which e.g. lockup on a read log page.
+	 */
+	if (ap_flags & ATA_FLAG_NO_LOG_PAGE)
+		return AC_ERR_DEV;
+
+retry:
+	ata_tf_init(dev, &tf);
+	if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) &&
+	    !(dev->horkage & ATA_HORKAGE_NO_NCQ_LOG)) {
+		tf.command = ATA_CMD_READ_LOG_DMA_EXT;
+		tf.protocol = ATA_PROT_DMA;
+		dma = true;
+	} else {
+		tf.command = ATA_CMD_READ_LOG_EXT;
+		tf.protocol = ATA_PROT_PIO;
+		dma = false;
+	}
+	tf.lbal = log;
+	tf.lbam = page;
+	tf.nsect = sectors;
+	tf.hob_nsect = sectors >> 8;
+	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_LBA48 | ATA_TFLAG_DEVICE;
+
+	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE,
+				     buf, sectors * ATA_SECT_SIZE, 0);
+
+	if (err_mask && dma) {
+		dev->horkage |= ATA_HORKAGE_NO_NCQ_LOG;
+		ata_dev_warn(dev, "READ LOG DMA EXT failed, trying unqueued\n");
+		goto retry;
+	}
+
+	DPRINTK("EXIT, err_mask=%x\n", err_mask);
+	return err_mask;
+}
+
 static int ata_do_link_spd_horkage(struct ata_device *dev)
 {
 	struct ata_link *plink = ata_dev_phys_link(dev);
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index ef68232b5222..528a4e1b2af3 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1488,70 +1488,6 @@ static const char *ata_err_string(unsigned int err_mask)
 }
 
 /**
- *	ata_read_log_page - read a specific log page
- *	@dev: target device
- *	@log: log to read
- *	@page: page to read
- *	@buf: buffer to store read page
- *	@sectors: number of sectors to read
- *
- *	Read log page using READ_LOG_EXT command.
- *
- *	LOCKING:
- *	Kernel thread context (may sleep).
- *
- *	RETURNS:
- *	0 on success, AC_ERR_* mask otherwise.
- */
-unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
-			       u8 page, void *buf, unsigned int sectors)
-{
-	unsigned long ap_flags = dev->link->ap->flags;
-	struct ata_taskfile tf;
-	unsigned int err_mask;
-	bool dma = false;
-
-	DPRINTK("read log page - log 0x%x, page 0x%x\n", log, page);
-
-	/*
-	 * Return error without actually issuing the command on controllers
-	 * which e.g. lockup on a read log page.
-	 */
-	if (ap_flags & ATA_FLAG_NO_LOG_PAGE)
-		return AC_ERR_DEV;
-
-retry:
-	ata_tf_init(dev, &tf);
-	if (dev->dma_mode && ata_id_has_read_log_dma_ext(dev->id) &&
-	    !(dev->horkage & ATA_HORKAGE_NO_NCQ_LOG)) {
-		tf.command = ATA_CMD_READ_LOG_DMA_EXT;
-		tf.protocol = ATA_PROT_DMA;
-		dma = true;
-	} else {
-		tf.command = ATA_CMD_READ_LOG_EXT;
-		tf.protocol = ATA_PROT_PIO;
-		dma = false;
-	}
-	tf.lbal = log;
-	tf.lbam = page;
-	tf.nsect = sectors;
-	tf.hob_nsect = sectors >> 8;
-	tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_LBA48 | ATA_TFLAG_DEVICE;
-
-	err_mask = ata_exec_internal(dev, &tf, NULL, DMA_FROM_DEVICE,
-				     buf, sectors * ATA_SECT_SIZE, 0);
-
-	if (err_mask && dma) {
-		dev->horkage |= ATA_HORKAGE_NO_NCQ_LOG;
-		ata_dev_warn(dev, "READ LOG DMA EXT failed, trying unqueued\n");
-		goto retry;
-	}
-
-	DPRINTK("EXIT, err_mask=%x\n", err_mask);
-	return err_mask;
-}
-
-/**
  *	ata_eh_read_log_10h - Read log page 10h for NCQ error details
  *	@dev: Device to read log page 10h from
  *	@tag: Resulting tag of the failed command
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 120fce0befd3..84569d563ae2 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -98,6 +98,8 @@ extern struct ata_port *ata_port_alloc(struct ata_host *host);
 extern const char *sata_spd_string(unsigned int spd);
 extern int ata_port_probe(struct ata_port *ap);
 extern void __ata_port_probe(struct ata_port *ap);
+extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
+				      u8 page, void *buf, unsigned int sectors);
 
 #define to_ata_port(d) container_of(d, struct ata_port, tdev)
 
@@ -160,8 +162,6 @@ extern void ata_eh_about_to_do(struct ata_link *link, struct ata_device *dev,
 			       unsigned int action);
 extern void ata_eh_done(struct ata_link *link, struct ata_device *dev,
 			unsigned int action);
-extern unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
-				      u8 page, void *buf, unsigned int sectors);
 extern void ata_eh_autopsy(struct ata_port *ap);
 const char *ata_get_cmd_descript(u8 command);
 extern void ata_eh_report(struct ata_port *ap);
-- 
2.11.0

Re: [PATCH 1/6] libata: move ata_read_log_page to libata-core.c

[Hannes Reinecke]

On 06/04/2017 02:42 PM, Christoph Hellwig wrote:
> It is core functionality, and only one of the users is in the EH code.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ata/libata-core.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/ata/libata-eh.c   | 64 -----------------------------------------------
>  drivers/ata/libata.h      |  4 +--
>  3 files changed, 66 insertions(+), 66 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 2/6] libata: factor out a ata_log_supported helper

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-core.c | 59 +++++++++++++----------------------------------
 1 file changed, 16 insertions(+), 43 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index d4bab5052268..0672733997bb 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2111,6 +2111,15 @@ unsigned int ata_read_log_page(struct ata_device *dev, u8 log,
 	return err_mask;
 }
 
+static bool ata_log_supported(struct ata_device *dev, u8 log)
+{
+	struct ata_port *ap = dev->link->ap;
+
+	if (ata_read_log_page(dev, ATA_LOG_DIRECTORY, 0, ap->sector_buf, 1))
+		return false;
+	return get_unaligned_le16(&ap->sector_buf[log * 2]) ? true : false;
+}
+
 static int ata_do_link_spd_horkage(struct ata_device *dev)
 {
 	struct ata_link *plink = ata_dev_phys_link(dev);
@@ -2158,21 +2167,9 @@ static void ata_dev_config_ncq_send_recv(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
 	unsigned int err_mask;
-	int log_index = ATA_LOG_NCQ_SEND_RECV * 2;
-	u16 log_pages;
 
-	err_mask = ata_read_log_page(dev, ATA_LOG_DIRECTORY,
-				     0, ap->sector_buf, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get Log Directory Emask 0x%x\n",
-			    err_mask);
-		return;
-	}
-	log_pages = get_unaligned_le16(&ap->sector_buf[log_index]);
-	if (!log_pages) {
-		ata_dev_warn(dev,
-			     "NCQ Send/Recv Log not supported\n");
+	if (!ata_log_supported(dev, ATA_LOG_NCQ_SEND_RECV)) {
+		ata_dev_warn(dev, "NCQ Send/Recv Log not supported\n");
 		return;
 	}
 	err_mask = ata_read_log_page(dev, ATA_LOG_NCQ_SEND_RECV,
@@ -2199,19 +2196,8 @@ static void ata_dev_config_ncq_non_data(struct ata_device *dev)
 {
 	struct ata_port *ap = dev->link->ap;
 	unsigned int err_mask;
-	int log_index = ATA_LOG_NCQ_NON_DATA * 2;
-	u16 log_pages;
 
-	err_mask = ata_read_log_page(dev, ATA_LOG_DIRECTORY,
-				     0, ap->sector_buf, 1);
-	if (err_mask) {
-		ata_dev_dbg(dev,
-			    "failed to get Log Directory Emask 0x%x\n",
-			    err_mask);
-		return;
-	}
-	log_pages = get_unaligned_le16(&ap->sector_buf[log_index]);
-	if (!log_pages) {
+	if (!ata_log_supported(dev, ATA_LOG_NCQ_NON_DATA)) {
 		ata_dev_warn(dev,
 			     "NCQ Send/Recv Log not supported\n");
 		return;
@@ -2339,7 +2325,7 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	struct ata_port *ap = dev->link->ap;
 	unsigned int err_mask;
 	u8 *identify_buf = ap->sector_buf;
-	int log_index = ATA_LOG_SATA_ID_DEV_DATA * 2, i, found = 0;
+	int i, found = 0;
 	u16 log_pages;
 
 	dev->zac_zones_optimal_open = U32_MAX;
@@ -2360,24 +2346,11 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	if (!(dev->flags & ATA_DFLAG_ZAC))
 		return;
 
-	/*
-	 * Read Log Directory to figure out if IDENTIFY DEVICE log
-	 * is supported.
-	 */
-	err_mask = ata_read_log_page(dev, ATA_LOG_DIRECTORY,
-				     0, ap->sector_buf, 1);
-	if (err_mask) {
-		ata_dev_info(dev,
-			     "failed to get Log Directory Emask 0x%x\n",
-			     err_mask);
-		return;
-	}
-	log_pages = get_unaligned_le16(&ap->sector_buf[log_index]);
-	if (log_pages == 0) {
-		ata_dev_warn(dev,
-			     "ATA Identify Device Log not supported\n");
+	if (!ata_log_supported(dev, ATA_LOG_SATA_ID_DEV_DATA)) {
+		ata_dev_warn(dev, "ATA Identify Device Log not supported\n");
 		return;
 	}
+
 	/*
 	 * Read IDENTIFY DEVICE data log, page 0, to figure out
 	 * if page 9 is supported.
-- 
2.11.0

Re: [PATCH 2/6] libata: factor out a ata_log_supported helper

[Hannes Reinecke]

On 06/04/2017 02:42 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ata/libata-core.c | 59 +++++++++++++----------------------------------
>  1 file changed, 16 insertions(+), 43 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 3/6] libata: clarify log page naming / grouping

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-core.c | 10 +++++-----
 include/linux/ata.h       | 10 +++++++---
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 0672733997bb..445e7050637b 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2226,7 +2226,7 @@ static void ata_dev_config_ncq_prio(struct ata_device *dev)
 	}
 
 	err_mask = ata_read_log_page(dev,
-				     ATA_LOG_SATA_ID_DEV_DATA,
+				     ATA_LOG_IDENTIFY_DEVICE,
 				     ATA_LOG_SATA_SETTINGS,
 				     ap->sector_buf,
 				     1);
@@ -2346,7 +2346,7 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	if (!(dev->flags & ATA_DFLAG_ZAC))
 		return;
 
-	if (!ata_log_supported(dev, ATA_LOG_SATA_ID_DEV_DATA)) {
+	if (!ata_log_supported(dev, ATA_LOG_IDENTIFY_DEVICE)) {
 		ata_dev_warn(dev, "ATA Identify Device Log not supported\n");
 		return;
 	}
@@ -2355,7 +2355,7 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	 * Read IDENTIFY DEVICE data log, page 0, to figure out
 	 * if page 9 is supported.
 	 */
-	err_mask = ata_read_log_page(dev, ATA_LOG_SATA_ID_DEV_DATA, 0,
+	err_mask = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, 0,
 				     identify_buf, 1);
 	if (err_mask) {
 		ata_dev_info(dev,
@@ -2379,7 +2379,7 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	/*
 	 * Read IDENTIFY DEVICE data log, page 9 (Zoned-device information)
 	 */
-	err_mask = ata_read_log_page(dev, ATA_LOG_SATA_ID_DEV_DATA,
+	err_mask = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE,
 				     ATA_LOG_ZONED_INFORMATION,
 				     identify_buf, 1);
 	if (!err_mask) {
@@ -2608,7 +2608,7 @@ int ata_dev_configure(struct ata_device *dev)
 
 			dev->flags |= ATA_DFLAG_DEVSLP;
 			err_mask = ata_read_log_page(dev,
-						     ATA_LOG_SATA_ID_DEV_DATA,
+						     ATA_LOG_IDENTIFY_DEVICE,
 						     ATA_LOG_SATA_SETTINGS,
 						     sata_setting,
 						     1);
diff --git a/include/linux/ata.h b/include/linux/ata.h
index ad7d9ee89ff0..44de34c954d8 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -336,11 +336,15 @@ enum {
 	/* READ_LOG_EXT pages */
 	ATA_LOG_DIRECTORY	= 0x0,
 	ATA_LOG_SATA_NCQ	= 0x10,
-	ATA_LOG_NCQ_NON_DATA	  = 0x12,
-	ATA_LOG_NCQ_SEND_RECV	  = 0x13,
-	ATA_LOG_SATA_ID_DEV_DATA  = 0x30,
+	ATA_LOG_NCQ_NON_DATA	= 0x12,
+	ATA_LOG_NCQ_SEND_RECV	= 0x13,
+	ATA_LOG_IDENTIFY_DEVICE	= 0x30,
+
+	/* Identify device log pages: */
 	ATA_LOG_SATA_SETTINGS	  = 0x08,
 	ATA_LOG_ZONED_INFORMATION = 0x09,
+
+	/* Identify device SATA settings log:*/
 	ATA_LOG_DEVSLP_OFFSET	  = 0x30,
 	ATA_LOG_DEVSLP_SIZE	  = 0x08,
 	ATA_LOG_DEVSLP_MDAT	  = 0x00,
-- 
2.11.0

Re: [PATCH 3/6] libata: clarify log page naming / grouping

[Hannes Reinecke]

On 06/04/2017 02:42 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ata/libata-core.c | 10 +++++-----
>  include/linux/ata.h       | 10 +++++++---
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 4/6] libata: factor out a ata_identify_page_supported helper

[Christoph Hellwig]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-core.c | 59 +++++++++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 27 deletions(-)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 445e7050637b..f57131115594 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2120,6 +2120,37 @@ static bool ata_log_supported(struct ata_device *dev, u8 log)
 	return get_unaligned_le16(&ap->sector_buf[log * 2]) ? true : false;
 }
 
+static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
+{
+	struct ata_port *ap = dev->link->ap;
+	unsigned int err, i;
+
+	if (!ata_log_supported(dev, ATA_LOG_IDENTIFY_DEVICE)) {
+		ata_dev_warn(dev, "ATA Identify Device Log not supported\n");
+		return false;
+	}
+
+	/*
+	 * Read IDENTIFY DEVICE data log, page 0, to figure out if the page is
+	 * supported.
+	 */
+	err = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, 0, ap->sector_buf,
+			1);
+	if (err) {
+		ata_dev_info(dev,
+			     "failed to get Device Identify Log Emask 0x%x\n",
+			     err);
+		return false;
+	}
+
+	for (i = 0; i < ap->sector_buf[8]; i++) {
+		if (ap->sector_buf[9 + i] == page)
+			return true;
+	}
+
+	return false;
+}
+
 static int ata_do_link_spd_horkage(struct ata_device *dev)
 {
 	struct ata_link *plink = ata_dev_phys_link(dev);
@@ -2325,8 +2356,6 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	struct ata_port *ap = dev->link->ap;
 	unsigned int err_mask;
 	u8 *identify_buf = ap->sector_buf;
-	int i, found = 0;
-	u16 log_pages;
 
 	dev->zac_zones_optimal_open = U32_MAX;
 	dev->zac_zones_optimal_nonseq = U32_MAX;
@@ -2346,31 +2375,7 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	if (!(dev->flags & ATA_DFLAG_ZAC))
 		return;
 
-	if (!ata_log_supported(dev, ATA_LOG_IDENTIFY_DEVICE)) {
-		ata_dev_warn(dev, "ATA Identify Device Log not supported\n");
-		return;
-	}
-
-	/*
-	 * Read IDENTIFY DEVICE data log, page 0, to figure out
-	 * if page 9 is supported.
-	 */
-	err_mask = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, 0,
-				     identify_buf, 1);
-	if (err_mask) {
-		ata_dev_info(dev,
-			     "failed to get Device Identify Log Emask 0x%x\n",
-			     err_mask);
-		return;
-	}
-	log_pages = identify_buf[8];
-	for (i = 0; i < log_pages; i++) {
-		if (identify_buf[9 + i] == ATA_LOG_ZONED_INFORMATION) {
-			found++;
-			break;
-		}
-	}
-	if (!found) {
+	if (!ata_identify_page_supported(dev, ATA_LOG_ZONED_INFORMATION)) {
 		ata_dev_warn(dev,
 			     "ATA Zoned Information Log not supported\n");
 		return;
-- 
2.11.0

Re: [PATCH 4/6] libata: factor out a ata_identify_page_supported helper

[Sergei Shtylyov]

Hello!

On 06/04/2017 03:42 PM, Christoph Hellwig wrote:

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ata/libata-core.c | 59 +++++++++++++++++++++++++----------------------
>  1 file changed, 32 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 445e7050637b..f57131115594 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2120,6 +2120,37 @@ static bool ata_log_supported(struct ata_device *dev, u8 log)
>  	return get_unaligned_le16(&ap->sector_buf[log * 2]) ? true : false;
>  }
>
> +static bool ata_identify_page_supported(struct ata_device *dev, u8 page)
> +{
> +	struct ata_port *ap = dev->link->ap;
> +	unsigned int err, i;
> +
> +	if (!ata_log_supported(dev, ATA_LOG_IDENTIFY_DEVICE)) {
> +		ata_dev_warn(dev, "ATA Identify Device Log not supported\n");
> +		return false;
> +	}
> +
> +	/*
> +	 * Read IDENTIFY DEVICE data log, page 0, to figure out if the page is
> +	 * supported.
> +	 */
> +	err = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, 0, ap->sector_buf,
> +			1);
> +	if (err) {
> +		ata_dev_info(dev,
> +			     "failed to get Device Identify Log Emask 0x%x\n",
> +			     err);

    Your line continuation style is spomewhat inconsistent: 2 tabs above and 
tabs/spaces here...

[...]

MBR, Sergei

Re: [PATCH 4/6] libata: factor out a ata_identify_page_supported helper

[Hannes Reinecke]

On 06/04/2017 02:42 PM, Christoph Hellwig wrote:
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ata/libata-core.c | 59 +++++++++++++++++++++++++----------------------
>  1 file changed, 32 insertions(+), 27 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 5/6] libata: implement SECURITY PROTOCOL IN/OUT

[Christoph Hellwig]

This allows us to use the generic OPAL code with ATA devices.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-core.c | 32 ++++++++++++++++++++
 drivers/ata/libata-scsi.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/ata.h       |  1 +
 include/linux/libata.h    |  1 +
 4 files changed, 110 insertions(+)

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f57131115594..6eb08595a1b5 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -2405,6 +2405,37 @@ static void ata_dev_config_zac(struct ata_device *dev)
 	}
 }
 
+static void ata_dev_config_trusted(struct ata_device *dev)
+{
+	struct ata_port *ap = dev->link->ap;
+	u64 trusted_cap;
+	unsigned int err;
+
+	if (!ata_identify_page_supported(dev, ATA_LOG_SECURITY)) {
+		ata_dev_warn(dev,
+			     "Security Log not supported\n");
+		return;
+	}
+
+	err = ata_read_log_page(dev, ATA_LOG_IDENTIFY_DEVICE, ATA_LOG_SECURITY,
+			ap->sector_buf, 1);
+	if (err) {
+		ata_dev_dbg(dev,
+			    "failed to read Security Log, Emask 0x%x\n", err);
+		return;
+	}
+
+	trusted_cap = get_unaligned_le64(&ap->sector_buf[40]);
+	if (!(trusted_cap & (1ULL << 63))) {
+		ata_dev_dbg(dev,
+			    "Trusted Computing capability qword not valid!\n");
+		return;
+	}
+
+	if (trusted_cap & (1 << 0))
+		dev->flags |= ATA_DFLAG_TRUSTED;
+}
+
 /**
  *	ata_dev_configure - Configure the specified ATA/ATAPI device
  *	@dev: Target device to configure
@@ -2629,6 +2660,7 @@ int ata_dev_configure(struct ata_device *dev)
 		}
 		ata_dev_config_sense_reporting(dev);
 		ata_dev_config_zac(dev);
+		ata_dev_config_trusted(dev);
 		dev->cdb_len = 16;
 	}
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 49ba9834c715..3d28f2bd79af 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3563,6 +3563,11 @@ static unsigned int ata_scsiop_maint_in(struct ata_scsi_args *args, u8 *rbuf)
 		    dev->class == ATA_DEV_ZAC)
 			supported = 3;
 		break;
+	case SECURITY_PROTOCOL_IN:
+	case SECURITY_PROTOCOL_OUT:
+		if (dev->flags & ATA_DFLAG_TRUSTED)
+			supported = 3;
+		break;
 	default:
 		break;
 	}
@@ -4067,6 +4072,71 @@ static unsigned int ata_scsi_mode_select_xlat(struct ata_queued_cmd *qc)
 	return 1;
 }
 
+static u8 ata_scsi_trusted_op(u32 len, bool send, bool dma)
+{
+	if (len == 0)
+		return ATA_CMD_TRUSTED_NONDATA;
+	else if (send)
+		return dma ? ATA_CMD_TRUSTED_SND_DMA : ATA_CMD_TRUSTED_SND;
+	else
+		return dma ? ATA_CMD_TRUSTED_RCV_DMA : ATA_CMD_TRUSTED_RCV;
+}
+
+static unsigned int ata_scsi_security_inout_xlat(struct ata_queued_cmd *qc)
+{
+	struct scsi_cmnd *scmd = qc->scsicmd;
+	const u8 *cdb = scmd->cmnd;
+	struct ata_taskfile *tf = &qc->tf;
+	u8 secp = cdb[1];
+	bool send = (cdb[0] == SECURITY_PROTOCOL_OUT);
+	u16 spsp = get_unaligned_be16(&cdb[2]);
+	u32 len = get_unaligned_be32(&cdb[6]);
+	bool dma = !(qc->dev->flags & ATA_DFLAG_PIO);
+
+	/*
+	 * We don't support the ATA "security" protocol.
+	 */
+	if (secp == 0xef) {
+		ata_scsi_set_invalid_field(qc->dev, scmd, 1, 0);
+		return 1;
+	}
+
+	if (cdb[4] & 7) { /* INC_512 */
+		if (len > 0xffff) {
+			ata_scsi_set_invalid_field(qc->dev, scmd, 6, 0);
+			return 1;
+		}
+	} else {
+		if (len > 0x01fffe00) {
+			ata_scsi_set_invalid_field(qc->dev, scmd, 6, 0);
+			return 1;
+		}
+
+		/* convert to the sector-based ATA addressing */
+		len = (len + 511) / 512;
+	}
+
+	tf->protocol = dma ? ATA_PROT_DMA : ATA_PROT_PIO;
+	tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR | ATA_TFLAG_LBA;
+	if (send)
+		tf->flags |= ATA_TFLAG_WRITE;
+	tf->command = ata_scsi_trusted_op(len, send, dma);
+	tf->feature = secp;
+	tf->lbam = spsp & 0xff;
+	tf->lbah = spsp >> 8;
+
+	if (len) {
+		tf->nsect = len & 0xff;
+		tf->lbal = len >> 8;
+	} else {
+		if (!send)
+			tf->lbah = (1 << 7);
+	}
+
+	ata_qc_set_pc_nbytes(qc);
+	return 0;
+}
+
 /**
  *	ata_get_xlat_func - check if SCSI to ATA translation is possible
  *	@dev: ATA device
@@ -4118,6 +4188,12 @@ static inline ata_xlat_func_t ata_get_xlat_func(struct ata_device *dev, u8 cmd)
 	case ZBC_OUT:
 		return ata_scsi_zbc_out_xlat;
 
+	case SECURITY_PROTOCOL_IN:
+	case SECURITY_PROTOCOL_OUT:
+		if (!(dev->flags & ATA_DFLAG_TRUSTED))
+			break;
+		return ata_scsi_security_inout_xlat;
+
 	case START_STOP:
 		return ata_scsi_start_stop_xlat;
 	}
diff --git a/include/linux/ata.h b/include/linux/ata.h
index 44de34c954d8..5781bf310560 100644
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -341,6 +341,7 @@ enum {
 	ATA_LOG_IDENTIFY_DEVICE	= 0x30,
 
 	/* Identify device log pages: */
+	ATA_LOG_SECURITY	  = 0x06,
 	ATA_LOG_SATA_SETTINGS	  = 0x08,
 	ATA_LOG_ZONED_INFORMATION = 0x09,
 
diff --git a/include/linux/libata.h b/include/linux/libata.h
index c9a69fc8821e..65711cb3a740 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -156,6 +156,7 @@ enum {
 	ATA_DFLAG_ACPI_PENDING	= (1 << 5), /* ACPI resume action pending */
 	ATA_DFLAG_ACPI_FAILED	= (1 << 6), /* ACPI on devcfg has failed */
 	ATA_DFLAG_AN		= (1 << 7), /* AN configured */
+	ATA_DFLAG_TRUSTED	= (1 << 8), /* device supports trusted send/recv */
 	ATA_DFLAG_DMADIR	= (1 << 10), /* device requires DMADIR */
 	ATA_DFLAG_CFG_MASK	= (1 << 12) - 1,
 
-- 
2.11.0

Re: [PATCH 5/6] libata: implement SECURITY PROTOCOL IN/OUT

[Hannes Reinecke]

On 06/04/2017 02:42 PM, Christoph Hellwig wrote:
> This allows us to use the generic OPAL code with ATA devices.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/ata/libata-core.c | 32 ++++++++++++++++++++
>  drivers/ata/libata-scsi.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/ata.h       |  1 +
>  include/linux/libata.h    |  1 +
>  4 files changed, 110 insertions(+)
> Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Christoph Hellwig]

Just wire up the generic TCG OPAL infrastructure to the SCSI disk driver
and the Security In/Out commands.

Note that I don't know of any actual SCSI disks that do support TCG OPAL,
but this is required to support ATA disks through libata.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/sd.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 drivers/scsi/sd.h |  2 ++
 2 files changed, 46 insertions(+)

diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index b6bb4e0ce0e3..782f909a223c 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -50,6 +50,7 @@
 #include <linux/string_helpers.h>
 #include <linux/async.h>
 #include <linux/slab.h>
+#include <linux/sed-opal.h>
 #include <linux/pm_runtime.h>
 #include <linux/pr.h>
 #include <linux/t10-pi.h>
@@ -643,6 +644,26 @@ static void scsi_disk_put(struct scsi_disk *sdkp)
 	mutex_unlock(&sd_ref_mutex);
 }
 
+#ifdef CONFIG_BLK_SED_OPAL
+static int sd_sec_submit(void *data, u16 spsp, u8 secp, void *buffer,
+		size_t len, bool send)
+{
+	struct scsi_device *sdev = data;
+	u8 cdb[12] = { 0, };
+	int ret;
+
+	cdb[0] = send ? SECURITY_PROTOCOL_OUT : SECURITY_PROTOCOL_IN;
+	cdb[1] = secp;
+	put_unaligned_be16(spsp, &cdb[2]);
+	put_unaligned_be32(len, &cdb[6]);
+
+	ret = scsi_execute_req(sdev, cdb,
+			send ? DMA_TO_DEVICE : DMA_FROM_DEVICE,
+			buffer, len, NULL, SD_TIMEOUT, SD_MAX_RETRIES, NULL);
+	return ret <= 0 ? ret : -EIO;
+}
+#endif /* CONFIG_BLK_SED_OPAL */
+
 static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
 					   unsigned int dix, unsigned int dif)
 {
@@ -1454,6 +1475,9 @@ static int sd_ioctl(struct block_device *bdev, fmode_t mode,
 	if (error)
 		goto out;
 
+	if (is_sed_ioctl(cmd))
+		return sed_ioctl(sdkp->opal_dev, cmd, p);
+
 	/*
 	 * Send SCSI addressing ioctls directly to mid level, send other
 	 * ioctls to block level and then onto mid level if they can't be
@@ -3014,6 +3038,17 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
 		sdkp->ws10 = 1;
 }
 
+static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
+{
+	struct scsi_device *sdev = sdkp->device;
+
+	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE,
+			SECURITY_PROTOCOL_IN) == 1 &&
+	    scsi_report_opcode(sdev, buffer, SD_BUF_SIZE,
+			SECURITY_PROTOCOL_OUT) == 1)
+		sdkp->security = 1;
+}
+
 /**
  *	sd_revalidate_disk - called the first time a new disk is seen,
  *	performs disk spin up, read_capacity, etc.
@@ -3067,6 +3102,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
 		sd_read_cache_type(sdkp, buffer);
 		sd_read_app_tag_own(sdkp, buffer);
 		sd_read_write_same(sdkp, buffer);
+		sd_read_security(sdkp, buffer);
 	}
 
 	sdkp->first_scan = 0;
@@ -3227,6 +3263,12 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 
 	sd_revalidate_disk(gd);
 
+	if (sdkp->security) {
+		sdkp->opal_dev = init_opal_dev(sdp, &sd_sec_submit);
+		if (sdkp->opal_dev)
+			sd_printk(KERN_NOTICE, sdkp, "supports TCG Opal\n");
+	}
+
 	sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
 		  sdp->removable ? "removable " : "");
 	scsi_autopm_put_device(sdp);
@@ -3376,6 +3418,8 @@ static int sd_remove(struct device *dev)
 
 	sd_zbc_remove(sdkp);
 
+	free_opal_dev(sdkp->opal_dev);
+
 	blk_register_region(devt, SD_MINORS, NULL,
 			    sd_default_probe, NULL, NULL);
 
diff --git a/drivers/scsi/sd.h b/drivers/scsi/sd.h
index 61d02efd366c..99c4dde9b6bf 100644
--- a/drivers/scsi/sd.h
+++ b/drivers/scsi/sd.h
@@ -71,6 +71,7 @@ struct scsi_disk {
 	struct scsi_device *device;
 	struct device	dev;
 	struct gendisk	*disk;
+	struct opal_dev *opal_dev;
 #ifdef CONFIG_BLK_DEV_ZONED
 	unsigned int	nr_zones;
 	unsigned int	zone_blocks;
@@ -114,6 +115,7 @@ struct scsi_disk {
 	unsigned	rc_basis: 2;
 	unsigned	zoned: 2;
 	unsigned	urswrz : 1;
+	unsigned	security : 1;
 	unsigned	ignore_medium_access_errors : 1;
 };
 #define to_scsi_disk(obj) container_of(obj,struct scsi_disk,dev)
-- 
2.11.0

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Scott Bauer]

On Sun, Jun 04, 2017 at 02:42:25PM +0200, Christoph Hellwig wrote:
> Just wire up the generic TCG OPAL infrastructure to the SCSI disk driver
> and the Security In/Out commands.
> 
> Note that I don't know of any actual SCSI disks that do support TCG OPAL,
> but this is required to support ATA disks through libata.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/sd.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/sd.h |  2 ++
>  2 files changed, 46 insertions(+)
> 
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index b6bb4e0ce0e3..782f909a223c 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -50,6 +50,7 @@
>  #include <linux/string_helpers.h>
>  #include <linux/async.h>
>  #include <linux/slab.h>
> +#include <linux/sed-opal.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/pr.h>
>  #include <linux/t10-pi.h>
> @@ -643,6 +644,26 @@ static void scsi_disk_put(struct scsi_disk *sdkp)
>  	mutex_unlock(&sd_ref_mutex);
>  }
>  
> +#ifdef CONFIG_BLK_SED_OPAL
> +static int sd_sec_submit(void *data, u16 spsp, u8 secp, void *buffer,
> +		size_t len, bool send)
> +{
> +	struct scsi_device *sdev = data;
> +	u8 cdb[12] = { 0, };
> +	int ret;
> +
> +	cdb[0] = send ? SECURITY_PROTOCOL_OUT : SECURITY_PROTOCOL_IN;
> +	cdb[1] = secp;
> +	put_unaligned_be16(spsp, &cdb[2]);
> +	put_unaligned_be32(len, &cdb[6]);
> +
> +	ret = scsi_execute_req(sdev, cdb,
> +			send ? DMA_TO_DEVICE : DMA_FROM_DEVICE,
> +			buffer, len, NULL, SD_TIMEOUT, SD_MAX_RETRIES, NULL);
> +	return ret <= 0 ? ret : -EIO;
> +}
> +#endif /* CONFIG_BLK_SED_OPAL */
> +
>  static unsigned char sd_setup_protect_cmnd(struct scsi_cmnd *scmd,
>  					   unsigned int dix, unsigned int dif)
>  {
> @@ -1454,6 +1475,9 @@ static int sd_ioctl(struct block_device *bdev, fmode_t mode,
>  	if (error)
>  		goto out;
>  
> +	if (is_sed_ioctl(cmd))
> +		return sed_ioctl(sdkp->opal_dev, cmd, p);
> +
>  	/*
>  	 * Send SCSI addressing ioctls directly to mid level, send other
>  	 * ioctls to block level and then onto mid level if they can't be
> @@ -3014,6 +3038,17 @@ static void sd_read_write_same(struct scsi_disk *sdkp, unsigned char *buffer)
>  		sdkp->ws10 = 1;
>  }
>  
> +static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
> +{
> +	struct scsi_device *sdev = sdkp->device;
> +
> +	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE,
> +			SECURITY_PROTOCOL_IN) == 1 &&
> +	    scsi_report_opcode(sdev, buffer, SD_BUF_SIZE,
> +			SECURITY_PROTOCOL_OUT) == 1)
> +		sdkp->security = 1;
> +}
> +
>  /**
>   *	sd_revalidate_disk - called the first time a new disk is seen,
>   *	performs disk spin up, read_capacity, etc.
> @@ -3067,6 +3102,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>  		sd_read_cache_type(sdkp, buffer);
>  		sd_read_app_tag_own(sdkp, buffer);
>  		sd_read_write_same(sdkp, buffer);
> +		sd_read_security(sdkp, buffer);
>  	}
>  
>  	sdkp->first_scan = 0;
> @@ -3227,6 +3263,12 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
>  
>  	sd_revalidate_disk(gd);
>  
> +	if (sdkp->security) {
> +		sdkp->opal_dev = init_opal_dev(sdp, &sd_sec_submit);
> +		if (sdkp->opal_dev)
> +			sd_printk(KERN_NOTICE, sdkp, "supports TCG Opal\n");
> +	}
> +
> 
I'm not familiar at all with ATA, but I noticed there was no unlock from suspend support
in the series. Does ATA not have a way to determine if we're coming out of a suspend?

I see there are some power-ops in scsi/sd.c, if you do want to add it you can mabe toss a

if (sdkp->security)
   opal_unlock_from_suspend(sdpk->opal_dev)

somewhere in the resume path? We handle null opal_devs and no unlock from suspend list
so calling it when nothing is set up is just a no-op.

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Christoph Hellwig]

On Mon, Jun 05, 2017 at 03:15:31PM -0600, Scott Bauer wrote:
> I'm not familiar at all with ATA, but I noticed there was no unlock from suspend support
> in the series. Does ATA not have a way to determine if we're coming out of a suspend?

I don't know, and not having a test system with a OPAL capable driver
and suspend support I could not even test the code.

> I see there are some power-ops in scsi/sd.c, if you do want to add it you can mabe toss a
> 
> if (sdkp->security)
>    opal_unlock_from_suspend(sdpk->opal_dev)
> 
> somewhere in the resume path? We handle null opal_devs and no unlock from suspend list
> so calling it when nothing is set up is just a no-op.

Yeah, maybe.  We'll just need someone who could test it first.

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Scott Bauer]

On Tue, Jun 06, 2017 at 11:59:55AM +0200, Christoph Hellwig wrote:
> On Mon, Jun 05, 2017 at 03:15:31PM -0600, Scott Bauer wrote:
> > I'm not familiar at all with ATA, but I noticed there was no unlock from suspend support
> > in the series. Does ATA not have a way to determine if we're coming out of a suspend?
> 
> I don't know, and not having a test system with a OPAL capable driver
> and suspend support I could not even test the code.
> 
> > I see there are some power-ops in scsi/sd.c, if you do want to add it you can mabe toss a
> > 
> > if (sdkp->security)
> >    opal_unlock_from_suspend(sdpk->opal_dev)
> > 
> > somewhere in the resume path? We handle null opal_devs and no unlock from suspend list
> > so calling it when nothing is set up is just a no-op.
> 
> Yeah, maybe.  We'll just need someone who could test it first.

I was given a sata drive that apparently has opal enabled on it. If it actually has opal
I can run some tests.

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Martin K. Petersen]

Christoph,

> +static void sd_read_security(struct scsi_disk *sdkp, unsigned char *buffer)
> +{
> +	struct scsi_device *sdev = sdkp->device;
> +
> +	if (scsi_report_opcode(sdev, buffer, SD_BUF_SIZE,
> +			SECURITY_PROTOCOL_IN) == 1 &&
> +	    scsi_report_opcode(sdev, buffer, SD_BUF_SIZE,
> +			SECURITY_PROTOCOL_OUT) == 1)
> +		sdkp->security = 1;
> +}
> +
>  /**
>   *	sd_revalidate_disk - called the first time a new disk is seen,
>   *	performs disk spin up, read_capacity, etc.
> @@ -3067,6 +3102,7 @@ static int sd_revalidate_disk(struct gendisk *disk)
>  		sd_read_cache_type(sdkp, buffer);
>  		sd_read_app_tag_own(sdkp, buffer);
>  		sd_read_write_same(sdkp, buffer);
> +		sd_read_security(sdkp, buffer);
>  	}

For WRITE SAME, scsi_report_opcode() is gated not only by
sdev->no_report_opcodes but by sdev->no_write_same.

I'm concerned about firing off REPORT OPCODES to random devices without
a sufficiently good heuristic. Doesn't look like SAT has anything to
offer in this department, though. Maybe it's time to consider a
vendor-specific Linux VPD page...

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Christoph Hellwig]

On Mon, Jun 05, 2017 at 08:48:00PM -0400, Martin K. Petersen wrote:
> For WRITE SAME, scsi_report_opcode() is gated not only by
> sdev->no_report_opcodes but by sdev->no_write_same.
> 
> I'm concerned about firing off REPORT OPCODES to random devices without
> a sufficiently good heuristic. Doesn't look like SAT has anything to
> offer in this department, though. Maybe it's time to consider a
> vendor-specific Linux VPD page...

Eww.  Given that as far as I can tell only ATA devices implement
OPAL we could key it off that for now.  But that's only going to
defer the problem until support for other security protocols comes
along for real SCSI devices.

But as we already set no_report_opcodes for all usb-storage and
quirked uas devices I think the worst offenders are already covered
anyway.

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Christoph Hellwig]

On Tue, Jun 06, 2017 at 11:58:02AM +0200, Christoph Hellwig wrote:
> On Mon, Jun 05, 2017 at 08:48:00PM -0400, Martin K. Petersen wrote:
> > For WRITE SAME, scsi_report_opcode() is gated not only by
> > sdev->no_report_opcodes but by sdev->no_write_same.
> > 
> > I'm concerned about firing off REPORT OPCODES to random devices without
> > a sufficiently good heuristic. Doesn't look like SAT has anything to
> > offer in this department, though. Maybe it's time to consider a
> > vendor-specific Linux VPD page...
> 
> Eww.  Given that as far as I can tell only ATA devices implement
> OPAL we could key it off that for now.  But that's only going to
> defer the problem until support for other security protocols comes
> along for real SCSI devices.
> 
> But as we already set no_report_opcodes for all usb-storage and
> quirked uas devices I think the worst offenders are already covered
> anyway.

Martin, how do we want to move ahead on this patch?

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Martin K. Petersen]

Christoph,

>> But as we already set no_report_opcodes for all usb-storage and
>> quirked uas devices I think the worst offenders are already covered
>> anyway.
>
> Martin, how do we want to move ahead on this patch?

I was suggesting the VPD because that may be easier for the SAS HBA
vendors to accommodate for SATA passthrough. But we can cross that
bridge when we get to it.

For libata I'm fine with keying off a supports_opal:1 flag in
scsi_device or something to that effect. I'd just like to reduce the
risk of introducing more RSOC regressions.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 6/6] sd: add support for TCG OPAL self encrypting disks

[Hannes Reinecke]

On 06/04/2017 02:42 PM, Christoph Hellwig wrote:
> Just wire up the generic TCG OPAL infrastructure to the SCSI disk driver
> and the Security In/Out commands.
> 
> Note that I don't know of any actual SCSI disks that do support TCG OPAL,
> but this is required to support ATA disks through libata.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/sd.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
>  drivers/scsi/sd.h |  2 ++
>  2 files changed, 46 insertions(+)
> 
Reviewed-by: Hannes Reinecke <hare@suse.com>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: TCG Opal support for libata

[Tejun Heo]

On Sun, Jun 04, 2017 at 02:42:19PM +0200, Christoph Hellwig wrote:
> Hi all,
> 
> this series adds support for using our new generic TCG OPAL code with
> SATA disks, and as side effect for SCSI disks (although so far it doesn't
> seem like none of those actually exist).

Applied 1-5 to libata/for-4.13.  Updated 4 for line continuation style
consistency as pointed out by Sergei.

Thanks.

-- 
tejun