On 2022/4/23 16:51, Lu Baolu wrote:
On 2022/4/23 16:37, Robin Murphy wrote:
On 2022-04-23 09:01, Lu Baolu wrote:
Hi Robin,
On 2022/4/19 15:20, Robin Murphy wrote:
On 2022-04-19 00:37, Lu Baolu wrote:
On 2022/4/19 6:09, Robin Murphy wrote:
On 2022-04-16 01:04, Lu Baolu wrote:
On 2022/4/14 20:42, Robin Murphy wrote:
@@ -1883,27 +1900,12 @@ static int iommu_bus_init(struct
bus_type *bus)
*/
int bus_set_iommu(struct bus_type *bus, const struct iommu_ops
*ops)
{
- int err;
-
- if (ops == NULL) {
- bus->iommu_ops = NULL;
- return 0;
- }
-
- if (bus->iommu_ops != NULL)
+ if (bus->iommu_ops && ops && bus->iommu_ops != ops)
return -EBUSY;
bus->iommu_ops = ops;
Do we still need to keep above lines in bus_set_iommu()?
It preserves the existing behaviour until each callsite and its
associated error handling are removed later on, which seems like
as good a thing to do as any. Since I'm already relaxing
iommu_device_register() to a warn-but-continue behaviour while it
keeps the bus ops on life-support internally, I figured not
changing too much at once would make it easier to bisect any
potential issues arising from this first step.
Fair enough. Thank you for the explanation.
Do you have a public tree that I could pull these patches and try them
on an Intel hardware? Or perhaps you have done this? I like the whole
idea of this series, but it's better to try it with a real hardware.
I haven't bothered with separate branches since there's so many
different pieces in-flight, but my complete (unstable) development
branch can be found here:
https://gitlab.arm.com/linux-arm/linux-rm/-/commits/iommu/bus
For now I'd recommend winding the head back to "iommu: Clean up
bus_set_iommu()" for testing - some of the patches above that have
already been posted and picked up by their respective subsystems,
but others are incomplete and barely compile-tested. I'll probably
rearrange it later this week to better reflect what's happened so far.
I wound the head back to "iommu: Clean up bus_set_iommu" and tested it
on an Intel machine. It got stuck during boot. This test was on a remote
machine and I have no means to access it physically. So I can't get any
kernel debugging messages. (I have to work from home these days. :-()
I guess it's due to the fact that intel_iommu_probe_device() callback
only works for the pci devices. The issue occurs when probing a device
other than a PCI one.
Yeah, I was wondering if that would be significant, since it's the
only driver that never registered itself for platform_bus_type so
won't have actually seen those calls before. I'm inclined to bodge
that as below for now, as long as it then works OK in terms of the
rest of the changes.
Thanks,
Robin.
----->8-----
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9fa1b98186a3..6e359f92ec00 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4565,6 +4565,10 @@ static struct iommu_device
*intel_iommu_probe_device(struct device *dev)
unsigned long flags;
u8 bus, devfn;
+ /* ANDD platform device support needs fixing */
+ if (!pdev)
+ return ERR_PTR(-ENODEV);
+
iommu = device_to_iommu(dev, &bus, &devfn);
if (!iommu)
return ERR_PTR(-ENODEV);
I haven't seen any real ANDD platform devices, hence this works for me.
Or more precisely, return NULL when @dev goes through device_to_iommu()?
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index df5c62ecf942..0d447739e076 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -797,8 +797,11 @@ struct intel_iommu *device_to_iommu(struct device
*dev, u8 *bus, u8 *devfn)
pf_pdev = pci_physfn(pdev);
dev = &pf_pdev->dev;
segment = pci_domain_nr(pdev->bus);
- } else if (has_acpi_companion(dev))
+ } else if (has_acpi_companion(dev)) {
dev = &ACPI_COMPANION(dev)->dev;
+ } else {
+ return NULL;
+ }
rcu_read_lock();
for_each_iommu(iommu, drhd) {
Best regards,
baolu
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
