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 <da...@gibson.dropbear.id.au>
> ---
> 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
