Re: [Qemu-devel] [RFC v3 03/10] vfio: add vfio-platform support

[Alexander Graf <agraf@suse.de>]

On 02.06.14 09:49, Eric Auger wrote:
> From: Kim Phillips <kim.phillips@linaro.org>
>
> Functions for which PCI and platform device support share are moved
> into common.c.  The common vfio_{get,put}_group() get an additional
> argument, a pointer to a vfio_reset_handler(), for which to pass on
> to qemu_register_reset, but only if it exists (the platform device
> code currently passes a NULL as its reset_handler).
>
> For the platform device code, we basically use SysBusDevice
> instead of PCIDevice.  Since realize() returns void, unlike
> PCIDevice's initfn, error codes are moved into the
> error message text with %m.
>
> Currently only MMIO access is supported at this time.
>
> The perceived path for future QEMU development is:
>
> - add support for interrupts
> - verify and test platform dev unmap path
> - test existing PCI path for any regressions
> - add support for creating platform devices on the qemu command line
>    - currently device address specification is hardcoded for test
>      development on Calxeda Midway's fff51000.ethernet device
> - reset is not supported and registration of reset functions is
>    bypassed for platform devices.
>    - there is no standard means of resetting a platform device,
>      unsure if it suffices to be handled at device--VFIO binding time
>
> [1] http://www.spinics.net/lists/kvm-arm/msg08195.html
>
> Changes (v2 -> v3):
> [work done by Eric Auger]
>
> This new version introduces 2 separate VFIO Device objects:
> - VFIOPCIDevice
> - VFIOPlatformDevice
>
> Both objects share a VFIODevice struct.
>
> Also a VFIORegion shared struct was created. It is embedded in
> VFIOBAR struct. VFIOPlatformDevice uses VFIORegion directly.
>
> Introducing those base classes induced quite a lot of tiny
> changes in the PCI code. Theoretically PCI and platform
> devices can be supported simultaneously. PCI modifications
> currently are not tested.
>
> The VFIODevice is not a QOM object due to the single inheritance
> model limitation.
>
> The VFIODevice struct embeds an ops structure which is
> specialized in each VFIO leaf device. This makes possible to call
> device specific functions in common parts, hence achieving better
> factorization.
>
> Reset handling typically is handled that way where a unique
> generic ResetHandler (in common.c) is used for both derived
> classes. It calls device specific methods.
>
> As in the original contribution, only MMIO is supported in that
> patch file (in mmap mode). IRQ support comes in a subsequent patch.
>
> Signed-off-by: Kim Phillips <kim.phillips@linaro.org>
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> ---
>   hw/vfio/Makefile.objs      |    2 +
>   hw/vfio/common.c           |  849 ++++++++++++++++++++++++++++
>   hw/vfio/pci.c              | 1316 ++++++++++----------------------------------
>   hw/vfio/platform.c         |  267 +++++++++
>   hw/vfio/vfio-common.h      |  143 +++++
>   linux-headers/linux/vfio.h |    1 +
>   6 files changed, 1565 insertions(+), 1013 deletions(-)

This patch is impossible to review. Please split it out into a part (or 
maybe even multiple patches) that separate pci specific VFIO support 
into its own file and then another set of patches implementing platform 
support.


Alex