diff mbox series

[v6,08/20] hw/block/nvme: allow use of any valid msix vector

Message ID 20200514044611.734782-9-its@irrelevant.dk
State New
Headers show
Series nvme: small fixes, refactoring and cleanups | expand

Commit Message

Klaus Jensen May 14, 2020, 4:45 a.m. UTC 
From: Klaus Jensen <k.jensen@samsung.com>

If the device uses MSI-X, any of the 2048 MSI-X interrupt vectors are
valid. If the device is not using MSI-X, vector will and can only be
zero at this point.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/block/nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé May 28, 2020, 3:44 p.m. UTC | #1 
On 5/14/20 6:45 AM, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> If the device uses MSI-X, any of the 2048 MSI-X interrupt vectors are
> valid. If the device is not using MSI-X, vector will and can only be
> zero at this point.
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Signed-off-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 c9d10df1f763..a2e6734c7a25 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -657,7 +657,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
>          trace_pci_nvme_err_invalid_create_cq_vector(vector);
>          return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
>      }
> -    if (unlikely(vector > n->params.num_queues)) {
> +    if (unlikely(vector > PCI_MSIX_FLAGS_QSIZE)) {
>          trace_pci_nvme_err_invalid_create_cq_vector(vector);
>          return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
>      }
> 

Is it worth adding this?

--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -613,6 +613,8 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeCmd *cmd)
 static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr,
     uint16_t cqid, uint16_t vector, uint16_t size, uint16_t irq_enabled)
 {
+    int ret;
+
     cq->ctrl = n;
     cq->cqid = cqid;
     cq->size = size;
@@ -623,7 +625,8 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl
*n, uint64_t dma_addr,
     cq->head = cq->tail = 0;
     QTAILQ_INIT(&cq->req_list);
     QTAILQ_INIT(&cq->sq_list);
-    msix_vector_use(&n->parent_obj, cq->vector);
+    ret = msix_vector_use(&n->parent_obj, cq->vector);
+    assert(ret == 0);
     n->cq[cqid] = cq;
     cq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_post_cqes, cq);
 }

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Philippe Mathieu-Daudé June 2, 2020, 4:06 p.m. UTC | #2 
On 5/28/20 5:44 PM, Philippe Mathieu-Daudé wrote:
> On 5/14/20 6:45 AM, Klaus Jensen wrote:
>> From: Klaus Jensen <k.jensen@samsung.com>
>>
>> If the device uses MSI-X, any of the 2048 MSI-X interrupt vectors are
>> valid. If the device is not using MSI-X, vector will and can only be
>> zero at this point.
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Signed-off-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 c9d10df1f763..a2e6734c7a25 100644
>> --- a/hw/block/nvme.c
>> +++ b/hw/block/nvme.c
>> @@ -657,7 +657,7 @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
>>          trace_pci_nvme_err_invalid_create_cq_vector(vector);
>>          return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
>>      }
>> -    if (unlikely(vector > n->params.num_queues)) {
>> +    if (unlikely(vector > PCI_MSIX_FLAGS_QSIZE)) {
>>          trace_pci_nvme_err_invalid_create_cq_vector(vector);
>>          return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
>>      }
>>
> 
> Is it worth adding this?
> 
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -613,6 +613,8 @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeCmd *cmd)
>  static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr,
>      uint16_t cqid, uint16_t vector, uint16_t size, uint16_t irq_enabled)
>  {
> +    int ret;
> +
>      cq->ctrl = n;
>      cq->cqid = cqid;
>      cq->size = size;
> @@ -623,7 +625,8 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl
> *n, uint64_t dma_addr,
>      cq->head = cq->tail = 0;
>      QTAILQ_INIT(&cq->req_list);
>      QTAILQ_INIT(&cq->sq_list);
> -    msix_vector_use(&n->parent_obj, cq->vector);
> +    ret = msix_vector_use(&n->parent_obj, cq->vector);
> +    assert(ret == 0);
>      n->cq[cqid] = cq;
>      cq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_post_cqes, cq);
>  }

FYI snippet sent as new patch:
https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg00404.html
diff mbox series

Patch

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index c9d10df1f763..a2e6734c7a25 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -657,7 +657,7 @@  static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
         trace_pci_nvme_err_invalid_create_cq_vector(vector);
         return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
     }
-    if (unlikely(vector > n->params.num_queues)) {
+    if (unlikely(vector > PCI_MSIX_FLAGS_QSIZE)) {
         trace_pci_nvme_err_invalid_create_cq_vector(vector);
         return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
     }