From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:47698)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <shankerd@codeaurora.org>) id 1aQQke-0005Lj-AY
	for qemu-devel@nongnu.org; Mon, 01 Feb 2016 21:31:33 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <shankerd@codeaurora.org>) id 1aQQkb-0007qX-2Y
	for qemu-devel@nongnu.org; Mon, 01 Feb 2016 21:31:32 -0500
Received: from smtp.codeaurora.org ([198.145.29.96]:58905)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <shankerd@codeaurora.org>) id 1aQQka-0007qS-Q9
	for qemu-devel@nongnu.org; Mon, 01 Feb 2016 21:31:28 -0500
References: <1454108405-4822-1-git-send-email-shankerd@codeaurora.org>
	<1454108405-4822-3-git-send-email-shankerd@codeaurora.org>
	<56AF6D98.2040706@linaro.org>
From: Shanker Donthineni <shankerd@codeaurora.org>
Message-ID: <56B014E6.80101@codeaurora.org>
Date: Mon, 1 Feb 2016 20:31:02 -0600
MIME-Version: 1.0
In-Reply-To: <56AF6D98.2040706@linaro.org>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 2/2] hw/vfio/platform: Add Qualcomm
 Technologies, Inc HiDMA device support
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Auger <eric.auger@linaro.org>, qemu-devel@nongnu.org
Cc: Kaya Sinan <okaya@codeaurora.org>, Peter Maydell <peter.maydell@linaro.org>, Christopher Covington <ccovingt@quicinc.com>, Alex Williamson <alex.williamson@redhat.com>, Vikram Sethi <vikrams@codeaurora.org>

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 <vikrams@codeaurora.org>
>>
>> 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 <vikrams@codeaurora.org>
>> Signed-off-by: Shanker Donthineni <shankerd@codeaurora.org>
>> ---
>>   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
>>
>