Re: [Qemu-devel] [PATCH v7 10/16] hw/vfio: calxeda xgmac device

[Alexander Graf <agraf@suse.de>]

On 31.10.14 15:05, Eric Auger wrote:
> The platform device class has become abstract. The device can be be
> instantiated on command line using such option.
> 
> -device vfio-calxeda-xgmac,host="fff51000.ethernet"
> 
> Signed-off-by: Eric Auger <eric.auger@linaro.org>
> 
> ---
> 
> v5 -> v6
> - back again following Alex Graf advises
> - fix a bug related to compat override
> 
> v4 -> v5:
> removed since device tree was moved to hw/arm/dyn_sysbus_devtree.c
> 
> v4: creation for device tree specialization
> ---
>  hw/vfio/Makefile.objs                |  1 +
>  hw/vfio/calxeda_xgmac.c              | 54 ++++++++++++++++++++++++++++++++++++
>  include/hw/vfio/vfio-calxeda-xgmac.h | 41 +++++++++++++++++++++++++++
>  3 files changed, 96 insertions(+)
>  create mode 100644 hw/vfio/calxeda_xgmac.c
>  create mode 100644 include/hw/vfio/vfio-calxeda-xgmac.h
> 
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index c5c76fe..913ab14 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -2,4 +2,5 @@ ifeq ($(CONFIG_LINUX), y)
>  obj-$(CONFIG_SOFTMMU) += common.o
>  obj-$(CONFIG_PCI) += pci.o
>  obj-$(CONFIG_SOFTMMU) += platform.o
> +obj-$(CONFIG_SOFTMMU) += calxeda_xgmac.o
>  endif
> diff --git a/hw/vfio/calxeda_xgmac.c b/hw/vfio/calxeda_xgmac.c
> new file mode 100644
> index 0000000..199e076
> --- /dev/null
> +++ b/hw/vfio/calxeda_xgmac.c
> @@ -0,0 +1,54 @@
> +/*
> + * calxeda xgmac example VFIO device
> + *
> + * Copyright Linaro Limited, 2014
> + *
> + * Authors:
> + *  Eric Auger <eric.auger@linaro.org>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "hw/vfio/vfio-calxeda-xgmac.h"
> +
> +static void calxeda_xgmac_realize(DeviceState *dev, Error **errp)
> +{
> +    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(dev);
> +    VFIOCalxedaXgmacDeviceClass *k = VFIO_CALXEDA_XGMAC_DEVICE_GET_CLASS(dev);
> +
> +    vdev->compat = g_strdup("calxeda,hb-xgmac");
> +
> +    k->parent_realize(dev, errp);

Since MMIO and IRQ line exposure happens in the parent, I would like to
see a comment here explaining the semantics of each region here. That
way users at least have the chance to figure out what each MMIO number
and IRQ number mean.

Also, since this device will probably get used as example code for
others, I'd like to make sure we set a proper precedence, even if it's
"trivial" in this case.


Alex