remove long broken SCSI to NVMe translations

remove long broken SCSI to NVMe translations

[Christoph Hellwig]

TEST UNIT READY wa broken for a year and a half, and FORMAT UNIT and
WRITE BUFFER for two years.  Instead of fixing them up remove them - no
one should be sending SCSI commands to non-scsi devices anyway.

[PATCH 1/2] nvme-scsi: remove TEST UNIT READY emuation

[Christoph Hellwig]

This emulation has been broken for a year and a half, so remove it
instead of hacking around it.

Reported-by: Wen Xiong <wenxiong at linux.vnet.ibm.com>
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/scsi.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/drivers/nvme/host/scsi.c b/drivers/nvme/host/scsi.c
index 1f7671e631dd..aff3b36d93b6 100644
--- a/drivers/nvme/host/scsi.c
+++ b/drivers/nvme/host/scsi.c
@@ -2204,18 +2204,6 @@ static int nvme_trans_format_unit(struct nvme_ns *ns, struct sg_io_hdr *hdr,
 	return res;
 }
 
-static int nvme_trans_test_unit_ready(struct nvme_ns *ns,
-					struct sg_io_hdr *hdr,
-					u8 *cmd)
-{
-	if (nvme_ctrl_ready(ns->ctrl))
-		return nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					    NOT_READY, SCSI_ASC_LUN_NOT_READY,
-					    SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-	else
-		return nvme_trans_completion(hdr, SAM_STAT_GOOD, NO_SENSE, 0, 0);
-}
-
 static int nvme_trans_write_buffer(struct nvme_ns *ns, struct sg_io_hdr *hdr,
 							u8 *cmd)
 {
@@ -2411,9 +2399,6 @@ static int nvme_scsi_translate(struct nvme_ns *ns, struct sg_io_hdr *hdr)
 	case FORMAT_UNIT:
 		retcode = nvme_trans_format_unit(ns, hdr, cmd);
 		break;
-	case TEST_UNIT_READY:
-		retcode = nvme_trans_test_unit_ready(ns, hdr, cmd);
-		break;
 	case WRITE_BUFFER:
 		retcode = nvme_trans_write_buffer(ns, hdr, cmd);
 		break;
-- 
2.11.0

[PATCH 2/2] nvme-scsi: remove FORMAT UNIT and WRITE BUFFER emulations

[Christoph Hellwig]

These have been broken for 2 years and no one noticed until now.
Let's kill them off.

Reported-by: Joseph Szczypek <joseph.szczypek at hpe.com>
Signed-off-by: Christoph Hellwig <hch at lst.de>
---
 drivers/nvme/host/scsi.c | 302 -----------------------------------------------
 1 file changed, 302 deletions(-)

diff --git a/drivers/nvme/host/scsi.c b/drivers/nvme/host/scsi.c
index aff3b36d93b6..01158243556a 100644
--- a/drivers/nvme/host/scsi.c
+++ b/drivers/nvme/host/scsi.c
@@ -1219,48 +1219,6 @@ static void nvme_trans_fill_read_cap(u8 *response, struct nvme_id_ns *id_ns,
 	}
 }
 
-/* Start Stop Unit Helper Functions */
-
-static int nvme_trans_send_activate_fw_cmd(struct nvme_ns *ns, struct sg_io_hdr *hdr,
-					u8 buffer_id)
-{
-	struct nvme_command c;
-	int nvme_sc;
-
-	memset(&c, 0, sizeof(c));
-	c.common.opcode = nvme_admin_activate_fw;
-	c.common.cdw10[0] = cpu_to_le32(buffer_id | NVME_FWACT_REPL_ACTV);
-
-	nvme_sc = nvme_submit_sync_cmd(ns->queue, &c, NULL, 0);
-	return nvme_trans_status_code(hdr, nvme_sc);
-}
-
-static int nvme_trans_send_download_fw_cmd(struct nvme_ns *ns, struct sg_io_hdr *hdr,
-					u8 opcode, u32 tot_len, u32 offset,
-					u8 buffer_id)
-{
-	int nvme_sc;
-	struct nvme_command c;
-
-	if (hdr->iovec_count > 0) {
-		/* Assuming SGL is not allowed for this command */
-		return nvme_trans_completion(hdr,
-					SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST,
-					SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-	}
-
-	memset(&c, 0, sizeof(c));
-	c.common.opcode = nvme_admin_download_fw;
-	c.dlfw.numd = cpu_to_le32((tot_len/BYTES_TO_DWORDS) - 1);
-	c.dlfw.offset = cpu_to_le32(offset/BYTES_TO_DWORDS);
-
-	nvme_sc = nvme_submit_user_cmd(ns->ctrl->admin_q, &c,
-			hdr->dxferp, tot_len, NULL, 0);
-	return nvme_trans_status_code(hdr, nvme_sc);
-}
-
 /* Mode Select Helper Functions */
 
 static inline void nvme_trans_modesel_get_bd_len(u8 *parm_list, u8 cdb10,
@@ -1420,155 +1378,6 @@ static int nvme_trans_modesel_data(struct nvme_ns *ns, struct sg_io_hdr *hdr,
 	return res;
 }
 
-/* Format Unit Helper Functions */
-
-static int nvme_trans_fmt_set_blk_size_count(struct nvme_ns *ns,
-					     struct sg_io_hdr *hdr)
-{
-	int res = 0;
-	int nvme_sc;
-	u8 flbas;
-
-	/*
-	 * SCSI Expects a MODE SELECT would have been issued prior to
-	 * a FORMAT UNIT, and the block size and number would be used
-	 * from the block descriptor in it. If a MODE SELECT had not
-	 * been issued, FORMAT shall use the current values for both.
-	 */
-
-	if (ns->mode_select_num_blocks == 0 || ns->mode_select_block_len == 0) {
-		struct nvme_id_ns *id_ns;
-
-		nvme_sc = nvme_identify_ns(ns->ctrl, ns->ns_id, &id_ns);
-		res = nvme_trans_status_code(hdr, nvme_sc);
-		if (res)
-			return res;
-
-		if (ns->mode_select_num_blocks == 0)
-			ns->mode_select_num_blocks = le64_to_cpu(id_ns->ncap);
-		if (ns->mode_select_block_len == 0) {
-			flbas = (id_ns->flbas) & 0x0F;
-			ns->mode_select_block_len =
-						(1 << (id_ns->lbaf[flbas].ds));
-		}
-
-		kfree(id_ns);
-	}
-
-	return 0;
-}
-
-static int nvme_trans_fmt_get_parm_header(struct sg_io_hdr *hdr, u8 len,
-					u8 format_prot_info, u8 *nvme_pf_code)
-{
-	int res;
-	u8 *parm_list;
-	u8 pf_usage, pf_code;
-
-	parm_list = kmalloc(len, GFP_KERNEL);
-	if (parm_list == NULL) {
-		res = -ENOMEM;
-		goto out;
-	}
-	res = nvme_trans_copy_from_user(hdr, parm_list, len);
-	if (res)
-		goto out_mem;
-
-	if ((parm_list[FORMAT_UNIT_IMMED_OFFSET] &
-				FORMAT_UNIT_IMMED_MASK) != 0) {
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-		goto out_mem;
-	}
-
-	if (len == FORMAT_UNIT_LONG_PARM_LIST_LEN &&
-	    (parm_list[FORMAT_UNIT_PROT_INT_OFFSET] & 0x0F) != 0) {
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-		goto out_mem;
-	}
-	pf_usage = parm_list[FORMAT_UNIT_PROT_FIELD_USAGE_OFFSET] &
-			FORMAT_UNIT_PROT_FIELD_USAGE_MASK;
-	pf_code = (pf_usage << 2) | format_prot_info;
-	switch (pf_code) {
-	case 0:
-		*nvme_pf_code = 0;
-		break;
-	case 2:
-		*nvme_pf_code = 1;
-		break;
-	case 3:
-		*nvme_pf_code = 2;
-		break;
-	case 7:
-		*nvme_pf_code = 3;
-		break;
-	default:
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-		break;
-	}
-
- out_mem:
-	kfree(parm_list);
- out:
-	return res;
-}
-
-static int nvme_trans_fmt_send_cmd(struct nvme_ns *ns, struct sg_io_hdr *hdr,
-				   u8 prot_info)
-{
-	int res;
-	int nvme_sc;
-	struct nvme_id_ns *id_ns;
-	u8 i;
-	u8 nlbaf;
-	u8 selected_lbaf = 0xFF;
-	u32 cdw10 = 0;
-	struct nvme_command c;
-
-	/* Loop thru LBAF's in id_ns to match reqd lbaf, put in cdw10 */
-	nvme_sc = nvme_identify_ns(ns->ctrl, ns->ns_id, &id_ns);
-	res = nvme_trans_status_code(hdr, nvme_sc);
-	if (res)
-		return res;
-
-	nlbaf = id_ns->nlbaf;
-
-	for (i = 0; i < nlbaf; i++) {
-		if (ns->mode_select_block_len == (1 << (id_ns->lbaf[i].ds))) {
-			selected_lbaf = i;
-			break;
-		}
-	}
-	if (selected_lbaf > 0x0F) {
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-				ILLEGAL_REQUEST, SCSI_ASC_INVALID_PARAMETER,
-				SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-	}
-	if (ns->mode_select_num_blocks != le64_to_cpu(id_ns->ncap)) {
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-				ILLEGAL_REQUEST, SCSI_ASC_INVALID_PARAMETER,
-				SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-	}
-
-	cdw10 |= prot_info << 5;
-	cdw10 |= selected_lbaf & 0x0F;
-	memset(&c, 0, sizeof(c));
-	c.format.opcode = nvme_admin_format_nvm;
-	c.format.nsid = cpu_to_le32(ns->ns_id);
-	c.format.cdw10 = cpu_to_le32(cdw10);
-
-	nvme_sc = nvme_submit_sync_cmd(ns->ctrl->admin_q, &c, NULL, 0);
-	res = nvme_trans_status_code(hdr, nvme_sc);
-
-	kfree(id_ns);
-	return res;
-}
-
 static inline u32 nvme_trans_io_get_num_cmds(struct sg_io_hdr *hdr,
 					struct nvme_trans_io_cdb *cdb_info,
 					u32 max_blocks)
@@ -2152,111 +1961,6 @@ static int nvme_trans_synchronize_cache(struct nvme_ns *ns,
 	return nvme_trans_status_code(hdr, nvme_sc);
 }
 
-static int nvme_trans_format_unit(struct nvme_ns *ns, struct sg_io_hdr *hdr,
-							u8 *cmd)
-{
-	int res;
-	u8 parm_hdr_len = 0;
-	u8 nvme_pf_code = 0;
-	u8 format_prot_info, long_list, format_data;
-
-	format_prot_info = (cmd[1] & 0xc0) >> 6;
-	long_list = cmd[1] & 0x20;
-	format_data = cmd[1] & 0x10;
-
-	if (format_data != 0) {
-		if (format_prot_info != 0) {
-			if (long_list == 0)
-				parm_hdr_len = FORMAT_UNIT_SHORT_PARM_LIST_LEN;
-			else
-				parm_hdr_len = FORMAT_UNIT_LONG_PARM_LIST_LEN;
-		}
-	} else if (format_data == 0 && format_prot_info != 0) {
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-		goto out;
-	}
-
-	/* Get parm header from data-in/out buffer */
-	/*
-	 * According to the translation spec, the only fields in the parameter
-	 * list we are concerned with are in the header. So allocate only that.
-	 */
-	if (parm_hdr_len > 0) {
-		res = nvme_trans_fmt_get_parm_header(hdr, parm_hdr_len,
-					format_prot_info, &nvme_pf_code);
-		if (res)
-			goto out;
-	}
-
-	/* Attempt to activate any previously downloaded firmware image */
-	res = nvme_trans_send_activate_fw_cmd(ns, hdr, 0);
-
-	/* Determine Block size and count and send format command */
-	res = nvme_trans_fmt_set_blk_size_count(ns, hdr);
-	if (res)
-		goto out;
-
-	res = nvme_trans_fmt_send_cmd(ns, hdr, nvme_pf_code);
-
- out:
-	return res;
-}
-
-static int nvme_trans_write_buffer(struct nvme_ns *ns, struct sg_io_hdr *hdr,
-							u8 *cmd)
-{
-	int res = 0;
-	u32 buffer_offset, parm_list_length;
-	u8 buffer_id, mode;
-
-	parm_list_length = get_unaligned_be24(&cmd[6]);
-	if (parm_list_length % BYTES_TO_DWORDS != 0) {
-		/* NVMe expects Firmware file to be a whole number of DWORDS */
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-		goto out;
-	}
-	buffer_id = cmd[2];
-	if (buffer_id > NVME_MAX_FIRMWARE_SLOT) {
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-		goto out;
-	}
-	mode = cmd[1] & 0x1f;
-	buffer_offset = get_unaligned_be24(&cmd[3]);
-
-	switch (mode) {
-	case DOWNLOAD_SAVE_ACTIVATE:
-		res = nvme_trans_send_download_fw_cmd(ns, hdr, nvme_admin_download_fw,
-						parm_list_length, buffer_offset,
-						buffer_id);
-		if (res)
-			goto out;
-		res = nvme_trans_send_activate_fw_cmd(ns, hdr, buffer_id);
-		break;
-	case DOWNLOAD_SAVE_DEFER_ACTIVATE:
-		res = nvme_trans_send_download_fw_cmd(ns, hdr, nvme_admin_download_fw,
-						parm_list_length, buffer_offset,
-						buffer_id);
-		break;
-	case ACTIVATE_DEFERRED_MICROCODE:
-		res = nvme_trans_send_activate_fw_cmd(ns, hdr, buffer_id);
-		break;
-	default:
-		res = nvme_trans_completion(hdr, SAM_STAT_CHECK_CONDITION,
-					ILLEGAL_REQUEST, SCSI_ASC_INVALID_CDB,
-					SCSI_ASCQ_CAUSE_NOT_REPORTABLE);
-		break;
-	}
-
- out:
-	return res;
-}
-
 struct scsi_unmap_blk_desc {
 	__be64	slba;
 	__be32	nlb;
@@ -2396,12 +2100,6 @@ static int nvme_scsi_translate(struct nvme_ns *ns, struct sg_io_hdr *hdr)
 	case SYNCHRONIZE_CACHE:
 		retcode = nvme_trans_synchronize_cache(ns, hdr);
 		break;
-	case FORMAT_UNIT:
-		retcode = nvme_trans_format_unit(ns, hdr, cmd);
-		break;
-	case WRITE_BUFFER:
-		retcode = nvme_trans_write_buffer(ns, hdr, cmd);
-		break;
 	case UNMAP:
 		retcode = nvme_trans_unmap(ns, hdr, cmd);
 		break;
-- 
2.11.0

remove long broken SCSI to NVMe translations

[Keith Busch]

On Wed, Jun 14, 2017@08:35:35AM +0200, Christoph Hellwig wrote:
> TEST UNIT READY wa broken for a year and a half, and FORMAT UNIT and
> WRITE BUFFER for two years.  Instead of fixing them up remove them - no
> one should be sending SCSI commands to non-scsi devices anyway.

This is a step in the right direction, but I don't think it goes far
enough. Can we remove the whole thing now?

remove long broken SCSI to NVMe translations

[Christoph Hellwig]

On Wed, Jun 14, 2017@11:00:25AM -0400, Keith Busch wrote:
> On Wed, Jun 14, 2017@08:35:35AM +0200, Christoph Hellwig wrote:
> > TEST UNIT READY wa broken for a year and a half, and FORMAT UNIT and
> > WRITE BUFFER for two years.  Instead of fixing them up remove them - no
> > one should be sending SCSI commands to non-scsi devices anyway.
> 
> This is a step in the right direction, but I don't think it goes far
> enough. Can we remove the whole thing now?

I'd love to.  The only real stumbling block was that some version of
SLES used it for persistent devices names.  So I'd like to sync with
Hannes and Johannes to make sure they've fixed this up and upstream
kernels without the scsi translation will work just fine on SLES.

remove long broken SCSI to NVMe translations

[Martin K. Petersen]

Keith,

>> TEST UNIT READY wa broken for a year and a half, and FORMAT UNIT and
>> WRITE BUFFER for two years.  Instead of fixing them up remove them - no
>> one should be sending SCSI commands to non-scsi devices anyway.
>
> This is a step in the right direction, but I don't think it goes far
> enough. Can we remove the whole thing now?

I agree. Let's just get rid of the whole thing.

-- 
Martin K. Petersen	Oracle Linux Engineering

remove long broken SCSI to NVMe translations

[Hannes Reinecke]

On 06/14/2017 05:01 PM, Christoph Hellwig wrote:
> On Wed, Jun 14, 2017@11:00:25AM -0400, Keith Busch wrote:
>> On Wed, Jun 14, 2017@08:35:35AM +0200, Christoph Hellwig wrote:
>>> TEST UNIT READY wa broken for a year and a half, and FORMAT UNIT and
>>> WRITE BUFFER for two years.  Instead of fixing them up remove them - no
>>> one should be sending SCSI commands to non-scsi devices anyway.
>>
>> This is a step in the right direction, but I don't think it goes far
>> enough. Can we remove the whole thing now?
> 
> I'd love to.  The only real stumbling block was that some version of
> SLES used it for persistent devices names.  So I'd like to sync with
> Hannes and Johannes to make sure they've fixed this up and upstream
> kernels without the scsi translation will work just fine on SLES.
> 
We actually only had an issue with early (ie v1.0 ?) devices, which
wouldn't have any WWID. And we didn't have a 'wwid' sysfs attribute to
boot, making it hard for us to print out any wwid without the SCSI
translation layer.

However, this has been fixed now, so I'm game with dropping it.

Someone should tell HPe, though; apparently they are using it for
firmware download ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare at 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)

remove long broken SCSI to NVMe translations

[Christoph Hellwig]

[you dropped all the people in the To and Cc list..]

> Someone should tell HPe, though; apparently they are using it for
> firmware download ...

Everything that could be done using the scsi emulation can be trivially
done using nvme-cli, as they should have done from day 0.

But adding the usual suspects to CC as they didn't seem to know it.
And adding Robert just for fun :)

remove long broken SCSI to NVMe translations

[Linda Knippers]

On 06/16/2017 04:31 AM, Christoph Hellwig wrote:
> [you dropped all the people in the To and Cc list..]
> 
>> Someone should tell HPe, though; apparently they are using it for
>> firmware download ...

Yes, that's how we discovered it was broken.

> Everything that could be done using the scsi emulation can be trivially
> done using nvme-cli, as they should have done from day 0.

I don't quite know the history of our use of the scsi translation but I
suspect it's because we adapted some existing utility to also support nvme.

David Milburn's patch is very simple.  Can we please fix the bug for now
so we have a little time to fix our tools?

Thanks,

-- ljk
> 
> But adding the usual suspects to CC as they didn't seem to know it.
> And adding Robert just for fun :)
> 
> _______________________________________________
> Linux-nvme mailing list
> Linux-nvme at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-nvme
> 

remove long broken SCSI to NVMe translations

[Christoph Hellwig]

On Fri, Jun 16, 2017@09:57:16AM -0400, Linda Knippers wrote:
> > Everything that could be done using the scsi emulation can be trivially
> > done using nvme-cli, as they should have done from day 0.
> 
> I don't quite know the history of our use of the scsi translation but I
> suspect it's because we adapted some existing utility to also support nvme.
> 
> David Milburn's patch is very simple.  Can we please fix the bug for now
> so we have a little time to fix our tools?

I don't think I want to expand usage of it.  The code only "worked"
for about two years, during which not too many NVMe products even
existed.  And I say "worked" because I found countless of sever bugs
(use after free, memory overruns, etc) issues when I first ran a SCS
I testsuite over it, which obviously didn't even cover the commands
you care about.

You might have a slightly better leaverage with distros already
shipping this code, but enabling it upstream does not seem in any
way helpful.

remove long broken SCSI to NVMe translations

[Keith Busch]

On Fri, Jun 16, 2017@09:57:16AM -0400, Linda Knippers wrote:
> David Milburn's patch is very simple.  Can we please fix the bug for now
> so we have a little time to fix our tools?

But if we applied this one, we'd implicitly concede this ill conceived
component is being maintained, and we'd forever have to apply "one more"
patch when we're trying to get rid of the need.

remove long broken SCSI to NVMe translations

[Micah Parrish]

On 06/16/2017 02:31 AM, Christoph Hellwig wrote:
> [you dropped all the people in the To and Cc list..]
>
>> Someone should tell HPe, though; apparently they are using it for
>> firmware download ...
> Everything that could be done using the scsi emulation can be trivially
> done using nvme-cli, as they should have done from day 0.
>
> But adding the usual suspects to CC as they didn't seem to know it.
> And adding Robert just for fun :)

I'm relatively new at nvme so please bear with me as I try to articulate 
this.

 From HPE's perspective, we don't want NVME-SCSI translation removed 
from Linux.  We do drive firmware updates and other management functions 
using a common cross platform management API which works on VMWare, 
Windows and Linux, and it relies on SCSI on all 3.  This software 
(called SOULAPI) is common across most of our drives and enclosures, so 
putting in a special case for nvme on linux would be difficult.  In the 
short term we'd have to wrap nvme-cli, which would introduce a new 
dependency.  Long term maybe we could rewrite our API to use native NVME 
commands on all supported OSes.

The reason the scsi translation has been broken so long is that we don't 
test firmware updates with upstream kernels.  From our perspective it 
broke in the last enterprise update cycle, but we didn't discover it 
because we didn't have a firmware update for these drives.  There are 
some process improvements we could make there to catch this kind of bug 
sooner, and I will be discussing them internally.

-Micah Parrish

remove long broken SCSI to NVMe translations

[Johannes Thumshirn]

On Thu, Jun 22, 2017@12:31:55PM -0600, Micah Parrish wrote:
> From HPE's perspective, we don't want NVME-SCSI translation removed from
> Linux.  We do drive firmware updates and other management functions using a
> common cross platform management API which works on VMWare, Windows and
> Linux, and it relies on SCSI on all 3.  This software (called SOULAPI) is
> common across most of our drives and enclosures, so putting in a special
> case for nvme on linux would be difficult.  In the short term we'd have to
> wrap nvme-cli, which would introduce a new dependency.  Long term maybe we
> could rewrite our API to use native NVME commands on all supported OSes.
> 
> The reason the scsi translation has been broken so long is that we don't
> test firmware updates with upstream kernels.  From our perspective it broke
> in the last enterprise update cycle, but we didn't discover it because we
> didn't have a firmware update for these drives.  There are some process
> improvements we could make there to catch this kind of bug sooner, and I
> will be discussing them internally.

There's no need to wrap nvme-cli, the firmware download litterally is the
following:

int nvme_download_firmware(int fd, uint32_t offset, uint32_t len, void *data,
			   uint32_t slot)
{
	struct nvme_admin_cmd cmd = {
		.opcode = nvme_admin_download_fw,
		.addr = (uint64_t)data,
		.data_len = len,
		.cdw10 = (len >> 2) - 1,
		.cdw11 = offset >> 2,
	};

	ioctl(fd, NVME_IOCTL_ADMIN_CMD, &cmd);

	struct nvme_admin_cmd cmd = {
		.opcode = nvme_admin_activate_fw,
		.cd10 = (action << 3) | slot; 
	};

	ioctl(fd, NVME_IOCTL_ADMIN_CMD, &cmd);
}

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

remove long broken SCSI to NVMe translations

[Judy Brock]

Adding some input fr Dell that didn't make it out to this reflector:

From: managementif@nvmexpress.org [mailto:managementif@nvmexpress.org] On Behalf Of Austin.Bolen@dell.com

	>>> The SNTL in Linux has been deprecated and disabled by default for almost 1?  years now.

Has it made it to all the distros?  I can check on distros like RHEL to see if/when they disabled it but any given OS will be used by customers for many years and so I think the majority of our customers are still running with the translation layer enabled and will only find out if things are broken when the migrate to a distro that has it disabled.
. . .
I checked on our most recent distros and looks like everything we ship today has the SNTL enabled:

-          RHEL 7.3:            Does not have the CONFIG_BLK_DEV_NVME_SCSI option at all so looks like SNTL is always enabled.
-          SLES 12 SP1:       Does not have the CONFIG_BLK_DEV_NVME_SCSI option at all so looks like SNTL is always enabled.
-          Ubuntu 16.04:   Has CONFIG_BLK_DEV_NVME_SCSI=y so looks like so looks like SNTL is enabled by default.

Thanks,
Austin


-----Original Message-----
From: Linux-nvme [mailto:linux-nvme-bounces@lists.infradead.org] On Behalf Of Johannes Thumshirn
Sent: Thursday, June 22, 2017 12:21 PM
To: Micah Parrish
Cc: Joseph Szczypek; Sagi Grimberg; linux-nvme at lists.infradead.org; Christoph Hellwig; David Milburn; Hannes Reinecke; keith.busch at intel.com; Robert Elliott
Subject: Re: remove long broken SCSI to NVMe translations

On Thu, Jun 22, 2017@12:31:55PM -0600, Micah Parrish wrote:
> From HPE's perspective, we don't want NVME-SCSI translation removed 
> from Linux.  We do drive firmware updates and other management 
> functions using a common cross platform management API which works on 
> VMWare, Windows and Linux, and it relies on SCSI on all 3.  This 
> software (called SOULAPI) is common across most of our drives and 
> enclosures, so putting in a special case for nvme on linux would be 
> difficult.  In the short term we'd have to wrap nvme-cli, which would 
> introduce a new dependency.  Long term maybe we could rewrite our API to use native NVME commands on all supported OSes.
> 
> The reason the scsi translation has been broken so long is that we 
> don't test firmware updates with upstream kernels.  From our 
> perspective it broke in the last enterprise update cycle, but we 
> didn't discover it because we didn't have a firmware update for these 
> drives.  There are some process improvements we could make there to 
> catch this kind of bug sooner, and I will be discussing them internally.

There's no need to wrap nvme-cli, the firmware download litterally is the
following:

int nvme_download_firmware(int fd, uint32_t offset, uint32_t len, void *data,
			   uint32_t slot)
{
	struct nvme_admin_cmd cmd = {
		.opcode = nvme_admin_download_fw,
		.addr = (uint64_t)data,
		.data_len = len,
		.cdw10 = (len >> 2) - 1,
		.cdw11 = offset >> 2,
	};

	ioctl(fd, NVME_IOCTL_ADMIN_CMD, &cmd);

	struct nvme_admin_cmd cmd = {
		.opcode = nvme_admin_activate_fw,
		.cd10 = (action << 3) | slot; 
	};

	ioctl(fd, NVME_IOCTL_ADMIN_CMD, &cmd);
}

-- 
Johannes Thumshirn                                          Storage
jthumshirn at suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 N?rnberg
GF: Felix Imend?rffer, Jane Smithard, Graham Norton HRB 21284 (AG N?rnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

_______________________________________________
Linux-nvme mailing list
Linux-nvme at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme