All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
@ 2015-12-11 22:54 ` Suravee Suthikulpanit
  0 siblings, 0 replies; 7+ messages in thread
From: Suravee Suthikulpanit @ 2015-12-11 22:54 UTC (permalink / raw)
  To: joro; +Cc: linux-kernel, iommu, Suravee Suthikulpanit

Current driver makes assumption that device with devid zero is always
included in the range of devices to be managed by IOMMU. However,
certain FW does not include devid zero in IVRS table.
This has caused IOMMU perf driver to fail to initialize.

This patch implements a workaround for this case by always assign
devid zero to be handled by the first IOMMU.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu_init.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..3bbad5c 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -656,6 +656,16 @@ static void __init set_iommu_for_device(struct amd_iommu *iommu, u16 devid)
 	amd_iommu_rlookup_table[devid] = iommu;
 }
 
+static struct amd_iommu *get_iommu_for_device(u16 devid)
+{
+	struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
+
+	/* Workaround: Always assign devid zero to the first IOMMU */
+	if (!iommu && !devid && amd_iommus_present)
+		iommu = amd_iommus[0];
+	return iommu;
+}
+
 /*
  * This function takes the device specific flags read from the ACPI
  * table and sets up the device table entry with that information
@@ -751,7 +761,7 @@ static int __init add_early_maps(void)
  */
 static void __init set_device_exclusion_range(u16 devid, struct ivmd_header *m)
 {
-	struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
+	struct amd_iommu *iommu = get_iommu_for_device(devid);
 
 	if (!(m->flags & IVMD_FLAG_EXCL_RANGE))
 		return;
@@ -2255,7 +2265,7 @@ u8 amd_iommu_pc_get_max_banks(u16 devid)
 	u8 ret = 0;
 
 	/* locate the iommu governing the devid */
-	iommu = amd_iommu_rlookup_table[devid];
+	iommu = get_iommu_for_device(devid);
 	if (iommu)
 		ret = iommu->max_banks;
 
@@ -2275,7 +2285,7 @@ u8 amd_iommu_pc_get_max_counters(u16 devid)
 	u8 ret = 0;
 
 	/* locate the iommu governing the devid */
-	iommu = amd_iommu_rlookup_table[devid];
+	iommu = get_iommu_for_device(devid);
 	if (iommu)
 		ret = iommu->max_counters;
 
@@ -2295,7 +2305,7 @@ int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
 		return -ENODEV;
 
 	/* Locate the iommu associated with the device ID */
-	iommu = amd_iommu_rlookup_table[devid];
+	iommu = get_iommu_for_device(devid);
 
 	/* Check for valid iommu and pc register indexing */
 	if (WARN_ON((iommu == NULL) || (fxn > 0x28) || (fxn & 7)))
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
@ 2015-12-11 22:54 ` Suravee Suthikulpanit
  0 siblings, 0 replies; 7+ messages in thread
From: Suravee Suthikulpanit @ 2015-12-11 22:54 UTC (permalink / raw)
  To: joro; +Cc: linux-kernel, iommu, Suravee Suthikulpanit

Current driver makes assumption that device with devid zero is always
included in the range of devices to be managed by IOMMU. However,
certain FW does not include devid zero in IVRS table.
This has caused IOMMU perf driver to fail to initialize.

This patch implements a workaround for this case by always assign
devid zero to be handled by the first IOMMU.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
---
 drivers/iommu/amd_iommu_init.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 013bdff..3bbad5c 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -656,6 +656,16 @@ static void __init set_iommu_for_device(struct amd_iommu *iommu, u16 devid)
 	amd_iommu_rlookup_table[devid] = iommu;
 }
 
+static struct amd_iommu *get_iommu_for_device(u16 devid)
+{
+	struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
+
+	/* Workaround: Always assign devid zero to the first IOMMU */
+	if (!iommu && !devid && amd_iommus_present)
+		iommu = amd_iommus[0];
+	return iommu;
+}
+
 /*
  * This function takes the device specific flags read from the ACPI
  * table and sets up the device table entry with that information
@@ -751,7 +761,7 @@ static int __init add_early_maps(void)
  */
 static void __init set_device_exclusion_range(u16 devid, struct ivmd_header *m)
 {
-	struct amd_iommu *iommu = amd_iommu_rlookup_table[devid];
+	struct amd_iommu *iommu = get_iommu_for_device(devid);
 
 	if (!(m->flags & IVMD_FLAG_EXCL_RANGE))
 		return;
@@ -2255,7 +2265,7 @@ u8 amd_iommu_pc_get_max_banks(u16 devid)
 	u8 ret = 0;
 
 	/* locate the iommu governing the devid */
-	iommu = amd_iommu_rlookup_table[devid];
+	iommu = get_iommu_for_device(devid);
 	if (iommu)
 		ret = iommu->max_banks;
 
@@ -2275,7 +2285,7 @@ u8 amd_iommu_pc_get_max_counters(u16 devid)
 	u8 ret = 0;
 
 	/* locate the iommu governing the devid */
-	iommu = amd_iommu_rlookup_table[devid];
+	iommu = get_iommu_for_device(devid);
 	if (iommu)
 		ret = iommu->max_counters;
 
@@ -2295,7 +2305,7 @@ int amd_iommu_pc_get_set_reg_val(u16 devid, u8 bank, u8 cntr, u8 fxn,
 		return -ENODEV;
 
 	/* Locate the iommu associated with the device ID */
-	iommu = amd_iommu_rlookup_table[devid];
+	iommu = get_iommu_for_device(devid);
 
 	/* Check for valid iommu and pc register indexing */
 	if (WARN_ON((iommu == NULL) || (fxn > 0x28) || (fxn & 7)))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
  2015-12-11 22:54 ` Suravee Suthikulpanit
  (?)
@ 2015-12-14 15:08 ` Joerg Roedel
  2015-12-16  6:20     ` Suravee Suthikulpanit
  2015-12-17  3:17     ` Suravee Suthikulanit
  -1 siblings, 2 replies; 7+ messages in thread
From: Joerg Roedel @ 2015-12-14 15:08 UTC (permalink / raw)
  To: Suravee Suthikulpanit; +Cc: linux-kernel, iommu

On Fri, Dec 11, 2015 at 04:54:38PM -0600, Suravee Suthikulpanit wrote:
> Current driver makes assumption that device with devid zero is always
> included in the range of devices to be managed by IOMMU. However,
> certain FW does not include devid zero in IVRS table.
> This has caused IOMMU perf driver to fail to initialize.

Hmm, this is a firmware bug. Is this bug seen in any systems that are
for sale?

> This patch implements a workaround for this case by always assign
> devid zero to be handled by the first IOMMU.

Otherwise its better to fix the firmware than to add workarounds.


	Joerg


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
  2015-12-14 15:08 ` Joerg Roedel
@ 2015-12-16  6:20     ` Suravee Suthikulpanit
  2015-12-17  3:17     ` Suravee Suthikulanit
  1 sibling, 0 replies; 7+ messages in thread
From: Suravee Suthikulpanit @ 2015-12-16  6:20 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-kernel, iommu

Hi Joerg,

On 12/14/2015 09:08 AM, Joerg Roedel wrote:
> On Fri, Dec 11, 2015 at 04:54:38PM -0600, Suravee Suthikulpanit wrote:
>> Current driver makes assumption that device with devid zero is always
>> included in the range of devices to be managed by IOMMU. However,
>> certain FW does not include devid zero in IVRS table.
>> This has caused IOMMU perf driver to fail to initialize.
>
> Hmm, this is a firmware bug. Is this bug seen in any systems that are
> for sale?
>
>> This patch implements a workaround for this case by always assign
>> devid zero to be handled by the first IOMMU.
>
> Otherwise its better to fix the firmware than to add workarounds.
>
>
> 	Joerg
>

Please correct me if I am wrong. But I don't think this is necessary a 
bug in the FW. From the CZ system, here are the IVHD device entry dump 
that belong to this IOMMU:

[    0.070351] AMD-Vi: device: 00:00.2 cap: 0040 seg: 0 flags: b8 info 0000
[    0.070354] AMD-Vi:        mmio-addr: 00000000feb80000
[    0.070384] AMD-Vi:   DEV_SELECT_RANGE_START     devid: 00:01.0 flags: 00
[    0.070386] AMD-Vi:   DEV_RANGE_END         devid: ff:1f.6
[    0.071414] AMD-Vi:   DEV_ALIAS_RANGE         devid: ff:00.0 flags: 
00 devid_to: 00:14.4
[    0.071417] AMD-Vi:   DEV_RANGE_END         devid: ff:1f.7
[    0.071423] AMD-Vi:   DEV_SPECIAL(HPET[0])        devid: 00:14.0
[    0.071426] AMD-Vi:   DEV_SPECIAL(IOAPIC[0])        devid: 00:14.0
[    0.071427] AMD-Vi:   DEV_SPECIAL(IOAPIC[1])        devid: 00:00.1

And here is the output from lspci:

00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1576
00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Device 1577
00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. 
[AMD/ATI] Device 9874 (rev c4)
00:01.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Device 9840
00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 157b
00:02.4 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 157c
00:03.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 157b
00:03.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 157c
00:08.0 Encryption controller: Advanced Micro Devices, Inc. [AMD] Device 
1578
00:09.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 157d
00:09.2 Audio device: Advanced Micro Devices, Inc. [AMD] Device 157a
00:10.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI 
Controller (rev 20)
00:11.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA 
Controller [AHCI mode] (rev 49)
00:12.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB EHCI 
Controller (rev 49)
00:14.0 SMBus: Advanced Micro Devices, Inc. [AMD] FCH SMBus Controller 
(rev 4a)
00:14.3 ISA bridge: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge 
(rev 11)
00:14.7 SD Host controller: Advanced Micro Devices, Inc. [AMD] FCH SD 
Flash Controller (rev 01)
00:18.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1570
00:18.1 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1571
00:18.2 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1572
00:18.3 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1573
00:18.4 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1574
00:18.5 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1575
01:00.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5720 
Gigabit Ethernet PCIe
01:00.1 Ethernet controller: Broadcom Corporation NetXtreme BCM5720 
Gigabit Ethernet PCIe
02:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)
02:00.1 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)

The IVHD seems valid. We should not need to include from devid 0 if it 
has already specified the IOAPIC[1] separately.

Thanks,
Suravee

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
@ 2015-12-16  6:20     ` Suravee Suthikulpanit
  0 siblings, 0 replies; 7+ messages in thread
From: Suravee Suthikulpanit @ 2015-12-16  6:20 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-kernel, iommu

Hi Joerg,

On 12/14/2015 09:08 AM, Joerg Roedel wrote:
> On Fri, Dec 11, 2015 at 04:54:38PM -0600, Suravee Suthikulpanit wrote:
>> Current driver makes assumption that device with devid zero is always
>> included in the range of devices to be managed by IOMMU. However,
>> certain FW does not include devid zero in IVRS table.
>> This has caused IOMMU perf driver to fail to initialize.
>
> Hmm, this is a firmware bug. Is this bug seen in any systems that are
> for sale?
>
>> This patch implements a workaround for this case by always assign
>> devid zero to be handled by the first IOMMU.
>
> Otherwise its better to fix the firmware than to add workarounds.
>
>
> 	Joerg
>

Please correct me if I am wrong. But I don't think this is necessary a 
bug in the FW. From the CZ system, here are the IVHD device entry dump 
that belong to this IOMMU:

[    0.070351] AMD-Vi: device: 00:00.2 cap: 0040 seg: 0 flags: b8 info 0000
[    0.070354] AMD-Vi:        mmio-addr: 00000000feb80000
[    0.070384] AMD-Vi:   DEV_SELECT_RANGE_START     devid: 00:01.0 flags: 00
[    0.070386] AMD-Vi:   DEV_RANGE_END         devid: ff:1f.6
[    0.071414] AMD-Vi:   DEV_ALIAS_RANGE         devid: ff:00.0 flags: 
00 devid_to: 00:14.4
[    0.071417] AMD-Vi:   DEV_RANGE_END         devid: ff:1f.7
[    0.071423] AMD-Vi:   DEV_SPECIAL(HPET[0])        devid: 00:14.0
[    0.071426] AMD-Vi:   DEV_SPECIAL(IOAPIC[0])        devid: 00:14.0
[    0.071427] AMD-Vi:   DEV_SPECIAL(IOAPIC[1])        devid: 00:00.1

And here is the output from lspci:

00:00.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1576
00:00.2 IOMMU: Advanced Micro Devices, Inc. [AMD] Device 1577
00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. 
[AMD/ATI] Device 9874 (rev c4)
00:01.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Device 9840
00:02.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 157b
00:02.4 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 157c
00:03.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 157b
00:03.1 PCI bridge: Advanced Micro Devices, Inc. [AMD] Device 157c
00:08.0 Encryption controller: Advanced Micro Devices, Inc. [AMD] Device 
1578
00:09.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 157d
00:09.2 Audio device: Advanced Micro Devices, Inc. [AMD] Device 157a
00:10.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB XHCI 
Controller (rev 20)
00:11.0 SATA controller: Advanced Micro Devices, Inc. [AMD] FCH SATA 
Controller [AHCI mode] (rev 49)
00:12.0 USB controller: Advanced Micro Devices, Inc. [AMD] FCH USB EHCI 
Controller (rev 49)
00:14.0 SMBus: Advanced Micro Devices, Inc. [AMD] FCH SMBus Controller 
(rev 4a)
00:14.3 ISA bridge: Advanced Micro Devices, Inc. [AMD] FCH LPC Bridge 
(rev 11)
00:14.7 SD Host controller: Advanced Micro Devices, Inc. [AMD] FCH SD 
Flash Controller (rev 01)
00:18.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1570
00:18.1 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1571
00:18.2 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1572
00:18.3 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1573
00:18.4 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1574
00:18.5 Host bridge: Advanced Micro Devices, Inc. [AMD] Device 1575
01:00.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5720 
Gigabit Ethernet PCIe
01:00.1 Ethernet controller: Broadcom Corporation NetXtreme BCM5720 
Gigabit Ethernet PCIe
02:00.0 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)
02:00.1 Ethernet controller: Intel Corporation 82599ES 10-Gigabit 
SFI/SFP+ Network Connection (rev 01)

The IVHD seems valid. We should not need to include from devid 0 if it 
has already specified the IOAPIC[1] separately.

Thanks,
Suravee

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
  2015-12-14 15:08 ` Joerg Roedel
@ 2015-12-17  3:17     ` Suravee Suthikulanit
  2015-12-17  3:17     ` Suravee Suthikulanit
  1 sibling, 0 replies; 7+ messages in thread
From: Suravee Suthikulanit @ 2015-12-17  3:17 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-kernel, iommu

On 12/14/2015 9:08 AM, Joerg Roedel wrote:
> On Fri, Dec 11, 2015 at 04:54:38PM -0600, Suravee Suthikulpanit wrote:
>> Current driver makes assumption that device with devid zero is always
>> included in the range of devices to be managed by IOMMU. However,
>> certain FW does not include devid zero in IVRS table.
>> This has caused IOMMU perf driver to fail to initialize.
>
> Hmm, this is a firmware bug. Is this bug seen in any systems that are
> for sale?
>
>> This patch implements a workaround for this case by always assign
>> devid zero to be handled by the first IOMMU.
>
> Otherwise its better to fix the firmware than to add workarounds.
>
>
> 	Joerg
>

Please ignore this patch. There are more stuff that I am planning to 
fix, and I am reworking them into V2. I'll send this out soon.

Suravee

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU
@ 2015-12-17  3:17     ` Suravee Suthikulanit
  0 siblings, 0 replies; 7+ messages in thread
From: Suravee Suthikulanit @ 2015-12-17  3:17 UTC (permalink / raw)
  To: Joerg Roedel; +Cc: linux-kernel, iommu

On 12/14/2015 9:08 AM, Joerg Roedel wrote:
> On Fri, Dec 11, 2015 at 04:54:38PM -0600, Suravee Suthikulpanit wrote:
>> Current driver makes assumption that device with devid zero is always
>> included in the range of devices to be managed by IOMMU. However,
>> certain FW does not include devid zero in IVRS table.
>> This has caused IOMMU perf driver to fail to initialize.
>
> Hmm, this is a firmware bug. Is this bug seen in any systems that are
> for sale?
>
>> This patch implements a workaround for this case by always assign
>> devid zero to be handled by the first IOMMU.
>
> Otherwise its better to fix the firmware than to add workarounds.
>
>
> 	Joerg
>

Please ignore this patch. There are more stuff that I am planning to 
fix, and I am reworking them into V2. I'll send this out soon.

Suravee

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2015-12-17  3:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-11 22:54 [PATCH] iommu/amd: Assign default IOMMU when there is only one IOMMU Suravee Suthikulpanit
2015-12-11 22:54 ` Suravee Suthikulpanit
2015-12-14 15:08 ` Joerg Roedel
2015-12-16  6:20   ` Suravee Suthikulpanit
2015-12-16  6:20     ` Suravee Suthikulpanit
2015-12-17  3:17   ` Suravee Suthikulanit
2015-12-17  3:17     ` Suravee Suthikulanit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.