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),