lkml.org 
[lkml]   [2024]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/1] iommu/vt-d: Fix WARN_ON in iommu probe path
From
On 2024/4/8 11:52, Tian, Kevin wrote:
>> From: Lu Baolu<baolu.lu@linux.intel.com>
>> Sent: Sunday, April 7, 2024 9:14 AM
>>
>> Commit 1a75cc710b95 ("iommu/vt-d: Use rbtree to track iommu probed
>> devices") adds all devices probed by the iommu driver in a rbtree
>> indexed by the source ID of each device. It assumes that each device
>> has a unique source ID. This assumption is incorrect and the VT-d
>> spec doesn't state this requirement either.
>>
>> The reason for using a rbtree to track devices is to look up the device
>> with PCI bus and devfunc in the paths of handling ATS invalidation time
>> out error and the PRI I/O page faults. Both are PCI ATS feature related.
>>
>> Only track the devices that have PCI ATS capabilities in the rbtree to
>> avoid unnecessary WARN_ON in the iommu probe path. Otherwise, on some
>> platforms below kernel splat will be displayed and the iommu probe results
>> in failure.
> Just be curious. What about two ATS capable devices putting behind
> a PCI-to-PCIe bridge?

I don't think ATS capable device putting behind a PCI-to-PCIe bridge is
a right configuration for the ATS service. The PCIe spec requires this
in section 10.1.1, that

"
ATS requires the following:
- ATS capable components must interoperate with [PCIe-1.1] compliant
components.
..
"

My understanding is that PCI-to-PCIe bridge is not a PCIe compliant
device.

>
>> WARNING: CPU: 3 PID: 166 at drivers/iommu/intel/iommu.c:158
>> intel_iommu_probe_device+0x319/0xd90
>> Call Trace:
>> <TASK>
>> ? __warn+0x7e/0x180
>> ? intel_iommu_probe_device+0x319/0xd90
>> ? report_bug+0x1f8/0x200
>> ? handle_bug+0x3c/0x70
>> ? exc_invalid_op+0x18/0x70
>> ? asm_exc_invalid_op+0x1a/0x20
>> ? intel_iommu_probe_device+0x319/0xd90
>> ? debug_mutex_init+0x37/0x50
>> __iommu_probe_device+0xf2/0x4f0
>> iommu_probe_device+0x22/0x70
>> iommu_bus_notifier+0x1e/0x40
>> notifier_call_chain+0x46/0x150
>> blocking_notifier_call_chain+0x42/0x60
>> bus_notify+0x2f/0x50
>> device_add+0x5ed/0x7e0
>> platform_device_add+0xf5/0x240
>> mfd_add_devices+0x3f9/0x500
>> ? preempt_count_add+0x4c/0xa0
>> ? up_write+0xa2/0x1b0
>> ? __debugfs_create_file+0xe3/0x150
>> intel_lpss_probe+0x49f/0x5b0
>> ? pci_conf1_write+0xa3/0xf0
>> intel_lpss_pci_probe+0xcf/0x110 [intel_lpss_pci]
>> pci_device_probe+0x95/0x120
>> really_probe+0xd9/0x370
>> ? __pfx___driver_attach+0x10/0x10
>> __driver_probe_device+0x73/0x150
>> driver_probe_device+0x19/0xa0
>> __driver_attach+0xb6/0x180
>> ? __pfx___driver_attach+0x10/0x10
>> bus_for_each_dev+0x77/0xd0
>> bus_add_driver+0x114/0x210
>> driver_register+0x5b/0x110
>> ? __pfx_intel_lpss_pci_driver_init+0x10/0x10 [intel_lpss_pci]
>> do_one_initcall+0x57/0x2b0
>> ? kmalloc_trace+0x21e/0x280
>> ? do_init_module+0x1e/0x210
>> do_init_module+0x5f/0x210
>> load_module+0x1d37/0x1fc0
>> ? init_module_from_file+0x86/0xd0
>> init_module_from_file+0x86/0xd0
>> idempotent_init_module+0x17c/0x230
>> __x64_sys_finit_module+0x56/0xb0
>> do_syscall_64+0x6e/0x140
>> entry_SYSCALL_64_after_hwframe+0x71/0x79
>>
>> Fixes: 1a75cc710b95 ("iommu/vt-d: Use rbtree to track iommu probed
>> devices")
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>> ---
>> drivers/iommu/intel/iommu.c | 11 +++++++----
>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 50eb9aed47cc..a7ecd90303dc 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -4299,9 +4299,11 @@ static struct iommu_device
>> *intel_iommu_probe_device(struct device *dev)
>> }
>>
>> dev_iommu_priv_set(dev, info);
>> - ret = device_rbtree_insert(iommu, info);
>> - if (ret)
>> - goto free;
>> + if (pdev && pci_ats_supported(pdev)) {
>> + ret = device_rbtree_insert(iommu, info);
>> + if (ret)
>> + goto free;
>> + }
> probably replace device_rbtree with ats_rbtree?

It makes sense. Probably I need a follow-up patch to do such cleanup.

Best regards,
baolu

\
 
 \ /
  Last update: 2024-05-27 16:28    [W:0.131 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site