[PATCH 0/16] Interrupt remapping support for AMD IOMMU

[PATCH 0/16] Interrupt remapping support for AMD IOMMU

[Joerg Roedel]

Hi,

here is a new version of the remaining patches to add interrupt
remapping support to the AMD IOMMU driver. The patches have been rebased
to v3.6-rc7 and tested again on various machines. As with the previous
version no issued were found with these patches too.

Regards,

	Joerg

Diffstat:

 drivers/iommu/Kconfig           |    2 +-
 drivers/iommu/amd_iommu.c       |  504 +++++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_init.c  |  237 +++++++++++++++++-
 drivers/iommu/amd_iommu_proto.h |    8 +
 drivers/iommu/amd_iommu_types.h |   53 ++++
 drivers/iommu/irq_remapping.c   |    5 +
 drivers/iommu/irq_remapping.h   |    6 +
 7 files changed, 801 insertions(+), 14 deletions(-)

[PATCH 01/16] iommu/amd: Keep track of HPET and IOAPIC device ids

[Joerg Roedel]

The IVRS ACPI table provides information about the IOAPICs
and the HPETs available in the system and which PCI device
ID they use in transactions. Save that information for later
usage in interrupt remapping.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c       |    3 ++
 drivers/iommu/amd_iommu_init.c  |   65 +++++++++++++++++++++++++++++++++++++--
 drivers/iommu/amd_iommu_types.h |   34 ++++++++++++++++++++
 3 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index b64502d..6f51483 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -72,6 +72,9 @@ static DEFINE_SPINLOCK(iommu_pd_list_lock);
 static LIST_HEAD(dev_data_list);
 static DEFINE_SPINLOCK(dev_data_list_lock);
 
+LIST_HEAD(ioapic_map);
+LIST_HEAD(hpet_map);
+
 /*
  * Domain for untranslated devices - only allocated
  * if iommu=pt passed on kernel cmd line.
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 18a89b7..a9f3948 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -55,6 +55,7 @@
 #define IVHD_DEV_ALIAS_RANGE            0x43
 #define IVHD_DEV_EXT_SELECT             0x46
 #define IVHD_DEV_EXT_SELECT_RANGE       0x47
+#define IVHD_DEV_SPECIAL		0x48
 
 #define IVHD_FLAG_HT_TUN_EN_MASK        0x01
 #define IVHD_FLAG_PASSPW_EN_MASK        0x02
@@ -690,6 +691,31 @@ static void __init set_dev_entry_from_acpi(struct amd_iommu *iommu,
 	set_iommu_for_device(iommu, devid);
 }
 
+static int add_special_device(u8 type, u8 id, u16 devid)
+{
+	struct devid_map *entry;
+	struct list_head *list;
+
+	if (type != 1 && type != 2)
+		return -EINVAL;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->id    = id;
+	entry->devid = devid;
+
+	if (type == 1)
+		list = &ioapic_map;
+	else
+		list = &hpet_map;
+
+	list_add_tail(&entry->list, list);
+
+	return 0;
+}
+
 /*
  * Reads the device exclusion range from ACPI and initialize IOMMU with
  * it
@@ -717,7 +743,7 @@ static void __init set_device_exclusion_range(u16 devid, struct ivmd_header *m)
  * Takes a pointer to an AMD IOMMU entry in the ACPI table and
  * initializes the hardware and our data structures with it.
  */
-static void __init init_iommu_from_acpi(struct amd_iommu *iommu,
+static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 					struct ivhd_header *h)
 {
 	u8 *p = (u8 *)h;
@@ -867,12 +893,43 @@ static void __init init_iommu_from_acpi(struct amd_iommu *iommu,
 							flags, ext_flags);
 			}
 			break;
+		case IVHD_DEV_SPECIAL: {
+			u8 handle, type;
+			const char *var;
+			u16 devid;
+			int ret;
+
+			handle = e->ext & 0xff;
+			devid  = (e->ext >>  8) & 0xffff;
+			type   = (e->ext >> 24) & 0xff;
+
+			if (type == 1)
+				var = "IOAPIC";
+			else if (type == 2)
+				var = "HPET";
+			else
+				var = "UNKNOWN";
+
+			DUMP_printk("  DEV_SPECIAL(%s[%d])\t\tdevid: %02x:%02x.%x\n",
+				    var, (int)handle,
+				    PCI_BUS(devid),
+				    PCI_SLOT(devid),
+				    PCI_FUNC(devid));
+
+			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
+			ret = add_special_device(type, handle, devid);
+			if (ret)
+				return ret;
+			break;
+		}
 		default:
 			break;
 		}
 
 		p += ivhd_entry_length(p);
 	}
+
+	return 0;
 }
 
 /* Initializes the device->iommu mapping for the driver */
@@ -912,6 +969,8 @@ static void __init free_iommu_all(void)
  */
 static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 {
+	int ret;
+
 	spin_lock_init(&iommu->lock);
 
 	/* Add IOMMU to internal data structures */
@@ -947,7 +1006,9 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 
 	iommu->int_enabled = false;
 
-	init_iommu_from_acpi(iommu, h);
+	ret = init_iommu_from_acpi(iommu, h);
+	if (ret)
+		return ret;
 	init_iommu_devices(iommu);
 
 	return 0;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index d0dab86..4495b78 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -565,6 +565,16 @@ struct amd_iommu {
 	u32 stored_l2[0x83];
 };
 
+struct devid_map {
+	struct list_head list;
+	u8 id;
+	u16 devid;
+};
+
+/* Map HPET and IOAPIC ids to the devid used by the IOMMU */
+extern struct list_head ioapic_map;
+extern struct list_head hpet_map;
+
 /*
  * List with all IOMMUs in the system. This list is not locked because it is
  * only written and read at driver initialization or suspend time
@@ -678,6 +688,30 @@ static inline u16 calc_devid(u8 bus, u8 devfn)
 	return (((u16)bus) << 8) | devfn;
 }
 
+static inline int get_ioapic_devid(int id)
+{
+	struct devid_map *entry;
+
+	list_for_each_entry(entry, &ioapic_map, list) {
+		if (entry->id == id)
+			return entry->devid;
+	}
+
+	return -EINVAL;
+}
+
+static inline int get_hpet_devid(int id)
+{
+	struct devid_map *entry;
+
+	list_for_each_entry(entry, &hpet_map, list) {
+		if (entry->id == id)
+			return entry->devid;
+	}
+
+	return -EINVAL;
+}
+
 #ifdef CONFIG_AMD_IOMMU_STATS
 
 struct __iommu_counter {
-- 
1.7.9.5

Re: [PATCH 01/16] iommu/amd: Keep track of HPET and IOAPIC device ids

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 02:23:51PM +0200, Joerg Roedel wrote:
> The IVRS ACPI table provides information about the IOAPICs
> and the HPETs available in the system and which PCI device
> ID they use in transactions. Save that information for later
> usage in interrupt remapping.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/iommu/amd_iommu.c       |    3 ++
>  drivers/iommu/amd_iommu_init.c  |   65 +++++++++++++++++++++++++++++++++++++--
>  drivers/iommu/amd_iommu_types.h |   34 ++++++++++++++++++++
>  3 files changed, 100 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index b64502d..6f51483 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -72,6 +72,9 @@ static DEFINE_SPINLOCK(iommu_pd_list_lock);
>  static LIST_HEAD(dev_data_list);
>  static DEFINE_SPINLOCK(dev_data_list_lock);
>  
> +LIST_HEAD(ioapic_map);
> +LIST_HEAD(hpet_map);
> +
>  /*
>   * Domain for untranslated devices - only allocated
>   * if iommu=pt passed on kernel cmd line.
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 18a89b7..a9f3948 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -55,6 +55,7 @@
>  #define IVHD_DEV_ALIAS_RANGE            0x43
>  #define IVHD_DEV_EXT_SELECT             0x46
>  #define IVHD_DEV_EXT_SELECT_RANGE       0x47
> +#define IVHD_DEV_SPECIAL		0x48
>  
>  #define IVHD_FLAG_HT_TUN_EN_MASK        0x01
>  #define IVHD_FLAG_PASSPW_EN_MASK        0x02
> @@ -690,6 +691,31 @@ static void __init set_dev_entry_from_acpi(struct amd_iommu *iommu,
>  	set_iommu_for_device(iommu, devid);
>  }
>  
> +static int add_special_device(u8 type, u8 id, u16 devid)
> +{
> +	struct devid_map *entry;
> +	struct list_head *list;
> +
> +	if (type != 1 && type != 2)
> +		return -EINVAL;

Should this be perhaps an enum? or #define?

> +
> +	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> +	if (!entry)
> +		return -ENOMEM;
> +
> +	entry->id    = id;
> +	entry->devid = devid;
> +
> +	if (type == 1)
> +		list = &ioapic_map;
> +	else
> +		list = &hpet_map;
> +
> +	list_add_tail(&entry->list, list);
> +
> +	return 0;
> +}
> +

Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

Re: [PATCH 01/16] iommu/amd: Keep track of HPET and IOAPIC device ids

[Joerg Roedel]

On Fri, Sep 28, 2012 at 10:08:34AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 02:23:51PM +0200, Joerg Roedel wrote:
> > +	if (type != 1 && type != 2)
> > +		return -EINVAL;
> 
> Should this be perhaps an enum? or #define?

Yup, that'll be better. I change it.

> Reviwed-by: Konrad Rzeszutek Wilk <konrad.wilk-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

Thanks!


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 02/16] iommu/amd: Add slab-cache for irq remapping tables

[Joerg Roedel]

The irq remapping tables for the AMD IOMMU need to be
aligned on a 128 byte boundary. Create a seperate slab-cache
to guarantee this alignment.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c       |    2 ++
 drivers/iommu/amd_iommu_init.c  |   23 +++++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h |    9 +++++++++
 drivers/iommu/irq_remapping.h   |    5 +++++
 4 files changed, 39 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6f51483..b6a8079 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -95,6 +95,8 @@ struct iommu_cmd {
 	u32 data[4];
 };
 
+struct kmem_cache *amd_iommu_irq_cache;
+
 static void update_domain(struct protection_domain *domain);
 static int __init alloc_passthrough_domain(void);
 
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index a9f3948..a046a0e 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -36,6 +36,7 @@
 
 #include "amd_iommu_proto.h"
 #include "amd_iommu_types.h"
+#include "irq_remapping.h"
 
 /*
  * definitions for the ACPI scanning code
@@ -124,6 +125,7 @@ struct ivmd_header {
 } __attribute__((packed));
 
 bool amd_iommu_dump;
+bool amd_iommu_irq_remap __read_mostly;
 
 static bool amd_iommu_detected;
 static bool __initdata amd_iommu_disabled;
@@ -1527,6 +1529,11 @@ static struct syscore_ops amd_iommu_syscore_ops = {
 
 static void __init free_on_init_error(void)
 {
+	if (amd_iommu_irq_cache) {
+		kmem_cache_destroy(amd_iommu_irq_cache);
+		amd_iommu_irq_cache = NULL;
+	}
+
 	amd_iommu_uninit_devices();
 
 	free_pages((unsigned long)amd_iommu_pd_alloc_bitmap,
@@ -1666,6 +1673,19 @@ static int __init early_amd_iommu_init(void)
 	if (ret)
 		goto out;
 
+	if (amd_iommu_irq_remap) {
+		/*
+		 * Interrupt remapping enabled, create kmem_cache for the
+		 * remapping tables.
+		 */
+		amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
+				MAX_IRQS_PER_TABLE * sizeof(u32),
+				IRQ_TABLE_ALIGNMENT,
+				0, NULL);
+		if (!amd_iommu_irq_cache)
+			goto out;
+	}
+
 	ret = init_memory_definitions(ivrs_base);
 	if (ret)
 		goto out;
@@ -1713,6 +1733,9 @@ static bool detect_ivrs(void)
 	/* Make sure ACS will be enabled during PCI probe */
 	pci_request_acs();
 
+	if (!disable_irq_remap)
+		amd_iommu_irq_remap = true;
+
 	return true;
 }
 
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 4495b78..953cea8 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -334,6 +334,15 @@ extern bool amd_iommu_np_cache;
 /* Only true if all IOMMUs support device IOTLBs */
 extern bool amd_iommu_iotlb_sup;
 
+#define MAX_IRQS_PER_TABLE	256
+#define IRQ_TABLE_ALIGNMENT	128
+
+/* Interrupt remapping feature used? */
+extern bool amd_iommu_irq_remap;
+
+/* kmem_cache to get tables with 128 byte alignement */
+extern struct kmem_cache *amd_iommu_irq_cache;
+
 /*
  * Make iterating over all IOMMUs easier
  */
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index b12974c..624d360 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -83,6 +83,11 @@ struct irq_remap_ops {
 
 extern struct irq_remap_ops intel_irq_remap_ops;
 
+#else  /* CONFIG_IRQ_REMAP */
+
+#define irq_remapping_enabled 0
+#define disable_irq_remap     1
+
 #endif /* CONFIG_IRQ_REMAP */
 
 #endif /* __IRQ_REMAPPING_H */
-- 
1.7.9.5

[PATCH 03/16] iommu/amd: Allocate data structures to keep track of irq remapping tables

[Joerg Roedel]

To easily map device ids to interrupt remapping table
entries a new lookup table is necessary.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_init.c  |   16 ++++++++++++++++
 drivers/iommu/amd_iommu_types.h |    9 +++++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index a046a0e..db100c5 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -181,6 +181,12 @@ u16 *amd_iommu_alias_table;
 struct amd_iommu **amd_iommu_rlookup_table;
 
 /*
+ * This table is used to find the irq remapping table for a given device id
+ * quickly.
+ */
+struct irq_remap_table **irq_lookup_table;
+
+/*
  * AMD IOMMU allows up to 2^16 differend protection domains. This is a bitmap
  * to know which ones are already in use.
  */
@@ -1529,9 +1535,13 @@ static struct syscore_ops amd_iommu_syscore_ops = {
 
 static void __init free_on_init_error(void)
 {
+	free_pages((unsigned long)irq_lookup_table,
+		   get_order(rlookup_table_size));
+
 	if (amd_iommu_irq_cache) {
 		kmem_cache_destroy(amd_iommu_irq_cache);
 		amd_iommu_irq_cache = NULL;
+
 	}
 
 	amd_iommu_uninit_devices();
@@ -1684,6 +1694,12 @@ static int __init early_amd_iommu_init(void)
 				0, NULL);
 		if (!amd_iommu_irq_cache)
 			goto out;
+
+		irq_lookup_table = (void *)__get_free_pages(
+				GFP_KERNEL | __GFP_ZERO,
+				get_order(rlookup_table_size));
+		if (!irq_lookup_table)
+			goto out;
 	}
 
 	ret = init_memory_definitions(ivrs_base);
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 953cea8..1a7d480 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -175,6 +175,7 @@
 #define DEV_ENTRY_EX            0x67
 #define DEV_ENTRY_SYSMGT1       0x68
 #define DEV_ENTRY_SYSMGT2       0x69
+#define DEV_ENTRY_IRQ_TBL_EN	0x80
 #define DEV_ENTRY_INIT_PASS     0xb8
 #define DEV_ENTRY_EINT_PASS     0xb9
 #define DEV_ENTRY_NMI_PASS      0xba
@@ -337,6 +338,14 @@ extern bool amd_iommu_iotlb_sup;
 #define MAX_IRQS_PER_TABLE	256
 #define IRQ_TABLE_ALIGNMENT	128
 
+struct irq_remap_table {
+	spinlock_t lock;
+	unsigned min_index;
+	u32 *table;
+};
+
+extern struct irq_remap_table **irq_lookup_table;
+
 /* Interrupt remapping feature used? */
 extern bool amd_iommu_irq_remap;
 
-- 
1.7.9.5

Re: [PATCH 03/16] iommu/amd: Allocate data structures to keep track of irq remapping tables

[Shuah Khan]

On Fri, Sep 28, 2012 at 6:23 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> To easily map device ids to interrupt remapping table
> entries a new lookup table is necessary.
>
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/iommu/amd_iommu_init.c  |   16 ++++++++++++++++
>  drivers/iommu/amd_iommu_types.h |    9 +++++++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index a046a0e..db100c5 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -181,6 +181,12 @@ u16 *amd_iommu_alias_table;
>  struct amd_iommu **amd_iommu_rlookup_table;
>
>  /*
> + * This table is used to find the irq remapping table for a given device id
> + * quickly.
> + */
> +struct irq_remap_table **irq_lookup_table;
> +
> +/*
>   * AMD IOMMU allows up to 2^16 differend protection

different

 domains. This is a bitmap
>   * to know which ones are already in use.
>   */
> @@ -1529,9 +1535,13 @@ static struct syscore_ops amd_iommu_syscore_ops = {
>
>  static void __init free_on_init_error(void)
>  {
> +       free_pages((unsigned long)irq_lookup_table,
> +                  get_order(rlookup_table_size));
> +
>         if (amd_iommu_irq_cache) {
>                 kmem_cache_destroy(amd_iommu_irq_cache);
>                 amd_iommu_irq_cache = NULL;
> +
>         }
>
>         amd_iommu_uninit_devices();
> @@ -1684,6 +1694,12 @@ static int __init early_amd_iommu_init(void)
>                                 0, NULL);
>                 if (!amd_iommu_irq_cache)
>                         goto out;
> +
> +               irq_lookup_table = (void *)__get_free_pages(
> +                               GFP_KERNEL | __GFP_ZERO,
> +                               get_order(rlookup_table_size));
> +               if (!irq_lookup_table)
> +                       goto out;
>         }
>
>         ret = init_memory_definitions(ivrs_base);
> diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> index 953cea8..1a7d480 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -175,6 +175,7 @@
>  #define DEV_ENTRY_EX            0x67
>  #define DEV_ENTRY_SYSMGT1       0x68
>  #define DEV_ENTRY_SYSMGT2       0x69
> +#define DEV_ENTRY_IRQ_TBL_EN   0x80
>  #define DEV_ENTRY_INIT_PASS     0xb8
>  #define DEV_ENTRY_EINT_PASS     0xb9
>  #define DEV_ENTRY_NMI_PASS      0xba
> @@ -337,6 +338,14 @@ extern bool amd_iommu_iotlb_sup;
>  #define MAX_IRQS_PER_TABLE     256
>  #define IRQ_TABLE_ALIGNMENT    128
>
> +struct irq_remap_table {
> +       spinlock_t lock;
> +       unsigned min_index;
> +       u32 *table;
> +};
> +
> +extern struct irq_remap_table **irq_lookup_table;
> +
>  /* Interrupt remapping feature used? */
>  extern bool amd_iommu_irq_remap;
>
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 04/16] iommu/amd: Check if IOAPIC information is correct

[Joerg Roedel]

When the IOAPIC information provided in the IVRS table is
not correct or not complete the system may not boot at all
when interrupt remapping is enabled. So check if this
information is correct and print out a firmware bug message
when it is not.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/Kconfig          |    2 +-
 drivers/iommu/amd_iommu_init.c |   27 ++++++++++++++++++++++++---
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index 9f69b56..e39f9db 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -42,7 +42,7 @@ config AMD_IOMMU
 	select PCI_PRI
 	select PCI_PASID
 	select IOMMU_API
-	depends on X86_64 && PCI && ACPI
+	depends on X86_64 && PCI && ACPI && X86_IO_APIC
 	---help---
 	  With this option you can enable support for AMD IOMMU hardware in
 	  your system. An IOMMU is a hardware component which provides
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index db100c5..3f0b68f 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -33,6 +33,7 @@
 #include <asm/gart.h>
 #include <asm/x86_init.h>
 #include <asm/iommu_table.h>
+#include <asm/io_apic.h>
 
 #include "amd_iommu_proto.h"
 #include "amd_iommu_types.h"
@@ -1572,6 +1573,23 @@ static void __init free_on_init_error(void)
 #endif
 }
 
+static bool __init check_ioapic_information(void)
+{
+	int idx;
+
+	for (idx = 0; idx < nr_ioapics; ++idx) {
+		int id = mpc_ioapic_id(idx);
+
+		if (get_ioapic_devid(id) < 0) {
+			pr_err(FW_BUG "AMD-Vi: IO-APIC[%d] not in IVRS table\n", id);
+			pr_err("AMD-Vi: Disabling interrupt remapping due to BIOS Bug\n");
+			return false;
+		}
+	}
+
+	return true;
+}
+
 /*
  * This is the hardware init function for AMD IOMMU in the system.
  * This function is called either from amd_iommu_init or from the interrupt
@@ -1658,9 +1676,6 @@ static int __init early_amd_iommu_init(void)
 	if (amd_iommu_pd_alloc_bitmap == NULL)
 		goto out;
 
-	/* init the device table */
-	init_device_table();
-
 	/*
 	 * let all alias entries point to itself
 	 */
@@ -1683,6 +1698,9 @@ static int __init early_amd_iommu_init(void)
 	if (ret)
 		goto out;
 
+	if (amd_iommu_irq_remap)
+		amd_iommu_irq_remap = check_ioapic_information();
+
 	if (amd_iommu_irq_remap) {
 		/*
 		 * Interrupt remapping enabled, create kmem_cache for the
@@ -1706,6 +1724,9 @@ static int __init early_amd_iommu_init(void)
 	if (ret)
 		goto out;
 
+	/* init the device table */
+	init_device_table();
+
 out:
 	/* Don't leak any ACPI memory */
 	early_acpi_os_unmap_memory((char __iomem *)ivrs_base, ivrs_size);
-- 
1.7.9.5

Re: [PATCH 04/16] iommu/amd: Check if IOAPIC information is correct

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 02:23:54PM +0200, Joerg Roedel wrote:
> When the IOAPIC information provided in the IVRS table is
> not correct or not complete the system may not boot at all
> when interrupt remapping is enabled. So check if this
> information is correct and print out a firmware bug message
> when it is not.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/iommu/Kconfig          |    2 +-
>  drivers/iommu/amd_iommu_init.c |   27 ++++++++++++++++++++++++---
>  2 files changed, 25 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 9f69b56..e39f9db 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -42,7 +42,7 @@ config AMD_IOMMU
>  	select PCI_PRI
>  	select PCI_PASID
>  	select IOMMU_API
> -	depends on X86_64 && PCI && ACPI
> +	depends on X86_64 && PCI && ACPI && X86_IO_APIC
>  	---help---
>  	  With this option you can enable support for AMD IOMMU hardware in
>  	  your system. An IOMMU is a hardware component which provides
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index db100c5..3f0b68f 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -33,6 +33,7 @@
>  #include <asm/gart.h>
>  #include <asm/x86_init.h>
>  #include <asm/iommu_table.h>
> +#include <asm/io_apic.h>
>  
>  #include "amd_iommu_proto.h"
>  #include "amd_iommu_types.h"
> @@ -1572,6 +1573,23 @@ static void __init free_on_init_error(void)
>  #endif
>  }
>  
> +static bool __init check_ioapic_information(void)
> +{
> +	int idx;
> +
> +	for (idx = 0; idx < nr_ioapics; ++idx) {

That ++idx looks odd - and it looks like the file has a combination of
i++ and ++i. Any particular reason for using this particular one instead
of the more common: "idx++" ? Or just habit?

Re: [PATCH 04/16] iommu/amd: Check if IOAPIC information is correct

[Joerg Roedel]

On Fri, Sep 28, 2012 at 10:16:45AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 02:23:54PM +0200, Joerg Roedel wrote:
> +static bool __init check_ioapic_information(void)
> > +{
> > +	int idx;
> > +
> > +	for (idx = 0; idx < nr_ioapics; ++idx) {
> 
> That ++idx looks odd - and it looks like the file has a combination of
> i++ and ++i. Any particular reason for using this particular one instead
> of the more common: "idx++" ? Or just habit?

Well, it's just a habbit from my time with C++ :)

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 05/16] iommu/amd: Split device table initialization into irq and dma part

[Joerg Roedel]

When the IOMMU is enabled very early (as with irq-remapping)
some devices are still in BIOS hand. When dma is blocked
early this can cause lots of IO_PAGE_FAULTs. So delay the
DMA initialization and do it right before the dma_ops are
initialized.
To be secure, block all interrupts by default when irq-remapping is
enabled in the system. They will be reenabled on demand
later.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_init.c |   19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 3f0b68f..8e9d71c 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1378,7 +1378,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)
  * Init the device table to not allow DMA access for devices and
  * suppress all page faults
  */
-static void init_device_table(void)
+static void init_device_table_dma(void)
 {
 	u32 devid;
 
@@ -1388,6 +1388,17 @@ static void init_device_table(void)
 	}
 }
 
+static void init_device_table(void)
+{
+	u32 devid;
+
+	if (!amd_iommu_irq_remap)
+		return;
+
+	for (devid = 0; devid <= amd_iommu_last_bdf; ++devid)
+		set_dev_entry_bit(devid, DEV_ENTRY_IRQ_TBL_EN);
+}
+
 static void iommu_init_flags(struct amd_iommu *iommu)
 {
 	iommu->acpi_flags & IVHD_FLAG_HT_TUN_EN_MASK ?
@@ -1778,8 +1789,14 @@ static bool detect_ivrs(void)
 
 static int amd_iommu_init_dma(void)
 {
+	struct amd_iommu *iommu;
 	int ret;
 
+	init_device_table_dma();
+
+	for_each_iommu(iommu)
+		iommu_flush_all_caches(iommu);
+
 	if (iommu_pass_through)
 		ret = amd_iommu_init_passthrough();
 	else
-- 
1.7.9.5

Re: [PATCH 05/16] iommu/amd: Split device table initialization into irq and dma part

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 02:23:55PM +0200, Joerg Roedel wrote:
> When the IOMMU is enabled very early (as with irq-remapping)
> some devices are still in BIOS hand. When dma is blocked
> early this can cause lots of IO_PAGE_FAULTs. So delay the
> DMA initialization and do it right before the dma_ops are
> initialized.
> To be secure, block all interrupts by default when irq-remapping is

What are you trying to be secure against?

> enabled in the system. They will be reenabled on demand
> later.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/iommu/amd_iommu_init.c |   19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 3f0b68f..8e9d71c 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1378,7 +1378,7 @@ static int __init init_memory_definitions(struct acpi_table_header *table)
>   * Init the device table to not allow DMA access for devices and
>   * suppress all page faults
>   */
> -static void init_device_table(void)
> +static void init_device_table_dma(void)
>  {
>  	u32 devid;
>  
> @@ -1388,6 +1388,17 @@ static void init_device_table(void)
>  	}
>  }
>  
> +static void init_device_table(void)
> +{
> +	u32 devid;
> +
> +	if (!amd_iommu_irq_remap)
> +		return;
> +
> +	for (devid = 0; devid <= amd_iommu_last_bdf; ++devid)
> +		set_dev_entry_bit(devid, DEV_ENTRY_IRQ_TBL_EN);
> +}
> +
>  static void iommu_init_flags(struct amd_iommu *iommu)
>  {
>  	iommu->acpi_flags & IVHD_FLAG_HT_TUN_EN_MASK ?
> @@ -1778,8 +1789,14 @@ static bool detect_ivrs(void)
>  
>  static int amd_iommu_init_dma(void)
>  {
> +	struct amd_iommu *iommu;
>  	int ret;
>  
> +	init_device_table_dma();
> +
> +	for_each_iommu(iommu)
> +		iommu_flush_all_caches(iommu);
> +
>  	if (iommu_pass_through)
>  		ret = amd_iommu_init_passthrough();
>  	else
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 05/16] iommu/amd: Split device table initialization into irq and dma part

[Joerg Roedel]

On Fri, Sep 28, 2012 at 10:17:53AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 02:23:55PM +0200, Joerg Roedel wrote:
> > When the IOMMU is enabled very early (as with irq-remapping)
> > some devices are still in BIOS hand. When dma is blocked
> > early this can cause lots of IO_PAGE_FAULTs. So delay the
> > DMA initialization and do it right before the dma_ops are
> > initialized.
> > To be secure, block all interrupts by default when irq-remapping is
> 
> What are you trying to be secure against?

Against attacks of faked MSI msgs that could DoS the system. MSI
messages are only specific DMA transactions in the end and a guest with
a device assigned has control over its DMA engine and can thus send
arbitrary interrupt requests to the host. There is a whole paper about
such attacks. I can't find right now, but I send you  a link when I find
it.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 05/16] iommu/amd: Split device table initialization into irq and dma part

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 04:25:55PM +0200, Joerg Roedel wrote:
> On Fri, Sep 28, 2012 at 10:17:53AM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Sep 28, 2012 at 02:23:55PM +0200, Joerg Roedel wrote:
> > > When the IOMMU is enabled very early (as with irq-remapping)
> > > some devices are still in BIOS hand. When dma is blocked
> > > early this can cause lots of IO_PAGE_FAULTs. So delay the
> > > DMA initialization and do it right before the dma_ops are
> > > initialized.
> > > To be secure, block all interrupts by default when irq-remapping is
> > 
> > What are you trying to be secure against?
> 
> Against attacks of faked MSI msgs that could DoS the system. MSI
> messages are only specific DMA transactions in the end and a guest with
> a device assigned has control over its DMA engine and can thus send
> arbitrary interrupt requests to the host. There is a whole paper about
> such attacks. I can't find right now, but I send you  a link when I find
> it.

I think I know which one you are talking about - that is the Joanna's
Rutkowski/Rafal Wojtczuk paper. Could you include a bit about it
in the description or at least the title in the git commit pls?

Re: [PATCH 05/16] iommu/amd: Split device table initialization into irq and dma part

[Joerg Roedel]

On Fri, Sep 28, 2012 at 11:00:10AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 04:25:55PM +0200, Joerg Roedel wrote:
> > On Fri, Sep 28, 2012 at 10:17:53AM -0400, Konrad Rzeszutek Wilk wrote:
> > > On Fri, Sep 28, 2012 at 02:23:55PM +0200, Joerg Roedel wrote:
> > > > When the IOMMU is enabled very early (as with irq-remapping)
> > > > some devices are still in BIOS hand. When dma is blocked
> > > > early this can cause lots of IO_PAGE_FAULTs. So delay the
> > > > DMA initialization and do it right before the dma_ops are
> > > > initialized.
> > > > To be secure, block all interrupts by default when irq-remapping is
> > > 
> > > What are you trying to be secure against?
> > 
> > Against attacks of faked MSI msgs that could DoS the system. MSI
> > messages are only specific DMA transactions in the end and a guest with
> > a device assigned has control over its DMA engine and can thus send
> > arbitrary interrupt requests to the host. There is a whole paper about
> > such attacks. I can't find right now, but I send you  a link when I find
> > it.
> 
> I think I know which one you are talking about - that is the Joanna's
> Rutkowski/Rafal Wojtczuk paper. Could you include a bit about it
> in the description or at least the title in the git commit pls?

Okay, I will do.

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 06/16] iommu/amd: Make sure IOMMU is not considered to translate itself

[Joerg Roedel]

The IVRS table usually includes the IOMMU device. But the
IOMMU does never translate itself, so make sure the IOMMU
driver knows this.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_init.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 8e9d71c..202b2b7 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1018,6 +1018,13 @@ static int __init init_iommu_one(struct amd_iommu *iommu, struct ivhd_header *h)
 	ret = init_iommu_from_acpi(iommu, h);
 	if (ret)
 		return ret;
+
+	/*
+	 * Make sure IOMMU is not considered to translate itself. The IVRS
+	 * table tells us so, but this is a lie!
+	 */
+	amd_iommu_rlookup_table[iommu->devid] = NULL;
+
 	init_iommu_devices(iommu);
 
 	return 0;
-- 
1.7.9.5

[PATCH 07/16] iommu/amd: Add IRTE invalidation routine

[Joerg Roedel]

Add routine to invalidate the IOMMU cache for interupt
translations. Also include the IRTE caches when flushing all
IOMMU caches.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c       |   27 +++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_types.h |    1 +
 2 files changed, 28 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index b6a8079..a2c1f7a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -902,6 +902,13 @@ static void build_inv_all(struct iommu_cmd *cmd)
 	CMD_SET_TYPE(cmd, CMD_INV_ALL);
 }
 
+static void build_inv_irt(struct iommu_cmd *cmd, u16 devid)
+{
+	memset(cmd, 0, sizeof(*cmd));
+	cmd->data[0] = devid;
+	CMD_SET_TYPE(cmd, CMD_INV_IRT);
+}
+
 /*
  * Writes the command to the IOMMUs command buffer and informs the
  * hardware about the new command.
@@ -1023,12 +1030,32 @@ static void iommu_flush_all(struct amd_iommu *iommu)
 	iommu_completion_wait(iommu);
 }
 
+static void iommu_flush_irt(struct amd_iommu *iommu, u16 devid)
+{
+	struct iommu_cmd cmd;
+
+	build_inv_irt(&cmd, devid);
+
+	iommu_queue_command(iommu, &cmd);
+}
+
+static void iommu_flush_irt_all(struct amd_iommu *iommu)
+{
+	u32 devid;
+
+	for (devid = 0; devid <= 0xffff; ++devid)
+		iommu_flush_irt(iommu, devid);
+
+	iommu_completion_wait(iommu);
+}
+
 void iommu_flush_all_caches(struct amd_iommu *iommu)
 {
 	if (iommu_feature(iommu, FEATURE_IA)) {
 		iommu_flush_all(iommu);
 	} else {
 		iommu_flush_dte_all(iommu);
+		iommu_flush_irt_all(iommu);
 		iommu_flush_tlb_all(iommu);
 	}
 }
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 1a7d480..b183306 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -152,6 +152,7 @@
 #define CMD_INV_DEV_ENTRY       0x02
 #define CMD_INV_IOMMU_PAGES	0x03
 #define CMD_INV_IOTLB_PAGES	0x04
+#define CMD_INV_IRT		0x05
 #define CMD_COMPLETE_PPR	0x07
 #define CMD_INV_ALL		0x08
 
-- 
1.7.9.5

Re: [PATCH 07/16] iommu/amd: Add IRTE invalidation routine

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 02:23:57PM +0200, Joerg Roedel wrote:
> Add routine to invalidate the IOMMU cache for interupt
> translations. Also include the IRTE caches when flushing all
> IOMMU caches.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/iommu/amd_iommu.c       |   27 +++++++++++++++++++++++++++
>  drivers/iommu/amd_iommu_types.h |    1 +
>  2 files changed, 28 insertions(+)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index b6a8079..a2c1f7a 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -902,6 +902,13 @@ static void build_inv_all(struct iommu_cmd *cmd)
>  	CMD_SET_TYPE(cmd, CMD_INV_ALL);
>  }
>  
> +static void build_inv_irt(struct iommu_cmd *cmd, u16 devid)
> +{
> +	memset(cmd, 0, sizeof(*cmd));
> +	cmd->data[0] = devid;
> +	CMD_SET_TYPE(cmd, CMD_INV_IRT);
> +}
> +
>  /*
>   * Writes the command to the IOMMUs command buffer and informs the
>   * hardware about the new command.
> @@ -1023,12 +1030,32 @@ static void iommu_flush_all(struct amd_iommu *iommu)
>  	iommu_completion_wait(iommu);
>  }
>  
> +static void iommu_flush_irt(struct amd_iommu *iommu, u16 devid)
> +{
> +	struct iommu_cmd cmd;
> +
> +	build_inv_irt(&cmd, devid);
> +
> +	iommu_queue_command(iommu, &cmd);
> +}
> +
> +static void iommu_flush_irt_all(struct amd_iommu *iommu)
> +{
> +	u32 devid;
> +
> +	for (devid = 0; devid <= 0xffff; ++devid)

Should this be #define? Or perhaps based of sizeof?

Is there a specific reason that it is u16 in the build_inv_irt but here it is u32?

> +		iommu_flush_irt(iommu, devid);
> +
> +	iommu_completion_wait(iommu);
> +}
> +
>  void iommu_flush_all_caches(struct amd_iommu *iommu)
>  {
>  	if (iommu_feature(iommu, FEATURE_IA)) {
>  		iommu_flush_all(iommu);
>  	} else {
>  		iommu_flush_dte_all(iommu);
> +		iommu_flush_irt_all(iommu);
>  		iommu_flush_tlb_all(iommu);
>  	}
>  }
> diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
> index 1a7d480..b183306 100644
> --- a/drivers/iommu/amd_iommu_types.h
> +++ b/drivers/iommu/amd_iommu_types.h
> @@ -152,6 +152,7 @@
>  #define CMD_INV_DEV_ENTRY       0x02
>  #define CMD_INV_IOMMU_PAGES	0x03
>  #define CMD_INV_IOTLB_PAGES	0x04
> +#define CMD_INV_IRT		0x05
>  #define CMD_COMPLETE_PPR	0x07
>  #define CMD_INV_ALL		0x08
>  
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 07/16] iommu/amd: Add IRTE invalidation routine

[Joerg Roedel]

On Fri, Sep 28, 2012 at 10:20:18AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 02:23:57PM +0200, Joerg Roedel wrote:
> > +static void iommu_flush_irt_all(struct amd_iommu *iommu)
> > +{
> > +	u32 devid;
> > +
> > +	for (devid = 0; devid <= 0xffff; ++devid)
> 
> Should this be #define? Or perhaps based of sizeof?

I'll add a define for that. The 0xffff is a hardware constant, so sizeof
will not work.

> Is there a specific reason that it is u16 in the build_inv_irt but
> here it is u32?

Yes, with u16 the loop-counter will overflow and make it to an endless
loop.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 08/16] iommu/amd: Add routines to manage irq remapping tables

[Joerg Roedel]

Add routines to:

* Alloc remapping tables and single entries from these
  tables
* Change entries in the tables
* Free entries in the table

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c |  228 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 228 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index a2c1f7a..bd277924 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -31,6 +31,12 @@
 #include <linux/amd-iommu.h>
 #include <linux/notifier.h>
 #include <linux/export.h>
+#include <linux/irq.h>
+#include <linux/msi.h>
+#include <asm/irq_remapping.h>
+#include <asm/io_apic.h>
+#include <asm/apic.h>
+#include <asm/hw_irq.h>
 #include <asm/msidef.h>
 #include <asm/proto.h>
 #include <asm/iommu.h>
@@ -3773,3 +3779,225 @@ int amd_iommu_device_info(struct pci_dev *pdev,
 	return 0;
 }
 EXPORT_SYMBOL(amd_iommu_device_info);
+
+#ifdef CONFIG_IRQ_REMAP
+
+/*****************************************************************************
+ *
+ * Interrupt Remapping Implementation
+ *
+ *****************************************************************************/
+
+union irte {
+	u32 val;
+	struct {
+		u32 valid	: 1,
+		    no_fault	: 1,
+		    int_type	: 3,
+		    rq_eoi	: 1,
+		    dm		: 1,
+		    rsvd_1	: 1,
+		    destination	: 8,
+		    vector	: 8,
+		    rsvd_2	: 8;
+	} fields;
+};
+
+#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
+#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
+#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
+#define DTE_IRQ_REMAP_ENABLE    1ULL
+
+static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
+{
+	u64 dte;
+
+	dte	= amd_iommu_dev_table[devid].data[2];
+	dte	&= ~DTE_IRQ_PHYS_ADDR_MASK;
+	dte	|= virt_to_phys(table->table);
+	dte	|= DTE_IRQ_REMAP_INTCTL;
+	dte	|= DTE_IRQ_TABLE_LEN;
+	dte	|= DTE_IRQ_REMAP_ENABLE;
+
+	amd_iommu_dev_table[devid].data[2] = dte;
+}
+
+#define IRTE_ALLOCATED (~1U)
+
+static struct irq_remap_table *get_irq_table(u16 devid, bool ioapic)
+{
+	struct irq_remap_table *table = NULL;
+	struct amd_iommu *iommu;
+	unsigned long flags;
+	u16 alias;
+
+	write_lock_irqsave(&amd_iommu_devtable_lock, flags);
+
+	iommu = amd_iommu_rlookup_table[devid];
+	if (!iommu)
+		goto out_unlock;
+
+	table = irq_lookup_table[devid];
+	if (table)
+		goto out;
+
+	alias = amd_iommu_alias_table[devid];
+	table = irq_lookup_table[alias];
+	if (table) {
+		irq_lookup_table[devid] = table;
+		set_dte_irq_entry(devid, table);
+		iommu_flush_dte(iommu, devid);
+		goto out;
+	}
+
+	/* Nothing there yet, allocate new irq remapping table */
+	table = kzalloc(sizeof(*table), GFP_ATOMIC);
+	if (!table)
+		goto out;
+
+	if (ioapic)
+		/* Keep the first 32 indexes free for IOAPIC interrupts */
+		table->min_index = 32;
+
+	table->table = kmem_cache_alloc(amd_iommu_irq_cache, GFP_ATOMIC);
+	if (!table->table) {
+		kfree(table);
+		goto out;
+	}
+
+	memset(table->table, 0, MAX_IRQS_PER_TABLE * sizeof(u32));
+
+	if (ioapic) {
+		int i;
+
+		for (i = 0; i < 32; ++i)
+			table->table[i] = IRTE_ALLOCATED;
+	}
+
+	irq_lookup_table[devid] = table;
+	set_dte_irq_entry(devid, table);
+	iommu_flush_dte(iommu, devid);
+	if (devid != alias) {
+		irq_lookup_table[alias] = table;
+		set_dte_irq_entry(devid, table);
+		iommu_flush_dte(iommu, alias);
+	}
+
+out:
+	iommu_completion_wait(iommu);
+
+out_unlock:
+	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
+
+	return table;
+}
+
+static int alloc_irq_index(struct irq_cfg *cfg, u16 devid, int count)
+{
+	struct irq_remap_table *table;
+	unsigned long flags;
+	int index, c;
+
+	table = get_irq_table(devid, false);
+	if (!table)
+		return -ENODEV;
+
+	spin_lock_irqsave(&table->lock, flags);
+
+	/* Scan table for free entries */
+	for (c = 0, index = table->min_index;
+	     index < MAX_IRQS_PER_TABLE;
+	     ++index) {
+		if (table->table[index] == 0)
+			c += 1;
+		else
+			c = 0;
+
+		if (c == count)	{
+			struct irq_2_iommu *irte_info;
+
+			for (; c != 0; --c)
+				table->table[index - c + 1] = IRTE_ALLOCATED;
+
+			index -= count - 1;
+
+			irte_info             = &cfg->irq_2_iommu;
+			irte_info->sub_handle = devid;
+			irte_info->irte_index = index;
+			irte_info->iommu      = (void *)cfg;
+
+			goto out;
+		}
+	}
+
+	index = -ENOSPC;
+
+out:
+	spin_unlock_irqrestore(&table->lock, flags);
+
+	return index;
+}
+
+static int get_irte(u16 devid, int index, union irte *irte)
+{
+	struct irq_remap_table *table;
+	unsigned long flags;
+
+	table = get_irq_table(devid, false);
+	if (!table)
+		return -ENOMEM;
+
+	spin_lock_irqsave(&table->lock, flags);
+	irte->val = table->table[index];
+	spin_unlock_irqrestore(&table->lock, flags);
+
+	return 0;
+}
+
+static int modify_irte(u16 devid, int index, union irte irte)
+{
+	struct irq_remap_table *table;
+	struct amd_iommu *iommu;
+	unsigned long flags;
+
+	iommu = amd_iommu_rlookup_table[devid];
+	if (iommu == NULL)
+		return -EINVAL;
+
+	table = get_irq_table(devid, false);
+	if (!table)
+		return -ENOMEM;
+
+	spin_lock_irqsave(&table->lock, flags);
+	table->table[index] = irte.val;
+	spin_unlock_irqrestore(&table->lock, flags);
+
+	iommu_flush_irt(iommu, devid);
+	iommu_completion_wait(iommu);
+
+	return 0;
+}
+
+static void free_irte(u16 devid, int index)
+{
+	struct irq_remap_table *table;
+	struct amd_iommu *iommu;
+	unsigned long flags;
+
+	iommu = amd_iommu_rlookup_table[devid];
+	if (iommu == NULL)
+		return;
+
+	table = get_irq_table(devid, false);
+	if (!table)
+		return;
+
+	spin_lock_irqsave(&table->lock, flags);
+	table->table[index] = 0;
+	spin_unlock_irqrestore(&table->lock, flags);
+
+	iommu_flush_irt(iommu, devid);
+	iommu_completion_wait(iommu);
+}
+
+#endif
-- 
1.7.9.5

Re: [PATCH 08/16] iommu/amd: Add routines to manage irq remapping tables

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 02:23:58PM +0200, Joerg Roedel wrote:
> Add routines to:
> 
> * Alloc remapping tables and single entries from these
>   tables
> * Change entries in the tables
> * Free entries in the table
> 
> Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/iommu/amd_iommu.c |  228 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 228 insertions(+)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index a2c1f7a..bd277924 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -31,6 +31,12 @@
>  #include <linux/amd-iommu.h>
>  #include <linux/notifier.h>
>  #include <linux/export.h>
> +#include <linux/irq.h>
> +#include <linux/msi.h>
> +#include <asm/irq_remapping.h>
> +#include <asm/io_apic.h>
> +#include <asm/apic.h>
> +#include <asm/hw_irq.h>
>  #include <asm/msidef.h>
>  #include <asm/proto.h>
>  #include <asm/iommu.h>
> @@ -3773,3 +3779,225 @@ int amd_iommu_device_info(struct pci_dev *pdev,
>  	return 0;
>  }
>  EXPORT_SYMBOL(amd_iommu_device_info);
> +
> +#ifdef CONFIG_IRQ_REMAP
> +
> +/*****************************************************************************
> + *
> + * Interrupt Remapping Implementation
> + *
> + *****************************************************************************/
> +
> +union irte {
> +	u32 val;
> +	struct {
> +		u32 valid	: 1,
> +		    no_fault	: 1,
> +		    int_type	: 3,
> +		    rq_eoi	: 1,
> +		    dm		: 1,
> +		    rsvd_1	: 1,
> +		    destination	: 8,
> +		    vector	: 8,
> +		    rsvd_2	: 8;
> +	} fields;
> +};
> +
> +#define DTE_IRQ_PHYS_ADDR_MASK	(((1ULL << 45)-1) << 6)
> +#define DTE_IRQ_REMAP_INTCTL    (2ULL << 60)
> +#define DTE_IRQ_TABLE_LEN       (8ULL << 1)
> +#define DTE_IRQ_REMAP_ENABLE    1ULL
> +
> +static void set_dte_irq_entry(u16 devid, struct irq_remap_table *table)
> +{
> +	u64 dte;
> +
> +	dte	= amd_iommu_dev_table[devid].data[2];
> +	dte	&= ~DTE_IRQ_PHYS_ADDR_MASK;
> +	dte	|= virt_to_phys(table->table);
> +	dte	|= DTE_IRQ_REMAP_INTCTL;
> +	dte	|= DTE_IRQ_TABLE_LEN;
> +	dte	|= DTE_IRQ_REMAP_ENABLE;
> +
> +	amd_iommu_dev_table[devid].data[2] = dte;
> +}
> +
> +#define IRTE_ALLOCATED (~1U)
> +
> +static struct irq_remap_table *get_irq_table(u16 devid, bool ioapic)
> +{
> +	struct irq_remap_table *table = NULL;
> +	struct amd_iommu *iommu;
> +	unsigned long flags;
> +	u16 alias;
> +
> +	write_lock_irqsave(&amd_iommu_devtable_lock, flags);
> +
> +	iommu = amd_iommu_rlookup_table[devid];
> +	if (!iommu)
> +		goto out_unlock;
> +
> +	table = irq_lookup_table[devid];
> +	if (table)
> +		goto out;
> +
> +	alias = amd_iommu_alias_table[devid];
> +	table = irq_lookup_table[alias];
> +	if (table) {
> +		irq_lookup_table[devid] = table;
> +		set_dte_irq_entry(devid, table);
> +		iommu_flush_dte(iommu, devid);
> +		goto out;
> +	}
> +
> +	/* Nothing there yet, allocate new irq remapping table */
> +	table = kzalloc(sizeof(*table), GFP_ATOMIC);
> +	if (!table)
> +		goto out;
> +
> +	if (ioapic)
> +		/* Keep the first 32 indexes free for IOAPIC interrupts */
> +		table->min_index = 32;
> +
> +	table->table = kmem_cache_alloc(amd_iommu_irq_cache, GFP_ATOMIC);
> +	if (!table->table) {
> +		kfree(table);
> +		goto out;
> +	}
> +
> +	memset(table->table, 0, MAX_IRQS_PER_TABLE * sizeof(u32));
> +
> +	if (ioapic) {
> +		int i;
> +
> +		for (i = 0; i < 32; ++i)
> +			table->table[i] = IRTE_ALLOCATED;
> +	}
> +
> +	irq_lookup_table[devid] = table;
> +	set_dte_irq_entry(devid, table);
> +	iommu_flush_dte(iommu, devid);
> +	if (devid != alias) {
> +		irq_lookup_table[alias] = table;
> +		set_dte_irq_entry(devid, table);
> +		iommu_flush_dte(iommu, alias);
> +	}
> +
> +out:
> +	iommu_completion_wait(iommu);
> +
> +out_unlock:
> +	write_unlock_irqrestore(&amd_iommu_devtable_lock, flags);
> +
> +	return table;
> +}
> +
> +static int alloc_irq_index(struct irq_cfg *cfg, u16 devid, int count)
> +{
> +	struct irq_remap_table *table;
> +	unsigned long flags;
> +	int index, c;
> +
> +	table = get_irq_table(devid, false);
> +	if (!table)
> +		return -ENODEV;
> +
> +	spin_lock_irqsave(&table->lock, flags);
> +
> +	/* Scan table for free entries */
> +	for (c = 0, index = table->min_index;
> +	     index < MAX_IRQS_PER_TABLE;
> +	     ++index) {
> +		if (table->table[index] == 0)
> +			c += 1;
> +		else
> +			c = 0;
> +
> +		if (c == count)	{
> +			struct irq_2_iommu *irte_info;
> +
> +			for (; c != 0; --c)
> +				table->table[index - c + 1] = IRTE_ALLOCATED;
> +
> +			index -= count - 1;
> +
> +			irte_info             = &cfg->irq_2_iommu;
> +			irte_info->sub_handle = devid;
> +			irte_info->irte_index = index;

Should the irte_info also contain the count information to know how
whether the table->table[index] (which would be IRTE_ALLOCATED at
this point) is owned by correct irte_info ?

> +			irte_info->iommu      = (void *)cfg;
> +
> +			goto out;
> +		}
> +	}
> +
> +	index = -ENOSPC;
> +
> +out:
> +	spin_unlock_irqrestore(&table->lock, flags);
> +
> +	return index;
> +}
> +
> +static int get_irte(u16 devid, int index, union irte *irte)
> +{
> +	struct irq_remap_table *table;
> +	unsigned long flags;
> +
> +	table = get_irq_table(devid, false);
> +	if (!table)
> +		return -ENOMEM;
> +
> +	spin_lock_irqsave(&table->lock, flags);
> +	irte->val = table->table[index];
> +	spin_unlock_irqrestore(&table->lock, flags);
> +
> +	return 0;
> +}
> +
> +static int modify_irte(u16 devid, int index, union irte irte)
> +{
> +	struct irq_remap_table *table;
> +	struct amd_iommu *iommu;
> +	unsigned long flags;
> +
> +	iommu = amd_iommu_rlookup_table[devid];
> +	if (iommu == NULL)
> +		return -EINVAL;
> +
> +	table = get_irq_table(devid, false);
> +	if (!table)
> +		return -ENOMEM;
> +
> +	spin_lock_irqsave(&table->lock, flags);
> +	table->table[index] = irte.val;
> +	spin_unlock_irqrestore(&table->lock, flags);
> +
> +	iommu_flush_irt(iommu, devid);
> +	iommu_completion_wait(iommu);
> +
> +	return 0;
> +}
> +
> +static void free_irte(u16 devid, int index)
> +{
> +	struct irq_remap_table *table;
> +	struct amd_iommu *iommu;
> +	unsigned long flags;
> +
> +	iommu = amd_iommu_rlookup_table[devid];
> +	if (iommu == NULL)
> +		return;
> +
> +	table = get_irq_table(devid, false);
> +	if (!table)
> +		return;
> +
> +	spin_lock_irqsave(&table->lock, flags);
> +	table->table[index] = 0;
> +	spin_unlock_irqrestore(&table->lock, flags);
> +
> +	iommu_flush_irt(iommu, devid);
> +	iommu_completion_wait(iommu);
> +}
> +
> +#endif
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 08/16] iommu/amd: Add routines to manage irq remapping tables

[Joerg Roedel]

On Fri, Sep 28, 2012 at 10:40:13AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 02:23:58PM +0200, Joerg Roedel wrote:
> > +			irte_info             = &cfg->irq_2_iommu;
> > +			irte_info->sub_handle = devid;
> > +			irte_info->irte_index = index;
> 
> Should the irte_info also contain the count information to know how
> whether the table->table[index] (which would be IRTE_ALLOCATED at
> this point) is owned by correct irte_info ?

No, because the irte_info used here is only for the first index. For the
following indexes up to index+count-1 other irte_info structs are used.
This is necessary because of the way MSI-X interrupts are set up with
interrupt remapping.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 09/16] iommu/amd: Add IOAPIC remapping routines

[Joerg Roedel]

Add the routine to setup interrupt remapping for ioapic
interrupts. Also add a routine to change the affinity of an
irq and to free an irq allocation for interrupt remapping.
The last two functions will also be used for MSI interrupts.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c |  126 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index bd277924..3df01b2 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -4000,4 +4000,130 @@ static void free_irte(u16 devid, int index)
 	iommu_completion_wait(iommu);
 }
 
+static int setup_ioapic_entry(int irq, struct IO_APIC_route_entry *entry,
+			      unsigned int destination, int vector,
+			      struct io_apic_irq_attr *attr)
+{
+	struct irq_remap_table *table;
+	struct irq_2_iommu *irte_info;
+	struct irq_cfg *cfg;
+	union irte irte;
+	int ioapic_id;
+	int index;
+	int devid;
+	int ret;
+
+	cfg = irq_get_chip_data(irq);
+	if (!cfg)
+		return -EINVAL;
+
+	irte_info = &cfg->irq_2_iommu;
+	ioapic_id = mpc_ioapic_id(attr->ioapic);
+	devid     = get_ioapic_devid(ioapic_id);
+
+	if (devid < 0)
+		return devid;
+
+	table = get_irq_table(devid, true);
+	if (table == NULL)
+		return -ENOMEM;
+
+	index = attr->ioapic_pin;
+
+	/* Setup IRQ remapping info */
+	irte_info->sub_handle = devid;
+	irte_info->irte_index = index;
+	irte_info->iommu      = (void *)cfg;
+
+	/* Setup IRTE for IOMMU */
+	irte.val		= 0;
+	irte.fields.vector      = vector;
+	irte.fields.int_type    = apic->irq_delivery_mode;
+	irte.fields.destination = destination;
+	irte.fields.dm          = apic->irq_dest_mode;
+	irte.fields.valid       = 1;
+
+	ret = modify_irte(devid, index, irte);
+	if (ret)
+		return ret;
+
+	/* Setup IOAPIC entry */
+	memset(entry, 0, sizeof(*entry));
+
+	entry->vector        = index;
+	entry->mask          = 0;
+	entry->trigger       = attr->trigger;
+	entry->polarity      = attr->polarity;
+
+	/*
+	 * Mask level triggered irqs.
+	 * Use IRQ_DELAYED_DISABLE for edge triggered irqs.
+	 */
+	if (attr->trigger)
+		entry->mask = 1;
+
+	return 0;
+}
+
+static int set_affinity(struct irq_data *data, const struct cpumask *mask,
+			bool force)
+{
+	struct irq_2_iommu *irte_info;
+	unsigned int dest, irq;
+	struct irq_cfg *cfg;
+	union irte irte;
+	int err;
+
+	if (!config_enabled(CONFIG_SMP))
+		return -1;
+
+	cfg       = data->chip_data;
+	irq       = data->irq;
+	irte_info = &cfg->irq_2_iommu;
+
+	if (!cpumask_intersects(mask, cpu_online_mask))
+		return -EINVAL;
+
+	if (get_irte(irte_info->sub_handle, irte_info->irte_index, &irte))
+		return -EBUSY;
+
+	if (assign_irq_vector(irq, cfg, mask))
+		return -EBUSY;
+
+	err = apic->cpu_mask_to_apicid_and(cfg->domain, mask, &dest);
+	if (err) {
+		if (assign_irq_vector(irq, cfg, data->affinity))
+			pr_err("AMD-Vi: Failed to recover vector for irq %d\n", irq);
+		return err;
+	}
+
+	irte.fields.vector      = cfg->vector;
+	irte.fields.destination = dest;
+
+	modify_irte(irte_info->sub_handle, irte_info->irte_index, irte);
+
+	if (cfg->move_in_progress)
+		send_cleanup_vector(cfg);
+
+	cpumask_copy(data->affinity, mask);
+
+	return 0;
+}
+
+static int free_irq(int irq)
+{
+	struct irq_2_iommu *irte_info;
+	struct irq_cfg *cfg;
+
+	cfg = irq_get_chip_data(irq);
+	if (!cfg)
+		return -EINVAL;
+
+	irte_info = &cfg->irq_2_iommu;
+
+	free_irte(irte_info->sub_handle, irte_info->irte_index);
+
+	return 0;
+}
+
 #endif
-- 
1.7.9.5

Re: [PATCH 09/16] iommu/amd: Add IOAPIC remapping routines

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 02:23:59PM +0200, Joerg Roedel wrote:
> Add the routine to setup interrupt remapping for ioapic
> interrupts. Also add a routine to change the affinity of an
> irq and to free an irq allocation for interrupt remapping.
> The last two functions will also be used for MSI interrupts.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/iommu/amd_iommu.c |  126 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 126 insertions(+)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index bd277924..3df01b2 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -4000,4 +4000,130 @@ static void free_irte(u16 devid, int index)
>  	iommu_completion_wait(iommu);
>  }
>  
> +static int setup_ioapic_entry(int irq, struct IO_APIC_route_entry *entry,
> +			      unsigned int destination, int vector,
> +			      struct io_apic_irq_attr *attr)
> +{
> +	struct irq_remap_table *table;
> +	struct irq_2_iommu *irte_info;
> +	struct irq_cfg *cfg;
> +	union irte irte;
> +	int ioapic_id;
> +	int index;
> +	int devid;
> +	int ret;
> +
> +	cfg = irq_get_chip_data(irq);
> +	if (!cfg)
> +		return -EINVAL;
> +
> +	irte_info = &cfg->irq_2_iommu;
> +	ioapic_id = mpc_ioapic_id(attr->ioapic);
> +	devid     = get_ioapic_devid(ioapic_id);
> +
> +	if (devid < 0)
> +		return devid;
> +
> +	table = get_irq_table(devid, true);
> +	if (table == NULL)
> +		return -ENOMEM;
> +
> +	index = attr->ioapic_pin;
> +
> +	/* Setup IRQ remapping info */
> +	irte_info->sub_handle = devid;
> +	irte_info->irte_index = index;
> +	irte_info->iommu      = (void *)cfg;
> +
> +	/* Setup IRTE for IOMMU */
> +	irte.val		= 0;
> +	irte.fields.vector      = vector;
> +	irte.fields.int_type    = apic->irq_delivery_mode;
> +	irte.fields.destination = destination;
> +	irte.fields.dm          = apic->irq_dest_mode;
> +	irte.fields.valid       = 1;
> +
> +	ret = modify_irte(devid, index, irte);
> +	if (ret)
> +		return ret;
> +
> +	/* Setup IOAPIC entry */
> +	memset(entry, 0, sizeof(*entry));
> +
> +	entry->vector        = index;
> +	entry->mask          = 0;
> +	entry->trigger       = attr->trigger;
> +	entry->polarity      = attr->polarity;
> +
> +	/*
> +	 * Mask level triggered irqs.
> +	 * Use IRQ_DELAYED_DISABLE for edge triggered irqs.

so how come it is not set?

> +	 */
> +	if (attr->trigger)
> +		entry->mask = 1;
> +
> +	return 0;
> +}
> +
> +static int set_affinity(struct irq_data *data, const struct cpumask *mask,
> +			bool force)
> +{
> +	struct irq_2_iommu *irte_info;
> +	unsigned int dest, irq;
> +	struct irq_cfg *cfg;
> +	union irte irte;
> +	int err;
> +
> +	if (!config_enabled(CONFIG_SMP))
> +		return -1;

-1? -ENOx something?
> +
> +	cfg       = data->chip_data;
> +	irq       = data->irq;
> +	irte_info = &cfg->irq_2_iommu;
> +
> +	if (!cpumask_intersects(mask, cpu_online_mask))
> +		return -EINVAL;
> +
> +	if (get_irte(irte_info->sub_handle, irte_info->irte_index, &irte))
> +		return -EBUSY;
> +
> +	if (assign_irq_vector(irq, cfg, mask))
> +		return -EBUSY;
> +
> +	err = apic->cpu_mask_to_apicid_and(cfg->domain, mask, &dest);
> +	if (err) {
> +		if (assign_irq_vector(irq, cfg, data->affinity))
> +			pr_err("AMD-Vi: Failed to recover vector for irq %d\n", irq);

If we do OK with the assignment of the vector, should we just continue
on instead of returning error?

> +		return err;
> +	}
> +
> +	irte.fields.vector      = cfg->vector;
> +	irte.fields.destination = dest;
> +
> +	modify_irte(irte_info->sub_handle, irte_info->irte_index, irte);
> +
> +	if (cfg->move_in_progress)
> +		send_cleanup_vector(cfg);
> +
> +	cpumask_copy(data->affinity, mask);
> +
> +	return 0;
> +}
> +
> +static int free_irq(int irq)
> +{
> +	struct irq_2_iommu *irte_info;
> +	struct irq_cfg *cfg;
> +
> +	cfg = irq_get_chip_data(irq);
> +	if (!cfg)
> +		return -EINVAL;
> +
> +	irte_info = &cfg->irq_2_iommu;
> +
> +	free_irte(irte_info->sub_handle, irte_info->irte_index);
> +
> +	return 0;
> +}
> +
>  #endif
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 09/16] iommu/amd: Add IOAPIC remapping routines

[Joerg Roedel]

On Fri, Sep 28, 2012 at 10:45:15AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 02:23:59PM +0200, Joerg Roedel wrote:
> > +	/*
> > +	 * Mask level triggered irqs.
> > +	 * Use IRQ_DELAYED_DISABLE for edge triggered irqs.
> 
> so how come it is not set?

Don't understand this. What is not set?

> > +	if (!config_enabled(CONFIG_SMP))
> > +		return -1;
> 
> -1? -ENOx something?

No, -1 for compatibiblity with the other implementation of this.

> > +	err = apic->cpu_mask_to_apicid_and(cfg->domain, mask, &dest);
> > +	if (err) {
> > +		if (assign_irq_vector(irq, cfg, data->affinity))
> > +			pr_err("AMD-Vi: Failed to recover vector for irq %d\n", irq);
> 
> If we do OK with the assignment of the vector, should we just continue
> on instead of returning error?

The purpose of this is to recover to the old affinity if we failed at
this point. Even in this case, setting the new affinity still failed and
we need to inform the caller about this. Thus returning the error.
Also note the pr_err message which also tells you about the
recovery-case :)


	Joerg


-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 09/16] iommu/amd: Add IOAPIC remapping routines

[Konrad Rzeszutek Wilk]

On Mon, Oct 01, 2012 at 10:40:51AM +0200, Joerg Roedel wrote:
> On Fri, Sep 28, 2012 at 10:45:15AM -0400, Konrad Rzeszutek Wilk wrote:
> > On Fri, Sep 28, 2012 at 02:23:59PM +0200, Joerg Roedel wrote:
> > > +	/*
> > > +	 * Mask level triggered irqs.
> > > +	 * Use IRQ_DELAYED_DISABLE for edge triggered irqs.
> > 
> > so how come it is not set?
> 
> Don't understand this. What is not set?

the IRQ_DELAYED_DISABLE. But I could not even find that enum anymore?
Has that been obsoleted?
> 
> > > +	if (!config_enabled(CONFIG_SMP))
> > > +		return -1;
> > 
> > -1? -ENOx something?
> 
> No, -1 for compatibiblity with the other implementation of this.

OK.
> 
> > > +	err = apic->cpu_mask_to_apicid_and(cfg->domain, mask, &dest);
> > > +	if (err) {
> > > +		if (assign_irq_vector(irq, cfg, data->affinity))
> > > +			pr_err("AMD-Vi: Failed to recover vector for irq %d\n", irq);
> > 
> > If we do OK with the assignment of the vector, should we just continue
> > on instead of returning error?
> 
> The purpose of this is to recover to the old affinity if we failed at
> this point. Even in this case, setting the new affinity still failed and
> we need to inform the caller about this. Thus returning the error.
> Also note the pr_err message which also tells you about the
> recovery-case :)

I was thinking the other way around. You recover the old affinity. Should
you print out a warning mentioning to the system admin that you could not
set the new affinity but reverted to the old one? Or will that not serve
anything except spam the kernel logs?

Re: [PATCH 09/16] iommu/amd: Add IOAPIC remapping routines

[Joerg Roedel]

On Mon, Oct 01, 2012 at 09:47:53AM -0400, Konrad Rzeszutek Wilk wrote:
> On Mon, Oct 01, 2012 at 10:40:51AM +0200, Joerg Roedel wrote:

> > Don't understand this. What is not set?
> 
> the IRQ_DELAYED_DISABLE. But I could not even find that enum anymore?
> Has that been obsoleted?

You are right, its was removed. Ok, I will remove that line of the
comment too.

> I was thinking the other way around. You recover the old affinity. Should
> you print out a warning mentioning to the system admin that you could not
> set the new affinity but reverted to the old one? Or will that not serve
> anything except spam the kernel logs?

Well, I don't think it makes sense to print an additional error at this
point. The error will be propagated upwards in the call-chain and in the
end it is up to the initiator how to handle this situation (for example
just return a failure to user-space).


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 09/16] iommu/amd: Add IOAPIC remapping routines

[Konrad Rzeszutek Wilk]

>> I was thinking the other way around. You recover the old affinity. Should
>> you print out a warning mentioning to the system admin that you could not
>> set the new affinity but reverted to the old one? Or will that not serve
>> anything except spam the kernel logs?
>
> Well, I don't think it makes sense to print an additional error at this
> point. The error will be propagated upwards in the call-chain and in the
> end it is up to the initiator how to handle this situation (for example
> just return a failure to user-space).

OK. Thx for the explanation. I was not sure whether the error would
propagate properly up the chain.

[PATCH 10/16] iommu/amd: Implement MSI routines for interrupt remapping

[Joerg Roedel]

Add routines to setup interrupt remapping for MSI
interrupts.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c |   74 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 3df01b2..84efecd 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -4126,4 +4126,78 @@ static int free_irq(int irq)
 	return 0;
 }
 
+static void compose_msi_msg(struct pci_dev *pdev,
+			    unsigned int irq, unsigned int dest,
+			    struct msi_msg *msg, u8 hpet_id)
+{
+	struct irq_2_iommu *irte_info;
+	struct irq_cfg *cfg;
+	union irte irte;
+
+	cfg = irq_get_chip_data(irq);
+	if (!cfg)
+		return;
+
+	irte_info = &cfg->irq_2_iommu;
+
+	irte.val		= 0;
+	irte.fields.vector	= cfg->vector;
+	irte.fields.int_type    = apic->irq_delivery_mode;
+	irte.fields.destination	= dest;
+	irte.fields.dm		= apic->irq_dest_mode;
+	irte.fields.valid	= 1;
+
+	modify_irte(irte_info->sub_handle, irte_info->irte_index, irte);
+
+	msg->address_hi = MSI_ADDR_BASE_HI;
+	msg->address_lo = MSI_ADDR_BASE_LO;
+	msg->data       = irte_info->irte_index;
+}
+
+static int msi_alloc_irq(struct pci_dev *pdev, int irq, int nvec)
+{
+	struct irq_cfg *cfg;
+	int index;
+	u16 devid;
+
+	if (!pdev)
+		return -EINVAL;
+
+	cfg = irq_get_chip_data(irq);
+	if (!cfg)
+		return -EINVAL;
+
+	devid = get_device_id(&pdev->dev);
+	index = alloc_irq_index(cfg, devid, nvec);
+
+	return index < 0 ? MAX_IRQS_PER_TABLE : index;
+}
+
+static int msi_setup_irq(struct pci_dev *pdev, unsigned int irq,
+			 int index, int offset)
+{
+	struct irq_2_iommu *irte_info;
+	struct irq_cfg *cfg;
+	u16 devid;
+
+	if (!pdev)
+		return -EINVAL;
+
+	cfg = irq_get_chip_data(irq);
+	if (!cfg)
+		return -EINVAL;
+
+	if (index >= MAX_IRQS_PER_TABLE)
+		return 0;
+
+	devid		= get_device_id(&pdev->dev);
+	irte_info	= &cfg->irq_2_iommu;
+
+	irte_info->sub_handle = devid;
+	irte_info->irte_index = index + offset;
+	irte_info->iommu      = (void *)cfg;
+
+	return 0;
+}
+
 #endif
-- 
1.7.9.5

Re: [PATCH 10/16] iommu/amd: Implement MSI routines for interrupt remapping

[Konrad Rzeszutek Wilk]

On Fri, Sep 28, 2012 at 02:24:00PM +0200, Joerg Roedel wrote:
> Add routines to setup interrupt remapping for MSI
> interrupts.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
> ---
>  drivers/iommu/amd_iommu.c |   74 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 3df01b2..84efecd 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -4126,4 +4126,78 @@ static int free_irq(int irq)
>  	return 0;
>  }
>  
> +static void compose_msi_msg(struct pci_dev *pdev,
> +			    unsigned int irq, unsigned int dest,
> +			    struct msi_msg *msg, u8 hpet_id)
> +{
> +	struct irq_2_iommu *irte_info;
> +	struct irq_cfg *cfg;
> +	union irte irte;
> +
> +	cfg = irq_get_chip_data(irq);
> +	if (!cfg)
> +		return;
> +
> +	irte_info = &cfg->irq_2_iommu;
> +
> +	irte.val		= 0;
> +	irte.fields.vector	= cfg->vector;
> +	irte.fields.int_type    = apic->irq_delivery_mode;
> +	irte.fields.destination	= dest;
> +	irte.fields.dm		= apic->irq_dest_mode;
> +	irte.fields.valid	= 1;
> +
> +	modify_irte(irte_info->sub_handle, irte_info->irte_index, irte);
> +
> +	msg->address_hi = MSI_ADDR_BASE_HI;
> +	msg->address_lo = MSI_ADDR_BASE_LO;
> +	msg->data       = irte_info->irte_index;
> +}
> +
> +static int msi_alloc_irq(struct pci_dev *pdev, int irq, int nvec)
> +{
> +	struct irq_cfg *cfg;
> +	int index;
> +	u16 devid;
> +
> +	if (!pdev)
> +		return -EINVAL;
> +
> +	cfg = irq_get_chip_data(irq);
> +	if (!cfg)
> +		return -EINVAL;
> +
> +	devid = get_device_id(&pdev->dev);
> +	index = alloc_irq_index(cfg, devid, nvec);
> +
> +	return index < 0 ? MAX_IRQS_PER_TABLE : index;
> +}
> +
> +static int msi_setup_irq(struct pci_dev *pdev, unsigned int irq,
> +			 int index, int offset)
> +{
> +	struct irq_2_iommu *irte_info;
> +	struct irq_cfg *cfg;
> +	u16 devid;
> +
> +	if (!pdev)
> +		return -EINVAL;
> +
> +	cfg = irq_get_chip_data(irq);
> +	if (!cfg)
> +		return -EINVAL;
> +
> +	if (index >= MAX_IRQS_PER_TABLE)
> +		return 0;

Hmm? Not an error code?

> +
> +	devid		= get_device_id(&pdev->dev);
> +	irte_info	= &cfg->irq_2_iommu;
> +
> +	irte_info->sub_handle = devid;
> +	irte_info->irte_index = index + offset;
> +	irte_info->iommu      = (void *)cfg;
> +
> +	return 0;
> +}
> +
>  #endif
> -- 
> 1.7.9.5
> 
> 
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 10/16] iommu/amd: Implement MSI routines for interrupt remapping

[Joerg Roedel]

On Fri, Sep 28, 2012 at 10:49:34AM -0400, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 28, 2012 at 02:24:00PM +0200, Joerg Roedel wrote:
> > +	if (index >= MAX_IRQS_PER_TABLE)
> > +		return 0;
> 
> Hmm? Not an error code?

No, when this case happens it just means that the interrupt is not
remapped (because the device itself is not handled by the IOMMU). This
is not an error code and only tells the code which composes the msi-msg
to use the default routine instead of the one for interrupt remapping.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

[PATCH 11/16] iommu/amd: Add call-back routine for HPET MSI

[Joerg Roedel]

Add a routine to setup a HPET MSI interrupt for remapping.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 84efecd..af245f4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -4200,4 +4200,30 @@ static int msi_setup_irq(struct pci_dev *pdev, unsigned int irq,
 	return 0;
 }
 
+static int setup_hpet_msi(unsigned int irq, unsigned int id)
+{
+	struct irq_2_iommu *irte_info;
+	struct irq_cfg *cfg;
+	int index, devid;
+
+	cfg = irq_get_chip_data(irq);
+	if (!cfg)
+		return -EINVAL;
+
+	irte_info = &cfg->irq_2_iommu;
+	devid     = get_hpet_devid(id);
+	if (devid < 0)
+		return devid;
+
+	index = alloc_irq_index(cfg, devid, 1);
+	if (index < 0)
+		return index;
+
+	irte_info->sub_handle = devid;
+	irte_info->irte_index = index;
+	irte_info->iommu      = (void *)cfg;
+
+	return 0;
+}
+
 #endif
-- 
1.7.9.5

[PATCH 12/16] iommu/amd: Add initialization routines for AMD interrupt remapping

[Joerg Roedel]

Add the six routines required to setup interrupt remapping
with the AMD IOMMU. Also put it all together into the AMD
specific irq_remap_ops.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c       |   16 +++++++++++++++
 drivers/iommu/amd_iommu_init.c  |   42 +++++++++++++++++++++++++++++++++++++++
 drivers/iommu/amd_iommu_proto.h |    8 ++++++++
 drivers/iommu/irq_remapping.h   |    1 +
 4 files changed, 67 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index af245f4..6fe4ad7 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -45,6 +45,7 @@
 
 #include "amd_iommu_proto.h"
 #include "amd_iommu_types.h"
+#include "irq_remapping.h"
 
 #define CMD_SET_TYPE(cmd, t) ((cmd)->data[1] |= ((t) << 28))
 
@@ -4226,4 +4227,19 @@ static int setup_hpet_msi(unsigned int irq, unsigned int id)
 	return 0;
 }
 
+struct irq_remap_ops amd_iommu_irq_ops = {
+	.supported		= amd_iommu_supported,
+	.prepare		= amd_iommu_prepare,
+	.enable			= amd_iommu_enable,
+	.disable		= amd_iommu_disable,
+	.reenable		= amd_iommu_reenable,
+	.enable_faulting	= amd_iommu_enable_faulting,
+	.setup_ioapic_entry	= setup_ioapic_entry,
+	.set_affinity		= set_affinity,
+	.free_irq		= free_irq,
+	.compose_msi_msg	= compose_msi_msg,
+	.msi_alloc_irq		= msi_alloc_irq,
+	.msi_setup_irq		= msi_setup_irq,
+	.setup_hpet_msi		= setup_hpet_msi,
+};
 #endif
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 202b2b7..42db810 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -34,6 +34,7 @@
 #include <asm/x86_init.h>
 #include <asm/iommu_table.h>
 #include <asm/io_apic.h>
+#include <asm/irq_remapping.h>
 
 #include "amd_iommu_proto.h"
 #include "amd_iommu_types.h"
@@ -1894,7 +1895,48 @@ static int __init iommu_go_to_state(enum iommu_init_state state)
 	return ret;
 }
 
+#ifdef CONFIG_IRQ_REMAP
+int __init amd_iommu_prepare(void)
+{
+	return iommu_go_to_state(IOMMU_ACPI_FINISHED);
+}
+
+int __init amd_iommu_supported(void)
+{
+	return amd_iommu_irq_remap ? 1 : 0;
+}
+
+int __init amd_iommu_enable(void)
+{
+	int ret;
+
+	ret = iommu_go_to_state(IOMMU_ENABLED);
+	if (ret)
+		return ret;
 
+	irq_remapping_enabled = 1;
+
+	return 0;
+}
+
+void amd_iommu_disable(void)
+{
+	amd_iommu_suspend();
+}
+
+int amd_iommu_reenable(int mode)
+{
+	amd_iommu_resume();
+
+	return 0;
+}
+
+int __init amd_iommu_enable_faulting(void)
+{
+	/* We enable MSI later when PCI is initialized */
+	return 0;
+}
+#endif
 
 /*
  * This is the core init function for AMD IOMMU hardware in the system.
diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
index 1a7f41c..c294961 100644
--- a/drivers/iommu/amd_iommu_proto.h
+++ b/drivers/iommu/amd_iommu_proto.h
@@ -32,6 +32,14 @@ extern void amd_iommu_uninit_devices(void);
 extern void amd_iommu_init_notifier(void);
 extern void amd_iommu_init_api(void);
 
+/* Needed for interrupt remapping */
+extern int amd_iommu_supported(void);
+extern int amd_iommu_prepare(void);
+extern int amd_iommu_enable(void);
+extern void amd_iommu_disable(void);
+extern int amd_iommu_reenable(int);
+extern int amd_iommu_enable_faulting(void);
+
 /* IOMMUv2 specific functions */
 struct iommu_domain;
 
diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
index 624d360..95363ac 100644
--- a/drivers/iommu/irq_remapping.h
+++ b/drivers/iommu/irq_remapping.h
@@ -82,6 +82,7 @@ struct irq_remap_ops {
 };
 
 extern struct irq_remap_ops intel_irq_remap_ops;
+extern struct irq_remap_ops amd_iommu_irq_ops;
 
 #else  /* CONFIG_IRQ_REMAP */
 
-- 
1.7.9.5

Re: [PATCH 12/16] iommu/amd: Add initialization routines for AMD interrupt remapping

[Shuah Khan]

Joerg,

On Fri, Sep 28, 2012 at 6:24 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> Add the six routines required to setup interrupt remapping
> with the AMD IOMMU. Also put it all together into the AMD
> specific irq_remap_ops.
>
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/iommu/amd_iommu.c       |   16 +++++++++++++++
>  drivers/iommu/amd_iommu_init.c  |   42 +++++++++++++++++++++++++++++++++++++++
>  drivers/iommu/amd_iommu_proto.h |    8 ++++++++
>  drivers/iommu/irq_remapping.h   |    1 +
>  4 files changed, 67 insertions(+)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index af245f4..6fe4ad7 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -45,6 +45,7 @@
>
>  #include "amd_iommu_proto.h"
>  #include "amd_iommu_types.h"
> +#include "irq_remapping.h"
>
>  #define CMD_SET_TYPE(cmd, t) ((cmd)->data[1] |= ((t) << 28))
>
> @@ -4226,4 +4227,19 @@ static int setup_hpet_msi(unsigned int irq, unsigned int id)
>         return 0;
>  }
>
> +struct irq_remap_ops amd_iommu_irq_ops = {
> +       .supported              = amd_iommu_supported,
> +       .prepare                = amd_iommu_prepare,
> +       .enable                 = amd_iommu_enable,
> +       .disable                = amd_iommu_disable,
> +       .reenable               = amd_iommu_reenable,
> +       .enable_faulting        = amd_iommu_enable_faulting,
> +       .setup_ioapic_entry     = setup_ioapic_entry,
> +       .set_affinity           = set_affinity,
> +       .free_irq               = free_irq,
> +       .compose_msi_msg        = compose_msi_msg,
> +       .msi_alloc_irq          = msi_alloc_irq,
> +       .msi_setup_irq          = msi_setup_irq,
> +       .setup_hpet_msi         = setup_hpet_msi,
> +};
>  #endif
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 202b2b7..42db810 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -34,6 +34,7 @@
>  #include <asm/x86_init.h>
>  #include <asm/iommu_table.h>
>  #include <asm/io_apic.h>
> +#include <asm/irq_remapping.h>
>
>  #include "amd_iommu_proto.h"
>  #include "amd_iommu_types.h"
> @@ -1894,7 +1895,48 @@ static int __init iommu_go_to_state(enum iommu_init_state state)
>         return ret;
>  }
>
> +#ifdef CONFIG_IRQ_REMAP
> +int __init amd_iommu_prepare(void)
> +{
> +       return iommu_go_to_state(IOMMU_ACPI_FINISHED);
> +}
> +
> +int __init amd_iommu_supported(void)
> +{
> +       return amd_iommu_irq_remap ? 1 : 0;
> +}
> +
> +int __init amd_iommu_enable(void)
> +{
> +       int ret;
> +
> +       ret = iommu_go_to_state(IOMMU_ENABLED);
> +       if (ret)
> +               return ret;
>
> +       irq_remapping_enabled = 1;
> +
> +       return 0;
> +}
> +
> +void amd_iommu_disable(void)
> +{
> +       amd_iommu_suspend();

Is it safe to attempt to suspend when iommu is in suspend state? In
other words what happens if amd_iommu_disable() gets called when iommu
is already in suspend state?

> +}
> +
> +int amd_iommu_reenable(int mode)
> +{
> +       amd_iommu_resume();

Same question as above. Safe to do a resume, when in resumed state?


> +
> +       return 0;
> +}
> +
> +int __init amd_iommu_enable_faulting(void)
> +{
> +       /* We enable MSI later when PCI is initialized */
> +       return 0;
> +}
> +#endif
>
>  /*
>   * This is the core init function for AMD IOMMU hardware in the system.
> diff --git a/drivers/iommu/amd_iommu_proto.h b/drivers/iommu/amd_iommu_proto.h
> index 1a7f41c..c294961 100644
> --- a/drivers/iommu/amd_iommu_proto.h
> +++ b/drivers/iommu/amd_iommu_proto.h
> @@ -32,6 +32,14 @@ extern void amd_iommu_uninit_devices(void);
>  extern void amd_iommu_init_notifier(void);
>  extern void amd_iommu_init_api(void);
>
> +/* Needed for interrupt remapping */
> +extern int amd_iommu_supported(void);
> +extern int amd_iommu_prepare(void);
> +extern int amd_iommu_enable(void);
> +extern void amd_iommu_disable(void);
> +extern int amd_iommu_reenable(int);
> +extern int amd_iommu_enable_faulting(void);
> +
>  /* IOMMUv2 specific functions */
>  struct iommu_domain;
>
> diff --git a/drivers/iommu/irq_remapping.h b/drivers/iommu/irq_remapping.h
> index 624d360..95363ac 100644
> --- a/drivers/iommu/irq_remapping.h
> +++ b/drivers/iommu/irq_remapping.h
> @@ -82,6 +82,7 @@ struct irq_remap_ops {
>  };
>
>  extern struct irq_remap_ops intel_irq_remap_ops;
> +extern struct irq_remap_ops amd_iommu_irq_ops;
>
>  #else  /* CONFIG_IRQ_REMAP */
>
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 12/16] iommu/amd: Add initialization routines for AMD interrupt remapping

[Joerg Roedel]

On Fri, Sep 28, 2012 at 05:18:18PM -0600, Shuah Khan wrote:
> > +void amd_iommu_disable(void)
> > +{
> > +       amd_iommu_suspend();
> 
> Is it safe to attempt to suspend when iommu is in suspend state? In
> other words what happens if amd_iommu_disable() gets called when iommu
> is already in suspend state?

Yes, this is safe. It just trns a bit to 0 which is already 0.

>> > +int amd_iommu_reenable(int mode)
> > +{
> > +       amd_iommu_resume();
> 
> Same question as above. Safe to do a resume, when in resumed state?

Safe too, it just writes values to the hardware which are already there.
This was also proven by testing.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 12/16] iommu/amd: Add initialization routines for AMD interrupt remapping

[Shuah Khan]

On Mon, Oct 1, 2012 at 2:05 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> On Fri, Sep 28, 2012 at 05:18:18PM -0600, Shuah Khan wrote:
>> > +void amd_iommu_disable(void)
>> > +{
>> > +       amd_iommu_suspend();
>>
>> Is it safe to attempt to suspend when iommu is in suspend state? In
>> other words what happens if amd_iommu_disable() gets called when iommu
>> is already in suspend state?
>
> Yes, this is safe. It just trns a bit to 0 which is already 0.
>
Good.
>>> > +int amd_iommu_reenable(int mode)
>> > +{
>> > +       amd_iommu_resume();
>>
>> Same question as above. Safe to do a resume, when in resumed state?
>
> Safe too, it just writes values to the hardware which are already there.
> This was also proven by testing.
>

Thanks. Sounds good.

-- Shuah

[PATCH 13/16] iommu/amd: Make sure irq remapping still works on dma init failure

[Joerg Roedel]

Do not deinitialize the AMD IOMMU driver completly when
interrupt remapping is already in use but the initialization
of the DMA layer fails for some reason. Make sure the IOMMU
can still be used to remap interrupts.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_init.c |   40 +++++++++++++++++++++++++++++++---------
 1 file changed, 31 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 42db810..0608ef4 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1396,6 +1396,16 @@ static void init_device_table_dma(void)
 	}
 }
 
+static void __init uninit_device_table_dma(void)
+{
+	u32 devid;
+
+	for (devid = 0; devid <= amd_iommu_last_bdf; ++devid) {
+		amd_iommu_dev_table[devid].data[0] = 0ULL;
+		amd_iommu_dev_table[devid].data[1] = 0ULL;
+	}
+}
+
 static void init_device_table(void)
 {
 	u32 devid;
@@ -1564,11 +1574,6 @@ static void __init free_on_init_error(void)
 
 	}
 
-	amd_iommu_uninit_devices();
-
-	free_pages((unsigned long)amd_iommu_pd_alloc_bitmap,
-		   get_order(MAX_DOMAIN_ID/8));
-
 	free_pages((unsigned long)amd_iommu_rlookup_table,
 		   get_order(rlookup_table_size));
 
@@ -1580,8 +1585,6 @@ static void __init free_on_init_error(void)
 
 	free_iommu_all();
 
-	free_unity_maps();
-
 #ifdef CONFIG_GART_IOMMU
 	/*
 	 * We failed to initialize the AMD IOMMU - try fallback to GART
@@ -1609,6 +1612,16 @@ static bool __init check_ioapic_information(void)
 	return true;
 }
 
+static void __init free_dma_resources(void)
+{
+	amd_iommu_uninit_devices();
+
+	free_pages((unsigned long)amd_iommu_pd_alloc_bitmap,
+		   get_order(MAX_DOMAIN_ID/8));
+
+	free_unity_maps();
+}
+
 /*
  * This is the hardware init function for AMD IOMMU in the system.
  * This function is called either from amd_iommu_init or from the interrupt
@@ -1949,8 +1962,17 @@ static int __init amd_iommu_init(void)
 
 	ret = iommu_go_to_state(IOMMU_INITIALIZED);
 	if (ret) {
-		disable_iommus();
-		free_on_init_error();
+		free_dma_resources();
+		if (!irq_remapping_enabled) {
+			disable_iommus();
+			free_on_init_error();
+		} else {
+			struct amd_iommu *iommu;
+
+			uninit_device_table_dma();
+			for_each_iommu(iommu)
+				iommu_flush_all_caches(iommu);
+		}
 	}
 
 	return ret;
-- 
1.7.9.5

[PATCH 14/16] iommu/irq: Use amd_iommu_irq_ops if supported

[Joerg Roedel]

Finally enable interrupt remapping for AMD systems.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/irq_remapping.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
index 151690d..faf85d6 100644
--- a/drivers/iommu/irq_remapping.c
+++ b/drivers/iommu/irq_remapping.c
@@ -51,6 +51,11 @@ early_param("intremap", setup_irqremap);
 void __init setup_irq_remapping_ops(void)
 {
 	remap_ops = &intel_irq_remap_ops;
+
+#ifdef CONFIG_AMD_IOMMU
+	if (amd_iommu_irq_ops.prepare() == 0)
+		remap_ops = &amd_iommu_irq_ops;
+#endif
 }
 
 int irq_remapping_supported(void)
-- 
1.7.9.5

Re: [PATCH 14/16] iommu/irq: Use amd_iommu_irq_ops if supported

[Shuah Khan]

On Fri, Sep 28, 2012 at 6:24 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> Finally enable interrupt remapping for AMD systems.
>
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  drivers/iommu/irq_remapping.c |    5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/iommu/irq_remapping.c b/drivers/iommu/irq_remapping.c
> index 151690d..faf85d6 100644
> --- a/drivers/iommu/irq_remapping.c
> +++ b/drivers/iommu/irq_remapping.c
> @@ -51,6 +51,11 @@ early_param("intremap", setup_irqremap);
>  void __init setup_irq_remapping_ops(void)
>  {
>         remap_ops = &intel_irq_remap_ops;
> +
> +#ifdef CONFIG_AMD_IOMMU
> +       if (amd_iommu_irq_ops.prepare() == 0)
> +               remap_ops = &amd_iommu_irq_ops;
> +#endif

Should remap_ops be set to null when amd_iommu_irq_ops.prepare()
fails?What happens if remap_ops left set to intel_irq_remap_ops?
Should remap_ops = &intel_irq_remap_ops; moved into #else case
forifdef CONFIG_AMD_IOMMU?
>  }
>
>  int irq_remapping_supported(void)
> --
> 1.7.9.5
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 14/16] iommu/irq: Use amd_iommu_irq_ops if supported

[Joerg Roedel]

On Fri, Sep 28, 2012 at 05:39:03PM -0600, Shuah Khan wrote:
> On Fri, Sep 28, 2012 at 6:24 AM, Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org> wrote:
> >  void __init setup_irq_remapping_ops(void)
> >  {
> >         remap_ops = &intel_irq_remap_ops;
> > +
> > +#ifdef CONFIG_AMD_IOMMU
> > +       if (amd_iommu_irq_ops.prepare() == 0)
> > +               remap_ops = &amd_iommu_irq_ops;
> > +#endif
> 
> Should remap_ops be set to null when amd_iommu_irq_ops.prepare()
> fails?What happens if remap_ops left set to intel_irq_remap_ops?
> Should remap_ops = &intel_irq_remap_ops; moved into #else case
> forifdef CONFIG_AMD_IOMMU?

Remap-Ops does not need to be set to NULL because irq_remapping_enabled
will not get set to true then and remap_ops will not get called. The
Intel path can't also be moved to #else because this would mean that
kernels can only support eihter, Intel or AMD IOMMU. But Linux can
support both in the same kernel.


	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 14/16] iommu/irq: Use amd_iommu_irq_ops if supported

[Shuah Khan]

On Mon, Oct 1, 2012 at 2:08 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
> On Fri, Sep 28, 2012 at 05:39:03PM -0600, Shuah Khan wrote:
>> On Fri, Sep 28, 2012 at 6:24 AM, Joerg Roedel <joerg.roedel@amd.com> wrote:
>> >  void __init setup_irq_remapping_ops(void)
>> >  {
>> >         remap_ops = &intel_irq_remap_ops;
>> > +
>> > +#ifdef CONFIG_AMD_IOMMU
>> > +       if (amd_iommu_irq_ops.prepare() == 0)
>> > +               remap_ops = &amd_iommu_irq_ops;
>> > +#endif
>>
>> Should remap_ops be set to null when amd_iommu_irq_ops.prepare()
>> fails?What happens if remap_ops left set to intel_irq_remap_ops?
>> Should remap_ops = &intel_irq_remap_ops; moved into #else case
>> forifdef CONFIG_AMD_IOMMU?
>
> Remap-Ops does not need to be set to NULL because irq_remapping_enabled
> will not get set to true then and remap_ops will not get called. The
> Intel path can't also be moved to #else because this would mean that
> kernels can only support eihter, Intel or AMD IOMMU. But Linux can
> support both in the same kernel.

Thanks for the explanation.

-- Shuah

[PATCH 15/16] iommu/amd: Print message to system log when irq remapping is enabled

[Joerg Roedel]

Print an indicator to dmesg to easily find out if interrupt
remapping is enabled of a given system.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu_init.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 0608ef4..45f0f47 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1196,6 +1196,8 @@ static void print_iommu_info(void)
 		}
 		pr_cont("\n");
 	}
+	if (irq_remapping_enabled)
+		pr_info("AMD-Vi: Interrupt remapping enabled\n");
 }
 
 static int __init amd_iommu_init_pci(void)
-- 
1.7.9.5

[PATCH 16/16] iommu/amd: Report irq remapping through IOMMU-API

[Joerg Roedel]

Report the availability of irq remapping through the
IOMMU-API to allow KVM device passthrough again without
additional module parameter overrides.

Signed-off-by: Joerg Roedel <joerg.roedel-5C7GfCeVMHo@public.gmane.org>
---
 drivers/iommu/amd_iommu.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 6fe4ad7..3f616f6 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3353,6 +3353,8 @@ static int amd_iommu_domain_has_cap(struct iommu_domain *domain,
 	switch (cap) {
 	case IOMMU_CAP_CACHE_COHERENCY:
 		return 1;
+	case IOMMU_CAP_INTR_REMAP:
+		return irq_remapping_enabled;
 	}
 
 	return 0;
-- 
1.7.9.5