[RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Antonios Motakis]

This is a preview of the base work, towards VFIO support on ARM platforms
with an IOMMU. It forms a base on to which to implement the functionality
necessary to enable using device tree devices on ARM (and other platforms
based on device trees) with VFIO.

This patch series has been subjected to limited testing on the Arndale board
(with the Exynos 5250 System MMU). More extensive testing will follow as more
features are implemented.

It depends on Cho KyongHo's patch series "iommu/exynos: Fixes and Enhancements
of System MMU driver with DT", applied on a Linux 3.10.1 kernel, and also my
own "iommu/exynos: add devices attached to the System MMU to an IOMMU group".
Those patches are required at least in order to test the proposed module on
Arndale.

The API used is identical to the existing VFIO API that is also used with
PCI devices. Only devices that include a basic set of IRQs and memory regions
are targeted; devices with complicated relationships with other devices on the
device tree are not taken into account at this stage.

The following IOCTLs have been found to be working on the Arndale with no
changes to VFIO:
 - VFIO_GET_API_VERSION
 - VFIO_CHECK_EXTENSION
The TYPE1 fix proposed here enables the following IOCTLs:
 - VFIO_GROUP_GET_STATUS
 - VFIO_GROUP_SET_CONTAINER
 - VFIO_SET_IOMMU
 - VFIO_IOMMU_GET_INFO
 - VFIO_IOMMU_MAP_DMA
The VFIO platform driver proposed here implements the following:
 - VFIO_GROUP_GET_DEVICE_FD
 - VFIO_DEVICE_GET_INFO
 - VFIO_DEVICE_GET_REGION_INFO
 - VFIO_DEVICE_GET_IRQ_INFO
In addition, the VFIO platform driver implements the following through
the VFIO device file descriptor:
 - MMAPing memory regions to the virtual address space of the VFIO user.
 - Read / write of memory regions directly through the file descriptor.

What still needs to be done, includes:
 - IRQs / IRQFD support.
 - The API is not currently extended with device tree specific information.
 - Proper binding of the VFIO_DT driver to devices; currently to test the
   driver, one has to edit the device tree and add "vfio" to the
   compatible property. However Linux does not support OF drivers that
   can be dynamically bound to any device.
 - More extensive testing with a functioning device doing DMA.
 - QEMU / KVM support.

Antonios Motakis (7):
  VFIO_IOMMU_TYPE1 workaround to build for platform devices
  Initial skeleton of VFIO support for Device Tree based devices
  Return info for device and its memory regions and interrupts
  VFIO: DT: Support MMAP of MMIO regions
  VFIO: DT: Read and write support for the device fd
  VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver
  VFIO: VFIO_PLATFORM: Update documentation for platform specific
    devices

 Documentation/vfio.txt          |  25 +++-
 drivers/vfio/Kconfig            |  13 +-
 drivers/vfio/Makefile           |   1 +
 drivers/vfio/vfio_iommu_type1.c |  22 ++-
 drivers/vfio/vfio_platform.c    | 308 ++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h       |   1 +
 6 files changed, 361 insertions(+), 9 deletions(-)
 create mode 100644 drivers/vfio/vfio_platform.c

-- 
1.8.1.2

[PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Antonios Motakis]

This is a workaround to make the VFIO_IOMMU_TYPE1 driver usable with
platform devices instead of PCI. A future permanent fix should support
both. This is required in order to use the Exynos SMMU, or ARM SMMU
driver with VFIO.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/Kconfig            |  2 +-
 drivers/vfio/vfio_iommu_type1.c | 22 ++++++++++++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 7cd5dec..1f84eda 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -6,7 +6,7 @@ config VFIO_IOMMU_TYPE1
 menuconfig VFIO
 	tristate "VFIO Non-Privileged userspace driver framework"
 	depends on IOMMU_API
-	select VFIO_IOMMU_TYPE1 if X86
+	select VFIO_IOMMU_TYPE1 if X86 || ARM
 	help
 	  VFIO provides a framework for secure userspace device drivers.
 	  See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 6f3fbc4..d7e6a12 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -30,7 +30,8 @@
 #include <linux/iommu.h>
 #include <linux/module.h>
 #include <linux/mm.h>
-#include <linux/pci.h>		/* pci_bus_type */
+#include <linux/pci.h>			/* pci_bus_type */
+#include <linux/platform_device.h>	/* platform_bus_type */
 #include <linux/sched.h>
 #include <linux/slab.h>
 #include <linux/uaccess.h>
@@ -46,6 +47,8 @@ module_param_named(allow_unsafe_interrupts,
 		   allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
 MODULE_PARM_DESC(allow_unsafe_interrupts,
 		 "Enable VFIO IOMMU support for on platforms without interrupt remapping support.");
+static struct bus_type *iommu_bus_type = NULL;
+static bool require_cap_intr_remap = false;
 
 struct vfio_iommu {
 	struct iommu_domain	*domain;
@@ -612,7 +615,8 @@ static void *vfio_iommu_type1_open(unsigned long arg)
 	/*
 	 * Wish we didn't have to know about bus_type here.
 	 */
-	iommu->domain = iommu_domain_alloc(&pci_bus_type);
+	iommu->domain = iommu_domain_alloc(iommu_bus_type);
+
 	if (!iommu->domain) {
 		kfree(iommu);
 		return ERR_PTR(-EIO);
@@ -624,7 +628,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
 	 * the way.  Fortunately we know interrupt remapping is global for
 	 * our iommus.
 	 */
-	if (!allow_unsafe_interrupts &&
+	if (require_cap_intr_remap && !allow_unsafe_interrupts &&
 	    !iommu_domain_has_cap(iommu->domain, IOMMU_CAP_INTR_REMAP)) {
 		pr_warn("%s: No interrupt remapping support.  Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
 		       __func__);
@@ -733,7 +737,17 @@ static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
 
 static int __init vfio_iommu_type1_init(void)
 {
-	if (!iommu_present(&pci_bus_type))
+#ifdef CONFIG_PCI
+	if (iommu_present(&pci_bus_type)) {
+		iommu_bus_type = &pci_bus_type;
+		/* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
+		require_cap_intr_remap = true;
+	}
+#endif
+	if (!iommu_bus_type && iommu_present(&platform_bus_type))
+		iommu_bus_type = &platform_bus_type;
+
+	if(!iommu_bus_type)
 		return -ENODEV;
 
 	return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
-- 
1.8.1.2

Re: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Alex Williamson]

On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
> This is a workaround to make the VFIO_IOMMU_TYPE1 driver usable with
> platform devices instead of PCI. A future permanent fix should support
> both. This is required in order to use the Exynos SMMU, or ARM SMMU
> driver with VFIO.
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  drivers/vfio/Kconfig            |  2 +-
>  drivers/vfio/vfio_iommu_type1.c | 22 ++++++++++++++++++----
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 7cd5dec..1f84eda 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -6,7 +6,7 @@ config VFIO_IOMMU_TYPE1
>  menuconfig VFIO
>  	tristate "VFIO Non-Privileged userspace driver framework"
>  	depends on IOMMU_API
> -	select VFIO_IOMMU_TYPE1 if X86
> +	select VFIO_IOMMU_TYPE1 if X86 || ARM
>  	help
>  	  VFIO provides a framework for secure userspace device drivers.
>  	  See Documentation/vfio.txt for more details.
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 6f3fbc4..d7e6a12 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -30,7 +30,8 @@
>  #include <linux/iommu.h>
>  #include <linux/module.h>
>  #include <linux/mm.h>
> -#include <linux/pci.h>		/* pci_bus_type */
> +#include <linux/pci.h>			/* pci_bus_type */
> +#include <linux/platform_device.h>	/* platform_bus_type */
>  #include <linux/sched.h>
>  #include <linux/slab.h>
>  #include <linux/uaccess.h>
> @@ -46,6 +47,8 @@ module_param_named(allow_unsafe_interrupts,
>  		   allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
>  MODULE_PARM_DESC(allow_unsafe_interrupts,
>  		 "Enable VFIO IOMMU support for on platforms without interrupt remapping support.");
> +static struct bus_type *iommu_bus_type = NULL;
> +static bool require_cap_intr_remap = false;
>  
>  struct vfio_iommu {
>  	struct iommu_domain	*domain;
> @@ -612,7 +615,8 @@ static void *vfio_iommu_type1_open(unsigned long arg)
>  	/*
>  	 * Wish we didn't have to know about bus_type here.
>  	 */
> -	iommu->domain = iommu_domain_alloc(&pci_bus_type);
> +	iommu->domain = iommu_domain_alloc(iommu_bus_type);
> +
>  	if (!iommu->domain) {
>  		kfree(iommu);
>  		return ERR_PTR(-EIO);
> @@ -624,7 +628,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
>  	 * the way.  Fortunately we know interrupt remapping is global for
>  	 * our iommus.
>  	 */
> -	if (!allow_unsafe_interrupts &&
> +	if (require_cap_intr_remap && !allow_unsafe_interrupts &&
>  	    !iommu_domain_has_cap(iommu->domain, IOMMU_CAP_INTR_REMAP)) {
>  		pr_warn("%s: No interrupt remapping support.  Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
>  		       __func__);
> @@ -733,7 +737,17 @@ static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
>  
>  static int __init vfio_iommu_type1_init(void)
>  {
> -	if (!iommu_present(&pci_bus_type))
> +#ifdef CONFIG_PCI
> +	if (iommu_present(&pci_bus_type)) {
> +		iommu_bus_type = &pci_bus_type;
> +		/* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
> +		require_cap_intr_remap = true;
> +	}
> +#endif
> +	if (!iommu_bus_type && iommu_present(&platform_bus_type))
> +		iommu_bus_type = &platform_bus_type;
> +
> +	if(!iommu_bus_type)
>  		return -ENODEV;
>  
>  	return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);

Is it possible to have a system with both PCI and platform devices?  How
would you support that?  Thanks,

Alex

Re: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Antonios Motakis]

On Mon, Sep 30, 2013 at 7:39 PM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
> > This is a workaround to make the VFIO_IOMMU_TYPE1 driver usable with
> > platform devices instead of PCI. A future permanent fix should support
> > both. This is required in order to use the Exynos SMMU, or ARM SMMU
> > driver with VFIO.
> >
> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > ---
> >  drivers/vfio/Kconfig            |  2 +-
> >  drivers/vfio/vfio_iommu_type1.c | 22 ++++++++++++++++++----
> >  2 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> > index 7cd5dec..1f84eda 100644
> > --- a/drivers/vfio/Kconfig
> > +++ b/drivers/vfio/Kconfig
> > @@ -6,7 +6,7 @@ config VFIO_IOMMU_TYPE1
> >  menuconfig VFIO
> >       tristate "VFIO Non-Privileged userspace driver framework"
> >       depends on IOMMU_API
> > -     select VFIO_IOMMU_TYPE1 if X86
> > +     select VFIO_IOMMU_TYPE1 if X86 || ARM
> >       help
> >         VFIO provides a framework for secure userspace device drivers.
> >         See Documentation/vfio.txt for more details.
> > diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> > index 6f3fbc4..d7e6a12 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -30,7 +30,8 @@
> >  #include <linux/iommu.h>
> >  #include <linux/module.h>
> >  #include <linux/mm.h>
> > -#include <linux/pci.h>               /* pci_bus_type */
> > +#include <linux/pci.h>                       /* pci_bus_type */
> > +#include <linux/platform_device.h>   /* platform_bus_type */
> >  #include <linux/sched.h>
> >  #include <linux/slab.h>
> >  #include <linux/uaccess.h>
> > @@ -46,6 +47,8 @@ module_param_named(allow_unsafe_interrupts,
> >                  allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
> >  MODULE_PARM_DESC(allow_unsafe_interrupts,
> >                "Enable VFIO IOMMU support for on platforms without interrupt remapping support.");
> > +static struct bus_type *iommu_bus_type = NULL;
> > +static bool require_cap_intr_remap = false;
> >
> >  struct vfio_iommu {
> >       struct iommu_domain     *domain;
> > @@ -612,7 +615,8 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> >       /*
> >        * Wish we didn't have to know about bus_type here.
> >        */
> > -     iommu->domain = iommu_domain_alloc(&pci_bus_type);
> > +     iommu->domain = iommu_domain_alloc(iommu_bus_type);
> > +
> >       if (!iommu->domain) {
> >               kfree(iommu);
> >               return ERR_PTR(-EIO);
> > @@ -624,7 +628,7 @@ static void *vfio_iommu_type1_open(unsigned long arg)
> >        * the way.  Fortunately we know interrupt remapping is global for
> >        * our iommus.
> >        */
> > -     if (!allow_unsafe_interrupts &&
> > +     if (require_cap_intr_remap && !allow_unsafe_interrupts &&
> >           !iommu_domain_has_cap(iommu->domain, IOMMU_CAP_INTR_REMAP)) {
> >               pr_warn("%s: No interrupt remapping support.  Use the module param \"allow_unsafe_interrupts\" to enable VFIO IOMMU support on this platform\n",
> >                      __func__);
> > @@ -733,7 +737,17 @@ static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
> >
> >  static int __init vfio_iommu_type1_init(void)
> >  {
> > -     if (!iommu_present(&pci_bus_type))
> > +#ifdef CONFIG_PCI
> > +     if (iommu_present(&pci_bus_type)) {
> > +             iommu_bus_type = &pci_bus_type;
> > +             /* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
> > +             require_cap_intr_remap = true;
> > +     }
> > +#endif
> > +     if (!iommu_bus_type && iommu_present(&platform_bus_type))
> > +             iommu_bus_type = &platform_bus_type;
> > +
> > +     if(!iommu_bus_type)
> >               return -ENODEV;
> >
> >       return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
>
> Is it possible to have a system with both PCI and platform devices?  How
> would you support that?  Thanks,
>
> Alex
>

It looks like it would take more work to support IOMMUs on both at the
same time, esp. since we need to know the right bus type.

However, I don't know of a system with an IOMMU on both PCI and a
platform bus. A better solution might be found before we can talk
about upstreaming, of course.

RE: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Yoder Stuart-B08248]

> >  static int __init vfio_iommu_type1_init(void)
> >  {
> > -	if (!iommu_present(&pci_bus_type))
> > +#ifdef CONFIG_PCI
> > +	if (iommu_present(&pci_bus_type)) {
> > +		iommu_bus_type = &pci_bus_type;
> > +		/* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
> > +		require_cap_intr_remap = true;
> > +	}
> > +#endif
> > +	if (!iommu_bus_type && iommu_present(&platform_bus_type))
> > +		iommu_bus_type = &platform_bus_type;
> > +
> > +	if(!iommu_bus_type)
> >  		return -ENODEV;
> >
> >  	return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
> 
> Is it possible to have a system with both PCI and platform devices?  How
> would you support that?  Thanks,

It most certainly is a requirement to support both.  This is how
all of our (FSL) SoCs will expect to work.

I need to understand better why 'iommu_bus_type' exists at
all...

Stuart

Re: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Antonios Motakis]

On Tue, Oct 1, 2013 at 9:21 PM, Yoder Stuart-B08248
<B08248-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> >  static int __init vfio_iommu_type1_init(void)
>> >  {
>> > -   if (!iommu_present(&pci_bus_type))
>> > +#ifdef CONFIG_PCI
>> > +   if (iommu_present(&pci_bus_type)) {
>> > +           iommu_bus_type = &pci_bus_type;
>> > +           /* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
>> > +           require_cap_intr_remap = true;
>> > +   }
>> > +#endif
>> > +   if (!iommu_bus_type && iommu_present(&platform_bus_type))
>> > +           iommu_bus_type = &platform_bus_type;
>> > +
>> > +   if(!iommu_bus_type)
>> >             return -ENODEV;
>> >
>> >     return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
>>
>> Is it possible to have a system with both PCI and platform devices?  How
>> would you support that?  Thanks,
>
> It most certainly is a requirement to support both.  This is how
> all of our (FSL) SoCs will expect to work.
>

Both with an IOMMU present?

> I need to understand better why 'iommu_bus_type' exists at
> all...

It exists because the API expects to know the bus type. Ideally, this
would be solved at the IOMMU API level, and we would be able to be
completely bus agnostic in the TYPE1 driver.

>
> Stuart

RE: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Yoder Stuart-B08248]

> -----Original Message-----
> From: Antonios Motakis [mailto:a.motakis@virtualopensystems.com]
> Sent: Wednesday, October 02, 2013 6:14 AM
> To: Yoder Stuart-B08248
> Cc: Alex Williamson; kvmarm@lists.cs.columbia.edu; iommu@lists.linux-
> foundation.org; linux-samsung-soc@vger.kernel.org; kvm@vger.kernel.org;
> agraf@suse.de; tech@virtualopensystems.com; kim.phillips@linaro.org
> Subject: Re: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for
> platform devices
> 
> On Tue, Oct 1, 2013 at 9:21 PM, Yoder Stuart-B08248
> <B08248@freescale.com> wrote:
> >> >  static int __init vfio_iommu_type1_init(void)
> >> >  {
> >> > -   if (!iommu_present(&pci_bus_type))
> >> > +#ifdef CONFIG_PCI
> >> > +   if (iommu_present(&pci_bus_type)) {
> >> > +           iommu_bus_type = &pci_bus_type;
> >> > +           /* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
> >> > +           require_cap_intr_remap = true;
> >> > +   }
> >> > +#endif
> >> > +   if (!iommu_bus_type && iommu_present(&platform_bus_type))
> >> > +           iommu_bus_type = &platform_bus_type;
> >> > +
> >> > +   if(!iommu_bus_type)
> >> >             return -ENODEV;
> >> >
> >> >     return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
> >>
> >> Is it possible to have a system with both PCI and platform devices?
> How
> >> would you support that?  Thanks,
> >
> > It most certainly is a requirement to support both.  This is how
> > all of our (FSL) SoCs will expect to work.
> >
> 
> Both with an IOMMU present?

Yes.  We expect to be able to pass through PCI devices and
platform devices at the same time...all with IOMMU isolation.

Stuart

RE: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Yoder Stuart-B08248]

> -----Original Message-----
> From: Antonios Motakis [mailto:a.motakis@virtualopensystems.com]
> Sent: Wednesday, October 02, 2013 6:14 AM
> To: Yoder Stuart-B08248
> Cc: Alex Williamson; kvmarm@lists.cs.columbia.edu; iommu@lists.linux-
> foundation.org; linux-samsung-soc@vger.kernel.org; kvm@vger.kernel.org;
> agraf@suse.de; tech@virtualopensystems.com; kim.phillips@linaro.org
> Subject: Re: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for
> platform devices
> 
> On Tue, Oct 1, 2013 at 9:21 PM, Yoder Stuart-B08248
> <B08248@freescale.com> wrote:
> >> >  static int __init vfio_iommu_type1_init(void)
> >> >  {
> >> > -   if (!iommu_present(&pci_bus_type))
> >> > +#ifdef CONFIG_PCI
> >> > +   if (iommu_present(&pci_bus_type)) {
> >> > +           iommu_bus_type = &pci_bus_type;
> >> > +           /* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
> >> > +           require_cap_intr_remap = true;
> >> > +   }
> >> > +#endif
> >> > +   if (!iommu_bus_type && iommu_present(&platform_bus_type))
> >> > +           iommu_bus_type = &platform_bus_type;
> >> > +
> >> > +   if(!iommu_bus_type)
> >> >             return -ENODEV;
> >> >
> >> >     return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
> >>
> >> Is it possible to have a system with both PCI and platform devices?
> How
> >> would you support that?  Thanks,
> >
> > It most certainly is a requirement to support both.  This is how
> > all of our (FSL) SoCs will expect to work.
> >
> 
> Both with an IOMMU present?

Yes.

Stuart

Re: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Alexander Graf]

On 01.10.2013, at 21:21, Yoder Stuart-B08248 wrote:

>>> static int __init vfio_iommu_type1_init(void)
>>> {
>>> -	if (!iommu_present(&pci_bus_type))
>>> +#ifdef CONFIG_PCI
>>> +	if (iommu_present(&pci_bus_type)) {
>>> +		iommu_bus_type = &pci_bus_type;
>>> +		/* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
>>> +		require_cap_intr_remap = true;
>>> +	}
>>> +#endif
>>> +	if (!iommu_bus_type && iommu_present(&platform_bus_type))
>>> +		iommu_bus_type = &platform_bus_type;
>>> +
>>> +	if(!iommu_bus_type)
>>> 		return -ENODEV;
>>> 
>>> 	return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
>> 
>> Is it possible to have a system with both PCI and platform devices?  How
>> would you support that?  Thanks,
> 
> It most certainly is a requirement to support both.  This is how
> all of our (FSL) SoCs will expect to work.

I thought the PCI bus emits a cookie that the system wide IOMMU can then use to differentiate the origin of the transaction? So the same IOMMU can be used for PCI as well as platform routing.


Alex

Re: [PATCH 1/7] VFIO_IOMMU_TYPE1 workaround to build for platform devices

[Don Dutile]

On 10/02/2013 08:14 AM, Alexander Graf wrote:
>
> On 01.10.2013, at 21:21, Yoder Stuart-B08248 wrote:
>
>>>> static int __init vfio_iommu_type1_init(void)
>>>> {
>>>> -	if (!iommu_present(&pci_bus_type))
>>>> +#ifdef CONFIG_PCI
>>>> +	if (iommu_present(&pci_bus_type)) {
>>>> +		iommu_bus_type =&pci_bus_type;
>>>> +		/* For PCI targets, IOMMU_CAP_INTR_REMAP is required */
>>>> +		require_cap_intr_remap = true;
>>>> +	}
>>>> +#endif
>>>> +	if (!iommu_bus_type&&  iommu_present(&platform_bus_type))
>>>> +		iommu_bus_type =&platform_bus_type;
>>>> +
>>>> +	if(!iommu_bus_type)
>>>> 		return -ENODEV;
>>>>
>>>> 	return vfio_register_iommu_driver(&vfio_iommu_driver_ops_type1);
>>>
>>> Is it possible to have a system with both PCI and platform devices?  How
>>> would you support that?  Thanks,
>>
>> It most certainly is a requirement to support both.  This is how
>> all of our (FSL) SoCs will expect to work.
>
> I thought the PCI bus emits a cookie that the system wide IOMMU can then use to differentiate the origin of the transaction? So the same IOMMU can be used for PCI as well as platform routing.
>
*can* be the same IOMMU, yes;
have to, no, so there can be multiple IOMMUs of different types.

>
> Alex
>
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/7] Initial skeleton of VFIO support for Device Tree based devices

[Antonios Motakis]

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 <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/Kconfig         |  11 +++
 drivers/vfio/Makefile        |   1 +
 drivers/vfio/vfio_platform.c | 187 +++++++++++++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h    |   1 +
 4 files changed, 200 insertions(+)
 create mode 100644 drivers/vfio/vfio_platform.c

diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 1f84eda..35254b7 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -13,4 +13,15 @@ menuconfig VFIO
 
 	  If you don't know what to do here, say N.
 
+config VFIO_PLATFORM
+	tristate "VFIO support for device tree based platform devices"
+	depends on VFIO && EVENTFD && OF
+	help
+	  Support for platform devices with VFIO. This is required to make
+	  use of platform devices present on device tree nodes using the VFIO
+	  framework. Devices that are not described in the device tree cannot
+	  be used by this driver.
+
+	  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..575c8dd 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_PLATFORM) += vfio_platform.o
diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
new file mode 100644
index 0000000..b9686b0
--- /dev/null
+++ b/drivers/vfio/vfio_platform.c
@@ -0,0 +1,187 @@
+/*
+ * Copyright (C) 2013 - Virtual Open Systems
+ * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
+ *
+ * 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 <linux/device.h>
+#include <linux/eventfd.h>
+#include <linux/interrupt.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/notifier.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/uaccess.h>
+#include <linux/vfio.h>
+
+#define DRIVER_VERSION  "0.1"
+#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
+#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
+
+struct vfio_platform_device {
+	struct platform_device	*pdev;
+};
+
+static void vfio_platform_release(void *device_data)
+{
+	module_put(THIS_MODULE);
+}
+
+static int vfio_platform_open(void *device_data)
+{
+	if (!try_module_get(THIS_MODULE))
+		return -ENODEV;
+
+	return 0;
+}
+
+static long vfio_platform_ioctl(void *device_data,
+			   unsigned int cmd, unsigned long arg)
+{
+	struct vfio_platform_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_PLATFORM;
+		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_platform_read(void *device_data, char __user *buf,
+			     size_t count, loff_t *ppos)
+{
+	return 0;
+}
+
+static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
+			      size_t count, loff_t *ppos)
+{
+	return 0;
+}
+
+static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
+{
+	return -EINVAL;
+}
+
+static const struct vfio_device_ops vfio_platform_ops = {
+	.name		= "vfio-platform",
+	.open		= vfio_platform_open,
+	.release	= vfio_platform_release,
+	.ioctl		= vfio_platform_ioctl,
+	.read		= vfio_platform_read,
+	.write		= vfio_platform_write,
+	.mmap		= vfio_platform_mmap,
+};
+
+static int vfio_platform_probe(struct platform_device *pdev)
+{
+	struct vfio_platform_device *vdev;
+	struct iommu_group *group;
+	int ret;
+
+	group = iommu_group_get(&pdev->dev);
+	if (!group) {
+		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
+		return -EINVAL;
+	}
+
+	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
+	if (!vdev) {
+		iommu_group_put(group);
+		return -ENOMEM;
+	}
+
+	vdev->pdev = pdev;
+
+	ret = vfio_add_group_dev(&pdev->dev, &vfio_platform_ops, vdev);
+	if (ret) {
+		iommu_group_put(group);
+		kfree(vdev);
+	}
+
+	return ret;
+}
+
+static int vfio_platform_remove(struct platform_device *pdev)
+{
+	struct vfio_platform_device *vdev;
+
+	vdev = vfio_del_group_dev(&pdev->dev);
+	if (!vdev)
+		return -EINVAL;
+
+	iommu_group_put(pdev->dev.iommu_group);
+	kfree(vdev);
+
+	return 0;
+}
+
+static const struct of_device_id vfio_platform_match[] = {
+	/* Before this series can be applied, we shall 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",
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(of, vfio_platform_match);
+
+static struct platform_driver vfio_platform_driver = {
+	.probe		= vfio_platform_probe,
+	.remove		= vfio_platform_remove,
+	.driver	= {
+		.name	= "vfio-platform",
+		.owner	= THIS_MODULE,
+		.of_match_table = vfio_platform_match,
+	},
+};
+
+module_platform_driver(vfio_platform_driver);
+
+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..8d7434d 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_PLATFORM (1 << 2)	/* vfio-platform device */
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };
-- 
1.8.1.2

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

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> bounces@lists.linux-foundation.org] On Behalf Of Antonios Motakis
> Sent: Monday, September 30, 2013 8:59 PM
> To: kvmarm@lists.cs.columbia.edu; alex.williamson@redhat.com
> Cc: linux-samsung-soc@vger.kernel.org; kvm@vger.kernel.org; agraf@suse.de; Yoder
> Stuart-B08248; iommu@lists.linux-foundation.org; Antonios Motakis;
> tech@virtualopensystems.com
> Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree based
> devices
> 
> 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 <a.motakis@virtualopensystems.com>
> ---
>  drivers/vfio/Kconfig         |  11 +++
>  drivers/vfio/Makefile        |   1 +
>  drivers/vfio/vfio_platform.c | 187 +++++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vfio.h    |   1 +
>  4 files changed, 200 insertions(+)
>  create mode 100644 drivers/vfio/vfio_platform.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 1f84eda..35254b7
> 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -13,4 +13,15 @@ menuconfig VFIO
> 
>  	  If you don't know what to do here, say N.
> 
> +config VFIO_PLATFORM
> +	tristate "VFIO support for device tree based platform devices"
> +	depends on VFIO && EVENTFD && OF
> +	help
> +	  Support for platform devices with VFIO. This is required to make
> +	  use of platform devices present on device tree nodes using the VFIO
> +	  framework. Devices that are not described in the device tree cannot
> +	  be used by this driver.
> +
> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
> diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c new

We can make this parallel to PCI, something like drivers/vfio/platform/platform.c

-Bharat

> file mode 100644 index 0000000..b9686b0
> --- /dev/null
> +++ b/drivers/vfio/vfio_platform.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (C) 2013 - Virtual Open Systems
> + * Author: Antonios Motakis <a.motakis@virtualopensystems.com>
> + *
> + * 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 <linux/device.h>
> +#include <linux/eventfd.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +#include <linux/vfio.h>
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis@virtualopensystems.com>"
> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
> +
> +struct vfio_platform_device {
> +	struct platform_device	*pdev;
> +};
> +
> +static void vfio_platform_release(void *device_data) {
> +	module_put(THIS_MODULE);
> +}
> +
> +static int vfio_platform_open(void *device_data) {
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static long vfio_platform_ioctl(void *device_data,
> +			   unsigned int cmd, unsigned long arg) {
> +	struct vfio_platform_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_PLATFORM;
> +		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_platform_read(void *device_data, char __user *buf,
> +			     size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
> +			      size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static int vfio_platform_mmap(void *device_data, struct vm_area_struct
> +*vma) {
> +	return -EINVAL;
> +}
> +
> +static const struct vfio_device_ops vfio_platform_ops = {
> +	.name		= "vfio-platform",
> +	.open		= vfio_platform_open,
> +	.release	= vfio_platform_release,
> +	.ioctl		= vfio_platform_ioctl,
> +	.read		= vfio_platform_read,
> +	.write		= vfio_platform_write,
> +	.mmap		= vfio_platform_mmap,
> +};
> +
> +static int vfio_platform_probe(struct platform_device *pdev) {
> +	struct vfio_platform_device *vdev;
> +	struct iommu_group *group;
> +	int ret;
> +
> +	group = iommu_group_get(&pdev->dev);
> +	if (!group) {
> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
> +		return -EINVAL;
> +	}
> +
> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> +	if (!vdev) {
> +		iommu_group_put(group);
> +		return -ENOMEM;
> +	}
> +
> +	vdev->pdev = pdev;
> +
> +	ret = vfio_add_group_dev(&pdev->dev, &vfio_platform_ops, vdev);
> +	if (ret) {
> +		iommu_group_put(group);
> +		kfree(vdev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int vfio_platform_remove(struct platform_device *pdev) {
> +	struct vfio_platform_device *vdev;
> +
> +	vdev = vfio_del_group_dev(&pdev->dev);
> +	if (!vdev)
> +		return -EINVAL;
> +
> +	iommu_group_put(pdev->dev.iommu_group);
> +	kfree(vdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id vfio_platform_match[] = {
> +	/* Before this series can be applied, we shall 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",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, vfio_platform_match);
> +
> +static struct platform_driver vfio_platform_driver = {
> +	.probe		= vfio_platform_probe,
> +	.remove		= vfio_platform_remove,
> +	.driver	= {
> +		.name	= "vfio-platform",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = vfio_platform_match,
> +	},
> +};
> +
> +module_platform_driver(vfio_platform_driver);
> +
> +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..8d7434d 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_PLATFORM (1 << 2)	/* vfio-platform device */
>  	__u32	num_regions;	/* Max region index + 1 */
>  	__u32	num_irqs;	/* Max IRQ index + 1 */
>  };
> --
> 1.8.1.2
> 
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Don Dutile]

On 09/30/2013 11:37 AM, Bhushan Bharat-R65777 wrote:
>
>
>> -----Original Message-----
>> From: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org [mailto:iommu-
>> bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org] On Behalf Of Antonios Motakis
>> Sent: Monday, September 30, 2013 8:59 PM
>> To: kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
>> Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; agraf-l3A5Bk7waGM@public.gmane.org; Yoder
>> Stuart-B08248; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Antonios Motakis;
>> tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
>> Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree based
>> devices
>>
>> 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<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>> ---
>>   drivers/vfio/Kconfig         |  11 +++
>>   drivers/vfio/Makefile        |   1 +
>>   drivers/vfio/vfio_platform.c | 187 +++++++++++++++++++++++++++++++++++++++++++
>>   include/uapi/linux/vfio.h    |   1 +
>>   4 files changed, 200 insertions(+)
>>   create mode 100644 drivers/vfio/vfio_platform.c
>>
>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 1f84eda..35254b7
>> 100644
>> --- a/drivers/vfio/Kconfig
>> +++ b/drivers/vfio/Kconfig
>> @@ -13,4 +13,15 @@ menuconfig VFIO
>>
>>   	  If you don't know what to do here, say N.
>>
>> +config VFIO_PLATFORM
>> +	tristate "VFIO support for device tree based platform devices"
>> +	depends on VFIO&&  EVENTFD&&  OF
>> +	help
>> +	  Support for platform devices with VFIO. This is required to make
>> +	  use of platform devices present on device tree nodes using the VFIO
>> +	  framework. Devices that are not described in the device tree cannot
>> +	  be used by this driver.
>> +
>> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
>> diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c new
>
> We can make this parallel to PCI, something like drivers/vfio/platform/platform.c
>
pls, no.  'platform' is too generic, and it really means 'arm-dt' ... so can
move it to the arch/arm space, and have it's kconfig conditional on ARM&&VFIO.
if kept under drivers/vfio, then use a better directory name that ties it to arm-dt.
thanks.
- Don


> -Bharat
>
>> file mode 100644 index 0000000..b9686b0
>> --- /dev/null
>> +++ b/drivers/vfio/vfio_platform.c
>> @@ -0,0 +1,187 @@
>> +/*
>> + * Copyright (C) 2013 - Virtual Open Systems
>> + * Author: Antonios Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>> + *
>> + * 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<linux/device.h>
>> +#include<linux/eventfd.h>
>> +#include<linux/interrupt.h>
>> +#include<linux/iommu.h>
>> +#include<linux/module.h>
>> +#include<linux/mutex.h>
>> +#include<linux/notifier.h>
>> +#include<linux/pm_runtime.h>
>> +#include<linux/slab.h>
>> +#include<linux/types.h>
>> +#include<linux/uaccess.h>
>> +#include<linux/vfio.h>
>> +
>> +#define DRIVER_VERSION  "0.1"
>> +#define DRIVER_AUTHOR   "Antonios Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
>> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
>> +
>> +struct vfio_platform_device {
>> +	struct platform_device	*pdev;
>> +};
>> +
>> +static void vfio_platform_release(void *device_data) {
>> +	module_put(THIS_MODULE);
>> +}
>> +
>> +static int vfio_platform_open(void *device_data) {
>> +	if (!try_module_get(THIS_MODULE))
>> +		return -ENODEV;
>> +
>> +	return 0;
>> +}
>> +
>> +static long vfio_platform_ioctl(void *device_data,
>> +			   unsigned int cmd, unsigned long arg) {
>> +	struct vfio_platform_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_PLATFORM;
>> +		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_platform_read(void *device_data, char __user *buf,
>> +			     size_t count, loff_t *ppos)
>> +{
>> +	return 0;
>> +}
>> +
>> +static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
>> +			      size_t count, loff_t *ppos)
>> +{
>> +	return 0;
>> +}
>> +
>> +static int vfio_platform_mmap(void *device_data, struct vm_area_struct
>> +*vma) {
>> +	return -EINVAL;
>> +}
>> +
>> +static const struct vfio_device_ops vfio_platform_ops = {
>> +	.name		= "vfio-platform",
>> +	.open		= vfio_platform_open,
>> +	.release	= vfio_platform_release,
>> +	.ioctl		= vfio_platform_ioctl,
>> +	.read		= vfio_platform_read,
>> +	.write		= vfio_platform_write,
>> +	.mmap		= vfio_platform_mmap,
>> +};
>> +
>> +static int vfio_platform_probe(struct platform_device *pdev) {
>> +	struct vfio_platform_device *vdev;
>> +	struct iommu_group *group;
>> +	int ret;
>> +
>> +	group = iommu_group_get(&pdev->dev);
>> +	if (!group) {
>> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
>> +		return -EINVAL;
>> +	}
>> +
>> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
>> +	if (!vdev) {
>> +		iommu_group_put(group);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	vdev->pdev = pdev;
>> +
>> +	ret = vfio_add_group_dev(&pdev->dev,&vfio_platform_ops, vdev);
>> +	if (ret) {
>> +		iommu_group_put(group);
>> +		kfree(vdev);
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int vfio_platform_remove(struct platform_device *pdev) {
>> +	struct vfio_platform_device *vdev;
>> +
>> +	vdev = vfio_del_group_dev(&pdev->dev);
>> +	if (!vdev)
>> +		return -EINVAL;
>> +
>> +	iommu_group_put(pdev->dev.iommu_group);
>> +	kfree(vdev);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct of_device_id vfio_platform_match[] = {
>> +	/* Before this series can be applied, we shall 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",
>> +	},
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, vfio_platform_match);
>> +
>> +static struct platform_driver vfio_platform_driver = {
>> +	.probe		= vfio_platform_probe,
>> +	.remove		= vfio_platform_remove,
>> +	.driver	= {
>> +		.name	= "vfio-platform",
>> +		.owner	= THIS_MODULE,
>> +		.of_match_table = vfio_platform_match,
>> +	},
>> +};
>> +
>> +module_platform_driver(vfio_platform_driver);
>> +
>> +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..8d7434d 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_PLATFORM (1<<  2)	/* vfio-platform device */
>>   	__u32	num_regions;	/* Max region index + 1 */
>>   	__u32	num_irqs;	/* Max IRQ index + 1 */
>>   };
>> --
>> 1.8.1.2
>>
>> _______________________________________________
>> iommu mailing list
>> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
>
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

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

[Alex Williamson]

On Mon, 2013-10-28 at 21:29 -0400, Don Dutile wrote:
> On 09/30/2013 11:37 AM, Bhushan Bharat-R65777 wrote:
> >
> >
> >> -----Original Message-----
> >> From: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org [mailto:iommu-
> >> bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org] On Behalf Of Antonios Motakis
> >> Sent: Monday, September 30, 2013 8:59 PM
> >> To: kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> >> Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; agraf-l3A5Bk7waGM@public.gmane.org; Yoder
> >> Stuart-B08248; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Antonios Motakis;
> >> tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
> >> Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree based
> >> devices
> >>
> >> 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<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> >> ---
> >>   drivers/vfio/Kconfig         |  11 +++
> >>   drivers/vfio/Makefile        |   1 +
> >>   drivers/vfio/vfio_platform.c | 187 +++++++++++++++++++++++++++++++++++++++++++
> >>   include/uapi/linux/vfio.h    |   1 +
> >>   4 files changed, 200 insertions(+)
> >>   create mode 100644 drivers/vfio/vfio_platform.c
> >>
> >> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 1f84eda..35254b7
> >> 100644
> >> --- a/drivers/vfio/Kconfig
> >> +++ b/drivers/vfio/Kconfig
> >> @@ -13,4 +13,15 @@ menuconfig VFIO
> >>
> >>   	  If you don't know what to do here, say N.
> >>
> >> +config VFIO_PLATFORM
> >> +	tristate "VFIO support for device tree based platform devices"
> >> +	depends on VFIO&&  EVENTFD&&  OF
> >> +	help
> >> +	  Support for platform devices with VFIO. This is required to make
> >> +	  use of platform devices present on device tree nodes using the VFIO
> >> +	  framework. Devices that are not described in the device tree cannot
> >> +	  be used by this driver.
> >> +
> >> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
> >> diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c new
> >
> > We can make this parallel to PCI, something like drivers/vfio/platform/platform.c
> >
> pls, no.  'platform' is too generic, and it really means 'arm-dt' ... so can
> move it to the arch/arm space, and have it's kconfig conditional on ARM&&VFIO.
> if kept under drivers/vfio, then use a better directory name that ties it to arm-dt.
> thanks.

The intention is that vfio platform device support is not arm-dt
specific.  This is to be used by both arm and embedded ppc.  The devices
we intend to support with them are known as platform drivers in the
kernel, thus the name.  I suppose the question remains whether the
interface here is really generic for any "platform" device or whether
we're making whether we're making an interface specifically for device
tree platform devices, or if those are one in the same.  In any case,
arm-dt is certainly not the answer.

Alex

> >> file mode 100644 index 0000000..b9686b0
> >> --- /dev/null
> >> +++ b/drivers/vfio/vfio_platform.c
> >> @@ -0,0 +1,187 @@
> >> +/*
> >> + * Copyright (C) 2013 - Virtual Open Systems
> >> + * Author: Antonios Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> >> + *
> >> + * 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<linux/device.h>
> >> +#include<linux/eventfd.h>
> >> +#include<linux/interrupt.h>
> >> +#include<linux/iommu.h>
> >> +#include<linux/module.h>
> >> +#include<linux/mutex.h>
> >> +#include<linux/notifier.h>
> >> +#include<linux/pm_runtime.h>
> >> +#include<linux/slab.h>
> >> +#include<linux/types.h>
> >> +#include<linux/uaccess.h>
> >> +#include<linux/vfio.h>
> >> +
> >> +#define DRIVER_VERSION  "0.1"
> >> +#define DRIVER_AUTHOR   "Antonios Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> >> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
> >> +
> >> +struct vfio_platform_device {
> >> +	struct platform_device	*pdev;
> >> +};
> >> +
> >> +static void vfio_platform_release(void *device_data) {
> >> +	module_put(THIS_MODULE);
> >> +}
> >> +
> >> +static int vfio_platform_open(void *device_data) {
> >> +	if (!try_module_get(THIS_MODULE))
> >> +		return -ENODEV;
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static long vfio_platform_ioctl(void *device_data,
> >> +			   unsigned int cmd, unsigned long arg) {
> >> +	struct vfio_platform_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_PLATFORM;
> >> +		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_platform_read(void *device_data, char __user *buf,
> >> +			     size_t count, loff_t *ppos)
> >> +{
> >> +	return 0;
> >> +}
> >> +
> >> +static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
> >> +			      size_t count, loff_t *ppos)
> >> +{
> >> +	return 0;
> >> +}
> >> +
> >> +static int vfio_platform_mmap(void *device_data, struct vm_area_struct
> >> +*vma) {
> >> +	return -EINVAL;
> >> +}
> >> +
> >> +static const struct vfio_device_ops vfio_platform_ops = {
> >> +	.name		= "vfio-platform",
> >> +	.open		= vfio_platform_open,
> >> +	.release	= vfio_platform_release,
> >> +	.ioctl		= vfio_platform_ioctl,
> >> +	.read		= vfio_platform_read,
> >> +	.write		= vfio_platform_write,
> >> +	.mmap		= vfio_platform_mmap,
> >> +};
> >> +
> >> +static int vfio_platform_probe(struct platform_device *pdev) {
> >> +	struct vfio_platform_device *vdev;
> >> +	struct iommu_group *group;
> >> +	int ret;
> >> +
> >> +	group = iommu_group_get(&pdev->dev);
> >> +	if (!group) {
> >> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
> >> +		return -EINVAL;
> >> +	}
> >> +
> >> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> >> +	if (!vdev) {
> >> +		iommu_group_put(group);
> >> +		return -ENOMEM;
> >> +	}
> >> +
> >> +	vdev->pdev = pdev;
> >> +
> >> +	ret = vfio_add_group_dev(&pdev->dev,&vfio_platform_ops, vdev);
> >> +	if (ret) {
> >> +		iommu_group_put(group);
> >> +		kfree(vdev);
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +
> >> +static int vfio_platform_remove(struct platform_device *pdev) {
> >> +	struct vfio_platform_device *vdev;
> >> +
> >> +	vdev = vfio_del_group_dev(&pdev->dev);
> >> +	if (!vdev)
> >> +		return -EINVAL;
> >> +
> >> +	iommu_group_put(pdev->dev.iommu_group);
> >> +	kfree(vdev);
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +static const struct of_device_id vfio_platform_match[] = {
> >> +	/* Before this series can be applied, we shall 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",
> >> +	},
> >> +	{},
> >> +};
> >> +MODULE_DEVICE_TABLE(of, vfio_platform_match);
> >> +
> >> +static struct platform_driver vfio_platform_driver = {
> >> +	.probe		= vfio_platform_probe,
> >> +	.remove		= vfio_platform_remove,
> >> +	.driver	= {
> >> +		.name	= "vfio-platform",
> >> +		.owner	= THIS_MODULE,
> >> +		.of_match_table = vfio_platform_match,
> >> +	},
> >> +};
> >> +
> >> +module_platform_driver(vfio_platform_driver);
> >> +
> >> +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..8d7434d 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_PLATFORM (1<<  2)	/* vfio-platform device */
> >>   	__u32	num_regions;	/* Max region index + 1 */
> >>   	__u32	num_irqs;	/* Max IRQ index + 1 */
> >>   };
> >> --
> >> 1.8.1.2
> >>
> >> _______________________________________________
> >> iommu mailing list
> >> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> >> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >
> >
> > _______________________________________________
> > iommu mailing list
> > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: kvm-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:kvm-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of
> Alex Williamson
> Sent: Tuesday, October 29, 2013 5:17 PM
> To: Don Dutile
> Cc: Bhushan Bharat-R65777; Antonios Motakis; kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org;
> linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; agraf-l3A5Bk7waGM@public.gmane.org; Yoder
> Stuart-B08248; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
> Subject: Re: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree based
> devices
> 
> On Mon, 2013-10-28 at 21:29 -0400, Don Dutile wrote:
> > On 09/30/2013 11:37 AM, Bhushan Bharat-R65777 wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org [mailto:iommu-
> > >> bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org] On Behalf Of Antonios Motakis
> > >> Sent: Monday, September 30, 2013 8:59 PM
> > >> To: kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> > >> Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > >> agraf-l3A5Bk7waGM@public.gmane.org; Yoder Stuart-B08248;
> > >> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Antonios Motakis;
> > >> tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
> > >> Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device
> > >> Tree based devices
> > >>
> > >> 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<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > >> ---
> > >>   drivers/vfio/Kconfig         |  11 +++
> > >>   drivers/vfio/Makefile        |   1 +
> > >>   drivers/vfio/vfio_platform.c | 187
> +++++++++++++++++++++++++++++++++++++++++++
> > >>   include/uapi/linux/vfio.h    |   1 +
> > >>   4 files changed, 200 insertions(+)
> > >>   create mode 100644 drivers/vfio/vfio_platform.c
> > >>
> > >> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index
> > >> 1f84eda..35254b7
> > >> 100644
> > >> --- a/drivers/vfio/Kconfig
> > >> +++ b/drivers/vfio/Kconfig
> > >> @@ -13,4 +13,15 @@ menuconfig VFIO
> > >>
> > >>   	  If you don't know what to do here, say N.
> > >>
> > >> +config VFIO_PLATFORM
> > >> +	tristate "VFIO support for device tree based platform devices"
> > >> +	depends on VFIO&&  EVENTFD&&  OF
> > >> +	help
> > >> +	  Support for platform devices with VFIO. This is required to make
> > >> +	  use of platform devices present on device tree nodes using the VFIO
> > >> +	  framework. Devices that are not described in the device tree cannot
> > >> +	  be used by this driver.
> > >> +
> > >> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
> > >> diff --git a/drivers/vfio/vfio_platform.c
> > >> b/drivers/vfio/vfio_platform.c new
> > >
> > > We can make this parallel to PCI, something like
> > > drivers/vfio/platform/platform.c
> > >
> > pls, no.  'platform' is too generic, and it really means 'arm-dt' ...
> > so can move it to the arch/arm space, and have it's kconfig conditional on
> ARM&&VFIO.
> > if kept under drivers/vfio, then use a better directory name that ties it to
> arm-dt.
> > thanks.
> 
> The intention is that vfio platform device support is not arm-dt specific.  This
> is to be used by both arm and embedded ppc.  The devices we intend to support
> with them are known as platform drivers in the kernel, thus the name.  I suppose
> the question remains whether the interface here is really generic for any
> "platform" device or whether we're making whether we're making an interface
> specifically for device tree platform devices, or if those are one in the same.
> In any case, arm-dt is certainly not the answer.

I think we should stick to "platform".

-Bharat

> 
> Alex
> 
> > >> file mode 100644 index 0000000..b9686b0
> > >> --- /dev/null
> > >> +++ b/drivers/vfio/vfio_platform.c
> > >> @@ -0,0 +1,187 @@
> > >> +/*
> > >> + * Copyright (C) 2013 - Virtual Open Systems
> > >> + * Author: Antonios Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > >> + *
> > >> + * 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<linux/device.h>
> > >> +#include<linux/eventfd.h>
> > >> +#include<linux/interrupt.h>
> > >> +#include<linux/iommu.h>
> > >> +#include<linux/module.h>
> > >> +#include<linux/mutex.h>
> > >> +#include<linux/notifier.h>
> > >> +#include<linux/pm_runtime.h>
> > >> +#include<linux/slab.h>
> > >> +#include<linux/types.h>
> > >> +#include<linux/uaccess.h>
> > >> +#include<linux/vfio.h>
> > >> +
> > >> +#define DRIVER_VERSION  "0.1"
> > >> +#define DRIVER_AUTHOR   "Antonios
> Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> > >> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-
> driver"
> > >> +
> > >> +struct vfio_platform_device {
> > >> +	struct platform_device	*pdev;
> > >> +};
> > >> +
> > >> +static void vfio_platform_release(void *device_data) {
> > >> +	module_put(THIS_MODULE);
> > >> +}
> > >> +
> > >> +static int vfio_platform_open(void *device_data) {
> > >> +	if (!try_module_get(THIS_MODULE))
> > >> +		return -ENODEV;
> > >> +
> > >> +	return 0;
> > >> +}
> > >> +
> > >> +static long vfio_platform_ioctl(void *device_data,
> > >> +			   unsigned int cmd, unsigned long arg) {
> > >> +	struct vfio_platform_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_PLATFORM;
> > >> +		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_platform_read(void *device_data, char __user *buf,
> > >> +			     size_t count, loff_t *ppos) {
> > >> +	return 0;
> > >> +}
> > >> +
> > >> +static ssize_t vfio_platform_write(void *device_data, const char __user
> *buf,
> > >> +			      size_t count, loff_t *ppos) {
> > >> +	return 0;
> > >> +}
> > >> +
> > >> +static int vfio_platform_mmap(void *device_data, struct
> > >> +vm_area_struct
> > >> +*vma) {
> > >> +	return -EINVAL;
> > >> +}
> > >> +
> > >> +static const struct vfio_device_ops vfio_platform_ops = {
> > >> +	.name		= "vfio-platform",
> > >> +	.open		= vfio_platform_open,
> > >> +	.release	= vfio_platform_release,
> > >> +	.ioctl		= vfio_platform_ioctl,
> > >> +	.read		= vfio_platform_read,
> > >> +	.write		= vfio_platform_write,
> > >> +	.mmap		= vfio_platform_mmap,
> > >> +};
> > >> +
> > >> +static int vfio_platform_probe(struct platform_device *pdev) {
> > >> +	struct vfio_platform_device *vdev;
> > >> +	struct iommu_group *group;
> > >> +	int ret;
> > >> +
> > >> +	group = iommu_group_get(&pdev->dev);
> > >> +	if (!group) {
> > >> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
> > >> +		return -EINVAL;
> > >> +	}
> > >> +
> > >> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > >> +	if (!vdev) {
> > >> +		iommu_group_put(group);
> > >> +		return -ENOMEM;
> > >> +	}
> > >> +
> > >> +	vdev->pdev = pdev;
> > >> +
> > >> +	ret = vfio_add_group_dev(&pdev->dev,&vfio_platform_ops, vdev);
> > >> +	if (ret) {
> > >> +		iommu_group_put(group);
> > >> +		kfree(vdev);
> > >> +	}
> > >> +
> > >> +	return ret;
> > >> +}
> > >> +
> > >> +static int vfio_platform_remove(struct platform_device *pdev) {
> > >> +	struct vfio_platform_device *vdev;
> > >> +
> > >> +	vdev = vfio_del_group_dev(&pdev->dev);
> > >> +	if (!vdev)
> > >> +		return -EINVAL;
> > >> +
> > >> +	iommu_group_put(pdev->dev.iommu_group);
> > >> +	kfree(vdev);
> > >> +
> > >> +	return 0;
> > >> +}
> > >> +
> > >> +static const struct of_device_id vfio_platform_match[] = {
> > >> +	/* Before this series can be applied, we shall 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",
> > >> +	},
> > >> +	{},
> > >> +};
> > >> +MODULE_DEVICE_TABLE(of, vfio_platform_match);
> > >> +
> > >> +static struct platform_driver vfio_platform_driver = {
> > >> +	.probe		= vfio_platform_probe,
> > >> +	.remove		= vfio_platform_remove,
> > >> +	.driver	= {
> > >> +		.name	= "vfio-platform",
> > >> +		.owner	= THIS_MODULE,
> > >> +		.of_match_table = vfio_platform_match,
> > >> +	},
> > >> +};
> > >> +
> > >> +module_platform_driver(vfio_platform_driver);
> > >> +
> > >> +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..8d7434d 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_PLATFORM (1<<  2)	/* vfio-platform device */
> > >>   	__u32	num_regions;	/* Max region index + 1 */
> > >>   	__u32	num_irqs;	/* Max IRQ index + 1 */
> > >>   };
> > >> --
> > >> 1.8.1.2
> > >>
> > >> _______________________________________________
> > >> iommu mailing list
> > >> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> > >> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> > >
> > >
> > > _______________________________________________
> > > iommu mailing list
> > > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> > > https://lists.linuxfoundation.org/mailman/listinfo/iommu
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in the
> > body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> > http://vger.kernel.org/majordomo-info.html
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a
> message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

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

[Don Dutile]

On 10/29/2013 07:47 AM, Alex Williamson wrote:
> On Mon, 2013-10-28 at 21:29 -0400, Don Dutile wrote:
>> On 09/30/2013 11:37 AM, Bhushan Bharat-R65777 wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org [mailto:iommu-
>>>> bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org] On Behalf Of Antonios Motakis
>>>> Sent: Monday, September 30, 2013 8:59 PM
>>>> To: kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
>>>> Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; agraf-l3A5Bk7waGM@public.gmane.org; Yoder
>>>> Stuart-B08248; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Antonios Motakis;
>>>> tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
>>>> Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree based
>>>> devices
>>>>
>>>> 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<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>>>> ---
>>>>    drivers/vfio/Kconfig         |  11 +++
>>>>    drivers/vfio/Makefile        |   1 +
>>>>    drivers/vfio/vfio_platform.c | 187 +++++++++++++++++++++++++++++++++++++++++++
>>>>    include/uapi/linux/vfio.h    |   1 +
>>>>    4 files changed, 200 insertions(+)
>>>>    create mode 100644 drivers/vfio/vfio_platform.c
>>>>
>>>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index 1f84eda..35254b7
>>>> 100644
>>>> --- a/drivers/vfio/Kconfig
>>>> +++ b/drivers/vfio/Kconfig
>>>> @@ -13,4 +13,15 @@ menuconfig VFIO
>>>>
>>>>    	  If you don't know what to do here, say N.
>>>>
>>>> +config VFIO_PLATFORM
>>>> +	tristate "VFIO support for device tree based platform devices"
>>>> +	depends on VFIO&&   EVENTFD&&   OF
>>>> +	help
>>>> +	  Support for platform devices with VFIO. This is required to make
>>>> +	  use of platform devices present on device tree nodes using the VFIO
>>>> +	  framework. Devices that are not described in the device tree cannot
>>>> +	  be used by this driver.
>>>> +
>>>> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
>>>> diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c new
>>>
>>> We can make this parallel to PCI, something like drivers/vfio/platform/platform.c
>>>
>> pls, no.  'platform' is too generic, and it really means 'arm-dt' ... so can
>> move it to the arch/arm space, and have it's kconfig conditional on ARM&&VFIO.
>> if kept under drivers/vfio, then use a better directory name that ties it to arm-dt.
>> thanks.
>
> The intention is that vfio platform device support is not arm-dt
> specific.  This is to be used by both arm and embedded ppc.  The devices
> we intend to support with them are known as platform drivers in the
> kernel, thus the name.  I suppose the question remains whether the
> interface here is really generic for any "platform" device or whether
> we're making whether we're making an interface specifically for device
> tree platform devices, or if those are one in the same.  In any case,
> arm-dt is certainly not the answer.
>
> Alex
>
I thought that was the intention until I saw this use in platform.c:
	static const struct of_device_id vfio_platform_match[] = {

So, of_device_id hit me as DT-specific, and thus, the file should have
a name that implies its not as platform-generic as one may want/expect.
I agree that the 'arm' part can be dropped, but it's not of-agnostic atm.


>>>> file mode 100644 index 0000000..b9686b0
>>>> --- /dev/null
>>>> +++ b/drivers/vfio/vfio_platform.c
>>>> @@ -0,0 +1,187 @@
>>>> +/*
>>>> + * Copyright (C) 2013 - Virtual Open Systems
>>>> + * Author: Antonios Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
>>>> + *
>>>> + * 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<linux/device.h>
>>>> +#include<linux/eventfd.h>
>>>> +#include<linux/interrupt.h>
>>>> +#include<linux/iommu.h>
>>>> +#include<linux/module.h>
>>>> +#include<linux/mutex.h>
>>>> +#include<linux/notifier.h>
>>>> +#include<linux/pm_runtime.h>
>>>> +#include<linux/slab.h>
>>>> +#include<linux/types.h>
>>>> +#include<linux/uaccess.h>
>>>> +#include<linux/vfio.h>
>>>> +
>>>> +#define DRIVER_VERSION  "0.1"
>>>> +#define DRIVER_AUTHOR   "Antonios Motakis<a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
>>>> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
>>>> +
>>>> +struct vfio_platform_device {
>>>> +	struct platform_device	*pdev;
>>>> +};
>>>> +
>>>> +static void vfio_platform_release(void *device_data) {
>>>> +	module_put(THIS_MODULE);
>>>> +}
>>>> +
>>>> +static int vfio_platform_open(void *device_data) {
>>>> +	if (!try_module_get(THIS_MODULE))
>>>> +		return -ENODEV;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static long vfio_platform_ioctl(void *device_data,
>>>> +			   unsigned int cmd, unsigned long arg) {
>>>> +	struct vfio_platform_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_PLATFORM;
>>>> +		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_platform_read(void *device_data, char __user *buf,
>>>> +			     size_t count, loff_t *ppos)
>>>> +{
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
>>>> +			      size_t count, loff_t *ppos)
>>>> +{
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int vfio_platform_mmap(void *device_data, struct vm_area_struct
>>>> +*vma) {
>>>> +	return -EINVAL;
>>>> +}
>>>> +
>>>> +static const struct vfio_device_ops vfio_platform_ops = {
>>>> +	.name		= "vfio-platform",
>>>> +	.open		= vfio_platform_open,
>>>> +	.release	= vfio_platform_release,
>>>> +	.ioctl		= vfio_platform_ioctl,
>>>> +	.read		= vfio_platform_read,
>>>> +	.write		= vfio_platform_write,
>>>> +	.mmap		= vfio_platform_mmap,
>>>> +};
>>>> +
>>>> +static int vfio_platform_probe(struct platform_device *pdev) {
>>>> +	struct vfio_platform_device *vdev;
>>>> +	struct iommu_group *group;
>>>> +	int ret;
>>>> +
>>>> +	group = iommu_group_get(&pdev->dev);
>>>> +	if (!group) {
>>>> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
>>>> +	if (!vdev) {
>>>> +		iommu_group_put(group);
>>>> +		return -ENOMEM;
>>>> +	}
>>>> +
>>>> +	vdev->pdev = pdev;
>>>> +
>>>> +	ret = vfio_add_group_dev(&pdev->dev,&vfio_platform_ops, vdev);
>>>> +	if (ret) {
>>>> +		iommu_group_put(group);
>>>> +		kfree(vdev);
>>>> +	}
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>> +static int vfio_platform_remove(struct platform_device *pdev) {
>>>> +	struct vfio_platform_device *vdev;
>>>> +
>>>> +	vdev = vfio_del_group_dev(&pdev->dev);
>>>> +	if (!vdev)
>>>> +		return -EINVAL;
>>>> +
>>>> +	iommu_group_put(pdev->dev.iommu_group);
>>>> +	kfree(vdev);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static const struct of_device_id vfio_platform_match[] = {
>>>> +	/* Before this series can be applied, we shall 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",
>>>> +	},
>>>> +	{},
>>>> +};
>>>> +MODULE_DEVICE_TABLE(of, vfio_platform_match);
>>>> +
>>>> +static struct platform_driver vfio_platform_driver = {
>>>> +	.probe		= vfio_platform_probe,
>>>> +	.remove		= vfio_platform_remove,
>>>> +	.driver	= {
>>>> +		.name	= "vfio-platform",
>>>> +		.owner	= THIS_MODULE,
>>>> +		.of_match_table = vfio_platform_match,
>>>> +	},
>>>> +};
>>>> +
>>>> +module_platform_driver(vfio_platform_driver);
>>>> +
>>>> +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..8d7434d 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_PLATFORM (1<<   2)	/* vfio-platform device */
>>>>    	__u32	num_regions;	/* Max region index + 1 */
>>>>    	__u32	num_irqs;	/* Max IRQ index + 1 */
>>>>    };
>>>> --
>>>> 1.8.1.2
>>>>
>>>> _______________________________________________
>>>> iommu mailing list
>>>> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>>
>>>
>>> _______________________________________________
>>> iommu mailing list
>>> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>

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

[Yoder Stuart-B08248]

> -----Original Message-----
> From: Don Dutile [mailto:ddutile@redhat.com]
> Sent: Tuesday, October 29, 2013 12:13 PM
> To: Alex Williamson
> Cc: Bhushan Bharat-R65777; Antonios Motakis;
> kvmarm@lists.cs.columbia.edu; linux-samsung-soc@vger.kernel.org;
> kvm@vger.kernel.org; agraf@suse.de; Yoder Stuart-B08248;
> iommu@lists.linux-foundation.org; tech@virtualopensystems.com
> Subject: Re: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree
> based devices
> 
> On 10/29/2013 07:47 AM, Alex Williamson wrote:
> > On Mon, 2013-10-28 at 21:29 -0400, Don Dutile wrote:
> >> On 09/30/2013 11:37 AM, Bhushan Bharat-R65777 wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> >>>> bounces@lists.linux-foundation.org] On Behalf Of Antonios Motakis
> >>>> Sent: Monday, September 30, 2013 8:59 PM
> >>>> To: kvmarm@lists.cs.columbia.edu; alex.williamson@redhat.com
> >>>> Cc: linux-samsung-soc@vger.kernel.org; kvm@vger.kernel.org;
> agraf@suse.de; Yoder
> >>>> Stuart-B08248; iommu@lists.linux-foundation.org; Antonios Motakis;
> >>>> tech@virtualopensystems.com
> >>>> Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device
> Tree based
> >>>> devices
> >>>>
> >>>> 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<a.motakis@virtualopensystems.com>
> >>>> ---
> >>>>    drivers/vfio/Kconfig         |  11 +++
> >>>>    drivers/vfio/Makefile        |   1 +
> >>>>    drivers/vfio/vfio_platform.c | 187
> +++++++++++++++++++++++++++++++++++++++++++
> >>>>    include/uapi/linux/vfio.h    |   1 +
> >>>>    4 files changed, 200 insertions(+)
> >>>>    create mode 100644 drivers/vfio/vfio_platform.c
> >>>>
> >>>> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index
> 1f84eda..35254b7
> >>>> 100644
> >>>> --- a/drivers/vfio/Kconfig
> >>>> +++ b/drivers/vfio/Kconfig
> >>>> @@ -13,4 +13,15 @@ menuconfig VFIO
> >>>>
> >>>>    	  If you don't know what to do here, say N.
> >>>>
> >>>> +config VFIO_PLATFORM
> >>>> +	tristate "VFIO support for device tree based platform
> devices"
> >>>> +	depends on VFIO&&   EVENTFD&&   OF
> >>>> +	help
> >>>> +	  Support for platform devices with VFIO. This is required to
> make
> >>>> +	  use of platform devices present on device tree nodes using
> the VFIO
> >>>> +	  framework. Devices that are not described in the device
> tree cannot
> >>>> +	  be used by this driver.
> >>>> +
> >>>> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
> >>>> diff --git a/drivers/vfio/vfio_platform.c
> b/drivers/vfio/vfio_platform.c new
> >>>
> >>> We can make this parallel to PCI, something like
> drivers/vfio/platform/platform.c
> >>>
> >> pls, no.  'platform' is too generic, and it really means 'arm-dt' ...
> so can
> >> move it to the arch/arm space, and have it's kconfig conditional on
> ARM&&VFIO.
> >> if kept under drivers/vfio, then use a better directory name that ties
> it to arm-dt.
> >> thanks.
> >
> > The intention is that vfio platform device support is not arm-dt
> > specific.  This is to be used by both arm and embedded ppc.  The
> devices
> > we intend to support with them are known as platform drivers in the
> > kernel, thus the name.  I suppose the question remains whether the
> > interface here is really generic for any "platform" device or whether
> > we're making whether we're making an interface specifically for device
> > tree platform devices, or if those are one in the same.  In any case,
> > arm-dt is certainly not the answer.
> >
> > Alex
> >
> I thought that was the intention until I saw this use in platform.c:
> 	static const struct of_device_id vfio_platform_match[] = {
> 
> So, of_device_id hit me as DT-specific, and thus, the file should have
> a name that implies its not as platform-generic as one may want/expect.
> I agree that the 'arm' part can be dropped, but it's not of-agnostic atm.

That part of the patch needs to be re-worked...and the comment
indicates it is not final.  The vfio-platform driver should not register
to handle specific compatible strings.  Kim Phillips has a 
patch posted that adds 'wildcard' support to platform drivers
so that a driver can bind to any device.

Stuart

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

[Yoder Stuart-B08248]

> -----Original Message-----
> From: Don Dutile [mailto:ddutile@redhat.com]
> Sent: Monday, October 28, 2013 8:30 PM
> To: Bhushan Bharat-R65777
> Cc: Antonios Motakis; kvmarm@lists.cs.columbia.edu;
> alex.williamson@redhat.com; linux-samsung-soc@vger.kernel.org;
> kvm@vger.kernel.org; agraf@suse.de; Yoder Stuart-B08248;
> iommu@lists.linux-foundation.org; tech@virtualopensystems.com
> Subject: Re: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree
> based devices
> 
> On 09/30/2013 11:37 AM, Bhushan Bharat-R65777 wrote:
> >
> >
> >> -----Original Message-----
> >> From: iommu-bounces@lists.linux-foundation.org [mailto:iommu-
> >> bounces@lists.linux-foundation.org] On Behalf Of Antonios Motakis
> >> Sent: Monday, September 30, 2013 8:59 PM
> >> To: kvmarm@lists.cs.columbia.edu; alex.williamson@redhat.com
> >> Cc: linux-samsung-soc@vger.kernel.org; kvm@vger.kernel.org;
> agraf@suse.de; Yoder
> >> Stuart-B08248; iommu@lists.linux-foundation.org; Antonios Motakis;
> >> tech@virtualopensystems.com
> >> Subject: [PATCH 2/7] Initial skeleton of VFIO support for Device Tree
> based
> >> devices
> >>
> >> 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<a.motakis@virtualopensystems.com>
> >> ---
> >>   drivers/vfio/Kconfig         |  11 +++
> >>   drivers/vfio/Makefile        |   1 +
> >>   drivers/vfio/vfio_platform.c | 187
> +++++++++++++++++++++++++++++++++++++++++++
> >>   include/uapi/linux/vfio.h    |   1 +
> >>   4 files changed, 200 insertions(+)
> >>   create mode 100644 drivers/vfio/vfio_platform.c
> >>
> >> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig index
> 1f84eda..35254b7
> >> 100644
> >> --- a/drivers/vfio/Kconfig
> >> +++ b/drivers/vfio/Kconfig
> >> @@ -13,4 +13,15 @@ menuconfig VFIO
> >>
> >>   	  If you don't know what to do here, say N.
> >>
> >> +config VFIO_PLATFORM
> >> +	tristate "VFIO support for device tree based platform devices"
> >> +	depends on VFIO&&  EVENTFD&&  OF
> >> +	help
> >> +	  Support for platform devices with VFIO. This is required to make
> >> +	  use of platform devices present on device tree nodes using the
> VFIO
> >> +	  framework. Devices that are not described in the device tree
> cannot
> >> +	  be used by this driver.
> >> +
> >> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
> >> diff --git a/drivers/vfio/vfio_platform.c
> b/drivers/vfio/vfio_platform.c new
> >
> > We can make this parallel to PCI, something like
> drivers/vfio/platform/platform.c
> >
> pls, no.  'platform' is too generic, and it really means 'arm-dt' ... so
> can
> move it to the arch/arm space, and have it's kconfig conditional on
> ARM&&VFIO.
> if kept under drivers/vfio, then use a better directory name that ties it
> to arm-dt.
> thanks.

This has nothing to do with ARM specifically.  We need this for Power architecture
as well.

And the name should be 'platform' as that _is_ the name of the bus
that these devices sit on.

This has nothing to do with device tree's per se.  It so happens that
the device tree is parsed and devices discovered there are added
to the platform bus.

The intent is to allow devices on the platform bus to be passed through
to user space using vfio.

Stuart

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

[Alex Williamson]

On Mon, 2013-09-30 at 17:28 +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 <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  drivers/vfio/Kconfig         |  11 +++
>  drivers/vfio/Makefile        |   1 +
>  drivers/vfio/vfio_platform.c | 187 +++++++++++++++++++++++++++++++++++++++++++

I agree with Bharat, I'd probably make a platform sub-directory.  Like
pci, you'll probably have several support files by the end and it seems
like platform devices may need device specific handling in places, which
would also be cleaner to have in the directory.

>  include/uapi/linux/vfio.h    |   1 +
>  4 files changed, 200 insertions(+)
>  create mode 100644 drivers/vfio/vfio_platform.c
> 
> diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> index 1f84eda..35254b7 100644
> --- a/drivers/vfio/Kconfig
> +++ b/drivers/vfio/Kconfig
> @@ -13,4 +13,15 @@ menuconfig VFIO
>  
>  	  If you don't know what to do here, say N.
>  
> +config VFIO_PLATFORM
> +	tristate "VFIO support for device tree based platform devices"
> +	depends on VFIO && EVENTFD && OF
> +	help
> +	  Support for platform devices with VFIO. This is required to make
> +	  use of platform devices present on device tree nodes using the VFIO
> +	  framework. Devices that are not described in the device tree cannot
> +	  be used by this driver.
> +
> +	  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..575c8dd 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_PLATFORM) += vfio_platform.o
> diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
> new file mode 100644
> index 0000000..b9686b0
> --- /dev/null
> +++ b/drivers/vfio/vfio_platform.c
> @@ -0,0 +1,187 @@
> +/*
> + * Copyright (C) 2013 - Virtual Open Systems
> + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> + *
> + * 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 <linux/device.h>
> +#include <linux/eventfd.h>
> +#include <linux/interrupt.h>
> +#include <linux/iommu.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/notifier.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/slab.h>
> +#include <linux/types.h>
> +#include <linux/uaccess.h>
> +#include <linux/vfio.h>
> +
> +#define DRIVER_VERSION  "0.1"
> +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
> +
> +struct vfio_platform_device {
> +	struct platform_device	*pdev;
> +};
> +
> +static void vfio_platform_release(void *device_data)
> +{
> +	module_put(THIS_MODULE);
> +}
> +
> +static int vfio_platform_open(void *device_data)
> +{
> +	if (!try_module_get(THIS_MODULE))
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
> +static long vfio_platform_ioctl(void *device_data,
> +			   unsigned int cmd, unsigned long arg)
> +{
> +	struct vfio_platform_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_PLATFORM;
> +		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_platform_read(void *device_data, char __user *buf,
> +			     size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
> +			      size_t count, loff_t *ppos)
> +{
> +	return 0;
> +}
> +
> +static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
> +{
> +	return -EINVAL;
> +}
> +
> +static const struct vfio_device_ops vfio_platform_ops = {
> +	.name		= "vfio-platform",
> +	.open		= vfio_platform_open,
> +	.release	= vfio_platform_release,
> +	.ioctl		= vfio_platform_ioctl,
> +	.read		= vfio_platform_read,
> +	.write		= vfio_platform_write,
> +	.mmap		= vfio_platform_mmap,
> +};
> +
> +static int vfio_platform_probe(struct platform_device *pdev)
> +{
> +	struct vfio_platform_device *vdev;
> +	struct iommu_group *group;
> +	int ret;
> +
> +	group = iommu_group_get(&pdev->dev);
> +	if (!group) {
> +		pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
> +		return -EINVAL;
> +	}
> +
> +	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> +	if (!vdev) {
> +		iommu_group_put(group);
> +		return -ENOMEM;
> +	}
> +
> +	vdev->pdev = pdev;
> +
> +	ret = vfio_add_group_dev(&pdev->dev, &vfio_platform_ops, vdev);
> +	if (ret) {
> +		iommu_group_put(group);
> +		kfree(vdev);
> +	}
> +
> +	return ret;
> +}
> +
> +static int vfio_platform_remove(struct platform_device *pdev)
> +{
> +	struct vfio_platform_device *vdev;
> +
> +	vdev = vfio_del_group_dev(&pdev->dev);
> +	if (!vdev)
> +		return -EINVAL;
> +
> +	iommu_group_put(pdev->dev.iommu_group);
> +	kfree(vdev);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id vfio_platform_match[] = {
> +	/* Before this series can be applied, we shall 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",
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, vfio_platform_match);
> +
> +static struct platform_driver vfio_platform_driver = {
> +	.probe		= vfio_platform_probe,
> +	.remove		= vfio_platform_remove,
> +	.driver	= {
> +		.name	= "vfio-platform",
> +		.owner	= THIS_MODULE,
> +		.of_match_table = vfio_platform_match,
> +	},
> +};
> +
> +module_platform_driver(vfio_platform_driver);
> +
> +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..8d7434d 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_PLATFORM (1 << 2)	/* vfio-platform device */
>  	__u32	num_regions;	/* Max region index + 1 */
>  	__u32	num_irqs;	/* Max IRQ index + 1 */
>  };

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

[Antonios Motakis]

On Mon, Sep 30, 2013 at 9:41 PM, Alex Williamson
<alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> On Mon, 2013-09-30 at 17:28 +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 <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > ---
> >  drivers/vfio/Kconfig         |  11 +++
> >  drivers/vfio/Makefile        |   1 +
> >  drivers/vfio/vfio_platform.c | 187 +++++++++++++++++++++++++++++++++++++++++++
>
> I agree with Bharat, I'd probably make a platform sub-directory.  Like
> pci, you'll probably have several support files by the end and it seems
> like platform devices may need device specific handling in places, which
> would also be cleaner to have in the directory.
>
> >  include/uapi/linux/vfio.h    |   1 +
> >  4 files changed, 200 insertions(+)
> >  create mode 100644 drivers/vfio/vfio_platform.c
> >
> > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> > index 1f84eda..35254b7 100644
> > --- a/drivers/vfio/Kconfig
> > +++ b/drivers/vfio/Kconfig
> > @@ -13,4 +13,15 @@ menuconfig VFIO
> >
> >         If you don't know what to do here, say N.
> >
> > +config VFIO_PLATFORM
> > +     tristate "VFIO support for device tree based platform devices"
> > +     depends on VFIO && EVENTFD && OF
> > +     help
> > +       Support for platform devices with VFIO. This is required to make
> > +       use of platform devices present on device tree nodes using the VFIO
> > +       framework. Devices that are not described in the device tree cannot
> > +       be used by this driver.
> > +
> > +       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..575c8dd 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_PLATFORM) += vfio_platform.o
> > diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
> > new file mode 100644
> > index 0000000..b9686b0
> > --- /dev/null
> > +++ b/drivers/vfio/vfio_platform.c
> > @@ -0,0 +1,187 @@
> > +/*
> > + * Copyright (C) 2013 - Virtual Open Systems
> > + * Author: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > + *
> > + * 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 <linux/device.h>
> > +#include <linux/eventfd.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/iommu.h>
> > +#include <linux/module.h>
> > +#include <linux/mutex.h>
> > +#include <linux/notifier.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/slab.h>
> > +#include <linux/types.h>
> > +#include <linux/uaccess.h>
> > +#include <linux/vfio.h>
> > +
> > +#define DRIVER_VERSION  "0.1"
> > +#define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> > +#define DRIVER_DESC     "VFIO Device Tree devices - User Level meta-driver"
> > +
> > +struct vfio_platform_device {
> > +     struct platform_device  *pdev;
> > +};
> > +
> > +static void vfio_platform_release(void *device_data)
> > +{
> > +     module_put(THIS_MODULE);
> > +}
> > +
> > +static int vfio_platform_open(void *device_data)
> > +{
> > +     if (!try_module_get(THIS_MODULE))
> > +             return -ENODEV;
> > +
> > +     return 0;
> > +}
> > +
> > +static long vfio_platform_ioctl(void *device_data,
> > +                        unsigned int cmd, unsigned long arg)
> > +{
> > +     struct vfio_platform_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_PLATFORM;
> > +             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_platform_read(void *device_data, char __user *buf,
> > +                          size_t count, loff_t *ppos)
> > +{
> > +     return 0;
> > +}
> > +
> > +static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
> > +                           size_t count, loff_t *ppos)
> > +{
> > +     return 0;
> > +}
> > +
> > +static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
> > +{
> > +     return -EINVAL;
> > +}
> > +
> > +static const struct vfio_device_ops vfio_platform_ops = {
> > +     .name           = "vfio-platform",
> > +     .open           = vfio_platform_open,
> > +     .release        = vfio_platform_release,
> > +     .ioctl          = vfio_platform_ioctl,
> > +     .read           = vfio_platform_read,
> > +     .write          = vfio_platform_write,
> > +     .mmap           = vfio_platform_mmap,
> > +};
> > +
> > +static int vfio_platform_probe(struct platform_device *pdev)
> > +{
> > +     struct vfio_platform_device *vdev;
> > +     struct iommu_group *group;
> > +     int ret;
> > +
> > +     group = iommu_group_get(&pdev->dev);
> > +     if (!group) {
> > +             pr_err("VFIO: No IOMMU group for device %s\n", pdev->name);
> > +             return -EINVAL;
> > +     }
> > +
> > +     vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
> > +     if (!vdev) {
> > +             iommu_group_put(group);
> > +             return -ENOMEM;
> > +     }
> > +
> > +     vdev->pdev = pdev;
> > +
> > +     ret = vfio_add_group_dev(&pdev->dev, &vfio_platform_ops, vdev);
> > +     if (ret) {
> > +             iommu_group_put(group);
> > +             kfree(vdev);
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> > +static int vfio_platform_remove(struct platform_device *pdev)
> > +{
> > +     struct vfio_platform_device *vdev;
> > +
> > +     vdev = vfio_del_group_dev(&pdev->dev);
> > +     if (!vdev)
> > +             return -EINVAL;
> > +
> > +     iommu_group_put(pdev->dev.iommu_group);
> > +     kfree(vdev);
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id vfio_platform_match[] = {
> > +     /* Before this series can be applied, we shall 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",
> > +     },
> > +     {},
> > +};
> > +MODULE_DEVICE_TABLE(of, vfio_platform_match);
> > +
> > +static struct platform_driver vfio_platform_driver = {
> > +     .probe          = vfio_platform_probe,
> > +     .remove         = vfio_platform_remove,
> > +     .driver = {
> > +             .name   = "vfio-platform",
> > +             .owner  = THIS_MODULE,
> > +             .of_match_table = vfio_platform_match,
> > +     },
> > +};
> > +
> > +module_platform_driver(vfio_platform_driver);
> > +
> > +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..8d7434d 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_PLATFORM (1 << 2)  /* vfio-platform device */
> >       __u32   num_regions;    /* Max region index + 1 */
> >       __u32   num_irqs;       /* Max IRQ index + 1 */
> >  };
>
>
>

Ack, thanks.

[PATCH 3/7] Return info for device and its memory regions and interrupts

[Antonios Motakis]

A VFIO userspace driver will start by opening the VFIO device
that corresponds to an IOMMU group, and will use the ioctl interface
to get the basic device info, such as number of memory regions and
interrupts, and their properties.

This patch implements the IOCTLs:
 - VFIO_DEVICE_GET_INFO
 - VFIO_DEVICE_GET_REGION_INFO
 - VFIO_DEVICE_GET_IRQ_INFO

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/vfio_platform.c | 60 ++++++++++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
index b9686b0..a0abcfa 100644
--- a/drivers/vfio/vfio_platform.c
+++ b/drivers/vfio/vfio_platform.c
@@ -28,6 +28,10 @@
 #include <linux/types.h>
 #include <linux/uaccess.h>
 #include <linux/vfio.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
 
 #define DRIVER_VERSION  "0.1"
 #define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
@@ -54,10 +58,13 @@ static long vfio_platform_ioctl(void *device_data,
 			   unsigned int cmd, unsigned long arg)
 {
 	struct vfio_platform_device *vdev = device_data;
+	struct device_node *of_node = vdev->pdev->dev.of_node;
 	unsigned long minsz;
 
 	if (cmd == VFIO_DEVICE_GET_INFO) {
 		struct vfio_device_info info;
+		struct resource res;
+		int cnt = 0;
 
 		minsz = offsetofend(struct vfio_device_info, num_irqs);
 
@@ -68,18 +75,57 @@ static long vfio_platform_ioctl(void *device_data,
 			return -EINVAL;
 
 		info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
-		info.num_regions = 0;
-		info.num_irqs = 0;
+
+		while (!of_address_to_resource(of_node, cnt, &res))
+			cnt++;
+
+		info.num_regions = cnt;
+
+		info.num_irqs = of_irq_count(of_node);
 
 		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_REGION_INFO) {
+		struct vfio_region_info info;
+		struct resource res;
 
-	else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
-		return -EINVAL;
+		minsz = offsetofend(struct vfio_region_info, offset);
+
+		if (copy_from_user(&info, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (info.argsz < minsz)
+			return -EINVAL;
+
+		if(of_address_to_resource(of_node, info.index, &res))
+			return -EINVAL;
+
+		info.offset = res.start;	/* map phys addr with offset */
+		info.size = resource_size(&res);
+		info.flags = 0;
+
+		return copy_to_user((void __user *)arg, &info, minsz);
+
+	} else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
+		struct vfio_irq_info info;
+		struct resource res;
+
+		minsz = offsetofend(struct vfio_irq_info, count);
+
+		if (copy_from_user(&info, (void __user *)arg, minsz))
+			return -EFAULT;
+
+		if (info.argsz < minsz)
+			return -EINVAL;
+
+		of_irq_to_resource(of_node, info.index, &res);
+
+		info.flags = 0;
+		info.count = 1;
+
+		return copy_to_user((void __user *)arg, &info, minsz);
 
-	else if (cmd == VFIO_DEVICE_SET_IRQS)
+	} else if (cmd == VFIO_DEVICE_SET_IRQS)
 		return -EINVAL;
 
 	else if (cmd == VFIO_DEVICE_RESET)
-- 
1.8.1.2

RE: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Bhushan Bharat-R65777]

> -----Original Message-----
> From: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org [mailto:iommu-
> bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org] On Behalf Of Antonios Motakis
> Sent: Monday, September 30, 2013 8:59 PM
> To: kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; agraf-l3A5Bk7waGM@public.gmane.org; Yoder
> Stuart-B08248; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Antonios Motakis;
> tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
> Subject: [PATCH 3/7] Return info for device and its memory regions and
> interrupts
> 
> A VFIO userspace driver will start by opening the VFIO device that corresponds
> to an IOMMU group, and will use the ioctl interface to get the basic device
> info, such as number of memory regions and interrupts, and their properties.
> 
> This patch implements the IOCTLs:
>  - VFIO_DEVICE_GET_INFO
>  - VFIO_DEVICE_GET_REGION_INFO
>  - VFIO_DEVICE_GET_IRQ_INFO
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  drivers/vfio/vfio_platform.c | 60 ++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c index
> b9686b0..a0abcfa 100644
> --- a/drivers/vfio/vfio_platform.c
> +++ b/drivers/vfio/vfio_platform.c
> @@ -28,6 +28,10 @@
>  #include <linux/types.h>
>  #include <linux/uaccess.h>
>  #include <linux/vfio.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> 
>  #define DRIVER_VERSION  "0.1"
>  #define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> @@ -54,10 +58,13 @@ static long vfio_platform_ioctl(void *device_data,
>  			   unsigned int cmd, unsigned long arg)  {
>  	struct vfio_platform_device *vdev = device_data;
> +	struct device_node *of_node = vdev->pdev->dev.of_node;
>  	unsigned long minsz;
> 
>  	if (cmd == VFIO_DEVICE_GET_INFO) {
>  		struct vfio_device_info info;
> +		struct resource res;
> +		int cnt = 0;
> 
>  		minsz = offsetofend(struct vfio_device_info, num_irqs);
> 
> @@ -68,18 +75,57 @@ static long vfio_platform_ioctl(void *device_data,
>  			return -EINVAL;
> 
>  		info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
> -		info.num_regions = 0;
> -		info.num_irqs = 0;
> +
> +		while (!of_address_to_resource(of_node, cnt, &res))
> +			cnt++;
> +
> +		info.num_regions = cnt;
> +
> +		info.num_irqs = of_irq_count(of_node);
> 
>  		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_REGION_INFO) {
> +		struct vfio_region_info info;
> +		struct resource res;
> 
> -	else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> -		return -EINVAL;
> +		minsz = offsetofend(struct vfio_region_info, offset);
> +
> +		if (copy_from_user(&info, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (info.argsz < minsz)
> +			return -EINVAL;
> +
> +		if(of_address_to_resource(of_node, info.index, &res))
> +			return -EINVAL;
> +
> +		info.offset = res.start;	/* map phys addr with offset */
> +		info.size = resource_size(&res);
> +		info.flags = 0;
> +
> +		return copy_to_user((void __user *)arg, &info, minsz);
> +
> +	} else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
> +		struct vfio_irq_info info;
> +		struct resource res;
> +
> +		minsz = offsetofend(struct vfio_irq_info, count);
> +
> +		if (copy_from_user(&info, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (info.argsz < minsz)
> +			return -EINVAL;
> +
> +		of_irq_to_resource(of_node, info.index, &res);

Why are we calling the above function if not using res?

> +
> +		info.flags = 0;
> +		info.count = 1;

I believe count here is number of interrupts, and we can have devices with more than 1 interrupt.

-Bharat

> +
> +		return copy_to_user((void __user *)arg, &info, minsz);
> 
> -	else if (cmd == VFIO_DEVICE_SET_IRQS)
> +	} else if (cmd == VFIO_DEVICE_SET_IRQS)
>  		return -EINVAL;
> 
>  	else if (cmd == VFIO_DEVICE_RESET)
> --
> 1.8.1.2
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Antonios Motakis]

[-- Attachment #1.1: Type: text/plain, Size: 5987 bytes --]

On Mon, Sep 30, 2013 at 5:49 PM, Bhushan Bharat-R65777 <R65777-KZfg59tc24xl57MIdRCFDg@public.gmane.org
> wrote:

>
>
> > -----Original Message-----
> > From: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org [mailto:iommu-
> > bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org] On Behalf Of Antonios Motakis
> > Sent: Monday, September 30, 2013 8:59 PM
> > To: kvmarm-FPEHb7Xf0XXUo1n7N8X6UoWGPAHP3yOg@public.gmane.org; alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> > Cc: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> agraf-l3A5Bk7waGM@public.gmane.org; Yoder
> > Stuart-B08248; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; Antonios Motakis;
> > tech-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
> > Subject: [PATCH 3/7] Return info for device and its memory regions and
> > interrupts
> >
> > A VFIO userspace driver will start by opening the VFIO device that
> corresponds
> > to an IOMMU group, and will use the ioctl interface to get the basic
> device
> > info, such as number of memory regions and interrupts, and their
> properties.
> >
> > This patch implements the IOCTLs:
> >  - VFIO_DEVICE_GET_INFO
> >  - VFIO_DEVICE_GET_REGION_INFO
> >  - VFIO_DEVICE_GET_IRQ_INFO
> >
> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > ---
> >  drivers/vfio/vfio_platform.c | 60
> ++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 53 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
> index
> > b9686b0..a0abcfa 100644
> > --- a/drivers/vfio/vfio_platform.c
> > +++ b/drivers/vfio/vfio_platform.c
> > @@ -28,6 +28,10 @@
> >  #include <linux/types.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/vfio.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> >
> >  #define DRIVER_VERSION  "0.1"
> >  #define DRIVER_AUTHOR   "Antonios Motakis <
> a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> > @@ -54,10 +58,13 @@ static long vfio_platform_ioctl(void *device_data,
> >                          unsigned int cmd, unsigned long arg)  {
> >       struct vfio_platform_device *vdev = device_data;
> > +     struct device_node *of_node = vdev->pdev->dev.of_node;
> >       unsigned long minsz;
> >
> >       if (cmd == VFIO_DEVICE_GET_INFO) {
> >               struct vfio_device_info info;
> > +             struct resource res;
> > +             int cnt = 0;
> >
> >               minsz = offsetofend(struct vfio_device_info, num_irqs);
> >
> > @@ -68,18 +75,57 @@ static long vfio_platform_ioctl(void *device_data,
> >                       return -EINVAL;
> >
> >               info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
> > -             info.num_regions = 0;
> > -             info.num_irqs = 0;
> > +
> > +             while (!of_address_to_resource(of_node, cnt, &res))
> > +                     cnt++;
> > +
> > +             info.num_regions = cnt;
> > +
> > +             info.num_irqs = of_irq_count(of_node);
> >
> >               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_REGION_INFO) {
> > +             struct vfio_region_info info;
> > +             struct resource res;
> >
> > -     else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> > -             return -EINVAL;
> > +             minsz = offsetofend(struct vfio_region_info, offset);
> > +
> > +             if (copy_from_user(&info, (void __user *)arg, minsz))
> > +                     return -EFAULT;
> > +
> > +             if (info.argsz < minsz)
> > +                     return -EINVAL;
> > +
> > +             if(of_address_to_resource(of_node, info.index, &res))
> > +                     return -EINVAL;
> > +
> > +             info.offset = res.start;        /* map phys addr with
> offset */
> > +             info.size = resource_size(&res);
> > +             info.flags = 0;
> > +
> > +             return copy_to_user((void __user *)arg, &info, minsz);
> > +
> > +     } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
> > +             struct vfio_irq_info info;
> > +             struct resource res;
> > +
> > +             minsz = offsetofend(struct vfio_irq_info, count);
> > +
> > +             if (copy_from_user(&info, (void __user *)arg, minsz))
> > +                     return -EFAULT;
> > +
> > +             if (info.argsz < minsz)
> > +                     return -EINVAL;
> > +
> > +             of_irq_to_resource(of_node, info.index, &res);
>
> Why are we calling the above function if not using res?
>

Good point, it is out of place now in this series. Though I will probably
reintroduce it when IRQ supports starts being implemented.


>
> > +
> > +             info.flags = 0;
> > +             info.count = 1;
>
> I believe count here is number of interrupts, and we can have devices with
> more than 1 interrupt.
>
>
Count here is not the number of interrupts of the device; if a device has
multiple interrupts, then we return multiple IRQ indexes. Count here is the
number of consecutive interrupts within the same index, which we don't use.
This is used for MSI-like interrupt blocks, but for platform devices we
simply use a distinct index per interrupt.


> -Bharat
>
> > +
> > +             return copy_to_user((void __user *)arg, &info, minsz);
> >
> > -     else if (cmd == VFIO_DEVICE_SET_IRQS)
> > +     } else if (cmd == VFIO_DEVICE_SET_IRQS)
> >               return -EINVAL;
> >
> >       else if (cmd == VFIO_DEVICE_RESET)
> > --
> > 1.8.1.2
> >
> > _______________________________________________
> > iommu mailing list
> > iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> > https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 8707 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Alex Williamson]

On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
> A VFIO userspace driver will start by opening the VFIO device
> that corresponds to an IOMMU group, and will use the ioctl interface
> to get the basic device info, such as number of memory regions and
> interrupts, and their properties.
> 
> This patch implements the IOCTLs:
>  - VFIO_DEVICE_GET_INFO
>  - VFIO_DEVICE_GET_REGION_INFO
>  - VFIO_DEVICE_GET_IRQ_INFO
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  drivers/vfio/vfio_platform.c | 60 ++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 53 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
> index b9686b0..a0abcfa 100644
> --- a/drivers/vfio/vfio_platform.c
> +++ b/drivers/vfio/vfio_platform.c
> @@ -28,6 +28,10 @@
>  #include <linux/types.h>
>  #include <linux/uaccess.h>
>  #include <linux/vfio.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
>  
>  #define DRIVER_VERSION  "0.1"
>  #define DRIVER_AUTHOR   "Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> @@ -54,10 +58,13 @@ static long vfio_platform_ioctl(void *device_data,
>  			   unsigned int cmd, unsigned long arg)
>  {
>  	struct vfio_platform_device *vdev = device_data;
> +	struct device_node *of_node = vdev->pdev->dev.of_node;
>  	unsigned long minsz;
>  
>  	if (cmd == VFIO_DEVICE_GET_INFO) {
>  		struct vfio_device_info info;
> +		struct resource res;
> +		int cnt = 0;
>  
>  		minsz = offsetofend(struct vfio_device_info, num_irqs);
>  
> @@ -68,18 +75,57 @@ static long vfio_platform_ioctl(void *device_data,
>  			return -EINVAL;
>  
>  		info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
> -		info.num_regions = 0;
> -		info.num_irqs = 0;
> +
> +		while (!of_address_to_resource(of_node, cnt, &res))
> +			cnt++;
> +
> +		info.num_regions = cnt;
> +
> +		info.num_irqs = of_irq_count(of_node);
>  
>  		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_REGION_INFO) {
> +		struct vfio_region_info info;
> +		struct resource res;
>  
> -	else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> -		return -EINVAL;
> +		minsz = offsetofend(struct vfio_region_info, offset);
> +
> +		if (copy_from_user(&info, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (info.argsz < minsz)
> +			return -EINVAL;
> +
> +		if(of_address_to_resource(of_node, info.index, &res))
> +			return -EINVAL;
> +
> +		info.offset = res.start;	/* map phys addr with offset */
> +		info.size = resource_size(&res);
> +		info.flags = 0;
> +
> +		return copy_to_user((void __user *)arg, &info, minsz);
> +
> +	} else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
> +		struct vfio_irq_info info;
> +		struct resource res;
> +
> +		minsz = offsetofend(struct vfio_irq_info, count);
> +
> +		if (copy_from_user(&info, (void __user *)arg, minsz))
> +			return -EFAULT;
> +
> +		if (info.argsz < minsz)
> +			return -EINVAL;
> +
> +		of_irq_to_resource(of_node, info.index, &res);
> +
> +		info.flags = 0;
> +		info.count = 1;
> +
> +		return copy_to_user((void __user *)arg, &info, minsz);
>  
> -	else if (cmd == VFIO_DEVICE_SET_IRQS)
> +	} else if (cmd == VFIO_DEVICE_SET_IRQS)
>  		return -EINVAL;
>  
>  	else if (cmd == VFIO_DEVICE_RESET)

I notice all the open firmware calls here and I'm curious, will all
platform devices be making use of open firmware?  I don't know if this
is synonymous with device tree or not.  Thanks,

Alex 

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Antonios Motakis]

[-- Attachment #1.1: Type: text/plain, Size: 5021 bytes --]

On Mon, Sep 30, 2013 at 7:39 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> wrote:

> On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
> > A VFIO userspace driver will start by opening the VFIO device
> > that corresponds to an IOMMU group, and will use the ioctl interface
> > to get the basic device info, such as number of memory regions and
> > interrupts, and their properties.
> >
> > This patch implements the IOCTLs:
> >  - VFIO_DEVICE_GET_INFO
> >  - VFIO_DEVICE_GET_REGION_INFO
> >  - VFIO_DEVICE_GET_IRQ_INFO
> >
> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > ---
> >  drivers/vfio/vfio_platform.c | 60
> ++++++++++++++++++++++++++++++++++++++------
> >  1 file changed, 53 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
> > index b9686b0..a0abcfa 100644
> > --- a/drivers/vfio/vfio_platform.c
> > +++ b/drivers/vfio/vfio_platform.c
> > @@ -28,6 +28,10 @@
> >  #include <linux/types.h>
> >  #include <linux/uaccess.h>
> >  #include <linux/vfio.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> >
> >  #define DRIVER_VERSION  "0.1"
> >  #define DRIVER_AUTHOR   "Antonios Motakis <
> a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>"
> > @@ -54,10 +58,13 @@ static long vfio_platform_ioctl(void *device_data,
> >                          unsigned int cmd, unsigned long arg)
> >  {
> >       struct vfio_platform_device *vdev = device_data;
> > +     struct device_node *of_node = vdev->pdev->dev.of_node;
> >       unsigned long minsz;
> >
> >       if (cmd == VFIO_DEVICE_GET_INFO) {
> >               struct vfio_device_info info;
> > +             struct resource res;
> > +             int cnt = 0;
> >
> >               minsz = offsetofend(struct vfio_device_info, num_irqs);
> >
> > @@ -68,18 +75,57 @@ static long vfio_platform_ioctl(void *device_data,
> >                       return -EINVAL;
> >
> >               info.flags = VFIO_DEVICE_FLAGS_PLATFORM;
> > -             info.num_regions = 0;
> > -             info.num_irqs = 0;
> > +
> > +             while (!of_address_to_resource(of_node, cnt, &res))
> > +                     cnt++;
> > +
> > +             info.num_regions = cnt;
> > +
> > +             info.num_irqs = of_irq_count(of_node);
> >
> >               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_REGION_INFO) {
> > +             struct vfio_region_info info;
> > +             struct resource res;
> >
> > -     else if (cmd == VFIO_DEVICE_GET_IRQ_INFO)
> > -             return -EINVAL;
> > +             minsz = offsetofend(struct vfio_region_info, offset);
> > +
> > +             if (copy_from_user(&info, (void __user *)arg, minsz))
> > +                     return -EFAULT;
> > +
> > +             if (info.argsz < minsz)
> > +                     return -EINVAL;
> > +
> > +             if(of_address_to_resource(of_node, info.index, &res))
> > +                     return -EINVAL;
> > +
> > +             info.offset = res.start;        /* map phys addr with
> offset */
> > +             info.size = resource_size(&res);
> > +             info.flags = 0;
> > +
> > +             return copy_to_user((void __user *)arg, &info, minsz);
> > +
> > +     } else if (cmd == VFIO_DEVICE_GET_IRQ_INFO) {
> > +             struct vfio_irq_info info;
> > +             struct resource res;
> > +
> > +             minsz = offsetofend(struct vfio_irq_info, count);
> > +
> > +             if (copy_from_user(&info, (void __user *)arg, minsz))
> > +                     return -EFAULT;
> > +
> > +             if (info.argsz < minsz)
> > +                     return -EINVAL;
> > +
> > +             of_irq_to_resource(of_node, info.index, &res);
> > +
> > +             info.flags = 0;
> > +             info.count = 1;
> > +
> > +             return copy_to_user((void __user *)arg, &info, minsz);
> >
> > -     else if (cmd == VFIO_DEVICE_SET_IRQS)
> > +     } else if (cmd == VFIO_DEVICE_SET_IRQS)
> >               return -EINVAL;
> >
> >       else if (cmd == VFIO_DEVICE_RESET)
>
> I notice all the open firmware calls here and I'm curious, will all
> platform devices be making use of open firmware?  I don't know if this
> is synonymous with device tree or not.  Thanks,
>
> Alex
>
>
This VFIO driver will support only devices implemented on the device tree.
While there can be platform devices outside the device tree, I don't think
it makes sense to support them from the same driver. This is why I
originally called the driver VFIO_DT, however I renamed it to VFIO_PLATFORM
after feedback from the first RFC. However personally, I still think the
VFIO_DT name is more appropriate since we don't support all platform
devices, only those that use the device tree.

[-- Attachment #1.2: Type: text/html, Size: 6374 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

RE: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Yoder Stuart-B08248]

> Antonios Motakis wrote
> > Alex Williamson <alex.williamson@redhat.com> wrote:
> > I notice all the open firmware calls here and I'm curious,
> > will all platform devices be making use of open firmware?
> > I don't know if this is synonymous with device tree or not.
> > Thanks,
>
> This VFIO driver will support only devices implemented
> on the device tree. While there can be platform devices
> outside the device tree, I don't think it makes sense
> to support them from the same driver. This is why I
> originally called the driver VFIO_DT, however I renamed
> it to VFIO_PLATFORM after feedback from the first
> RFC. However personally, I still think the VFIO_DT name
> is more appropriate since we don't support all platform
> devices, only those that use the device tree.

But there is no 'device tree' bus.  The bus type we're
dealing with is a platform bus.

vfio for platform devices should be independent of whether
the device was discovered in a device tree or not.
All you're doing is exposing mappable regions and IRQs
to user space and it does not matter where the info originated.

You should be using platform bus structs here not
reparsing device tree nodes.  The struct
platform_device already has resource info in the 
struct:

struct platform_device {
        const char      *name;
        u32             id;
        struct device   dev;
        u32             num_resources;
        struct resource *resource;
};

Stuart

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Alex Williamson]

On Tue, 2013-10-01 at 19:32 +0000, Yoder Stuart-B08248 wrote:
> > Antonios Motakis wrote
> > > Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > > I notice all the open firmware calls here and I'm curious,
> > > will all platform devices be making use of open firmware?
> > > I don't know if this is synonymous with device tree or not.
> > > Thanks,
> >
> > This VFIO driver will support only devices implemented
> > on the device tree. While there can be platform devices
> > outside the device tree, I don't think it makes sense
> > to support them from the same driver. This is why I
> > originally called the driver VFIO_DT, however I renamed
> > it to VFIO_PLATFORM after feedback from the first
> > RFC. However personally, I still think the VFIO_DT name
> > is more appropriate since we don't support all platform
> > devices, only those that use the device tree.
> 
> But there is no 'device tree' bus.  The bus type we're
> dealing with is a platform bus.
> 
> vfio for platform devices should be independent of whether
> the device was discovered in a device tree or not.
> All you're doing is exposing mappable regions and IRQs
> to user space and it does not matter where the info originated.
> 
> You should be using platform bus structs here not
> reparsing device tree nodes.  The struct
> platform_device already has resource info in the 
> struct:
> 
> struct platform_device {
>         const char      *name;
>         u32             id;
>         struct device   dev;
>         u32             num_resources;
>         struct resource *resource;
> };

It seems likely(?) that platform devices could be described via ACPI at
some point, so keeping this abstraction would be a plus.  Thanks,

Alex 

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Antonios Motakis]

On Tue, Oct 1, 2013 at 9:32 PM, Yoder Stuart-B08248
<B08248-KZfg59tc24xl57MIdRCFDg@public.gmane.org> wrote:
>> Antonios Motakis wrote
>> > Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > I notice all the open firmware calls here and I'm curious,
>> > will all platform devices be making use of open firmware?
>> > I don't know if this is synonymous with device tree or not.
>> > Thanks,
>>
>> This VFIO driver will support only devices implemented
>> on the device tree. While there can be platform devices
>> outside the device tree, I don't think it makes sense
>> to support them from the same driver. This is why I
>> originally called the driver VFIO_DT, however I renamed
>> it to VFIO_PLATFORM after feedback from the first
>> RFC. However personally, I still think the VFIO_DT name
>> is more appropriate since we don't support all platform
>> devices, only those that use the device tree.
>
> But there is no 'device tree' bus.  The bus type we're
> dealing with is a platform bus.
>
> vfio for platform devices should be independent of whether
> the device was discovered in a device tree or not.
> All you're doing is exposing mappable regions and IRQs
> to user space and it does not matter where the info originated.
>
> You should be using platform bus structs here not
> reparsing device tree nodes.  The struct
> platform_device already has resource info in the
> struct:
>
> struct platform_device {
>         const char      *name;
>         u32             id;
>         struct device   dev;
>         u32             num_resources;
>         struct resource *resource;
> };
>
> Stuart
>

I will look into this. However, can we rely to have access to all
device resources through platform abstractions, for every type of
platform device? It seems to me that platform devices that are not
backed by a specific description mechanism (such as device tree) may
include a lot of hard coded values etc in their drivers.

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Alexander Graf]

On 02.10.2013, at 13:21, Antonios Motakis wrote:

> On Tue, Oct 1, 2013 at 9:32 PM, Yoder Stuart-B08248
> <B08248@freescale.com> wrote:
>>> Antonios Motakis wrote
>>>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>>> I notice all the open firmware calls here and I'm curious,
>>>> will all platform devices be making use of open firmware?
>>>> I don't know if this is synonymous with device tree or not.
>>>> Thanks,
>>> 
>>> This VFIO driver will support only devices implemented
>>> on the device tree. While there can be platform devices
>>> outside the device tree, I don't think it makes sense
>>> to support them from the same driver. This is why I
>>> originally called the driver VFIO_DT, however I renamed
>>> it to VFIO_PLATFORM after feedback from the first
>>> RFC. However personally, I still think the VFIO_DT name
>>> is more appropriate since we don't support all platform
>>> devices, only those that use the device tree.
>> 
>> But there is no 'device tree' bus.  The bus type we're
>> dealing with is a platform bus.
>> 
>> vfio for platform devices should be independent of whether
>> the device was discovered in a device tree or not.
>> All you're doing is exposing mappable regions and IRQs
>> to user space and it does not matter where the info originated.
>> 
>> You should be using platform bus structs here not
>> reparsing device tree nodes.  The struct
>> platform_device already has resource info in the
>> struct:
>> 
>> struct platform_device {
>>        const char      *name;
>>        u32             id;
>>        struct device   dev;
>>        u32             num_resources;
>>        struct resource *resource;
>> };
>> 
>> Stuart
>> 
> 
> I will look into this. However, can we rely to have access to all
> device resources through platform abstractions, for every type of
> platform device? It seems to me that platform devices that are not
> backed by a specific description mechanism (such as device tree) may
> include a lot of hard coded values etc in their drivers.

The same can be true for device tree based drivers. We should tell user space which device tree node this is based off of so it can go and find properties itself, similar to how a kernel driver can still read dt properties for a platform device. But this is optional - we could just as well give user space a hint on an acpi interface.

What exactly would you consider not abstracted information that the kernel driver would need to know about? All cases I can think of (reset logic, power constraints) are very device specific and would require a new vfio-dt-mydevice driver on top of your generic framework to handle these small device specific bits.


Alex

RE: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Yoder Stuart-B08248]

> -----Original Message-----
> From: Antonios Motakis [mailto:a.motakis@virtualopensystems.com]
> Sent: Wednesday, October 02, 2013 6:22 AM
> To: Yoder Stuart-B08248
> Cc: Alex Williamson; kvm-arm; Linux IOMMU; Linux Samsung SOC; KVM devel
> mailing list; Alexander Graf; VirtualOpenSystems Technical Team
> Subject: Re: [PATCH 3/7] Return info for device and its memory regions
> and interrupts
> 
> On Tue, Oct 1, 2013 at 9:32 PM, Yoder Stuart-B08248
> <B08248@freescale.com> wrote:
> >> Antonios Motakis wrote
> >> > Alex Williamson <alex.williamson@redhat.com> wrote:
> >> > I notice all the open firmware calls here and I'm curious,
> >> > will all platform devices be making use of open firmware?
> >> > I don't know if this is synonymous with device tree or not.
> >> > Thanks,
> >>
> >> This VFIO driver will support only devices implemented
> >> on the device tree. While there can be platform devices
> >> outside the device tree, I don't think it makes sense
> >> to support them from the same driver. This is why I
> >> originally called the driver VFIO_DT, however I renamed
> >> it to VFIO_PLATFORM after feedback from the first
> >> RFC. However personally, I still think the VFIO_DT name
> >> is more appropriate since we don't support all platform
> >> devices, only those that use the device tree.
> >
> > But there is no 'device tree' bus.  The bus type we're
> > dealing with is a platform bus.
> >
> > vfio for platform devices should be independent of whether
> > the device was discovered in a device tree or not.
> > All you're doing is exposing mappable regions and IRQs
> > to user space and it does not matter where the info originated.
> >
> > You should be using platform bus structs here not
> > reparsing device tree nodes.  The struct
> > platform_device already has resource info in the
> > struct:
> >
> > struct platform_device {
> >         const char      *name;
> >         u32             id;
> >         struct device   dev;
> >         u32             num_resources;
> >         struct resource *resource;
> > };
> >
> > Stuart
> >
> 
> I will look into this. However, can we rely to have access to all
> device resources through platform abstractions, for every type of
> platform device

The only resources we care about in vfio are mappable regions
and irqs.  So, yes I think we can rely on access to those
resources.

> It seems to me that platform devices that are not
> backed by a specific description mechanism (such as device tree) may
> include a lot of hard coded values etc in their drivers.

If the platform device struct does not have reg/irq resources described
then we can't expose them to user space with vfio.

Stuart

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Antonios Motakis]

On Wed, Oct 2, 2013 at 3:03 PM, Yoder Stuart-B08248
<B08248@freescale.com> wrote:
>
>
>> -----Original Message-----
>> From: Antonios Motakis [mailto:a.motakis@virtualopensystems.com]
>> Sent: Wednesday, October 02, 2013 6:22 AM
>> To: Yoder Stuart-B08248
>> Cc: Alex Williamson; kvm-arm; Linux IOMMU; Linux Samsung SOC; KVM devel
>> mailing list; Alexander Graf; VirtualOpenSystems Technical Team
>> Subject: Re: [PATCH 3/7] Return info for device and its memory regions
>> and interrupts
>>
>> On Tue, Oct 1, 2013 at 9:32 PM, Yoder Stuart-B08248
>> <B08248@freescale.com> wrote:
>> >> Antonios Motakis wrote
>> >> > Alex Williamson <alex.williamson@redhat.com> wrote:
>> >> > I notice all the open firmware calls here and I'm curious,
>> >> > will all platform devices be making use of open firmware?
>> >> > I don't know if this is synonymous with device tree or not.
>> >> > Thanks,
>> >>
>> >> This VFIO driver will support only devices implemented
>> >> on the device tree. While there can be platform devices
>> >> outside the device tree, I don't think it makes sense
>> >> to support them from the same driver. This is why I
>> >> originally called the driver VFIO_DT, however I renamed
>> >> it to VFIO_PLATFORM after feedback from the first
>> >> RFC. However personally, I still think the VFIO_DT name
>> >> is more appropriate since we don't support all platform
>> >> devices, only those that use the device tree.
>> >
>> > But there is no 'device tree' bus.  The bus type we're
>> > dealing with is a platform bus.
>> >
>> > vfio for platform devices should be independent of whether
>> > the device was discovered in a device tree or not.
>> > All you're doing is exposing mappable regions and IRQs
>> > to user space and it does not matter where the info originated.
>> >
>> > You should be using platform bus structs here not
>> > reparsing device tree nodes.  The struct
>> > platform_device already has resource info in the
>> > struct:
>> >
>> > struct platform_device {
>> >         const char      *name;
>> >         u32             id;
>> >         struct device   dev;
>> >         u32             num_resources;
>> >         struct resource *resource;
>> > };
>> >
>> > Stuart
>> >
>>
>> I will look into this. However, can we rely to have access to all
>> device resources through platform abstractions, for every type of
>> platform device
>
> The only resources we care about in vfio are mappable regions
> and irqs.  So, yes I think we can rely on access to those
> resources.
>
>> It seems to me that platform devices that are not
>> backed by a specific description mechanism (such as device tree) may
>> include a lot of hard coded values etc in their drivers.
>
> If the platform device struct does not have reg/irq resources described
> then we can't expose them to user space with vfio.

Which is my concern actually. If the struct does not describe reg/irq
resources, or even worse if it describes them partially, do we want to
let the user shoot himself on the foot?

Granted this might happen with device tree too, but I find it much
more likely with generic platform devices.

Maybe we should adopt a whitelist instead?

>
> Stuart
>

Re: [PATCH 3/7] Return info for device and its memory regions and interrupts

[Stuart Yoder]

>>> I will look into this. However, can we rely to have access to all
>>> device resources through platform abstractions, for every type of
>>> platform device
>>
>> The only resources we care about in vfio are mappable regions
>> and irqs.  So, yes I think we can rely on access to those
>> resources.
>>
>>> It seems to me that platform devices that are not
>>> backed by a specific description mechanism (such as device tree) may
>>> include a lot of hard coded values etc in their drivers.
>>
>> If the platform device struct does not have reg/irq resources described
>> then we can't expose them to user space with vfio.
>
> Which is my concern actually. If the struct does not describe reg/irq
> resources, or even worse if it describes them partially, do we want to
> let the user shoot himself on the foot?
>
> Granted this might happen with device tree too, but I find it much
> more likely with generic platform devices.
>
> Maybe we should adopt a whitelist instead?

Just realize I never replied to this query...

If there are no IRQs in the platform device struct we should
just return 0x0 for number of interrupts.
If the information in the platform device struct is only partially
complete, we should just return an error to the caller of the
ioctl.

Stuart

[PATCH 4/7] VFIO: DT: Support MMAP of MMIO regions

[Antonios Motakis]

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/vfio_platform.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
index a0abcfa..6364316 100644
--- a/drivers/vfio/vfio_platform.c
+++ b/drivers/vfio/vfio_platform.c
@@ -103,6 +103,10 @@ static long vfio_platform_ioctl(void *device_data,
 		info.offset = res.start;	/* map phys addr with offset */
 		info.size = resource_size(&res);
 		info.flags = 0;
+		/* Only regions addressed with PAGE granularity can be MMAPed
+		 * securely. */
+		if (!(info.offset & ~PAGE_MASK) && !(info.size & ~PAGE_MASK))
+			info.flags |= VFIO_REGION_INFO_FLAG_MMAP;
 
 		return copy_to_user((void __user *)arg, &info, minsz);
 
@@ -134,6 +138,18 @@ static long vfio_platform_ioctl(void *device_data,
 	return -ENOTTY;
 }
 
+static bool is_in_resource(struct resource res, u64 addr, u64 size)
+{
+	if (addr < res.start)
+		return false;
+	if (addr > res.end)
+		return false;
+	if (addr + size - 1 > res.end)
+		return false;
+
+	return true;
+}
+
 static ssize_t vfio_platform_read(void *device_data, char __user *buf,
 			     size_t count, loff_t *ppos)
 {
@@ -148,6 +164,34 @@ static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
 
 static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
 {
+	struct vfio_platform_device *vdev = device_data;
+	struct device_node *of_node = vdev->pdev->dev.of_node;
+	u64 req_len = vma->vm_end - vma->vm_start;
+	u64 addr = vma->vm_pgoff << PAGE_SHIFT;
+	struct resource res;
+	int i;
+
+	if (vma->vm_end < vma->vm_start)
+		return -EINVAL;
+	if (vma->vm_start & ~PAGE_MASK)
+		return -EINVAL;
+	if (vma->vm_end & ~PAGE_MASK)
+		return -EINVAL;
+	if ((vma->vm_flags & VM_SHARED) == 0)
+		return -EINVAL;
+
+	for (i = 0; !of_address_to_resource(of_node, i, &res); i++) {
+		if (!is_in_resource(res, addr, req_len))
+			continue;
+
+		vma->vm_private_data = vdev;
+		vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
+		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
+		return remap_pfn_range(vma, vma->vm_start, addr,
+				       req_len, vma->vm_page_prot);
+	}
+
 	return -EINVAL;
 }
 
-- 
1.8.1.2

[PATCH 5/7] VFIO: DT: Read and write support for the device fd

[Antonios Motakis]

VFIO returns a file descriptor which we can use to manipulate the memory
regions of the device. Since some memory regions we cannot mmap due to
security concerns, we also allow to read and write to this file descriptor
directly.

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 drivers/vfio/vfio_platform.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/vfio/vfio_platform.c b/drivers/vfio/vfio_platform.c
index 6364316..b92d7bb 100644
--- a/drivers/vfio/vfio_platform.c
+++ b/drivers/vfio/vfio_platform.c
@@ -102,7 +102,8 @@ static long vfio_platform_ioctl(void *device_data,
 
 		info.offset = res.start;	/* map phys addr with offset */
 		info.size = resource_size(&res);
-		info.flags = 0;
+		info.flags = VFIO_REGION_INFO_FLAG_READ
+				| VFIO_REGION_INFO_FLAG_WRITE;
 		/* Only regions addressed with PAGE granularity can be MMAPed
 		 * securely. */
 		if (!(info.offset & ~PAGE_MASK) && !(info.size & ~PAGE_MASK))
@@ -153,13 +154,43 @@ static bool is_in_resource(struct resource res, u64 addr, u64 size)
 static ssize_t vfio_platform_read(void *device_data, char __user *buf,
 			     size_t count, loff_t *ppos)
 {
-	return 0;
+	struct vfio_platform_device *vdev = device_data;
+	struct device_node *of_node = vdev->pdev->dev.of_node;
+	struct resource res;
+	int i;
+
+	for (i = 0; !of_address_to_resource(of_node, i, &res); i++) {
+		if (!is_in_resource(res, *ppos, count))
+			continue;
+
+		if (copy_to_user(buf, ppos, count))
+			return -EFAULT;
+		else
+			return count;
+	}
+
+	return -EINVAL;
 }
 
 static ssize_t vfio_platform_write(void *device_data, const char __user *buf,
 			      size_t count, loff_t *ppos)
 {
-	return 0;
+	struct vfio_platform_device *vdev = device_data;
+	struct device_node *of_node = vdev->pdev->dev.of_node;
+	struct resource res;
+	int i;
+
+	for (i = 0; !of_address_to_resource(of_node, i, &res); i++) {
+		if (!is_in_resource(res, *ppos, count))
+			continue;
+
+		if (copy_from_user(ppos, buf, count))
+			return -EFAULT;
+		else
+			return count;
+	}
+
+	return -EINVAL;
 }
 
 static int vfio_platform_mmap(void *device_data, struct vm_area_struct *vma)
-- 
1.8.1.2

[PATCH 6/7] VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver

[Antonios Motakis]

The VFIO documentation is slightly out of date. This minor correction
replaces references to VFIO_IOMMU_X86 with the correct reference to
VFIO_IOMMU_TYPE1

Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
---
 Documentation/vfio.txt | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index 8eda363..b4eafa6 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -167,8 +167,8 @@ group and can access them as follows:
 	int container, group, device, i;
 	struct vfio_group_status group_status =
 					{ .argsz = sizeof(group_status) };
-	struct vfio_iommu_x86_info iommu_info = { .argsz = sizeof(iommu_info) };
-	struct vfio_iommu_x86_dma_map dma_map = { .argsz = sizeof(dma_map) };
+	struct vfio_iommu_type1_info iommu_info = { .argsz = sizeof(iommu_info) };
+	struct vfio_iommu_type1_dma_map dma_map = { .argsz = sizeof(dma_map) };
 	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
 
 	/* Create a new container */
@@ -177,7 +177,7 @@ group and can access them as follows:
 	if (ioctl(container, VFIO_GET_API_VERSION) != VFIO_API_VERSION)
 		/* Unknown API version */
 
-	if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_X86_IOMMU))
+	if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU))
 		/* Doesn't support the IOMMU driver we want. */
 
 	/* Open the group */
@@ -193,7 +193,7 @@ group and can access them as follows:
 	ioctl(group, VFIO_GROUP_SET_CONTAINER, &container);
 
 	/* Enable the IOMMU model we want */
-	ioctl(container, VFIO_SET_IOMMU, VFIO_X86_IOMMU)
+	ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU)
 
 	/* Get addition IOMMU info */
 	ioctl(container, VFIO_IOMMU_GET_INFO, &iommu_info);
-- 
1.8.1.2

Re: [PATCH 6/7] VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver

[Alex Williamson]

On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
> The VFIO documentation is slightly out of date. This minor correction
> replaces references to VFIO_IOMMU_X86 with the correct reference to
> VFIO_IOMMU_TYPE1
> 
> Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> ---
>  Documentation/vfio.txt | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> index 8eda363..b4eafa6 100644
> --- a/Documentation/vfio.txt
> +++ b/Documentation/vfio.txt
> @@ -167,8 +167,8 @@ group and can access them as follows:
>  	int container, group, device, i;
>  	struct vfio_group_status group_status =
>  					{ .argsz = sizeof(group_status) };
> -	struct vfio_iommu_x86_info iommu_info = { .argsz = sizeof(iommu_info) };
> -	struct vfio_iommu_x86_dma_map dma_map = { .argsz = sizeof(dma_map) };
> +	struct vfio_iommu_type1_info iommu_info = { .argsz = sizeof(iommu_info) };
> +	struct vfio_iommu_type1_dma_map dma_map = { .argsz = sizeof(dma_map) };
>  	struct vfio_device_info device_info = { .argsz = sizeof(device_info) };
>  
>  	/* Create a new container */
> @@ -177,7 +177,7 @@ group and can access them as follows:
>  	if (ioctl(container, VFIO_GET_API_VERSION) != VFIO_API_VERSION)
>  		/* Unknown API version */
>  
> -	if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_X86_IOMMU))
> +	if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU))
>  		/* Doesn't support the IOMMU driver we want. */
>  
>  	/* Open the group */
> @@ -193,7 +193,7 @@ group and can access them as follows:
>  	ioctl(group, VFIO_GROUP_SET_CONTAINER, &container);
>  
>  	/* Enable the IOMMU model we want */
> -	ioctl(container, VFIO_SET_IOMMU, VFIO_X86_IOMMU)
> +	ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU)
>  
>  	/* Get addition IOMMU info */
>  	ioctl(container, VFIO_IOMMU_GET_INFO, &iommu_info);


Your tree seems out of date, we've already got these:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/vfio.txt?id=dac09b57c0ff57e55dd6fa5109076c8af0468a75
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/vfio.txt?id=b0e59b85abd42f8a525ff032d5063457e1378808

Re: [PATCH 6/7] VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver

[Antonios Motakis]

[-- Attachment #1.1: Type: text/plain, Size: 2659 bytes --]

On Mon, Sep 30, 2013 at 7:39 PM, Alex Williamson <alex.williamson-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> wrote:

> On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
> > The VFIO documentation is slightly out of date. This minor correction
> > replaces references to VFIO_IOMMU_X86 with the correct reference to
> > VFIO_IOMMU_TYPE1
> >
> > Signed-off-by: Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>
> > ---
> >  Documentation/vfio.txt | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
> > index 8eda363..b4eafa6 100644
> > --- a/Documentation/vfio.txt
> > +++ b/Documentation/vfio.txt
> > @@ -167,8 +167,8 @@ group and can access them as follows:
> >       int container, group, device, i;
> >       struct vfio_group_status group_status =
> >                                       { .argsz = sizeof(group_status) };
> > -     struct vfio_iommu_x86_info iommu_info = { .argsz =
> sizeof(iommu_info) };
> > -     struct vfio_iommu_x86_dma_map dma_map = { .argsz = sizeof(dma_map)
> };
> > +     struct vfio_iommu_type1_info iommu_info = { .argsz =
> sizeof(iommu_info) };
> > +     struct vfio_iommu_type1_dma_map dma_map = { .argsz =
> sizeof(dma_map) };
> >       struct vfio_device_info device_info = { .argsz =
> sizeof(device_info) };
> >
> >       /* Create a new container */
> > @@ -177,7 +177,7 @@ group and can access them as follows:
> >       if (ioctl(container, VFIO_GET_API_VERSION) != VFIO_API_VERSION)
> >               /* Unknown API version */
> >
> > -     if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_X86_IOMMU))
> > +     if (!ioctl(container, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU))
> >               /* Doesn't support the IOMMU driver we want. */
> >
> >       /* Open the group */
> > @@ -193,7 +193,7 @@ group and can access them as follows:
> >       ioctl(group, VFIO_GROUP_SET_CONTAINER, &container);
> >
> >       /* Enable the IOMMU model we want */
> > -     ioctl(container, VFIO_SET_IOMMU, VFIO_X86_IOMMU)
> > +     ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU)
> >
> >       /* Get addition IOMMU info */
> >       ioctl(container, VFIO_IOMMU_GET_INFO, &iommu_info);
>
>
> Your tree seems out of date, we've already got these:
>
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/vfio.txt?id=dac09b57c0ff57e55dd6fa5109076c8af0468a75
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/Documentation/vfio.txt?id=b0e59b85abd42f8a525ff032d5063457e1378808
>
>
And I was wondering what was up with that. Acknowledged, thanks!

[-- Attachment #1.2: Type: text/html, Size: 3762 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

[PATCH 7/7] VFIO: VFIO_PLATFORM: Update documentation for platform specific devices

[Antonios Motakis]

Update Documentation/vfio.txt with information regarding the device tree
based platform devices support.

What is still missing in this RFC series, is information on how to bind
the driver to platform devices, as there is currently for PCI. This will
be added when proper VFIO driver binding is implemented.
---
 Documentation/vfio.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
index b4eafa6..e8daa2b 100644
--- a/Documentation/vfio.txt
+++ b/Documentation/vfio.txt
@@ -237,6 +237,23 @@ group and can access them as follows:
 	/* Gratuitous device reset and go... */
 	ioctl(device, VFIO_DEVICE_RESET);
 
+For platform devices, if a device has multiple regions and multiple IRQs the
+index that will be assigned by VFIO to those resources will correspond to the
+order within the associated "reg" and "interrupt" properties in the device tree
+representation of the target device.
+
+For example, if a device is represented like this in the device tree:
+
+        reg = <0x101e2000 0x1000 0x101e4000 0x1000>;
+        interrupts = <24 25 26 27>;
+
+Region #0 is 0x101e2000, region #1 is 0x101e4000
+Interrupt #0 is 24, and so on.
+
+Additionally for a platform device, unlike PCI devices, an offset referring to
+a region within a VFIO device file descriptor will match the physical address
+of that region as defined in the device tree.
+
 VFIO User API
 -------------------------------------------------------------------------------
 
-- 
1.8.1.2

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Alex Williamson]

On Mon, 2013-09-30 at 17:28 +0200, Antonios Motakis wrote:
> This is a preview of the base work, towards VFIO support on ARM platforms
> with an IOMMU. It forms a base on to which to implement the functionality
> necessary to enable using device tree devices on ARM (and other platforms
> based on device trees) with VFIO.
> 
> This patch series has been subjected to limited testing on the Arndale board
> (with the Exynos 5250 System MMU). More extensive testing will follow as more
> features are implemented.
> 
> It depends on Cho KyongHo's patch series "iommu/exynos: Fixes and Enhancements
> of System MMU driver with DT", applied on a Linux 3.10.1 kernel, and also my
> own "iommu/exynos: add devices attached to the System MMU to an IOMMU group".
> Those patches are required at least in order to test the proposed module on
> Arndale.
> 
> The API used is identical to the existing VFIO API that is also used with
> PCI devices. Only devices that include a basic set of IRQs and memory regions
> are targeted; devices with complicated relationships with other devices on the
> device tree are not taken into account at this stage.
> 
> The following IOCTLs have been found to be working on the Arndale with no
> changes to VFIO:
>  - VFIO_GET_API_VERSION
>  - VFIO_CHECK_EXTENSION
> The TYPE1 fix proposed here enables the following IOCTLs:
>  - VFIO_GROUP_GET_STATUS
>  - VFIO_GROUP_SET_CONTAINER
>  - VFIO_SET_IOMMU
>  - VFIO_IOMMU_GET_INFO
>  - VFIO_IOMMU_MAP_DMA
> The VFIO platform driver proposed here implements the following:
>  - VFIO_GROUP_GET_DEVICE_FD
>  - VFIO_DEVICE_GET_INFO
>  - VFIO_DEVICE_GET_REGION_INFO
>  - VFIO_DEVICE_GET_IRQ_INFO
> In addition, the VFIO platform driver implements the following through
> the VFIO device file descriptor:
>  - MMAPing memory regions to the virtual address space of the VFIO user.
>  - Read / write of memory regions directly through the file descriptor.
> 
> What still needs to be done, includes:
>  - IRQs / IRQFD support.
>  - The API is not currently extended with device tree specific information.
>  - Proper binding of the VFIO_DT driver to devices; currently to test the
>    driver, one has to edit the device tree and add "vfio" to the
>    compatible property. However Linux does not support OF drivers that
>    can be dynamically bound to any device.
>  - More extensive testing with a functioning device doing DMA.
>  - QEMU / KVM support.
> 
> Antonios Motakis (7):
>   VFIO_IOMMU_TYPE1 workaround to build for platform devices
>   Initial skeleton of VFIO support for Device Tree based devices
>   Return info for device and its memory regions and interrupts
>   VFIO: DT: Support MMAP of MMIO regions
>   VFIO: DT: Read and write support for the device fd
>   VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver
>   VFIO: VFIO_PLATFORM: Update documentation for platform specific
>     devices
> 
>  Documentation/vfio.txt          |  25 +++-
>  drivers/vfio/Kconfig            |  13 +-
>  drivers/vfio/Makefile           |   1 +
>  drivers/vfio/vfio_iommu_type1.c |  22 ++-
>  drivers/vfio/vfio_platform.c    | 308 ++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vfio.h       |   1 +
>  6 files changed, 361 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/vfio/vfio_platform.c

Looks like a good first pass to me, I'll be interested to see what the
other platform device folks think.  Thanks,

Alex

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Christoffer Dall]

On Mon, Sep 30, 2013 at 05:28:36PM +0200, Antonios Motakis wrote:
> This is a preview of the base work, towards VFIO support on ARM platforms
> with an IOMMU. It forms a base on to which to implement the functionality
> necessary to enable using device tree devices on ARM (and other platforms
> based on device trees) with VFIO.
> 
> This patch series has been subjected to limited testing on the Arndale board
> (with the Exynos 5250 System MMU). More extensive testing will follow as more
> features are implemented.

Which device did you access behind the system MMU?  I seem to recall
someone telling me that only the GPU is behind the system MMU on
Arndale.

> 
> It depends on Cho KyongHo's patch series "iommu/exynos: Fixes and Enhancements
> of System MMU driver with DT", applied on a Linux 3.10.1 kernel, and also my
> own "iommu/exynos: add devices attached to the System MMU to an IOMMU group".
> Those patches are required at least in order to test the proposed module on
> Arndale.
> 
> The API used is identical to the existing VFIO API that is also used with
> PCI devices. Only devices that include a basic set of IRQs and memory regions
> are targeted; devices with complicated relationships with other devices on the
> device tree are not taken into account at this stage.
> 
> The following IOCTLs have been found to be working on the Arndale with no
> changes to VFIO:
>  - VFIO_GET_API_VERSION
>  - VFIO_CHECK_EXTENSION
> The TYPE1 fix proposed here enables the following IOCTLs:
>  - VFIO_GROUP_GET_STATUS
>  - VFIO_GROUP_SET_CONTAINER
>  - VFIO_SET_IOMMU
>  - VFIO_IOMMU_GET_INFO
>  - VFIO_IOMMU_MAP_DMA
> The VFIO platform driver proposed here implements the following:
>  - VFIO_GROUP_GET_DEVICE_FD
>  - VFIO_DEVICE_GET_INFO
>  - VFIO_DEVICE_GET_REGION_INFO
>  - VFIO_DEVICE_GET_IRQ_INFO
> In addition, the VFIO platform driver implements the following through
> the VFIO device file descriptor:
>  - MMAPing memory regions to the virtual address space of the VFIO user.
>  - Read / write of memory regions directly through the file descriptor.
> 
> What still needs to be done, includes:
>  - IRQs / IRQFD support.
>  - The API is not currently extended with device tree specific information.
>  - Proper binding of the VFIO_DT driver to devices; currently to test the
>    driver, one has to edit the device tree and add "vfio" to the
>    compatible property. However Linux does not support OF drivers that
>    can be dynamically bound to any device.
>  - More extensive testing with a functioning device doing DMA.
>  - QEMU / KVM support.
> 
> Antonios Motakis (7):
>   VFIO_IOMMU_TYPE1 workaround to build for platform devices
>   Initial skeleton of VFIO support for Device Tree based devices
>   Return info for device and its memory regions and interrupts
>   VFIO: DT: Support MMAP of MMIO regions
>   VFIO: DT: Read and write support for the device fd
>   VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver
>   VFIO: VFIO_PLATFORM: Update documentation for platform specific
>     devices
> 
>  Documentation/vfio.txt          |  25 +++-
>  drivers/vfio/Kconfig            |  13 +-
>  drivers/vfio/Makefile           |   1 +
>  drivers/vfio/vfio_iommu_type1.c |  22 ++-
>  drivers/vfio/vfio_platform.c    | 308 ++++++++++++++++++++++++++++++++++++++++
>  include/uapi/linux/vfio.h       |   1 +
>  6 files changed, 361 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/vfio/vfio_platform.c
> 
> -- 
> 1.8.1.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Christoffer

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Antonios Motakis]

On Tue, Oct 1, 2013 at 5:47 PM, Christoffer Dall
<christoffer.dall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Sep 30, 2013 at 05:28:36PM +0200, Antonios Motakis wrote:
>> This is a preview of the base work, towards VFIO support on ARM platforms
>> with an IOMMU. It forms a base on to which to implement the functionality
>> necessary to enable using device tree devices on ARM (and other platforms
>> based on device trees) with VFIO.
>>
>> This patch series has been subjected to limited testing on the Arndale board
>> (with the Exynos 5250 System MMU). More extensive testing will follow as more
>> features are implemented.
>
> Which device did you access behind the system MMU?  I seem to recall
> someone telling me that only the GPU is behind the system MMU on
> Arndale.

Hello Christoffer,

Actually, on the Arndale you will find a few devices behind a SMMU,
including the MFC (codec) device, etc. I've tested MMAP and DMA
mapping functions for example, but the implementation is not yet
complete enough to fully use the device.

You can take a look into arch/arm/boot/dts/exynos5250.dtsi for devices
with an iommu entry. Unfortunately, the Arndale is not a very well
documented board with regards to its peripherals...

Ideally, we would have access to a device that is simple and
documented enough, in order to write a trivial user space VFIO driver
to verify functionality. In practice, our options for validation are
limited, until we can finally fully assign a device at least.

Antonios

>
>>
>> It depends on Cho KyongHo's patch series "iommu/exynos: Fixes and Enhancements
>> of System MMU driver with DT", applied on a Linux 3.10.1 kernel, and also my
>> own "iommu/exynos: add devices attached to the System MMU to an IOMMU group".
>> Those patches are required at least in order to test the proposed module on
>> Arndale.
>>
>> The API used is identical to the existing VFIO API that is also used with
>> PCI devices. Only devices that include a basic set of IRQs and memory regions
>> are targeted; devices with complicated relationships with other devices on the
>> device tree are not taken into account at this stage.
>>
>> The following IOCTLs have been found to be working on the Arndale with no
>> changes to VFIO:
>>  - VFIO_GET_API_VERSION
>>  - VFIO_CHECK_EXTENSION
>> The TYPE1 fix proposed here enables the following IOCTLs:
>>  - VFIO_GROUP_GET_STATUS
>>  - VFIO_GROUP_SET_CONTAINER
>>  - VFIO_SET_IOMMU
>>  - VFIO_IOMMU_GET_INFO
>>  - VFIO_IOMMU_MAP_DMA
>> The VFIO platform driver proposed here implements the following:
>>  - VFIO_GROUP_GET_DEVICE_FD
>>  - VFIO_DEVICE_GET_INFO
>>  - VFIO_DEVICE_GET_REGION_INFO
>>  - VFIO_DEVICE_GET_IRQ_INFO
>> In addition, the VFIO platform driver implements the following through
>> the VFIO device file descriptor:
>>  - MMAPing memory regions to the virtual address space of the VFIO user.
>>  - Read / write of memory regions directly through the file descriptor.
>>
>> What still needs to be done, includes:
>>  - IRQs / IRQFD support.
>>  - The API is not currently extended with device tree specific information.
>>  - Proper binding of the VFIO_DT driver to devices; currently to test the
>>    driver, one has to edit the device tree and add "vfio" to the
>>    compatible property. However Linux does not support OF drivers that
>>    can be dynamically bound to any device.
>>  - More extensive testing with a functioning device doing DMA.
>>  - QEMU / KVM support.
>>
>> Antonios Motakis (7):
>>   VFIO_IOMMU_TYPE1 workaround to build for platform devices
>>   Initial skeleton of VFIO support for Device Tree based devices
>>   Return info for device and its memory regions and interrupts
>>   VFIO: DT: Support MMAP of MMIO regions
>>   VFIO: DT: Read and write support for the device fd
>>   VFIO: Update documentation for VFIO_IOMMU_TYPE1 driver
>>   VFIO: VFIO_PLATFORM: Update documentation for platform specific
>>     devices
>>
>>  Documentation/vfio.txt          |  25 +++-
>>  drivers/vfio/Kconfig            |  13 +-
>>  drivers/vfio/Makefile           |   1 +
>>  drivers/vfio/vfio_iommu_type1.c |  22 ++-
>>  drivers/vfio/vfio_platform.c    | 308 ++++++++++++++++++++++++++++++++++++++++
>>  include/uapi/linux/vfio.h       |   1 +
>>  6 files changed, 361 insertions(+), 9 deletions(-)
>>  create mode 100644 drivers/vfio/vfio_platform.c
>>
>> --
>> 1.8.1.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
> --
> Christoffer

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Kim Phillips]

On Mon, 30 Sep 2013 17:28:36 +0200
Antonios Motakis <a.motakis@virtualopensystems.com> wrote:

> This is a preview of the base work, towards VFIO support on ARM platforms
> with an IOMMU. It forms a base on to which to implement the functionality
> necessary to enable using device tree devices on ARM (and other platforms
> based on device trees) with VFIO.
> 
> This patch series has been subjected to limited testing on the Arndale board
> (with the Exynos 5250 System MMU). More extensive testing will follow as more
> features are implemented.

Hi, I'm trying to attach the vfio-platform driver to an(y) iommu-backed
device on the arndale.  I chose the relatively simple 'tv' device,
isolated it in the arndale device tree (to get rid of any dependency
conflicts), and, when I run Alex' vfio-correctness-tests [1], I get
this:

# echo 14650000.sysmmu > /sys/bus/platform/devices/14650000.sysmmu/driver/unbind
# echo 14650000.sysmmu > /sys/bus/platform/drivers/vfio-platform/bind
# ls -l /sys/bus/platform/devices/14650000.sysmmu/iommu_group
lrwxrwxrwx 1 root root 0 Oct  1 23:14 /sys/bus/platform/devices/14650000.sysmmu/iommu_group -> ../../kernel/iommu_groups/43
# vfio-tests/vfio-correctness-tests 43
[   84.567643] ------------[ cut here ]------------
[   84.572258] WARNING: CPU: 0 PID: 1606 at drivers/iommu/exynos-iommu.c:472 __exynos_sysmmu_enable+0x170/0x184()
[   84.582225] Modules linked in:
[   84.585268] CPU: 0 PID: 1606 Comm: vfio-correctnes Not tainted 3.13.0-rc1-00296-gb703d74-dirty #46
[   84.594219] [<c0014d20>] (unwind_backtrace+0x0/0xf4) from [<c0011648>] (show_stack+0x10/0x14)
[   84.602719] [<c0011648>] (show_stack+0x10/0x14) from [<c0377948>] (dump_stack+0x7c/0xb0)
[   84.610791] [<c0377948>] (dump_stack+0x7c/0xb0) from [<c001dc50>] (warn_slowpath_common+0x6c/0x88)
[   84.619730] [<c001dc50>] (warn_slowpath_common+0x6c/0x88) from [<c001dd08>] (warn_slowpath_null+0x1c/0x24)
[   84.629365] [<c001dd08>] (warn_slowpath_null+0x1c/0x24) from [<c02dace8>] (__exynos_sysmmu_enable+0x170/0x184)
[   84.639348] [<c02dace8>] (__exynos_sysmmu_enable+0x170/0x184) from [<c02dad4c>] (exynos_iommu_attach_device+0x50/0x108)
[   84.650111] [<c02dad4c>] (exynos_iommu_attach_device+0x50/0x108) from [<c02d939c>] (iommu_attach_device+0x64/0xb8)
[   84.660440] [<c02d939c>] (iommu_attach_device+0x64/0xb8) from [<c02d92ec>] (iommu_group_for_each_dev+0x78/0x94)
[   84.670509] [<c02d92ec>] (iommu_group_for_each_dev+0x78/0x94) from [<c0263678>] (vfio_iommu_type1_attach_group+0xd0/0x144)
[   84.681530] [<c0263678>] (vfio_iommu_type1_attach_group+0xd0/0x144) from [<c02629e0>] (vfio_fops_unl_ioctl+0x3ac/0x460)
[   84.692295] [<c02629e0>] (vfio_fops_unl_ioctl+0x3ac/0x460) from [<c00c1f14>] (do_vfs_ioctl+0x3f0/0x5b8)
[   84.701668] [<c00c1f14>] (do_vfs_ioctl+0x3f0/0x5b8) from [<c00c2114>] (SyS_ioctl+0x38/0x60)
[   84.710002] [<c00c2114>] (SyS_ioctl+0x38/0x60) from [<c000e1a0>] (ret_fast_syscall+0x0/0x30)
[   84.718417] ---[ end trace a1400d7981f699ce ]---
[   84.723071] vfio-platform 14650000.sysmmu: exynos_iommu_attach_device: Failed to attach IOMMU with pgtable 0x6eae8000

It's failing this test:

        if (WARN_ON(!client))
                return -ENODEV;

but I haven't been able to figure out why...I have the exynos
iommu v10 patchseries applied, and even though there's not much
documentation available for this device, I figure this simple test is
supposed to 'just work.'  Any ideas?

Thanks,

Kim

[1] git://github.com/awilliam/tests.git

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Antonios Motakis]

[-- Attachment #1.1: Type: text/plain, Size: 4246 bytes --]

On Sun, Dec 1, 2013 at 1:09 PM, Kim Phillips <kim.phillips-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>wrote:

> On Mon, 30 Sep 2013 17:28:36 +0200
> Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> wrote:
>
> > This is a preview of the base work, towards VFIO support on ARM platforms
> > with an IOMMU. It forms a base on to which to implement the functionality
> > necessary to enable using device tree devices on ARM (and other platforms
> > based on device trees) with VFIO.
> >
> > This patch series has been subjected to limited testing on the Arndale
> board
> > (with the Exynos 5250 System MMU). More extensive testing will follow as
> more
> > features are implemented.
>
> Hi, I'm trying to attach the vfio-platform driver to an(y) iommu-backed
> device on the arndale.  I chose the relatively simple 'tv' device,
> isolated it in the arndale device tree (to get rid of any dependency
> conflicts), and, when I run Alex' vfio-correctness-tests [1], I get
> this:
>
> # echo 14650000.sysmmu >
> /sys/bus/platform/devices/14650000.sysmmu/driver/unbind
> # echo 14650000.sysmmu > /sys/bus/platform/drivers/vfio-platform/bind
> # ls -l /sys/bus/platform/devices/14650000.sysmmu/iommu_group
> lrwxrwxrwx 1 root root 0 Oct  1 23:14
> /sys/bus/platform/devices/14650000.sysmmu/iommu_group ->
> ../../kernel/iommu_groups/43
> # vfio-tests/vfio-correctness-tests 43
> [   84.567643] ------------[ cut here ]------------
> [   84.572258] WARNING: CPU: 0 PID: 1606 at
> drivers/iommu/exynos-iommu.c:472 __exynos_sysmmu_enable+0x170/0x184()
> [   84.582225] Modules linked in:
> [   84.585268] CPU: 0 PID: 1606 Comm: vfio-correctnes Not tainted
> 3.13.0-rc1-00296-gb703d74-dirty #46
> [   84.594219] [<c0014d20>] (unwind_backtrace+0x0/0xf4) from [<c0011648>]
> (show_stack+0x10/0x14)
> [   84.602719] [<c0011648>] (show_stack+0x10/0x14) from [<c0377948>]
> (dump_stack+0x7c/0xb0)
> [   84.610791] [<c0377948>] (dump_stack+0x7c/0xb0) from [<c001dc50>]
> (warn_slowpath_common+0x6c/0x88)
> [   84.619730] [<c001dc50>] (warn_slowpath_common+0x6c/0x88) from
> [<c001dd08>] (warn_slowpath_null+0x1c/0x24)
> [   84.629365] [<c001dd08>] (warn_slowpath_null+0x1c/0x24) from
> [<c02dace8>] (__exynos_sysmmu_enable+0x170/0x184)
> [   84.639348] [<c02dace8>] (__exynos_sysmmu_enable+0x170/0x184) from
> [<c02dad4c>] (exynos_iommu_attach_device+0x50/0x108)
> [   84.650111] [<c02dad4c>] (exynos_iommu_attach_device+0x50/0x108) from
> [<c02d939c>] (iommu_attach_device+0x64/0xb8)
> [   84.660440] [<c02d939c>] (iommu_attach_device+0x64/0xb8) from
> [<c02d92ec>] (iommu_group_for_each_dev+0x78/0x94)
> [   84.670509] [<c02d92ec>] (iommu_group_for_each_dev+0x78/0x94) from
> [<c0263678>] (vfio_iommu_type1_attach_group+0xd0/0x144)
> [   84.681530] [<c0263678>] (vfio_iommu_type1_attach_group+0xd0/0x144)
> from [<c02629e0>] (vfio_fops_unl_ioctl+0x3ac/0x460)
> [   84.692295] [<c02629e0>] (vfio_fops_unl_ioctl+0x3ac/0x460) from
> [<c00c1f14>] (do_vfs_ioctl+0x3f0/0x5b8)
> [   84.701668] [<c00c1f14>] (do_vfs_ioctl+0x3f0/0x5b8) from [<c00c2114>]
> (SyS_ioctl+0x38/0x60)
> [   84.710002] [<c00c2114>] (SyS_ioctl+0x38/0x60) from [<c000e1a0>]
> (ret_fast_syscall+0x0/0x30)
> [   84.718417] ---[ end trace a1400d7981f699ce ]---
> [   84.723071] vfio-platform 14650000.sysmmu: exynos_iommu_attach_device:
> Failed to attach IOMMU with pgtable 0x6eae8000
>
> It's failing this test:
>
>         if (WARN_ON(!client))
>                 return -ENODEV;
>
> but I haven't been able to figure out why...I have the exynos
> iommu v10 patchseries applied, and even though there's not much
> documentation available for this device, I figure this simple test is
> supposed to 'just work.'  Any ideas?
>

Hello Kim,

Thanks for your interest on our VFIO work.

VFIO for platform is still under development, and cannot be used yet to
fully assign a device. However, looking at the example you ran, I would not
have expected it to fail so spectacularly, so this is definitely something
I will investigate for our next update to the patch series. Thanks for
pointing this out. I plan to release an updated series with several
improvements and fixes soon enough.

Best regards,
Antonios


>
> Thanks,
>
> Kim
>
> [1] git://github.com/awilliam/tests.git
>

[-- Attachment #1.2: Type: text/html, Size: 5319 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Antonios Motakis]

On Sun, Dec 1, 2013 at 1:09 PM, Kim Phillips <kim.phillips-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>
> On Mon, 30 Sep 2013 17:28:36 +0200
> Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> wrote:
>
> > This is a preview of the base work, towards VFIO support on ARM platforms
> > with an IOMMU. It forms a base on to which to implement the functionality
> > necessary to enable using device tree devices on ARM (and other platforms
> > based on device trees) with VFIO.
> >
> > This patch series has been subjected to limited testing on the Arndale board
> > (with the Exynos 5250 System MMU). More extensive testing will follow as more
> > features are implemented.
>
> Hi, I'm trying to attach the vfio-platform driver to an(y) iommu-backed
> device on the arndale.  I chose the relatively simple 'tv' device,
> isolated it in the arndale device tree (to get rid of any dependency
> conflicts), and, when I run Alex' vfio-correctness-tests [1], I get
> this:
>
> # echo 14650000.sysmmu > /sys/bus/platform/devices/14650000.sysmmu/driver/unbind
> # echo 14650000.sysmmu > /sys/bus/platform/drivers/vfio-platform/bind
> # ls -l /sys/bus/platform/devices/14650000.sysmmu/iommu_group
> lrwxrwxrwx 1 root root 0 Oct  1 23:14 /sys/bus/platform/devices/14650000.sysmmu/iommu_group -> ../../kernel/iommu_groups/43
> # vfio-tests/vfio-correctness-tests 43
> [   84.567643] ------------[ cut here ]------------
> [   84.572258] WARNING: CPU: 0 PID: 1606 at drivers/iommu/exynos-iommu.c:472 __exynos_sysmmu_enable+0x170/0x184()
> [   84.582225] Modules linked in:
> [   84.585268] CPU: 0 PID: 1606 Comm: vfio-correctnes Not tainted 3.13.0-rc1-00296-gb703d74-dirty #46
> [   84.594219] [<c0014d20>] (unwind_backtrace+0x0/0xf4) from [<c0011648>] (show_stack+0x10/0x14)
> [   84.602719] [<c0011648>] (show_stack+0x10/0x14) from [<c0377948>] (dump_stack+0x7c/0xb0)
> [   84.610791] [<c0377948>] (dump_stack+0x7c/0xb0) from [<c001dc50>] (warn_slowpath_common+0x6c/0x88)
> [   84.619730] [<c001dc50>] (warn_slowpath_common+0x6c/0x88) from [<c001dd08>] (warn_slowpath_null+0x1c/0x24)
> [   84.629365] [<c001dd08>] (warn_slowpath_null+0x1c/0x24) from [<c02dace8>] (__exynos_sysmmu_enable+0x170/0x184)
> [   84.639348] [<c02dace8>] (__exynos_sysmmu_enable+0x170/0x184) from [<c02dad4c>] (exynos_iommu_attach_device+0x50/0x108)
> [   84.650111] [<c02dad4c>] (exynos_iommu_attach_device+0x50/0x108) from [<c02d939c>] (iommu_attach_device+0x64/0xb8)
> [   84.660440] [<c02d939c>] (iommu_attach_device+0x64/0xb8) from [<c02d92ec>] (iommu_group_for_each_dev+0x78/0x94)
> [   84.670509] [<c02d92ec>] (iommu_group_for_each_dev+0x78/0x94) from [<c0263678>] (vfio_iommu_type1_attach_group+0xd0/0x144)
> [   84.681530] [<c0263678>] (vfio_iommu_type1_attach_group+0xd0/0x144) from [<c02629e0>] (vfio_fops_unl_ioctl+0x3ac/0x460)
> [   84.692295] [<c02629e0>] (vfio_fops_unl_ioctl+0x3ac/0x460) from [<c00c1f14>] (do_vfs_ioctl+0x3f0/0x5b8)
> [   84.701668] [<c00c1f14>] (do_vfs_ioctl+0x3f0/0x5b8) from [<c00c2114>] (SyS_ioctl+0x38/0x60)
> [   84.710002] [<c00c2114>] (SyS_ioctl+0x38/0x60) from [<c000e1a0>] (ret_fast_syscall+0x0/0x30)
> [   84.718417] ---[ end trace a1400d7981f699ce ]---
> [   84.723071] vfio-platform 14650000.sysmmu: exynos_iommu_attach_device: Failed to attach IOMMU with pgtable 0x6eae8000
>
> It's failing this test:
>
>         if (WARN_ON(!client))
>                 return -ENODEV;
>
> but I haven't been able to figure out why...I have the exynos
> iommu v10 patchseries applied, and even though there's not much
> documentation available for this device, I figure this simple test is
> supposed to 'just work.'  Any ideas?
>

Hello Kim,

Thanks for your interest on our VFIO work.

VFIO for platform is still under development, and cannot be used yet
to fully assign a device. However, looking at the example you ran, I
would not have expected it to fail so spectacularly, so this is
definitely something I will investigate for our next update to the
patch series. Thanks for pointing this out. I plan to release an
updated series with several improvements and fixes soon enough.

Best regards,
Antonios

>
> Thanks,
>
> Kim
>
> [1] git://github.com/awilliam/tests.git

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Kim Phillips]

On Mon, 2 Dec 2013 14:56:58 +0100
Antonios Motakis <a.motakis@virtualopensystems.com> wrote:

> VFIO for platform is still under development, and cannot be used yet
> to fully assign a device. However, looking at the example you ran, I
> would not have expected it to fail so spectacularly, so this is
> definitely something I will investigate for our next update to the

it fails in executing this call in vfio-correctness-tests.c [1]:

ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU);

> patch series. Thanks for pointing this out. I plan to release an
> updated series with several improvements and fixes soon enough.

may I ask how you test the patchseries currently?  If on arndale, any
particular device?  Also, I'm assuming you're using a different
userspace test than Alex Williamson's vfio-tests?

Kim

[1] git://github.com/awilliam/tests.git

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Antonios Motakis]

On Mon, Dec 2, 2013 at 5:08 PM, Kim Phillips <kim.phillips-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Mon, 2 Dec 2013 14:56:58 +0100
> Antonios Motakis <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org> wrote:
>
>> VFIO for platform is still under development, and cannot be used yet
>> to fully assign a device. However, looking at the example you ran, I
>> would not have expected it to fail so spectacularly, so this is
>> definitely something I will investigate for our next update to the
>
> it fails in executing this call in vfio-correctness-tests.c [1]:
>
> ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU);
>

Hm, I realize now that you where trying to bind the SMMU to the VFIO
driver. This is not the right way to use it, the SMMU device needs to
be bound by a valid SMMU driver. However, you need to bind the target
device to vfio-platform. To do that you need to edit the device tree,
as mentioned in the patch series description; this is because at the
time it was not possible to have a platform driver dynamically bind to
any device (this particular point will be fixed in future versions)

Also, you need to check from the device tree if the device is actually
linked with an SMMU. In my case I was testing using the MFC.

>> patch series. Thanks for pointing this out. I plan to release an
>> updated series with several improvements and fixes soon enough.
>
> may I ask how you test the patchseries currently?  If on arndale, any
> particular device?  Also, I'm assuming you're using a different
> userspace test than Alex Williamson's vfio-tests?
>

Since this is a work in progress, I initially used a custom test where
I directly put each IOCTL I was testing. Since devices on the Arndale
are not very well documented, today I am testing on the FastModels
SMMU model which includes a pl330 controller.

Keep in mind this version of the patchset is not supposed to fully let
you use a device with VFIO yet, just discover it and attempt some
basic mappings. In the meantime we have fixed several bugs when
actually trying to access memory regions, and added missing
functionality. Soon we will release a functional version along with
some easily reproducible tests.

Antonios

> Kim
>
> [1] git://github.com/awilliam/tests.git

Re: [RFC PATCH v2 0/7] VFIO for device tree based platform devices (work in progress)

[Kim Phillips]

On Mon, 2 Dec 2013 17:30:11 +0100
Antonios Motakis <a.motakis@virtualopensystems.com> wrote:

> On Mon, Dec 2, 2013 at 5:08 PM, Kim Phillips <kim.phillips@linaro.org> wrote:
> > On Mon, 2 Dec 2013 14:56:58 +0100
> > Antonios Motakis <a.motakis@virtualopensystems.com> wrote:
> >
> >> VFIO for platform is still under development, and cannot be used yet
> >> to fully assign a device. However, looking at the example you ran, I
> >> would not have expected it to fail so spectacularly, so this is
> >> definitely something I will investigate for our next update to the
> >
> > it fails in executing this call in vfio-correctness-tests.c [1]:
> >
> > ioctl(container, VFIO_SET_IOMMU, VFIO_TYPE1_IOMMU);
> >
> 
> Hm, I realize now that you where trying to bind the SMMU to the VFIO
> driver. This is not the right way to use it, the SMMU device needs to
> be bound by a valid SMMU driver. However, you need to bind the target

look at the device address - not the compatible name:  I'm binding the
'tv' device, made compatible with the sysmmu via Cho KyongHo's "ARM:
dts: Add description of System MMU of Exynos SoCs":

       sysmmu_tv: sysmmu@14650000 {
               compatible = "samsung,exynos4210-sysmmu";
               reg = <0x14650000 0x1000>;
               interrupt-parent = <&combiner>;
               interrupts = <7 4>;
               clock-names = "sysmmu";
               clocks = <&clock 349>;
               samsung,power-domain = <&pd_disp1>;
       };

although I don't understand why it's being made compatible with the
sysmmu driver.

> device to vfio-platform. To do that you need to edit the device tree,
> as mentioned in the patch series description; this is because at the
> time it was not possible to have a platform driver dynamically bind to
> any device (this particular point will be fixed in future versions)

I'm using this:

https://lkml.org/lkml/2013/10/11/51

to bind.

> Also, you need to check from the device tree if the device is actually
> linked with an SMMU. In my case I was testing using the MFC.

ok, I thought it was, but I'll double-check, or may try using the MFC.

> >> patch series. Thanks for pointing this out. I plan to release an
> >> updated series with several improvements and fixes soon enough.
> >
> > may I ask how you test the patchseries currently?  If on arndale, any
> > particular device?  Also, I'm assuming you're using a different
> > userspace test than Alex Williamson's vfio-tests?
> >
> 
> Since this is a work in progress, I initially used a custom test where
> I directly put each IOCTL I was testing. Since devices on the Arndale
> are not very well documented, today I am testing on the FastModels
> SMMU model which includes a pl330 controller.

ok, thanks for the info.

> Keep in mind this version of the patchset is not supposed to fully let
> you use a device with VFIO yet, just discover it and attempt some
> basic mappings. In the meantime we have fixed several bugs when
> actually trying to access memory regions, and added missing
> functionality. Soon we will release a functional version along with
> some easily reproducible tests.

ok, no problem, thanks.

Kim