Re: [PATCH v4] hw/nvme: Use ioeventfd to handle doorbell updates

[Jinhao Fan]

at 3:06 PM, Klaus Jensen <its@irrelevant.dk> wrote:

> On Jul 26 14:08, Klaus Jensen wrote:
>> Alright. Forget about the iommu, that was just a coincidence.
>> 
>> This patch seems to fix it. I guess it is the
>> event_notifier_set_handler(..., NULL) that does the trick, but I'd like
>> to understand why ;)
>> 
>> 
>> diff --git i/hw/nvme/ctrl.c w/hw/nvme/ctrl.c
>> index 533ad14e7a61..3bc3c6bfbe78 100644
>> --- i/hw/nvme/ctrl.c
>> +++ w/hw/nvme/ctrl.c
>> @@ -4238,7 +4238,9 @@ static void nvme_cq_notifier(EventNotifier *e)
>>     NvmeCQueue *cq = container_of(e, NvmeCQueue, notifier);
>>     NvmeCtrl *n = cq->ctrl;
>> 
>> -    event_notifier_test_and_clear(&cq->notifier);
>> +    if (!event_notifier_test_and_clear(e)) {
>> +        return;
>> +    }
>> 
>>     nvme_update_cq_head(cq);
>> 
>> @@ -4275,7 +4277,9 @@ static void nvme_sq_notifier(EventNotifier *e)
>> {
>>     NvmeSQueue *sq = container_of(e, NvmeSQueue, notifier);
>> 
>> -    event_notifier_test_and_clear(&sq->notifier);
>> +    if (!event_notifier_test_and_clear(e)) {
>> +        return;
>> +    }
>> 

Yes, virtio also checks the return value of event_notifier_test_and_clear().

>>     nvme_process_sq(sq);
>> }
>> @@ -4307,6 +4311,8 @@ static void nvme_free_sq(NvmeSQueue *sq, NvmeCtrl *n)
>>     if (sq->ioeventfd_enabled) {
>>         memory_region_del_eventfd(&n->iomem,
>>                                   0x1000 + offset, 4, false, 0, 
>> &sq->notifier);
>> +        event_notifier_set_handler(&sq->notifier, NULL);
>> +        nvme_sq_notifier(&sq->notifier);
>>         event_notifier_cleanup(&sq->notifier);
>>     }
>>     g_free(sq->io_req);
>> @@ -4697,6 +4703,8 @@ static void nvme_free_cq(NvmeCQueue *cq, NvmeCtrl *n)
>>     if (cq->ioeventfd_enabled) {
>>         memory_region_del_eventfd(&n->iomem,
>>                                   0x1000 + offset, 4, false, 0, 
>> &cq->notifier);
>> +        event_notifier_set_handler(&cq->notifier, NULL);
>> +        nvme_cq_notifier(&cq->notifier);
>>         event_notifier_cleanup(&cq->notifier);
>>     }
>>     if (msix_enabled(&n->parent_obj)) {
> 

I’m a bit messed up here. I will further check how virtio handles queue 
deletion today.