[regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Linux regression tracking (Thorsten Leemhuis)]

Hi, Thorsten here, the Linux kernel's regression tracker.

Lu Baolu, I noticed a report about a regression in bugzilla.kernel.org
that appears to be caused by a change of yours:

2b989ab9bc89b2 ("iommu/vt-d: Add helper to allocate paging domain")
[v6.11-rc1]

As many (most?) kernel developers don't keep an eye on the bug tracker,
I decided to forward it with this mail. To quote from
https://bugzilla.kernel.org/show_bug.cgi?id=219198 :

> USB is working on my Dell WD19TB Thunderbolt Dock with Fedora kernel build 6.10.0-64.fc41.x86_64
> 
> It does not work with 6.11.0-0.rc4.38.fc41.x86_64 anymore.

The user later bisected the problem with a vanilla kernel to the commit
of yours. There are various error messages in comments to that ticket.
Some of them:

> Aug 27 11:51:07 b0v9by3 kernel: DMAR: DRHD: handling fault status reg 2
> Aug 27 11:51:07 b0v9by3 kernel: DMAR: [DMA Read NO_PASID] Request device [04:00.0] fault addr 0xffffe000 [fault reason 0x0c] non-zero reserved fields in PTE
and

> Aug 26 10:33:49 foobar kernel: xhci_hcd 0000:04:00.0: Abort failed to stop command ring: -110
> Aug 26 10:33:49 foobar kernel: xhci_hcd 0000:04:00.0: xHCI host controller not responding, assume dead
> Aug 26 10:33:49 foobar kernel: xhci_hcd 0000:04:00.0: HC died; cleaning up
> Aug 26 10:33:49 foobar kernel: xhci_hcd 0000:04:00.0: Error while assigning device slot ID: Command Aborted
> Aug 26 10:33:49 foobar kernel: xhci_hcd 0000:04:00.0: Max number of devices this xHCI host supports is 64.
> Aug 26 10:33:49 foobar kernel: usb usb5-port2: couldn't allocate usb_device
> Aug 26 10:33:49 foobar kernel: usb usb6-port2: couldn't allocate usb_device
> Aug 26 10:33:57 foobar kernel: usb usb2-port1: unable to enumerate USB device

See the ticket for way more details.

The initial report is from Markus Rathgeb, who is CCed. Adam Williamson
(also CCed) deal with similar symptoms on another machine. Both are from
Dell. In both cases Thunderbolt is involved, too.

There is also another report with somewhat similar errors messages, also
from a Dell machine:
https://lore.kernel.org/all/3b42a3cc-2f76-4d42-abe2-5f4e8ffe10e4@molgen.mpg.de/

Not sure if that's the the same problem, hence did not CC that reporter.
Will send a heads-up to that thread and point to this thread and the ticket.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.

[1]

P.S.: let me use this mail to also add the report to the list of tracked
regressions to ensure it's doesn't fall through the cracks:

#regzbot introduced: f90584f4beb84211c
#regzbot title: iommu/vt-d: usb and thunderbould are misbehaving/broken
#regzbot from: Markus Rathgeb <maggu2810@gmail.com>
#regzbot duplicate: https://bugzilla.kernel.org/show_bug.cgi?id=219198
#regzbot ignore-activity

RE: [regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Tian, Kevin]

> From: Linux regression tracking (Thorsten Leemhuis)
> <regressions@leemhuis.info>
> 
> Hi, Thorsten here, the Linux kernel's regression tracker.
> 
> Lu Baolu, I noticed a report about a regression in bugzilla.kernel.org
> that appears to be caused by a change of yours:
> 
> 2b989ab9bc89b2 ("iommu/vt-d: Add helper to allocate paging domain")
> [v6.11-rc1]
> 

This doesn't appear to be the cause as it only affects the scenario 
which assigns a device to userspace.

Probably f90584f4beb8 ("iommu/vt-d: Add helper to flush caches
for context change") is more relevant. There was a bug reported
against it and fixed a few days ago:

https://lore.kernel.org/linux-iommu/20240815124857.70038-1-baolu.lu@linux.intel.com/

You may have a try.

Re: [regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Markus Rathgeb]

> Probably f90584f4beb8 ("iommu/vt-d: Add helper to flush caches
> for context change") is more relevant. There was a bug reported
> against it and fixed a few days ago:

I added a comment on bugzilla, too.
While adding additional logs to the bugzilla entry I realized that one
commit has not been tested.

good: 804f98e224e41c16e3b70f97790f84894745a392
untested: 2b989ab9bc89b29dd4b5509408b8fa42337eda56
bad: f90584f4beb84211c4d21b319cc13f391fe9f3c2

I do not understand why git bisect told me there is no remaining commit to test.

Re: [regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Markus Rathgeb]

I did a git reset to 2b989ab9bc89b29dd4b5509408b8fa42337eda56
Build and tested the kernel.

This seems to be a good commit.

So, the first bad commit on my side seems to be
f90584f4beb84211c4d21b319cc13f391fe9f3c2

Re: [regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Markus Rathgeb]

While reading the diff of f90584f4beb84211c4d21b319cc13f391fe9f3c2 I
wonder if the following change is correct.

It is about iommu.c and the change to domain_context_clear_one function.

Previously you first store the domain_id to did_old and then you call
context_clear_entry.
After that you use iommu->flush.flush_context and provide the did_old,
so the value before clearing the entry.

Now, you call context_clear_entry and after that you call
intel_context_flush_present that contains the remaining part.
In intel_context_flush_present the domain_id will betaken (AFTER it
has been cleared) and call flush_context with it.

Is this done by intention or changed behaviour?

Re: [regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Markus Rathgeb]

With respect to my previous comment I tested to fix it myself (I am
not a kernel hacker and do not know anything about iommu etc.).

After applying the following change to the v6.11-rc5 it seems to fix my problem.
I can connect, disconnect and connect the dock and USB is working and
no DMAR error.




From 5f621c079f0f8bce9895ae05a9cd81b001a58089 Mon Sep 17 00:00:00 2001
From: Markus Rathgeb <maggu2810@gmail.com>
Date: Thu, 29 Aug 2024 09:37:17 +0200
Subject: [PATCH] fix: iommu/vt-d: Add helper to flush caches for context
 change

Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>
---
 drivers/iommu/intel/iommu.c | 7 +++++--
 drivers/iommu/intel/iommu.h | 3 ++-
 drivers/iommu/intel/pasid.c | 4 ++--
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9ff8b83c19a3..e92e06e604b2 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1944,6 +1944,7 @@ static void domain_context_clear_one(struct
device_domain_info *info, u8 bus, u8
 {
     struct intel_iommu *iommu = info->iommu;
     struct context_entry *context;
+    u16 did_old;

     spin_lock(&iommu->lock);
     context = iommu_context_addr(iommu, bus, devfn, 0);
@@ -1952,10 +1953,12 @@ static void domain_context_clear_one(struct
device_domain_info *info, u8 bus, u8
         return;
     }

+    did_old = context_domain_id(context);
+
     context_clear_entry(context);
     __iommu_flush_cache(iommu, context, sizeof(*context));
     spin_unlock(&iommu->lock);
-    intel_context_flush_present(info, context, true);
+    intel_context_flush_present(info, context, did_old, true);
 }

 static int domain_setup_first_level(struct intel_iommu *iommu,
@@ -4269,7 +4272,7 @@ static int context_flip_pri(struct
device_domain_info *info, bool enable)

     if (!ecap_coherent(iommu->ecap))
         clflush_cache_range(context, sizeof(*context));
-    intel_context_flush_present(info, context, true);
+    intel_context_flush_present(info, context,
context_domain_id(context), true);
     spin_unlock(&iommu->lock);

     return 0;
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index b67c14da1240..0a4ce98faa60 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -988,7 +988,7 @@ static inline int context_domain_id(struct context_entry *c)
     return((c->hi >> 8) & 0xffff);
 }

-static inline void context_clear_entry(struct context_entry *context)
+static inline void     context_clear_entry(struct context_entry *context)
 {
     context->lo = 0;
     context->hi = 0;
@@ -1154,6 +1154,7 @@ void cache_tag_flush_range_np(struct dmar_domain
*domain, unsigned long start,

 void intel_context_flush_present(struct device_domain_info *info,
                  struct context_entry *context,
+                 u16 did,
                  bool affect_domains);

 #ifdef CONFIG_INTEL_IOMMU_SVM
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 5792c817cefa..cb4bbb45ac2a 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -694,7 +694,7 @@ static void device_pasid_table_teardown(struct
device *dev, u8 bus, u8 devfn)
     context_clear_entry(context);
     __iommu_flush_cache(iommu, context, sizeof(*context));
     spin_unlock(&iommu->lock);
-    intel_context_flush_present(info, context, false);
+    intel_context_flush_present(info, context,
context_domain_id(context), false);
 }

 static int pci_pasid_table_teardown(struct pci_dev *pdev, u16 alias,
void *data)
@@ -885,10 +885,10 @@ static void __context_flush_dev_iotlb(struct
device_domain_info *info)
  */
 void intel_context_flush_present(struct device_domain_info *info,
                  struct context_entry *context,
+                 u16 did,
                  bool flush_domains)
 {
     struct intel_iommu *iommu = info->iommu;
-    u16 did = context_domain_id(context);
     struct pasid_entry *pte;
     int i;

-- 
2.46.0

RE: [regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Tian, Kevin]

> From: Markus Rathgeb <maggu2810@gmail.com>
> Sent: Thursday, August 29, 2024 4:34 PM
> 
> With respect to my previous comment I tested to fix it myself (I am
> not a kernel hacker and do not know anything about iommu etc.).
> 
> After applying the following change to the v6.11-rc5 it seems to fix my
> problem.
> I can connect, disconnect and connect the dock and USB is working and
> no DMAR error.
> 

as said there was already a fix posted which is similar to below:

https://lore.kernel.org/linux-iommu/20240815124857.70038-1-baolu.lu@linux.intel.com/

> 
> 
> 
> From 5f621c079f0f8bce9895ae05a9cd81b001a58089 Mon Sep 17 00:00:00
> 2001
> From: Markus Rathgeb <maggu2810@gmail.com>
> Date: Thu, 29 Aug 2024 09:37:17 +0200
> Subject: [PATCH] fix: iommu/vt-d: Add helper to flush caches for context
>  change
> 
> Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>
> ---
>  drivers/iommu/intel/iommu.c | 7 +++++--
>  drivers/iommu/intel/iommu.h | 3 ++-
>  drivers/iommu/intel/pasid.c | 4 ++--
>  3 files changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 9ff8b83c19a3..e92e06e604b2 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1944,6 +1944,7 @@ static void domain_context_clear_one(struct
> device_domain_info *info, u8 bus, u8
>  {
>      struct intel_iommu *iommu = info->iommu;
>      struct context_entry *context;
> +    u16 did_old;
> 
>      spin_lock(&iommu->lock);
>      context = iommu_context_addr(iommu, bus, devfn, 0);
> @@ -1952,10 +1953,12 @@ static void domain_context_clear_one(struct
> device_domain_info *info, u8 bus, u8
>          return;
>      }
> 
> +    did_old = context_domain_id(context);
> +
>      context_clear_entry(context);
>      __iommu_flush_cache(iommu, context, sizeof(*context));
>      spin_unlock(&iommu->lock);
> -    intel_context_flush_present(info, context, true);
> +    intel_context_flush_present(info, context, did_old, true);
>  }
> 
>  static int domain_setup_first_level(struct intel_iommu *iommu,
> @@ -4269,7 +4272,7 @@ static int context_flip_pri(struct
> device_domain_info *info, bool enable)
> 
>      if (!ecap_coherent(iommu->ecap))
>          clflush_cache_range(context, sizeof(*context));
> -    intel_context_flush_present(info, context, true);
> +    intel_context_flush_present(info, context,
> context_domain_id(context), true);
>      spin_unlock(&iommu->lock);
> 
>      return 0;
> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index b67c14da1240..0a4ce98faa60 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -988,7 +988,7 @@ static inline int context_domain_id(struct
> context_entry *c)
>      return((c->hi >> 8) & 0xffff);
>  }
> 
> -static inline void context_clear_entry(struct context_entry *context)
> +static inline void     context_clear_entry(struct context_entry *context)
>  {
>      context->lo = 0;
>      context->hi = 0;
> @@ -1154,6 +1154,7 @@ void cache_tag_flush_range_np(struct
> dmar_domain
> *domain, unsigned long start,
> 
>  void intel_context_flush_present(struct device_domain_info *info,
>                   struct context_entry *context,
> +                 u16 did,
>                   bool affect_domains);
> 
>  #ifdef CONFIG_INTEL_IOMMU_SVM
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index 5792c817cefa..cb4bbb45ac2a 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -694,7 +694,7 @@ static void device_pasid_table_teardown(struct
> device *dev, u8 bus, u8 devfn)
>      context_clear_entry(context);
>      __iommu_flush_cache(iommu, context, sizeof(*context));
>      spin_unlock(&iommu->lock);
> -    intel_context_flush_present(info, context, false);
> +    intel_context_flush_present(info, context,
> context_domain_id(context), false);
>  }
> 
>  static int pci_pasid_table_teardown(struct pci_dev *pdev, u16 alias,
> void *data)
> @@ -885,10 +885,10 @@ static void __context_flush_dev_iotlb(struct
> device_domain_info *info)
>   */
>  void intel_context_flush_present(struct device_domain_info *info,
>                   struct context_entry *context,
> +                 u16 did,
>                   bool flush_domains)
>  {
>      struct intel_iommu *iommu = info->iommu;
> -    u16 did = context_domain_id(context);
>      struct pasid_entry *pte;
>      int i;
> 
> --
> 2.46.0

Re: [regression] usb and thunderbould are misbehaving or broken due to iommu/vt-d change

[Baolu Lu]

On 2024/8/30 8:29, Tian, Kevin wrote:
>> From: Markus Rathgeb<maggu2810@gmail.com>
>> Sent: Thursday, August 29, 2024 4:34 PM
>>
>> With respect to my previous comment I tested to fix it myself (I am
>> not a kernel hacker and do not know anything about iommu etc.).
>>
>> After applying the following change to the v6.11-rc5 it seems to fix my
>> problem.
>> I can connect, disconnect and connect the dock and USB is working and
>> no DMAR error.
>>
> as said there was already a fix posted which is similar to below:
> 
> https://lore.kernel.org/linux-iommu/20240815124857.70038-1-baolu.lu@linux.intel.com/

Above patch has been merged in v6.11-rc6. Please try the latest kernel
version.

Thanks,
baolu