diff mbox series

[v3,4/5] hw/nvme: fix mmio read

Message ID 20210714060125.994882-5-its@irrelevant.dk
State New
Headers show
Series hw/nvme: fix mmio read | expand

Commit Message

Klaus Jensen July 14, 2021, 6:01 a.m. UTC
From: Klaus Jensen <k.jensen@samsung.com>

The new PMR test unearthed a long-standing issue with MMIO reads on
big-endian hosts.

Fix this by unconditionally storing all controller registers in little
endian.

Cc: Gollu Appalanaidu <anaidu.gollu@samsung.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/ctrl.c | 300 ++++++++++++++++++++++++++++---------------------
 1 file changed, 173 insertions(+), 127 deletions(-)

Comments

Peter Maydell July 19, 2021, 9:52 a.m. UTC | #1
On Wed, 14 Jul 2021 at 07:01, Klaus Jensen <its@irrelevant.dk> wrote:
>
> From: Klaus Jensen <k.jensen@samsung.com>
>
> The new PMR test unearthed a long-standing issue with MMIO reads on
> big-endian hosts.
>
> Fix this by unconditionally storing all controller registers in little
> endian.
>
> Cc: Gollu Appalanaidu <anaidu.gollu@samsung.com>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/nvme/ctrl.c | 300 ++++++++++++++++++++++++++++---------------------
>  1 file changed, 173 insertions(+), 127 deletions(-)

> @@ -5708,22 +5712,38 @@ static int nvme_start_ctrl(NvmeCtrl *n)
>
>  static void nvme_cmb_enable_regs(NvmeCtrl *n)
>  {
> -    NVME_CMBLOC_SET_CDPCILS(n->bar.cmbloc, 1);
> -    NVME_CMBLOC_SET_CDPMLS(n->bar.cmbloc, 1);
> -    NVME_CMBLOC_SET_BIR(n->bar.cmbloc, NVME_CMB_BIR);
> +    uint32_t cmbloc = 0, cmbsz = 0;
>
> -    NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
> -    NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
> -    NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 1);
> -    NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
> -    NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
> -    NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
> -    NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
> +    NVME_CMBLOC_SET_CDPCILS(cmbloc, 1);
> +    NVME_CMBLOC_SET_CDPMLS(cmbloc, 1);
> +    NVME_CMBLOC_SET_BIR(cmbloc, NVME_CMB_BIR);
> +    stl_le_p(&n->bar.cmbloc, cmbloc);
> +
> +    NVME_CMBSZ_SET_SQS(cmbsz, 1);
> +    NVME_CMBSZ_SET_CQS(cmbsz, 0);
> +    NVME_CMBSZ_SET_LISTS(cmbsz, 1);
> +    NVME_CMBSZ_SET_RDS(cmbsz, 1);
> +    NVME_CMBSZ_SET_WDS(cmbsz, 1);
> +    NVME_CMBSZ_SET_SZU(cmbsz, 2); /* MBs */
> +    NVME_CMBSZ_SET_SZ(cmbsz, n->params.cmb_size_mb);
> +    stl_le_p(&n->bar.cmbsz, cmbsz);

This used to only set the listed fields and left the
rest of the registers untouched. Now it zeroes the other
fields in the registers. If this is an intentional change it
should be separate from this patch, I think.

> @@ -5747,9 +5767,10 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>                             " when MSI-X is enabled");
>              /* should be ignored, fall through for now */
>          }
> -        n->bar.intms |= data & 0xffffffff;
> +        intms |= data & 0xffffffff;

intms is a uint32_t so the & here is unnecessary; you can just
say "intms |= data;".

> +        stl_le_p(&n->bar.intms, intms);
>          n->bar.intmc = n->bar.intms;
> -        trace_pci_nvme_mmio_intm_set(data & 0xffffffff, n->bar.intmc);
> +        trace_pci_nvme_mmio_intm_set(data & 0xffffffff, intms);
>          nvme_irq_check(n);
>          break;
>      case NVME_REG_INTMC:
> @@ -5759,44 +5780,55 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>                             " when MSI-X is enabled");
>              /* should be ignored, fall through for now */
>          }
> -        n->bar.intms &= ~(data & 0xffffffff);
> +        intms &= ~(data & 0xffffffff);

Similarly here just "intms &= ~data;" is enough.

> +        stl_le_p(&n->bar.intms, intms);
>          n->bar.intmc = n->bar.intms;
> -        trace_pci_nvme_mmio_intm_clr(data & 0xffffffff, n->bar.intmc);
> +        trace_pci_nvme_mmio_intm_clr(data & 0xffffffff, intms);
>          nvme_irq_check(n);
>          break;

> @@ -5818,26 +5850,30 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>          }
>          break;
>      case NVME_REG_AQA:
> -        n->bar.aqa = data & 0xffffffff;
> +        stl_le_p(&n->bar.aqa, data & 0xffffffff);

You don't need to mask here, stl_le_p() takes a uint32_t argument and
will only write 4 bytes.

>          trace_pci_nvme_mmio_aqattr(data & 0xffffffff);
>          break;
>      case NVME_REG_ASQ:
> -        n->bar.asq = size == 8 ? data :
> -            (n->bar.asq & ~0xffffffffULL) | (data & 0xffffffff);
> +        asq = size == 8 ? data :
> +            (asq & ~0xffffffffULL) | (data & 0xffffffff);
> +        stq_le_p(&n->bar.asq, asq);

You could save doing the manual assembly of the 64-byte value and just write
the data you have:

           stn_le_p(&n->bar.asq, size, data);

>          trace_pci_nvme_mmio_asqaddr(data);
>          break;
>      case NVME_REG_ASQ + 4:
> -        n->bar.asq = (n->bar.asq & 0xffffffff) | (data << 32);
> -        trace_pci_nvme_mmio_asqaddr_hi(data, n->bar.asq);
> +        asq = (asq & 0xffffffff) | (data << 32);
> +        stq_le_p(&n->bar.asq, asq);
> +        trace_pci_nvme_mmio_asqaddr_hi(data, asq);

Similarly here
           stn_le_p(&n->bar.asq + 4, size, data);
           trace_pci_nvme_mmio_asqaddr_hi(data, ldq_le_p(&n->bar.asq));

(and then you don't need 'asq' as a local variable in this function.)

>          break;
>      case NVME_REG_ACQ:
>          trace_pci_nvme_mmio_acqaddr(data);
> -        n->bar.acq = size == 8 ? data :
> -            (n->bar.acq & ~0xffffffffULL) | (data & 0xffffffff);
> +        acq = size == 8 ? data :
> +            (acq & ~0xffffffffULL) | (data & 0xffffffff);
> +        stq_le_p(&n->bar.acq, acq);
>          break;
>      case NVME_REG_ACQ + 4:
> -        n->bar.acq = (n->bar.acq & 0xffffffff) | (data << 32);
> -        trace_pci_nvme_mmio_acqaddr_hi(data, n->bar.acq);
> +        acq = (acq & 0xffffffff) | (data << 32);
> +        stq_le_p(&n->bar.acq, acq);
> +        trace_pci_nvme_mmio_acqaddr_hi(data, acq);
>          break;

Same remarks as for ASQ apply here for ACQ.

>      case NVME_REG_CMBLOC:
>          NVME_GUEST_ERR(pci_nvme_ub_mmiowr_cmbloc_reserved,
> @@ -5849,12 +5885,13 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>                         "invalid write to read only CMBSZ, ignored");
>          return;
>      case NVME_REG_CMBMSC:
> -        if (!NVME_CAP_CMBS(n->bar.cap)) {
> +        if (!NVME_CAP_CMBS(cap)) {
>              return;
>          }
>
> -        n->bar.cmbmsc = size == 8 ? data :
> -            (n->bar.cmbmsc & ~0xffffffff) | (data & 0xffffffff);
> +        cmbmsc = size == 8 ? data :
> +            (cmbmsc & ~0xffffffff) | (data & 0xffffffff);
> +        stq_le_p(&n->bar.cmbmsc, cmbmsc);
>          n->cmb.cmse = false;

and for CMBMSC

>
>          if (NVME_CMBMSC_CRE(data)) {
> @@ -5863,7 +5900,8 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>              if (NVME_CMBMSC_CMSE(data)) {
>                  hwaddr cba = NVME_CMBMSC_CBA(data) << CMBMSC_CBA_SHIFT;
>                  if (cba + int128_get64(n->cmb.mem.size) < cba) {
> -                    NVME_CMBSTS_SET_CBAI(n->bar.cmbsts, 1);
> +                    NVME_CMBSTS_SET_CBAI(cmbsts, 1);
> +                    stl_le_p(&n->bar.cmbsts, cmbsts);
>                      return;
>                  }
>
> @@ -5877,7 +5915,8 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>
>          return;
>      case NVME_REG_CMBMSC + 4:
> -        n->bar.cmbmsc = (n->bar.cmbmsc & 0xffffffff) | (data << 32);
> +        cmbmsc = (cmbmsc & 0xffffffff) | (data << 32);
> +        stq_le_p(&n->bar.cmbmsc, cmbmsc);

>          return;
>
>      case NVME_REG_PMRCAP:
> @@ -5885,19 +5924,20 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>                         "invalid write to PMRCAP register, ignored");
>          return;
>      case NVME_REG_PMRCTL:
> -        if (!NVME_CAP_PMRS(n->bar.cap)) {
> +        if (!NVME_CAP_PMRS(cap)) {
>              return;
>          }
>
> -        n->bar.pmrctl = data;
> +        stl_le_p(&n->bar.pmrctl, data & 0xffffffff);

More unnecessary masking

>          if (NVME_PMRCTL_EN(data)) {
>              memory_region_set_enabled(&n->pmr.dev->mr, true);
> -            n->bar.pmrsts = 0;
> +            pmrsts = 0;
>          } else {
>              memory_region_set_enabled(&n->pmr.dev->mr, false);
> -            NVME_PMRSTS_SET_NRDY(n->bar.pmrsts, 1);
> +            NVME_PMRSTS_SET_NRDY(pmrsts, 1);
>              n->pmr.cmse = false;
>          }
> +        stl_le_p(&n->bar.pmrsts, pmrsts);
>          return;
>      case NVME_REG_PMRSTS:
>          NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrsts_readonly,
> @@ -5912,18 +5952,20 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>                         "invalid write to PMRSWTP register, ignored");
>          return;
>      case NVME_REG_PMRMSCL:
> -        if (!NVME_CAP_PMRS(n->bar.cap)) {
> +        if (!NVME_CAP_PMRS(cap)) {
>              return;
>          }
>
> -        n->bar.pmrmscl = data & 0xffffffff;
> +        pmrmscl = data & 0xffffffff;

pmrmscl is a uint32_t so the mask is unneeded

> +        stl_le_p(&n->bar.pmrmscl, pmrmscl);
>          n->pmr.cmse = false;
>
> -        if (NVME_PMRMSCL_CMSE(n->bar.pmrmscl)) {
> -            hwaddr cba = n->bar.pmrmscu |
> -                (NVME_PMRMSCL_CBA(n->bar.pmrmscl) << PMRMSCL_CBA_SHIFT);
> +        if (NVME_PMRMSCL_CMSE(data)) {
> +            hwaddr cba = pmrmscu |
> +                (NVME_PMRMSCL_CBA(pmrmscl) << PMRMSCL_CBA_SHIFT);
>              if (cba + int128_get64(n->pmr.dev->mr.size) < cba) {
> -                NVME_PMRSTS_SET_CBAI(n->bar.pmrsts, 1);
> +                NVME_PMRSTS_SET_CBAI(pmrsts, 1);
> +                stl_le_p(&n->bar.pmrsts, pmrsts);
>                  return;
>              }
>
> @@ -5933,11 +5975,11 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>
>          return;
>      case NVME_REG_PMRMSCU:
> -        if (!NVME_CAP_PMRS(n->bar.cap)) {
> +        if (!NVME_CAP_PMRS(cap)) {
>              return;
>          }
>
> -        n->bar.pmrmscu = data & 0xffffffff;
> +        stl_le_p(&n->bar.pmrmscu, data & 0xffffffff);

Unneeded mask

>          return;
>      default:
>          NVME_GUEST_ERR(pci_nvme_ub_mmiowr_invalid,

> @@ -6265,14 +6306,17 @@ static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
>
>  static void nvme_init_pmr(NvmeCtrl *n, PCIDevice *pci_dev)
>  {
> -    NVME_PMRCAP_SET_RDS(n->bar.pmrcap, 1);
> -    NVME_PMRCAP_SET_WDS(n->bar.pmrcap, 1);
> -    NVME_PMRCAP_SET_BIR(n->bar.pmrcap, NVME_PMR_BIR);
> -    /* Turn on bit 1 support */
> -    NVME_PMRCAP_SET_PMRWBM(n->bar.pmrcap, 0x02);
> -    NVME_PMRCAP_SET_CMSS(n->bar.pmrcap, 1);
> +    uint32_t pmrcap = 0;
>
> -    pci_register_bar(pci_dev, NVME_PMRCAP_BIR(n->bar.pmrcap),
> +    NVME_PMRCAP_SET_RDS(pmrcap, 1);
> +    NVME_PMRCAP_SET_WDS(pmrcap, 1);
> +    NVME_PMRCAP_SET_BIR(pmrcap, NVME_PMR_BIR);
> +    /* Turn on bit 1 support */
> +    NVME_PMRCAP_SET_PMRWBM(pmrcap, 0x02);
> +    NVME_PMRCAP_SET_CMSS(pmrcap, 1);
> +    stl_le_p(&n->bar.pmrcap, pmrcap);
> +
> +    pci_register_bar(pci_dev, NVME_PMR_BIR,
>                       PCI_BASE_ADDRESS_SPACE_MEMORY |
>                       PCI_BASE_ADDRESS_MEM_TYPE_64 |
>                       PCI_BASE_ADDRESS_MEM_PREFETCH, &n->pmr.dev->mr);

Again, this function is changing from "set these fields" to
"set these fields and zero the rest".

> @@ -6362,6 +6406,7 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
>  {
>      NvmeIdCtrl *id = &n->id_ctrl;
>      uint8_t *pci_conf = pci_dev->config;
> +    uint64_t cap = 0;
>
>      id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
>      id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
> @@ -6440,17 +6485,18 @@ static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
>          id->cmic |= NVME_CMIC_MULTI_CTRL;
>      }
>
> -    NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
> -    NVME_CAP_SET_CQR(n->bar.cap, 1);
> -    NVME_CAP_SET_TO(n->bar.cap, 0xf);
> -    NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_NVM);
> -    NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_CSI_SUPP);
> -    NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_ADMIN_ONLY);
> -    NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
> -    NVME_CAP_SET_CMBS(n->bar.cap, n->params.cmb_size_mb ? 1 : 0);
> -    NVME_CAP_SET_PMRS(n->bar.cap, n->pmr.dev ? 1 : 0);
> +    NVME_CAP_SET_MQES(cap, 0x7ff);
> +    NVME_CAP_SET_CQR(cap, 1);
> +    NVME_CAP_SET_TO(cap, 0xf);
> +    NVME_CAP_SET_CSS(cap, NVME_CAP_CSS_NVM);
> +    NVME_CAP_SET_CSS(cap, NVME_CAP_CSS_CSI_SUPP);
> +    NVME_CAP_SET_CSS(cap, NVME_CAP_CSS_ADMIN_ONLY);
> +    NVME_CAP_SET_MPSMAX(cap, 4);
> +    NVME_CAP_SET_CMBS(cap, n->params.cmb_size_mb ? 1 : 0);
> +    NVME_CAP_SET_PMRS(cap, n->pmr.dev ? 1 : 0);
> +    stq_le_p(&n->bar.cap, cap);

Same here.

> -    n->bar.vs = NVME_SPEC_VER;
> +    stl_le_p(&n->bar.vs, NVME_SPEC_VER);
>      n->bar.intmc = n->bar.intms = 0;
>  }

thanks
-- PMM
diff mbox series

Patch

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 0449cc4dee9b..6d06ed990f2d 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -439,10 +439,12 @@  static uint8_t nvme_sq_empty(NvmeSQueue *sq)
 
 static void nvme_irq_check(NvmeCtrl *n)
 {
+    uint32_t intms = ldl_le_p(&n->bar.intms);
+
     if (msix_enabled(&(n->parent_obj))) {
         return;
     }
-    if (~n->bar.intms & n->irq_status) {
+    if (~intms & n->irq_status) {
         pci_irq_assert(&n->parent_obj);
     } else {
         pci_irq_deassert(&n->parent_obj);
@@ -1289,7 +1291,7 @@  static void nvme_post_cqes(void *opaque)
         if (ret) {
             trace_pci_nvme_err_addr_write(addr);
             trace_pci_nvme_err_cfs();
-            n->bar.csts = NVME_CSTS_FAILED;
+            stl_le_p(&n->bar.csts, NVME_CSTS_FAILED);
             break;
         }
         QTAILQ_REMOVE(&cq->req_list, req, entry);
@@ -4022,7 +4024,7 @@  static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeRequest *req)
         trace_pci_nvme_err_invalid_create_sq_sqid(sqid);
         return NVME_INVALID_QID | NVME_DNR;
     }
-    if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
+    if (unlikely(!qsize || qsize > NVME_CAP_MQES(ldq_le_p(&n->bar.cap)))) {
         trace_pci_nvme_err_invalid_create_sq_size(qsize);
         return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
     }
@@ -4208,7 +4210,7 @@  static uint16_t nvme_cmd_effects(NvmeCtrl *n, uint8_t csi, uint32_t buf_len,
         return NVME_INVALID_FIELD | NVME_DNR;
     }
 
-    switch (NVME_CC_CSS(n->bar.cc)) {
+    switch (NVME_CC_CSS(ldl_le_p(&n->bar.cc))) {
     case NVME_CC_CSS_NVM:
         src_iocs = nvme_cse_iocs_nvm;
         /* fall through */
@@ -4370,7 +4372,7 @@  static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeRequest *req)
         trace_pci_nvme_err_invalid_create_cq_cqid(cqid);
         return NVME_INVALID_QID | NVME_DNR;
     }
-    if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
+    if (unlikely(!qsize || qsize > NVME_CAP_MQES(ldq_le_p(&n->bar.cap)))) {
         trace_pci_nvme_err_invalid_create_cq_size(qsize);
         return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
     }
@@ -5163,17 +5165,19 @@  static void nvme_update_dmrsl(NvmeCtrl *n)
 
 static void nvme_select_iocs_ns(NvmeCtrl *n, NvmeNamespace *ns)
 {
+    uint32_t cc = ldl_le_p(&n->bar.cc);
+
     ns->iocs = nvme_cse_iocs_none;
     switch (ns->csi) {
     case NVME_CSI_NVM:
-        if (NVME_CC_CSS(n->bar.cc) != NVME_CC_CSS_ADMIN_ONLY) {
+        if (NVME_CC_CSS(cc) != NVME_CC_CSS_ADMIN_ONLY) {
             ns->iocs = nvme_cse_iocs_nvm;
         }
         break;
     case NVME_CSI_ZONED:
-        if (NVME_CC_CSS(n->bar.cc) == NVME_CC_CSS_CSI) {
+        if (NVME_CC_CSS(cc) == NVME_CC_CSS_CSI) {
             ns->iocs = nvme_cse_iocs_zoned;
-        } else if (NVME_CC_CSS(n->bar.cc) == NVME_CC_CSS_NVM) {
+        } else if (NVME_CC_CSS(cc) == NVME_CC_CSS_NVM) {
             ns->iocs = nvme_cse_iocs_nvm;
         }
         break;
@@ -5510,7 +5514,7 @@  static void nvme_process_sq(void *opaque)
         if (nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd))) {
             trace_pci_nvme_err_addr_read(addr);
             trace_pci_nvme_err_cfs();
-            n->bar.csts = NVME_CSTS_FAILED;
+            stl_le_p(&n->bar.csts, NVME_CSTS_FAILED);
             break;
         }
         nvme_inc_sq_head(sq);
@@ -5565,8 +5569,6 @@  static void nvme_ctrl_reset(NvmeCtrl *n)
     n->aer_queued = 0;
     n->outstanding_aers = 0;
     n->qs_created = false;
-
-    n->bar.cc = 0;
 }
 
 static void nvme_ctrl_shutdown(NvmeCtrl *n)
@@ -5605,7 +5607,12 @@  static void nvme_select_iocs(NvmeCtrl *n)
 
 static int nvme_start_ctrl(NvmeCtrl *n)
 {
-    uint32_t page_bits = NVME_CC_MPS(n->bar.cc) + 12;
+    uint64_t cap = ldq_le_p(&n->bar.cap);
+    uint32_t cc = ldl_le_p(&n->bar.cc);
+    uint32_t aqa = ldl_le_p(&n->bar.aqa);
+    uint64_t asq = ldq_le_p(&n->bar.asq);
+    uint64_t acq = ldq_le_p(&n->bar.acq);
+    uint32_t page_bits = NVME_CC_MPS(cc) + 12;
     uint32_t page_size = 1 << page_bits;
 
     if (unlikely(n->cq[0])) {
@@ -5616,73 +5623,72 @@  static int nvme_start_ctrl(NvmeCtrl *n)
         trace_pci_nvme_err_startfail_sq();
         return -1;
     }
-    if (unlikely(!n->bar.asq)) {
+    if (unlikely(!asq)) {
         trace_pci_nvme_err_startfail_nbarasq();
         return -1;
     }
-    if (unlikely(!n->bar.acq)) {
+    if (unlikely(!acq)) {
         trace_pci_nvme_err_startfail_nbaracq();
         return -1;
     }
-    if (unlikely(n->bar.asq & (page_size - 1))) {
-        trace_pci_nvme_err_startfail_asq_misaligned(n->bar.asq);
+    if (unlikely(asq & (page_size - 1))) {
+        trace_pci_nvme_err_startfail_asq_misaligned(asq);
         return -1;
     }
-    if (unlikely(n->bar.acq & (page_size - 1))) {
-        trace_pci_nvme_err_startfail_acq_misaligned(n->bar.acq);
+    if (unlikely(acq & (page_size - 1))) {
+        trace_pci_nvme_err_startfail_acq_misaligned(acq);
         return -1;
     }
-    if (unlikely(!(NVME_CAP_CSS(n->bar.cap) & (1 << NVME_CC_CSS(n->bar.cc))))) {
-        trace_pci_nvme_err_startfail_css(NVME_CC_CSS(n->bar.cc));
+    if (unlikely(!(NVME_CAP_CSS(cap) & (1 << NVME_CC_CSS(cc))))) {
+        trace_pci_nvme_err_startfail_css(NVME_CC_CSS(cc));
         return -1;
     }
-    if (unlikely(NVME_CC_MPS(n->bar.cc) <
-                 NVME_CAP_MPSMIN(n->bar.cap))) {
+    if (unlikely(NVME_CC_MPS(cc) < NVME_CAP_MPSMIN(cap))) {
         trace_pci_nvme_err_startfail_page_too_small(
-                    NVME_CC_MPS(n->bar.cc),
-                    NVME_CAP_MPSMIN(n->bar.cap));
+                    NVME_CC_MPS(cc),
+                    NVME_CAP_MPSMIN(cap));
         return -1;
     }
-    if (unlikely(NVME_CC_MPS(n->bar.cc) >
-                 NVME_CAP_MPSMAX(n->bar.cap))) {
+    if (unlikely(NVME_CC_MPS(cc) >
+                 NVME_CAP_MPSMAX(cap))) {
         trace_pci_nvme_err_startfail_page_too_large(
-                    NVME_CC_MPS(n->bar.cc),
-                    NVME_CAP_MPSMAX(n->bar.cap));
+                    NVME_CC_MPS(cc),
+                    NVME_CAP_MPSMAX(cap));
         return -1;
     }
-    if (unlikely(NVME_CC_IOCQES(n->bar.cc) <
+    if (unlikely(NVME_CC_IOCQES(cc) <
                  NVME_CTRL_CQES_MIN(n->id_ctrl.cqes))) {
         trace_pci_nvme_err_startfail_cqent_too_small(
-                    NVME_CC_IOCQES(n->bar.cc),
-                    NVME_CTRL_CQES_MIN(n->bar.cap));
+                    NVME_CC_IOCQES(cc),
+                    NVME_CTRL_CQES_MIN(cap));
         return -1;
     }
-    if (unlikely(NVME_CC_IOCQES(n->bar.cc) >
+    if (unlikely(NVME_CC_IOCQES(cc) >
                  NVME_CTRL_CQES_MAX(n->id_ctrl.cqes))) {
         trace_pci_nvme_err_startfail_cqent_too_large(
-                    NVME_CC_IOCQES(n->bar.cc),
-                    NVME_CTRL_CQES_MAX(n->bar.cap));
+                    NVME_CC_IOCQES(cc),
+                    NVME_CTRL_CQES_MAX(cap));
         return -1;
     }
-    if (unlikely(NVME_CC_IOSQES(n->bar.cc) <
+    if (unlikely(NVME_CC_IOSQES(cc) <
                  NVME_CTRL_SQES_MIN(n->id_ctrl.sqes))) {
         trace_pci_nvme_err_startfail_sqent_too_small(
-                    NVME_CC_IOSQES(n->bar.cc),
-                    NVME_CTRL_SQES_MIN(n->bar.cap));
+                    NVME_CC_IOSQES(cc),
+                    NVME_CTRL_SQES_MIN(cap));
         return -1;
     }
-    if (unlikely(NVME_CC_IOSQES(n->bar.cc) >
+    if (unlikely(NVME_CC_IOSQES(cc) >
                  NVME_CTRL_SQES_MAX(n->id_ctrl.sqes))) {
         trace_pci_nvme_err_startfail_sqent_too_large(
-                    NVME_CC_IOSQES(n->bar.cc),
-                    NVME_CTRL_SQES_MAX(n->bar.cap));
+                    NVME_CC_IOSQES(cc),
+                    NVME_CTRL_SQES_MAX(cap));
         return -1;
     }
-    if (unlikely(!NVME_AQA_ASQS(n->bar.aqa))) {
+    if (unlikely(!NVME_AQA_ASQS(aqa))) {
         trace_pci_nvme_err_startfail_asqent_sz_zero();
         return -1;
     }
-    if (unlikely(!NVME_AQA_ACQS(n->bar.aqa))) {
+    if (unlikely(!NVME_AQA_ACQS(aqa))) {
         trace_pci_nvme_err_startfail_acqent_sz_zero();
         return -1;
     }
@@ -5690,12 +5696,10 @@  static int nvme_start_ctrl(NvmeCtrl *n)
     n->page_bits = page_bits;
     n->page_size = page_size;
     n->max_prp_ents = n->page_size / sizeof(uint64_t);
-    n->cqe_size = 1 << NVME_CC_IOCQES(n->bar.cc);
-    n->sqe_size = 1 << NVME_CC_IOSQES(n->bar.cc);
-    nvme_init_cq(&n->admin_cq, n, n->bar.acq, 0, 0,
-                 NVME_AQA_ACQS(n->bar.aqa) + 1, 1);
-    nvme_init_sq(&n->admin_sq, n, n->bar.asq, 0, 0,
-                 NVME_AQA_ASQS(n->bar.aqa) + 1);
+    n->cqe_size = 1 << NVME_CC_IOCQES(cc);
+    n->sqe_size = 1 << NVME_CC_IOSQES(cc);
+    nvme_init_cq(&n->admin_cq, n, acq, 0, 0, NVME_AQA_ACQS(aqa) + 1, 1);
+    nvme_init_sq(&n->admin_sq, n, asq, 0, 0, NVME_AQA_ASQS(aqa) + 1);
 
     nvme_set_timestamp(n, 0ULL);
 
@@ -5708,22 +5712,38 @@  static int nvme_start_ctrl(NvmeCtrl *n)
 
 static void nvme_cmb_enable_regs(NvmeCtrl *n)
 {
-    NVME_CMBLOC_SET_CDPCILS(n->bar.cmbloc, 1);
-    NVME_CMBLOC_SET_CDPMLS(n->bar.cmbloc, 1);
-    NVME_CMBLOC_SET_BIR(n->bar.cmbloc, NVME_CMB_BIR);
+    uint32_t cmbloc = 0, cmbsz = 0;
 
-    NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1);
-    NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0);
-    NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 1);
-    NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 1);
-    NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 1);
-    NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */
-    NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->params.cmb_size_mb);
+    NVME_CMBLOC_SET_CDPCILS(cmbloc, 1);
+    NVME_CMBLOC_SET_CDPMLS(cmbloc, 1);
+    NVME_CMBLOC_SET_BIR(cmbloc, NVME_CMB_BIR);
+    stl_le_p(&n->bar.cmbloc, cmbloc);
+
+    NVME_CMBSZ_SET_SQS(cmbsz, 1);
+    NVME_CMBSZ_SET_CQS(cmbsz, 0);
+    NVME_CMBSZ_SET_LISTS(cmbsz, 1);
+    NVME_CMBSZ_SET_RDS(cmbsz, 1);
+    NVME_CMBSZ_SET_WDS(cmbsz, 1);
+    NVME_CMBSZ_SET_SZU(cmbsz, 2); /* MBs */
+    NVME_CMBSZ_SET_SZ(cmbsz, n->params.cmb_size_mb);
+    stl_le_p(&n->bar.cmbsz, cmbsz);
 }
 
 static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
                            unsigned size)
 {
+    uint64_t cap = ldq_le_p(&n->bar.cap);
+    uint32_t cc = ldl_le_p(&n->bar.cc);
+    uint32_t intms = ldl_le_p(&n->bar.intms);
+    uint32_t csts = ldl_le_p(&n->bar.csts);
+    uint64_t asq = ldq_le_p(&n->bar.asq);
+    uint64_t acq = ldq_le_p(&n->bar.acq);
+    uint64_t cmbmsc = ldq_le_p(&n->bar.cmbmsc);
+    uint32_t cmbsts = ldl_le_p(&n->bar.cmbsts);
+    uint32_t pmrsts = ldl_le_p(&n->bar.pmrsts);
+    uint32_t pmrmscl = ldl_le_p(&n->bar.pmrmscl);
+    uint32_t pmrmscu = ldl_le_p(&n->bar.pmrmscu);
+
     if (unlikely(offset & (sizeof(uint32_t) - 1))) {
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_misaligned32,
                        "MMIO write not 32-bit aligned,"
@@ -5747,9 +5767,10 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
                            " when MSI-X is enabled");
             /* should be ignored, fall through for now */
         }
-        n->bar.intms |= data & 0xffffffff;
+        intms |= data & 0xffffffff;
+        stl_le_p(&n->bar.intms, intms);
         n->bar.intmc = n->bar.intms;
-        trace_pci_nvme_mmio_intm_set(data & 0xffffffff, n->bar.intmc);
+        trace_pci_nvme_mmio_intm_set(data & 0xffffffff, intms);
         nvme_irq_check(n);
         break;
     case NVME_REG_INTMC:
@@ -5759,44 +5780,55 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
                            " when MSI-X is enabled");
             /* should be ignored, fall through for now */
         }
-        n->bar.intms &= ~(data & 0xffffffff);
+        intms &= ~(data & 0xffffffff);
+        stl_le_p(&n->bar.intms, intms);
         n->bar.intmc = n->bar.intms;
-        trace_pci_nvme_mmio_intm_clr(data & 0xffffffff, n->bar.intmc);
+        trace_pci_nvme_mmio_intm_clr(data & 0xffffffff, intms);
         nvme_irq_check(n);
         break;
     case NVME_REG_CC:
         trace_pci_nvme_mmio_cfg(data & 0xffffffff);
+
         /* Windows first sends data, then sends enable bit */
-        if (!NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc) &&
-            !NVME_CC_SHN(data) && !NVME_CC_SHN(n->bar.cc))
+        if (!NVME_CC_EN(data) && !NVME_CC_EN(cc) &&
+            !NVME_CC_SHN(data) && !NVME_CC_SHN(cc))
         {
-            n->bar.cc = data;
+            cc = data;
         }
 
-        if (NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc)) {
-            n->bar.cc = data;
+        if (NVME_CC_EN(data) && !NVME_CC_EN(cc)) {
+            cc = data;
+
+            /* flush CC since nvme_start_ctrl() needs the value */
+            stl_le_p(&n->bar.cc, cc);
             if (unlikely(nvme_start_ctrl(n))) {
                 trace_pci_nvme_err_startfail();
-                n->bar.csts = NVME_CSTS_FAILED;
+                csts = NVME_CSTS_FAILED;
             } else {
                 trace_pci_nvme_mmio_start_success();
-                n->bar.csts = NVME_CSTS_READY;
+                csts = NVME_CSTS_READY;
             }
-        } else if (!NVME_CC_EN(data) && NVME_CC_EN(n->bar.cc)) {
+        } else if (!NVME_CC_EN(data) && NVME_CC_EN(cc)) {
             trace_pci_nvme_mmio_stopped();
             nvme_ctrl_reset(n);
-            n->bar.csts &= ~NVME_CSTS_READY;
+            cc = 0;
+            csts &= ~NVME_CSTS_READY;
         }
-        if (NVME_CC_SHN(data) && !(NVME_CC_SHN(n->bar.cc))) {
+
+        if (NVME_CC_SHN(data) && !(NVME_CC_SHN(cc))) {
             trace_pci_nvme_mmio_shutdown_set();
             nvme_ctrl_shutdown(n);
-            n->bar.cc = data;
-            n->bar.csts |= NVME_CSTS_SHST_COMPLETE;
-        } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(n->bar.cc)) {
+            cc = data;
+            csts |= NVME_CSTS_SHST_COMPLETE;
+        } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(cc)) {
             trace_pci_nvme_mmio_shutdown_cleared();
-            n->bar.csts &= ~NVME_CSTS_SHST_COMPLETE;
-            n->bar.cc = data;
+            csts &= ~NVME_CSTS_SHST_COMPLETE;
+            cc = data;
         }
+
+        stl_le_p(&n->bar.cc, cc);
+        stl_le_p(&n->bar.csts, csts);
+
         break;
     case NVME_REG_CSTS:
         if (data & (1 << 4)) {
@@ -5818,26 +5850,30 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         }
         break;
     case NVME_REG_AQA:
-        n->bar.aqa = data & 0xffffffff;
+        stl_le_p(&n->bar.aqa, data & 0xffffffff);
         trace_pci_nvme_mmio_aqattr(data & 0xffffffff);
         break;
     case NVME_REG_ASQ:
-        n->bar.asq = size == 8 ? data :
-            (n->bar.asq & ~0xffffffffULL) | (data & 0xffffffff);
+        asq = size == 8 ? data :
+            (asq & ~0xffffffffULL) | (data & 0xffffffff);
+        stq_le_p(&n->bar.asq, asq);
         trace_pci_nvme_mmio_asqaddr(data);
         break;
     case NVME_REG_ASQ + 4:
-        n->bar.asq = (n->bar.asq & 0xffffffff) | (data << 32);
-        trace_pci_nvme_mmio_asqaddr_hi(data, n->bar.asq);
+        asq = (asq & 0xffffffff) | (data << 32);
+        stq_le_p(&n->bar.asq, asq);
+        trace_pci_nvme_mmio_asqaddr_hi(data, asq);
         break;
     case NVME_REG_ACQ:
         trace_pci_nvme_mmio_acqaddr(data);
-        n->bar.acq = size == 8 ? data :
-            (n->bar.acq & ~0xffffffffULL) | (data & 0xffffffff);
+        acq = size == 8 ? data :
+            (acq & ~0xffffffffULL) | (data & 0xffffffff);
+        stq_le_p(&n->bar.acq, acq);
         break;
     case NVME_REG_ACQ + 4:
-        n->bar.acq = (n->bar.acq & 0xffffffff) | (data << 32);
-        trace_pci_nvme_mmio_acqaddr_hi(data, n->bar.acq);
+        acq = (acq & 0xffffffff) | (data << 32);
+        stq_le_p(&n->bar.acq, acq);
+        trace_pci_nvme_mmio_acqaddr_hi(data, acq);
         break;
     case NVME_REG_CMBLOC:
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_cmbloc_reserved,
@@ -5849,12 +5885,13 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
                        "invalid write to read only CMBSZ, ignored");
         return;
     case NVME_REG_CMBMSC:
-        if (!NVME_CAP_CMBS(n->bar.cap)) {
+        if (!NVME_CAP_CMBS(cap)) {
             return;
         }
 
-        n->bar.cmbmsc = size == 8 ? data :
-            (n->bar.cmbmsc & ~0xffffffff) | (data & 0xffffffff);
+        cmbmsc = size == 8 ? data :
+            (cmbmsc & ~0xffffffff) | (data & 0xffffffff);
+        stq_le_p(&n->bar.cmbmsc, cmbmsc);
         n->cmb.cmse = false;
 
         if (NVME_CMBMSC_CRE(data)) {
@@ -5863,7 +5900,8 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
             if (NVME_CMBMSC_CMSE(data)) {
                 hwaddr cba = NVME_CMBMSC_CBA(data) << CMBMSC_CBA_SHIFT;
                 if (cba + int128_get64(n->cmb.mem.size) < cba) {
-                    NVME_CMBSTS_SET_CBAI(n->bar.cmbsts, 1);
+                    NVME_CMBSTS_SET_CBAI(cmbsts, 1);
+                    stl_le_p(&n->bar.cmbsts, cmbsts);
                     return;
                 }
 
@@ -5877,7 +5915,8 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
 
         return;
     case NVME_REG_CMBMSC + 4:
-        n->bar.cmbmsc = (n->bar.cmbmsc & 0xffffffff) | (data << 32);
+        cmbmsc = (cmbmsc & 0xffffffff) | (data << 32);
+        stq_le_p(&n->bar.cmbmsc, cmbmsc);
         return;
 
     case NVME_REG_PMRCAP:
@@ -5885,19 +5924,20 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
                        "invalid write to PMRCAP register, ignored");
         return;
     case NVME_REG_PMRCTL:
-        if (!NVME_CAP_PMRS(n->bar.cap)) {
+        if (!NVME_CAP_PMRS(cap)) {
             return;
         }
 
-        n->bar.pmrctl = data;
+        stl_le_p(&n->bar.pmrctl, data & 0xffffffff);
         if (NVME_PMRCTL_EN(data)) {
             memory_region_set_enabled(&n->pmr.dev->mr, true);
-            n->bar.pmrsts = 0;
+            pmrsts = 0;
         } else {
             memory_region_set_enabled(&n->pmr.dev->mr, false);
-            NVME_PMRSTS_SET_NRDY(n->bar.pmrsts, 1);
+            NVME_PMRSTS_SET_NRDY(pmrsts, 1);
             n->pmr.cmse = false;
         }
+        stl_le_p(&n->bar.pmrsts, pmrsts);
         return;
     case NVME_REG_PMRSTS:
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_pmrsts_readonly,
@@ -5912,18 +5952,20 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
                        "invalid write to PMRSWTP register, ignored");
         return;
     case NVME_REG_PMRMSCL:
-        if (!NVME_CAP_PMRS(n->bar.cap)) {
+        if (!NVME_CAP_PMRS(cap)) {
             return;
         }
 
-        n->bar.pmrmscl = data & 0xffffffff;
+        pmrmscl = data & 0xffffffff;
+        stl_le_p(&n->bar.pmrmscl, pmrmscl);
         n->pmr.cmse = false;
 
-        if (NVME_PMRMSCL_CMSE(n->bar.pmrmscl)) {
-            hwaddr cba = n->bar.pmrmscu |
-                (NVME_PMRMSCL_CBA(n->bar.pmrmscl) << PMRMSCL_CBA_SHIFT);
+        if (NVME_PMRMSCL_CMSE(data)) {
+            hwaddr cba = pmrmscu |
+                (NVME_PMRMSCL_CBA(pmrmscl) << PMRMSCL_CBA_SHIFT);
             if (cba + int128_get64(n->pmr.dev->mr.size) < cba) {
-                NVME_PMRSTS_SET_CBAI(n->bar.pmrsts, 1);
+                NVME_PMRSTS_SET_CBAI(pmrsts, 1);
+                stl_le_p(&n->bar.pmrsts, pmrsts);
                 return;
             }
 
@@ -5933,11 +5975,11 @@  static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
 
         return;
     case NVME_REG_PMRMSCU:
-        if (!NVME_CAP_PMRS(n->bar.cap)) {
+        if (!NVME_CAP_PMRS(cap)) {
             return;
         }
 
-        n->bar.pmrmscu = data & 0xffffffff;
+        stl_le_p(&n->bar.pmrmscu, data & 0xffffffff);
         return;
     default:
         NVME_GUEST_ERR(pci_nvme_ub_mmiowr_invalid,
@@ -5952,7 +5994,6 @@  static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
 {
     NvmeCtrl *n = (NvmeCtrl *)opaque;
     uint8_t *ptr = (uint8_t *)&n->bar;
-    uint64_t val = 0;
 
     trace_pci_nvme_mmio_read(addr, size);
 
@@ -5982,13 +6023,11 @@  static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
      * made it to persistent media
      */
     if (addr == NVME_REG_PMRSTS &&
-        (NVME_PMRCAP_PMRWBM(n->bar.pmrcap) & 0x02)) {
+        (NVME_PMRCAP_PMRWBM(ldl_le_p(&n->bar.pmrcap)) & 0x02)) {
         memory_region_msync(&n->pmr.dev->mr, 0, n->pmr.dev->size);
     }
 
-    memcpy(&val, ptr + addr, size);
-
-    return val;
+    return ldn_le_p(ptr + addr, size);
 }
 
 static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
@@ -6246,6 +6285,7 @@  static void nvme_init_state(NvmeCtrl *n)
 static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
 {
     uint64_t cmb_size = n->params.cmb_size_mb * MiB;
+    uint64_t cap = ldq_le_p(&n->bar.cap);
 
     n->cmb.buf = g_malloc0(cmb_size);
     memory_region_init_io(&n->cmb.mem, OBJECT(n), &nvme_cmb_ops, n,
@@ -6255,7 +6295,8 @@  static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
                      PCI_BASE_ADDRESS_MEM_TYPE_64 |
                      PCI_BASE_ADDRESS_MEM_PREFETCH, &n->cmb.mem);
 
-    NVME_CAP_SET_CMBS(n->bar.cap, 1);
+    NVME_CAP_SET_CMBS(cap, 1);
+    stq_le_p(&n->bar.cap, cap);
 
     if (n->params.legacy_cmb) {
         nvme_cmb_enable_regs(n);
@@ -6265,14 +6306,17 @@  static void nvme_init_cmb(NvmeCtrl *n, PCIDevice *pci_dev)
 
 static void nvme_init_pmr(NvmeCtrl *n, PCIDevice *pci_dev)
 {
-    NVME_PMRCAP_SET_RDS(n->bar.pmrcap, 1);
-    NVME_PMRCAP_SET_WDS(n->bar.pmrcap, 1);
-    NVME_PMRCAP_SET_BIR(n->bar.pmrcap, NVME_PMR_BIR);
-    /* Turn on bit 1 support */
-    NVME_PMRCAP_SET_PMRWBM(n->bar.pmrcap, 0x02);
-    NVME_PMRCAP_SET_CMSS(n->bar.pmrcap, 1);
+    uint32_t pmrcap = 0;
 
-    pci_register_bar(pci_dev, NVME_PMRCAP_BIR(n->bar.pmrcap),
+    NVME_PMRCAP_SET_RDS(pmrcap, 1);
+    NVME_PMRCAP_SET_WDS(pmrcap, 1);
+    NVME_PMRCAP_SET_BIR(pmrcap, NVME_PMR_BIR);
+    /* Turn on bit 1 support */
+    NVME_PMRCAP_SET_PMRWBM(pmrcap, 0x02);
+    NVME_PMRCAP_SET_CMSS(pmrcap, 1);
+    stl_le_p(&n->bar.pmrcap, pmrcap);
+
+    pci_register_bar(pci_dev, NVME_PMR_BIR,
                      PCI_BASE_ADDRESS_SPACE_MEMORY |
                      PCI_BASE_ADDRESS_MEM_TYPE_64 |
                      PCI_BASE_ADDRESS_MEM_PREFETCH, &n->pmr.dev->mr);
@@ -6362,6 +6406,7 @@  static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
 {
     NvmeIdCtrl *id = &n->id_ctrl;
     uint8_t *pci_conf = pci_dev->config;
+    uint64_t cap = 0;
 
     id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
     id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
@@ -6440,17 +6485,18 @@  static void nvme_init_ctrl(NvmeCtrl *n, PCIDevice *pci_dev)
         id->cmic |= NVME_CMIC_MULTI_CTRL;
     }
 
-    NVME_CAP_SET_MQES(n->bar.cap, 0x7ff);
-    NVME_CAP_SET_CQR(n->bar.cap, 1);
-    NVME_CAP_SET_TO(n->bar.cap, 0xf);
-    NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_NVM);
-    NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_CSI_SUPP);
-    NVME_CAP_SET_CSS(n->bar.cap, NVME_CAP_CSS_ADMIN_ONLY);
-    NVME_CAP_SET_MPSMAX(n->bar.cap, 4);
-    NVME_CAP_SET_CMBS(n->bar.cap, n->params.cmb_size_mb ? 1 : 0);
-    NVME_CAP_SET_PMRS(n->bar.cap, n->pmr.dev ? 1 : 0);
+    NVME_CAP_SET_MQES(cap, 0x7ff);
+    NVME_CAP_SET_CQR(cap, 1);
+    NVME_CAP_SET_TO(cap, 0xf);
+    NVME_CAP_SET_CSS(cap, NVME_CAP_CSS_NVM);
+    NVME_CAP_SET_CSS(cap, NVME_CAP_CSS_CSI_SUPP);
+    NVME_CAP_SET_CSS(cap, NVME_CAP_CSS_ADMIN_ONLY);
+    NVME_CAP_SET_MPSMAX(cap, 4);
+    NVME_CAP_SET_CMBS(cap, n->params.cmb_size_mb ? 1 : 0);
+    NVME_CAP_SET_PMRS(cap, n->pmr.dev ? 1 : 0);
+    stq_le_p(&n->bar.cap, cap);
 
-    n->bar.vs = NVME_SPEC_VER;
+    stl_le_p(&n->bar.vs, NVME_SPEC_VER);
     n->bar.intmc = n->bar.intms = 0;
 }
 
@@ -6601,7 +6647,7 @@  static void nvme_set_smart_warning(Object *obj, Visitor *v, const char *name,
 
     cap = NVME_SMART_SPARE | NVME_SMART_TEMPERATURE | NVME_SMART_RELIABILITY
           | NVME_SMART_MEDIA_READ_ONLY | NVME_SMART_FAILED_VOLATILE_MEDIA;
-    if (NVME_CAP_PMRS(n->bar.cap)) {
+    if (NVME_CAP_PMRS(ldq_le_p(&n->bar.cap))) {
         cap |= NVME_SMART_PMR_UNRELIABLE;
     }