[PATCH 0/3] hw/block/nvme: misc fixes

[PATCH 0/3] hw/block/nvme: misc fixes

[Klaus Jensen]

From: Klaus Jensen <k.jensen@samsung.com>

Misc fixes from Gollu.

Gollu Appalanaidu (3):
  hw/block/nvme: fix set feature for error recovery
  hw/block/nvme: fix set feature save field check
  hw/block/nvme: align with existing style

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

-- 
2.30.0

[PATCH 1/3] hw/block/nvme: fix set feature for error recovery

[Klaus Jensen]

From: Gollu Appalanaidu <anaidu.gollu@samsung.com>

Only enable DULBE if the namespace supports it.

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 21aec90637fa..e7983ff422f2 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -3396,7 +3396,9 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
         }
 
         assert(ns);
-        ns->features.err_rec = dw11;
+        if (NVME_ID_NS_NSFEAT_DULBE(ns->id_ns.nsfeat))  {
+            ns->features.err_rec = dw11;
+        }
         break;
     case NVME_VOLATILE_WRITE_CACHE:
         for (i = 1; i <= n->num_namespaces; i++) {
-- 
2.30.0

[PATCH 2/3] hw/block/nvme: fix set feature save field check

[Klaus Jensen]

From: Gollu Appalanaidu <anaidu.gollu@samsung.com>

Currently, no features are saveable, so the current check is not wrong,
but add a check against the feature capabilities to make sure this will
not regress if saveable features are added later.

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index e7983ff422f2..1be5b54e0fed 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -3324,7 +3324,7 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
 
     trace_pci_nvme_setfeat(nvme_cid(req), nsid, fid, save, dw11);
 
-    if (save) {
+    if (save && !(nvme_feature_cap[fid] & NVME_FEAT_CAP_SAVE)) {
         return NVME_FID_NOT_SAVEABLE | NVME_DNR;
     }
 
-- 
2.30.0

[PATCH 3/3] hw/block/nvme: align with existing style

[Klaus Jensen]

From: Gollu Appalanaidu <anaidu.gollu@samsung.com>

Change status checks to align with the existing style and remove the
explicit check against NVME_SUCCESS.

Cc: Dmitry Fomichev <dmitry.fomichev@wdc.com>
Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reviewed-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 1be5b54e0fed..98d84fe26644 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1198,7 +1198,7 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n, NvmeNamespace *ns,
         status = nvme_check_zone_state_for_write(zone);
     }
 
-    if (status != NVME_SUCCESS) {
+    if (status) {
         trace_pci_nvme_err_zone_write_not_ok(slba, nlb, status);
     } else {
         assert(nvme_wp_is_valid(zone));
@@ -1253,7 +1253,7 @@ static uint16_t nvme_check_zone_read(NvmeNamespace *ns, uint64_t slba,
     uint16_t status;
 
     status = nvme_check_zone_state_for_read(zone);
-    if (status != NVME_SUCCESS) {
+    if (status) {
         ;
     } else if (unlikely(end > bndry)) {
         if (!ns->params.cross_zone_read) {
@@ -1266,7 +1266,7 @@ static uint16_t nvme_check_zone_read(NvmeNamespace *ns, uint64_t slba,
             do {
                 zone++;
                 status = nvme_check_zone_state_for_read(zone);
-                if (status != NVME_SUCCESS) {
+                if (status) {
                     break;
                 }
             } while (end > nvme_zone_rd_boundary(ns, zone));
@@ -1677,7 +1677,7 @@ static uint16_t nvme_read(NvmeCtrl *n, NvmeRequest *req)
 
     if (ns->params.zoned) {
         status = nvme_check_zone_read(ns, slba, nlb);
-        if (status != NVME_SUCCESS) {
+        if (status) {
             trace_pci_nvme_err_zone_read_not_ok(slba, nlb, status);
             goto invalid;
         }
@@ -1748,12 +1748,12 @@ static uint16_t nvme_do_write(NvmeCtrl *n, NvmeRequest *req, bool append,
         zone = nvme_get_zone_by_slba(ns, slba);
 
         status = nvme_check_zone_write(n, ns, zone, slba, nlb, append);
-        if (status != NVME_SUCCESS) {
+        if (status) {
             goto invalid;
         }
 
         status = nvme_auto_open_zone(ns, zone);
-        if (status != NVME_SUCCESS) {
+        if (status) {
             goto invalid;
         }
 
@@ -1852,14 +1852,14 @@ static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone,
     switch (state) {
     case NVME_ZONE_STATE_EMPTY:
         status = nvme_aor_check(ns, 1, 0);
-        if (status != NVME_SUCCESS) {
+        if (status) {
             return status;
         }
         nvme_aor_inc_active(ns);
         /* fall through */
     case NVME_ZONE_STATE_CLOSED:
         status = nvme_aor_check(ns, 0, 1);
-        if (status != NVME_SUCCESS) {
+        if (status) {
             if (state == NVME_ZONE_STATE_EMPTY) {
                 nvme_aor_dec_active(ns);
             }
@@ -1972,7 +1972,7 @@ static uint16_t nvme_set_zd_ext(NvmeNamespace *ns, NvmeZone *zone)
 
     if (state == NVME_ZONE_STATE_EMPTY) {
         status = nvme_aor_check(ns, 1, 0);
-        if (status != NVME_SUCCESS) {
+        if (status) {
             return status;
         }
         nvme_aor_inc_active(ns);
@@ -3301,7 +3301,7 @@ static uint16_t nvme_set_feature_timestamp(NvmeCtrl *n, NvmeRequest *req)
 
     ret = nvme_dma(n, (uint8_t *)&timestamp, sizeof(timestamp),
                    DMA_DIRECTION_TO_DEVICE, req);
-    if (ret != NVME_SUCCESS) {
+    if (ret) {
         return ret;
     }
 
-- 
2.30.0

Re: [PATCH 3/3] hw/block/nvme: align with existing style

[Dmitry Fomichev]

On Mon, 2021-01-25 at 09:22 +0100, Klaus Jensen wrote:
> From: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> 
> Change status checks to align with the existing style and remove the
> explicit check against NVME_SUCCESS.
> 
> Cc: Dmitry Fomichev <dmitry.fomichev@wdc.com>
> Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> Reviewed-by: Klaus Jensen <k.jensen@samsung.com>

Looks good.
Reviewed-by: Dmitry Fomichev <dmitry.fomichev@wdc.com>

> ---
>  hw/block/nvme.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 1be5b54e0fed..98d84fe26644 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1198,7 +1198,7 @@ static uint16_t nvme_check_zone_write(NvmeCtrl *n, NvmeNamespace *ns,
>          status = nvme_check_zone_state_for_write(zone);
>      }
>  
> 
> 
> 
> -    if (status != NVME_SUCCESS) {
> +    if (status) {
>          trace_pci_nvme_err_zone_write_not_ok(slba, nlb, status);
>      } else {
>          assert(nvme_wp_is_valid(zone));
> @@ -1253,7 +1253,7 @@ static uint16_t nvme_check_zone_read(NvmeNamespace *ns, uint64_t slba,
>      uint16_t status;
>  
> 
> 
> 
>      status = nvme_check_zone_state_for_read(zone);
> -    if (status != NVME_SUCCESS) {
> +    if (status) {
>          ;
>      } else if (unlikely(end > bndry)) {
>          if (!ns->params.cross_zone_read) {
> @@ -1266,7 +1266,7 @@ static uint16_t nvme_check_zone_read(NvmeNamespace *ns, uint64_t slba,
>              do {
>                  zone++;
>                  status = nvme_check_zone_state_for_read(zone);
> -                if (status != NVME_SUCCESS) {
> +                if (status) {
>                      break;
>                  }
>              } while (end > nvme_zone_rd_boundary(ns, zone));
> @@ -1677,7 +1677,7 @@ static uint16_t nvme_read(NvmeCtrl *n, NvmeRequest *req)
>  
> 
> 
> 
>      if (ns->params.zoned) {
>          status = nvme_check_zone_read(ns, slba, nlb);
> -        if (status != NVME_SUCCESS) {
> +        if (status) {
>              trace_pci_nvme_err_zone_read_not_ok(slba, nlb, status);
>              goto invalid;
>          }
> @@ -1748,12 +1748,12 @@ static uint16_t nvme_do_write(NvmeCtrl *n, NvmeRequest *req, bool append,
>          zone = nvme_get_zone_by_slba(ns, slba);
>  
> 
> 
> 
>          status = nvme_check_zone_write(n, ns, zone, slba, nlb, append);
> -        if (status != NVME_SUCCESS) {
> +        if (status) {
>              goto invalid;
>          }
>  
> 
> 
> 
>          status = nvme_auto_open_zone(ns, zone);
> -        if (status != NVME_SUCCESS) {
> +        if (status) {
>              goto invalid;
>          }
>  
> 
> 
> 
> @@ -1852,14 +1852,14 @@ static uint16_t nvme_open_zone(NvmeNamespace *ns, NvmeZone *zone,
>      switch (state) {
>      case NVME_ZONE_STATE_EMPTY:
>          status = nvme_aor_check(ns, 1, 0);
> -        if (status != NVME_SUCCESS) {
> +        if (status) {
>              return status;
>          }
>          nvme_aor_inc_active(ns);
>          /* fall through */
>      case NVME_ZONE_STATE_CLOSED:
>          status = nvme_aor_check(ns, 0, 1);
> -        if (status != NVME_SUCCESS) {
> +        if (status) {
>              if (state == NVME_ZONE_STATE_EMPTY) {
>                  nvme_aor_dec_active(ns);
>              }
> @@ -1972,7 +1972,7 @@ static uint16_t nvme_set_zd_ext(NvmeNamespace *ns, NvmeZone *zone)
>  
> 
> 
> 
>      if (state == NVME_ZONE_STATE_EMPTY) {
>          status = nvme_aor_check(ns, 1, 0);
> -        if (status != NVME_SUCCESS) {
> +        if (status) {
>              return status;
>          }
>          nvme_aor_inc_active(ns);
> @@ -3301,7 +3301,7 @@ static uint16_t nvme_set_feature_timestamp(NvmeCtrl *n, NvmeRequest *req)
>  
> 
> 
> 
>      ret = nvme_dma(n, (uint8_t *)&timestamp, sizeof(timestamp),
>                     DMA_DIRECTION_TO_DEVICE, req);
> -    if (ret != NVME_SUCCESS) {
> +    if (ret) {
>          return ret;
>      }
>  
> 
> 
> 

Re: [PATCH 0/3] hw/block/nvme: misc fixes

[Keith Busch]

On Mon, Jan 25, 2021 at 09:22:24AM +0100, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Misc fixes from Gollu.

Looks good.

Reviewed-by: Keith Busch <kbusch@kernel.org>

Re: [PATCH 0/3] hw/block/nvme: misc fixes

[Klaus Jensen]

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

On Jan 25 10:02, Keith Busch wrote:
> On Mon, Jan 25, 2021 at 09:22:24AM +0100, Klaus Jensen wrote:
> > From: Klaus Jensen <k.jensen@samsung.com>
> > 
> > Misc fixes from Gollu.
> 
> Looks good.
> 
> Reviewed-by: Keith Busch <kbusch@kernel.org>

Thanks, applied!

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