[PATCH 0/9] AMD IOMMU cleanups, fixes and IVRS bug workarounds

[PATCH 0/9] AMD IOMMU cleanups, fixes and IVRS bug workarounds

[Joerg Roedel]

Hi,

this patch-set contains some cleanups and patches for the
AMD IOMM driver. The most important part is a workaround
that can be used to get interrupt remapping working even the
the IVRS table provided by the BIOS is broken.


	Joerg

Joerg Roedel (9):
  iommu/amd: Remove map_sg_no_iommu()
  iommu/amd: Use AMD specific data structure for irq remapping
  iommu/amd: Properly initialize irq-table lock
  iommu/amd: Move add_special_device() to __init
  iommu/amd: Extend IVRS special device data structure
  iommu/amd: Add early maps for ioapic and hpet
  iommu/amd: Add ioapic and hpet ivrs override
  iommu/amd: Don't report firmware bugs with cmd-line ivrs overrides
  iommu/amd: Document ivrs_ioapic and ivrs_hpet parameters

 Documentation/kernel-parameters.txt |   14 ++++
 drivers/iommu/amd_iommu.c           |   79 +++++++-----------
 drivers/iommu/amd_iommu_init.c      |  151 +++++++++++++++++++++++++++++++----
 drivers/iommu/amd_iommu_types.h     |    1 +
 4 files changed, 182 insertions(+), 63 deletions(-)

-- 
1.7.9.5

[PATCH 1/9] iommu/amd: Remove map_sg_no_iommu()

[Joerg Roedel]

This function was intended as a fall-back if the map_sg
function is called for a device not mapped by the IOMMU.
Since the AMD IOMMU driver uses per-device dma_ops this can
never happen. So this function isn't needed anymore.

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu.c |   22 +---------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index b287ca3..3b2aff0 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2839,24 +2839,6 @@ static void unmap_page(struct device *dev, dma_addr_t dma_addr, size_t size,
 }
 
 /*
- * This is a special map_sg function which is used if we should map a
- * device which is not handled by an AMD IOMMU in the system.
- */
-static int map_sg_no_iommu(struct device *dev, struct scatterlist *sglist,
-			   int nelems, int dir)
-{
-	struct scatterlist *s;
-	int i;
-
-	for_each_sg(sglist, s, nelems, i) {
-		s->dma_address = (dma_addr_t)sg_phys(s);
-		s->dma_length  = s->length;
-	}
-
-	return nelems;
-}
-
-/*
  * The exported map_sg function for dma_ops (handles scatter-gather
  * lists).
  */
@@ -2875,9 +2857,7 @@ static int map_sg(struct device *dev, struct scatterlist *sglist,
 	INC_STATS_COUNTER(cnt_map_sg);
 
 	domain = get_domain(dev);
-	if (PTR_ERR(domain) == -EINVAL)
-		return map_sg_no_iommu(dev, sglist, nelems, dir);
-	else if (IS_ERR(domain))
+	if (IS_ERR(domain))
 		return 0;
 
 	dma_mask = *dev->dma_mask;
-- 
1.7.9.5

Re: [PATCH 1/9] iommu/amd: Remove map_sg_no_iommu()

[Shuah Khan]

On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <joro@8bytes.org> wrote:
> This function was intended as a fall-back if the map_sg
> function is called for a device not mapped by the IOMMU.
> Since the AMD IOMMU driver uses per-device dma_ops this can
> never happen. So this function isn't needed anymore.
>
> Signed-off-by: Joerg Roedel <joro@8bytes.org>
> ---

Reviewed-by: Shuah Khan <shuahkhan@gmail.com>

[PATCH 2/9] iommu/amd: Use AMD specific data structure for irq remapping

[Joerg Roedel]

For compatibility reasons the irq remapping code for the AMD
IOMMU used the same per-irq data structure as the Intel
implementation. Now that support for the AMD specific data
structure is upstream we can use this one instead.

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu.c |   54 ++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 3b2aff0..72fa570 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3987,7 +3987,7 @@ static int alloc_irq_index(struct irq_cfg *cfg, u16 devid, int count)
 			c = 0;
 
 		if (c == count)	{
-			struct irq_2_iommu *irte_info;
+			struct irq_2_irte *irte_info;
 
 			for (; c != 0; --c)
 				table->table[index - c + 1] = IRTE_ALLOCATED;
@@ -3995,9 +3995,9 @@ static int alloc_irq_index(struct irq_cfg *cfg, u16 devid, int count)
 			index -= count - 1;
 
 			cfg->remapped	      = 1;
-			irte_info             = &cfg->irq_2_iommu;
-			irte_info->sub_handle = devid;
-			irte_info->irte_index = index;
+			irte_info             = &cfg->irq_2_irte;
+			irte_info->devid      = devid;
+			irte_info->index      = index;
 
 			goto out;
 		}
@@ -4078,7 +4078,7 @@ static int setup_ioapic_entry(int irq, struct IO_APIC_route_entry *entry,
 			      struct io_apic_irq_attr *attr)
 {
 	struct irq_remap_table *table;
-	struct irq_2_iommu *irte_info;
+	struct irq_2_irte *irte_info;
 	struct irq_cfg *cfg;
 	union irte irte;
 	int ioapic_id;
@@ -4090,7 +4090,7 @@ static int setup_ioapic_entry(int irq, struct IO_APIC_route_entry *entry,
 	if (!cfg)
 		return -EINVAL;
 
-	irte_info = &cfg->irq_2_iommu;
+	irte_info = &cfg->irq_2_irte;
 	ioapic_id = mpc_ioapic_id(attr->ioapic);
 	devid     = get_ioapic_devid(ioapic_id);
 
@@ -4105,8 +4105,8 @@ static int setup_ioapic_entry(int irq, struct IO_APIC_route_entry *entry,
 
 	/* Setup IRQ remapping info */
 	cfg->remapped	      = 1;
-	irte_info->sub_handle = devid;
-	irte_info->irte_index = index;
+	irte_info->devid      = devid;
+	irte_info->index      = index;
 
 	/* Setup IRTE for IOMMU */
 	irte.val		= 0;
@@ -4140,7 +4140,7 @@ static int setup_ioapic_entry(int irq, struct IO_APIC_route_entry *entry,
 static int set_affinity(struct irq_data *data, const struct cpumask *mask,
 			bool force)
 {
-	struct irq_2_iommu *irte_info;
+	struct irq_2_irte *irte_info;
 	unsigned int dest, irq;
 	struct irq_cfg *cfg;
 	union irte irte;
@@ -4151,12 +4151,12 @@ static int set_affinity(struct irq_data *data, const struct cpumask *mask,
 
 	cfg       = data->chip_data;
 	irq       = data->irq;
-	irte_info = &cfg->irq_2_iommu;
+	irte_info = &cfg->irq_2_irte;
 
 	if (!cpumask_intersects(mask, cpu_online_mask))
 		return -EINVAL;
 
-	if (get_irte(irte_info->sub_handle, irte_info->irte_index, &irte))
+	if (get_irte(irte_info->devid, irte_info->index, &irte))
 		return -EBUSY;
 
 	if (assign_irq_vector(irq, cfg, mask))
@@ -4172,7 +4172,7 @@ static int set_affinity(struct irq_data *data, const struct cpumask *mask,
 	irte.fields.vector      = cfg->vector;
 	irte.fields.destination = dest;
 
-	modify_irte(irte_info->sub_handle, irte_info->irte_index, irte);
+	modify_irte(irte_info->devid, irte_info->index, irte);
 
 	if (cfg->move_in_progress)
 		send_cleanup_vector(cfg);
@@ -4184,16 +4184,16 @@ static int set_affinity(struct irq_data *data, const struct cpumask *mask,
 
 static int free_irq(int irq)
 {
-	struct irq_2_iommu *irte_info;
+	struct irq_2_irte *irte_info;
 	struct irq_cfg *cfg;
 
 	cfg = irq_get_chip_data(irq);
 	if (!cfg)
 		return -EINVAL;
 
-	irte_info = &cfg->irq_2_iommu;
+	irte_info = &cfg->irq_2_irte;
 
-	free_irte(irte_info->sub_handle, irte_info->irte_index);
+	free_irte(irte_info->devid, irte_info->index);
 
 	return 0;
 }
@@ -4202,7 +4202,7 @@ 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_2_irte *irte_info;
 	struct irq_cfg *cfg;
 	union irte irte;
 
@@ -4210,7 +4210,7 @@ static void compose_msi_msg(struct pci_dev *pdev,
 	if (!cfg)
 		return;
 
-	irte_info = &cfg->irq_2_iommu;
+	irte_info = &cfg->irq_2_irte;
 
 	irte.val		= 0;
 	irte.fields.vector	= cfg->vector;
@@ -4219,11 +4219,11 @@ static void compose_msi_msg(struct pci_dev *pdev,
 	irte.fields.dm		= apic->irq_dest_mode;
 	irte.fields.valid	= 1;
 
-	modify_irte(irte_info->sub_handle, irte_info->irte_index, irte);
+	modify_irte(irte_info->devid, irte_info->index, irte);
 
 	msg->address_hi = MSI_ADDR_BASE_HI;
 	msg->address_lo = MSI_ADDR_BASE_LO;
-	msg->data       = irte_info->irte_index;
+	msg->data       = irte_info->index;
 }
 
 static int msi_alloc_irq(struct pci_dev *pdev, int irq, int nvec)
@@ -4248,7 +4248,7 @@ static int msi_alloc_irq(struct pci_dev *pdev, int irq, int nvec)
 static int msi_setup_irq(struct pci_dev *pdev, unsigned int irq,
 			 int index, int offset)
 {
-	struct irq_2_iommu *irte_info;
+	struct irq_2_irte *irte_info;
 	struct irq_cfg *cfg;
 	u16 devid;
 
@@ -4263,18 +4263,18 @@ static int msi_setup_irq(struct pci_dev *pdev, unsigned int irq,
 		return 0;
 
 	devid		= get_device_id(&pdev->dev);
-	irte_info	= &cfg->irq_2_iommu;
+	irte_info	= &cfg->irq_2_irte;
 
 	cfg->remapped	      = 1;
-	irte_info->sub_handle = devid;
-	irte_info->irte_index = index + offset;
+	irte_info->devid      = devid;
+	irte_info->index      = index + offset;
 
 	return 0;
 }
 
 static int setup_hpet_msi(unsigned int irq, unsigned int id)
 {
-	struct irq_2_iommu *irte_info;
+	struct irq_2_irte *irte_info;
 	struct irq_cfg *cfg;
 	int index, devid;
 
@@ -4282,7 +4282,7 @@ static int setup_hpet_msi(unsigned int irq, unsigned int id)
 	if (!cfg)
 		return -EINVAL;
 
-	irte_info = &cfg->irq_2_iommu;
+	irte_info = &cfg->irq_2_irte;
 	devid     = get_hpet_devid(id);
 	if (devid < 0)
 		return devid;
@@ -4292,8 +4292,8 @@ static int setup_hpet_msi(unsigned int irq, unsigned int id)
 		return index;
 
 	cfg->remapped	      = 1;
-	irte_info->sub_handle = devid;
-	irte_info->irte_index = index;
+	irte_info->devid      = devid;
+	irte_info->index      = index;
 
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH 2/9] iommu/amd: Use AMD specific data structure for irq remapping

[Shuah Khan]

On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <joro@8bytes.org> wrote:
> For compatibility reasons the irq remapping code for the AMD
> IOMMU used the same per-irq data structure as the Intel
> implementation. Now that support for the AMD specific data
> structure is upstream we can use this one instead.
>
> Signed-off-by: Joerg Roedel <joro@8bytes.org>
> ---

Reviewed-by: Shuah Khan <shuahkhan@gmail.com>

[PATCH 3/9] iommu/amd: Properly initialize irq-table lock

[Joerg Roedel]

Fixes a lockdep warning.

Cc: stable@vger.kernel.org # >= v3.7
Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 72fa570..33900b5 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -3927,6 +3927,9 @@ static struct irq_remap_table *get_irq_table(u16 devid, bool ioapic)
 	if (!table)
 		goto out;
 
+	/* Initialize table spin-lock */
+	spin_lock_init(&table->lock);
+
 	if (ioapic)
 		/* Keep the first 32 indexes free for IOAPIC interrupts */
 		table->min_index = 32;
-- 
1.7.9.5

[PATCH 4/9] iommu/amd: Move add_special_device() to __init

[Joerg Roedel]

The function is only called by other __init functions, so it
can be moved to __init too.

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu_init.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index e3c2d74..6dc8d8d 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -703,7 +703,7 @@ 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)
+static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
 {
 	struct devid_map *entry;
 	struct list_head *list;
-- 
1.7.9.5

[PATCH 5/9] iommu/amd: Extend IVRS special device data structure

[Joerg Roedel]

This patch extends the devid_map data structure to allow
ioapic and hpet entries in ivrs to be overridden on the
kernel command line.

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu_init.c  |   28 +++++++++++++++++++---------
 drivers/iommu/amd_iommu_types.h |    1 +
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 6dc8d8d..3a210f0 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -708,20 +708,30 @@ static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
 	struct devid_map *entry;
 	struct list_head *list;
 
-	if (type != IVHD_SPECIAL_IOAPIC && type != IVHD_SPECIAL_HPET)
+	if (type == IVHD_SPECIAL_IOAPIC)
+		list = &ioapic_map;
+	else if (type == IVHD_SPECIAL_HPET)
+		list = &hpet_map;
+	else
 		return -EINVAL;
 
+	list_for_each_entry(entry, list, list) {
+		if (!(entry->id == id && entry->cmd_line))
+			continue;
+
+		pr_info("AMD-Vi: Command-line override present for %s id %d - ignoring\n",
+			type == IVHD_SPECIAL_IOAPIC ? "IOAPIC" : "HPET", id);
+
+		return 0;
+	}
+
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
 		return -ENOMEM;
 
-	entry->id    = id;
-	entry->devid = devid;
-
-	if (type == IVHD_SPECIAL_IOAPIC)
-		list = &ioapic_map;
-	else
-		list = &hpet_map;
+	entry->id	= id;
+	entry->devid	= devid;
+	entry->cmd_line	= cmd_line;
 
 	list_add_tail(&entry->list, list);
 
@@ -929,7 +939,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 				    PCI_FUNC(devid));
 
 			set_dev_entry_from_acpi(iommu, devid, e->flags, 0);
-			ret = add_special_device(type, handle, devid);
+			ret = add_special_device(type, handle, devid, false);
 			if (ret)
 				return ret;
 			break;
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index e38ab43..f0a85fa 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -591,6 +591,7 @@ struct devid_map {
 	struct list_head list;
 	u8 id;
 	u16 devid;
+	bool cmd_line;
 };
 
 /* Map HPET and IOAPIC ids to the devid used by the IOMMU */
-- 
1.7.9.5

[PATCH 6/9] iommu/amd: Add early maps for ioapic and hpet

[Joerg Roedel]

This is needed in a later patch were ioapic_map and hpet_map
entries are created before the slab allocator is initialized
(and thus add_special_device() can't be used).

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu_init.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 3a210f0..2a3b1b1 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -213,6 +213,13 @@ enum iommu_init_state {
 	IOMMU_INIT_ERROR,
 };
 
+/* Early ioapic and hpet maps from kernel command line */
+#define EARLY_MAP_SIZE		4
+static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
+static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
+static int __initdata early_ioapic_map_size;
+static int __initdata early_hpet_map_size;
+
 static enum iommu_init_state init_state = IOMMU_START_STATE;
 
 static int amd_iommu_enable_interrupts(void);
@@ -738,6 +745,31 @@ static int __init add_special_device(u8 type, u8 id, u16 devid, bool cmd_line)
 	return 0;
 }
 
+static int __init add_early_maps(void)
+{
+	int i, ret;
+
+	for (i = 0; i < early_ioapic_map_size; ++i) {
+		ret = add_special_device(IVHD_SPECIAL_IOAPIC,
+					 early_ioapic_map[i].id,
+					 early_ioapic_map[i].devid,
+					 early_ioapic_map[i].cmd_line);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < early_hpet_map_size; ++i) {
+		ret = add_special_device(IVHD_SPECIAL_HPET,
+					 early_hpet_map[i].id,
+					 early_hpet_map[i].devid,
+					 early_hpet_map[i].cmd_line);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
 /*
  * Reads the device exclusion range from ACPI and initializes the IOMMU with
  * it
@@ -774,6 +806,12 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu,
 	u32 dev_i, ext_flags = 0;
 	bool alias = false;
 	struct ivhd_entry *e;
+	int ret;
+
+
+	ret = add_early_maps();
+	if (ret)
+		return ret;
 
 	/*
 	 * First save the recommended feature enable bits from ACPI
-- 
1.7.9.5

[PATCH 7/9] iommu/amd: Add ioapic and hpet ivrs override

[Joerg Roedel]

Add two new kernel commandline parameters ivrs_ioapic and
ivrs_hpet to override the Id->DeviceId mapping from the IVRS
ACPI table. This can be used to work around broken BIOSes to
get interrupt remapping working on AMD systems.

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu_init.c |   64 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 2a3b1b1..030d6ab 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2145,8 +2145,68 @@ static int __init parse_amd_iommu_options(char *str)
 	return 1;
 }
 
-__setup("amd_iommu_dump", parse_amd_iommu_dump);
-__setup("amd_iommu=", parse_amd_iommu_options);
+static int __init parse_ivrs_ioapic(char *str)
+{
+	unsigned int bus, dev, fn;
+	int ret, id, i;
+	u16 devid;
+
+	ret = sscanf(str, "[%d]=%x:%x.%x", &id, &bus, &dev, &fn);
+
+	if (ret != 4) {
+		pr_err("AMD-Vi: Invalid command line: ivrs_ioapic%s\n", str);
+		return 1;
+	}
+
+	if (early_ioapic_map_size == EARLY_MAP_SIZE) {
+		pr_err("AMD-Vi: Early IOAPIC map overflow - ignoring ivrs_ioapic%s\n",
+			str);
+		return 1;
+	}
+
+	devid = ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
+
+	i				= early_ioapic_map_size++;
+	early_ioapic_map[i].id		= id;
+	early_ioapic_map[i].devid	= devid;
+	early_ioapic_map[i].cmd_line	= true;
+
+	return 1;
+}
+
+static int __init parse_ivrs_hpet(char *str)
+{
+	unsigned int bus, dev, fn;
+	int ret, id, i;
+	u16 devid;
+
+	ret = sscanf(str, "[%d]=%x:%x.%x", &id, &bus, &dev, &fn);
+
+	if (ret != 4) {
+		pr_err("AMD-Vi: Invalid command line: ivrs_hpet%s\n", str);
+		return 1;
+	}
+
+	if (early_hpet_map_size == EARLY_MAP_SIZE) {
+		pr_err("AMD-Vi: Early HPET map overflow - ignoring ivrs_hpet%s\n",
+			str);
+		return 1;
+	}
+
+	devid = ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
+
+	i				= early_hpet_map_size++;
+	early_hpet_map[i].id		= id;
+	early_hpet_map[i].devid		= devid;
+	early_hpet_map[i].cmd_line	= true;
+
+	return 1;
+}
+
+__setup("amd_iommu_dump",	parse_amd_iommu_dump);
+__setup("amd_iommu=",		parse_amd_iommu_options);
+__setup("ivrs_ioapic",		parse_ivrs_ioapic);
+__setup("ivrs_hpet",		parse_ivrs_hpet);
 
 IOMMU_INIT_FINISH(amd_iommu_detect,
 		  gart_iommu_hole_init,
-- 
1.7.9.5

Re: [PATCH 7/9] iommu/amd: Add ioapic and hpet ivrs override

[Joerg Roedel]

On Tue, Apr 09, 2013 at 10:13:02PM +0200, Joerg Roedel wrote:
> Add two new kernel commandline parameters ivrs_ioapic and
> ivrs_hpet to override the Id->DeviceId mapping from the IVRS
> ACPI table. This can be used to work around broken BIOSes to
> get interrupt remapping working on AMD systems.
> 
> Signed-off-by: Joerg Roedel <joro@8bytes.org>

Forgot to add:

Tested-by: Borislav Petkov <bp@suse.de>

Re: [PATCH 7/9] iommu/amd: Add ioapic and hpet ivrs override

[Suravee Suthikulanit]

On 4/9/2013 3:29 PM, Joerg Roedel wrote:
> On Tue, Apr 09, 2013 at 10:13:02PM +0200, Joerg Roedel wrote:
>> Add two new kernel commandline parameters ivrs_ioapic and
>> ivrs_hpet to override the Id->DeviceId mapping from the IVRS
>> ACPI table. This can be used to work around broken BIOSes to
>> get interrupt remapping working on AMD systems.
>>
>> Signed-off-by: Joerg Roedel <joro@8bytes.org>
> Forgot to add:
>
> Tested-by: Borislav Petkov <bp@suse.de>
I also tested on some other systems here and the patch works also.

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

Re: [PATCH 7/9] iommu/amd: Add ioapic and hpet ivrs override

[Joerg Roedel]

On Thu, Apr 11, 2013 at 04:40:07PM -0500, Suthikulpanit, Suravee wrote:
> On 4/9/2013 3:29 PM, Joerg Roedel wrote:
> >On Tue, Apr 09, 2013 at 10:13:02PM +0200, Joerg Roedel wrote:
> >>Add two new kernel commandline parameters ivrs_ioapic and
> >>ivrs_hpet to override the Id->DeviceId mapping from the IVRS
> >>ACPI table. This can be used to work around broken BIOSes to
> >>get interrupt remapping working on AMD systems.
> >>
> >>Signed-off-by: Joerg Roedel <joro@8bytes.org>
> >Forgot to add:
> >
> >Tested-by: Borislav Petkov <bp@suse.de>
> I also tested on some other systems here and the patch works also.

Cool, thanks for testing. I'll add your

Tested-by: Suravee Suthikulanit <suravee.suthikulpanit@amd.com>


	Joerg

[PATCH 8/9] iommu/amd: Don't report firmware bugs with cmd-line ivrs overrides

[Joerg Roedel]

When the IVRS entries for IOAPIC and HPET are overridden on
the kernel command line, a problem detected in the check
function might not be a firmware bug anymore. So disable
the firmware bug reporting if the user provided valid
ivrs_ioapic or ivrs_hpet entries on the command line.

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 drivers/iommu/amd_iommu_init.c |   19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 030d6ab..9767941 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -219,6 +219,7 @@ static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
 static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
 static int __initdata early_ioapic_map_size;
 static int __initdata early_hpet_map_size;
+static bool __initdata cmdline_maps;
 
 static enum iommu_init_state init_state = IOMMU_START_STATE;
 
@@ -1686,18 +1687,28 @@ static void __init free_on_init_error(void)
 
 static bool __init check_ioapic_information(void)
 {
+	const char *fw_bug = FW_BUG;
 	bool ret, has_sb_ioapic;
 	int idx;
 
 	has_sb_ioapic = false;
 	ret           = false;
 
+	/*
+	 * If we have map overrides on the kernel command line the
+	 * messages in this function might not describe firmware bugs
+	 * anymore - so be careful
+	 */
+	if (cmdline_maps)
+		fw_bug = "";
+
 	for (idx = 0; idx < nr_ioapics; idx++) {
 		int devid, id = mpc_ioapic_id(idx);
 
 		devid = get_ioapic_devid(id);
 		if (devid < 0) {
-			pr_err(FW_BUG "AMD-Vi: IOAPIC[%d] not in IVRS table\n", id);
+			pr_err("%sAMD-Vi: IOAPIC[%d] not in IVRS table\n",
+				fw_bug, id);
 			ret = false;
 		} else if (devid == IOAPIC_SB_DEVID) {
 			has_sb_ioapic = true;
@@ -1714,11 +1725,11 @@ static bool __init check_ioapic_information(void)
 		 * when the BIOS is buggy and provides us the wrong
 		 * device id for the IOAPIC in the system.
 		 */
-		pr_err(FW_BUG "AMD-Vi: No southbridge IOAPIC found in IVRS table\n");
+		pr_err("%sAMD-Vi: No southbridge IOAPIC found\n", fw_bug);
 	}
 
 	if (!ret)
-		pr_err("AMD-Vi: Disabling interrupt remapping due to BIOS Bug(s)\n");
+		pr_err("AMD-Vi: Disabling interrupt remapping\n");
 
 	return ret;
 }
@@ -2166,6 +2177,7 @@ static int __init parse_ivrs_ioapic(char *str)
 
 	devid = ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
 
+	cmdline_maps			= true;
 	i				= early_ioapic_map_size++;
 	early_ioapic_map[i].id		= id;
 	early_ioapic_map[i].devid	= devid;
@@ -2195,6 +2207,7 @@ static int __init parse_ivrs_hpet(char *str)
 
 	devid = ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
 
+	cmdline_maps			= true;
 	i				= early_hpet_map_size++;
 	early_hpet_map[i].id		= id;
 	early_hpet_map[i].devid		= devid;
-- 
1.7.9.5

Re: [PATCH 8/9] iommu/amd: Don't report firmware bugs with cmd-line ivrs overrides

[Shuah Khan]

On Tue, Apr 9, 2013 at 2:13 PM, Joerg Roedel <joro@8bytes.org> wrote:
> When the IVRS entries for IOAPIC and HPET are overridden on
> the kernel command line, a problem detected in the check
> function might not be a firmware bug anymore. So disable
> the firmware bug reporting if the user provided valid
> ivrs_ioapic or ivrs_hpet entries on the command line.
>
> Signed-off-by: Joerg Roedel <joro@8bytes.org>
> ---
>  drivers/iommu/amd_iommu_init.c |   19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 030d6ab..9767941 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -219,6 +219,7 @@ static struct devid_map __initdata early_ioapic_map[EARLY_MAP_SIZE];
>  static struct devid_map __initdata early_hpet_map[EARLY_MAP_SIZE];
>  static int __initdata early_ioapic_map_size;
>  static int __initdata early_hpet_map_size;
> +static bool __initdata cmdline_maps;
>
>  static enum iommu_init_state init_state = IOMMU_START_STATE;
>
> @@ -1686,18 +1687,28 @@ static void __init free_on_init_error(void)
>
>  static bool __init check_ioapic_information(void)
>  {
> +       const char *fw_bug = FW_BUG;
>         bool ret, has_sb_ioapic;
>         int idx;
>
>         has_sb_ioapic = false;
>         ret           = false;
>
> +       /*
> +        * If we have map overrides on the kernel command line the
> +        * messages in this function might not describe firmware bugs
> +        * anymore - so be careful
> +        */
> +       if (cmdline_maps)
> +               fw_bug = "";
> +
>         for (idx = 0; idx < nr_ioapics; idx++) {
>                 int devid, id = mpc_ioapic_id(idx);
>
>                 devid = get_ioapic_devid(id);
>                 if (devid < 0) {
> -                       pr_err(FW_BUG "AMD-Vi: IOAPIC[%d] not in IVRS table\n", id);
> +                       pr_err("%sAMD-Vi: IOAPIC[%d] not in IVRS table\n",
> +                               fw_bug, id);
>                         ret = false;
>                 } else if (devid == IOAPIC_SB_DEVID) {
>                         has_sb_ioapic = true;
> @@ -1714,11 +1725,11 @@ static bool __init check_ioapic_information(void)
>                  * when the BIOS is buggy and provides us the wrong
>                  * device id for the IOAPIC in the system.
>                  */
> -               pr_err(FW_BUG "AMD-Vi: No southbridge IOAPIC found in IVRS table\n");
> +               pr_err("%sAMD-Vi: No southbridge IOAPIC found\n", fw_bug);
>         }
>
>         if (!ret)
> -               pr_err("AMD-Vi: Disabling interrupt remapping due to BIOS Bug(s)\n");
> +               pr_err("AMD-Vi: Disabling interrupt remapping\n");

Can this made conditional on cmdline_maps and keep the original
message in the case of a real BIOS bug? That way it is explicit and
easier to debug.

>
>         return ret;
>  }
> @@ -2166,6 +2177,7 @@ static int __init parse_ivrs_ioapic(char *str)
>
>         devid = ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
>
> +       cmdline_maps                    = true;
>         i                               = early_ioapic_map_size++;
>         early_ioapic_map[i].id          = id;
>         early_ioapic_map[i].devid       = devid;
> @@ -2195,6 +2207,7 @@ static int __init parse_ivrs_hpet(char *str)
>
>         devid = ((bus & 0xff) << 8) | ((dev & 0x1f) << 3) | (fn & 0x7);
>
> +       cmdline_maps                    = true;
>         i                               = early_hpet_map_size++;
>         early_hpet_map[i].id            = id;
>         early_hpet_map[i].devid         = devid;
> --
> 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 9/9] iommu/amd: Document ivrs_ioapic and ivrs_hpet parameters

[Joerg Roedel]

Document the new kernel commandline parameters in the
appropriate file.

Signed-off-by: Joerg Roedel <joro@8bytes.org>
---
 Documentation/kernel-parameters.txt |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 4609e81..2848b1b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1226,6 +1226,20 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	iucv=		[HW,NET]
 
+	ivrs_ioapic	[HW,X86_64]
+			Provide an override to the IOAPIC-ID<->DEVICE-ID
+			mapping provided in the IVRS ACPI table. For
+			example, to map IOAPIC-ID decimal 10 to
+			PCI device 00:14.0 write the parameter as:
+				ivrs_ioapic[10]=00:14.0
+
+	ivrs_hpet	[HW,X86_64]
+			Provide an override to the HPET-ID<->DEVICE-ID
+			mapping provided in the IVRS ACPI table. For
+			example, to map HPET-ID decimal 0 to
+			PCI device 00:14.0 write the parameter as:
+				ivrs_hpet[0]=00:14.0
+
 	js=		[HW,JOY] Analog joystick
 			See Documentation/input/joystick.txt.
 
-- 
1.7.9.5

Re: [PATCH 0/9] AMD IOMMU cleanups, fixes and IVRS bug workarounds

[Shuah Khan]

On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <joro@8bytes.org> wrote:
> Hi,
>
> this patch-set contains some cleanups and patches for the
> AMD IOMM driver. The most important part is a workaround
> that can be used to get interrupt remapping working even the
> the IVRS table provided by the BIOS is broken.
>
>
>         Joerg
>
> Joerg Roedel (9):
>   iommu/amd: Remove map_sg_no_iommu()
>   iommu/amd: Use AMD specific data structure for irq remapping
>   iommu/amd: Properly initialize irq-table lock
>   iommu/amd: Move add_special_device() to __init
>   iommu/amd: Extend IVRS special device data structure
>   iommu/amd: Add early maps for ioapic and hpet
>   iommu/amd: Add ioapic and hpet ivrs override
>   iommu/amd: Don't report firmware bugs with cmd-line ivrs overrides
>   iommu/amd: Document ivrs_ioapic and ivrs_hpet parameters
>
>  Documentation/kernel-parameters.txt |   14 ++++
>  drivers/iommu/amd_iommu.c           |   79 +++++++-----------
>  drivers/iommu/amd_iommu_init.c      |  151 +++++++++++++++++++++++++++++++----
>  drivers/iommu/amd_iommu_types.h     |    1 +
>  4 files changed, 182 insertions(+), 63 deletions(-)
>

Joerg,

Reviewed all the patches in this set. No longer have access to test machine. :(

Reviewed-by: Shuah Khan <shuahkhan@gmail.com>

-- Shuah

Re: [PATCH 0/9] AMD IOMMU cleanups, fixes and IVRS bug workarounds

[Joerg Roedel]

Hi Shuah,

On Wed, Apr 10, 2013 at 10:06:02AM -0600, Shuah Khan wrote:
> On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <joro@8bytes.org> wrote:
> >  Documentation/kernel-parameters.txt |   14 ++++
> >  drivers/iommu/amd_iommu.c           |   79 +++++++-----------
> >  drivers/iommu/amd_iommu_init.c      |  151 +++++++++++++++++++++++++++++++----
> >  drivers/iommu/amd_iommu_types.h     |    1 +
> >  4 files changed, 182 insertions(+), 63 deletions(-)

> Reviewed all the patches in this set. No longer have access to test machine. :(

Oh, that's sad. You were the only one having a machine wich actually has
unity-mapped ranges defined in the BIOS table. The code for those
mappings was basically untested before you ran it on that machine.
 
> Reviewed-by: Shuah Khan <shuahkhan@gmail.com>

Thanks.


	Joerg

Re: [PATCH 0/9] AMD IOMMU cleanups, fixes and IVRS bug workarounds

[Andrew Cooks]

On Fri, Apr 12, 2013 at 4:06 PM, Joerg Roedel <joro@8bytes.org> wrote:
> Hi Shuah,
>
> On Wed, Apr 10, 2013 at 10:06:02AM -0600, Shuah Khan wrote:
>> On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <joro@8bytes.org> wrote:
>> >  Documentation/kernel-parameters.txt |   14 ++++
>> >  drivers/iommu/amd_iommu.c           |   79 +++++++-----------
>> >  drivers/iommu/amd_iommu_init.c      |  151 +++++++++++++++++++++++++++++++----
>> >  drivers/iommu/amd_iommu_types.h     |    1 +
>> >  4 files changed, 182 insertions(+), 63 deletions(-)
>
>> Reviewed all the patches in this set. No longer have access to test machine. :(
>
> Oh, that's sad. You were the only one having a machine wich actually has
> unity-mapped ranges defined in the BIOS table. The code for those
> mappings was basically untested before you ran it on that machine.
>
What is the machine in question? Maybe someone else has access to one,
if it's not too exotic.

--
a.

Re: [PATCH 0/9] AMD IOMMU cleanups, fixes and IVRS bug workarounds

[Joerg Roedel]

On Sat, Apr 13, 2013 at 11:06:22PM +0800, Andrew Cooks wrote:
> On Fri, Apr 12, 2013 at 4:06 PM, Joerg Roedel <joro@8bytes.org> wrote:

> > Oh, that's sad. You were the only one having a machine wich actually has
> > unity-mapped ranges defined in the BIOS table. The code for those
> > mappings was basically untested before you ran it on that machine.
> >
> What is the machine in question? Maybe someone else has access to one,
> if it's not too exotic.

Shuah had access to a HP server machine (don't know which one) that
defined unity-map ranges in the BIOS table. Shuah certainly knows the
details about that machine.


	Joerg

Re: [PATCH 0/9] AMD IOMMU cleanups, fixes and IVRS bug workarounds

[Shuah Khan]

On Sat, Apr 13, 2013 at 9:26 AM, Joerg Roedel <joro@8bytes.org> wrote:
> On Sat, Apr 13, 2013 at 11:06:22PM +0800, Andrew Cooks wrote:
>> On Fri, Apr 12, 2013 at 4:06 PM, Joerg Roedel <joro@8bytes.org> wrote:
>
>> > Oh, that's sad. You were the only one having a machine wich actually has
>> > unity-mapped ranges defined in the BIOS table. The code for those
>> > mappings was basically untested before you ran it on that machine.
>> >
>> What is the machine in question? Maybe someone else has access to one,
>> if it's not too exotic.
>
> Shuah had access to a HP server machine (don't know which one) that
> defined unity-map ranges in the BIOS table. Shuah certainly knows the
> details about that machine.
>

Joerg/Andrew,

It is a DL385 Gen8 server. Unfortunately I left HP as of yesterday and no longer
have access to the system. Maybe there others that have access to one.

-- Shuah