On Sat, Aug 09, 2014 at 03:25:42PM +0100, Eric Auger wrote:
> Introduce the VFIODevice struct that is going to be shared by
> VFIOPCIDevice and VFIOPlatformDevice.
>
> Additional fields will be added there later on for review
> convenience.
>
> the group's device_list becomes a list of VFIODevice
>
> This obliges to rework the reset_handler which becomes generic and
> calls VFIODevice ops that are specialized in each parent object.
> Also functions that iterate on this list must take care that the
> devices can be something else than VFIOPCIDevice. The type is used
> to discriminate them.
>
> we profit from this step to change the prototype of
> vfio_unmask_intx, vfio_mask_intx, vfio_disable_irqindex which now
> apply to VFIODevice. They are renamed as *_irqindex.
> The index is passed as parameter to anticipate their usage for
> platform IRQs
>
> Signed-off-by: Eric Auger <eric.au...@linaro.org>
>
> ---
>
> v4->v5:
> - fix style issues
> - in vfio_initfn, rework allocation of vdev->vbasedev.name and
> replace snprintf by g_strdup_printf
> ---
> hw/vfio/pci.c | 239
> +++++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 146 insertions(+), 93 deletions(-)
>
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index c2cdd73..ae827c5 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -56,6 +56,11 @@
> #define VFIO_ALLOW_KVM_MSI 1
> #define VFIO_ALLOW_KVM_MSIX 1
>
> +enum {
> + VFIO_DEVICE_TYPE_PCI = 0,
> + VFIO_DEVICE_TYPE_PLATFORM = 1,
> +};
> +
> struct VFIOPCIDevice;
>
> typedef struct VFIOQuirk {
> @@ -193,9 +198,27 @@ typedef struct VFIOMSIXInfo {
> void *mmap;
> } VFIOMSIXInfo;
>
> +typedef struct VFIODeviceOps VFIODeviceOps;
> +
> +typedef struct VFIODevice {
> + QLIST_ENTRY(VFIODevice) next;
> + struct VFIOGroup *group;
> + char *name;
> + int fd;
> + int type;
I'm assuming this takes values from the enum above. Is this actually
necessary as a field, or could the same information be derived from
the device's QOM class information?
> + bool reset_works;
> + bool needs_reset;
> + VFIODeviceOps *ops;
> +} VFIODevice;
> +
> +struct VFIODeviceOps {
> + bool (*vfio_compute_needs_reset)(VFIODevice *vdev);
> + int (*vfio_hot_reset_multi)(VFIODevice *vdev);
> +};
Shouldn't these be methods in the QOM class, rather than a separate
ops structure?
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
pgp9U_i1Rl64C.pgp
Description: PGP signature
