2013-04-09 20:13:35

by Joerg Roedel

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

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


2013-04-09 20:13:36

by Joerg Roedel

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

Fixes a lockdep warning.

Cc: [email protected] # >= v3.7
Signed-off-by: Joerg Roedel <[email protected]>
---
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

2013-04-09 20:13:45

by Joerg Roedel

[permalink] [raw]
Subject: [PATCH 9/9] iommu/amd: Document ivrs_ioapic and ivrs_hpet parameters

Document the new kernel commandline parameters in the
appropriate file.

Signed-off-by: Joerg Roedel <[email protected]>
---
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

2013-04-09 20:13:58

by Joerg Roedel

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

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 <[email protected]>
---
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

2013-04-09 20:14:16

by Joerg Roedel

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

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 <[email protected]>
---
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

2013-04-09 20:14:38

by Joerg Roedel

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

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 <[email protected]>
---
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

2013-04-09 20:14:37

by Joerg Roedel

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

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 <[email protected]>
---
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

2013-04-09 20:14:36

by Joerg Roedel

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

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 <[email protected]>
---
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

2013-04-09 20:15:35

by Joerg Roedel

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

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 <[email protected]>
---
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

2013-04-09 20:15:34

by Joerg Roedel

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

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

Signed-off-by: Joerg Roedel <[email protected]>
---
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

2013-04-09 20:29:46

by Joerg Roedel

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

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 <[email protected]>

Forgot to add:

Tested-by: Borislav Petkov <[email protected]>

2013-04-10 15:59:49

by Shuah Khan

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

On Tue, Apr 9, 2013 at 2:13 PM, Joerg Roedel <[email protected]> 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 <[email protected]>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2013-04-10 16:03:07

by Shuah Khan

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

On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <[email protected]> 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 <[email protected]>
> ---

Reviewed-by: Shuah Khan <[email protected]>

2013-04-10 16:03:51

by Shuah Khan

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

On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <[email protected]> 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 <[email protected]>
> ---

Reviewed-by: Shuah Khan <[email protected]>

2013-04-10 16:06:05

by Shuah Khan

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

On Tue, Apr 9, 2013 at 2:12 PM, Joerg Roedel <[email protected]> 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 <[email protected]>

-- Shuah

2013-04-11 21:40:22

by Suthikulpanit, Suravee

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

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 <[email protected]>
> Forgot to add:
>
> Tested-by: Borislav Petkov <[email protected]>
I also tested on some other systems here and the patch works also.

Suravee
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>

2013-04-12 08:04:56

by Joerg Roedel

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

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 <[email protected]>
> >Forgot to add:
> >
> >Tested-by: Borislav Petkov <[email protected]>
> 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 <[email protected]>


Joerg

2013-04-12 08:06:43

by Joerg Roedel

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

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 <[email protected]> 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 <[email protected]>

Thanks.


Joerg

2013-04-13 15:06:25

by Andrew Cooks

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

On Fri, Apr 12, 2013 at 4:06 PM, Joerg Roedel <[email protected]> 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 <[email protected]> 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.

2013-04-13 15:26:07

by Joerg Roedel

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

On Sat, Apr 13, 2013 at 11:06:22PM +0800, Andrew Cooks wrote:
> On Fri, Apr 12, 2013 at 4:06 PM, Joerg Roedel <[email protected]> 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

2013-04-13 23:14:31

by Shuah Khan

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

On Sat, Apr 13, 2013 at 9:26 AM, Joerg Roedel <[email protected]> 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 <[email protected]> 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