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.
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 <[email protected]>
---
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;
+ }
if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
ret = intel_pasid_alloc_table(dev);
@@ -4336,7 +4338,8 @@ static void intel_iommu_release_device(struct device *dev)
struct intel_iommu *iommu = info->iommu;
mutex_lock(&iommu->iopf_lock);
- device_rbtree_remove(info);
+ if (dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev)))
+ device_rbtree_remove(info);
mutex_unlock(&iommu->iopf_lock);
if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev) &&
--
2.34.1
> From: Lu Baolu <[email protected]>
> 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?
>
> 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 <[email protected]>
> ---
> 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?
>
> if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
> ret = intel_pasid_alloc_table(dev);
> @@ -4336,7 +4338,8 @@ static void intel_iommu_release_device(struct
> device *dev)
> struct intel_iommu *iommu = info->iommu;
>
> mutex_lock(&iommu->iopf_lock);
> - device_rbtree_remove(info);
> + if (dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev)))
> + device_rbtree_remove(info);
> mutex_unlock(&iommu->iopf_lock);
>
> if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev) &&
> --
> 2.34.1
On 2024/4/8 11:52, Tian, Kevin wrote:
>> From: Lu Baolu<[email protected]>
>> 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<[email protected]>
>> ---
>> 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
+Bjorn
> From: Baolu Lu <[email protected]>
> Sent: Monday, April 8, 2024 3:15 PM
>
> On 2024/4/8 11:52, Tian, Kevin wrote:
> >> From: Lu Baolu<[email protected]>
> >> 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.
>
what is the pci core policy on such device? pci_enable_ats() doesn't
prevent such device and I saw various places in pci core do handle
the PCI_EXP_TYPE_PCIE_BRIDGE type...
> From: Baolu Lu <[email protected]>
> Sent: Thursday, April 11, 2024 10:48 AM
>
> On 4/7/24 9:14 AM, Lu Baolu wrote:
> > 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.
> >
> > 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<[email protected]>
> > ---
> > drivers/iommu/intel/iommu.c | 11 +++++++----
> > 1 file changed, 7 insertions(+), 4 deletions(-)
>
> This patch closes a boot bug described here.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10689
>
> I will queue it for v6.9-rc if no objection.
>
let's do it. the issue which I raised is rare and can be tackled when
it arises.
Reviewed-by: Kevin Tian <[email protected]>
On 4/7/24 9:14 AM, Lu Baolu wrote:
> 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.
>
> 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<[email protected]>
> ---
> drivers/iommu/intel/iommu.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
This patch closes a boot bug described here.
Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/10689
I will queue it for v6.9-rc if no objection.
Best regards,
baolu
On 4/8/2024 11:52 AM, Tian, Kevin wrote:
>> From: Lu Baolu <[email protected]>
>> 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?
No PCI-to-PCIe bridges such device AFAIK, ATS TLP couldn't be
routed via PCIe-to-PCI bridge unmodified. ATS is based on PCIe.
But the device-rbtree is possible holding mutiple device nodes behind
PCIe-to-PCI bridge indexed by the same requester-ID.
Thanks,
Ethan
>> 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 <[email protected]>
>> ---
>> 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?
>
>> if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
>> ret = intel_pasid_alloc_table(dev);
>> @@ -4336,7 +4338,8 @@ static void intel_iommu_release_device(struct
>> device *dev)
>> struct intel_iommu *iommu = info->iommu;
>>
>> mutex_lock(&iommu->iopf_lock);
>> - device_rbtree_remove(info);
>> + if (dev_is_pci(dev) && pci_ats_supported(to_pci_dev(dev)))
>> + device_rbtree_remove(info);
>> mutex_unlock(&iommu->iopf_lock);
>>
>> if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev) &&
>> --
>> 2.34.1
>