Re: [Qemu-devel] [PATCH 2/2] hw/vfio/platform: Add Qualcomm Technologies, Inc HiDMA device support

[Shanker Donthineni]

Hi Eric,

On 02/01/2016 08:37 AM, Eric Auger wrote:
> Hi Shanker, Vikram,
> On 01/30/2016 12:00 AM, Shanker Donthineni wrote:
> > From: Vikram Sethi <address@hidden>
> >
> > This patch introduces a Qualcomm Technologies, Inc HiDMA
> > device and allows the instantiation of the vfio-qcom-hidma
> > device from the QEMU command line
> > (-device vfio-qcom-hidma,host="<device>").
> >
> > A device tree node is created for the guest containing compat,
> > dma-coherent, reg and interrupts properties.
> >
> > Signed-off-by: Vikram Sethi <address@hidden>
> > Signed-off-by: Shanker Donthineni <address@hidden>
> > ---
> >   hw/arm/sysbus-fdt.c               |  2 ++
> >   hw/vfio/Makefile.objs             |  1 +
> >   hw/vfio/qcom-hidma.c              | 57 +++++++++++++++++++++++++++++++++++++++
> >   include/hw/vfio/vfio-qcom-hidma.h | 49 +++++++++++++++++++++++++++++++++
> >   4 files changed, 109 insertions(+)
> >   create mode 100644 hw/vfio/qcom-hidma.c
> >   create mode 100644 include/hw/vfio/vfio-qcom-hidma.h
> >
> > diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> > index 6ee7af2..4a7419e 100644
> > --- a/hw/arm/sysbus-fdt.c
> > +++ b/hw/arm/sysbus-fdt.c
> > @@ -28,6 +28,7 @@
> >   #include "sysemu/sysemu.h"
> >   #include "hw/vfio/vfio-platform.h"
> >   #include "hw/vfio/vfio-calxeda-xgmac.h"
> > +#include "hw/vfio/vfio-qcom-hidma.h"
> >   #include "hw/arm/fdt.h"
> >   
> >   /*
> > @@ -126,6 +127,7 @@ fail_reg:
> >   /* list of supported dynamic sysbus devices */
> >   static const NodeCreationPair add_fdt_node_functions[] = {
> >       {TYPE_VFIO_CALXEDA_XGMAC, add_generic_platform_fdt_node},
> > +    {TYPE_VFIO_QCOM_HIDMA, add_generic_platform_fdt_node},
> >       {"", NULL}, /* last element */
> >   };
> >   
> > diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> > index d324863..9bcb093 100644
> > --- a/hw/vfio/Makefile.objs
> > +++ b/hw/vfio/Makefile.objs
> > @@ -3,4 +3,5 @@ obj-$(CONFIG_SOFTMMU) += common.o
> >   obj-$(CONFIG_PCI) += pci.o pci-quirks.o
> >   obj-$(CONFIG_SOFTMMU) += platform.o
> >   obj-$(CONFIG_SOFTMMU) += calxeda-xgmac.o
> > +obj-$(CONFIG_SOFTMMU) += qcom-hidma.o
> >   endif
> > diff --git a/hw/vfio/qcom-hidma.c b/hw/vfio/qcom-hidma.c
> > new file mode 100644
> > index 0000000..04acbd8
> > --- /dev/null
> > +++ b/hw/vfio/qcom-hidma.c
> > @@ -0,0 +1,57 @@
> > +/*
> > + * Qualcomm Technologies, Inc VFIO HiDMA platform device
> > + *
> > + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#include "hw/vfio/vfio-qcom-hidma.h"
> > +
> > +static void qcom_hidma_realize(DeviceState *dev, Error **errp)
> > +{
> > +    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(dev);
> > +    VFIOQcomHidmaDeviceClass *k = VFIO_QCOM_HIDMA_DEVICE_GET_CLASS(dev);
> > +
> > +    vdev->compat = g_strdup("qcom,hidma");
> > +
> > +    k->parent_realize(dev, errp);
> > +}
> > +
> > +static const VMStateDescription vfio_platform_vmstate = {
> > +    .name = TYPE_VFIO_QCOM_HIDMA,
> > +    .unmigratable = 1,
> > +};
> > +
> > +static void vfio_qcom_hidma_class_init(ObjectClass *klass, void *data)
> > +{
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> > +    VFIOQcomHidmaDeviceClass *vcxc = VFIO_QCOM_HIDMA_DEVICE_CLASS(klass);
> > +
> > +    vcxc->parent_realize = dc->realize;
> > +    dc->realize = qcom_hidma_realize;
> > +    dc->desc = "VFIO QCOM HIDMA";
> If I am not wrong you miss the dc->vmsd = &vfio_platform_vmstate
> (VMStateDescription attachement)
>
> This will cause an error with CLANG - I got that one already, reported
> by Peter ;-) -
>
> error: unused variable
> 'vfio_platform_vmstate' [-Werror,-Wunused-const-variable]
> static const VMStateDescription vfio_platform_vmstate = {

Thanks for your finding, I will fix.

> > +}
> > +
> > +static const TypeInfo vfio_qcom_hidma_dev_info = {
> > +    .name = TYPE_VFIO_QCOM_HIDMA,
> > +    .parent = TYPE_VFIO_PLATFORM,
> > +    .instance_size = sizeof(VFIOQcomHidmaDevice),
> > +    .class_init = vfio_qcom_hidma_class_init,
> > +    .class_size = sizeof(VFIOQcomHidmaDeviceClass),
> > +};
> > +
> > +static void register_qcom_hidma_dev_type(void)
> > +{
> > +    type_register_static(&vfio_qcom_hidma_dev_info);
> > +}
> > +
> > +type_init(register_qcom_hidma_dev_type)
> > diff --git a/include/hw/vfio/vfio-qcom-hidma.h 
> > b/include/hw/vfio/vfio-qcom-hidma.h
> > new file mode 100644
> > index 0000000..a7cc8e6
> > --- /dev/null
> > +++ b/include/hw/vfio/vfio-qcom-hidma.h
> > @@ -0,0 +1,49 @@
> > +/*
> > + * Qualcomm Technologies, Inc VFIO HiDMA platform device
> > + *
> > + * Copyright (c) 2016, The Linux Foundation. All rights reserved.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 and
> > + * only version 2 as published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +
> > +#ifndef HW_VFIO_VFIO_QCOM_HIDMA_H
> > +#define HW_VFIO_VFIO_QCOM_HIDMA_H
> > +
> > +#include "hw/vfio/vfio-platform.h"
> > +
> > +#define TYPE_VFIO_QCOM_HIDMA "vfio-qcom-hidma"
> > +
> > +/**
> > + * This device exposes:
> > + * - two MMIO regions corresponding to its register space
> > + * - 1 IRQ
> level sensitive?

Yes, the current HiDMA driver that we are attempting to upstream uses 
level sensitive interrupts.

> Best Regards
>
> Eric
> > + */
> > +typedef struct VFIOQcomHidmaDevice {
> > +    VFIOPlatformDevice vdev;
> > +} VFIOQcomHidmaDevice;
> > +
> > +typedef struct VFIOQcomHidmaDeviceClass {
> > +    /*< private >*/
> > +    VFIOPlatformDeviceClass parent_class;
> > +    /*< public >*/
> > +    DeviceRealize parent_realize;
> > +} VFIOQcomHidmaDeviceClass;
> > +
> > +#define VFIO_QCOM_HIDMA_DEVICE(obj) \
> > +     OBJECT_CHECK(VFIOQcomHidmaDevice, (obj), TYPE_VFIO_QCOM_HIDMA)
> > +#define VFIO_QCOM_HIDMA_DEVICE_CLASS(klass) \
> > +     OBJECT_CLASS_CHECK(VFIOQcomHidmaDeviceClass, (klass), \
> > +                        TYPE_VFIO_QCOM_HIDMA)
> > +#define VFIO_QCOM_HIDMA_DEVICE_GET_CLASS(obj) \
> > +     OBJECT_GET_CLASS(VFIOQcomHidmaDeviceClass, (obj), \
> > +                      TYPE_VFIO_QCOM_HIDMA)
> > +
> > +#endif