[PATCH] iommu/amd: Override wrong IVRS IOAPIC on Raven Ridge systems
Raven Ridge systems may have malfunction touchpad or hang at boot if
incorrect IVRS IOAPIC is provided by BIOS.
Users already found correct "ivrs_ioapic=" values, let's put them inside
kernel to workaround buggy BIOS.
BugLink: https://bugs.launchpad.net/bugs/1795292
BugLink: https://bugs.launchpad.net/bugs/1837688
Signed-off-by: Kai-Heng Feng
---
drivers/iommu/amd_iommu_init.c | 75 ++
1 file changed, 75 insertions(+)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 4413aa67000e..06fd008281e5 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -21,6 +21,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1109,6 +1110,78 @@ static int __init add_early_maps(void)
return 0;
}
+struct quirk_entry {
+ u8 id;
+ u16 devid;
+};
+
+enum {
+ DELL_INSPIRON_7375 = 0,
+ DELL_LATITUDE_5495,
+ LENOVO_IDEAPAD_330S_15ARR,
+};
+
+static const struct quirk_entry ivrs_ioapic_quirks[][3] __initconst = {
+ /* ivrs_ioapic[4]=00:14.0 ivrs_ioapic[5]=00:00.2 */
+ [DELL_INSPIRON_7375] = {
+ { .id = 4, .devid = 0xa0 },
+ { .id = 5, .devid = 0x2 },
+ {}
+ },
+ /* ivrs_ioapic[4]=00:14.0 */
+ [DELL_LATITUDE_5495] = {
+ { .id = 4, .devid = 0xa0 },
+ {}
+ },
+ /* ivrs_ioapic[32]=00:14.0 */
+ [LENOVO_IDEAPAD_330S_15ARR] = {
+ { .id = 32, .devid = 0xa0 },
+ {}
+ },
+ {}
+};
+
+static int __init ivrs_ioapic_quirk_cb(const struct dmi_system_id *d)
+{
+ const struct quirk_entry *i;
+
+ for (i = d->driver_data; i->id != 0 && i->devid != 0; i++)
+ add_special_device(IVHD_SPECIAL_IOAPIC, i->id, &i->devid, 0);
+
+ return 0;
+}
+
+static const struct dmi_system_id ivrs_quirks[] __initconst = {
+ {
+ .callback = ivrs_ioapic_quirk_cb,
+ .ident = "Dell Inspiron 7375",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7375"),
+ },
+ .driver_data = (void *)&ivrs_ioapic_quirks[DELL_INSPIRON_7375],
+ },
+ {
+ .callback = ivrs_ioapic_quirk_cb,
+ .ident = "Dell Latitude 5495",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 5495"),
+ },
+ .driver_data = (void *)&ivrs_ioapic_quirks[DELL_LATITUDE_5495],
+ },
+ {
+ .callback = ivrs_ioapic_quirk_cb,
+ .ident = "Lenovo ideapad 330S-15ARR",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_MATCH(DMI_PRODUCT_NAME, "81FB"),
+ },
+ .driver_data = (void
*)&ivrs_ioapic_quirks[LENOVO_IDEAPAD_330S_15ARR],
+ },
+ {}
+};
+
/*
* Reads the device exclusion range from ACPI and initializes the IOMMU with
* it
@@ -1153,6 +1226,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu
*iommu,
if (ret)
return ret;
+ dmi_check_system(ivrs_quirks);
+
/*
* First save the recommended feature enable bits from ACPI
*/
--
2.17.1
Re: [PATCH] iommu/amd: Override wrong IVRS IOAPIC on Raven Ridge systems
at 23:39, Joerg Roedel wrote: On Thu, Aug 08, 2019 at 06:17:07PM +0800, Kai-Heng Feng wrote: Raven Ridge systems may have malfunction touchpad or hang at boot if incorrect IVRS IOAPIC is provided by BIOS. Users already found correct "ivrs_ioapic=" values, let's put them inside kernel to workaround buggy BIOS. Will that still work when a fixed BIOS for these laptops is released? Do you mean that we should stop applying these quirks once a BIOS fix is confirmed? We can modify the quirk to compare BIOS version, if there’s an unlikely BIOS update really fixes the issue. Before that happens, I think it’s OK to let the quirks stay this way. Kai-Heng Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/amd: Override wrong IVRS IOAPIC on Raven Ridge systems
Raven Ridge systems may have malfunction touchpad or hang at boot if
incorrect IVRS IOAPIC is provided by BIOS.
Users already found correct "ivrs_ioapic=" values, let's put them inside
kernel to workaround buggy BIOS.
BugLink: https://bugs.launchpad.net/bugs/1795292
BugLink: https://bugs.launchpad.net/bugs/1837688
Signed-off-by: Kai-Heng Feng
---
v2:
Split the quirk to another file.
drivers/iommu/Makefile | 2 +-
drivers/iommu/amd_iommu.h| 14 +
drivers/iommu/amd_iommu_init.c | 5 +-
drivers/iommu/amd_iommu_quirks.c | 90
4 files changed, 109 insertions(+), 2 deletions(-)
create mode 100644 drivers/iommu/amd_iommu.h
create mode 100644 drivers/iommu/amd_iommu_quirks.c
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index f13f36ae1af6..c6a277e69848 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
obj-$(CONFIG_IOMMU_IOVA) += iova.o
obj-$(CONFIG_OF_IOMMU) += of_iommu.o
obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
-obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
+obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_quirks.o
obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += amd_iommu_debugfs.o
obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
diff --git a/drivers/iommu/amd_iommu.h b/drivers/iommu/amd_iommu.h
new file mode 100644
index ..12d540d9b59b
--- /dev/null
+++ b/drivers/iommu/amd_iommu.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef AMD_IOMMU_H
+#define AMD_IOMMU_H
+
+int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line);
+
+#ifdef CONFIG_DMI
+void amd_iommu_apply_ivrs_quirks(void);
+#else
+static void amd_iommu_apply_ivrs_quirks(void) { }
+#endif
+
+#endif
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 4413aa67000e..568c52317757 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -32,6 +32,7 @@
#include
#include
+#include "amd_iommu.h"
#include "amd_iommu_proto.h"
#include "amd_iommu_types.h"
#include "irq_remapping.h"
@@ -1002,7 +1003,7 @@ static void __init set_dev_entry_from_acpi(struct
amd_iommu *iommu,
set_iommu_for_device(iommu, devid);
}
-static int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
+int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
{
struct devid_map *entry;
struct list_head *list;
@@ -1153,6 +1154,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu
*iommu,
if (ret)
return ret;
+ amd_iommu_apply_ivrs_quirks();
+
/*
* First save the recommended feature enable bits from ACPI
*/
diff --git a/drivers/iommu/amd_iommu_quirks.c b/drivers/iommu/amd_iommu_quirks.c
new file mode 100644
index ..14181f0f5c2a
--- /dev/null
+++ b/drivers/iommu/amd_iommu_quirks.c
@@ -0,0 +1,90 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/*
+ * Quirks for AMD IOMMU
+ *
+ * Copyright (C) 2019 Kai-Heng Feng
+ */
+
+#include
+
+#include "amd_iommu.h"
+
+#define IVHD_SPECIAL_IOAPIC1
+
+struct ivrs_quirk_entry {
+ u8 id;
+ u16 devid;
+};
+
+enum {
+ DELL_INSPIRON_7375 = 0,
+ DELL_LATITUDE_5495,
+ LENOVO_IDEAPAD_330S_15ARR,
+};
+
+static const struct ivrs_quirk_entry ivrs_ioapic_quirks[][3] __initconst = {
+ /* ivrs_ioapic[4]=00:14.0 ivrs_ioapic[5]=00:00.2 */
+ [DELL_INSPIRON_7375] = {
+ { .id = 4, .devid = 0xa0 },
+ { .id = 5, .devid = 0x2 },
+ {}
+ },
+ /* ivrs_ioapic[4]=00:14.0 */
+ [DELL_LATITUDE_5495] = {
+ { .id = 4, .devid = 0xa0 },
+ {}
+ },
+ /* ivrs_ioapic[32]=00:14.0 */
+ [LENOVO_IDEAPAD_330S_15ARR] = {
+ { .id = 32, .devid = 0xa0 },
+ {}
+ },
+ {}
+};
+
+static int __init ivrs_ioapic_quirk_cb(const struct dmi_system_id *d)
+{
+ const struct ivrs_quirk_entry *i;
+
+ for (i = d->driver_data; i->id != 0 && i->devid != 0; i++)
+ add_special_device(IVHD_SPECIAL_IOAPIC, i->id, (u16
*)&i->devid, 0);
+
+ return 0;
+}
+
+static const struct dmi_system_id ivrs_quirks[] __initconst = {
+ {
+ .callback = ivrs_ioapic_quirk_cb,
+ .ident = "Dell Inspiron 7375",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7375"),
+ },
+ .driver_data = (void *)&ivrs_ioapic_quirks[DELL_INSPIRON_7375],
+ },
+ {
+ .callback = ivrs_ioapic_quirk_cb,
+ .ident = "Dell Latitude 54
[PATCH v3] iommu/amd: Override wrong IVRS IOAPIC on Raven Ridge systems
Raven Ridge systems may have malfunction touchpad or hang at boot if
incorrect IVRS IOAPIC is provided by BIOS.
Users already found correct "ivrs_ioapic=" values, let's put them inside
kernel to workaround buggy BIOS.
BugLink: https://bugs.launchpad.net/bugs/1795292
BugLink: https://bugs.launchpad.net/bugs/1837688
Reported-by: kbuild test robot
Signed-off-by: Kai-Heng Feng
---
v3:
Fix compilation error when CONFIG_DMI is not enabled.
v2:
Split the quirk to another file.
drivers/iommu/Makefile | 2 +-
drivers/iommu/amd_iommu.h| 14 +
drivers/iommu/amd_iommu_init.c | 5 +-
drivers/iommu/amd_iommu_quirks.c | 92
4 files changed, 111 insertions(+), 2 deletions(-)
create mode 100644 drivers/iommu/amd_iommu.h
create mode 100644 drivers/iommu/amd_iommu_quirks.c
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index f13f36ae1af6..c6a277e69848 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_IOMMU_IO_PGTABLE_LPAE) += io-pgtable-arm.o
obj-$(CONFIG_IOMMU_IOVA) += iova.o
obj-$(CONFIG_OF_IOMMU) += of_iommu.o
obj-$(CONFIG_MSM_IOMMU) += msm_iommu.o
-obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o
+obj-$(CONFIG_AMD_IOMMU) += amd_iommu.o amd_iommu_init.o amd_iommu_quirks.o
obj-$(CONFIG_AMD_IOMMU_DEBUGFS) += amd_iommu_debugfs.o
obj-$(CONFIG_AMD_IOMMU_V2) += amd_iommu_v2.o
obj-$(CONFIG_ARM_SMMU) += arm-smmu.o
diff --git a/drivers/iommu/amd_iommu.h b/drivers/iommu/amd_iommu.h
new file mode 100644
index ..12d540d9b59b
--- /dev/null
+++ b/drivers/iommu/amd_iommu.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef AMD_IOMMU_H
+#define AMD_IOMMU_H
+
+int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line);
+
+#ifdef CONFIG_DMI
+void amd_iommu_apply_ivrs_quirks(void);
+#else
+static void amd_iommu_apply_ivrs_quirks(void) { }
+#endif
+
+#endif
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 4413aa67000e..568c52317757 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -32,6 +32,7 @@
#include
#include
+#include "amd_iommu.h"
#include "amd_iommu_proto.h"
#include "amd_iommu_types.h"
#include "irq_remapping.h"
@@ -1002,7 +1003,7 @@ static void __init set_dev_entry_from_acpi(struct
amd_iommu *iommu,
set_iommu_for_device(iommu, devid);
}
-static int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
+int __init add_special_device(u8 type, u8 id, u16 *devid, bool cmd_line)
{
struct devid_map *entry;
struct list_head *list;
@@ -1153,6 +1154,8 @@ static int __init init_iommu_from_acpi(struct amd_iommu
*iommu,
if (ret)
return ret;
+ amd_iommu_apply_ivrs_quirks();
+
/*
* First save the recommended feature enable bits from ACPI
*/
diff --git a/drivers/iommu/amd_iommu_quirks.c b/drivers/iommu/amd_iommu_quirks.c
new file mode 100644
index ..c235f79b7a20
--- /dev/null
+++ b/drivers/iommu/amd_iommu_quirks.c
@@ -0,0 +1,92 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+/*
+ * Quirks for AMD IOMMU
+ *
+ * Copyright (C) 2019 Kai-Heng Feng
+ */
+
+#ifdef CONFIG_DMI
+#include
+
+#include "amd_iommu.h"
+
+#define IVHD_SPECIAL_IOAPIC1
+
+struct ivrs_quirk_entry {
+ u8 id;
+ u16 devid;
+};
+
+enum {
+ DELL_INSPIRON_7375 = 0,
+ DELL_LATITUDE_5495,
+ LENOVO_IDEAPAD_330S_15ARR,
+};
+
+static const struct ivrs_quirk_entry ivrs_ioapic_quirks[][3] __initconst = {
+ /* ivrs_ioapic[4]=00:14.0 ivrs_ioapic[5]=00:00.2 */
+ [DELL_INSPIRON_7375] = {
+ { .id = 4, .devid = 0xa0 },
+ { .id = 5, .devid = 0x2 },
+ {}
+ },
+ /* ivrs_ioapic[4]=00:14.0 */
+ [DELL_LATITUDE_5495] = {
+ { .id = 4, .devid = 0xa0 },
+ {}
+ },
+ /* ivrs_ioapic[32]=00:14.0 */
+ [LENOVO_IDEAPAD_330S_15ARR] = {
+ { .id = 32, .devid = 0xa0 },
+ {}
+ },
+ {}
+};
+
+static int __init ivrs_ioapic_quirk_cb(const struct dmi_system_id *d)
+{
+ const struct ivrs_quirk_entry *i;
+
+ for (i = d->driver_data; i->id != 0 && i->devid != 0; i++)
+ add_special_device(IVHD_SPECIAL_IOAPIC, i->id, (u16
*)&i->devid, 0);
+
+ return 0;
+}
+
+static const struct dmi_system_id ivrs_quirks[] __initconst = {
+ {
+ .callback = ivrs_ioapic_quirk_cb,
+ .ident = "Dell Inspiron 7375",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+ DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7375"),
+ },
+ .driver_data = (void *)&ivrs_ioapic_quirks[DELL_INSPIRON_7375],
+ },
+
[Regression] "iommu/amd: Relax locking in dma_ops path" makes tg3 ethernet transmit queue timeout
Hi,
Broadcom ethernet tg3 unusable after commit 92d420ec028d ("iommu/amd: Relax
locking in dma_ops path").
After a short period it stops:
[ 122.717144] WARNING: CPU: 0 PID: 0 at net/sched/sch_generic.c:303
dev_watchdog+0x237/0x240()
[ 122.717152] NETDEV WATCHDOG: enp3s0 (tg3): transmit queue 0 timed out
After testing the patch section by section, this is the part that caused the
regression:
@@ -2578,19 +2580,8 @@ static dma_addr_t map_page(struct device *dev, struct
page *page,
dma_mask = *dev->dma_mask;
- spin_lock_irqsave(&domain->lock, flags);
-
- addr = __map_single(dev, domain->priv, paddr, size, dir, false,
+ return __map_single(dev, domain->priv, paddr, size, dir, false,
dma_mask);
- if (addr == DMA_ERROR_CODE)
- goto out;
-
- domain_flush_complete(domain);
-
-out:
- spin_unlock_irqrestore(&domain->lock, flags);
-
- return addr;
}
Particularly, as soon as the spinlock is removed, the issue can be reproduced.
Function domain_flush_complete() doesn't seem to affect the status.
However, the .map_page callback was removed by be62dbf554c5 ("iommu/amd:
Convert AMD iommu driver to the dma-iommu api"), so there's no easy revert for
this issue.
This is still reproducible as of today's mainline kernel, v5.7-rc6.
Kai-Heng
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Regression] "iommu/amd: Relax locking in dma_ops path" makes tg3 ethernet transmit queue timeout
> On May 18, 2020, at 21:32, Joerg Roedel wrote:
>
> On Mon, May 18, 2020 at 05:06:45PM +0800, Kai-Heng Feng wrote:
>> Particularly, as soon as the spinlock is removed, the issue can be
>> reproduced.
>> Function domain_flush_complete() doesn't seem to affect the status.
>>
>> However, the .map_page callback was removed by be62dbf554c5
>> ("iommu/amd: Convert AMD iommu driver to the dma-iommu api"), so
>> there's no easy revert for this issue.
>>
>> This is still reproducible as of today's mainline kernel, v5.7-rc6.
>
> Is there any error message from the IOMMU driver?
>
As of mainline kernel, there's no error message from IOMMU driver.
There are some complains from v4.15-rc1:
https://pastebin.ubuntu.com/p/qn4TXkFxsc/
Kai-Heng
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Regression] "iommu/amd: Relax locking in dma_ops path" makes tg3 ethernet transmit queue timeout
> On May 18, 2020, at 22:05, Kai-Heng Feng wrote:
>
>
>
>> On May 18, 2020, at 21:32, Joerg Roedel wrote:
>>
>> On Mon, May 18, 2020 at 05:06:45PM +0800, Kai-Heng Feng wrote:
>>> Particularly, as soon as the spinlock is removed, the issue can be
>>> reproduced.
>>> Function domain_flush_complete() doesn't seem to affect the status.
>>>
>>> However, the .map_page callback was removed by be62dbf554c5
>>> ("iommu/amd: Convert AMD iommu driver to the dma-iommu api"), so
>>> there's no easy revert for this issue.
>>>
>>> This is still reproducible as of today's mainline kernel, v5.7-rc6.
>>
>> Is there any error message from the IOMMU driver?
>>
>
> As of mainline kernel, there's no error message from IOMMU driver.
> There are some complains from v4.15-rc1:
> https://pastebin.ubuntu.com/p/qn4TXkFxsc/
Just tested v5.7-rc6, the issue disappears as soon as kernel boots with
"iommu=off".
Kai-Heng
>
> Kai-Heng
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Regression] "iommu/amd: Relax locking in dma_ops path" makes tg3 ethernet transmit queue timeout
> On May 18, 2020, at 23:32, Kai-Heng Feng wrote:
>
>
>
>> On May 18, 2020, at 22:05, Kai-Heng Feng wrote:
>>
>>
>>
>>> On May 18, 2020, at 21:32, Joerg Roedel wrote:
>>>
>>> On Mon, May 18, 2020 at 05:06:45PM +0800, Kai-Heng Feng wrote:
>>>> Particularly, as soon as the spinlock is removed, the issue can be
>>>> reproduced.
>>>> Function domain_flush_complete() doesn't seem to affect the status.
>>>>
>>>> However, the .map_page callback was removed by be62dbf554c5
>>>> ("iommu/amd: Convert AMD iommu driver to the dma-iommu api"), so
>>>> there's no easy revert for this issue.
>>>>
>>>> This is still reproducible as of today's mainline kernel, v5.7-rc6.
>>>
>>> Is there any error message from the IOMMU driver?
>>>
>>
>> As of mainline kernel, there's no error message from IOMMU driver.
>> There are some complains from v4.15-rc1:
>> https://pastebin.ubuntu.com/p/qn4TXkFxsc/
>
> Just tested v5.7-rc6, the issue disappears as soon as kernel boots with
> "iommu=off".
I am still seeing the issue on v5.8-rc3. The issue goes away as soon as
"iommu=off" is added.
Kai-Heng
>
> Kai-Heng
>
>>
>> Kai-Heng
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[BUG] "Pre-boot DMA Protection" makes AMDGPU stop working
Hi, A more detailed bug report can be found at [1]. I have a AMD Renoir system that can't enter graphical session because there are many IOMMU splat. Alex suggested to disable "Pre-boot DMA Protection", I can confirm once it's disabled, AMDGPU starts working with IOMMU enabled. So raise the issue here because I have no knowledge on how to reset the IOMMU. [1] https://gitlab.freedesktop.org/drm/amd/-/issues/1204 Kai-Heng ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu: Fallback to default setting when def_domain_type() callback returns 0
Commit 28b41e2c6aeb ("iommu: Move def_domain type check for untrusted
device into core") not only moved the check for untrusted device to
IOMMU core, it also introduced a behavioral change by returning
def_domain_type() directly without checking its return value. That makes
many devices no longer use the default IOMMU setting.
So revert back to the old behavior which defaults to
iommu_def_domain_type when driver callback returns 0.
Fixes: 28b41e2c6aeb ("iommu: Move def_domain type check for untrusted device
into core")
Signed-off-by: Kai-Heng Feng
---
drivers/iommu/iommu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 5419c4b9f27a..faac4f795025 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1507,14 +1507,15 @@ EXPORT_SYMBOL_GPL(fsl_mc_device_group);
static int iommu_get_def_domain_type(struct device *dev)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
+ unsigned int type = 0;
if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
return IOMMU_DOMAIN_DMA;
if (ops->def_domain_type)
- return ops->def_domain_type(dev);
+ type = ops->def_domain_type(dev);
- return 0;
+ return (type == 0) ? iommu_def_domain_type : type;
}
static int iommu_group_alloc_default_domain(struct bus_type *bus,
--
2.31.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Fallback to default setting when def_domain_type() callback returns 0
On Tue, Jul 6, 2021 at 5:27 PM Robin Murphy wrote:
>
> On 2021-07-06 07:51, Kai-Heng Feng wrote:
> > Commit 28b41e2c6aeb ("iommu: Move def_domain type check for untrusted
> > device into core") not only moved the check for untrusted device to
> > IOMMU core, it also introduced a behavioral change by returning
> > def_domain_type() directly without checking its return value. That makes
> > many devices no longer use the default IOMMU setting.
> >
> > So revert back to the old behavior which defaults to
> > iommu_def_domain_type when driver callback returns 0.
> >
> > Fixes: 28b41e2c6aeb ("iommu: Move def_domain type check for untrusted
> > device into core")
>
> Are you sure about that? From that same commit:
>
> @@ -1507,7 +1509,7 @@ static int iommu_alloc_default_domain(struct
> iommu_group *group,
> if (group->default_domain)
> return 0;
>
> - type = iommu_get_def_domain_type(dev);
> + type = iommu_get_def_domain_type(dev) ? : iommu_def_domain_type;
>
> return iommu_group_alloc_default_domain(dev->bus, group, type);
> }
>
> AFAICS the other two callers should also handle 0 correctly. Have you
> seen a problem in practice?
Thanks for pointing out how the return value is being handled by the callers.
However, the same check is missing in probe_get_default_domain_type():
static int probe_get_default_domain_type(struct device *dev, void *data)
{
struct __group_domain_type *gtype = data;
unsigned int type = iommu_get_def_domain_type(dev);
...
}
I personally prefer the old way instead of open coding with ternary
operator, so I'll do that in v2.
In practice, this causes a kernel panic when probing Realtek WiFi.
Because of the bug, dma_ops isn't set by probe_finalize(),
dma_map_single() falls back to swiotlb which isn't set and caused a
kernel panic.
I didn't attach the panic log because the system simply is frozen at
that point so the message is not logged to the storage.
I'll see if I can find another way to collect the log and attach it in v2.
Kai-Heng
>
> Robin.
>
> > Signed-off-by: Kai-Heng Feng
> > ---
> > drivers/iommu/iommu.c | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 5419c4b9f27a..faac4f795025 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -1507,14 +1507,15 @@ EXPORT_SYMBOL_GPL(fsl_mc_device_group);
> > static int iommu_get_def_domain_type(struct device *dev)
> > {
> > const struct iommu_ops *ops = dev->bus->iommu_ops;
> > + unsigned int type = 0;
> >
> > if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
> > return IOMMU_DOMAIN_DMA;
> >
> > if (ops->def_domain_type)
> > - return ops->def_domain_type(dev);
> > + type = ops->def_domain_type(dev);
> >
> > - return 0;
> > + return (type == 0) ? iommu_def_domain_type : type;
> > }
> >
> > static int iommu_group_alloc_default_domain(struct bus_type *bus,
> >
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Fallback to default setting when def_domain_type() callback returns 0
On Wed, Jul 7, 2021 at 2:03 AM Robin Murphy wrote:
>
> On 2021-07-06 17:21, Kai-Heng Feng wrote:
> > On Tue, Jul 6, 2021 at 5:27 PM Robin Murphy wrote:
> >>
> >> On 2021-07-06 07:51, Kai-Heng Feng wrote:
> >>> Commit 28b41e2c6aeb ("iommu: Move def_domain type check for untrusted
> >>> device into core") not only moved the check for untrusted device to
> >>> IOMMU core, it also introduced a behavioral change by returning
> >>> def_domain_type() directly without checking its return value. That makes
> >>> many devices no longer use the default IOMMU setting.
> >>>
> >>> So revert back to the old behavior which defaults to
> >>> iommu_def_domain_type when driver callback returns 0.
> >>>
> >>> Fixes: 28b41e2c6aeb ("iommu: Move def_domain type check for untrusted
> >>> device into core")
> >>
> >> Are you sure about that? From that same commit:
> >>
> >> @@ -1507,7 +1509,7 @@ static int iommu_alloc_default_domain(struct
> >> iommu_group *group,
> >> if (group->default_domain)
> >> return 0;
> >>
> >> - type = iommu_get_def_domain_type(dev);
> >> + type = iommu_get_def_domain_type(dev) ? : iommu_def_domain_type;
> >>
> >> return iommu_group_alloc_default_domain(dev->bus, group, type);
> >>}
> >>
> >> AFAICS the other two callers should also handle 0 correctly. Have you
> >> seen a problem in practice?
> >
> > Thanks for pointing out how the return value is being handled by the
> > callers.
> > However, the same check is missing in probe_get_default_domain_type():
> > static int probe_get_default_domain_type(struct device *dev, void *data)
> > {
> > struct __group_domain_type *gtype = data;
> > unsigned int type = iommu_get_def_domain_type(dev);
> > ...
> > }
>
> I'm still not following - the next line right after that is "if (type)",
> which means it won't touch gtype, and if that happens for every
> iteration, probe_alloc_default_domain() subsequently hits its "if
> (!gtype.type)" condition and still ends up with iommu_def_domain_type.
> This *was* one of the other two callers I was talking about (the second
> being iommu_change_dev_def_domain()), and in fact on second look I think
> your proposed change will actually break this logic, since it's
> necessary to differentiate between a specific type being requested for
> the given device, and a "don't care" response which only implies to use
> the global default type if it's still standing after *all* the
> appropriate devices have been queried.
You are right, what I am seeing here is that the AMD GFX is using
IOMMU_DOMAIN_IDENTITY, and makes other devices in the same group,
including the Realtek WiFi, to use IOMMU_DOMAIN_IDENTITY as well.
>
> > I personally prefer the old way instead of open coding with ternary
> > operator, so I'll do that in v2.
> >
> > In practice, this causes a kernel panic when probing Realtek WiFi.
> > Because of the bug, dma_ops isn't set by probe_finalize(),
> > dma_map_single() falls back to swiotlb which isn't set and caused a
> > kernel panic.
>
> Hmm, but if that's the case, wouldn't it still be a problem anyway if
> the end result was IOMMU_DOMAIN_IDENTITY? I can't claim to fully
> understand the x86 swiotlb and no_iommu dance, but nothing really stands
> out to give me confidence that it handles the passthrough options correctly.
Yes, I think AMD IOMMU driver needs more thourough check on
static void __init amd_iommu_init_dma_ops(void)
{
swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
...
}
Since swiotlb gets disabled by the check, but the Realtek WiFi is only
capable of doing 32bit DMA, the kernel panics because
io_tlb_default_mem is NULL.
Thanks again for pointing to the right direction, this patch is indeed
incorrect.
Kai-Heng
>
> Robin.
>
> > I didn't attach the panic log because the system simply is frozen at
> > that point so the message is not logged to the storage.
> > I'll see if I can find another way to collect the log and attach it in v2.
> >
> > Kai-Heng
> >
> >>
> >> Robin.
> >>
> >>> Signed-off-by: Kai-Heng Feng
> >>> ---
> >>>drivers/iommu/iommu.c | 5 +++--
> >>>1 file changed, 3 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu
[PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
We are seeing kernel panic on rtw88 probe routine because swiotlb isn't set: [ 252.036773] rtw_8821ce :06:00.0: enabling device ( -> 0003) [ 252.037084] Kernel panic - not syncing: Can not allocate SWIOTLB buffer earlier and can't now provide you with the DMA bounce buffer [ 252.037146] CPU: 7 PID: 1174 Comm: modprobe Not tainted 5.13.0+ #39 [ 252.037175] Hardware name: HP HP ProDesk 405 G6 Small Form Factor PC/8835, BIOS S05 Ver. 02.04.00 06/03/2021 [ 252.037218] Call Trace: [ 252.037231] dump_stack_lvl+0x4a/0x5f [ 252.037251] dump_stack+0x10/0x12 [ 252.037267] panic+0x101/0x2e3 [ 252.037284] swiotlb_tbl_map_single.cold+0xc/0x73 [ 252.037305] ? __mod_lruvec_page_state+0x95/0xb0 [ 252.037329] ? kmalloc_large_node+0x8c/0xb0 [ 252.037348] ? __netdev_alloc_skb+0x44/0x160 [ 252.037370] swiotlb_map+0x61/0x240 [ 252.037387] ? __alloc_skb+0xed/0x1e0 [ 252.037404] dma_map_page_attrs+0x12c/0x1f0 [ 252.037422] ? __netdev_alloc_skb+0x44/0x160 [ 252.037443] rtw_pci_probe+0x30f/0x872 [rtw88_pci] [ 252.037467] local_pci_probe+0x48/0x80 [ 252.037487] pci_device_probe+0x105/0x1c0 [ 252.037506] really_probe+0x1fe/0x3f0 [ 252.037524] __driver_probe_device+0x109/0x180 [ 252.037545] driver_probe_device+0x23/0x90 [ 252.037564] __driver_attach+0xac/0x1b0 [ 252.037582] ? __device_attach_driver+0xe0/0xe0 [ 252.037602] bus_for_each_dev+0x7e/0xc0 [ 252.037620] driver_attach+0x1e/0x20 [ 252.037637] bus_add_driver+0x135/0x1f0 [ 252.037654] driver_register+0x95/0xf0 [ 252.037672] ? 0xc0fa [ 252.037687] __pci_register_driver+0x68/0x70 [ 252.037707] rtw_8821ce_driver_init+0x23/0x1000 [rtw88_8821ce] [ 252.037734] do_one_initcall+0x48/0x1d0 [ 252.037752] ? __cond_resched+0x1a/0x50 [ 252.037771] ? kmem_cache_alloc_trace+0x29d/0x3c0 [ 252.037792] do_init_module+0x62/0x280 [ 252.037810] load_module+0x2577/0x27c0 [ 252.037862] __do_sys_finit_module+0xbf/0x120 [ 252.037877] __x64_sys_finit_module+0x1a/0x20 [ 252.037893] do_syscall_64+0x3b/0xc0 [ 252.037907] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 252.037925] RIP: 0033:0x7ff5a2f9408d [ 252.037938] Code: 27 0d 00 0f 05 eb a9 66 0f 1f 44 00 00 f3 0f 1e fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d ab dd 0c 00 f7 d8 64 89 01 48 [ 252.037993] RSP: 002b:7fffaa89dce8 EFLAGS: 0246 ORIG_RAX: 0139 [ 252.038017] RAX: ffda RBX: 55fd4f881080 RCX: 7ff5a2f9408d [ 252.038039] RDX: RSI: 55fd4f63ec02 RDI: 0009 [ 252.038063] RBP: 0004 R08: R09: 55fd4f8885b0 [ 252.038085] R10: 0009 R11: 0246 R12: 55fd4f63ec02 [ 252.038107] R13: 55fd4f881120 R14: R15: 55fd4f88e350 [ 252.038293] Kernel Offset: 0x3060 from 0x8100 (relocation range: 0x8000-0xbfff) Because the Realtek WiFi (PCI 06:00.0) is in the same IOMMU group as AMD graphics (PCI 01:00.0), [1.326166] pci :01:00.0: Adding to iommu group 0 ... [1.326268] pci :06:00.0: Adding to iommu group 0 And the AMD graphics supports iommu v2, so the group uses intentity mapping based on the query from amd_iommu_def_domain_type(). However, the Realtek WiFi doesn't support 64bit DMA, so we need to enable swiotlb, which was disabled by amd_iommu_init_dma_ops(), to make remapping work. Cc: Robin Murphy Signed-off-by: Kai-Heng Feng --- 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 811a49a95d04..7c5111ed5c97 100644 --- a/drivers/iommu/amd/iommu.c +++ b/drivers/iommu/amd/iommu.c @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) iommu = amd_iommu_rlookup_table[dev_data->devid]; dev_data->iommu_v2 = iommu->is_iommu_v2; + + if (dev_data->iommu_v2) + swiotlb = 1; } dev_iommu_priv_set(dev, dev_data); -- 2.31.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Thu, Jul 8, 2021 at 6:18 PM Robin Murphy wrote: > > On 2021-07-08 10:28, Joerg Roedel wrote: > > On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: > >> @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) > >> > >> iommu = amd_iommu_rlookup_table[dev_data->devid]; > >> dev_data->iommu_v2 = iommu->is_iommu_v2; > >> + > >> +if (dev_data->iommu_v2) > >> +swiotlb = 1; > > > > This looks like the big hammer, as it will affect all other systems > > where the AMD GPUs are in their own group. > > > > What is needed here is an explicit check whether a non-iommu-v2 device > > is direct-mapped because it shares a group with the GPU, and only enable > > swiotlb in this case. > > Right, it's basically about whether any DMA-limited device might at any > time end up in an IOMMU_DOMAIN_IDENTITY domain. And given the > possibility of device hotplug and the user being silly with the sysfs > interface, I don't think we can categorically determine that at boot time. > > Also note that Intel systems are likely to be similarly affected (in > fact intel-iommu doesn't even have the iommu_default_passthough() check > so it's probably even easier to blow up). swiotlb is enabled by pci_swiotlb_detect_4gb() and intel-iommu doesn't disable it. I wonder if we can take the same approach in amd-iommu? Kai-Heng > > Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/amd: Keep swiotlb enabled to ensure devices with 32bit DMA still work
We are seeing kernel panic on rtw88 probe routine because swiotlb isn't
set:
[ 252.036773] rtw_8821ce :06:00.0: enabling device ( -> 0003)
[ 252.037084] Kernel panic - not syncing: Can not allocate SWIOTLB buffer
earlier and can't now provide you with the DMA bounce buffer
[ 252.037146] CPU: 7 PID: 1174 Comm: modprobe Not tainted 5.13.0+ #39
[ 252.037175] Hardware name: HP HP ProDesk 405 G6 Small Form Factor PC/8835,
BIOS S05 Ver. 02.04.00 06/03/2021
[ 252.037218] Call Trace:
[ 252.037231] dump_stack_lvl+0x4a/0x5f
[ 252.037251] dump_stack+0x10/0x12
[ 252.037267] panic+0x101/0x2e3
[ 252.037284] swiotlb_tbl_map_single.cold+0xc/0x73
[ 252.037305] ? __mod_lruvec_page_state+0x95/0xb0
[ 252.037329] ? kmalloc_large_node+0x8c/0xb0
[ 252.037348] ? __netdev_alloc_skb+0x44/0x160
[ 252.037370] swiotlb_map+0x61/0x240
[ 252.037387] ? __alloc_skb+0xed/0x1e0
[ 252.037404] dma_map_page_attrs+0x12c/0x1f0
[ 252.037422] ? __netdev_alloc_skb+0x44/0x160
[ 252.037443] rtw_pci_probe+0x30f/0x872 [rtw88_pci]
[ 252.037467] local_pci_probe+0x48/0x80
[ 252.037487] pci_device_probe+0x105/0x1c0
[ 252.037506] really_probe+0x1fe/0x3f0
[ 252.037524] __driver_probe_device+0x109/0x180
[ 252.037545] driver_probe_device+0x23/0x90
[ 252.037564] __driver_attach+0xac/0x1b0
[ 252.037582] ? __device_attach_driver+0xe0/0xe0
[ 252.037602] bus_for_each_dev+0x7e/0xc0
[ 252.037620] driver_attach+0x1e/0x20
[ 252.037637] bus_add_driver+0x135/0x1f0
[ 252.037654] driver_register+0x95/0xf0
[ 252.037672] ? 0xc0fa
[ 252.037687] __pci_register_driver+0x68/0x70
[ 252.037707] rtw_8821ce_driver_init+0x23/0x1000 [rtw88_8821ce]
[ 252.037734] do_one_initcall+0x48/0x1d0
[ 252.037752] ? __cond_resched+0x1a/0x50
[ 252.037771] ? kmem_cache_alloc_trace+0x29d/0x3c0
[ 252.037792] do_init_module+0x62/0x280
[ 252.037810] load_module+0x2577/0x27c0
[ 252.037862] __do_sys_finit_module+0xbf/0x120
[ 252.037877] __x64_sys_finit_module+0x1a/0x20
[ 252.037893] do_syscall_64+0x3b/0xc0
[ 252.037907] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 252.037925] RIP: 0033:0x7ff5a2f9408d
[ 252.037938] Code: 27 0d 00 0f 05 eb a9 66 0f 1f 44 00 00 f3 0f 1e fa 48 89
f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01
f0 ff ff 73 01 c3 48 8b 0d ab dd 0c 00 f7 d8 64 89 01 48
[ 252.037993] RSP: 002b:7fffaa89dce8 EFLAGS: 0246 ORIG_RAX:
0139
[ 252.038017] RAX: ffda RBX: 55fd4f881080 RCX: 7ff5a2f9408d
[ 252.038039] RDX: RSI: 55fd4f63ec02 RDI: 0009
[ 252.038063] RBP: 0004 R08: R09: 55fd4f8885b0
[ 252.038085] R10: 0009 R11: 0246 R12: 55fd4f63ec02
[ 252.038107] R13: 55fd4f881120 R14: R15: 55fd4f88e350
[ 252.038293] Kernel Offset: 0x3060 from 0x8100 (relocation
range: 0x8000-0xbfff)
Because the Realtek WiFi (PCI 06:00.0) is in the same IOMMU group as AMD
graphics (PCI 01:00.0),
[1.326166] pci :01:00.0: Adding to iommu group 0
...
[1.326268] pci :06:00.0: Adding to iommu group 0
And the AMD graphics supports iommu_v2, so the group uses intentity
mapping based on the query from amd_iommu_def_domain_type().
However, the Realtek WiFi only supports 32bit DMA, so we need to
make sure swiotlb is enabled.
The swiotlb is enabled by pci_swiotlb_detect_4gb() to support legacy
devices, but it gets disabled later by amd_iommu_init_dma_ops(). Keep
swiotlb enabled to resolve the issue.
Cc: Robin Murphy
Signed-off-by: Kai-Heng Feng
---
v2:
- Keep swiotlb enabled if it's already set.
- Some wording change.
drivers/iommu/amd/iommu.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 811a49a95d04..a893a6b6aeba 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1774,7 +1774,8 @@ void amd_iommu_domain_update(struct protection_domain
*domain)
static void __init amd_iommu_init_dma_ops(void)
{
- swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
+ if (!swiotlb)
+ swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
if (amd_iommu_unmap_flush)
pr_info("IO/TLB flush on unmap enabled\n");
--
2.31.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Wed, Jul 14, 2021 at 7:57 AM Konrad Rzeszutek Wilk wrote: > > On Thu, Jul 08, 2021 at 03:43:42PM +0100, Robin Murphy wrote: > > On 2021-07-08 14:57, Kai-Heng Feng wrote: > > > On Thu, Jul 8, 2021 at 6:18 PM Robin Murphy wrote: > > > > > > > > On 2021-07-08 10:28, Joerg Roedel wrote: > > > > > On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: > > > > > > @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) > > > > > > > > > > > > iommu = amd_iommu_rlookup_table[dev_data->devid]; > > > > > > dev_data->iommu_v2 = iommu->is_iommu_v2; > > > > > > + > > > > > > +if (dev_data->iommu_v2) > > > > > > +swiotlb = 1; > > > > > > > > > > This looks like the big hammer, as it will affect all other systems > > > > > where the AMD GPUs are in their own group. > > > > > > > > > > What is needed here is an explicit check whether a non-iommu-v2 device > > > > > is direct-mapped because it shares a group with the GPU, and only > > > > > enable > > > > > swiotlb in this case. > > > > > > > > Right, it's basically about whether any DMA-limited device might at any > > > > time end up in an IOMMU_DOMAIN_IDENTITY domain. And given the > > > > possibility of device hotplug and the user being silly with the sysfs > > > > interface, I don't think we can categorically determine that at boot > > > > time. > > > > > > > > Also note that Intel systems are likely to be similarly affected (in > > > > fact intel-iommu doesn't even have the iommu_default_passthough() check > > > > so it's probably even easier to blow up). > > > > > > swiotlb is enabled by pci_swiotlb_detect_4gb() and intel-iommu doesn't > > > disable it. > > > > Oh, right... I did say I found this dance hard to follow. Clearly I > > shouldn't have trusted what I thought I remembered from looking at it > > yesterday :) > > > > Also not helped by the fact that it sets iommu_detected which *does* disable > > SWIOTLB, but only on IA-64. > > > > > I wonder if we can take the same approach in amd-iommu? > > > > Certainly if there's a precedent for leaving SWIOTLB enabled even if it > > *might* be redundant, that seems like the easiest option (it's what we do on > > arm64 too, but then we have system topologies where some devices may not be > > behind IOMMUs even when others are). More fun would be to try to bring it up > > at the first sign of IOMMU_DOMAIN_IDENTITY if it was disabled previously, > > but I don't have the highest hope of that being practical. > > > It is kind of silly to enable SWIOTLB which will just eat 64MB of memory > "just in case". > > The SWIOTLB does have support to do late initialization (xen-pcifront > does that for example - so if you add devices that can't do 64-bit it > will allocate something like 4MB). > > Would that be a better choice going forward - that is allocate this > under those conditions? But how to practically do swiotlb late init on 32-bit capable devices? On the first DMA map requested by the driver? Kai-Heng > > > > Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Wed, Jul 14, 2021 at 6:25 PM Joerg Roedel wrote: > > On Tue, Jul 13, 2021 at 07:57:40PM -0400, Konrad Rzeszutek Wilk wrote: > > The SWIOTLB does have support to do late initialization (xen-pcifront > > does that for example - so if you add devices that can't do 64-bit it > > will allocate something like 4MB). > > That sounds like a way to evaluate. I suggest to allocate the SWIOTLB > memory at boot and when the IOMMUs are initialized we re-evaluate what > we ended up with and free the SWIOTLB memory if its not needed. > > If that turns out to be wrong during runtime (e.g. because a device is > switched to a passthrough default domain at runtime), we allocate a > small aperture for this device like the above mentioned 4MB. I am currently working on this but I found that 4MB is not enough, 16MB is the minimal size to make the device work. How do I know the right SWIOTLB size for each device? > > (A boot option to always keep the aperture around might also be helpful > for some setups) OK, will also implement this in next iteration. Kai-Heng > > Regards, > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: Disable IOMMU on Stoney Ridge systems
Serious screen flickering when Stoney Ridge outputs to a 4K monitor.
According to Alex Deucher, IOMMU isn't enabled on Windows, so let's do
the same here to avoid screen flickering on 4K monitor.
Cc: Alex Deucher
Bug: https://gitlab.freedesktop.org/drm/amd/issues/961
Signed-off-by: Kai-Heng Feng
---
drivers/iommu/amd_iommu_init.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 568c52317757..e05f1b269be6 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2516,6 +2516,7 @@ static int __init early_amd_iommu_init(void)
struct acpi_table_header *ivrs_base;
acpi_status status;
int i, remap_cache_sz, ret = 0;
+ u32 pci_id;
if (!amd_iommu_detected)
return -ENODEV;
@@ -2603,6 +2604,13 @@ static int __init early_amd_iommu_init(void)
if (ret)
goto out;
+ /* Get the host bridge VID/PID and disable IOMMU if it's Stoney Ridge */
+ pci_id = read_pci_config(0, 0, 0, 0);
+ if ((pci_id & 0x) == 0x1022 && (pci_id >> 16) == 0x1576) {
+ pr_info("Disable IOMMU on Stoney Ridge\n");
+ amd_iommu_disabled = true;
+ }
+
/* Disable any previously enabled IOMMUs */
if (!is_kdump_kernel() || amd_iommu_disabled)
disable_iommus();
@@ -2711,7 +2719,7 @@ static int __init state_next(void)
ret = early_amd_iommu_init();
init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
if (init_state == IOMMU_ACPI_FINISHED && amd_iommu_disabled) {
- pr_info("AMD IOMMU disabled on kernel command-line\n");
+ pr_info("AMD IOMMU disabled\n");
init_state = IOMMU_CMDLINE_DISABLED;
ret = -EINVAL;
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge systems
Serious screen flickering when Stoney Ridge outputs to a 4K monitor.
According to Alex Deucher, IOMMU isn't enabled on Windows, so let's do
the same here to avoid screen flickering on 4K monitor.
Cc: Alex Deucher
Bug: https://gitlab.freedesktop.org/drm/amd/issues/961
Signed-off-by: Kai-Heng Feng
---
v2:
- Find Stoney graphics instead of host bridge.
drivers/iommu/amd_iommu_init.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 568c52317757..139aa6fdadda 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2516,6 +2516,7 @@ static int __init early_amd_iommu_init(void)
struct acpi_table_header *ivrs_base;
acpi_status status;
int i, remap_cache_sz, ret = 0;
+ u32 pci_id;
if (!amd_iommu_detected)
return -ENODEV;
@@ -2603,6 +2604,16 @@ static int __init early_amd_iommu_init(void)
if (ret)
goto out;
+ /* Disable IOMMU if there's Stoney Ridge graphics */
+ for (i = 0; i < 32; i++) {
+ pci_id = read_pci_config(0, i, 0, 0);
+ if ((pci_id & 0x) == 0x1002 && (pci_id >> 16) == 0x98e4) {
+ pr_info("Disable IOMMU on Stoney Ridge\n");
+ amd_iommu_disabled = true;
+ break;
+ }
+ }
+
/* Disable any previously enabled IOMMUs */
if (!is_kdump_kernel() || amd_iommu_disabled)
disable_iommus();
@@ -2711,7 +2722,7 @@ static int __init state_next(void)
ret = early_amd_iommu_init();
init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
if (init_state == IOMMU_ACPI_FINISHED && amd_iommu_disabled) {
- pr_info("AMD IOMMU disabled on kernel command-line\n");
+ pr_info("AMD IOMMU disabled\n");
init_state = IOMMU_CMDLINE_DISABLED;
ret = -EINVAL;
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge systems
Hi Joerg, > On Dec 3, 2019, at 01:00, Christoph Hellwig wrote: > > On Fri, Nov 29, 2019 at 10:21:54PM +0800, Kai-Heng Feng wrote: >> Serious screen flickering when Stoney Ridge outputs to a 4K monitor. >> >> According to Alex Deucher, IOMMU isn't enabled on Windows, so let's do >> the same here to avoid screen flickering on 4K monitor. > > Disabling the IOMMU entirely seem pretty severe. Isn't it enough to > identity map the GPU device? Ok, there's set_device_exclusion_range() to exclude the device from IOMMU. However I don't know how to generate range_start and range_length, which are read from ACPI. Can you please give me some advice here? Kai-Henge ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge systems
> On Dec 5, 2019, at 00:08, Deucher, Alexander > wrote: > >> -Original Message- >> From: Deucher, Alexander >> Sent: Monday, December 2, 2019 11:37 AM >> To: Lucas Stach ; Kai-Heng Feng >> ; j...@8bytes.org; Koenig, Christian >> (christian.koe...@amd.com) >> Cc: iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org >> Subject: RE: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge >> systems >> >>> -Original Message----- >>> From: Lucas Stach >>> Sent: Sunday, December 1, 2019 7:43 AM >>> To: Kai-Heng Feng ; j...@8bytes.org >>> Cc: Deucher, Alexander ; >>> iommu@lists.linux-foundation.org; linux-ker...@vger.kernel.org >>> Subject: Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge >>> systems >>> >>> Am Freitag, den 29.11.2019, 22:21 +0800 schrieb Kai-Heng Feng: >>>> Serious screen flickering when Stoney Ridge outputs to a 4K monitor. >>>> >>>> According to Alex Deucher, IOMMU isn't enabled on Windows, so let's >>>> do the same here to avoid screen flickering on 4K monitor. >>> >>> This doesn't seem like a good solution, especially if there isn't a >>> method for the user to opt-out. Some users might prefer having the >>> IOMMU support to 4K display output. >>> >>> But before using the big hammer of disabling or breaking one of those >>> features, we should take a look at what's the issue here. Screen >>> flickering caused by the IOMMU being active hints to the IOMMU not >>> being able to sustain the translation bandwidth required by the high- >>> bandwidth isochronous transfers caused by 4K scanout, most likely due >>> to insufficient TLB space. >>> >>> As far as I know the framebuffer memory for the display buffers is >>> located in stolen RAM, and thus contigous in memory. I don't know the >>> details of the GPU integration on those APUs, but maybe there even is >>> a way to bypass the IOMMU for the stolen VRAM regions? >>> >>> If there isn't and all GPU traffic passes through the IOMMU when >>> active, we should check if the stolen RAM is mapped with hugepages on >>> the IOMMU side. All the stolen RAM can most likely be mapped with a >>> few hugepage mappings, which should reduce IOMMU TLB demand by a >> large margin. >> >> The is no issue when we scan out of the carve out region. The issue occurs >> when we scan out of regular system memory (scatter/gather). Many newer >> laptops have very small carve out regions (e.g., 32 MB), so we have to use >> regular system pages to support multiple high resolution displays. The >> problem is, the latency gets too high at some point when the IOMMU is >> involved. Huge pages would probably help in this case, but I'm not sure if >> there is any way to guarantee that we get huge pages for system memory. I >> guess we could use CMA or something like that. > > Thomas recently sent out a patch set to add huge page support to ttm: > https://patchwork.freedesktop.org/series/70090/ > We'd still need a way to guarantee huge pages for the display buffer. Is there an amdgpu counterpart to let me test out? Kai-Heng > > Alex > >> >> Alex >> >>> >>> Regards, >>> Lucas >>> >>>> Cc: Alex Deucher >>>> Bug: >>>> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi >>>> tl >>>> >>> >> ab.freedesktop.org%2Fdrm%2Famd%2Fissues%2F961&data=02%7C01% >>> 7Calexa >>>> >>> >> nder.deucher%40amd.com%7C30540b2bf2be417c4d9508d7765bf07f%7C3dd >>> 8961fe4 >>>> >>> >> 884e608e11a82d994e183d%7C0%7C0%7C637108010075463266&sdata=1 >>> ZIZUWos >>>> cPiB4auOY10jlGzoFeWszYMDBQG0CtrrOO8%3D&reserved=0 >>>> Signed-off-by: Kai-Heng Feng >>>> --- >>>> v2: >>>> - Find Stoney graphics instead of host bridge. >>>> >>>> drivers/iommu/amd_iommu_init.c | 13 - >>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/iommu/amd_iommu_init.c >>>> b/drivers/iommu/amd_iommu_init.c index 568c52317757..139aa6fdadda >>>> 100644 >>>> --- a/drivers/iommu/amd_iommu_init.c >>>> +++ b/drivers/iommu/amd_iommu_init.c >>>> @@ -2516,6 +2516,7 @@ static int __init early_amd_iommu_init(
Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge systems
> On Dec 17, 2019, at 17:53, Joerg Roedel wrote: > > On Fri, Dec 06, 2019 at 01:57:41PM +0800, Kai-Heng Feng wrote: >> Hi Joerg, >> >>> On Dec 3, 2019, at 01:00, Christoph Hellwig wrote: >>> >>> On Fri, Nov 29, 2019 at 10:21:54PM +0800, Kai-Heng Feng wrote: >>>> Serious screen flickering when Stoney Ridge outputs to a 4K monitor. >>>> >>>> According to Alex Deucher, IOMMU isn't enabled on Windows, so let's do >>>> the same here to avoid screen flickering on 4K monitor. >>> >>> Disabling the IOMMU entirely seem pretty severe. Isn't it enough to >>> identity map the GPU device? >> >> Ok, there's set_device_exclusion_range() to exclude the device from IOMMU. >> However I don't know how to generate range_start and range_length, which are >> read from ACPI. > > set_device_exclusion_range() is not the solution here. The best is if > the GPU device is put into a passthrough domain at boot, in which it > will be identity mapped. DMA still goes through the IOMMU in this case, > but it only needs to lookup the device-table, page-table walks will not > be done anymore. > > The best way to implement this is to put it into the > amd_iommu_add_device() in drivers/iommu/amd_iommu.c. There is this > check: > >if (dev_data->iommu_v2) > iommu_request_dm_for_dev(dev); > > The iommu_request_dm_for_dev() function causes the device to be identity > mapped. The check can be extended to also check for a device white-list > for devices that need identity mapping. My patch looks like this but the original behavior (4K screen flickering) is still the same: diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c index bd25674ee4db..f913a25c9e92 100644 --- a/drivers/iommu/amd_iommu.c +++ b/drivers/iommu/amd_iommu.c @@ -42,6 +42,7 @@ #include #include #include +#include #include "amd_iommu_proto.h" #include "amd_iommu_types.h" @@ -2159,6 +2160,8 @@ static int amd_iommu_add_device(struct device *dev) struct iommu_domain *domain; struct amd_iommu *iommu; int ret, devid; + bool need_identity_mapping = false; + u32 header; if (!check_device(dev) || get_dev_data(dev)) return 0; @@ -2184,7 +2187,11 @@ static int amd_iommu_add_device(struct device *dev) BUG_ON(!dev_data); - if (dev_data->iommu_v2) + header = read_pci_config(0, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid)); + if ((header & 0x) == 0x1002 && (header >> 16) == 0x98e4) + need_identity_mapping = true; + + if (dev_data->iommu_v2 || need_identity_mapping) iommu_request_dm_for_dev(dev); /* Domains are initialized for this device - have a look what we ended up with */ $ dmesg | grep -i direct [0.011446] Using GB pages for direct mapping [0.703369] pci :00:01.0: Using iommu direct mapping [0.703830] pci :00:08.0: Using iommu direct mapping So the graphics device (pci :00:01.0:) is using direct mapping after the change. Kai-Heng > > HTH, > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge systems
> On Dec 20, 2019, at 03:15, Deucher, Alexander > wrote: > >> -Original Message----- >> From: Kai-Heng Feng >> Sent: Wednesday, December 18, 2019 12:45 PM >> To: Joerg Roedel >> Cc: Christoph Hellwig ; Deucher, Alexander >> ; iommu@lists.linux-foundation.org; Kernel >> development list >> Subject: Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge >> systems >> >> >> >>> On Dec 17, 2019, at 17:53, Joerg Roedel wrote: >>> >>> On Fri, Dec 06, 2019 at 01:57:41PM +0800, Kai-Heng Feng wrote: >>>> Hi Joerg, >>>> >>>>> On Dec 3, 2019, at 01:00, Christoph Hellwig wrote: >>>>> >>>>> On Fri, Nov 29, 2019 at 10:21:54PM +0800, Kai-Heng Feng wrote: >>>>>> Serious screen flickering when Stoney Ridge outputs to a 4K monitor. >>>>>> >>>>>> According to Alex Deucher, IOMMU isn't enabled on Windows, so let's >>>>>> do the same here to avoid screen flickering on 4K monitor. >>>>> >>>>> Disabling the IOMMU entirely seem pretty severe. Isn't it enough to >>>>> identity map the GPU device? >>>> >>>> Ok, there's set_device_exclusion_range() to exclude the device from >> IOMMU. >>>> However I don't know how to generate range_start and range_length, >> which are read from ACPI. >>> >>> set_device_exclusion_range() is not the solution here. The best is if >>> the GPU device is put into a passthrough domain at boot, in which it >>> will be identity mapped. DMA still goes through the IOMMU in this >>> case, but it only needs to lookup the device-table, page-table walks >>> will not be done anymore. >>> >>> The best way to implement this is to put it into the >>> amd_iommu_add_device() in drivers/iommu/amd_iommu.c. There is this >>> check: >>> >>> if (dev_data->iommu_v2) >>> iommu_request_dm_for_dev(dev); >>> >>> The iommu_request_dm_for_dev() function causes the device to be >>> identity mapped. The check can be extended to also check for a device >>> white-list for devices that need identity mapping. >> >> My patch looks like this but the original behavior (4K screen flickering) is >> still >> the same: > > Does reverting the patch to disable ATS along with this patch help? Unfortunately it doesn't help. Kai-Heng > > Alex > >> >> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c >> index bd25674ee4db..f913a25c9e92 100644 >> --- a/drivers/iommu/amd_iommu.c >> +++ b/drivers/iommu/amd_iommu.c >> @@ -42,6 +42,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "amd_iommu_proto.h" >> #include "amd_iommu_types.h" >> @@ -2159,6 +2160,8 @@ static int amd_iommu_add_device(struct device >> *dev) >>struct iommu_domain *domain; >>struct amd_iommu *iommu; >>int ret, devid; >> + bool need_identity_mapping = false; >> + u32 header; >> >>if (!check_device(dev) || get_dev_data(dev)) >>return 0; >> @@ -2184,7 +2187,11 @@ static int amd_iommu_add_device(struct device >> *dev) >> >>BUG_ON(!dev_data); >> >> - if (dev_data->iommu_v2) >> + header = read_pci_config(0, PCI_BUS_NUM(devid), PCI_SLOT(devid), >> PCI_FUNC(devid)); >> + if ((header & 0x) == 0x1002 && (header >> 16) == 0x98e4) >> + need_identity_mapping = true; >> + >> + if (dev_data->iommu_v2 || need_identity_mapping) >>iommu_request_dm_for_dev(dev); >> >>/* Domains are initialized for this device - have a look what we >> ended up >> with */ >> >> >> $ dmesg | grep -i direct >> [0.011446] Using GB pages for direct mapping >> [0.703369] pci :00:01.0: Using iommu direct mapping >> [0.703830] pci :00:08.0: Using iommu direct mapping >> >> So the graphics device (pci :00:01.0:) is using direct mapping after the >> change. >> >> Kai-Heng >> >>> >>> HTH, >>> >>> Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge systems
> On Dec 20, 2019, at 10:13, Kai-Heng Feng wrote: > > > >> On Dec 20, 2019, at 03:15, Deucher, Alexander >> wrote: >> >>> -----Original Message- >>> From: Kai-Heng Feng >>> Sent: Wednesday, December 18, 2019 12:45 PM >>> To: Joerg Roedel >>> Cc: Christoph Hellwig ; Deucher, Alexander >>> ; iommu@lists.linux-foundation.org; Kernel >>> development list >>> Subject: Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge >>> systems >>> >>> >>> >>>> On Dec 17, 2019, at 17:53, Joerg Roedel wrote: >>>> >>>> On Fri, Dec 06, 2019 at 01:57:41PM +0800, Kai-Heng Feng wrote: >>>>> Hi Joerg, >>>>> >>>>>> On Dec 3, 2019, at 01:00, Christoph Hellwig wrote: >>>>>> >>>>>> On Fri, Nov 29, 2019 at 10:21:54PM +0800, Kai-Heng Feng wrote: >>>>>>> Serious screen flickering when Stoney Ridge outputs to a 4K monitor. >>>>>>> >>>>>>> According to Alex Deucher, IOMMU isn't enabled on Windows, so let's >>>>>>> do the same here to avoid screen flickering on 4K monitor. >>>>>> >>>>>> Disabling the IOMMU entirely seem pretty severe. Isn't it enough to >>>>>> identity map the GPU device? >>>>> >>>>> Ok, there's set_device_exclusion_range() to exclude the device from >>> IOMMU. >>>>> However I don't know how to generate range_start and range_length, >>> which are read from ACPI. >>>> >>>> set_device_exclusion_range() is not the solution here. The best is if >>>> the GPU device is put into a passthrough domain at boot, in which it >>>> will be identity mapped. DMA still goes through the IOMMU in this >>>> case, but it only needs to lookup the device-table, page-table walks >>>> will not be done anymore. >>>> >>>> The best way to implement this is to put it into the >>>> amd_iommu_add_device() in drivers/iommu/amd_iommu.c. There is this >>>> check: >>>> >>>> if (dev_data->iommu_v2) >>>>iommu_request_dm_for_dev(dev); >>>> >>>> The iommu_request_dm_for_dev() function causes the device to be >>>> identity mapped. The check can be extended to also check for a device >>>> white-list for devices that need identity mapping. >>> >>> My patch looks like this but the original behavior (4K screen flickering) >>> is still >>> the same: >> >> Does reverting the patch to disable ATS along with this patch help? > > Unfortunately it doesn't help. Any further suggestion to let me try? Kai-Heng > > Kai-Heng > >> >> Alex >> >>> >>> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c >>> index bd25674ee4db..f913a25c9e92 100644 >>> --- a/drivers/iommu/amd_iommu.c >>> +++ b/drivers/iommu/amd_iommu.c >>> @@ -42,6 +42,7 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #include "amd_iommu_proto.h" >>> #include "amd_iommu_types.h" >>> @@ -2159,6 +2160,8 @@ static int amd_iommu_add_device(struct device >>> *dev) >>> struct iommu_domain *domain; >>> struct amd_iommu *iommu; >>> int ret, devid; >>> + bool need_identity_mapping = false; >>> + u32 header; >>> >>> if (!check_device(dev) || get_dev_data(dev)) >>> return 0; >>> @@ -2184,7 +2187,11 @@ static int amd_iommu_add_device(struct device >>> *dev) >>> >>> BUG_ON(!dev_data); >>> >>> - if (dev_data->iommu_v2) >>> + header = read_pci_config(0, PCI_BUS_NUM(devid), PCI_SLOT(devid), >>> PCI_FUNC(devid)); >>> + if ((header & 0x) == 0x1002 && (header >> 16) == 0x98e4) >>> + need_identity_mapping = true; >>> + >>> + if (dev_data->iommu_v2 || need_identity_mapping) >>> iommu_request_dm_for_dev(dev); >>> >>> /* Domains are initialized for this device - have a look what we >>> ended up >>> with */ >>> >>> >>> $ dmesg | grep -i direct >>> [0.011446] Using GB pages for direct mapping >>> [0.703369] pci :00:01.0: Using iommu direct mapping >>> [0.703830] pci :00:08.0: Using iommu direct mapping >>> >>> So the graphics device (pci :00:01.0:) is using direct mapping after the >>> change. >>> >>> Kai-Heng >>> >>>> >>>> HTH, >>>> >>>>Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge systems
Hi Joerg, > On Jan 6, 2020, at 16:37, Kai-Heng Feng wrote: > > > >> On Dec 20, 2019, at 10:13, Kai-Heng Feng wrote: >> >> >> >>> On Dec 20, 2019, at 03:15, Deucher, Alexander >>> wrote: >>> >>>> -Original Message- >>>> From: Kai-Heng Feng >>>> Sent: Wednesday, December 18, 2019 12:45 PM >>>> To: Joerg Roedel >>>> Cc: Christoph Hellwig ; Deucher, Alexander >>>> ; iommu@lists.linux-foundation.org; Kernel >>>> development list >>>> Subject: Re: [PATCH v2] iommu/amd: Disable IOMMU on Stoney Ridge >>>> systems >>>> >>>> >>>> >>>>> On Dec 17, 2019, at 17:53, Joerg Roedel wrote: >>>>> >>>>> On Fri, Dec 06, 2019 at 01:57:41PM +0800, Kai-Heng Feng wrote: >>>>>> Hi Joerg, >>>>>> >>>>>>> On Dec 3, 2019, at 01:00, Christoph Hellwig wrote: >>>>>>> >>>>>>> On Fri, Nov 29, 2019 at 10:21:54PM +0800, Kai-Heng Feng wrote: >>>>>>>> Serious screen flickering when Stoney Ridge outputs to a 4K monitor. >>>>>>>> >>>>>>>> According to Alex Deucher, IOMMU isn't enabled on Windows, so let's >>>>>>>> do the same here to avoid screen flickering on 4K monitor. >>>>>>> >>>>>>> Disabling the IOMMU entirely seem pretty severe. Isn't it enough to >>>>>>> identity map the GPU device? >>>>>> >>>>>> Ok, there's set_device_exclusion_range() to exclude the device from >>>> IOMMU. >>>>>> However I don't know how to generate range_start and range_length, >>>> which are read from ACPI. >>>>> >>>>> set_device_exclusion_range() is not the solution here. The best is if >>>>> the GPU device is put into a passthrough domain at boot, in which it >>>>> will be identity mapped. DMA still goes through the IOMMU in this >>>>> case, but it only needs to lookup the device-table, page-table walks >>>>> will not be done anymore. >>>>> >>>>> The best way to implement this is to put it into the >>>>> amd_iommu_add_device() in drivers/iommu/amd_iommu.c. There is this >>>>> check: >>>>> >>>>> if (dev_data->iommu_v2) >>>>> iommu_request_dm_for_dev(dev); >>>>> >>>>> The iommu_request_dm_for_dev() function causes the device to be >>>>> identity mapped. The check can be extended to also check for a device >>>>> white-list for devices that need identity mapping. >>>> >>>> My patch looks like this but the original behavior (4K screen flickering) >>>> is still >>>> the same: >>> >>> Does reverting the patch to disable ATS along with this patch help? >> >> Unfortunately it doesn't help. > > Any further suggestion to let me try? Since using identity mapping with ATS doesn't help, Is it possible to merge this patch as is? Kai-Heng > > Kai-Heng > >> >> Kai-Heng >> >>> >>> Alex >>> >>>> >>>> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c >>>> index bd25674ee4db..f913a25c9e92 100644 >>>> --- a/drivers/iommu/amd_iommu.c >>>> +++ b/drivers/iommu/amd_iommu.c >>>> @@ -42,6 +42,7 @@ >>>> #include >>>> #include >>>> #include >>>> +#include >>>> >>>> #include "amd_iommu_proto.h" >>>> #include "amd_iommu_types.h" >>>> @@ -2159,6 +2160,8 @@ static int amd_iommu_add_device(struct device >>>> *dev) >>>> struct iommu_domain *domain; >>>> struct amd_iommu *iommu; >>>> int ret, devid; >>>> + bool need_identity_mapping = false; >>>> + u32 header; >>>> >>>> if (!check_device(dev) || get_dev_data(dev)) >>>> return 0; >>>> @@ -2184,7 +2187,11 @@ static int amd_iommu_add_device(struct device >>>> *dev) >>>> >>>> BUG_ON(!dev_data); >>>> >>>> - if (dev_data->iommu_v2) >>>> + header = read_pci_config(0, PCI_BUS_NUM(devid), PCI_SLOT(devid), >>>> PCI_FUNC(devid)); >>>> + if ((header & 0x) == 0x1002 && (header >> 16) == 0x98e4) >>>> + need_identity_mapping = true; >>>> + >>>> + if (dev_data->iommu_v2 || need_identity_mapping) >>>> iommu_request_dm_for_dev(dev); >>>> >>>> /* Domains are initialized for this device - have a look what we >>>> ended up >>>> with */ >>>> >>>> >>>> $ dmesg | grep -i direct >>>> [0.011446] Using GB pages for direct mapping >>>> [0.703369] pci :00:01.0: Using iommu direct mapping >>>> [0.703830] pci :00:08.0: Using iommu direct mapping >>>> >>>> So the graphics device (pci :00:01.0:) is using direct mapping after >>>> the >>>> change. >>>> >>>> Kai-Heng >>>> >>>>> >>>>> HTH, >>>>> >>>>> Joerg > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v3] iommu/amd: Disable IOMMU on Stoney Ridge systems
Serious screen flickering when Stoney Ridge outputs to a 4K monitor.
Use identity-mapping and PCI ATS doesn't help this issue.
According to Alex Deucher, IOMMU isn't enabled on Windows, so let's do
the same here to avoid screen flickering on 4K monitor.
Cc: Alex Deucher
Bug: https://gitlab.freedesktop.org/drm/amd/issues/961
Signed-off-by: Kai-Heng Feng
---
v3:
- Update commit message to mention identity-mapping and ATS don't help.
v2:
- Find Stoney graphics instead of host bridge.
drivers/iommu/amd_iommu_init.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 2759a8d57b7f..6be3853a5d97 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -2523,6 +2523,7 @@ static int __init early_amd_iommu_init(void)
struct acpi_table_header *ivrs_base;
acpi_status status;
int i, remap_cache_sz, ret = 0;
+ u32 pci_id;
if (!amd_iommu_detected)
return -ENODEV;
@@ -2610,6 +2611,16 @@ static int __init early_amd_iommu_init(void)
if (ret)
goto out;
+ /* Disable IOMMU if there's Stoney Ridge graphics */
+ for (i = 0; i < 32; i++) {
+ pci_id = read_pci_config(0, i, 0, 0);
+ if ((pci_id & 0x) == 0x1002 && (pci_id >> 16) == 0x98e4) {
+ pr_info("Disable IOMMU on Stoney Ridge\n");
+ amd_iommu_disabled = true;
+ break;
+ }
+ }
+
/* Disable any previously enabled IOMMUs */
if (!is_kdump_kernel() || amd_iommu_disabled)
disable_iommus();
@@ -2718,7 +2729,7 @@ static int __init state_next(void)
ret = early_amd_iommu_init();
init_state = ret ? IOMMU_INIT_ERROR : IOMMU_ACPI_FINISHED;
if (init_state == IOMMU_ACPI_FINISHED && amd_iommu_disabled) {
- pr_info("AMD IOMMU disabled on kernel command-line\n");
+ pr_info("AMD IOMMU disabled\n");
init_state = IOMMU_CMDLINE_DISABLED;
ret = -EINVAL;
}
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Regression] "iommu/amd: Relax locking in dma_ops path" makes tg3 ethernet transmit queue timeout
Hi Joerg,
> On Aug 21, 2020, at 21:43, Joerg Roedel wrote:
>
> Hi Kai,
>
> On Mon, Jun 29, 2020 at 08:33:22PM +0800, Kai-Heng Feng wrote:
>> I am still seeing the issue on v5.8-rc3. The issue goes away as soon
>> as "iommu=off" is added.
>
> Can you probably help with debugging this issue? I've had no luck so far
> getting my hands on a machine with tg3 hardware and an AMD IOMMU.
Of course, I still have the system at my side.
The offending commit is 92d420ec028d ("iommu/amd: Relax locking in dma_ops
path"), however be62dbf554c5 ("iommu/amd: Convert AMD iommu driver to the
dma-iommu api") removed .map_page entirely so I don't know where to start.
Kai-Heng
>
> Regards,
>
> Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [bugzilla-dae...@bugzilla.kernel.org: [Bug 209149] New: "iommu/vt-d: Enable PCI ACS for platform opt in hint" makes NVMe config space not accessible after S3]
[+Cc Christoph]
> On Sep 24, 2020, at 00:03, Bjorn Helgaas wrote:
>
> [+cc IOMMU and NVMe folks]
>
> Sorry, I forgot to forward this to linux-pci when it was first
> reported.
>
> Apparently this happens with v5.9-rc3, and may be related to
> 50310600ebda ("iommu/vt-d: Enable PCI ACS for platform opt in hint"),
> which appeared in v5.8-rc3.
>
> There are several dmesg logs and proposed patches in the bugzilla, but
> no analysis yet of what the problem is. From the first dmesg
> attachment (https://bugzilla.kernel.org/attachment.cgi?id=292327):
AFAIK Intel is working on it internally.
Comet Lake probably needs ACS quirk like older generation chips.
>
> [ 50.434945] PM: suspend entry (deep)
> [ 50.802086] nvme :01:00.0: saving config space at offset 0x0 (reading
> 0x11e0f)
> [ 50.842775] ACPI: Preparing to enter system sleep state S3
> [ 50.858922] ACPI: Waking up from system sleep state S3
> [ 50.883622] nvme :01:00.0: can't change power state from D3hot to D0
> (config space inaccessible)
> [ 50.947352] nvme :01:00.0: restoring config space at offset 0x0 (was
> 0x, writing 0x11e0f)
> [ 50.947816] pcieport :00:1b.0: DPC: containment event, status:0x1f01
> source:0x
> [ 50.947817] pcieport :00:1b.0: DPC: unmasked uncorrectable error
> detected
> [ 50.947829] pcieport :00:1b.0: PCIe Bus Error: severity=Uncorrected
> (Non-Fatal), type=Transaction Layer, (Receiver ID)
> [ 50.947830] pcieport :00:1b.0: device [8086:06ac] error
> status/mask=0020/0001
> [ 50.947831] pcieport :00:1b.0:[21] ACSViol(First)
> [ 50.947841] pcieport :00:1b.0: AER: broadcast error_detected message
> [ 50.947843] nvme nvme0: frozen state error detected, reset controller
>
> I suspect the nvme "can't change power state" and restore config space
> errors are a consequence of the DPC event. If DPC disables the link,
> the device is inaccessible.
>
> I don't know what caused the ACS Violation. The AER TLP Header Log
> might have a clue, but unfortunately we didn't print it.
>
> Tangent:
>
> The fact that we didn't print the AER TLP Header log looks like
> a bug in itself. PCIe r5.0, sec 6.2.7, table 6-5, says many
> errors, including ACS Violation, should log the TLP header. But
> aer_get_device_error_info() only reads the log for error bits in
> AER_LOG_TLP_MASKS, which doesn't include PCI_ERR_UNC_ACSV.
>
> I don't think there's a "TLP Header Log Valid" bit, and it's ugly to
> have to update AER_LOG_TLP_MASKS if new errors are added. I think
> maybe we should always print the header log.
I can attach TLP Header if there's a patch...
Kai-Heng
>
> - Forwarded message from bugzilla-dae...@bugzilla.kernel.org -
>
> Date: Fri, 04 Sep 2020 14:31:20 +
> From: bugzilla-dae...@bugzilla.kernel.org
> To: bj...@helgaas.com
> Subject: [Bug 209149] New: "iommu/vt-d: Enable PCI ACS for platform opt in
> hint" makes NVMe config space not accessible after S3
> Message-ID:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=209149
>
>Bug ID: 209149
> Summary: "iommu/vt-d: Enable PCI ACS for platform opt in hint"
>makes NVMe config space not accessible after S3
> Product: Drivers
> Version: 2.5
>Kernel Version: mainline
> Hardware: All
>OS: Linux
> Tree: Mainline
>Status: NEW
> Severity: normal
> Priority: P1
> Component: PCI
> Assignee: drivers_...@kernel-bugs.osdl.org
> Reporter: kai.heng.f...@canonical.com
>Regression: No
>
> Here's the error:
> [ 50.947816] pcieport :00:1b.0: DPC: containment event, status:0x1f01
> source:0x
> [ 50.947817] pcieport :00:1b.0: DPC: unmasked uncorrectable error
> detected
> [ 50.947829] pcieport :00:1b.0: PCIe Bus Error: severity=Uncorrected
> (Non-Fatal), type=Transaction Layer, (Receiver ID)
> [ 50.947830] pcieport :00:1b.0: device [8086:06ac] error
> status/mask=0020/0001
> [ 50.947831] pcieport :00:1b.0:[21] ACSViol(First)
> [ 50.947841] pcieport :00:1b.0: AER: broadcast error_detected message
> [ 50.947843] nvme nvme0: frozen state error detected, reset controller
>
> --
> You are receiving this mail because:
> You are watching the assignee of the bug.
>
> - End forwarded message -
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [bugzilla-dae...@bugzilla.kernel.org: [Bug 209149] New: "iommu/vt-d: Enable PCI ACS for platform opt in hint" makes NVMe config space not accessible after S3]
Raj, > On Sep 25, 2020, at 03:44, Raj, Ashok wrote: > > Hi Alex > >>> Apparently it also requires to disable RR, and I'm not able to confirm if >>> CML requires that as well. >>> >>> pci_quirk_disable_intel_spt_pch_acs_redir() also seems to consult the same >>> table, so i'm not sure why we need the other patch in bugzilla is required. >> >> If we're talking about the Intel bug where PCH root ports implement >> the ACS capability and control registers as dword rather than word >> registers, then how is ACS getting enabled in order to generate an ACS >> violation? The standard ACS code would write to the control register >> word at offset 6, which is still the read-only capability register on >> those devices. Thanks, > > > Right... Maybe we need header log to figure out what exatly is happening. > Please let me know what logs you need. As Bjorn mentioned earlier, there's currently no way to dump TLP header log? Kai-Heng ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
