Re: [PATCH 10/22] iommu: Introduce direct mapped region handling

[Robin Murphy]

On 28/05/15 17:41, Joerg Roedel wrote:

From: Joerg Roedel 

Add two new functions to the IOMMU-API to allow the IOMMU
drivers to export the requirements for direct mapped regions
per device.
This is useful for exporting the information in Intel VT-d's
RMRR entries or AMD-Vi's unity mappings.


Whilst I agree with the utility of a common abstraction for the IOMMU 
core to use (I think we could easily turn Marek's proposal for ARM[1] 
into a generic of_iommu_ backend for this), I'm less convinced that it 
needs to be exported as part of the API.


I was envisioning eventually moving the IOVA data inside the 
iommu_domain itself for the managed DMA-API implementation, so that the 
IOMMU core would be responsible for setting it up. Then the core just 
needs to ask the driver about any relevant reservations as it creates a 
domain/attaches a device, and the callers can remain in blissful 
ignorance. Besides, beyond probe time, I don't see many good reasons at 
all for a caller to have a struct device for something, yet not have 
taken control of any DMA the firmware left running.


Is that at odds with your ideas?

Robin.

[1]:http://article.gmane.org/gmane.linux.kernel.samsung-soc/45429


Signed-off-by: Joerg Roedel 
---
  drivers/iommu/iommu.c | 16 
  include/linux/iommu.h | 31 +++
  2 files changed, 47 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index a0a38bd..6b8d6e7 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1469,3 +1469,19 @@ int iommu_domain_set_attr(struct iommu_domain *domain,
return ret;
  }
  EXPORT_SYMBOL_GPL(iommu_domain_set_attr);
+
+void iommu_get_dm_regions(struct device *dev, struct list_head *list)
+{
+   const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+   if (ops && ops->get_dm_regions)
+   ops->get_dm_regions(dev, list);
+}
+
+void iommu_put_dm_regions(struct device *dev, struct list_head *list)
+{
+   const struct iommu_ops *ops = dev->bus->iommu_ops;
+
+   if (ops && ops->put_dm_regions)
+   ops->put_dm_regions(dev, list);
+}
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 683a1c4..6894999 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -114,6 +114,20 @@ enum iommu_attr {
DOMAIN_ATTR_MAX,
  };

+/**
+ * struct iommu_dm_region - descriptor for a direct mapped memory region
+ * @list: Linked list pointers
+ * @start: System physical start address of the region
+ * @length: Length of the region in bytes
+ * @prot: IOMMU Protection flags (READ/WRITE/...)
+ */
+struct iommu_dm_region {
+   struct list_headlist;
+   phys_addr_t start;
+   size_t  length;
+   int prot;
+};
+
  #ifdef CONFIG_IOMMU_API

  /**
@@ -159,6 +173,10 @@ struct iommu_ops {
int (*domain_set_attr)(struct iommu_domain *domain,
   enum iommu_attr attr, void *data);

+   /* Request/Free a list of direct mapping requirements for a device */
+   void (*get_dm_regions)(struct device *dev, struct list_head *list);
+   void (*put_dm_regions)(struct device *dev, struct list_head *list);
+
/* Window handling functions */
int (*domain_window_enable)(struct iommu_domain *domain, u32 wnd_nr,
phys_addr_t paddr, u64 size, int prot);
@@ -205,6 +223,9 @@ extern phys_addr_t iommu_iova_to_phys(struct iommu_domain 
*domain, dma_addr_t io
  extern void iommu_set_fault_handler(struct iommu_domain *domain,
iommu_fault_handler_t handler, void *token);

+extern void iommu_get_dm_regions(struct device *dev, struct list_head *list);
+extern void iommu_put_dm_regions(struct device *dev, struct list_head *list);
+
  extern int iommu_attach_group(struct iommu_domain *domain,
  struct iommu_group *group);
  extern void iommu_detach_group(struct iommu_domain *domain,
@@ -379,6 +400,16 @@ static inline void iommu_set_fault_handler(struct 
iommu_domain *domain,
  {
  }

+static inline void iommu_get_dm_regions(struct device *dev,
+   struct list_head *list)
+{
+}
+
+static inline void iommu_put_dm_regions(struct device *dev,
+   struct list_head *list)
+{
+}
+
  static inline int iommu_attach_group(struct iommu_domain *domain,
 struct iommu_group *group)
  {



___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 08/22] iommu: Make sure a device is always attached to a domain

[Robin Murphy]

On 28/05/15 17:41, Joerg Roedel wrote:

From: Joerg Roedel 

Make use of the default domain and re-attach a device to it
when it is detached from another domain. Also enforce that a
device has to be in the default domain before it can be
attached to a different domain.

Signed-off-by: Joerg Roedel 
---
  drivers/iommu/iommu.c | 83 ++-
  1 file changed, 75 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index adeedd2..7bce522 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -52,6 +52,7 @@ struct iommu_group {
char *name;
int id;
struct iommu_domain *default_domain;
+   struct iommu_domain *domain;
  };

  struct iommu_device {
@@ -78,6 +79,12 @@ struct iommu_group_attribute iommu_group_attr_##_name =  
\

  static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
 unsigned type);
+static int __iommu_attach_device(struct iommu_domain *domain,
+struct device *dev);
+static int __iommu_attach_group(struct iommu_domain *domain,
+   struct iommu_group *group);
+static void __iommu_detach_group(struct iommu_domain *domain,
+struct iommu_group *group);

  static ssize_t iommu_group_attr_show(struct kobject *kobj,
 struct attribute *__attr, char *buf)
@@ -376,6 +383,8 @@ rename:

mutex_lock(&group->mutex);
list_add_tail(&device->list, &group->devices);
+   if (group->domain)
+   __iommu_attach_device(group->domain, dev);
mutex_unlock(&group->mutex);

/* Notify any listeners about change to group. */
@@ -455,19 +464,30 @@ static int iommu_group_device_count(struct iommu_group 
*group)
   * The group->mutex is held across callbacks, which will block calls to
   * iommu_group_add/remove_device.
   */
-int iommu_group_for_each_dev(struct iommu_group *group, void *data,
-int (*fn)(struct device *, void *))
+static int __iommu_group_for_each_dev(struct iommu_group *group, void *data,
+ int (*fn)(struct device *, void *))
  {
struct iommu_device *device;
int ret = 0;

-   mutex_lock(&group->mutex);
list_for_each_entry(device, &group->devices, list) {


Maybe worth using list_for_each_entry_safe so we can reuse this for 
things like group teardown?



ret = fn(device->dev, data);
if (ret)
break;
}
+   return ret;
+}
+
+
+int iommu_group_for_each_dev(struct iommu_group *group, void *data,
+int (*fn)(struct device *, void *))
+{
+   int ret;
+
+   mutex_lock(&group->mutex);
+   ret = __iommu_group_for_each_dev(group, data, fn);
mutex_unlock(&group->mutex);
+
return ret;
  }
  EXPORT_SYMBOL_GPL(iommu_group_for_each_dev);
@@ -1013,7 +1033,7 @@ int iommu_attach_device(struct iommu_domain *domain, 
struct device *dev)
if (iommu_group_device_count(group) != 1)
goto out_unlock;

-   ret = __iommu_attach_device(domain, dev);
+   ret = __iommu_attach_group(domain, group);

  out_unlock:
mutex_unlock(&group->mutex);
@@ -1048,7 +1068,7 @@ void iommu_detach_device(struct iommu_domain *domain, 
struct device *dev)
goto out_unlock;
}

-   __iommu_detach_device(domain, dev);
+   __iommu_detach_group(domain, group);

  out_unlock:
mutex_unlock(&group->mutex);
@@ -1073,10 +1093,31 @@ static int iommu_group_do_attach_device(struct device 
*dev, void *data)
return __iommu_attach_device(domain, dev);
  }

+static int __iommu_attach_group(struct iommu_domain *domain,
+   struct iommu_group *group)
+{
+   int ret;
+
+   if (group->default_domain && group->domain != group->default_domain)
+   return -EBUSY;
+
+   ret = __iommu_group_for_each_dev(group, domain,
+iommu_group_do_attach_device);
+   if (ret == 0)
+   group->domain = domain;
+
+   return ret;
+}
+
  int iommu_attach_group(struct iommu_domain *domain, struct iommu_group *group)
  {
-   return iommu_group_for_each_dev(group, domain,
-   iommu_group_do_attach_device);
+   int ret;
+
+   mutex_lock(&group->mutex);
+   ret = __iommu_attach_group(domain, group);
+   mutex_unlock(&group->mutex);
+
+   return ret;
  }
  EXPORT_SYMBOL_GPL(iommu_attach_group);

@@ -1089,9 +1130,35 @@ static int iommu_group_do_detach_device(struct device 
*dev, void *data)
return 0;
  }

+static void __iommu_detach_group(struct iommu_domain *domain,
+struct iommu_group *group)
+{
+   int ret;
+
+   if (!group->default_d

Re: [PATCH 06/22] iommu: Allocate a default domain for iommu groups

[Robin Murphy]

Hi Joerg,

On 28/05/15 17:41, Joerg Roedel wrote:

From: Joerg Roedel 

The default domain will be used (if supported by the iommu
driver) when the devices in the iommu group are not attached
to any other domain.

Signed-off-by: Joerg Roedel 
---
  drivers/iommu/iommu.c | 32 
  1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d69e0ca..fbfc015 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -51,6 +51,7 @@ struct iommu_group {
void (*iommu_data_release)(void *iommu_data);
char *name;
int id;
+   struct iommu_domain *default_domain;
  };

  struct iommu_device {
@@ -75,6 +76,9 @@ struct iommu_group_attribute iommu_group_attr_##_name =   
\
  #define to_iommu_group(_kobj) \
container_of(_kobj, struct iommu_group, kobj)

+static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+unsigned type);
+
  static ssize_t iommu_group_attr_show(struct kobject *kobj,
 struct attribute *__attr, char *buf)
  {
@@ -137,6 +141,9 @@ static void iommu_group_release(struct kobject *kobj)
ida_remove(&iommu_group_ida, group->id);
mutex_unlock(&iommu_group_mutex);

+   if (group->default_domain)
+   iommu_domain_free(group->default_domain);
+
kfree(group->name);
kfree(group);
  }
@@ -701,7 +708,18 @@ static struct iommu_group 
*iommu_group_get_for_pci_dev(struct pci_dev *pdev)
return group;

/* No shared group found, allocate new */
-   return iommu_group_alloc();
+   group = iommu_group_alloc();
+   if (group) {
+   /*
+* Try to allocate a default domain - needs support from the
+* IOMMU driver.
+*/
+   group->default_domain = __iommu_domain_alloc(pdev->dev.bus,
+IOMMU_DOMAIN_DMA);
+   group->domain = group->default_domain;
+   }
+
+   return group;
  }

  /**
@@ -922,22 +940,28 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
  }
  EXPORT_SYMBOL_GPL(iommu_set_fault_handler);

-struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
+static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+unsigned type)


I'd be happier if we passed the iommu_ops in here...


  {
struct iommu_domain *domain;

if (bus == NULL || bus->iommu_ops == NULL)
return NULL;

-   domain = bus->iommu_ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
+   domain = bus->iommu_ops->domain_alloc(type);
if (!domain)
return NULL;

domain->ops  = bus->iommu_ops;
-   domain->type = IOMMU_DOMAIN_UNMANAGED;
+   domain->type = type;

return domain;
  }
+
+struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
+{


..having pulled them out of the bus here.


+   return __iommu_domain_alloc(bus, IOMMU_DOMAIN_UNMANAGED);
+}
  EXPORT_SYMBOL_GPL(iommu_domain_alloc);

  void iommu_domain_free(struct iommu_domain *domain)



Even if it's only to symbolically inch us closer to having some other 
solution for platform bus devices. The SMMU page size debate is one 
thing, but it's really only a subtle case of the "IOMMU instances" 
problem. Once there are two different IOMMU drivers in the system each 
serving a subset of platform devices, how can we make sure the domain 
allocated here is wrapped in the right driver-private struct? I've had a 
try with making iommu_ops per-instance, but even that doesn't work in 
this case.


Robin.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 06/22] iommu: Allocate a default domain for iommu groups

[Robin Murphy]

Hi Joerg,

On 28/05/15 17:41, Joerg Roedel wrote:

From: Joerg Roedel 

The default domain will be used (if supported by the iommu
driver) when the devices in the iommu group are not attached
to any other domain.

Signed-off-by: Joerg Roedel 
---
  drivers/iommu/iommu.c | 32 
  1 file changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d69e0ca..fbfc015 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -51,6 +51,7 @@ struct iommu_group {
void (*iommu_data_release)(void *iommu_data);
char *name;
int id;
+   struct iommu_domain *default_domain;
  };

  struct iommu_device {
@@ -75,6 +76,9 @@ struct iommu_group_attribute iommu_group_attr_##_name =   
\
  #define to_iommu_group(_kobj) \
container_of(_kobj, struct iommu_group, kobj)

+static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+unsigned type);
+
  static ssize_t iommu_group_attr_show(struct kobject *kobj,
 struct attribute *__attr, char *buf)
  {
@@ -137,6 +141,9 @@ static void iommu_group_release(struct kobject *kobj)
ida_remove(&iommu_group_ida, group->id);
mutex_unlock(&iommu_group_mutex);

+   if (group->default_domain)
+   iommu_domain_free(group->default_domain);
+
kfree(group->name);
kfree(group);
  }
@@ -701,7 +708,18 @@ static struct iommu_group 
*iommu_group_get_for_pci_dev(struct pci_dev *pdev)
return group;

/* No shared group found, allocate new */
-   return iommu_group_alloc();
+   group = iommu_group_alloc();
+   if (group) {
+   /*
+* Try to allocate a default domain - needs support from the
+* IOMMU driver.
+*/
+   group->default_domain = __iommu_domain_alloc(pdev->dev.bus,
+IOMMU_DOMAIN_DMA);
+   group->domain = group->default_domain;
+   }
+
+   return group;
  }

  /**
@@ -922,22 +940,28 @@ void iommu_set_fault_handler(struct iommu_domain *domain,
  }
  EXPORT_SYMBOL_GPL(iommu_set_fault_handler);

-struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
+static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
+unsigned type)


I'd be happier if we passed the iommu_ops in here...


  {
struct iommu_domain *domain;

if (bus == NULL || bus->iommu_ops == NULL)
return NULL;

-   domain = bus->iommu_ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
+   domain = bus->iommu_ops->domain_alloc(type);
if (!domain)
return NULL;

domain->ops  = bus->iommu_ops;
-   domain->type = IOMMU_DOMAIN_UNMANAGED;
+   domain->type = type;

return domain;
  }
+
+struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
+{


..having pulled them out of the bus here.


+   return __iommu_domain_alloc(bus, IOMMU_DOMAIN_UNMANAGED);
+}
  EXPORT_SYMBOL_GPL(iommu_domain_alloc);

  void iommu_domain_free(struct iommu_domain *domain)



Even if it's only to symbolically inch us closer to having some other 
solution for platform bus devices. The SMMU page size debate is one 
thing, but it's really only a subtle case of the "IOMMU instances" 
problem. Once there are two different IOMMU drivers in the system each 
serving a subset of platform devices, how can we make sure the domain 
allocated here is wrapped in the right driver-private struct? I've had a 
try with making iommu_ops per-instance, but even that doesn't work in 
this case.


Robin.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC/PATCH 0/9] IOMMU probe deferral support

[Will Deacon]

Hi Laurent,

On Fri, May 15, 2015 at 12:00:01AM +0100, Laurent Pinchart wrote:
> This patch series attempts to implement support for deferring probe of both
> IOMMU drivers and bus master drivers.

Have you had a chance to look at any of the feedback you've received on
this?

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 2/4] iommu: Implement common IOMMU ops for DMA mapping

[Robin Murphy]

Taking inspiration from the existing arch/arm code, break out some
generic functions to interface the DMA-API to the IOMMU-API. This will
do the bulk of the heavy lifting for IOMMU-backed dma-mapping.

Signed-off-by: Robin Murphy 
---
 drivers/iommu/Kconfig |   7 +
 drivers/iommu/Makefile|   1 +
 drivers/iommu/dma-iommu.c | 560 ++
 include/linux/dma-iommu.h |  94 
 4 files changed, 662 insertions(+)
 create mode 100644 drivers/iommu/dma-iommu.c
 create mode 100644 include/linux/dma-iommu.h

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 1ae4e54..9107b6e 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -48,6 +48,13 @@ config OF_IOMMU
def_bool y
depends on OF && IOMMU_API
 
+# IOMMU-agnostic DMA-mapping layer
+config IOMMU_DMA
+   bool
+   depends on NEED_SG_DMA_LENGTH
+   select IOMMU_API
+   select IOMMU_IOVA
+
 config FSL_PAMU
bool "Freescale IOMMU support"
depends on PPC32
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index 080ffab..574b241 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -1,6 +1,7 @@
 obj-$(CONFIG_IOMMU_API) += iommu.o
 obj-$(CONFIG_IOMMU_API) += iommu-traces.o
 obj-$(CONFIG_IOMMU_API) += iommu-sysfs.o
+obj-$(CONFIG_IOMMU_DMA) += dma-iommu.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE) += io-pgtable.o
 obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
 obj-$(CONFIG_IOMMU_IOVA) += iova.o
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
new file mode 100644
index 000..ae65c76
--- /dev/null
+++ b/drivers/iommu/dma-iommu.c
@@ -0,0 +1,560 @@
+/*
+ * A fairly generic DMA-API to IOMMU-API glue layer.
+ *
+ * Copyright (C) 2014-2015 ARM Ltd.
+ *
+ * based in part on arch/arm/mm/dma-mapping.c:
+ * Copyright (C) 2000-2004 Russell King
+ *
+ * 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, see .
+ */
+
+#define pr_fmt(fmt)"%s: " fmt, __func__
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+int iommu_dma_init(void)
+{
+   return iommu_iova_cache_init();
+}
+
+struct iommu_dma_domain {
+   struct iommu_domain *domain;
+   struct iova_domain *iovad;
+   struct kref kref;
+};
+
+/**
+ * iommu_dma_create_domain - Create a DMA mapping domain
+ * @ops: iommu_ops representing the IOMMU backing this domain. It is down to
+ *   the IOMMU driver whether a domain may span multiple IOMMU instances
+ * @base: IOVA at which the mappable address space starts
+ * @size: Size of IOVA space
+ *
+ * @base and @size should be exact multiples of IOMMU page granularity to
+ * avoid rounding surprises. If necessary, we reserve the page at address 0
+ * to ensure it is an invalid IOVA.
+ *
+ * Return: Pointer to a domain initialised with the given IOVA range,
+ * or NULL on failure. If successful, the caller holds an initial
+ * reference, which may be released with iommu_dma_release_domain()
+ * once a device is attached.
+ */
+struct iommu_dma_domain *iommu_dma_create_domain(const struct iommu_ops *ops,
+   dma_addr_t base, u64 size)
+{
+   struct iommu_dma_domain *dom;
+   struct iommu_domain *domain;
+   struct iova_domain *iovad;
+   unsigned long order, base_pfn, end_pfn;
+
+   dom = kzalloc(sizeof(*dom), GFP_KERNEL);
+   if (!dom)
+   return NULL;
+   /*
+* HACK(sort of): These domains currently belong to this layer and are
+* opaque from outside it, so they are "unmanaged" by the IOMMU API
+* itself. Once we have default domain support worked out, then we can
+* turn things inside out and put these inside managed IOMMU domains...
+*/
+   domain = ops->domain_alloc(IOMMU_DOMAIN_UNMANAGED);
+   if (!domain)
+   goto out_free_dma_domain;
+
+   domain->ops = ops;
+   domain->type = IOMMU_DOMAIN_UNMANAGED;
+
+   /* Use the smallest supported page size for IOVA granularity */
+   order = __ffs(ops->pgsize_bitmap);
+   base_pfn = max_t(unsigned long, 1, base >> order);
+   end_pfn = (base + size - 1) >> order;
+
+   /* Check the domain allows at least some access to the device... */
+   if (domain->geometry.force_aperture) {
+   if (base > domain->geometry.aperture_end ||
+   base + size <= domain->geometry.aperture_start) {
+   pr_warn("specified DM

[PATCH v2 1/4] iommu/iova: Avoid over-allocating when size-aligned

[Robin Murphy]

Currently, allocating a size-aligned IOVA region quietly adjusts the
actual allocation size in the process, returning a rounded-up
power-of-two-sized allocation. This results in mismatched behaviour in
the IOMMU driver if the original size was not a power of two, where the
original size is mapped, but the rounded-up IOVA size is unmapped.

Whilst some IOMMUs will happily unmap already-unmapped pages, others
consider this an error, so fix it by computing the necessary alignment
padding without altering the actual allocation size. Also clean up by
making pad_size unsigned, since negative padding makes no sense.

CC: David Woodhouse 
Signed-off-by: Robin Murphy 
---
 drivers/iommu/intel-iommu.c |  2 ++
 drivers/iommu/iova.c| 23 ++-
 2 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 68d43be..ada1fb9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2910,6 +2910,8 @@ static struct iova *intel_alloc_iova(struct device *dev,
 
/* Restrict dma_mask to the width that the iommu can handle */
dma_mask = min_t(uint64_t, DOMAIN_MAX_ADDR(domain->gaw), dma_mask);
+   /* Ensure we reserve the whole size-aligned region */
+   nrpages = __roundup_pow_of_two(nrpages);
 
if (!dmar_forcedac && dma_mask > DMA_BIT_MASK(32)) {
/*
diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c
index 9dd8208..2c4a51b 100644
--- a/drivers/iommu/iova.c
+++ b/drivers/iommu/iova.c
@@ -120,19 +120,14 @@ __cached_rbnode_delete_update(struct iova_domain *iovad, 
struct iova *free)
}
 }
 
-/* Computes the padding size required, to make the
- * the start address naturally aligned on its size
+/*
+ * Computes the padding size required, to make the start address
+ * naturally aligned on the power-of-two order of its size
  */
-static int
-iova_get_pad_size(int size, unsigned int limit_pfn)
+static unsigned int
+iova_get_pad_size(unsigned int size, unsigned int limit_pfn)
 {
-   unsigned int pad_size = 0;
-   unsigned int order = ilog2(size);
-
-   if (order)
-   pad_size = (limit_pfn + 1) % (1 << order);
-
-   return pad_size;
+   return (limit_pfn + 1 - size) & (__roundup_pow_of_two(size) - 1);
 }
 
 static int __alloc_and_insert_iova_range(struct iova_domain *iovad,
@@ -264,12 +259,6 @@ alloc_iova(struct iova_domain *iovad, unsigned long size,
if (!new_iova)
return NULL;
 
-   /* If size aligned is set then round the size to
-* to next power of two.
-*/
-   if (size_aligned)
-   size = __roundup_pow_of_two(size);
-
ret = __alloc_and_insert_iova_range(iovad, size, limit_pfn,
new_iova, size_aligned);
 
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 0/4] More cleanups and fixes for Intel VT-d

[David Woodhouse]

On Thu, 2015-06-11 at 15:47 +0200, Joerg Roedel wrote:
> 
> here are a number of additional fixes for the Intel VT-d
> driver and its behavior in a kdump kernel. The most
> important fix is the iommu=pt case, which is broken without
> these patches.

It looks like the original kdump patch set has basically now been
rewritten. Perhaps the best thing to do is for Zhenhua to resubmit an
updated and clean version rather than preserving that
churn?

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 0/4] arm64: IOMMU-backed DMA mapping

[Robin Murphy]

Hi all,

Here's a quick repost of [1] with a couple of minor fixes:
- fix scatterlist dma_len for segments with nonzero offset
- adjust the bus notifier priority with a less silly value

The branch at [2] has been updated as well.

Robin.

[1]:http://thread.gmane.org/gmane.linux.kernel.iommu/9721
[2]:git://linux-arm.org/linux-rm iommu/dma

Robin Murphy (4):
  iommu/iova: Avoid over-allocating when size-aligned
  iommu: Implement common IOMMU ops for DMA mapping
  arm64: Add IOMMU dma_ops
  arm64: Hook up IOMMU dma_ops

 arch/arm64/Kconfig   |   1 +
 arch/arm64/include/asm/device.h  |   3 +
 arch/arm64/include/asm/dma-mapping.h |  25 +-
 arch/arm64/mm/dma-mapping.c  | 357 ++
 drivers/iommu/Kconfig|   7 +
 drivers/iommu/Makefile   |   1 +
 drivers/iommu/dma-iommu.c| 560 +++
 drivers/iommu/intel-iommu.c  |   2 +
 drivers/iommu/iova.c |  23 +-
 include/linux/dma-iommu.h|  94 ++
 10 files changed, 1051 insertions(+), 22 deletions(-)
 create mode 100644 drivers/iommu/dma-iommu.c
 create mode 100644 include/linux/dma-iommu.h

-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH v2 4/4] arm64: Hook up IOMMU dma_ops

[Robin Murphy]

With iommu_dma_ops in place, hook them up to the configuration code, so
IOMMU-fronted devices will get them automatically.

Signed-off-by: Robin Murphy 
---
 arch/arm64/Kconfig   |  1 +
 arch/arm64/include/asm/dma-mapping.h | 11 ++-
 arch/arm64/mm/dma-mapping.c  | 15 +++
 3 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 7796af4..257cd7e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -70,6 +70,7 @@ config ARM64
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE
select HAVE_SYSCALL_TRACEPOINTS
+   select IOMMU_DMA if IOMMU_SUPPORT
select IRQ_DOMAIN
select MODULES_USE_ELF_RELA
select NO_BOOTMEM
diff --git a/arch/arm64/include/asm/dma-mapping.h 
b/arch/arm64/include/asm/dma-mapping.h
index 835a8d1..a87b507 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -45,11 +45,8 @@ static inline struct dma_map_ops *get_dma_ops(struct device 
*dev)
return __generic_dma_ops(dev);
 }
 
-static inline void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 
size,
- struct iommu_ops *iommu, bool coherent)
-{
-   dev->archdata.dma_coherent = coherent;
-}
+void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+   struct iommu_ops *iommu, bool coherent);
 #define arch_setup_dma_ops arch_setup_dma_ops
 
 /* do not use this function in a driver */
@@ -64,6 +61,10 @@ static inline bool is_device_dma_coherent(struct device *dev)
 #include 
 
 #ifdef CONFIG_IOMMU_DMA
+
+void arch_teardown_dma_ops(struct device *dev);
+#define arch_teardown_dma_ops  arch_teardown_dma_ops
+
 static inline struct iommu_dma_domain *arch_get_dma_domain(struct device *dev)
 {
return dev->archdata.dma_domain;
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index 51b7ae5..c1e1a07 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -769,6 +769,14 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 
dma_base, u64 size,
mutex_unlock(&iommu_dma_notifier_lock);
 }
 
+void arch_teardown_dma_ops(struct device *dev)
+{
+   if (dev->archdata.dma_domain) {
+   iommu_dma_detach_device(dev);
+   dev->archdata.dma_ops = NULL;
+   }
+}
+
 #else
 
 static void __iommu_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
@@ -776,3 +784,10 @@ static void __iommu_setup_dma_ops(struct device *dev, u64 
dma_base, u64 size,
 { }
 
 #endif  /* CONFIG_IOMMU_DMA */
+
+void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
+   struct iommu_ops *iommu, bool coherent)
+{
+   dev->archdata.dma_coherent = coherent;
+   __iommu_setup_dma_ops(dev, dma_base, size, iommu);
+}
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v10 0/10] iommu/vt-d: Fix intel vt-d faults in kdump kernel

[David Woodhouse]

On Fri, 2015-04-10 at 16:42 +0800, Li, Zhen-Hua wrote:
> This patchset is an update of Bill Sumner's patchset, implements a fix for:
> If a kernel boots with intel_iommu=on on a system that supports intel vt-d, 
> when a panic happens, the kdump kernel will boot with these faults:

But, in the general case, it *does* boot.

There are two cases where it doesn't actually boot, and those are the
interesting ones.

Firstly, a device just keeps generating faults and we die in an
interrupt storm, reporting the same fault over and over again. That can
actually happen without kdump/kexec and the correct fix for that is to
have rate-limiting, disable fault reporting for the offending device
after too many are seen, and then eventually to tie it in to the PCIe
error handling as has been discussed elsewhere.

Secondly, there are devices which do not correctly respond to a
hardware reset. This is broken hardware, and if we really have to copy
the old contexts from the crashed kernel to work around it then I'd
like it to be on a blacklist basis — we do it only for hardware which
is *known* to be broken in this way.

(There's also some cases where the device driver doesn't even *try* to
reset the hardware and just assumes it'll find it in a sane state as
the BIOS or a cleanly shut down kexec would have left it. In those
cases of course we can just fix the driver).

I don't much like the idea of doing this context copy for *all*
hardware. That's masking hardware issues with reset that we really
*ought* to be finding.

I believe that most of the offending hardware is HP's; they like to do
the most, erm, "interesting" things with odd hardware and RMRRs and
stuff. So Zhen-Hua would you be able to provide the list of broken
devices that HP has shipped, for the purpose of such a blacklist?

I assume you've already contacted the hardware folks responsible and
insisted that their devices are fixed to be resettable already, right?

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[David Woodhouse]

On Thu, 2015-06-11 at 15:44 +0100, David Woodhouse wrote:
> It used to look like this
> ...
> Now it looks like this

That's not quite right; each root entry is for only *one* bus, and the
correct version looks more like this...

It used to look like this:

Root Table Address Register
   |
   V

Root Table (struct root_entry) Context Table (struct context_entry)
-- 
0x00: CTX TBL for bus #0 ->  CTX entry for 00:00.0 (bits 63-0)
0x08: unused CTX entry for 00:00.0 (bits 127-64)
0x10: CTX TBL for bus #1 .   CTX entry for 00:00.1 (bits 63-0)
...   ...|   ...
0xff8:unused |   CTX entry for 00:1f.7 (bits 127-64)
 |
 |
 | Context Table (struct context_entry)
 | 
0x00:->  CTX entry for 01:00.0 (bits 63-0)
0x00:CTX entry for 01:00.0 (bits 127-64)
0x08:CTX entry for 01:00.1 (bits 63-0)
...  ...
0xff8:   CTX entry for 01:1f.7 (bits 127-64)


Now it looks like this, with each context entry taking twice the amount
of space:

Root Table Address Register
   |
   V

Root Table (struct root_entry) Context Table (struct context_entry)
-- 
0x00: CTX TBL LO for bus #0 -->  CTX entry for 00:00.0 (bits 63-0)
0x08: CTX TBL HI for bus #0 --.  CTX entry for 00:00.0 (bits 127-64)
0x10: CTX TBL LO for bus #1   |  CTX entry for 00:00.0 (bits 191-128)
...   ... |  ...
0xff8:CTX TBL HI for bus #FF  |  CTX entry for 00:0f.7 (bits 255-192)
  |
  |
  |Context Table (struct context_entry)
  |
0x00: >  CTX entry for 00:10.0 (bits 63-0)
0x00:CTX entry for 00:10.0 (bits 127-64)
0x08:CTX entry for 00:10.0 (bits 191-128)
...  ...
0xff8:   CTX entry for 00:1f.7 (bits 63-0)

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH v2 1/3] drm/exynos: fimd: ensure proper hw state in fimd_clear_channel()

[Inki Dae]

On 2015년 06월 03일 17:26, Marek Szyprowski wrote:
> One should not do any assumptions on the stare of the fimd hardware
> during driver initialization, so to properly reset fimd before enabling
> IOMMU, one should ensure that all power domains and clocks are really
> enabled. This patch adds pm_runtime and clocks management in the
> fimd_clear_channel() function to ensure that any access to fimd
> registers will be performed with clocks and power domains enabled.
> 
> Signed-off-by: Marek Szyprowski 
> Tested-by: Javier Martinez Canillas 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 22 ++
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c 
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 96618534358e..3ec9d4299a86 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -242,12 +242,21 @@ static void fimd_enable_shadow_channel_path(struct 
> fimd_context *ctx,
>   writel(val, ctx->regs + SHADOWCON);
>  }
>  
> +static int fimd_enable_vblank(struct exynos_drm_crtc *crtc);
> +static void fimd_disable_vblank(struct exynos_drm_crtc *crtc);

You can remove abvoe declarations. See the below comment.

> +
>  static void fimd_clear_channel(struct fimd_context *ctx)
>  {
>   unsigned int win, ch_enabled = 0;
>  
>   DRM_DEBUG_KMS("%s\n", __FILE__);
>  
> + /* Hardware is in unknown state, so ensure it gets enabled properly */
> + pm_runtime_get_sync(ctx->dev);
> +
> + clk_prepare_enable(ctx->bus_clk);
> + clk_prepare_enable(ctx->lcd_clk);
> +
>   /* Check if any channel is enabled. */
>   for (win = 0; win < WINDOWS_NR; win++) {
>   u32 val = readl(ctx->regs + WINCON(win));
> @@ -265,12 +274,17 @@ static void fimd_clear_channel(struct fimd_context *ctx)
>  
>   /* Wait for vsync, as disable channel takes effect at next vsync */
>   if (ch_enabled) {
> - unsigned int state = ctx->suspended;
> -
> - ctx->suspended = 0;
> + ctx->suspended = false;
> + fimd_enable_vblank(ctx->crtc);

I think you can call enable_vblank callback instead of
fimd_enable_vblank function because ctx object has exynos_crtc object.

i.e.,
struct exynos_drm_crtc_ops *ops = ctx->crtc->ops;
...
if (ops->enable_vblank)
ops->enable_vblank(ctx->crtc);
...

>   fimd_wait_for_vblank(ctx->crtc);
> - ctx->suspended = state;
> + fimd_disable_vblank(ctx->crtc);

Ditto.

Thanks,
Inki Dae

> + ctx->suspended = true;
>   }
> +
> + clk_disable_unprepare(ctx->lcd_clk);
> + clk_disable_unprepare(ctx->bus_clk);
> +
> + pm_runtime_put(ctx->dev);
>  }
>  
>  static int fimd_iommu_attach_devices(struct fimd_context *ctx,
> 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[Joerg Roedel]

On Thu, Jun 11, 2015 at 04:25:54PM +0200, Joerg Roedel wrote:
> Okay, reading the VT-d spec again, the extended context-entry table seem
> to exist in parallel to the current context-entry table, right? So this
> patch should still work, even with extended entries present.

I found section 3.4.4. of the Spec, and learned that the two tables do
not exist in parallel. The extended table ist just split into two parts
because the context entry was doubled in size.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[David Woodhouse]

On Thu, 2015-06-11 at 16:25 +0200, Joerg Roedel wrote:
> On Thu, Jun 11, 2015 at 03:07:02PM +0100, David Woodhouse wrote:
> > On Thu, 2015-06-11 at 15:47 +0200, Joerg Roedel wrote:
> > > From: Joerg Roedel 
> > > 
> > > Hide the copied context entries from the IOMMU driver by
> > > considering them as non-present. This is implemented by
> > > setting the first AVL bit (bit 67) in the context entry to
> > > one. If this bit is set, the context_present() function
> > > returns false.
> > > 
> > > Signed-off-by: Joerg Roedel 
> > 
> > In the extended context entry, bit 67 is the PGE bit. There are no bits
> > which are available to software, to my knowledge.
> 
> Okay, reading the VT-d spec again, the extended context-entry table seem
> to exist in parallel to the current context-entry table, right? So this
> patch should still work, even with extended entries present.

No, the extended context-entry exists *instead* of the legacy
context-entry. Note that all the bits in the legacy context-entry are
present in precisely the same place in the extended context-entry. It's
just that the extended context-entry defines meanings for more of them.

When you enable the DMA_RTADDR_RTT bit in the Root Table Address
register, the context-entries magically double in size.

It used to look like this:


Root Table Address Register
   |
   V

Root Table (struct root_entry) Context Table (struct context_entry)
-- 
0x00: Context-table pointer  ->  Context entry for 00:00.0
0x08: unused Context entry for 00:00.1 
0x10: unused Context entry for 00:00.2
...   ......
0xff8:...Context entry for ff:1f.7


Now it looks like this

Root Table Address Register
   |
   V

Root Table (struct root_entry) Context Table (struct context_entry)
-- 
0x00: Context-table ptr #1  ->  Context entry for 00:00.0: lo
0x08: Context-table ptr #2  --, Context entry for 00:00.0: hi
0x10: unused  | Context entry for 00:00.1: lo
...   ... | ...
0xff8:... | Context entry for 7f:1f.7: hi
  |
  |
  |Context Table (struct context_entry)
   --> 
0x00:   Context entry for 80:00.0: lo
0x08:   Context entry for 80:00.1: hi
... ...
0xff8:  Context entry for ff:1f.7: hi


This was implemented in http://git.kernel.org/linus/03ecc32c52 but
*all* that patch did was allocate the second page of context-table,
fill in the appropriate new pointer in the root table, and adjust the
way we calculate the *location* of a context-entry. In 4.1 we're still
only using the same old bits of the context-entry, which as noted are
in the same place in both cases. Even the mapping from the old 2-bit T
field to the new 3-bit TT field works out that way, for now.

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[Joerg Roedel]

On Thu, Jun 11, 2015 at 03:07:02PM +0100, David Woodhouse wrote:
> On Thu, 2015-06-11 at 15:47 +0200, Joerg Roedel wrote:
> > From: Joerg Roedel 
> > 
> > Hide the copied context entries from the IOMMU driver by
> > considering them as non-present. This is implemented by
> > setting the first AVL bit (bit 67) in the context entry to
> > one. If this bit is set, the context_present() function
> > returns false.
> > 
> > Signed-off-by: Joerg Roedel 
> 
> In the extended context entry, bit 67 is the PGE bit. There are no bits
> which are available to software, to my knowledge.

Okay, reading the VT-d spec again, the extended context-entry table seem
to exist in parallel to the current context-entry table, right? So this
patch should still work, even with extended entries present.

If that's true, then we only need to consider the kdump case while
setting up the extended context table entry (which means don't trust any
content that is already there).


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[David Woodhouse]

On Thu, 2015-06-11 at 16:12 +0200, Joerg Roedel wrote:
> On Thu, Jun 11, 2015 at 03:07:02PM +0100, David Woodhouse wrote:
> > On Thu, 2015-06-11 at 15:47 +0200, Joerg Roedel wrote:
> > > From: Joerg Roedel 
> > > 
> > > Hide the copied context entries from the IOMMU driver by
> > > considering them as non-present. This is implemented by
> > > setting the first AVL bit (bit 67) in the context entry to
> > > one. If this bit is set, the context_present() function
> > > returns false.
> > > 
> > > Signed-off-by: Joerg Roedel 
> > 
> > In the extended context entry, bit 67 is the PGE bit. There are no 
> > bits
> > which are available to software, to my knowledge.
> 
> Ah you are right, I was looking at the context-entry format without
> PASID. Then I have to solve this differently. Thanks for pointing 
> that out

While looking for spare bits, I was pondering the FPD (fault processing
disable) bit. Which we're likely to want to set when we are seeing
fault storms from misbehaving devices, although that hasn't been
implemented yet.

That code path is going to want to keep some per-device state over and
above what's in the context entry (and it's going to tie in with
generic PCI/device error handling too).

In the short term, perhaps it's not unreasonable to set the FPD bit on
'inherited' domains? That might be what you wanted anyway?

Later on, we'll want to be able to turn that on and off at runtime and
it won't purely serve as a marker for 'inherited' domains. But as I
said, at that point we'll *need* external state anyway. So I think
that's OK, perhaps?

-- 
dwmw2


smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[Joerg Roedel]

On Thu, Jun 11, 2015 at 03:07:02PM +0100, David Woodhouse wrote:
> On Thu, 2015-06-11 at 15:47 +0200, Joerg Roedel wrote:
> > From: Joerg Roedel 
> > 
> > Hide the copied context entries from the IOMMU driver by
> > considering them as non-present. This is implemented by
> > setting the first AVL bit (bit 67) in the context entry to
> > one. If this bit is set, the context_present() function
> > returns false.
> > 
> > Signed-off-by: Joerg Roedel 
> 
> In the extended context entry, bit 67 is the PGE bit. There are no bits
> which are available to software, to my knowledge.

Ah you are right, I was looking at the context-entry format without
PASID. Then I have to solve this differently. Thanks for pointing that
out.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[David Woodhouse]

On Thu, 2015-06-11 at 15:47 +0200, Joerg Roedel wrote:
> From: Joerg Roedel 
> 
> Hide the copied context entries from the IOMMU driver by
> considering them as non-present. This is implemented by
> setting the first AVL bit (bit 67) in the context entry to
> one. If this bit is set, the context_present() function
> returns false.
> 
> Signed-off-by: Joerg Roedel 

In the extended context entry, bit 67 is the PGE bit. There are no bits
which are available to software, to my knowledge.

-- 
David WoodhouseOpen Source Technology Centre
david.woodho...@intel.com  Intel Corporation


smime.p7s
Description: S/MIME cryptographic signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 2/4] iommu/vt-d: Don't consider copied context entries as present

[Joerg Roedel]

From: Joerg Roedel 

Hide the copied context entries from the IOMMU driver by
considering them as non-present. This is implemented by
setting the first AVL bit (bit 67) in the context entry to
one. If this bit is set, the context_present() function
returns false.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/intel-iommu.c | 24 ++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index b0fd5f2..f50d065 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -234,11 +234,21 @@ struct context_entry {
u64 hi;
 };
 
-static inline bool context_present(struct context_entry *context)
+static inline bool __context_present(struct context_entry *context)
 {
return (context->lo & 1);
 }
 
+static inline bool context_copied(struct context_entry *context)
+{
+   return (((context->hi >> 3) & 0xfULL) == 1);
+}
+
+static inline bool context_present(struct context_entry *context)
+{
+   return __context_present(context) && !context_copied(context);
+}
+
 static inline int context_fault_enable(struct context_entry *c)
 {
return((c->lo >> 1) & 0x1);
@@ -269,6 +279,11 @@ static inline void context_set_present(struct 
context_entry *context)
context->lo |= 1;
 }
 
+static inline void context_set_copied(struct context_entry *context)
+{
+   context->hi |= 1ULL << 3;
+}
+
 static inline void context_set_fault_enable(struct context_entry *context)
 {
context->lo &= (((u64)-1) << 2) | 1;
@@ -1919,6 +1934,9 @@ static int domain_context_mapping_one(struct dmar_domain 
*domain,
}
}
 
+   /* First clear any left-overs, like a copied context entry */
+   context_clear_entry(context);
+
context_set_domain_id(context, id);
 
if (translation != CONTEXT_TT_PASS_THROUGH) {
@@ -4911,13 +4929,15 @@ static int copy_one_context_table(struct intel_iommu 
*iommu,
memcpy_fromio(&ce, &ctxt_tbl_old[devfn],
  sizeof(struct context_entry));
 
-   if (!context_present(&ce))
+   if (!__context_present(&ce))
continue;
 
did = context_domain_id(&ce);
if (did >=0 && did < cap_ndoms(iommu->cap))
set_bit(did, iommu->domain_ids);
 
+   context_set_copied(&ce);
+
ctxt_tbl[devfn] = ce;
}
 
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 4/4] iommu/vt-d: Make sure si_domain is allocated for kdump kernel

[Joerg Roedel]

From: Joerg Roedel 

The si_domain needs to be allocated in the kdump kernel too.
Otherwise the iommu=pt case breaks with a panic of the kdump
kernel.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/intel-iommu.c | 23 +++
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index fcb9310..e5af94a 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2781,10 +2781,6 @@ static int __init 
iommu_prepare_static_identity_mapping(int hw)
int i;
int ret = 0;
 
-   ret = si_domain_init(hw);
-   if (ret)
-   return -EFAULT;
-
for_each_pci_dev(pdev) {
ret = dev_prepare_static_identity_mapping(&pdev->dev, hw);
if (ret)
@@ -2798,7 +2794,7 @@ static int __init 
iommu_prepare_static_identity_mapping(int hw)
 
if (dev->bus != &acpi_bus_type)
continue;
-   
+
adev= to_acpi_device(dev);
mutex_lock(&adev->physical_node_lock);
list_for_each_entry(pn, &adev->physical_node_list, 
node) {
@@ -2979,9 +2975,20 @@ static int __init init_dmars(void)
check_tylersburg_isoch();
 
/*
-* In the second kernel: Skip setting-up new domains for
-* si, rmrr, and the isa bus on the expectation that these
-* translations were copied from the old kernel.
+* Make sure the si_domain is allocated in the case we are in a
+* kdump kernel.
+*/
+   if (iommu_identity_mapping) {
+   ret = si_domain_init(hw_pass_through);
+   if (ret)
+   goto free_iommu;
+   }
+
+   /*
+* In the second kernel: Skip setting-up new domains for rmrr
+* and the isa bus, they will be allocated and assigned on the
+* first DMA-API call. This is to keep the old mappings alive
+* until the device driver of the kdump kernel takes over.
 */
if (g_translation_pre_enabled)
goto skip_new_domains_for_si_rmrr_isa;
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 1/4] iommu/vt-d: Remove iommu_attach_domain_with_id()

[Joerg Roedel]

From: Joerg Roedel 

We don't re-use domain-ids from the old kernel, so this
function is not needed anymore. Remove it.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/intel-iommu.c | 13 +
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 58abddb..b0fd5f2 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1648,16 +1648,6 @@ static int iommu_attach_domain(struct dmar_domain 
*domain,
return num;
 }
 
-static int iommu_attach_domain_with_id(struct dmar_domain *domain,
-  struct intel_iommu *iommu,
-  int domain_number)
-{
-   if (domain_number >= 0)
-   return domain_number;
-
-   return iommu_attach_domain(domain, iommu);
-}
-
 static int iommu_attach_vm_domain(struct dmar_domain *domain,
  struct intel_iommu *iommu)
 {
@@ -2326,7 +2316,6 @@ static struct dmar_domain *get_domain_for_dev(struct 
device *dev, int gaw)
u16 dma_alias;
unsigned long flags;
u8 bus, devfn;
-   int did = -1;   /* Default to "no domain_id supplied" */
 
domain = find_domain(dev);
if (domain)
@@ -2361,7 +2350,7 @@ static struct dmar_domain *get_domain_for_dev(struct 
device *dev, int gaw)
if (!domain)
return NULL;
 
-   domain->id = iommu_attach_domain_with_id(domain, iommu, did);
+   domain->id = iommu_attach_domain(domain, iommu);
if (domain->id < 0) {
free_domain_mem(domain);
return NULL;
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 3/4] iommu/vt-d: Remove unmap_device_dma()

[Joerg Roedel]

From: Joerg Roedel 

This removes the unmap_device_dma() function and the
code-path calling it. That code looks like a left-over from
Bill Sumners patch-set, as it only makes sense when kdump
recovery is implemented like in his original patch-set.

With the way it is implemented now this code doesn't make
sense at all and can be removed.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/intel-iommu.c | 44 
 1 file changed, 44 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f50d065..fcb9310 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -400,9 +400,6 @@ static LIST_HEAD(__iommu_remapped_mem);
  */
 static int intel_iommu_load_translation_tables(struct intel_iommu *iommu);
 
-static void unmap_device_dma(struct dmar_domain *domain,
-   struct device *dev,
-   struct intel_iommu *iommu);
 static void iommu_check_pre_te_status(struct intel_iommu *iommu);
 static u8 g_translation_pre_enabled;
 
@@ -3120,25 +3117,6 @@ static struct dmar_domain 
*__get_valid_domain_for_dev(struct device *dev)
return NULL;
}
 
-   /* if in kdump kernel, we need to unmap the mapped dma pages,
-* detach this device first.
-*/
-   if (likely(domain_context_mapped(dev))) {
-   iommu = domain_get_iommu(domain);
-   if (iommu->pre_enabled_trans) {
-   unmap_device_dma(domain, dev, iommu);
-
-   domain = get_domain_for_dev(dev,
-   DEFAULT_DOMAIN_ADDRESS_WIDTH);
-   if (!domain) {
-   pr_err("Allocating domain for %s failed",
-  dev_name(dev));
-   return NULL;
-   }
-   } else
-   return domain;
-   }
-
ret = domain_context_mapping(domain, dev, CONTEXT_TT_MULTI_LEVEL);
if (ret) {
pr_err("Domain context map for %s failed",
@@ -5063,28 +5041,6 @@ out_unmap:
return ret;
 }
 
-static void unmap_device_dma(struct dmar_domain *domain,
-   struct device *dev,
-   struct intel_iommu *iommu)
-{
-   struct context_entry *ce;
-   struct iova *iova;
-   phys_addr_t phys_addr;
-   dma_addr_t dev_addr;
-   struct pci_dev *pdev;
-
-   pdev = to_pci_dev(dev);
-   ce = iommu_context_addr(iommu, pdev->bus->number, pdev->devfn, 1);
-   phys_addr = context_address_root(ce) << VTD_PAGE_SHIFT;
-   dev_addr = phys_to_dma(dev, phys_addr);
-
-   iova = find_iova(&domain->iovad, IOVA_PFN(dev_addr));
-   if (iova)
-   intel_unmap(dev, dev_addr);
-
-   domain_remove_one_dev_info(domain, dev);
-}
-
 static void iommu_check_pre_te_status(struct intel_iommu *iommu)
 {
u32 sts;
-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH 0/4] More cleanups and fixes for Intel VT-d

[Joerg Roedel]

Hi,

here are a number of additional fixes for the Intel VT-d
driver and its behavior in a kdump kernel. The most
important fix is the iommu=pt case, which is broken without
these patches.

Regards,

Joerg

Joerg Roedel (4):
  iommu/vt-d: Remove iommu_attach_domain_with_id()
  iommu/vt-d: Don't consider copied context entries as present
  iommu/vt-d: Remove unmap_device_dma()
  iommu/vt-d: Make sure si_domain is allocated for kdump kernel

 drivers/iommu/intel-iommu.c | 104 
 1 file changed, 38 insertions(+), 66 deletions(-)

-- 
1.9.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: iommu/amd: Implement dm_region call-backs

[Joerg Roedel]

On Thu, Jun 11, 2015 at 11:02:51AM +0300, Dan Carpenter wrote:
> On Thu, Jun 11, 2015 at 09:43:27AM +0200, Joerg Roedel wrote:
> > What static checker do you use, btw?
> 
> I'm using Smatch.
> 
> The ERR_PTR stuff requires that you run
> 
>   smatch_scripts/build_kernel_data.sh
> 
> At the start and that takes a couple hours.

I see, thanks.

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: iommu/amd: Implement dm_region call-backs

[Dan Carpenter]

On Thu, Jun 11, 2015 at 09:43:27AM +0200, Joerg Roedel wrote:
> On Wed, Jun 10, 2015 at 06:39:20PM +0300, Dan Carpenter wrote:
> > Hello Joerg Roedel,
> > 
> > The patch b61238c4a5e1: "iommu/amd: Implement dm_region call-backs"
> > from May 28, 2015, leads to the following static checker warning:
> > 
> > drivers/iommu/amd_iommu.c:3153 amd_iommu_get_dm_regions()
> > error: potential null dereference 'region'.  (kzalloc returns null)
> 
> Thanks for the report, I added a check for the return value and folded
> it back into the original patch.
> 
> What static checker do you use, btw?

I'm using Smatch.

The ERR_PTR stuff requires that you run

smatch_scripts/build_kernel_data.sh

At the start and that takes a couple hours.

regards,
dan carpenter

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [patch -next] iommu: checking for NULL instead of IS_ERR

[Joerg Roedel]

On Wed, Jun 10, 2015 at 01:59:27PM +0300, Dan Carpenter wrote:
> The iommu_group_alloc() and iommu_group_get_for_dev() functions return
> error pointers, they never return NULL.
> 
> Signed-off-by: Dan Carpenter 

Applied, thanks.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: iommu/amd: Implement dm_region call-backs

[Joerg Roedel]

On Wed, Jun 10, 2015 at 06:39:20PM +0300, Dan Carpenter wrote:
> Hello Joerg Roedel,
> 
> The patch b61238c4a5e1: "iommu/amd: Implement dm_region call-backs"
> from May 28, 2015, leads to the following static checker warning:
> 
>   drivers/iommu/amd_iommu.c:3153 amd_iommu_get_dm_regions()
>   error: potential null dereference 'region'.  (kzalloc returns null)

Thanks for the report, I added a check for the return value and folded
it back into the original patch.

What static checker do you use, btw?


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[PATCH] iommu/amd: Handle errors returned from iommu_init_device

[Joerg Roedel]

Hi Dan,

On Wed, Jun 10, 2015 at 06:36:18PM +0300, Dan Carpenter wrote:
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 0a3da4517107: "iommu/amd: Put IOMMUv2 devices in a direct 
> mapped domain" from May 28, 2015, leads to the following Smatch 
> complaint:
> 
> drivers/iommu/amd_iommu.c:2285 amd_iommu_add_device()
>error: we previously assumed 'dev_data' could be null (see line 2279)
> 
> drivers/iommu/amd_iommu.c
>   2278dev_data = get_dev_data(dev);
>   2279if (dev_data && dev_data->iommu_v2)
> 
> Check.
> 
>   2280iommu_request_dm_for_dev(dev);
>   2281
>   2282/* Domains are initialized for this device - have a 
> look what we ended up with */
>   2283domain = iommu_get_domain_for_dev(dev);
>   2284if (domain->type == IOMMU_DOMAIN_IDENTITY) {
>   2285dev_data->passthrough = true;
> ^^
> Unchecked dereference.
> 
>   2286dev->archdata.dma_ops = &nommu_dma_ops;
>   2287} else {

This was a bit more complicated. I fixed it with this add-on patch:

>From 8a683d98936d7aa4e3ae554efca3a52c042593b9 Mon Sep 17 00:00:00 2001
From: Joerg Roedel 
Date: Thu, 11 Jun 2015 09:21:39 +0200
Subject: [PATCH] iommu/amd: Handle errors returned from iommu_init_device

Without this patch only -ENOTSUPP is handled, but there are
other possible errors. Handle them too.

Reported-by: Dan Carpenter 
Signed-off-by: Joerg Roedel 
---
 drivers/iommu/amd_iommu.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6659b51..73fc4b7 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2265,7 +2265,11 @@ static int amd_iommu_add_device(struct device *dev)
iommu = amd_iommu_rlookup_table[devid];
 
ret = iommu_init_device(dev);
-   if (ret == -ENOTSUPP) {
+   if (ret) {
+   if (ret != -ENOTSUPP)
+   pr_err("Failed to initialize device %s - trying to 
proceed anyway\n",
+   dev_name(dev));
+
iommu_ignore_device(dev);
dev->archdata.dma_ops = &nommu_dma_ops;
goto out;
@@ -2273,7 +2277,10 @@ static int amd_iommu_add_device(struct device *dev)
init_iommu_group(dev);
 
dev_data = get_dev_data(dev);
-   if (dev_data && dev_data->iommu_v2)
+
+   BUG_ON(!dev_data);
+
+   if (dev_data->iommu_v2)
iommu_request_dm_for_dev(dev);
 
/* Domains are initialized for this device - have a look what we ended 
up with */
-- 
1.8.4.5

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: iommu: Introduce iommu_request_dm_for_dev()

[Joerg Roedel]

Hi Dan,

On Wed, Jun 10, 2015 at 02:02:44PM +0300, Dan Carpenter wrote:
> This is a semi-automatic email about new static checker warnings.
> 
> The patch eeae3fba3afe: "iommu: Introduce iommu_request_dm_for_dev()" 
> from May 28, 2015, leads to the following Smatch complaint:
> 
> drivers/iommu/iommu.c:1581 iommu_request_dm_for_dev()
>error: we previously assumed 'group->default_domain' could be null 
> (see line 1558)

Thanks for the report, I folded a fix back into the patch introducing
this function.


Joerg

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu