All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lu Baolu <baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>,
	David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org,
	ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org,
	Zhenyu Wang <zhenyuw-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 1/1] Revert "iommu/vt-d: Clean up pasid quirk for pre-production devices"
Date: Mon, 16 Jul 2018 14:02:12 +0800	[thread overview]
Message-ID: <5B4C34E4.30004@linux.intel.com> (raw)
In-Reply-To: <1531031001-5783-1-git-send-email-baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

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 <ashok.raj@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  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

WARNING: multiple messages have this Message-ID (diff)
From: Lu Baolu <baolu.lu@linux.intel.com>
To: Joerg Roedel <joro@8bytes.org>, David Woodhouse <dwmw2@infradead.org>
Cc: ashok.raj@intel.com, iommu@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, intel-gfx@lists.freedesktop.org,
	Zhenyu Wang <zhenyuw@linux.intel.com>
Subject: Re: [PATCH 1/1] Revert "iommu/vt-d: Clean up pasid quirk for pre-production devices"
Date: Mon, 16 Jul 2018 14:02:12 +0800	[thread overview]
Message-ID: <5B4C34E4.30004@linux.intel.com> (raw)
In-Reply-To: <1531031001-5783-1-git-send-email-baolu.lu@linux.intel.com>

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 <ashok.raj@intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  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)


  parent reply	other threads:[~2018-07-16  6:02 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-08  6:23 [PATCH 1/1] Revert "iommu/vt-d: Clean up pasid quirk for pre-production devices" Lu Baolu
2018-07-08  6:23 ` Lu Baolu
     [not found] ` <1531031001-5783-1-git-send-email-baolu.lu-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-07-16  6:02   ` Lu Baolu [this message]
2018-07-16  6:02     ` Lu Baolu
     [not found]     ` <5B4C34E4.30004-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2018-07-16  7:53       ` Zhenyu Wang
2018-07-16  7:53         ` Zhenyu Wang
2018-07-20 11:56 ` Joerg Roedel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5B4C34E4.30004@linux.intel.com \
    --to=baolu.lu-vuqaysv1563yd54fqh9/ca@public.gmane.org \
    --cc=ashok.raj-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org \
    --cc=intel-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
    --cc=iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=zhenyuw-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.