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

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

[Gollu Appalanaidu]

Make uniform hexadecimal numbers format.

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
---
 hw/block/nvme.c      | 30 +++++++++++++++---------------
 include/block/nvme.h | 10 +++++-----
 2 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index d439e44db8..21e85374bf 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -2728,18 +2728,18 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest *req)
 
     /*
      * In the base NVM command set, Flush may apply to all namespaces
-     * (indicated by NSID being set to 0xFFFFFFFF). But if that feature is used
+     * (indicated by NSID being set to 0xffffffff). But if that feature is used
      * along with TP 4056 (Namespace Types), it may be pretty screwed up.
      *
-     * If NSID is indeed set to 0xFFFFFFFF, we simply cannot associate the
+     * If NSID is indeed set to 0xffffffff, we simply cannot associate the
      * opcode with a specific command since we cannot determine a unique I/O
      * command set. Opcode 0x0 could have any other meaning than something
      * equivalent to flushing and say it DOES have completely different
-     * semantics in some other command set - does an NSID of 0xFFFFFFFF then
+     * semantics in some other command set - does an NSID of 0xffffffff then
      * mean "for all namespaces, apply whatever command set specific command
      * that uses the 0x0 opcode?" Or does it mean "for all namespaces, apply
      * whatever command that uses the 0x0 opcode if, and only if, it allows
-     * NSID to be 0xFFFFFFFF"?
+     * NSID to be 0xffffffff"?
      *
      * Anyway (and luckily), for now, we do not care about this since the
      * device only supports namespace types that includes the NVM Flush command
@@ -3948,8 +3948,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
             return NVME_INVALID_FIELD | NVME_DNR;
         }
 
-        trace_pci_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
-                                    ((dw11 >> 16) & 0xFFFF) + 1,
+        trace_pci_nvme_setfeat_numq((dw11 & 0xffff) + 1,
+                                    ((dw11 >> 16) & 0xffff) + 1,
                                     n->params.max_ioqpairs,
                                     n->params.max_ioqpairs);
         req->cqe.result = cpu_to_le32((n->params.max_ioqpairs - 1) |
@@ -4436,7 +4436,7 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         }
         break;
     case 0x20:  /* NSSR */
-        if (data == 0x4E564D65) {
+        if (data == 0x4e564d65) {
             trace_pci_nvme_ub_mmiowr_ssreset_unsupported();
         } else {
             /* The spec says that writes of other values have no effect */
@@ -4506,11 +4506,11 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         n->bar.cmbmsc = (n->bar.cmbmsc & 0xffffffff) | (data << 32);
         return;
 
-    case 0xE00: /* PMRCAP */
+    case 0xe00: /* PMRCAP */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrcap_readonly,
                        "invalid write to PMRCAP register, ignored");
         return;
-    case 0xE04: /* PMRCTL */
+    case 0xe04: /* PMRCTL */
         n->bar.pmrctl = data;
         if (NVME_PMRCTL_EN(data)) {
             memory_region_set_enabled(&n->pmr.dev->mr, true);
@@ -4521,19 +4521,19 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
             n->pmr.cmse = false;
         }
         return;
-    case 0xE08: /* PMRSTS */
+    case 0xe08: /* PMRSTS */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrsts_readonly,
                        "invalid write to PMRSTS register, ignored");
         return;
-    case 0xE0C: /* PMREBS */
+    case 0xe0C: /* PMREBS */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrebs_readonly,
                        "invalid write to PMREBS register, ignored");
         return;
-    case 0xE10: /* PMRSWTP */
+    case 0xe10: /* PMRSWTP */
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrswtp_readonly,
                        "invalid write to PMRSWTP register, ignored");
         return;
-    case 0xE14: /* PMRMSCL */
+    case 0xe14: /* PMRMSCL */
         if (!NVME_CAP_PMRS(n->bar.cap)) {
             return;
         }
@@ -4553,7 +4553,7 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         }
 
         return;
-    case 0xE18: /* PMRMSCU */
+    case 0xe18: /* PMRMSCU */
         if (!NVME_CAP_PMRS(n->bar.cap)) {
             return;
         }
@@ -4595,7 +4595,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
          * from PMRSTS should ensure prior writes
          * made it to persistent media
          */
-        if (addr == 0xE08 &&
+        if (addr == 0xe08 &&
             (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
             memory_region_msync(&n->pmr.dev->mr, 0, n->pmr.dev->size);
         }
diff --git a/include/block/nvme.h b/include/block/nvme.h
index 372d0f2799..fc65cfcb01 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -840,8 +840,8 @@ enum NvmeStatusCodes {
     NVME_FEAT_NOT_NS_SPEC       = 0x010f,
     NVME_FW_REQ_SUSYSTEM_RESET  = 0x0110,
     NVME_NS_ALREADY_ATTACHED    = 0x0118,
-    NVME_NS_NOT_ATTACHED        = 0x011A,
-    NVME_NS_CTRL_LIST_INVALID   = 0x011C,
+    NVME_NS_NOT_ATTACHED        = 0x011a,
+    NVME_NS_CTRL_LIST_INVALID   = 0x011c,
     NVME_CONFLICTING_ATTRS      = 0x0180,
     NVME_INVALID_PROT_INFO      = 0x0181,
     NVME_WRITE_TO_RO            = 0x0182,
@@ -1392,9 +1392,9 @@ typedef enum NvmeZoneState {
     NVME_ZONE_STATE_IMPLICITLY_OPEN  = 0x02,
     NVME_ZONE_STATE_EXPLICITLY_OPEN  = 0x03,
     NVME_ZONE_STATE_CLOSED           = 0x04,
-    NVME_ZONE_STATE_READ_ONLY        = 0x0D,
-    NVME_ZONE_STATE_FULL             = 0x0E,
-    NVME_ZONE_STATE_OFFLINE          = 0x0F,
+    NVME_ZONE_STATE_READ_ONLY        = 0x0d,
+    NVME_ZONE_STATE_FULL             = 0x0e,
+    NVME_ZONE_STATE_OFFLINE          = 0x0f,
 } NvmeZoneState;
 
 static inline void _nvme_check_size(void)
-- 
2.17.1

[PATCH 2/2] hw/block/nvme: align reserved fields declarations

[Gollu Appalanaidu]

Align the Reserved fields declaration in NvmeBar

Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
---
 include/block/nvme.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/block/nvme.h b/include/block/nvme.h
index fc65cfcb01..e5bd00bb85 100644
--- a/include/block/nvme.h
+++ b/include/block/nvme.h
@@ -7,7 +7,7 @@ typedef struct QEMU_PACKED NvmeBar {
     uint32_t    intms;
     uint32_t    intmc;
     uint32_t    cc;
-    uint32_t    rsvd1;
+    uint8_t     rsvd24[4];
     uint32_t    csts;
     uint32_t    nssrc;
     uint32_t    aqa;
-- 
2.17.1

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

[Klaus Jensen]

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

On Mar 17 15:00, Gollu Appalanaidu wrote:
>Make uniform hexadecimal numbers format.
>
>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>---
> hw/block/nvme.c      | 30 +++++++++++++++---------------
> include/block/nvme.h | 10 +++++-----
> 2 files changed, 20 insertions(+), 20 deletions(-)
>
>diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>index d439e44db8..21e85374bf 100644
>--- a/hw/block/nvme.c
>+++ b/hw/block/nvme.c
>@@ -2728,18 +2728,18 @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeRequest *req)
>
>     /*
>      * In the base NVM command set, Flush may apply to all namespaces
>-     * (indicated by NSID being set to 0xFFFFFFFF). But if that feature is used
>+     * (indicated by NSID being set to 0xffffffff). But if that feature is used
>      * along with TP 4056 (Namespace Types), it may be pretty screwed up.
>      *
>-     * If NSID is indeed set to 0xFFFFFFFF, we simply cannot associate the
>+     * If NSID is indeed set to 0xffffffff, we simply cannot associate the
>      * opcode with a specific command since we cannot determine a unique I/O
>      * command set. Opcode 0x0 could have any other meaning than something
>      * equivalent to flushing and say it DOES have completely different
>-     * semantics in some other command set - does an NSID of 0xFFFFFFFF then
>+     * semantics in some other command set - does an NSID of 0xffffffff then
>      * mean "for all namespaces, apply whatever command set specific command
>      * that uses the 0x0 opcode?" Or does it mean "for all namespaces, apply
>      * whatever command that uses the 0x0 opcode if, and only if, it allows
>-     * NSID to be 0xFFFFFFFF"?
>+     * NSID to be 0xffffffff"?
>      *
>      * Anyway (and luckily), for now, we do not care about this since the
>      * device only supports namespace types that includes the NVM Flush command
>@@ -3948,8 +3948,8 @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeRequest *req)
>             return NVME_INVALID_FIELD | NVME_DNR;
>         }
>
>-        trace_pci_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
>-                                    ((dw11 >> 16) & 0xFFFF) + 1,
>+        trace_pci_nvme_setfeat_numq((dw11 & 0xffff) + 1,
>+                                    ((dw11 >> 16) & 0xffff) + 1,
>                                     n->params.max_ioqpairs,
>                                     n->params.max_ioqpairs);
>         req->cqe.result = cpu_to_le32((n->params.max_ioqpairs - 1) |
>@@ -4436,7 +4436,7 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>         }
>         break;
>     case 0x20:  /* NSSR */
>-        if (data == 0x4E564D65) {
>+        if (data == 0x4e564d65) {
>             trace_pci_nvme_ub_mmiowr_ssreset_unsupported();
>         } else {
>             /* The spec says that writes of other values have no effect */
>@@ -4506,11 +4506,11 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>         n->bar.cmbmsc = (n->bar.cmbmsc & 0xffffffff) | (data << 32);
>         return;
>
>-    case 0xE00: /* PMRCAP */
>+    case 0xe00: /* PMRCAP */
>         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrcap_readonly,
>                        "invalid write to PMRCAP register, ignored");
>         return;
>-    case 0xE04: /* PMRCTL */
>+    case 0xe04: /* PMRCTL */
>         n->bar.pmrctl = data;
>         if (NVME_PMRCTL_EN(data)) {
>             memory_region_set_enabled(&n->pmr.dev->mr, true);
>@@ -4521,19 +4521,19 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>             n->pmr.cmse = false;
>         }
>         return;
>-    case 0xE08: /* PMRSTS */
>+    case 0xe08: /* PMRSTS */
>         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrsts_readonly,
>                        "invalid write to PMRSTS register, ignored");
>         return;
>-    case 0xE0C: /* PMREBS */
>+    case 0xe0C: /* PMREBS */
>         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrebs_readonly,
>                        "invalid write to PMREBS register, ignored");
>         return;
>-    case 0xE10: /* PMRSWTP */
>+    case 0xe10: /* PMRSWTP */
>         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrswtp_readonly,
>                        "invalid write to PMRSWTP register, ignored");
>         return;
>-    case 0xE14: /* PMRMSCL */
>+    case 0xe14: /* PMRMSCL */
>         if (!NVME_CAP_PMRS(n->bar.cap)) {
>             return;
>         }
>@@ -4553,7 +4553,7 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>         }
>
>         return;
>-    case 0xE18: /* PMRMSCU */
>+    case 0xe18: /* PMRMSCU */
>         if (!NVME_CAP_PMRS(n->bar.cap)) {
>             return;
>         }
>@@ -4595,7 +4595,7 @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
>          * from PMRSTS should ensure prior writes
>          * made it to persistent media
>          */
>-        if (addr == 0xE08 &&
>+        if (addr == 0xe08 &&
>             (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
>             memory_region_msync(&n->pmr.dev->mr, 0, n->pmr.dev->size);
>         }
>diff --git a/include/block/nvme.h b/include/block/nvme.h
>index 372d0f2799..fc65cfcb01 100644
>--- a/include/block/nvme.h
>+++ b/include/block/nvme.h
>@@ -840,8 +840,8 @@ enum NvmeStatusCodes {
>     NVME_FEAT_NOT_NS_SPEC       = 0x010f,
>     NVME_FW_REQ_SUSYSTEM_RESET  = 0x0110,
>     NVME_NS_ALREADY_ATTACHED    = 0x0118,
>-    NVME_NS_NOT_ATTACHED        = 0x011A,
>-    NVME_NS_CTRL_LIST_INVALID   = 0x011C,
>+    NVME_NS_NOT_ATTACHED        = 0x011a,
>+    NVME_NS_CTRL_LIST_INVALID   = 0x011c,
>     NVME_CONFLICTING_ATTRS      = 0x0180,
>     NVME_INVALID_PROT_INFO      = 0x0181,
>     NVME_WRITE_TO_RO            = 0x0182,
>@@ -1392,9 +1392,9 @@ typedef enum NvmeZoneState {
>     NVME_ZONE_STATE_IMPLICITLY_OPEN  = 0x02,
>     NVME_ZONE_STATE_EXPLICITLY_OPEN  = 0x03,
>     NVME_ZONE_STATE_CLOSED           = 0x04,
>-    NVME_ZONE_STATE_READ_ONLY        = 0x0D,
>-    NVME_ZONE_STATE_FULL             = 0x0E,
>-    NVME_ZONE_STATE_OFFLINE          = 0x0F,
>+    NVME_ZONE_STATE_READ_ONLY        = 0x0d,
>+    NVME_ZONE_STATE_FULL             = 0x0e,
>+    NVME_ZONE_STATE_OFFLINE          = 0x0f,
> } NvmeZoneState;
>
> static inline void _nvme_check_size(void)
>-- 
>2.17.1
>

I do prefer lower case hex in code, but for comments I'd actually like 
if they used the same format as in the spec (i.e. "FFFFFFFFh").

This is total bikeshedding, but I guess this patch basically is too :)

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

Re: [PATCH 2/2] hw/block/nvme: align reserved fields declarations

[Klaus Jensen]

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

On Mar 17 15:00, Gollu Appalanaidu wrote:
>Align the Reserved fields declaration in NvmeBar
>
>Signed-off-by: Gollu Appalanaidu <anaidu.gollu@samsung.com>
>---
> include/block/nvme.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/include/block/nvme.h b/include/block/nvme.h
>index fc65cfcb01..e5bd00bb85 100644
>--- a/include/block/nvme.h
>+++ b/include/block/nvme.h
>@@ -7,7 +7,7 @@ typedef struct QEMU_PACKED NvmeBar {
>     uint32_t    intms;
>     uint32_t    intmc;
>     uint32_t    cc;
>-    uint32_t    rsvd1;
>+    uint8_t     rsvd24[4];
>     uint32_t    csts;
>     uint32_t    nssrc;
>     uint32_t    aqa;
>-- 
>2.17.1
>

Thanks. Applied to nvme-next with a small commit message fixup!

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