Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI pages

2015-10-06 Thread Alex Williamson 
On Tue, 2015-10-06 at 09:05 +, Bhushan Bharat wrote:
> 
> 
> > -Original Message-
> > From: Alex Williamson [mailto:alex.william...@redhat.com]
> > Sent: Tuesday, October 06, 2015 4:15 AM
> > To: Bhushan Bharat-R65777 <bharat.bhus...@freescale.com>
> > Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
> > christoffer.d...@linaro.org; eric.au...@linaro.org; pranavku...@linaro.org;
> > marc.zyng...@arm.com; will.dea...@arm.com
> > Subject: Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI
> > pages
> > 
> > On Mon, 2015-10-05 at 06:27 +, Bhushan Bharat wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: Alex Williamson [mailto:alex.william...@redhat.com]
> > > > Sent: Saturday, October 03, 2015 4:16 AM
> > > > To: Bhushan Bharat-R65777 <bharat.bhus...@freescale.com>
> > > > Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
> > > > christoffer.d...@linaro.org; eric.au...@linaro.org;
> > > > pranavku...@linaro.org; marc.zyng...@arm.com; will.dea...@arm.com
> > > > Subject: Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap
> > > > MSI pages
> > > >
> > > > On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote:
> > > > > For MSI interrupts to work for a pass-through devices we need to
> > > > > have mapping of msi-pages in iommu. Now on some platforms (like
> > > > > x86) does this msi-pages mapping happens magically and in these
> > > > > case they chooses an iova which they somehow know that it will
> > > > > never overlap with guest memory. But this magic iova selection may
> > > > > not be always true for all platform (like PowerPC and ARM64).
> > > > >
> > > > > Also on x86 platform, there is no problem as long as running a
> > > > > x86-guest on x86-host but there can be issues when running a
> > > > > non-x86 guest on
> > > > > x86 host or other userspace applications like (I think ODP/DPDK).
> > > > > As in these cases there can be chances that it overlaps with guest
> > > > > memory mapping.
> > > >
> > > > Wow, it's amazing anything works... smoke and mirrors.
> > > >
> > > > > This patch add interface to iommu-map and iommu-unmap msi-pages
> > at
> > > > > reserved iova chosen by userspace.
> > > > >
> > > > > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> > > > > ---
> > > > >  drivers/vfio/vfio.c |  52 +++
> > > > >  drivers/vfio/vfio_iommu_type1.c | 111
> > > > 
> > > > >  include/linux/vfio.h|   9 +++-
> > > > >  3 files changed, 171 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index
> > > > > 2fb29df..a817d2d 100644
> > > > > --- a/drivers/vfio/vfio.c
> > > > > +++ b/drivers/vfio/vfio.c
> > > > > @@ -605,6 +605,58 @@ static int vfio_iommu_group_notifier(struct
> > > > notifier_block *nb,
> > > > >   return NOTIFY_OK;
> > > > >  }
> > > > >
> > > > > +int vfio_device_map_msi(struct vfio_device *device, uint64_t
> > msi_addr,
> > > > > + uint32_t size, uint64_t *msi_iova) {
> > > > > + struct vfio_container *container = device->group->container;
> > > > > + struct vfio_iommu_driver *driver;
> > > > > + int ret;
> > > > > +
> > > > > + /* Validate address and size */
> > > > > + if (!msi_addr || !size || !msi_iova)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + down_read(>group_lock);
> > > > > +
> > > > > + driver = container->iommu_driver;
> > > > > + if (!driver || !driver->ops || !driver->ops->msi_map) {
> > > > > + up_read(>group_lock);
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + ret = driver->ops->msi_map(container->iommu_data,
> > > > > +msi_addr, size, msi_iova);
> > > > > +
> > > > > + up_read(

RE: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI pages

2015-10-06 Thread Bhushan Bharat 


> -Original Message-
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Tuesday, October 06, 2015 4:15 AM
> To: Bhushan Bharat-R65777 <bharat.bhus...@freescale.com>
> Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
> christoffer.d...@linaro.org; eric.au...@linaro.org; pranavku...@linaro.org;
> marc.zyng...@arm.com; will.dea...@arm.com
> Subject: Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI
> pages
> 
> On Mon, 2015-10-05 at 06:27 +, Bhushan Bharat wrote:
> >
> >
> > > -Original Message-
> > > From: Alex Williamson [mailto:alex.william...@redhat.com]
> > > Sent: Saturday, October 03, 2015 4:16 AM
> > > To: Bhushan Bharat-R65777 <bharat.bhus...@freescale.com>
> > > Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
> > > christoffer.d...@linaro.org; eric.au...@linaro.org;
> > > pranavku...@linaro.org; marc.zyng...@arm.com; will.dea...@arm.com
> > > Subject: Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap
> > > MSI pages
> > >
> > > On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote:
> > > > For MSI interrupts to work for a pass-through devices we need to
> > > > have mapping of msi-pages in iommu. Now on some platforms (like
> > > > x86) does this msi-pages mapping happens magically and in these
> > > > case they chooses an iova which they somehow know that it will
> > > > never overlap with guest memory. But this magic iova selection may
> > > > not be always true for all platform (like PowerPC and ARM64).
> > > >
> > > > Also on x86 platform, there is no problem as long as running a
> > > > x86-guest on x86-host but there can be issues when running a
> > > > non-x86 guest on
> > > > x86 host or other userspace applications like (I think ODP/DPDK).
> > > > As in these cases there can be chances that it overlaps with guest
> > > > memory mapping.
> > >
> > > Wow, it's amazing anything works... smoke and mirrors.
> > >
> > > > This patch add interface to iommu-map and iommu-unmap msi-pages
> at
> > > > reserved iova chosen by userspace.
> > > >
> > > > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> > > > ---
> > > >  drivers/vfio/vfio.c |  52 +++
> > > >  drivers/vfio/vfio_iommu_type1.c | 111
> > > 
> > > >  include/linux/vfio.h|   9 +++-
> > > >  3 files changed, 171 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index
> > > > 2fb29df..a817d2d 100644
> > > > --- a/drivers/vfio/vfio.c
> > > > +++ b/drivers/vfio/vfio.c
> > > > @@ -605,6 +605,58 @@ static int vfio_iommu_group_notifier(struct
> > > notifier_block *nb,
> > > > return NOTIFY_OK;
> > > >  }
> > > >
> > > > +int vfio_device_map_msi(struct vfio_device *device, uint64_t
> msi_addr,
> > > > +   uint32_t size, uint64_t *msi_iova) {
> > > > +   struct vfio_container *container = device->group->container;
> > > > +   struct vfio_iommu_driver *driver;
> > > > +   int ret;
> > > > +
> > > > +   /* Validate address and size */
> > > > +   if (!msi_addr || !size || !msi_iova)
> > > > +   return -EINVAL;
> > > > +
> > > > +   down_read(>group_lock);
> > > > +
> > > > +   driver = container->iommu_driver;
> > > > +   if (!driver || !driver->ops || !driver->ops->msi_map) {
> > > > +   up_read(>group_lock);
> > > > +   return -EINVAL;
> > > > +   }
> > > > +
> > > > +   ret = driver->ops->msi_map(container->iommu_data,
> > > > +  msi_addr, size, msi_iova);
> > > > +
> > > > +   up_read(>group_lock);
> > > > +   return ret;
> > > > +}
> > > > +
> > > > +int vfio_device_unmap_msi(struct vfio_device *device, uint64_t
> > > msi_iova,
> > > > + uint64_t size)
> > > > +{
> > > > +   struct vfio_container *container = device->group->container;
> > > > +   struct vfio_iommu_driver

RE: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI pages

2015-10-05 Thread Bhushan Bharat 


> -Original Message-
> From: Alex Williamson [mailto:alex.william...@redhat.com]
> Sent: Saturday, October 03, 2015 4:16 AM
> To: Bhushan Bharat-R65777 <bharat.bhus...@freescale.com>
> Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
> christoffer.d...@linaro.org; eric.au...@linaro.org; pranavku...@linaro.org;
> marc.zyng...@arm.com; will.dea...@arm.com
> Subject: Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI
> pages
> 
> On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote:
> > For MSI interrupts to work for a pass-through devices we need to have
> > mapping of msi-pages in iommu. Now on some platforms (like x86) does
> > this msi-pages mapping happens magically and in these case they
> > chooses an iova which they somehow know that it will never overlap
> > with guest memory. But this magic iova selection may not be always
> > true for all platform (like PowerPC and ARM64).
> >
> > Also on x86 platform, there is no problem as long as running a
> > x86-guest on x86-host but there can be issues when running a non-x86
> > guest on
> > x86 host or other userspace applications like (I think ODP/DPDK).
> > As in these cases there can be chances that it overlaps with guest
> > memory mapping.
> 
> Wow, it's amazing anything works... smoke and mirrors.
> 
> > This patch add interface to iommu-map and iommu-unmap msi-pages at
> > reserved iova chosen by userspace.
> >
> > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> > ---
> >  drivers/vfio/vfio.c |  52 +++
> >  drivers/vfio/vfio_iommu_type1.c | 111
> 
> >  include/linux/vfio.h|   9 +++-
> >  3 files changed, 171 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index
> > 2fb29df..a817d2d 100644
> > --- a/drivers/vfio/vfio.c
> > +++ b/drivers/vfio/vfio.c
> > @@ -605,6 +605,58 @@ static int vfio_iommu_group_notifier(struct
> notifier_block *nb,
> > return NOTIFY_OK;
> >  }
> >
> > +int vfio_device_map_msi(struct vfio_device *device, uint64_t msi_addr,
> > +   uint32_t size, uint64_t *msi_iova) {
> > +   struct vfio_container *container = device->group->container;
> > +   struct vfio_iommu_driver *driver;
> > +   int ret;
> > +
> > +   /* Validate address and size */
> > +   if (!msi_addr || !size || !msi_iova)
> > +   return -EINVAL;
> > +
> > +   down_read(>group_lock);
> > +
> > +   driver = container->iommu_driver;
> > +   if (!driver || !driver->ops || !driver->ops->msi_map) {
> > +   up_read(>group_lock);
> > +   return -EINVAL;
> > +   }
> > +
> > +   ret = driver->ops->msi_map(container->iommu_data,
> > +  msi_addr, size, msi_iova);
> > +
> > +   up_read(>group_lock);
> > +   return ret;
> > +}
> > +
> > +int vfio_device_unmap_msi(struct vfio_device *device, uint64_t
> msi_iova,
> > + uint64_t size)
> > +{
> > +   struct vfio_container *container = device->group->container;
> > +   struct vfio_iommu_driver *driver;
> > +   int ret;
> > +
> > +   /* Validate address and size */
> > +   if (!msi_iova || !size)
> > +   return -EINVAL;
> > +
> > +   down_read(>group_lock);
> > +
> > +   driver = container->iommu_driver;
> > +   if (!driver || !driver->ops || !driver->ops->msi_unmap) {
> > +   up_read(>group_lock);
> > +   return -EINVAL;
> > +   }
> > +
> > +   ret = driver->ops->msi_unmap(container->iommu_data,
> > +msi_iova, size);
> > +
> > +   up_read(>group_lock);
> > +   return ret;
> > +}
> > +
> >  /**
> >   * VFIO driver API
> >   */
> > diff --git a/drivers/vfio/vfio_iommu_type1.c
> > b/drivers/vfio/vfio_iommu_type1.c index 3315fb6..ab376c2 100644
> > --- a/drivers/vfio/vfio_iommu_type1.c
> > +++ b/drivers/vfio/vfio_iommu_type1.c
> > @@ -1003,12 +1003,34 @@ out_free:
> > return ret;
> >  }
> >
> > +static void vfio_iommu_unmap_all_reserved_regions(struct vfio_iommu
> > +*iommu) {
> > +   struct vfio_resvd_region *region;
> > +   struct vfio_domain *d;
> > +
> > +   list_for_each_entry(region, >reserved_iova_list, next) {
> > +   list_for_each_entry(d, >domain_l

Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI pages

2015-10-05 Thread Alex Williamson 
On Mon, 2015-10-05 at 06:27 +, Bhushan Bharat wrote:
> 
> 
> > -Original Message-
> > From: Alex Williamson [mailto:alex.william...@redhat.com]
> > Sent: Saturday, October 03, 2015 4:16 AM
> > To: Bhushan Bharat-R65777 <bharat.bhus...@freescale.com>
> > Cc: kvm...@lists.cs.columbia.edu; kvm@vger.kernel.org;
> > christoffer.d...@linaro.org; eric.au...@linaro.org; pranavku...@linaro.org;
> > marc.zyng...@arm.com; will.dea...@arm.com
> > Subject: Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI
> > pages
> > 
> > On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote:
> > > For MSI interrupts to work for a pass-through devices we need to have
> > > mapping of msi-pages in iommu. Now on some platforms (like x86) does
> > > this msi-pages mapping happens magically and in these case they
> > > chooses an iova which they somehow know that it will never overlap
> > > with guest memory. But this magic iova selection may not be always
> > > true for all platform (like PowerPC and ARM64).
> > >
> > > Also on x86 platform, there is no problem as long as running a
> > > x86-guest on x86-host but there can be issues when running a non-x86
> > > guest on
> > > x86 host or other userspace applications like (I think ODP/DPDK).
> > > As in these cases there can be chances that it overlaps with guest
> > > memory mapping.
> > 
> > Wow, it's amazing anything works... smoke and mirrors.
> > 
> > > This patch add interface to iommu-map and iommu-unmap msi-pages at
> > > reserved iova chosen by userspace.
> > >
> > > Signed-off-by: Bharat Bhushan <bharat.bhus...@freescale.com>
> > > ---
> > >  drivers/vfio/vfio.c |  52 +++
> > >  drivers/vfio/vfio_iommu_type1.c | 111
> > 
> > >  include/linux/vfio.h|   9 +++-
> > >  3 files changed, 171 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index
> > > 2fb29df..a817d2d 100644
> > > --- a/drivers/vfio/vfio.c
> > > +++ b/drivers/vfio/vfio.c
> > > @@ -605,6 +605,58 @@ static int vfio_iommu_group_notifier(struct
> > notifier_block *nb,
> > >   return NOTIFY_OK;
> > >  }
> > >
> > > +int vfio_device_map_msi(struct vfio_device *device, uint64_t msi_addr,
> > > + uint32_t size, uint64_t *msi_iova) {
> > > + struct vfio_container *container = device->group->container;
> > > + struct vfio_iommu_driver *driver;
> > > + int ret;
> > > +
> > > + /* Validate address and size */
> > > + if (!msi_addr || !size || !msi_iova)
> > > + return -EINVAL;
> > > +
> > > + down_read(>group_lock);
> > > +
> > > + driver = container->iommu_driver;
> > > + if (!driver || !driver->ops || !driver->ops->msi_map) {
> > > + up_read(>group_lock);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = driver->ops->msi_map(container->iommu_data,
> > > +msi_addr, size, msi_iova);
> > > +
> > > + up_read(>group_lock);
> > > + return ret;
> > > +}
> > > +
> > > +int vfio_device_unmap_msi(struct vfio_device *device, uint64_t
> > msi_iova,
> > > +   uint64_t size)
> > > +{
> > > + struct vfio_container *container = device->group->container;
> > > + struct vfio_iommu_driver *driver;
> > > + int ret;
> > > +
> > > + /* Validate address and size */
> > > + if (!msi_iova || !size)
> > > + return -EINVAL;
> > > +
> > > + down_read(>group_lock);
> > > +
> > > + driver = container->iommu_driver;
> > > + if (!driver || !driver->ops || !driver->ops->msi_unmap) {
> > > + up_read(>group_lock);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + ret = driver->ops->msi_unmap(container->iommu_data,
> > > +  msi_iova, size);
> > > +
> > > + up_read(>group_lock);
> > > + return ret;
> > > +}
> > > +
> > >  /**
> > >   * VFIO driver API
> > >   */
> > > diff --git a/drivers/vfio/vfio_iommu_type1.c
> > > b/drivers/vfio/vfio_iommu_type1.c index 3315fb6..ab376c2 100644
> > > --- a/drive

Re: [RFC PATCH 4/6] vfio: Add interface to iommu-map/unmap MSI pages

2015-10-02 Thread Alex Williamson 
On Wed, 2015-09-30 at 20:26 +0530, Bharat Bhushan wrote:
> For MSI interrupts to work for a pass-through devices we need
> to have mapping of msi-pages in iommu. Now on some platforms
> (like x86) does this msi-pages mapping happens magically and in these
> case they chooses an iova which they somehow know that it will never
> overlap with guest memory. But this magic iova selection
> may not be always true for all platform (like PowerPC and ARM64).
> 
> Also on x86 platform, there is no problem as long as running a x86-guest
> on x86-host but there can be issues when running a non-x86 guest on
> x86 host or other userspace applications like (I think ODP/DPDK).
> As in these cases there can be chances that it overlaps with guest
> memory mapping.

Wow, it's amazing anything works... smoke and mirrors.

> This patch add interface to iommu-map and iommu-unmap msi-pages at
> reserved iova chosen by userspace.
> 
> Signed-off-by: Bharat Bhushan 
> ---
>  drivers/vfio/vfio.c |  52 +++
>  drivers/vfio/vfio_iommu_type1.c | 111 
> 
>  include/linux/vfio.h|   9 +++-
>  3 files changed, 171 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2fb29df..a817d2d 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -605,6 +605,58 @@ static int vfio_iommu_group_notifier(struct 
> notifier_block *nb,
>   return NOTIFY_OK;
>  }
>  
> +int vfio_device_map_msi(struct vfio_device *device, uint64_t msi_addr,
> + uint32_t size, uint64_t *msi_iova)
> +{
> + struct vfio_container *container = device->group->container;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + /* Validate address and size */
> + if (!msi_addr || !size || !msi_iova)
> + return -EINVAL;
> +
> + down_read(>group_lock);
> +
> + driver = container->iommu_driver;
> + if (!driver || !driver->ops || !driver->ops->msi_map) {
> + up_read(>group_lock);
> + return -EINVAL;
> + }
> +
> + ret = driver->ops->msi_map(container->iommu_data,
> +msi_addr, size, msi_iova);
> +
> + up_read(>group_lock);
> + return ret;
> +}
> +
> +int vfio_device_unmap_msi(struct vfio_device *device, uint64_t msi_iova,
> +   uint64_t size)
> +{
> + struct vfio_container *container = device->group->container;
> + struct vfio_iommu_driver *driver;
> + int ret;
> +
> + /* Validate address and size */
> + if (!msi_iova || !size)
> + return -EINVAL;
> +
> + down_read(>group_lock);
> +
> + driver = container->iommu_driver;
> + if (!driver || !driver->ops || !driver->ops->msi_unmap) {
> + up_read(>group_lock);
> + return -EINVAL;
> + }
> +
> + ret = driver->ops->msi_unmap(container->iommu_data,
> +  msi_iova, size);
> +
> + up_read(>group_lock);
> + return ret;
> +}
> +
>  /**
>   * VFIO driver API
>   */
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 3315fb6..ab376c2 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -1003,12 +1003,34 @@ out_free:
>   return ret;
>  }
>  
> +static void vfio_iommu_unmap_all_reserved_regions(struct vfio_iommu *iommu)
> +{
> + struct vfio_resvd_region *region;
> + struct vfio_domain *d;
> +
> + list_for_each_entry(region, >reserved_iova_list, next) {
> + list_for_each_entry(d, >domain_list, next) {
> + if (!region->map_paddr)
> + continue;
> +
> + if (!iommu_iova_to_phys(d->domain, region->iova))
> + continue;
> +
> + iommu_unmap(d->domain, region->iova, PAGE_SIZE);

PAGE_SIZE?  Why not region->size?

> + region->map_paddr = 0;
> + cond_resched();
> + }
> + }
> +}
> +
>  static void vfio_iommu_unmap_unpin_all(struct vfio_iommu *iommu)
>  {
>   struct rb_node *node;
>  
>   while ((node = rb_first(>dma_list)))
>   vfio_remove_dma(iommu, rb_entry(node, struct vfio_dma, node));
> +
> + vfio_iommu_unmap_all_reserved_regions(iommu);
>  }
>  
>  static void vfio_iommu_type1_detach_group(void *iommu_data,
> @@ -1048,6 +1070,93 @@ done:
>   mutex_unlock(>lock);
>  }
>  
> +static int vfio_iommu_type1_msi_map(void *iommu_data, uint64_t msi_addr,
> + uint64_t size, uint64_t *msi_iova)
> +{
> + struct vfio_iommu *iommu = iommu_data;
> + struct vfio_resvd_region *region;
> + int ret;
> +
> + mutex_lock(>lock);
> +
> + /* Do not try ceate iommu-mapping if msi reconfig not allowed */
> + if (!iommu->allow_msi_reconfig) {
> + mutex_unlock(>lock);
> +