LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 0/4] iommu/vt-d: Several cleanup patches
@ 2018-05-04  5:08 Lu Baolu
  2018-05-04  5:08 ` [PATCH 1/4] iommu: Clean up the comments for iommu_group_alloc Lu Baolu
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Lu Baolu @ 2018-05-04  5:08 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse; +Cc: ashok.raj, iommu, linux-kernel, Lu Baolu

Hi,

This includes several cleanup patches which aim to make the
code more concise and easier for reading. There aren't any
functionality changes.

Best regards,
Lu Baolu

Lu Baolu (4):
  iommu: Clean up the comments for iommu_group_alloc
  iommu/vt-d: Clean up unused variable in find_or_alloc_domain
  iommu/vt-d: Clean up pasid quirk for pre-production devices
  iommu/vt-d: Remove unnecessary parentheses

 drivers/iommu/intel-iommu.c | 36 +++---------------------------------
 drivers/iommu/intel-svm.c   |  2 +-
 drivers/iommu/iommu.c       |  1 -
 include/linux/intel-iommu.h |  1 -
 4 files changed, 4 insertions(+), 36 deletions(-)

-- 
2.7.4

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

* [PATCH 1/4] iommu: Clean up the comments for iommu_group_alloc
  2018-05-04  5:08 [PATCH 0/4] iommu/vt-d: Several cleanup patches Lu Baolu
@ 2018-05-04  5:08 ` Lu Baolu
  2018-05-04  5:08 ` [PATCH 2/4] iommu/vt-d: Clean up unused variable in find_or_alloc_domain Lu Baolu
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2018-05-04  5:08 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse; +Cc: ashok.raj, iommu, linux-kernel, Lu Baolu

@name parameter has been removed.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index d2aa2320..d87e7c2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -322,7 +322,6 @@ static struct kobj_type iommu_group_ktype = {
 
 /**
  * iommu_group_alloc - Allocate a new group
- * @name: Optional name to associate with group, visible in sysfs
  *
  * This function is called by an iommu driver to allocate a new iommu
  * group.  The iommu group represents the minimum granularity of the iommu.
-- 
2.7.4

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

* [PATCH 2/4] iommu/vt-d: Clean up unused variable in find_or_alloc_domain
  2018-05-04  5:08 [PATCH 0/4] iommu/vt-d: Several cleanup patches Lu Baolu
  2018-05-04  5:08 ` [PATCH 1/4] iommu: Clean up the comments for iommu_group_alloc Lu Baolu
@ 2018-05-04  5:08 ` Lu Baolu
  2018-05-04  5:08 ` [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices Lu Baolu
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2018-05-04  5:08 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse; +Cc: ashok.raj, iommu, linux-kernel, Lu Baolu

Remove it to make the code more concise.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 749d8f2..9064607 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2533,7 +2533,7 @@ static struct dmar_domain *find_or_alloc_domain(struct device *dev, int gaw)
 	struct device_domain_info *info = NULL;
 	struct dmar_domain *domain = NULL;
 	struct intel_iommu *iommu;
-	u16 req_id, dma_alias;
+	u16 dma_alias;
 	unsigned long flags;
 	u8 bus, devfn;
 
@@ -2541,8 +2541,6 @@ static struct dmar_domain *find_or_alloc_domain(struct device *dev, int gaw)
 	if (!iommu)
 		return NULL;
 
-	req_id = ((u16)bus << 8) | devfn;
-
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
 
-- 
2.7.4

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

* [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices
  2018-05-04  5:08 [PATCH 0/4] iommu/vt-d: Several cleanup patches Lu Baolu
  2018-05-04  5:08 ` [PATCH 1/4] iommu: Clean up the comments for iommu_group_alloc Lu Baolu
  2018-05-04  5:08 ` [PATCH 2/4] iommu/vt-d: Clean up unused variable in find_or_alloc_domain Lu Baolu
@ 2018-05-04  5:08 ` Lu Baolu
  2018-07-07  7:01   ` Chris Wilson
  2018-05-04  5:08 ` [PATCH 4/4] iommu/vt-d: Remove unnecessary parentheses Lu Baolu
  2018-05-15 14:35 ` [PATCH 0/4] iommu/vt-d: Several cleanup patches Joerg Roedel
  4 siblings, 1 reply; 8+ messages in thread
From: Lu Baolu @ 2018-05-04  5:08 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse; +Cc: ashok.raj, iommu, linux-kernel, Lu Baolu

The pasid28 quirk is needed only for some pre-production devices.
Remove it to make the code concise.

Signed-off-by: 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, 2 insertions(+), 31 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 9064607..10bce33 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -485,37 +485,14 @@ 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
 
-/* 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)))
+#define ecs_enabled(iommu)	(intel_iommu_ecs && ecap_ecs(iommu->ecap))
+#define pasid_enabled(iommu)	(ecs_enabled(iommu) && ecap_pasid(iommu->ecap))
 
 int intel_iommu_gfx_mapped;
 EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
@@ -578,11 +555,6 @@ 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 ef169d6..1df9401 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -121,7 +121,6 @@
 #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

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

* [PATCH 4/4] iommu/vt-d: Remove unnecessary parentheses
  2018-05-04  5:08 [PATCH 0/4] iommu/vt-d: Several cleanup patches Lu Baolu
                   ` (2 preceding siblings ...)
  2018-05-04  5:08 ` [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices Lu Baolu
@ 2018-05-04  5:08 ` Lu Baolu
  2018-05-15 14:35 ` [PATCH 0/4] iommu/vt-d: Several cleanup patches Joerg Roedel
  4 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2018-05-04  5:08 UTC (permalink / raw)
  To: Joerg Roedel, David Woodhouse; +Cc: ashok.raj, iommu, linux-kernel, Lu Baolu

Remove unnecessary parentheses to comply with preferred coding
style.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-svm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index e8cd984..45f6e58 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -319,7 +319,7 @@ int intel_svm_bind_mm(struct device *dev, int *pasid, int flags, struct svm_dev_
 	} else
 		pasid_max = 1 << 20;
 
-	if ((flags & SVM_FLAG_SUPERVISOR_MODE)) {
+	if (flags & SVM_FLAG_SUPERVISOR_MODE) {
 		if (!ecap_srs(iommu->ecap))
 			return -EINVAL;
 	} else if (pasid) {
-- 
2.7.4

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

* Re: [PATCH 0/4] iommu/vt-d: Several cleanup patches
  2018-05-04  5:08 [PATCH 0/4] iommu/vt-d: Several cleanup patches Lu Baolu
                   ` (3 preceding siblings ...)
  2018-05-04  5:08 ` [PATCH 4/4] iommu/vt-d: Remove unnecessary parentheses Lu Baolu
@ 2018-05-15 14:35 ` Joerg Roedel
  4 siblings, 0 replies; 8+ messages in thread
From: Joerg Roedel @ 2018-05-15 14:35 UTC (permalink / raw)
  To: Lu Baolu; +Cc: David Woodhouse, ashok.raj, iommu, linux-kernel

On Fri, May 04, 2018 at 01:08:15PM +0800, Lu Baolu wrote:
> Lu Baolu (4):
>   iommu: Clean up the comments for iommu_group_alloc
>   iommu/vt-d: Clean up unused variable in find_or_alloc_domain
>   iommu/vt-d: Clean up pasid quirk for pre-production devices
>   iommu/vt-d: Remove unnecessary parentheses
> 
>  drivers/iommu/intel-iommu.c | 36 +++---------------------------------
>  drivers/iommu/intel-svm.c   |  2 +-
>  drivers/iommu/iommu.c       |  1 -
>  include/linux/intel-iommu.h |  1 -
>  4 files changed, 4 insertions(+), 36 deletions(-)

Applied, thanks.

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

* Re: [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices
  2018-05-04  5:08 ` [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices Lu Baolu
@ 2018-07-07  7:01   ` Chris Wilson
  2018-07-07  7:21     ` Lu Baolu
  0 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2018-07-07  7:01 UTC (permalink / raw)
  To: David Woodhouse, Joerg Roedel, Lu Baolu
  Cc: ashok.raj, iommu, linux-kernel, Lu Baolu, Balestrieri, Francesco

Quoting Lu Baolu (2018-05-04 06:08:18)
> The pasid28 quirk is needed only for some pre-production devices.
> Remove it to make the code concise.

Every skylake mixing iommu and i915 is now inoperable on boot. Reads
by the GPU from iommu map pages return zero, writes disappear into the
void, no errors flagged.

Please revert until the matter is resolved.
-Chris

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

* Re: [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices
  2018-07-07  7:01   ` Chris Wilson
@ 2018-07-07  7:21     ` Lu Baolu
  0 siblings, 0 replies; 8+ messages in thread
From: Lu Baolu @ 2018-07-07  7:21 UTC (permalink / raw)
  To: Chris Wilson, David Woodhouse, Joerg Roedel
  Cc: ashok.raj, iommu, linux-kernel, Balestrieri, Francesco

Hi Chris,

On 07/07/2018 03:01 PM, Chris Wilson wrote:
> Quoting Lu Baolu (2018-05-04 06:08:18)
>> The pasid28 quirk is needed only for some pre-production devices.
>> Remove it to make the code concise.
> Every skylake mixing iommu and i915 is now inoperable on boot. Reads
> by the GPU from iommu map pages return zero, writes disappear into the
> void, no errors flagged.
>
> Please revert until the matter is resolved.

Yes. I also got reports about the i915 issue.

I will submit a revert patch as soon as possible.

I am sorry for the inconvenience.

Best regards,
Lu Baolu

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

end of thread, other threads:[~2018-07-07  7:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-04  5:08 [PATCH 0/4] iommu/vt-d: Several cleanup patches Lu Baolu
2018-05-04  5:08 ` [PATCH 1/4] iommu: Clean up the comments for iommu_group_alloc Lu Baolu
2018-05-04  5:08 ` [PATCH 2/4] iommu/vt-d: Clean up unused variable in find_or_alloc_domain Lu Baolu
2018-05-04  5:08 ` [PATCH 3/4] iommu/vt-d: Clean up pasid quirk for pre-production devices Lu Baolu
2018-07-07  7:01   ` Chris Wilson
2018-07-07  7:21     ` Lu Baolu
2018-05-04  5:08 ` [PATCH 4/4] iommu/vt-d: Remove unnecessary parentheses Lu Baolu
2018-05-15 14:35 ` [PATCH 0/4] iommu/vt-d: Several cleanup patches Joerg Roedel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).