* [PATCH] hw/block/nvme: fix csi field for cns 0x00 and 0x11 [not found] <CGME20210426074741epcas5p1ac30fed5ef8c21a1b7e5685920ff6847@epcas5p1.samsung.com> @ 2021-04-26 7:46 ` Gollu Appalanaidu 2021-04-26 11:03 ` Klaus Jensen 0 siblings, 1 reply; 3+ messages in thread From: Gollu Appalanaidu @ 2021-04-26 7:46 UTC (permalink / raw) To: qemu-devel Cc: fam, kwolf, qemu-block, Gollu Appalanaidu, mreitz, its, stefanha, kbusch As per the TP 4056d Namespace types CNS 0x00 and CNS 0x11 CSI field shouldn't use but it is being used for these two Identify command CNS values, fix that. Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com> --- hw/nvme/ctrl.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c index 2e7498a73e..1657b1d04a 100644 --- a/hw/nvme/ctrl.c +++ b/hw/nvme/ctrl.c @@ -4244,11 +4244,16 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeRequest *req, bool active) } } - if (c->csi == NVME_CSI_NVM && nvme_csi_has_nvm_support(ns)) { - return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); + if (active && nvme_csi_has_nvm_support(ns)) { + goto out; + } else if (!active && ns->csi == NVME_CSI_NVM) { + goto out; + } else { + return NVME_INVALID_CMD_SET | NVME_DNR; } - return NVME_INVALID_CMD_SET | NVME_DNR; +out: + return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); } static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req) -- 2.17.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/block/nvme: fix csi field for cns 0x00 and 0x11 2021-04-26 7:46 ` [PATCH] hw/block/nvme: fix csi field for cns 0x00 and 0x11 Gollu Appalanaidu @ 2021-04-26 11:03 ` Klaus Jensen 2021-04-27 6:06 ` Gollu Appalanaidu 0 siblings, 1 reply; 3+ messages in thread From: Klaus Jensen @ 2021-04-26 11:03 UTC (permalink / raw) To: Gollu Appalanaidu Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, kbusch [-- Attachment #1: Type: text/plain, Size: 2971 bytes --] On Apr 26 13:16, Gollu Appalanaidu wrote: >As per the TP 4056d Namespace types CNS 0x00 and CNS 0x11 >CSI field shouldn't use but it is being used for these two >Identify command CNS values, fix that. > >Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com> >--- > hw/nvme/ctrl.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > >diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c >index 2e7498a73e..1657b1d04a 100644 >--- a/hw/nvme/ctrl.c >+++ b/hw/nvme/ctrl.c >@@ -4244,11 +4244,16 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeRequest *req, bool active) > } > } > >- if (c->csi == NVME_CSI_NVM && nvme_csi_has_nvm_support(ns)) { >- return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); >+ if (active && nvme_csi_has_nvm_support(ns)) { >+ goto out; >+ } else if (!active && ns->csi == NVME_CSI_NVM) { >+ goto out; >+ } else { >+ return NVME_INVALID_CMD_SET | NVME_DNR; > } > >- return NVME_INVALID_CMD_SET | NVME_DNR; >+out: >+ return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); > } > > static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req) >-- >2.17.1 > > Looking closer at this, since we only support the NVM and Zoned command sets, we can get rid of the `nvme_csi_has_nvm_support()` helper and just assume NVM command set support for all namespaces. The way different command sets are handled doesn't scale anyway, so we might as well simplify the logic a bit. Something like this (compile-tested only) patch maybe? diff --git i/hw/nvme/ctrl.c w/hw/nvme/ctrl.c index 2e7498a73e70..7fcd6992358d 100644 --- i/hw/nvme/ctrl.c +++ w/hw/nvme/ctrl.c @@ -4178,16 +4178,6 @@ static uint16_t nvme_rpt_empty_id_struct(NvmeCtrl *n, NvmeRequest *req) return nvme_c2h(n, id, sizeof(id), req); } -static inline bool nvme_csi_has_nvm_support(NvmeNamespace *ns) -{ - switch (ns->csi) { - case NVME_CSI_NVM: - case NVME_CSI_ZONED: - return true; - } - return false; -} - static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeRequest *req) { trace_pci_nvme_identify_ctrl(); @@ -4244,7 +4234,7 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeRequest *req, bool active) } } - if (c->csi == NVME_CSI_NVM && nvme_csi_has_nvm_support(ns)) { + if (active || ns->csi == NVME_CSI_NVM) { return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); } @@ -4315,7 +4305,7 @@ static uint16_t nvme_identify_ns_csi(NvmeCtrl *n, NvmeRequest *req, } } - if (c->csi == NVME_CSI_NVM && nvme_csi_has_nvm_support(ns)) { + if (c->csi == NVME_CSI_NVM) { return nvme_rpt_empty_id_struct(n, req); } else if (c->csi == NVME_CSI_ZONED && ns->csi == NVME_CSI_ZONED) { return nvme_c2h(n, (uint8_t *)ns->id_ns_zoned, sizeof(NvmeIdNsZoned), [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] hw/block/nvme: fix csi field for cns 0x00 and 0x11 2021-04-26 11:03 ` Klaus Jensen @ 2021-04-27 6:06 ` Gollu Appalanaidu 0 siblings, 0 replies; 3+ messages in thread From: Gollu Appalanaidu @ 2021-04-27 6:06 UTC (permalink / raw) To: Klaus Jensen; +Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, kbusch [-- Attachment #1: Type: text/plain, Size: 3051 bytes --] On Mon, Apr 26, 2021 at 01:03:04PM +0200, Klaus Jensen wrote: >On Apr 26 13:16, Gollu Appalanaidu wrote: >>As per the TP 4056d Namespace types CNS 0x00 and CNS 0x11 >>CSI field shouldn't use but it is being used for these two >>Identify command CNS values, fix that. >> >>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com> >>--- >>hw/nvme/ctrl.c | 11 ++++++++--- >>1 file changed, 8 insertions(+), 3 deletions(-) >> >>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c >>index 2e7498a73e..1657b1d04a 100644 >>--- a/hw/nvme/ctrl.c >>+++ b/hw/nvme/ctrl.c >>@@ -4244,11 +4244,16 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeRequest *req, bool active) >> } >> } >> >>- if (c->csi == NVME_CSI_NVM && nvme_csi_has_nvm_support(ns)) { >>- return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); >>+ if (active && nvme_csi_has_nvm_support(ns)) { >>+ goto out; >>+ } else if (!active && ns->csi == NVME_CSI_NVM) { >>+ goto out; >>+ } else { >>+ return NVME_INVALID_CMD_SET | NVME_DNR; >> } >> >>- return NVME_INVALID_CMD_SET | NVME_DNR; >>+out: >>+ return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); >>} >> >>static uint16_t nvme_identify_ns_attached_list(NvmeCtrl *n, NvmeRequest *req) >>-- >>2.17.1 >> >> > >Looking closer at this, since we only support the NVM and Zoned >command sets, we can get rid of the `nvme_csi_has_nvm_support()` >helper and just assume NVM command set support for all namespaces. The >way different command sets are handled doesn't scale anyway, so we >might as well simplify the logic a bit. > >Something like this (compile-tested only) patch maybe? > >diff --git i/hw/nvme/ctrl.c w/hw/nvme/ctrl.c >index 2e7498a73e70..7fcd6992358d 100644 >--- i/hw/nvme/ctrl.c >+++ w/hw/nvme/ctrl.c >@@ -4178,16 +4178,6 @@ static uint16_t nvme_rpt_empty_id_struct(NvmeCtrl *n, NvmeRequest *req) > return nvme_c2h(n, id, sizeof(id), req); > } > >-static inline bool nvme_csi_has_nvm_support(NvmeNamespace *ns) >-{ >- switch (ns->csi) { >- case NVME_CSI_NVM: >- case NVME_CSI_ZONED: >- return true; >- } >- return false; >-} >- > static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeRequest *req) > { > trace_pci_nvme_identify_ctrl(); >@@ -4244,7 +4234,7 @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeRequest *req, bool active) > } > } > >- if (c->csi == NVME_CSI_NVM && nvme_csi_has_nvm_support(ns)) { >+ if (active || ns->csi == NVME_CSI_NVM) { > return nvme_c2h(n, (uint8_t *)&ns->id_ns, sizeof(NvmeIdNs), req); > } > >@@ -4315,7 +4305,7 @@ static uint16_t nvme_identify_ns_csi(NvmeCtrl *n, NvmeRequest *req, > } > } > >- if (c->csi == NVME_CSI_NVM && nvme_csi_has_nvm_support(ns)) { >+ if (c->csi == NVME_CSI_NVM) { > return nvme_rpt_empty_id_struct(n, req); > } else if (c->csi == NVME_CSI_ZONED && ns->csi == NVME_CSI_ZONED) { > return nvme_c2h(n, (uint8_t *)ns->id_ns_zoned, sizeof(NvmeIdNsZoned), > Sure, will make changes and submit v2 [-- Attachment #2: Type: text/plain, Size: 0 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-04-27 6:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20210426074741epcas5p1ac30fed5ef8c21a1b7e5685920ff6847@epcas5p1.samsung.com>
2021-04-26 7:46 ` [PATCH] hw/block/nvme: fix csi field for cns 0x00 and 0x11 Gollu Appalanaidu
2021-04-26 11:03 ` Klaus Jensen
2021-04-27 6:06 ` Gollu Appalanaidu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).