RE: [PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

2013-08-05 Thread Yoder Stuart-B08248
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 284ff24..1e4bef2 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -147,6 +147,7 @@ struct vfio_device_info {
>   __u32   flags;
>  #define VFIO_DEVICE_FLAGS_RESET  (1 << 0)/* Device supports reset
> */
>  #define VFIO_DEVICE_FLAGS_PCI(1 << 1)/* vfio-pci device */
> +#define VFIO_DEVICE_FLAGS_DT (1 << 2)/* vfio-dt device */
>   __u32   num_regions;/* Max region index + 1 */
>   __u32   num_irqs;   /* Max IRQ index + 1 */
>  };

In the RFC I sent out the proposed flags were:

   #define VFIO_DEVICE_FLAGS_PLATFORM (1 << ?) /* A platform bus device */
   #define VFIO_DEVICE_FLAGS_DEVTREE  (1 << ?) /* device tree info available */

Since you are only implementing the first part, the platform
device support, just call it:  VFIO_DEVICE_FLAGS_PLATFORM

I think the 'platform' term is more accurate as we are providing
a mechanism to expose devices on the /sys/bus/platform bus to
user space.

Stuart

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

2013-08-05 Thread Alex Williamson
On Mon, 2013-08-05 at 15:17 +0200, Antonios Motakis wrote:
> Platform devices in the Linux kernel are usually managed by the DT
> interface. This patch forms the base to support these kind of devices
> with VFIO.
> 
> Signed-off-by: Antonios Motakis 
> ---
>  drivers/vfio/Kconfig  |  10 +++
>  drivers/vfio/Makefile |   1 +
>  drivers/vfio/vfio_dt.c| 187 
> ++
>  include/uapi/linux/vfio.h |   1 +
>  4 files changed, 199 insertions(+)
>  create mode 100644 drivers/vfio/vfio_dt.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 1f84eda..a77a2e4 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -13,4 +13,14 @@ menuconfig VFIO
>  
> If you don't know what to do here, say N.
>  
> +config VFIO_DT
> + tristate "VFIO support for Device Tree devices"
> + depends on VFIO && EVENTFD

I think there needs to be another depends item here, this would allow
configuration even on architectures that have no concept of device tree.
Also, do we want to put this in a subdirectory like we've done for pci?
The rest looks like a reasonable start.  Thanks,

Alex

> + help
> +   Support for the VFIO Device Tree driver.  This is required to make
> +   use of platform devices present on Device Tree nodes using the VFIO
> +   framework.
> +
> +   If you don't know what to do here, say N.
> +
>  source "drivers/vfio/pci/Kconfig"
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 2398d4a..d599a67 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_VFIO) += vfio.o
>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>  obj-$(CONFIG_VFIO_PCI) += pci/
> +obj-$(CONFIG_VFIO_DT) += vfio_dt.o
> diff --git a/drivers/vfio/vfio_dt.c b/drivers/vfio/vfio_dt.c
> new file mode 100644
> index 000..ad4d31d
> --- /dev/null
> +++ b/drivers/vfio/vfio_dt.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (C) 2013 - Virtual Open Systems
> + * Author: Antonios Motakis 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "Antonios Motakis "
> +#define DRIVER_DESC "VFIO Device Tree devices - User Level meta-driver"
> +
> +struct vfio_dt_device {
> + struct platform_device  *pdev;
> +};
> +
> +static void vfio_dt_release(void *device_data)
> +{
> + module_put(THIS_MODULE);
> +}
> +
> +static int vfio_dt_open(void *device_data)
> +{
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static long vfio_dt_ioctl(void *device_data,
> +unsigned int cmd, unsigned long arg)
> +{
> + struct vfio_dt_device *vdev = device_data;
> + unsigned long minsz;
> +
> + if (cmd == VFIO_DEVICE_GET_INFO) {
> + struct vfio_device_info info;
> +
> + minsz = offsetofend(struct vfio_device_info, num_irqs);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + info.flags = VFIO_DEVICE_FLAGS_DT;
> + info.num_regions = 0;
> + info.num_irqs = 0;
> +
> + return copy_to_user((void __user *)arg, &info, minsz);
> +
> + } else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
> + return -EINVAL;
> +
> + else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> + return -EINVAL;
> +
> + else if (cmd == VFIO_DEVICE_SET_IRQS)
> + return -EINVAL;
> +
> + else if (cmd == VFIO_DEVICE_RESET)
> + return -EINVAL;
> +
> + return -ENOTTY;
> +}
> +
> +static ssize_t vfio_dt_read(void *device_data, char __user *buf,
> +  size_t count, loff_t *ppos)
> +{
> + return 0;
> +}
> +
> +static ssize_t vfio_dt_write(void *device_data, const char __user *buf,
> +   size_t count, loff_t *ppos)
> +{
> + return 0;
> +}
> +
> +static int vfio_dt_mmap(void *device_data, struct vm_area_struct *vma)
> +{
> + return -EINVAL;
> +}
> +
> +static const str

Re: [PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

2013-08-05 Thread Mark Rutland
>  > +static const struct of_device_id vfio_dt_match[] = {
>  > + � � /* In the future, we can implement a better mechanism to bind
>  the
>  > + � � �* module to any device. For now add the compatible property to
>  the
>  > + � � �* dtb of the devices we want to use. � */
>  > + � � {
>  > + � � � � � � .compatible = "vfio-dt",
>  > + � � },
>  > + � � {},
>  > +};
> 
>  This definitely doesn't belong in the dt. It's purely a Linux
>  abstraction and does not represent a piece of hardware or common
>  interface.
> 
>  We need to think of a better mechanism for binding the module to these
>  devices now.
> 
>I already make this remark in the cover letter; thanks for confirming it.
>�
>Antonios

Sorry, I found the cover letter a little unclear in that regard.

Thanks for the clarification :)

Mark.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] Initial skeleton of VFIO support for Device Tree based devices

2013-08-05 Thread Mark Rutland
[adding DT maintainers to Cc]

On Mon, Aug 05, 2013 at 02:17:11PM +0100, Antonios Motakis wrote:
> Platform devices in the Linux kernel are usually managed by the DT
> interface. This patch forms the base to support these kind of devices
> with VFIO.
> 
> Signed-off-by: Antonios Motakis 
> ---
>  drivers/vfio/Kconfig  |  10 +++
>  drivers/vfio/Makefile |   1 +
>  drivers/vfio/vfio_dt.c| 187 
> ++
>  include/uapi/linux/vfio.h |   1 +
>  4 files changed, 199 insertions(+)
>  create mode 100644 drivers/vfio/vfio_dt.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 1f84eda..a77a2e4 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -13,4 +13,14 @@ menuconfig VFIO
>  
> If you don't know what to do here, say N.
>  
> +config VFIO_DT
> + tristate "VFIO support for Device Tree devices"
> + depends on VFIO && EVENTFD
> + help
> +   Support for the VFIO Device Tree driver.  This is required to make
> +   use of platform devices present on Device Tree nodes using the VFIO
> +   framework.
> +
> +   If you don't know what to do here, say N.
> +
>  source "drivers/vfio/pci/Kconfig"
> diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
> index 2398d4a..d599a67 100644
> --- a/drivers/vfio/Makefile
> +++ b/drivers/vfio/Makefile
> @@ -1,3 +1,4 @@
>  obj-$(CONFIG_VFIO) += vfio.o
>  obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
>  obj-$(CONFIG_VFIO_PCI) += pci/
> +obj-$(CONFIG_VFIO_DT) += vfio_dt.o
> diff --git a/drivers/vfio/vfio_dt.c b/drivers/vfio/vfio_dt.c
> new file mode 100644
> index 000..ad4d31d
> --- /dev/null
> +++ b/drivers/vfio/vfio_dt.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (C) 2013 - Virtual Open Systems
> + * Author: Antonios Motakis 
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License, 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.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "Antonios Motakis "
> +#define DRIVER_DESC "VFIO Device Tree devices - User Level meta-driver"
> +
> +struct vfio_dt_device {
> + struct platform_device  *pdev;
> +};
> +
> +static void vfio_dt_release(void *device_data)
> +{
> + module_put(THIS_MODULE);
> +}
> +
> +static int vfio_dt_open(void *device_data)
> +{
> + if (!try_module_get(THIS_MODULE))
> + return -ENODEV;
> +
> + return 0;
> +}
> +
> +static long vfio_dt_ioctl(void *device_data,
> +unsigned int cmd, unsigned long arg)
> +{
> + struct vfio_dt_device *vdev = device_data;
> + unsigned long minsz;
> +
> + if (cmd == VFIO_DEVICE_GET_INFO) {
> + struct vfio_device_info info;
> +
> + minsz = offsetofend(struct vfio_device_info, num_irqs);
> +
> + if (copy_from_user(&info, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (info.argsz < minsz)
> + return -EINVAL;
> +
> + info.flags = VFIO_DEVICE_FLAGS_DT;
> + info.num_regions = 0;
> + info.num_irqs = 0;
> +
> + return copy_to_user((void __user *)arg, &info, minsz);
> +
> + } else if (cmd == VFIO_DEVICE_GET_REGION_INFO)
> + return -EINVAL;
> +
> + else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> + return -EINVAL;
> +
> + else if (cmd == VFIO_DEVICE_SET_IRQS)
> + return -EINVAL;
> +
> + else if (cmd == VFIO_DEVICE_RESET)
> + return -EINVAL;
> +
> + return -ENOTTY;
> +}
> +
> +static ssize_t vfio_dt_read(void *device_data, char __user *buf,
> +  size_t count, loff_t *ppos)
> +{
> + return 0;
> +}
> +
> +static ssize_t vfio_dt_write(void *device_data, const char __user *buf,
> +   size_t count, loff_t *ppos)
> +{
> + return 0;
> +}
> +
> +static int vfio_dt_mmap(void *device_data, struct vm_area_struct *vma)
> +{
> + return -EINVAL;
> +}
> +
> +static const struct vfio_device_ops vfio_dt_ops = {
> + .name   = "vfio-dts",
> + .open   = vfio_dt_open,
> + .release= vfio_dt_release,
> + .ioctl  = vfio_dt_ioctl,
> + .read   = vfio_dt_re