nvme: PCI/e configuration from specification

index d5bf95b79b..9d5414c80f 100644
@@ -25,6 +25,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/range.h"
 #include "hw/block/block.h"
 #include "hw/hw.h"
 #include "hw/pci/msix.h"

@@ -39,6 +40,9 @@
 #include "trace.h"
 #include "nvme.h"
 
+#define PCI_EXP_LNKCAP_AOC 0x00400000 /* ASPM Optionality Compliance (AOC) */
+#define PCI_EXP_DEVCAP2_CTDS 0x10 /* Completion Timeout Disable Supported (CTDS) */
+
 #define NVME_GUEST_ERR(trace, fmt, ...) \
     do { \
         (trace_##trace)(__VA_ARGS__); \

@@ -1195,14 +1199,31 @@ static const MemoryRegionOps nvme_cmb_ops = {
     },
 };
 
+static uint32_t nvme_pci_read_config(PCIDevice *pci_dev, uint32_t addr, int len)
+{
+    uint32_t val; /* Value to be returned */
+
+    val = pci_default_read_config(pci_dev, addr, len);
+    if (ranges_overlap(addr, len, PCI_BASE_ADDRESS_2, 4) && (!(pci_dev->config[PCI_COMMAND] & PCI_COMMAND_IO))) {
+        /* When CMD.IOSE is not set */
+        val = 0 ;
+    }
+
+    return val;
+}
+
 static void nvme_realize(PCIDevice *pci_dev, Error **errp)
 {
+    static const uint16_t nvme_pm_offset = 0x80;
+    static const uint16_t nvme_pcie_offset = nvme_pm_offset + PCI_PM_SIZEOF;
+
     NvmeCtrl *n = NVME(pci_dev);
     NvmeIdCtrl *id = &n->id_ctrl;
 
     int i;
     int64_t bs_size;
     uint8_t *pci_conf;
+    uint8_t *pci_wmask;
 
     if (!n->conf.blk) {
         error_setg(errp, "drive property not set");

@@ -1226,14 +1247,43 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
     }
 
     pci_conf = pci_dev->config;
+    pci_wmask = pci_dev->wmask;
     pci_conf[PCI_INTERRUPT_PIN] = 1;
     pci_config_set_prog_interface(pci_dev->config, 0x2);
     pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
-    pcie_endpoint_cap_init(&n->parent_obj, 0x80);
+
+    // Configure the PMC capability
+    (void)pci_add_capability(pci_dev, PCI_CAP_ID_PM, nvme_pm_offset, PCI_PM_SIZEOF, errp);
+    if (NULL != *errp) {
+        return;
+    }
+
+    //  - PCI Power Management v1.2, No PME support, No Soft Reset, Make state writeable
+    pci_set_word(pci_conf + nvme_pm_offset + PCI_PM_PMC, PCI_PM_CAP_VER_1_2);
+    pci_set_word(pci_conf + nvme_pm_offset + PCI_PM_CTRL, PCI_PM_CTRL_NO_SOFT_RESET);
+    pci_set_word(pci_wmask + nvme_pm_offset + PCI_PM_CTRL, PCI_PM_CTRL_STATE_MASK);
+
+    // Disable QEMU default QEMU_PCIE_LNKSTA_DLLLA to disabled active flag in the Link Status Register of PCIE
+    pci_dev->cap_present &= ~(QEMU_PCIE_LNKSTA_DLLLA);
+
+    // PCIE Capability
+    pcie_endpoint_cap_init(&n->parent_obj, nvme_pcie_offset);
+
+    // PCIE Function Level Reset (FLRC) as required by 1.2 spec
+    pcie_cap_flr_init(&n->parent_obj);
+
+    // PCIE Configured with L0s by default by QEMU, configure missing AOC flag required by compliance
+    pci_long_test_and_set_mask(pci_conf + pci_dev->exp.exp_cap + PCI_EXP_LNKCAP, PCI_EXP_LNKCAP_AOC);
+
+    // Compliance requires Completion Timeout Disable Supported (CTDS).
+    pci_long_test_and_set_mask(pci_conf + pci_dev->exp.exp_cap + PCI_EXP_DEVCAP2, PCI_EXP_DEVCAP2_CTDS);
+
+    // Make the End-End TLP Prefix readonly as NVME spec doesnt acknowledge this field
+    pci_word_test_and_clear_mask(pci_wmask + pci_dev->exp.exp_cap + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_EETLPPB);
 
     n->num_namespaces = 1;
     n->num_queues = 64;
-    n->reg_size = pow2ceil(0x1004 + 2 * (n->num_queues + 1) * 4);
+    n->reg_size = pow2ceil(0x2000 + 2 * (n->num_queues + 1) * 4);
     n->ns_size = bs_size / (uint64_t)n->num_namespaces;
 
     n->namespaces = g_new0(NvmeNamespace, n->num_namespaces);

@@ -1245,6 +1295,10 @@ static void nvme_realize(PCIDevice *pci_dev, Error **errp)
     pci_register_bar(&n->parent_obj, 0,
         PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
         &n->iomem);
+
+    // Expose the NVME memory through Address Space IO (Optional by spec)
+    pci_register_bar(&n->parent_obj, 2, PCI_BASE_ADDRESS_SPACE_IO, &n->iomem);
+
     msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, NULL);
 
     id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));

@@ -1355,6 +1409,7 @@ static void nvme_class_init(ObjectClass *oc, void *data)
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
 
     pc->realize = nvme_realize;
+    pc->config_read = nvme_pci_read_config;
     pc->exit = nvme_exit;
     pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
     pc->vendor_id = PCI_VENDOR_ID_INTEL;

index 7a83142578..54dc918f54 100644
@@ -1,3 +1,4 @@
 #include "standard-headers/linux/pci_regs.h"
 
 #define  PCI_PM_CAP_VER_1_1     0x0002  /* PCI PM spec ver. 1.1 */
+#define  PCI_PM_CAP_VER_1_2     0x0003  /* PCI PM spec ver. 1.2 */

index 9d5414c80f..24a51d33ea 100644
@@ -62,14 +62,24 @@ static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
     }
 }
 
-static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid)
+static int nvme_valid_sqid(NvmeCtrl *n, uint16_t sqid)
 {
-    return sqid < n->num_queues && n->sq[sqid] != NULL ? 0 : -1;
+    return sqid < n->num_queues;
 }
 
-static int nvme_check_cqid(NvmeCtrl *n, uint16_t cqid)
+static int nvme_used_sqid(NvmeCtrl *n, uint16_t sqid)
 {
-    return cqid < n->num_queues && n->cq[cqid] != NULL ? 0 : -1;
+    return sqid < n->num_queues && n->sq[sqid] != NULL ? 1 : 0;
+}
+
+static int nvme_valid_cqid(NvmeCtrl *n, uint16_t cqid)
+{
+    return cqid < n->num_queues;
+}
+
+static int nvme_used_cqid(NvmeCtrl *n, uint16_t cqid)
+{
+    return cqid < n->num_queues && n->cq[cqid] != NULL ? 1 : 0;
 }
 
 static void nvme_inc_cq_tail(NvmeCQueue *cq)

@@ -433,7 +443,7 @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeCmd *cmd)
     NvmeCQueue *cq;
     uint16_t qid = le16_to_cpu(c->qid);
 
-    if (unlikely(!qid || nvme_check_sqid(n, qid))) {
+    if (unlikely(!qid || !nvme_used_sqid(n, qid))) {
         trace_nvme_err_invalid_del_sq(qid);
         return NVME_INVALID_QID | NVME_DNR;
     }

@@ -446,7 +456,7 @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeCmd *cmd)
         assert(req->aiocb);
         blk_aio_cancel(req->aiocb);
     }
-    if (!nvme_check_cqid(n, sq->cqid)) {
+    if (nvme_used_cqid(n, sq->cqid)) {
         cq = n->cq[sq->cqid];
         QTAILQ_REMOVE(&cq->sq_list, sq, entry);
 

@@ -504,11 +514,11 @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd *cmd)
 
     trace_nvme_create_sq(prp1, sqid, cqid, qsize, qflags);
 
-    if (unlikely(!cqid || nvme_check_cqid(n, cqid))) {
+    if (unlikely(!cqid || !nvme_used_cqid(n, cqid))) {
         trace_nvme_err_invalid_create_sq_cqid(cqid);
         return NVME_INVALID_CQID | NVME_DNR;
     }
-    if (unlikely(!sqid || !nvme_check_sqid(n, sqid))) {
+    if (unlikely(!sqid || !nvme_valid_sqid(n, sqid) || nvme_used_sqid(n, sqid))) {
         trace_nvme_err_invalid_create_sq_sqid(sqid);
         return NVME_INVALID_QID | NVME_DNR;
     }

@@ -546,9 +556,9 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeCmd *cmd)
     NvmeCQueue *cq;
     uint16_t qid = le16_to_cpu(c->qid);
 
-    if (unlikely(!qid || nvme_check_cqid(n, qid))) {
+    if (unlikely(!qid || !nvme_used_cqid(n, qid))) {
         trace_nvme_err_invalid_del_cq_cqid(qid);
-        return NVME_INVALID_CQID | NVME_DNR;
+        return NVME_INVALID_QID | NVME_DNR;
     }
 
     cq = n->cq[qid];

@@ -592,9 +602,9 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
     trace_nvme_create_cq(prp1, cqid, vector, qsize, qflags,
                          NVME_CQ_FLAGS_IEN(qflags) != 0);
 
-    if (unlikely(!cqid || !nvme_check_cqid(n, cqid))) {
+    if (unlikely(!cqid || !nvme_valid_cqid(n, cqid) || nvme_used_cqid(n, cqid))) {
         trace_nvme_err_invalid_create_cq_cqid(cqid);
-        return NVME_INVALID_CQID | NVME_DNR;
+        return NVME_INVALID_QID | NVME_DNR;
     }
     if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
         trace_nvme_err_invalid_create_cq_size(qsize);

@@ -604,7 +614,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
         trace_nvme_err_invalid_create_cq_addr(prp1);
         return NVME_INVALID_FIELD | NVME_DNR;
     }
-    if (unlikely(vector > n->num_queues)) {
+    if (unlikely(vector >= n->num_queues)) {
         trace_nvme_err_invalid_create_cq_vector(vector);
         return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
     }

@@ -1091,7 +1101,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
         NvmeCQueue *cq;
 
         qid = (addr - (0x1000 + (1 << 2))) >> 3;
-        if (unlikely(nvme_check_cqid(n, qid))) {
+        if (unlikely(!nvme_used_cqid(n, qid))) {
             NVME_GUEST_ERR(nvme_ub_db_wr_invalid_cq,
                            "completion queue doorbell write"
                            " for nonexistent queue,"

@@ -1129,7 +1139,7 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
         NvmeSQueue *sq;
 
         qid = (addr - 0x1000) >> 3;
-        if (unlikely(nvme_check_sqid(n, qid))) {
+        if (unlikely(!nvme_used_sqid(n, qid))) {
             NVME_GUEST_ERR(nvme_ub_db_wr_invalid_sq,
                            "submission queue doorbell write"
                            " for nonexistent queue,"

index 24a51d33ea..206d8428fd 100644
@@ -805,7 +805,7 @@ static void nvme_process_sq(void *opaque)
     }
 }
 
-static void nvme_clear_ctrl(NvmeCtrl *n)
+static void nvme_clear_ctrl(NvmeCtrl *n, bool reset)
 {
     int i;
 

@@ -820,8 +820,18 @@ static void nvme_clear_ctrl(NvmeCtrl *n)
         }
     }
 
+    if (reset) {
+        // Reset clears all except for AWA, ASW, ACQ
+        n->bar.cc = 0;
+        n->bar.csts = 0;
+    }
+
+    // Update the IRQ status
+    n->bar.intmc = n->bar.intms = 0;
+    n->irq_status = 0;
+    nvme_irq_check(n);
+
     blk_flush(n->conf.blk);
-    n->bar.cc = 0;
 }
 
 static int nvme_start_ctrl(NvmeCtrl *n)

@@ -963,16 +973,14 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         nvme_irq_check(n);
         break;
     case 0x14:  /* CC */
-        trace_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))
-        {
-            n->bar.cc = data;
-        }
+        trace_nvme_mmio_cfg(data & NVME_CC_WR_MASK);
+
+        uint32_t previous_cc = n->bar.cc;
+
+        // CC is all writeable
+        n->bar.cc = data & NVME_CC_WR_MASK;
 
-        if (NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc)) {
-            n->bar.cc = data;
+        if (NVME_CC_EN(data) && !NVME_CC_EN(previous_cc)) {
             if (unlikely(nvme_start_ctrl(n))) {
                 trace_nvme_err_startfail();
                 n->bar.csts = NVME_CSTS_FAILED;

@@ -980,21 +988,24 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
                 trace_nvme_mmio_start_success();
                 n->bar.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(previous_cc)) {
             trace_nvme_mmio_stopped();
-            nvme_clear_ctrl(n);
-            n->bar.csts &= ~NVME_CSTS_READY;
+            nvme_clear_ctrl(n, true);
+
         }
-        if (NVME_CC_SHN(data) && !(NVME_CC_SHN(n->bar.cc))) {
+
+        if (NVME_CC_SHN(data) && !(NVME_CC_SHN(previous_cc))) {
             trace_nvme_mmio_shutdown_set();
-            nvme_clear_ctrl(n);
-            n->bar.cc = data;
+            nvme_clear_ctrl(n, false);
             n->bar.csts |= NVME_CSTS_SHST_COMPLETE;
-        } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(n->bar.cc)) {
+
+        } else if (!NVME_CC_SHN(data) && NVME_CC_SHN(previous_cc)) {
             trace_nvme_mmio_shutdown_cleared();
             n->bar.csts &= ~NVME_CSTS_SHST_COMPLETE;
-            n->bar.cc = data;
+
         }
+
         break;
     case 0x1C:  /* CSTS */
         if (data & (1 << 4)) {

@@ -1016,23 +1027,23 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
         }
         break;
     case 0x24:  /* AQA */
-        n->bar.aqa = data & 0xffffffff;
-        trace_nvme_mmio_aqattr(data & 0xffffffff);
+        n->bar.aqa = data & NVME_AQA_WR_MASK;
+        trace_nvme_mmio_aqattr(n->bar.aqa);
         break;
     case 0x28:  /* ASQ */
-        n->bar.asq = data;
-        trace_nvme_mmio_asqaddr(data);
+        n->bar.asq = data & NVME_ASQ_WR_MASK;
+        trace_nvme_mmio_asqaddr(n->bar.asq);
         break;
     case 0x2c:  /* ASQ hi */
-        n->bar.asq |= data << 32;
+        n->bar.asq |= (data << 32) & NVME_ASQ_WR_MASK;
         trace_nvme_mmio_asqaddr_hi(data, n->bar.asq);
         break;
     case 0x30:  /* ACQ */
-        trace_nvme_mmio_acqaddr(data);
-        n->bar.acq = data;
+        n->bar.acq = data & NVME_ACQ_WR_MASK;
+        trace_nvme_mmio_acqaddr(n->bar.acq);
         break;
     case 0x34:  /* ACQ hi */
-        n->bar.acq |= data << 32;
+        n->bar.acq |= (data << 32) & NVME_ACQ_WR_MASK;
         trace_nvme_mmio_acqaddr_hi(data, n->bar.acq);
         break;
     case 0x38:  /* CMBLOC */

@@ -1390,7 +1401,7 @@ static void nvme_exit(PCIDevice *pci_dev)
 {
     NvmeCtrl *n = NVME(pci_dev);
 
-    nvme_clear_ctrl(n);
+    nvme_clear_ctrl(n, true);
     g_free(n->namespaces);
     g_free(n->cq);
     g_free(n->sq);

index 849a6f3fa3..4da6740543 100644
@@ -90,6 +90,11 @@ enum NvmeCcMask {
     CC_IOCQES_MASK  = 0xf,
 };
 
+#define NVME_CC_WR_MASK    (0x00fffff1)
+#define NVME_AQA_WR_MASK   (0x0fff0fff)
+#define NVME_ASQ_WR_MASK   (0xfffffffffffff000)
+#define NVME_ACQ_WR_MASK   (0xfffffffffffff000)
+
 #define NVME_CC_EN(cc)     ((cc >> CC_EN_SHIFT)     & CC_EN_MASK)
 #define NVME_CC_CSS(cc)    ((cc >> CC_CSS_SHIFT)    & CC_CSS_MASK)
 #define NVME_CC_MPS(cc)    ((cc >> CC_MPS_SHIFT)    & CC_MPS_MASK)

index 206d8428fd..f639d7ae73 100644
@@ -265,6 +265,11 @@ static void nvme_post_cqes(void *opaque)
     NvmeCtrl *n = cq->ctrl;
     NvmeRequest *req, *next;
 
+    // Fast path if nothing to be processed
+    if (QTAILQ_EMPTY(&cq->req_list)) {
+        return;
+    }
+
     QTAILQ_FOREACH_SAFE(req, &cq->req_list, entry, next) {
         NvmeSQueue *sq;
         hwaddr addr;

@@ -1130,14 +1135,21 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
             return;
         }
 
+        /*
+         CLARIFICATION
+         When CQ was full *before* the db write, nvme_post_cqes skipped processing of responses and
+         as a side effect SQ was unable to process new requests (As they are limited by size).
+         When CQ is cleared, spawn both processing of all CQs and SQs. As call to timer_mod
+         is serial, first handle the CQ to clear any pending requests and then clear the associated SQs.
+         */
         start_sqs = nvme_cq_full(cq) ? 1 : 0;
         cq->head = new_head;
         if (start_sqs) {
             NvmeSQueue *sq;
+            timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
             QTAILQ_FOREACH(sq, &cq->sq_list, entry) {
                 timer_mod(sq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
             }
-            timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
         }
 
         if (cq->tail == cq->head) {

index f639d7ae73..e2dca6e57f 100644
@@ -1152,8 +1152,12 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
             }
         }
 
+        // When we have more messages, we should make sure irq is asserted. When MSIx is used
+        // this will make sure another notification is sent to the guest.
         if (cq->tail == cq->head) {
             nvme_irq_deassert(n, cq);
+        } else {
+            nvme_irq_assert(n, cq);
         }
     } else {
         /* Submission queue doorbell write */