* remove long broken SCSI to NVMe translations
@ 2017-06-14 6:35 Christoph Hellwig
2017-06-14 6:35 ` [PATCH 1/2] nvme-scsi: remove TEST UNIT READY emuation Christoph Hellwig
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Christoph Hellwig @ 2017-06-14 6:35 UTC (permalink / raw)
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.
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/2] nvme-scsi: remove TEST UNIT READY emuation 2017-06-14 6:35 remove long broken SCSI to NVMe translations Christoph Hellwig @ 2017-06-14 6:35 ` Christoph Hellwig 2017-06-14 6:35 ` [PATCH 2/2] nvme-scsi: remove FORMAT UNIT and WRITE BUFFER emulations Christoph Hellwig 2017-06-14 15:00 ` remove long broken SCSI to NVMe translations Keith Busch 2 siblings, 0 replies; 14+ messages in thread From: Christoph Hellwig @ 2017-06-14 6:35 UTC (permalink / raw) 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/2] nvme-scsi: remove FORMAT UNIT and WRITE BUFFER emulations 2017-06-14 6:35 remove long broken SCSI to NVMe translations Christoph Hellwig 2017-06-14 6:35 ` [PATCH 1/2] nvme-scsi: remove TEST UNIT READY emuation Christoph Hellwig @ 2017-06-14 6:35 ` Christoph Hellwig 2017-06-14 15:00 ` remove long broken SCSI to NVMe translations Keith Busch 2 siblings, 0 replies; 14+ messages in thread From: Christoph Hellwig @ 2017-06-14 6:35 UTC (permalink / raw) 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-14 6:35 remove long broken SCSI to NVMe translations Christoph Hellwig 2017-06-14 6:35 ` [PATCH 1/2] nvme-scsi: remove TEST UNIT READY emuation Christoph Hellwig 2017-06-14 6:35 ` [PATCH 2/2] nvme-scsi: remove FORMAT UNIT and WRITE BUFFER emulations Christoph Hellwig @ 2017-06-14 15:00 ` Keith Busch 2017-06-14 15:01 ` Christoph Hellwig 2017-06-14 15:14 ` Martin K. Petersen 2 siblings, 2 replies; 14+ messages in thread From: Keith Busch @ 2017-06-14 15:00 UTC (permalink / raw) 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? ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-14 15:00 ` remove long broken SCSI to NVMe translations Keith Busch @ 2017-06-14 15:01 ` Christoph Hellwig 2017-06-16 6:11 ` Hannes Reinecke 2017-06-14 15:14 ` Martin K. Petersen 1 sibling, 1 reply; 14+ messages in thread From: Christoph Hellwig @ 2017-06-14 15:01 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-14 15:01 ` Christoph Hellwig @ 2017-06-16 6:11 ` Hannes Reinecke 2017-06-16 8:31 ` Christoph Hellwig 0 siblings, 1 reply; 14+ messages in thread From: Hannes Reinecke @ 2017-06-16 6:11 UTC (permalink / raw) 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) ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-16 6:11 ` Hannes Reinecke @ 2017-06-16 8:31 ` Christoph Hellwig 2017-06-16 13:57 ` Linda Knippers 2017-06-22 18:31 ` Micah Parrish 0 siblings, 2 replies; 14+ messages in thread From: Christoph Hellwig @ 2017-06-16 8:31 UTC (permalink / raw) [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 :) ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-16 8:31 ` Christoph Hellwig @ 2017-06-16 13:57 ` Linda Knippers 2017-06-20 6:48 ` Christoph Hellwig 2017-06-20 14:27 ` Keith Busch 2017-06-22 18:31 ` Micah Parrish 1 sibling, 2 replies; 14+ messages in thread From: Linda Knippers @ 2017-06-16 13:57 UTC (permalink / raw) 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-16 13:57 ` Linda Knippers @ 2017-06-20 6:48 ` Christoph Hellwig 2017-06-20 14:27 ` Keith Busch 1 sibling, 0 replies; 14+ messages in thread From: Christoph Hellwig @ 2017-06-20 6:48 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-16 13:57 ` Linda Knippers 2017-06-20 6:48 ` Christoph Hellwig @ 2017-06-20 14:27 ` Keith Busch 1 sibling, 0 replies; 14+ messages in thread From: Keith Busch @ 2017-06-20 14:27 UTC (permalink / raw) 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-16 8:31 ` Christoph Hellwig 2017-06-16 13:57 ` Linda Knippers @ 2017-06-22 18:31 ` Micah Parrish 2017-06-22 19:21 ` Johannes Thumshirn 1 sibling, 1 reply; 14+ messages in thread From: Micah Parrish @ 2017-06-22 18:31 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-22 18:31 ` Micah Parrish @ 2017-06-22 19:21 ` Johannes Thumshirn 2017-06-30 13:07 ` Judy Brock 0 siblings, 1 reply; 14+ messages in thread From: Johannes Thumshirn @ 2017-06-22 19:21 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-22 19:21 ` Johannes Thumshirn @ 2017-06-30 13:07 ` Judy Brock 0 siblings, 0 replies; 14+ messages in thread From: Judy Brock @ 2017-06-30 13:07 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
* remove long broken SCSI to NVMe translations 2017-06-14 15:00 ` remove long broken SCSI to NVMe translations Keith Busch 2017-06-14 15:01 ` Christoph Hellwig @ 2017-06-14 15:14 ` Martin K. Petersen 1 sibling, 0 replies; 14+ messages in thread From: Martin K. Petersen @ 2017-06-14 15:14 UTC (permalink / raw) 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2017-06-30 13:07 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-06-14 6:35 remove long broken SCSI to NVMe translations Christoph Hellwig 2017-06-14 6:35 ` [PATCH 1/2] nvme-scsi: remove TEST UNIT READY emuation Christoph Hellwig 2017-06-14 6:35 ` [PATCH 2/2] nvme-scsi: remove FORMAT UNIT and WRITE BUFFER emulations Christoph Hellwig 2017-06-14 15:00 ` remove long broken SCSI to NVMe translations Keith Busch 2017-06-14 15:01 ` Christoph Hellwig 2017-06-16 6:11 ` Hannes Reinecke 2017-06-16 8:31 ` Christoph Hellwig 2017-06-16 13:57 ` Linda Knippers 2017-06-20 6:48 ` Christoph Hellwig 2017-06-20 14:27 ` Keith Busch 2017-06-22 18:31 ` Micah Parrish 2017-06-22 19:21 ` Johannes Thumshirn 2017-06-30 13:07 ` Judy Brock 2017-06-14 15:14 ` Martin K. Petersen
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.