* [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe
@ 2017-12-18 5:00 Hikaru Nishida
2017-12-22 7:31 ` Hikaru Nishida
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Hikaru Nishida @ 2017-12-18 5:00 UTC (permalink / raw)
To: qemu-devel
Cc: Hikaru Nishida, Keith Busch, Kevin Wolf, Max Reitz,
open list:nvme
Pin-based interrupt of NVMe controller did not work properly
because using an obsolated function pci_irq_pulse().
To fix this, change to use pci_irq_assert() / pci_irq_deassert()
instead of pci_irq_pulse().
Signed-off-by: Hikaru Nishida <hikarupsp@gmail.com>
---
hw/block/nvme.c | 39 ++++++++++++++++++++++++++++++++++-----
hw/block/nvme.h | 1 +
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 441e21e..2d164fc 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -82,13 +82,40 @@ static uint8_t nvme_sq_empty(NvmeSQueue *sq)
return sq->head == sq->tail;
}
-static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
+static void nvme_irq_check(NvmeCtrl *n)
+{
+ if (msix_enabled(&(n->parent_obj))) {
+ return;
+ }
+ if (~n->bar.intms & n->irq_status) {
+ pci_irq_assert(&n->parent_obj);
+ } else {
+ pci_irq_deassert(&n->parent_obj);
+ }
+}
+
+static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
{
if (cq->irq_enabled) {
if (msix_enabled(&(n->parent_obj))) {
msix_notify(&(n->parent_obj), cq->vector);
} else {
- pci_irq_pulse(&n->parent_obj);
+ assert(cq->cqid < 64);
+ n->irq_status |= 1 << cq->cqid;
+ nvme_irq_check(n);
+ }
+ }
+}
+
+static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
+{
+ if (cq->irq_enabled) {
+ if (msix_enabled(&(n->parent_obj))) {
+ return;
+ } else {
+ assert(cq->cqid < 64);
+ n->irq_status &= ~(1 << cq->cqid);
+ nvme_irq_check(n);
}
}
}
@@ -220,7 +247,7 @@ static void nvme_post_cqes(void *opaque)
sizeof(req->cqe));
QTAILQ_INSERT_TAIL(&sq->req_list, req, entry);
}
- nvme_isr_notify(n, cq);
+ nvme_irq_assert(n, cq);
}
static void nvme_enqueue_req_completion(NvmeCQueue *cq, NvmeRequest *req)
@@ -753,10 +780,12 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
case 0xc:
n->bar.intms |= data & 0xffffffff;
n->bar.intmc = n->bar.intms;
+ nvme_irq_check(n);
break;
case 0x10:
n->bar.intms &= ~(data & 0xffffffff);
n->bar.intmc = n->bar.intms;
+ nvme_irq_check(n);
break;
case 0x14:
/* Windows first sends data, then sends enable bit */
@@ -851,8 +880,8 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
}
- if (cq->tail != cq->head) {
- nvme_isr_notify(n, cq);
+ if (cq->tail == cq->head) {
+ nvme_irq_deassert(n, cq);
}
} else {
uint16_t new_tail = val & 0xffff;
diff --git a/hw/block/nvme.h b/hw/block/nvme.h
index 6aab338..7b62dad 100644
--- a/hw/block/nvme.h
+++ b/hw/block/nvme.h
@@ -775,6 +775,7 @@ typedef struct NvmeCtrl {
uint32_t cmbsz;
uint32_t cmbloc;
uint8_t *cmbuf;
+ uint64_t irq_status;
char *serial;
NvmeNamespace *namespaces;
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe
2017-12-18 5:00 [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe Hikaru Nishida
@ 2017-12-22 7:31 ` Hikaru Nishida
2018-01-04 18:06 ` Hikaru Nishida
2018-01-04 18:20 ` Keith Busch
2018-01-08 15:28 ` Kevin Wolf
2 siblings, 1 reply; 6+ messages in thread
From: Hikaru Nishida @ 2017-12-22 7:31 UTC (permalink / raw)
To: qemu-devel
Cc: Hikaru Nishida, Keith Busch, Kevin Wolf, Max Reitz,
open list:nvme
ping
http://patchwork.ozlabs.org/patch/849786/
2017-12-18 14:00 GMT+09:00 Hikaru Nishida <hikarupsp@gmail.com>:
> Pin-based interrupt of NVMe controller did not work properly
> because using an obsolated function pci_irq_pulse().
> To fix this, change to use pci_irq_assert() / pci_irq_deassert()
> instead of pci_irq_pulse().
>
> Signed-off-by: Hikaru Nishida <hikarupsp@gmail.com>
> ---
> hw/block/nvme.c | 39 ++++++++++++++++++++++++++++++++++-----
> hw/block/nvme.h | 1 +
> 2 files changed, 35 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 441e21e..2d164fc 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -82,13 +82,40 @@ static uint8_t nvme_sq_empty(NvmeSQueue *sq)
> return sq->head == sq->tail;
> }
>
> -static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
> +static void nvme_irq_check(NvmeCtrl *n)
> +{
> + if (msix_enabled(&(n->parent_obj))) {
> + return;
> + }
> + if (~n->bar.intms & n->irq_status) {
> + pci_irq_assert(&n->parent_obj);
> + } else {
> + pci_irq_deassert(&n->parent_obj);
> + }
> +}
> +
> +static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
> {
> if (cq->irq_enabled) {
> if (msix_enabled(&(n->parent_obj))) {
> msix_notify(&(n->parent_obj), cq->vector);
> } else {
> - pci_irq_pulse(&n->parent_obj);
> + assert(cq->cqid < 64);
> + n->irq_status |= 1 << cq->cqid;
> + nvme_irq_check(n);
> + }
> + }
> +}
> +
> +static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
> +{
> + if (cq->irq_enabled) {
> + if (msix_enabled(&(n->parent_obj))) {
> + return;
> + } else {
> + assert(cq->cqid < 64);
> + n->irq_status &= ~(1 << cq->cqid);
> + nvme_irq_check(n);
> }
> }
> }
> @@ -220,7 +247,7 @@ static void nvme_post_cqes(void *opaque)
> sizeof(req->cqe));
> QTAILQ_INSERT_TAIL(&sq->req_list, req, entry);
> }
> - nvme_isr_notify(n, cq);
> + nvme_irq_assert(n, cq);
> }
>
> static void nvme_enqueue_req_completion(NvmeCQueue *cq, NvmeRequest *req)
> @@ -753,10 +780,12 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
> case 0xc:
> n->bar.intms |= data & 0xffffffff;
> n->bar.intmc = n->bar.intms;
> + nvme_irq_check(n);
> break;
> case 0x10:
> n->bar.intms &= ~(data & 0xffffffff);
> n->bar.intmc = n->bar.intms;
> + nvme_irq_check(n);
> break;
> case 0x14:
> /* Windows first sends data, then sends enable bit */
> @@ -851,8 +880,8 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
> timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
> }
>
> - if (cq->tail != cq->head) {
> - nvme_isr_notify(n, cq);
> + if (cq->tail == cq->head) {
> + nvme_irq_deassert(n, cq);
> }
> } else {
> uint16_t new_tail = val & 0xffff;
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 6aab338..7b62dad 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -775,6 +775,7 @@ typedef struct NvmeCtrl {
> uint32_t cmbsz;
> uint32_t cmbloc;
> uint8_t *cmbuf;
> + uint64_t irq_status;
>
> char *serial;
> NvmeNamespace *namespaces;
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe
2017-12-22 7:31 ` Hikaru Nishida
@ 2018-01-04 18:06 ` Hikaru Nishida
0 siblings, 0 replies; 6+ messages in thread
From: Hikaru Nishida @ 2018-01-04 18:06 UTC (permalink / raw)
To: qemu-devel
Cc: Hikaru Nishida, Keith Busch, Kevin Wolf, Max Reitz,
open list:nvme
ping...
2017-12-22 16:31 GMT+09:00 Hikaru Nishida <hikarupsp@gmail.com>:
> ping
> http://patchwork.ozlabs.org/patch/849786/
>
> 2017-12-18 14:00 GMT+09:00 Hikaru Nishida <hikarupsp@gmail.com>:
>> Pin-based interrupt of NVMe controller did not work properly
>> because using an obsolated function pci_irq_pulse().
>> To fix this, change to use pci_irq_assert() / pci_irq_deassert()
>> instead of pci_irq_pulse().
>>
>> Signed-off-by: Hikaru Nishida <hikarupsp@gmail.com>
>> ---
>> hw/block/nvme.c | 39 ++++++++++++++++++++++++++++++++++-----
>> hw/block/nvme.h | 1 +
>> 2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
>> index 441e21e..2d164fc 100644
>> --- a/hw/block/nvme.c
>> +++ b/hw/block/nvme.c
>> @@ -82,13 +82,40 @@ static uint8_t nvme_sq_empty(NvmeSQueue *sq)
>> return sq->head == sq->tail;
>> }
>>
>> -static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
>> +static void nvme_irq_check(NvmeCtrl *n)
>> +{
>> + if (msix_enabled(&(n->parent_obj))) {
>> + return;
>> + }
>> + if (~n->bar.intms & n->irq_status) {
>> + pci_irq_assert(&n->parent_obj);
>> + } else {
>> + pci_irq_deassert(&n->parent_obj);
>> + }
>> +}
>> +
>> +static void nvme_irq_assert(NvmeCtrl *n, NvmeCQueue *cq)
>> {
>> if (cq->irq_enabled) {
>> if (msix_enabled(&(n->parent_obj))) {
>> msix_notify(&(n->parent_obj), cq->vector);
>> } else {
>> - pci_irq_pulse(&n->parent_obj);
>> + assert(cq->cqid < 64);
>> + n->irq_status |= 1 << cq->cqid;
>> + nvme_irq_check(n);
>> + }
>> + }
>> +}
>> +
>> +static void nvme_irq_deassert(NvmeCtrl *n, NvmeCQueue *cq)
>> +{
>> + if (cq->irq_enabled) {
>> + if (msix_enabled(&(n->parent_obj))) {
>> + return;
>> + } else {
>> + assert(cq->cqid < 64);
>> + n->irq_status &= ~(1 << cq->cqid);
>> + nvme_irq_check(n);
>> }
>> }
>> }
>> @@ -220,7 +247,7 @@ static void nvme_post_cqes(void *opaque)
>> sizeof(req->cqe));
>> QTAILQ_INSERT_TAIL(&sq->req_list, req, entry);
>> }
>> - nvme_isr_notify(n, cq);
>> + nvme_irq_assert(n, cq);
>> }
>>
>> static void nvme_enqueue_req_completion(NvmeCQueue *cq, NvmeRequest *req)
>> @@ -753,10 +780,12 @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
>> case 0xc:
>> n->bar.intms |= data & 0xffffffff;
>> n->bar.intmc = n->bar.intms;
>> + nvme_irq_check(n);
>> break;
>> case 0x10:
>> n->bar.intms &= ~(data & 0xffffffff);
>> n->bar.intmc = n->bar.intms;
>> + nvme_irq_check(n);
>> break;
>> case 0x14:
>> /* Windows first sends data, then sends enable bit */
>> @@ -851,8 +880,8 @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
>> timer_mod(cq->timer, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + 500);
>> }
>>
>> - if (cq->tail != cq->head) {
>> - nvme_isr_notify(n, cq);
>> + if (cq->tail == cq->head) {
>> + nvme_irq_deassert(n, cq);
>> }
>> } else {
>> uint16_t new_tail = val & 0xffff;
>> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
>> index 6aab338..7b62dad 100644
>> --- a/hw/block/nvme.h
>> +++ b/hw/block/nvme.h
>> @@ -775,6 +775,7 @@ typedef struct NvmeCtrl {
>> uint32_t cmbsz;
>> uint32_t cmbloc;
>> uint8_t *cmbuf;
>> + uint64_t irq_status;
>>
>> char *serial;
>> NvmeNamespace *namespaces;
>> --
>> 2.7.4
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe
2017-12-18 5:00 [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe Hikaru Nishida
2017-12-22 7:31 ` Hikaru Nishida
@ 2018-01-04 18:20 ` Keith Busch
2018-01-08 15:28 ` Kevin Wolf
2 siblings, 0 replies; 6+ messages in thread
From: Keith Busch @ 2018-01-04 18:20 UTC (permalink / raw)
To: Hikaru Nishida; +Cc: qemu-devel, Kevin Wolf, Max Reitz, open list:nvme
On Mon, Dec 18, 2017 at 02:00:43PM +0900, Hikaru Nishida wrote:
> Pin-based interrupt of NVMe controller did not work properly
> because using an obsolated function pci_irq_pulse().
> To fix this, change to use pci_irq_assert() / pci_irq_deassert()
> instead of pci_irq_pulse().
Looks good. Thanks for the patch, and sorry for the delay.
Reviewed-by: Keith Busch <keith.busch@intel.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe
2017-12-18 5:00 [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe Hikaru Nishida
2017-12-22 7:31 ` Hikaru Nishida
2018-01-04 18:20 ` Keith Busch
@ 2018-01-08 15:28 ` Kevin Wolf
2018-01-09 0:38 ` Hikaru Nishida
2 siblings, 1 reply; 6+ messages in thread
From: Kevin Wolf @ 2018-01-08 15:28 UTC (permalink / raw)
To: Hikaru Nishida; +Cc: qemu-devel, Keith Busch, Max Reitz, open list:nvme
Am 18.12.2017 um 06:00 hat Hikaru Nishida geschrieben:
> Pin-based interrupt of NVMe controller did not work properly
> because using an obsolated function pci_irq_pulse().
> To fix this, change to use pci_irq_assert() / pci_irq_deassert()
> instead of pci_irq_pulse().
>
> Signed-off-by: Hikaru Nishida <hikarupsp@gmail.com>
Thanks, applied to the block branch.
I had to resolve conflicts with the tracing patches and chose to keep
the trace points from pci_irq_pulse() for pci_irq_assert(), but didn't
add them to pci_irq_deassert(). Please check if this makes sense to you.
Here is the commit after my conflict resolution:
http://repo.or.cz/qemu/kevin.git/commitdiff/44c55a9159f2048a26c07e50dbc21c934917b82c
Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe
2018-01-08 15:28 ` Kevin Wolf
@ 2018-01-09 0:38 ` Hikaru Nishida
0 siblings, 0 replies; 6+ messages in thread
From: Hikaru Nishida @ 2018-01-09 0:38 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, Keith Busch, Max Reitz, open list:nvme
Thank you for applying my patch.
> I had to resolve conflicts with the tracing patches and chose to keep
> the trace points from pci_irq_pulse() for pci_irq_assert(), but didn't
> add them to pci_irq_deassert(). Please check if this makes sense to you.
It makes sense for now.
I will send another patch shortly to improve some points of the
tracing functions.
Hikaru Nishida
2018-01-09 0:28 GMT+09:00 Kevin Wolf <kwolf@redhat.com>:
> Am 18.12.2017 um 06:00 hat Hikaru Nishida geschrieben:
>> Pin-based interrupt of NVMe controller did not work properly
>> because using an obsolated function pci_irq_pulse().
>> To fix this, change to use pci_irq_assert() / pci_irq_deassert()
>> instead of pci_irq_pulse().
>>
>> Signed-off-by: Hikaru Nishida <hikarupsp@gmail.com>
>
> Thanks, applied to the block branch.
>
> I had to resolve conflicts with the tracing patches and chose to keep
> the trace points from pci_irq_pulse() for pci_irq_assert(), but didn't
> add them to pci_irq_deassert(). Please check if this makes sense to you.
> Here is the commit after my conflict resolution:
>
> http://repo.or.cz/qemu/kevin.git/commitdiff/44c55a9159f2048a26c07e50dbc21c934917b82c
>
> Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-01-09 0:38 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-18 5:00 [Qemu-devel] [PATCH] hw/block: Fix pin-based interrupt behaviour of NVMe Hikaru Nishida
2017-12-22 7:31 ` Hikaru Nishida
2018-01-04 18:06 ` Hikaru Nishida
2018-01-04 18:20 ` Keith Busch
2018-01-08 15:28 ` Kevin Wolf
2018-01-09 0:38 ` Hikaru Nishida
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).