qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] hw/nvme/ctrl: fix csi field for cns 0x00 and 0x11
       [not found] <CGME20210427063530epcas5p4c260382227175e68d29da6d10957ad95@epcas5p4.samsung.com>
@ 2021-04-27  6:30 ` Gollu Appalanaidu
  2021-05-31 20:39   ` Klaus Jensen
  2021-06-09 20:10   ` Klaus Jensen
  0 siblings, 2 replies; 3+ messages in thread
From: Gollu Appalanaidu @ 2021-04-27  6:30 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.

Remove 'nvme_csi_has_nvm_support()' helper as suggested by
Klaus we can safely assume NVM command set support for all
namespaces.

Suggested-by: Klaus Jensen <its@irrelevant.dk>
Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
---
-v2: add sugggestions from Klaus
We can Remove 'nvme_csi_has_nvm_support()' helper, we can
assume NVM command set support for all namespaces.

 hw/nvme/ctrl.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 2e7498a73e..7fcd699235 100644
--- a/hw/nvme/ctrl.c
+++ b/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),
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] hw/nvme/ctrl: fix csi field for cns 0x00 and 0x11
  2021-04-27  6:30 ` [PATCH v2] hw/nvme/ctrl: fix csi field for cns 0x00 and 0x11 Gollu Appalanaidu
@ 2021-05-31 20:39   ` Klaus Jensen
  2021-06-09 20:10   ` Klaus Jensen
  1 sibling, 0 replies; 3+ messages in thread
From: Klaus Jensen @ 2021-05-31 20:39 UTC (permalink / raw)
  To: Gollu Appalanaidu
  Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, kbusch

[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]

On Apr 27 12:00, 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.
>
>Remove 'nvme_csi_has_nvm_support()' helper as suggested by
>Klaus we can safely assume NVM command set support for all
>namespaces.
>
>Suggested-by: Klaus Jensen <its@irrelevant.dk>
>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>

LGTM.

Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

>---
>-v2: add sugggestions from Klaus
>We can Remove 'nvme_csi_has_nvm_support()' helper, we can
>assume NVM command set support for all namespaces.
>
> hw/nvme/ctrl.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>index 2e7498a73e..7fcd699235 100644
>--- a/hw/nvme/ctrl.c
>+++ b/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),
>-- 
>2.17.1
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] hw/nvme/ctrl: fix csi field for cns 0x00 and 0x11
  2021-04-27  6:30 ` [PATCH v2] hw/nvme/ctrl: fix csi field for cns 0x00 and 0x11 Gollu Appalanaidu
  2021-05-31 20:39   ` Klaus Jensen
@ 2021-06-09 20:10   ` Klaus Jensen
  1 sibling, 0 replies; 3+ messages in thread
From: Klaus Jensen @ 2021-06-09 20:10 UTC (permalink / raw)
  To: Gollu Appalanaidu
  Cc: fam, kwolf, qemu-block, qemu-devel, mreitz, stefanha, kbusch

[-- Attachment #1: Type: text/plain, Size: 2133 bytes --]

On Apr 27 12:00, 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.
>
>Remove 'nvme_csi_has_nvm_support()' helper as suggested by
>Klaus we can safely assume NVM command set support for all
>namespaces.
>
>Suggested-by: Klaus Jensen <its@irrelevant.dk>
>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>---
>-v2: add sugggestions from Klaus
>We can Remove 'nvme_csi_has_nvm_support()' helper, we can
>assume NVM command set support for all namespaces.
>
> hw/nvme/ctrl.c | 14 ++------------
> 1 file changed, 2 insertions(+), 12 deletions(-)
>
>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>index 2e7498a73e..7fcd699235 100644
>--- a/hw/nvme/ctrl.c
>+++ b/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),
>-- 
>2.17.1
>

Applied to nvme-next. Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-06-09 20:13 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210427063530epcas5p4c260382227175e68d29da6d10957ad95@epcas5p4.samsung.com>
2021-04-27  6:30 ` [PATCH v2] hw/nvme/ctrl: fix csi field for cns 0x00 and 0x11 Gollu Appalanaidu
2021-05-31 20:39   ` Klaus Jensen
2021-06-09 20:10   ` Klaus Jensen

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