Re: [PATCH v2 1/3] hw/nvme: support irq(de)assertion with eventfd

[Jinhao Fan <fanjinhao21s@ict.ac.cn>]

On 8/25/2022 7:56 PM, Klaus Jensen wrote:
> On Aug 25 19:16, Jinhao Fan wrote:
>> On 8/25/2022 5:33 PM, Klaus Jensen wrote:
>>> I'm still a bit perplexed by this issue, so I just tried moving
>>> nvme_init_irq_notifier() to the end of nvme_init_cq() and removing this
>>> first_io_cqe thing. I did not observe any particular issues?
>>>
>>> What bad behavior did you encounter, it seems to work fine to me
>>
>> The kernel boots up and got stuck, waiting for interrupts. Then the request
>> times out and got retried three times. Finally the driver seems to decide
>> that the drive is down and continues to boot.
>>
>> I added some prints during debugging and found that the MSI-X message which
>> got registered in KVM via kvm_irqchip_add_msi_route() is not the same as the
>> one actually used in msix_notify().
>>
>> Are you sure you are using KVM's irqfd?
>>
> 
> Pretty sure? Using "ioeventfd=on,irq-eventfd=on" on the controller.
> 
> And the following patch.
> 
> 
> diff --git i/hw/nvme/ctrl.c w/hw/nvme/ctrl.c
> index 30bbda7bb5ae..b2e41d3bd745 100644
> --- i/hw/nvme/ctrl.c
> +++ w/hw/nvme/ctrl.c
> @@ -1490,21 +1490,6 @@ static void nvme_post_cqes(void *opaque)
>               if (!pending) {
>                   n->cq_pending++;
>               }
> -
> -            if (unlikely(cq->first_io_cqe)) {
> -                /*
> -                 * Initilize event notifier when first cqe is posted. For irqfd
> -                 * support we need to register the MSI message in KVM. We
> -                 * can not do this registration at CQ creation time because
> -                 * Linux's NVMe driver changes the MSI message after CQ creation.
> -                 */
> -                cq->first_io_cqe = false;
> -
> -                if (n->params.irq_eventfd) {
> -                    nvme_init_irq_notifier(n, cq);
> -                }
> -            }
> -
>           }
>   
>           nvme_irq_assert(n, cq);
> @@ -4914,11 +4899,14 @@ static void nvme_init_cq(NvmeCQueue *cq, NvmeCtrl *n, uint64_t dma_addr,
>       }
>       n->cq[cqid] = cq;
>       cq->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, nvme_post_cqes, cq);
> +
>       /*
>        * Only enable irqfd for IO queues since we always emulate admin queue
>        * in main loop thread
>        */
> -    cq->first_io_cqe = cqid != 0;
> +    if (cqid && n->params.irq_eventfd) {
> +        nvme_init_irq_notifier(n, cq);
> +    }
>   }
> 
> 

Interesting. This patch does not work well for me. Here is the kernel 
boot log related to NVMe:

[    0.625096] nvme nvme0: 4/0/0 default/read/poll queues
[    0.627386] IPI shorthand broadcast: enabled
[    0.627534] nvme nvme0: Ignoring bogus Namespace Identifiers
[    0.628167] sched_clock: Marking stable (528813377, 
99314943)->(636909773, -8781453)
[    0.629204] Spurious interrupt (vector 0xef) on CPU#0. Acked
....
[   30.901423] nvme nvme0: I/O 320 (Read) QID 2 timeout, aborting
[   30.902696] nvme nvme0: Abort status: 0x0

Notice the "spurious interrupt" line. I believe this means the MSI 
message is incorrect.