: /hw/nvme/ctrl error handling if descriptors are greater than 1024

[PATCH]: /hw/nvme/ctrl error handling if descriptors are greater than 1024

Posted by Padmakar Kalghatgi 2 years, 10 months ago

From: padmakar

 

if the number of descriptors or pages is more than 1024,

dma writes or reads will result in failure. Hence, we check

if the number of descriptors or pages is more than 1024

in the nvme module and return Internal Device error.

 

Signed-off-by: Padmakar Kalghatgi

---

hw/nvme/ctrl.c       | 14 ++++++++++++++

hw/nvme/trace-events |  1 +

2 files changed, 15 insertions(+)

 

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c

index 40a7efc..082592f 100644

--- a/hw/nvme/ctrl.c

+++ b/hw/nvme/ctrl.c

@@ -602,6 +602,20 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, NvmeSg *sg,
hwaddr addr, size_t len)

         return NVME_SUCCESS;

     }

+    /*

+     *  The QEMU has an inherent issue where in if the no.

+     *  of descriptors is more than 1024, it will result in

+     *  failure during the dma write or reads. Hence, we need

+     *  to return the error.

+     */

+

+    if (((sg->flags & NVME_SG_DMA) && ((sg->qsg.nsg + 1) > IOV_MAX)) ||

+        ((sg->iov.niov + 1) > IOV_MAX)) {

+            NVME_GUEST_ERR(pci_nvme_ub_sg_desc_toohigh,

+                       "number of descriptors is greater than 1024");

+            return NVME_INTERNAL_DEV_ERROR;

+    }

+

     trace_pci_nvme_map_addr(addr, len);

     if (nvme_addr_is_cmb(n, addr)) {

diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events

index ea33d0c..bfe1a3b 100644

--- a/hw/nvme/trace-events

+++ b/hw/nvme/trace-events

@@ -202,3 +202,4 @@ pci_nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t
new_head) "completion qu

pci_nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write
for nonexistent queue, sqid=%"PRIu32", ignoring"

pci_nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail)
"submission queue doorbell write value beyond queue size, sqid=%"PRIu32",
new_head=%"PRIu16", ignoring"

pci_nvme_ub_unknown_css_value(void) "unknown value in cc.css field"

+pci_nvme_ub_sg_desc_toohigh(void) "the number of sg descriptors is too
high"

-- 

2.7.0.windows.1

Re: [PATCH]: /hw/nvme/ctrl error handling if descriptors are greater than 1024

Posted by Klaus Jensen 2 years, 10 months ago

Hi Padmakar,

Patch looks good, but it got messed up by your MUA. You've previously 
contributed correctly formatted patches, so please revert to that method 
when sending patches in the future ;)

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

On Jul  6 16:13, Padmakar Kalghatgi wrote:
>From: padmakar
>
>
>
>if the number of descriptors or pages is more than 1024,
>
>dma writes or reads will result in failure. Hence, we check
>
>if the number of descriptors or pages is more than 1024
>
>in the nvme module and return Internal Device error.
>
>
>
>Signed-off-by: Padmakar Kalghatgi
>
>---
>
>hw/nvme/ctrl.c       | 14 ++++++++++++++
>
>hw/nvme/trace-events |  1 +
>
>2 files changed, 15 insertions(+)
>
>
>
>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>
>index 40a7efc..082592f 100644
>
>--- a/hw/nvme/ctrl.c
>
>+++ b/hw/nvme/ctrl.c
>
>@@ -602,6 +602,20 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, NvmeSg *sg,
>hwaddr addr, size_t len)
>
>         return NVME_SUCCESS;
>
>     }
>
>+    /*
>
>+     *  The QEMU has an inherent issue where in if the no.
>
>+     *  of descriptors is more than 1024, it will result in
>
>+     *  failure during the dma write or reads. Hence, we need
>
>+     *  to return the error.
>
>+     */
>
>+
>
>+    if (((sg->flags & NVME_SG_DMA) && ((sg->qsg.nsg + 1) > IOV_MAX)) ||
>
>+        ((sg->iov.niov + 1) > IOV_MAX)) {
>
>+            NVME_GUEST_ERR(pci_nvme_ub_sg_desc_toohigh,
>
>+                       "number of descriptors is greater than 1024");
>
>+            return NVME_INTERNAL_DEV_ERROR;
>
>+    }
>
>+
>
>     trace_pci_nvme_map_addr(addr, len);
>
>     if (nvme_addr_is_cmb(n, addr)) {
>
>diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
>
>index ea33d0c..bfe1a3b 100644
>
>--- a/hw/nvme/trace-events
>
>+++ b/hw/nvme/trace-events
>
>@@ -202,3 +202,4 @@ pci_nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t
>new_head) "completion qu
>
>pci_nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write
>for nonexistent queue, sqid=%"PRIu32", ignoring"
>
>pci_nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail)
>"submission queue doorbell write value beyond queue size, sqid=%"PRIu32",
>new_head=%"PRIu16", ignoring"
>
>pci_nvme_ub_unknown_css_value(void) "unknown value in cc.css field"
>
>+pci_nvme_ub_sg_desc_toohigh(void) "the number of sg descriptors is too
>high"
>
>-- 
>
>2.7.0.windows.1
>
>
>

Re: [PATCH]: /hw/nvme/ctrl error handling if descriptors are greater than 1024

Posted by Klaus Jensen 2 years, 9 months ago

On Jul  6 16:13, Padmakar Kalghatgi wrote:
>From: padmakar
>
>
>
>if the number of descriptors or pages is more than 1024,
>
>dma writes or reads will result in failure. Hence, we check
>
>if the number of descriptors or pages is more than 1024
>
>in the nvme module and return Internal Device error.
>
>
>
>Signed-off-by: Padmakar Kalghatgi
>
>---
>
>hw/nvme/ctrl.c       | 14 ++++++++++++++
>
>hw/nvme/trace-events |  1 +
>
>2 files changed, 15 insertions(+)
>
>
>
>diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
>
>index 40a7efc..082592f 100644
>
>--- a/hw/nvme/ctrl.c
>
>+++ b/hw/nvme/ctrl.c
>
>@@ -602,6 +602,20 @@ static uint16_t nvme_map_addr(NvmeCtrl *n, NvmeSg *sg,
>hwaddr addr, size_t len)
>
>         return NVME_SUCCESS;
>
>     }
>
>+    /*
>
>+     *  The QEMU has an inherent issue where in if the no.
>
>+     *  of descriptors is more than 1024, it will result in
>
>+     *  failure during the dma write or reads. Hence, we need
>
>+     *  to return the error.
>
>+     */
>
>+
>
>+    if (((sg->flags & NVME_SG_DMA) && ((sg->qsg.nsg + 1) > IOV_MAX)) ||
>
>+        ((sg->iov.niov + 1) > IOV_MAX)) {
>
>+            NVME_GUEST_ERR(pci_nvme_ub_sg_desc_toohigh,
>
>+                       "number of descriptors is greater than 1024");
>
>+            return NVME_INTERNAL_DEV_ERROR;
>
>+    }
>
>+
>
>     trace_pci_nvme_map_addr(addr, len);
>
>     if (nvme_addr_is_cmb(n, addr)) {
>
>diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
>
>index ea33d0c..bfe1a3b 100644
>
>--- a/hw/nvme/trace-events
>
>+++ b/hw/nvme/trace-events
>
>@@ -202,3 +202,4 @@ pci_nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t
>new_head) "completion qu
>
>pci_nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write
>for nonexistent queue, sqid=%"PRIu32", ignoring"
>
>pci_nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail)
>"submission queue doorbell write value beyond queue size, sqid=%"PRIu32",
>new_head=%"PRIu16", ignoring"
>
>pci_nvme_ub_unknown_css_value(void) "unknown value in cc.css field"
>
>+pci_nvme_ub_sg_desc_toohigh(void) "the number of sg descriptors is too
>high"
>
>-- 
>
>2.7.0.windows.1
>
>
>

Applied to nvme-next, but made the error message more generic (this also 
applied to PRPs).