From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:33698) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gkQ8s-0006Ep-Ds for qemu-devel@nongnu.org; Fri, 18 Jan 2019 04:08:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gkQ8r-0002j5-MO for qemu-devel@nongnu.org; Fri, 18 Jan 2019 04:08:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55794) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gkQ8r-0002gU-E4 for qemu-devel@nongnu.org; Fri, 18 Jan 2019 04:08:45 -0500 References: <20190111165801.15181-1-eric.auger@redhat.com> <20190111165801.15181-2-eric.auger@redhat.com> <739abfa7-5faa-0c0c-7e81-80136e702e08@redhat.com> <34fa59d1-225b-fbea-dca5-fdc9b789662a@ozlabs.ru> From: Auger Eric Message-ID: <0da57281-7039-b942-5efd-6e07c6a4c94a@redhat.com> Date: Fri, 18 Jan 2019 10:08:12 +0100 MIME-Version: 1.0 In-Reply-To: <34fa59d1-225b-fbea-dca5-fdc9b789662a@ozlabs.ru> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] vfio-pci: Introduce vfio_register_event_notifier helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy , eric.auger.pro@gmail.com, qemu-devel@nongnu.org, alex.williamson@redhat.com Hi Alexey, On 1/18/19 5:14 AM, Alexey Kardashevskiy wrote: > > > On 17/01/2019 20:16, Auger Eric wrote: >> Hi Alexey, Cornelia, >> >> On 1/17/19 4:46 AM, Alexey Kardashevskiy wrote: >>> >>> >>> On 12/01/2019 03:58, Eric Auger wrote: >>>> The code used to attach the eventfd handler for the ERR and >>>> REQ irq indices can be factorized into a helper. In subsequent >>>> patches we will extend this helper to support other irq indices. >>>> >>>> We test the notification is allowed outside of the helper: >>>> respectively check vdev->pci_aer and VFIO_FEATURE_ENABLE_REQ. >>>> Depending on the returned value we set vdev->pci_aer and >>>> vdev->req_enabled. An error handle is introduced for future usage >>>> although not strictly useful here.> >>>> Signed-off-by: Eric Auger >>>> --- >>>> hw/vfio/pci.c | 291 ++++++++++++++++++++++---------------------------- >>>> 1 file changed, 127 insertions(+), 164 deletions(-) >>>> >>>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >>>> index c0cb1ec289..c589a4e666 100644 >>>> --- a/hw/vfio/pci.c >>>> +++ b/hw/vfio/pci.c >>>> @@ -105,6 +105,95 @@ static void vfio_intx_eoi(VFIODevice *vbasedev) >>>> vfio_unmask_single_irqindex(vbasedev, VFIO_PCI_INTX_IRQ_INDEX); >>>> } >>>> >>>> +/* >>>> + * vfio_register_event_notifier - setup/tear down eventfd >>>> + * notification and handling for IRQ indices that span over >>>> + * a single IRQ >>>> + * >>>> + * @vdev: VFIO device handle >>>> + * @index: IRQ index the eventfd/handler is associated to >>>> + * @target_state: true means notifier needs to be set up >>>> + * @handler to attach if @target_state is true >>>> + * @errp error handle >>>> + */ >>>> +static int vfio_register_event_notifier(VFIOPCIDevice *vdev, >>>> + int index, >>>> + bool target_state, >>>> + void (*handler)(void *opaque), >>>> + Error **errp) >>>> +{ >>>> + struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info), >>>> + .index = index }; >>>> + struct vfio_irq_set *irq_set; >>>> + EventNotifier *notifier; >>>> + int argsz, ret = 0; >>>> + int32_t *pfd, fd; >>>> + >>>> + switch (index) { >>> >>> I'd pass the notifier as a parameter as well so index/handler/notifier >>> would walk together. >> >> I tend to agree with Cornelia. moving the notifier out of this helper >> would remove some factorization and this way, the caller does not have >> to care about it. > > > Then why pass the handler? It also could go into this switch, > vfio_register_event_notifier()/vfio_set_event_handler() is never called > with more than one handler per index (or NULL but then target_state==false). I don't have any strong opinion here. I will align with the majority's opinion. Thanks Eric > >