Messages in this thread | | | Date | Sat, 23 Apr 2022 16:51:23 +0800 | Subject | Re: [PATCH 02/13] iommu: Move bus setup to IOMMU device registration | From | Lu Baolu <> |
| |
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.
Best regards, baolu
| |