From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56695) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZehKn-0004G2-VI for qemu-devel@nongnu.org; Wed, 23 Sep 2015 06:31:34 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZehKk-00048p-Pn for qemu-devel@nongnu.org; Wed, 23 Sep 2015 06:31:33 -0400 References: <1442495357-26547-1-git-send-email-david@gibson.dropbear.id.au> <1442495357-26547-2-git-send-email-david@gibson.dropbear.id.au> From: Thomas Huth Message-ID: <56027F7D.7090705@redhat.com> Date: Wed, 23 Sep 2015 12:31:25 +0200 MIME-Version: 1.0 In-Reply-To: <1442495357-26547-2-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 01/10] vfio: Remove unneeded union from VFIOContainer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , alex.williamson@redhat.com, aik@ozlabs.ru, gwshan@linux.vnet.ibm.com Cc: lvivier@redhat.com, pbonzini@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 17/09/15 15:09, David Gibson wrote: > Currently the VFIOContainer iommu_data field contains a union with > different information for different host iommu types. However: > * It only actually contains information for the x86-like "Type1" iommu > * Because we have a common listener the Type1 fields are actually used > on all IOMMU types, including the SPAPR TCE type as well > * There's no tag in the VFIOContainer to tell you which union member is > valid anyway. > > In fact we now have a general structure for the listener which is unlikely > to ever need per-iommu-type information, so this patch removes the union. > > In a similar way we can unify the setup of the vfio memory listener in > vfio_connect_container() that is currently split across a switch on iommu > type, but is effectively the same in both cases. > > The iommu_data.release pointer was only needed as a cleanup function > which would handle potentially different data in the union. With the > union gone, it too can be removed. > > Signed-off-by: David Gibson > --- > hw/vfio/common.c | 51 +++++++++++++++++-------------------------- > include/hw/vfio/vfio-common.h | 14 +++--------- > 2 files changed, 23 insertions(+), 42 deletions(-) ... > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index 59a321d..aff18cd 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -64,21 +64,13 @@ typedef struct VFIOAddressSpace { > > struct VFIOGroup; > > -typedef struct VFIOType1 { > - MemoryListener listener; > - int error; > - bool initialized; > -} VFIOType1; > - > typedef struct VFIOContainer { > VFIOAddressSpace *space; > int fd; /* /dev/vfio/vfio, empowered by the attached groups */ > struct { > - /* enable abstraction to support various iommu backends */ > - union { > - VFIOType1 type1; > - }; > - void (*release)(struct VFIOContainer *); > + MemoryListener listener; > + int error; > + bool initialized; > } iommu_data; > QLIST_HEAD(, VFIOGuestIOMMU) giommu_list; > QLIST_HEAD(, VFIOGroup) group_list; > I think I agree with Alexey here ... keeping the iommu_data struct around those fields looks cumbersome. Is there a reason you did not remove the struct completely? Thomas