From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:38410) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gkLaj-0006pq-IK for qemu-devel@nongnu.org; Thu, 17 Jan 2019 23:17:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gkLYF-0007a0-0J for qemu-devel@nongnu.org; Thu, 17 Jan 2019 23:14:39 -0500 Received: from mail-pf1-x442.google.com ([2607:f8b0:4864:20::442]:39834) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gkLYE-00078j-Dw for qemu-devel@nongnu.org; Thu, 17 Jan 2019 23:14:38 -0500 Received: by mail-pf1-x442.google.com with SMTP id r136so5910415pfc.6 for ; Thu, 17 Jan 2019 20:14:30 -0800 (PST) References: <20190111165801.15181-1-eric.auger@redhat.com> <20190111165801.15181-2-eric.auger@redhat.com> <739abfa7-5faa-0c0c-7e81-80136e702e08@redhat.com> From: Alexey Kardashevskiy Message-ID: <34fa59d1-225b-fbea-dca5-fdc9b789662a@ozlabs.ru> Date: Fri, 18 Jan 2019 15:14:24 +1100 MIME-Version: 1.0 In-Reply-To: <739abfa7-5faa-0c0c-7e81-80136e702e08@redhat.com> 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: Auger Eric , eric.auger.pro@gmail.com, qemu-devel@nongnu.org, alex.williamson@redhat.com 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). -- Alexey