2020-07-21 00:23:44

by Baolu Lu

[permalink] [raw]
Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu

The VT-d spec requires (10.4.4 Global Command Register, TE field) that:

Hardware implementations supporting DMA draining must drain any in-flight
DMA read/write requests queued within the Root-Complex before completing
the translation enable command and reflecting the status of the command
through the TES field in the Global Status register.

Unfortunately, some integrated graphic devices fail to do so after some
kind of power state transition. As the result, the system might stuck in
iommu_disable_translation(), waiting for the completion of TE transition.

This provides a quirk list for those devices and skips TE disabling if
the qurik hits.

Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=208363
Tested-by: Koba Ko <[email protected]>
Cc: Ashok Raj <[email protected]>
Cc: [email protected]
Signed-off-by: Lu Baolu <[email protected]>
---
drivers/iommu/intel/dmar.c | 1 +
drivers/iommu/intel/iommu.c | 27 +++++++++++++++++++++++++++
include/linux/dmar.h | 1 +
include/linux/intel-iommu.h | 2 ++
4 files changed, 31 insertions(+)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 683b812c5c47..16f47041f1bf 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1102,6 +1102,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
}

drhd->iommu = iommu;
+ iommu->drhd = drhd;

return 0;

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 98390a6d8113..11418b14cc3f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -356,6 +356,7 @@ static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
static int iommu_identity_mapping;
static int intel_no_bounce;
+static int iommu_skip_te_disable;

#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA 4
@@ -1633,6 +1634,10 @@ static void iommu_disable_translation(struct intel_iommu *iommu)
u32 sts;
unsigned long flag;

+ if (iommu_skip_te_disable && iommu->drhd->gfx_dedicated &&
+ (cap_read_drain(iommu->cap) || cap_write_drain(iommu->cap)))
+ return;
+
raw_spin_lock_irqsave(&iommu->register_lock, flag);
iommu->gcmd &= ~DMA_GCMD_TE;
writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
@@ -4043,6 +4048,7 @@ static void __init init_no_remapping_devices(void)

/* This IOMMU has *only* gfx devices. Either bypass it or
set the gfx_mapped flag, as appropriate */
+ drhd->gfx_dedicated = 1;
if (!dmar_map_gfx) {
drhd->ignored = 1;
for_each_active_dev_scope(drhd->devices,
@@ -6160,6 +6166,27 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0044, quirk_calpella_no_shadow_g
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0062, quirk_calpella_no_shadow_gtt);
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x006a, quirk_calpella_no_shadow_gtt);

+static void quirk_igfx_skip_te_disable(struct pci_dev *dev)
+{
+ unsigned short ver;
+
+ if (!IS_GFX_DEVICE(dev))
+ return;
+
+ ver = (dev->device >> 8) & 0xff;
+ if (ver != 0x45 && ver != 0x46 && ver != 0x4c &&
+ ver != 0x4e && ver != 0x8a && ver != 0x98 &&
+ ver != 0x9a)
+ return;
+
+ if (risky_device(dev))
+ return;
+
+ pci_info(dev, "Skip IOMMU disabling for graphics\n");
+ iommu_skip_te_disable = 1;
+}
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID, quirk_igfx_skip_te_disable);
+
/* On Tylersburg chipsets, some BIOSes have been known to enable the
ISOCH DMAR unit for the Azalia sound device, but not give it any
TLB entries, which causes it to deadlock. Check for that. We do
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index d7bf029df737..65565820328a 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -48,6 +48,7 @@ struct dmar_drhd_unit {
u16 segment; /* PCI domain */
u8 ignored:1; /* ignore drhd */
u8 include_all:1;
+ u8 gfx_dedicated:1; /* graphic dedicated */
struct intel_iommu *iommu;
};

diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index bf6009a344f5..329629e1e9de 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -600,6 +600,8 @@ struct intel_iommu {
struct iommu_device iommu; /* IOMMU core code handle */
int node;
u32 flags; /* Software defined flags */
+
+ struct dmar_drhd_unit *drhd;
};

/* PCI domain-device relationship */
--
2.17.1


2020-07-21 14:48:22

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu

> -----Original Message-----
> From: iommu <[email protected]> On Behalf Of Lu
> Baolu
> Sent: Monday, July 20, 2020 7:17 PM
> To: Joerg Roedel
> Cc: Ashok Raj; [email protected]; [email protected]; Koba
> Ko; [email protected]
> Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated
> iommu
>
> The VT-d spec requires (10.4.4 Global Command Register, TE field) that:
>
> Hardware implementations supporting DMA draining must drain any in-flight
> DMA read/write requests queued within the Root-Complex before completing
> the translation enable command and reflecting the status of the command
> through the TES field in the Global Status register.
>
> Unfortunately, some integrated graphic devices fail to do so after some
> kind of power state transition. As the result, the system might stuck in
> iommu_disable_translation(), waiting for the completion of TE transition.
>
> This provides a quirk list for those devices and skips TE disabling if
> the qurik hits.
>
> Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=208363
That one is for TGL.

I think you also want to add this one for ICL:
Fixes: https://bugzilla.kernel.org/show_bug.cgi?id=206571

> Tested-by: Koba Ko <[email protected]>
> Cc: Ashok Raj <[email protected]>
> Cc: [email protected]
> Signed-off-by: Lu Baolu <[email protected]>
> ---
> drivers/iommu/intel/dmar.c | 1 +
> drivers/iommu/intel/iommu.c | 27 +++++++++++++++++++++++++++
> include/linux/dmar.h | 1 +
> include/linux/intel-iommu.h | 2 ++
> 4 files changed, 31 insertions(+)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 683b812c5c47..16f47041f1bf 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1102,6 +1102,7 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
> }
>
> drhd->iommu = iommu;
> + iommu->drhd = drhd;
>
> return 0;
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 98390a6d8113..11418b14cc3f 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -356,6 +356,7 @@ static int intel_iommu_strict;
> static int intel_iommu_superpage = 1;
> static int iommu_identity_mapping;
> static int intel_no_bounce;
> +static int iommu_skip_te_disable;
>
> #define IDENTMAP_GFX 2
> #define IDENTMAP_AZALIA 4
> @@ -1633,6 +1634,10 @@ static void iommu_disable_translation(struct
> intel_iommu *iommu)
> u32 sts;
> unsigned long flag;
>
> + if (iommu_skip_te_disable && iommu->drhd->gfx_dedicated &&
> + (cap_read_drain(iommu->cap) || cap_write_drain(iommu->cap)))
> + return;
> +
> raw_spin_lock_irqsave(&iommu->register_lock, flag);
> iommu->gcmd &= ~DMA_GCMD_TE;
> writel(iommu->gcmd, iommu->reg + DMAR_GCMD_REG);
> @@ -4043,6 +4048,7 @@ static void __init init_no_remapping_devices(void)
>
> /* This IOMMU has *only* gfx devices. Either bypass it or
> set the gfx_mapped flag, as appropriate */
> + drhd->gfx_dedicated = 1;
> if (!dmar_map_gfx) {
> drhd->ignored = 1;
> for_each_active_dev_scope(drhd->devices,
> @@ -6160,6 +6166,27 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,
> 0x0044, quirk_calpella_no_shadow_g
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x0062,
> quirk_calpella_no_shadow_gtt);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x006a,
> quirk_calpella_no_shadow_gtt);
>
> +static void quirk_igfx_skip_te_disable(struct pci_dev *dev)
> +{
> + unsigned short ver;
> +
> + if (!IS_GFX_DEVICE(dev))
> + return;
> +
> + ver = (dev->device >> 8) & 0xff;
> + if (ver != 0x45 && ver != 0x46 && ver != 0x4c &&
> + ver != 0x4e && ver != 0x8a && ver != 0x98 &&
> + ver != 0x9a)
> + return;
> +
> + if (risky_device(dev))
> + return;
> +
> + pci_info(dev, "Skip IOMMU disabling for graphics\n");
> + iommu_skip_te_disable = 1;
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, PCI_ANY_ID,
> quirk_igfx_skip_te_disable);
> +
> /* On Tylersburg chipsets, some BIOSes have been known to enable the
> ISOCH DMAR unit for the Azalia sound device, but not give it any
> TLB entries, which causes it to deadlock. Check for that. We do
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index d7bf029df737..65565820328a 100644
> --- a/include/linux/dmar.h
> +++ b/include/linux/dmar.h
> @@ -48,6 +48,7 @@ struct dmar_drhd_unit {
> u16 segment; /* PCI domain */
> u8 ignored:1; /* ignore drhd */
> u8 include_all:1;
> + u8 gfx_dedicated:1; /* graphic dedicated */
> struct intel_iommu *iommu;
> };
>
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index bf6009a344f5..329629e1e9de 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -600,6 +600,8 @@ struct intel_iommu {
> struct iommu_device iommu; /* IOMMU core code handle */
> int node;
> u32 flags; /* Software defined flags */
> +
> + struct dmar_drhd_unit *drhd;
> };
>
> /* PCI domain-device relationship */
> --
> 2.17.1
>
> _______________________________________________
> iommu mailing list
> [email protected]
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

2020-07-21 23:12:26

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu

Hi Limonciello,

On 7/21/20 10:44 PM, Limonciello, Mario wrote:
>> -----Original Message-----
>> From: iommu<[email protected]> On Behalf Of Lu
>> Baolu
>> Sent: Monday, July 20, 2020 7:17 PM
>> To: Joerg Roedel
>> Cc: Ashok Raj;[email protected];[email protected]; Koba
>> Ko;[email protected]
>> Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated
>> iommu
>>
>> The VT-d spec requires (10.4.4 Global Command Register, TE field) that:
>>
>> Hardware implementations supporting DMA draining must drain any in-flight
>> DMA read/write requests queued within the Root-Complex before completing
>> the translation enable command and reflecting the status of the command
>> through the TES field in the Global Status register.
>>
>> Unfortunately, some integrated graphic devices fail to do so after some
>> kind of power state transition. As the result, the system might stuck in
>> iommu_disable_translation(), waiting for the completion of TE transition.
>>
>> This provides a quirk list for those devices and skips TE disabling if
>> the qurik hits.
>>
>> Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=208363
> That one is for TGL.
>
> I think you also want to add this one for ICL:
> Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=206571
>

Do you mean someone have tested that this patch also fixes the problem
described in 206571?

Best regards,
baolu

2020-07-21 23:46:46

by Mario Limonciello

[permalink] [raw]
Subject: RE: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu



> -----Original Message-----
> From: Lu Baolu <[email protected]>
> Sent: Tuesday, July 21, 2020 6:07 PM
> To: Limonciello, Mario; Joerg Roedel
> Cc: [email protected]; Ashok Raj; [email protected];
> [email protected]; Koba Ko; [email protected]
> Subject: Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated
> iommu
>
>
> [EXTERNAL EMAIL]
>
> Hi Limonciello,
>
> On 7/21/20 10:44 PM, Limonciello, Mario wrote:
> >> -----Original Message-----
> >> From: iommu<[email protected]> On Behalf Of Lu
> >> Baolu
> >> Sent: Monday, July 20, 2020 7:17 PM
> >> To: Joerg Roedel
> >> Cc: Ashok Raj;[email protected];[email protected]; Koba
> >> Ko;[email protected]
> >> Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated
> >> iommu
> >>
> >> The VT-d spec requires (10.4.4 Global Command Register, TE field) that:
> >>
> >> Hardware implementations supporting DMA draining must drain any in-flight
> >> DMA read/write requests queued within the Root-Complex before completing
> >> the translation enable command and reflecting the status of the command
> >> through the TES field in the Global Status register.
> >>
> >> Unfortunately, some integrated graphic devices fail to do so after some
> >> kind of power state transition. As the result, the system might stuck in
> >> iommu_disable_translation(), waiting for the completion of TE transition.
> >>
> >> This provides a quirk list for those devices and skips TE disabling if
> >> the qurik hits.
> >>
> >> Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=208363
> > That one is for TGL.
> >
> > I think you also want to add this one for ICL:
> > Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=206571
> >
>
> Do you mean someone have tested that this patch also fixes the problem
> described in 206571?
>

Yes, confusingly https://bugzilla.kernel.org/show_bug.cgi?id=208363#c31 actually
is the XPS 9300 ICL system and issue.

I also have a private confirmation from another person that it resolves it for
them on another ICL platform.

Christian, maybe you can add a tested by clause for the ICL testing.

2020-07-23 01:45:59

by Baolu Lu

[permalink] [raw]
Subject: Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated iommu

On 7/22/20 7:45 AM, Limonciello, Mario wrote:
>
>
>> -----Original Message-----
>> From: Lu Baolu <[email protected]>
>> Sent: Tuesday, July 21, 2020 6:07 PM
>> To: Limonciello, Mario; Joerg Roedel
>> Cc: [email protected]; Ashok Raj; [email protected];
>> [email protected]; Koba Ko; [email protected]
>> Subject: Re: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated
>> iommu
>>
>>
>> [EXTERNAL EMAIL]
>>
>> Hi Limonciello,
>>
>> On 7/21/20 10:44 PM, Limonciello, Mario wrote:
>>>> -----Original Message-----
>>>> From: iommu<[email protected]> On Behalf Of Lu
>>>> Baolu
>>>> Sent: Monday, July 20, 2020 7:17 PM
>>>> To: Joerg Roedel
>>>> Cc: Ashok Raj;[email protected];[email protected]; Koba
>>>> Ko;[email protected]
>>>> Subject: [PATCH 1/1] iommu/vt-d: Skip TE disabling on quirky gfx dedicated
>>>> iommu
>>>>
>>>> The VT-d spec requires (10.4.4 Global Command Register, TE field) that:
>>>>
>>>> Hardware implementations supporting DMA draining must drain any in-flight
>>>> DMA read/write requests queued within the Root-Complex before completing
>>>> the translation enable command and reflecting the status of the command
>>>> through the TES field in the Global Status register.
>>>>
>>>> Unfortunately, some integrated graphic devices fail to do so after some
>>>> kind of power state transition. As the result, the system might stuck in
>>>> iommu_disable_translation(), waiting for the completion of TE transition.
>>>>
>>>> This provides a quirk list for those devices and skips TE disabling if
>>>> the qurik hits.
>>>>
>>>> Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=208363
>>> That one is for TGL.
>>>
>>> I think you also want to add this one for ICL:
>>> Fixes:https://bugzilla.kernel.org/show_bug.cgi?id=206571
>>>
>>
>> Do you mean someone have tested that this patch also fixes the problem
>> described in 206571?
>>
>
> Yes, confusingly https://bugzilla.kernel.org/show_bug.cgi?id=208363#c31 actually
> is the XPS 9300 ICL system and issue.
>
> I also have a private confirmation from another person that it resolves it for
> them on another ICL platform.
>

Okay! Thank you very much! I just posted v2 with this tag added.

Best regards,
baolu