Re: [PATCH 1/1] Revert "iommu/vt-d: Clean up pasid quirk for pre-production devices"
On Sun, Jul 08, 2018 at 02:23:21PM +0800, Lu Baolu wrote:
> This reverts commit ab96746aaa344fb720a198245a837e266fad3b62.
>
> The commit ab96746aaa34 ("iommu/vt-d: Clean up pasid quirk for
> pre-production devices") triggers ECS mode on some platforms
> which have broken ECS support. As the result, graphic device
> will be inoperable on boot.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107017
>
> Cc: Ashok Raj
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel-iommu.c | 32 ++--
> include/linux/intel-iommu.h | 1 +
> 2 files changed, 31 insertions(+), 2 deletions(-)
Applied to iommu/fixes.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] Revert "iommu/vt-d: Clean up pasid quirk for pre-production devices"
On 2018.07.16 14:02:12 +0800, Lu Baolu wrote:
> Hi Joerg,
>
> The graphic guys are looking forward to having this in 4.18.
> Is it possible to take it in the following rcs?
>
This breakes intel gfx driver in 4.18 when gfx dmar is on.
Please include this fix ASAP.
Tested-by: Zhenyu Wang
thanks!
>
> On 07/08/2018 02:23 PM, Lu Baolu wrote:
> > This reverts commit ab96746aaa344fb720a198245a837e266fad3b62.
> >
> > The commit ab96746aaa34 ("iommu/vt-d: Clean up pasid quirk for
> > pre-production devices") triggers ECS mode on some platforms
> > which have broken ECS support. As the result, graphic device
> > will be inoperable on boot.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107017
> >
> > Cc: Ashok Raj
> > Signed-off-by: Lu Baolu
> > ---
> > drivers/iommu/intel-iommu.c | 32 ++--
> > include/linux/intel-iommu.h | 1 +
> > 2 files changed, 31 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > index b344a88..115ff26 100644
> > --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -484,14 +484,37 @@ static int dmar_forcedac;
> > static int intel_iommu_strict;
> > static int intel_iommu_superpage = 1;
> > static int intel_iommu_ecs = 1;
> > +static int intel_iommu_pasid28;
> > static int iommu_identity_mapping;
> >
> > #define IDENTMAP_ALL 1
> > #define IDENTMAP_GFX 2
> > #define IDENTMAP_AZALIA4
> >
> > -#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap))
> > -#define pasid_enabled(iommu) (ecs_enabled(iommu) &&
> > ecap_pasid(iommu->ecap))
> > +/* Broadwell and Skylake have broken ECS support — normal so-called "second
> > + * level" translation of DMA requests-without-PASID doesn't actually happen
> > + * unless you also set the NESTE bit in an extended context-entry. Which of
> > + * course means that SVM doesn't work because it's trying to do nested
> > + * translation of the physical addresses it finds in the process page
> > tables,
> > + * through the IOVA->phys mapping found in the "second level" page tables.
> > + *
> > + * The VT-d specification was retroactively changed to change the
> > definition
> > + * of the capability bits and pretend that Broadwell/Skylake never
> > happened...
> > + * but unfortunately the wrong bit was changed. It's ECS which is broken,
> > but
> > + * for some reason it was the PASID capability bit which was redefined
> > (from
> > + * bit 28 on BDW/SKL to bit 40 in future).
> > + *
> > + * So our test for ECS needs to eschew those implementations which set the
> > old
> > + * PASID capabiity bit 28, since those are the ones on which ECS is broken.
> > + * Unless we are working around the 'pasid28' limitations, that is, by
> > putting
> > + * the device into passthrough mode for normal DMA and thus masking the
> > bug.
> > + */
> > +#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap) && \
> > + (intel_iommu_pasid28 ||
> > !ecap_broken_pasid(iommu->ecap)))
> > +/* PASID support is thus enabled if ECS is enabled and *either* of the old
> > + * or new capability bits are set. */
> > +#define pasid_enabled(iommu) (ecs_enabled(iommu) &&
> > \
> > + (ecap_pasid(iommu->ecap) ||
> > ecap_broken_pasid(iommu->ecap)))
> >
> > int intel_iommu_gfx_mapped;
> > EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
> > @@ -554,6 +577,11 @@ static int __init intel_iommu_setup(char *str)
> > printk(KERN_INFO
> > "Intel-IOMMU: disable extended context table
> > support\n");
> > intel_iommu_ecs = 0;
> > + } else if (!strncmp(str, "pasid28", 7)) {
> > + printk(KERN_INFO
> > + "Intel-IOMMU: enable pre-production PASID
> > support\n");
> > + intel_iommu_pasid28 = 1;
> > + iommu_identity_mapping |= IDENTMAP_GFX;
> > } else if (!strncmp(str, "tboot_noforce", 13)) {
> > printk(KERN_INFO
> > "Intel-IOMMU: not forcing on after tboot. This
> > could expose security risk for tboot\n");
> > diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> > index 1df9401..ef169d6 100644
> > --- a/include/linux/intel-iommu.h
> > +++ b/include/linux/intel-iommu.h
> > @@ -121,6 +121,7 @@
> > #define ecap_srs(e)((e >> 31) & 0x1)
> > #define ecap_ers(e)((e >> 30) & 0x1)
> > #define ecap_prs(e)((e >> 29) & 0x1)
> > +#define ecap_broken_pasid(e) ((e >> 28) & 0x1)
> > #define ecap_dis(e)((e >> 27) & 0x1)
> > #define ecap_nest(e) ((e >> 26) & 0x1)
> > #define ecap_mts(e)((e >> 25) & 0x1)
>
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net
Re: [PATCH 1/1] Revert "iommu/vt-d: Clean up pasid quirk for pre-production devices"
Hi Joerg,
The graphic guys are looking forward to having this in 4.18.
Is it possible to take it in the following rcs?
Best regards,
Lu Baolu
On 07/08/2018 02:23 PM, Lu Baolu wrote:
> This reverts commit ab96746aaa344fb720a198245a837e266fad3b62.
>
> The commit ab96746aaa34 ("iommu/vt-d: Clean up pasid quirk for
> pre-production devices") triggers ECS mode on some platforms
> which have broken ECS support. As the result, graphic device
> will be inoperable on boot.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107017
>
> Cc: Ashok Raj
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel-iommu.c | 32 ++--
> include/linux/intel-iommu.h | 1 +
> 2 files changed, 31 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index b344a88..115ff26 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -484,14 +484,37 @@ static int dmar_forcedac;
> static int intel_iommu_strict;
> static int intel_iommu_superpage = 1;
> static int intel_iommu_ecs = 1;
> +static int intel_iommu_pasid28;
> static int iommu_identity_mapping;
>
> #define IDENTMAP_ALL 1
> #define IDENTMAP_GFX 2
> #define IDENTMAP_AZALIA 4
>
> -#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap))
> -#define pasid_enabled(iommu) (ecs_enabled(iommu) && ecap_pasid(iommu->ecap))
> +/* Broadwell and Skylake have broken ECS support — normal so-called "second
> + * level" translation of DMA requests-without-PASID doesn't actually happen
> + * unless you also set the NESTE bit in an extended context-entry. Which of
> + * course means that SVM doesn't work because it's trying to do nested
> + * translation of the physical addresses it finds in the process page tables,
> + * through the IOVA->phys mapping found in the "second level" page tables.
> + *
> + * The VT-d specification was retroactively changed to change the definition
> + * of the capability bits and pretend that Broadwell/Skylake never
> happened...
> + * but unfortunately the wrong bit was changed. It's ECS which is broken, but
> + * for some reason it was the PASID capability bit which was redefined (from
> + * bit 28 on BDW/SKL to bit 40 in future).
> + *
> + * So our test for ECS needs to eschew those implementations which set the
> old
> + * PASID capabiity bit 28, since those are the ones on which ECS is broken.
> + * Unless we are working around the 'pasid28' limitations, that is, by
> putting
> + * the device into passthrough mode for normal DMA and thus masking the bug.
> + */
> +#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap) && \
> + (intel_iommu_pasid28 ||
> !ecap_broken_pasid(iommu->ecap)))
> +/* PASID support is thus enabled if ECS is enabled and *either* of the old
> + * or new capability bits are set. */
> +#define pasid_enabled(iommu) (ecs_enabled(iommu) && \
> + (ecap_pasid(iommu->ecap) ||
> ecap_broken_pasid(iommu->ecap)))
>
> int intel_iommu_gfx_mapped;
> EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
> @@ -554,6 +577,11 @@ static int __init intel_iommu_setup(char *str)
> printk(KERN_INFO
> "Intel-IOMMU: disable extended context table
> support\n");
> intel_iommu_ecs = 0;
> + } else if (!strncmp(str, "pasid28", 7)) {
> + printk(KERN_INFO
> + "Intel-IOMMU: enable pre-production PASID
> support\n");
> + intel_iommu_pasid28 = 1;
> + iommu_identity_mapping |= IDENTMAP_GFX;
> } else if (!strncmp(str, "tboot_noforce", 13)) {
> printk(KERN_INFO
> "Intel-IOMMU: not forcing on after tboot. This
> could expose security risk for tboot\n");
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index 1df9401..ef169d6 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -121,6 +121,7 @@
> #define ecap_srs(e) ((e >> 31) & 0x1)
> #define ecap_ers(e) ((e >> 30) & 0x1)
> #define ecap_prs(e) ((e >> 29) & 0x1)
> +#define ecap_broken_pasid(e) ((e >> 28) & 0x1)
> #define ecap_dis(e) ((e >> 27) & 0x1)
> #define ecap_nest(e) ((e >> 26) & 0x1)
> #define ecap_mts(e) ((e >> 25) & 0x1)
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/1] Revert "iommu/vt-d: Clean up pasid quirk for pre-production devices"
This reverts commit ab96746aaa344fb720a198245a837e266fad3b62.
The commit ab96746aaa34 ("iommu/vt-d: Clean up pasid quirk for
pre-production devices") triggers ECS mode on some platforms
which have broken ECS support. As the result, graphic device
will be inoperable on boot.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107017
Cc: Ashok Raj
Signed-off-by: Lu Baolu
---
drivers/iommu/intel-iommu.c | 32 ++--
include/linux/intel-iommu.h | 1 +
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index b344a88..115ff26 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -484,14 +484,37 @@ static int dmar_forcedac;
static int intel_iommu_strict;
static int intel_iommu_superpage = 1;
static int intel_iommu_ecs = 1;
+static int intel_iommu_pasid28;
static int iommu_identity_mapping;
#define IDENTMAP_ALL 1
#define IDENTMAP_GFX 2
#define IDENTMAP_AZALIA4
-#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap))
-#define pasid_enabled(iommu) (ecs_enabled(iommu) && ecap_pasid(iommu->ecap))
+/* Broadwell and Skylake have broken ECS support — normal so-called "second
+ * level" translation of DMA requests-without-PASID doesn't actually happen
+ * unless you also set the NESTE bit in an extended context-entry. Which of
+ * course means that SVM doesn't work because it's trying to do nested
+ * translation of the physical addresses it finds in the process page tables,
+ * through the IOVA->phys mapping found in the "second level" page tables.
+ *
+ * The VT-d specification was retroactively changed to change the definition
+ * of the capability bits and pretend that Broadwell/Skylake never happened...
+ * but unfortunately the wrong bit was changed. It's ECS which is broken, but
+ * for some reason it was the PASID capability bit which was redefined (from
+ * bit 28 on BDW/SKL to bit 40 in future).
+ *
+ * So our test for ECS needs to eschew those implementations which set the old
+ * PASID capabiity bit 28, since those are the ones on which ECS is broken.
+ * Unless we are working around the 'pasid28' limitations, that is, by putting
+ * the device into passthrough mode for normal DMA and thus masking the bug.
+ */
+#define ecs_enabled(iommu) (intel_iommu_ecs && ecap_ecs(iommu->ecap) && \
+ (intel_iommu_pasid28 ||
!ecap_broken_pasid(iommu->ecap)))
+/* PASID support is thus enabled if ECS is enabled and *either* of the old
+ * or new capability bits are set. */
+#define pasid_enabled(iommu) (ecs_enabled(iommu) &&\
+ (ecap_pasid(iommu->ecap) ||
ecap_broken_pasid(iommu->ecap)))
int intel_iommu_gfx_mapped;
EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -554,6 +577,11 @@ static int __init intel_iommu_setup(char *str)
printk(KERN_INFO
"Intel-IOMMU: disable extended context table
support\n");
intel_iommu_ecs = 0;
+ } else if (!strncmp(str, "pasid28", 7)) {
+ printk(KERN_INFO
+ "Intel-IOMMU: enable pre-production PASID
support\n");
+ intel_iommu_pasid28 = 1;
+ iommu_identity_mapping |= IDENTMAP_GFX;
} else if (!strncmp(str, "tboot_noforce", 13)) {
printk(KERN_INFO
"Intel-IOMMU: not forcing on after tboot. This
could expose security risk for tboot\n");
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 1df9401..ef169d6 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -121,6 +121,7 @@
#define ecap_srs(e)((e >> 31) & 0x1)
#define ecap_ers(e)((e >> 30) & 0x1)
#define ecap_prs(e)((e >> 29) & 0x1)
+#define ecap_broken_pasid(e) ((e >> 28) & 0x1)
#define ecap_dis(e)((e >> 27) & 0x1)
#define ecap_nest(e) ((e >> 26) & 0x1)
#define ecap_mts(e)((e >> 25) & 0x1)
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
