From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:49671) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH55k-0006SN-Qn for qemu-devel@nongnu.org; Tue, 12 Aug 2014 01:57:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XH55b-0006O8-0I for qemu-devel@nongnu.org; Tue, 12 Aug 2014 01:57:52 -0400 Received: from mail-wi0-f181.google.com ([209.85.212.181]:48348) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XH55a-0006NI-N7 for qemu-devel@nongnu.org; Tue, 12 Aug 2014 01:57:42 -0400 Received: by mail-wi0-f181.google.com with SMTP id bs8so5209858wib.14 for ; Mon, 11 Aug 2014 22:57:41 -0700 (PDT) Message-ID: <53E9ACD1.2040607@linaro.org> Date: Tue, 12 Aug 2014 07:57:37 +0200 From: Eric Auger MIME-Version: 1.0 References: <1407594349-9291-1-git-send-email-eric.auger@linaro.org> <1407594349-9291-7-git-send-email-eric.auger@linaro.org> <1407784838.9800.96.camel@ul30vt.home> In-Reply-To: <1407784838.9800.96.camel@ul30vt.home> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 06/10] hw/vfio: create common module List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: agraf@suse.de, kim.phillips@freescale.com, eric.auger@st.com, peter.maydell@linaro.org, Kim Phillips , patches@linaro.org, will.deacon@arm.com, qemu-devel@nongnu.org, a.rigo@virtualopensystems.com, Bharat.Bhushan@freescale.com, stuart.yoder@freescale.com, joel.schopp@amd.com, a.motakis@virtualopensystems.com, kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org On 08/11/2014 09:20 PM, Alex Williamson wrote: > On Sat, 2014-08-09 at 15:25 +0100, Eric Auger wrote: >> A new common module is created. It implements all functions >> that have no device specificity (PCI, Platform). >> >> This patch only consists in move (no functional changes) >> >> Signed-off-by: Kim Phillips >> Signed-off-by: Eric Auger >> >> --- >> >> v4 -> v5: >> - integrate "sPAPR/IOMMU: Fix TCE entry permission" >> - VFIOdevice .name dealloc removed from vfio_put_base_device >> - add some includes according to vfio inclusion policy >> >> v3 -> v4: >> [Eric Auger] >> move done after all PCI modifications to anticipate for >> VFIO Platform needs. Purpose is to alleviate the whole >> review process. >> >> <= v3 >> First split done by Kim Phillips >> --- >> hw/vfio/Makefile.objs | 1 + >> hw/vfio/common.c | 990 ++++++++++++++++++++++++++++++++++++++ >> hw/vfio/pci.c | 1070 +---------------------------------------- >> include/hw/vfio/vfio-common.h | 151 ++++++ >> 4 files changed, 1147 insertions(+), 1065 deletions(-) >> create mode 100644 hw/vfio/common.c >> create mode 100644 include/hw/vfio/vfio-common.h >> >> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs >> index 31c7dab..e31f30e 100644 >> --- a/hw/vfio/Makefile.objs >> +++ b/hw/vfio/Makefile.objs >> @@ -1,3 +1,4 @@ >> ifeq ($(CONFIG_LINUX), y) >> +obj-$(CONFIG_SOFTMMU) += common.o >> obj-$(CONFIG_PCI) += pci.o >> endif >> diff --git a/hw/vfio/common.c b/hw/vfio/common.c >> new file mode 100644 >> index 0000000..297c508 >> --- /dev/null >> +++ b/hw/vfio/common.c >> @@ -0,0 +1,990 @@ >> +/* >> + * generic functions used by VFIO devices >> + * >> + * Copyright Red Hat, Inc. 2012 >> + * >> + * Authors: >> + * Alex Williamson >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2. See >> + * the COPYING file in the top-level directory. >> + * >> + * Based on qemu-kvm device-assignment: >> + * Adapted for KVM by Qumranet. >> + * Copyright (c) 2007, Neocleus, Alex Novik (alex@neocleus.com) >> + * Copyright (c) 2007, Neocleus, Guy Zana (guy@neocleus.com) >> + * Copyright (C) 2008, Qumranet, Amit Shah (amit.shah@qumranet.com) >> + * Copyright (C) 2008, Red Hat, Amit Shah (amit.shah@redhat.com) >> + * Copyright (C) 2008, IBM, Muli Ben-Yehuda (muli@il.ibm.com) >> + */ >> + >> +#include >> +#include >> +#include >> + >> +#include "hw/vfio/vfio-common.h" >> +#include "hw/vfio/vfio.h" >> +#include "exec/address-spaces.h" >> +#include "exec/memory.h" >> +#include "hw/hw.h" >> +#include "qemu/error-report.h" >> +#include "sysemu/kvm.h" >> + >> +QLIST_HEAD(, VFIOGroup) >> + group_list = QLIST_HEAD_INITIALIZER(group_list); >> + >> +QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces = >> + QLIST_HEAD_INITIALIZER(vfio_address_spaces); >> + >> +#ifdef CONFIG_KVM >> +/* >> + * We have a single VFIO pseudo device per KVM VM. Once created it lives >> + * for the life of the VM. Closing the file descriptor only drops our >> + * reference to it and the device's reference to kvm. Therefore once >> + * initialized, this file descriptor is only released on QEMU exit and >> + * we'll re-use it should another vfio device be attached before then. >> + */ >> +static int vfio_kvm_device_fd = -1; >> +#endif >> + >> +/* >> + * Common VFIO interrupt disable >> + */ >> +void vfio_disable_irqindex(VFIODevice *vbasedev, int index) >> +{ >> + struct vfio_irq_set irq_set = { >> + .argsz = sizeof(irq_set), >> + .flags = VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_TRIGGER, >> + .index = index, >> + .start = 0, >> + .count = 0, >> + }; >> + >> + ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set); >> +} >> + >> +void vfio_unmask_irqindex(VFIODevice *vbasedev, int index) >> +{ >> + struct vfio_irq_set irq_set = { >> + .argsz = sizeof(irq_set), >> + .flags = VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_UNMASK, >> + .index = index, >> + .start = 0, >> + .count = 1, >> + }; >> + >> + ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set); >> +} >> + >> +#ifdef CONFIG_KVM /* Unused outside of CONFIG_KVM code */ > > Can we remove the ifdef here and in the common header now? I'm hoping > the compiler won't complain once it's no longer static. OK > > ... >> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c >> index 5f218b7..d2ccb3b 100644 >> --- a/hw/vfio/pci.c >> +++ b/hw/vfio/pci.c >> @@ -39,27 +39,12 @@ >> #include "qemu/range.h" >> #include "sysemu/kvm.h" >> #include "sysemu/sysemu.h" >> -#include "hw/vfio/vfio.h" >> +#include "hw/vfio/vfio-common.h" >> >> -/* #define DEBUG_VFIO */ >> -#ifdef DEBUG_VFIO >> -#define DPRINTF(fmt, ...) \ >> - do { fprintf(stderr, "vfio: " fmt, ## __VA_ARGS__); } while (0) >> -#else >> -#define DPRINTF(fmt, ...) \ >> - do { } while (0) >> -#endif >> - >> -/* Extra debugging, trap acceleration paths for more logging */ >> -#define VFIO_ALLOW_MMAP 1 >> -#define VFIO_ALLOW_KVM_INTX 1 >> -#define VFIO_ALLOW_KVM_MSI 1 >> -#define VFIO_ALLOW_KVM_MSIX 1 >> - >> -enum { >> - VFIO_DEVICE_TYPE_PCI = 0, >> - VFIO_DEVICE_TYPE_PLATFORM = 1, >> -}; >> +extern const MemoryRegionOps vfio_region_ops; >> +extern const MemoryListener vfio_memory_listener; >> +extern QLIST_HEAD(, VFIOGroup) group_list; >> +extern QLIST_HEAD(, VFIOAddressSpace) vfio_address_spaces; > > This seems odd, why doesn't the common header provide these for us? We > should also rename group_list to vfio_group_list to be polite to the > rest of the namespace. Thanks, OK will rework that Thanks Eric > > Alex >