Messages in this thread | | | Subject | Re: [RFC PATCH] iommu/vt-d: Fix IOMMU field not populated on device hot re-plug | From | Lu Baolu <> | Date | Wed, 28 Aug 2019 08:56:18 +0800 |
| |
Hi Janusz,
On 8/27/19 5:35 PM, Janusz Krzysztofik wrote: > Hi Lu, > > On Monday, August 26, 2019 10:29:12 AM CEST Lu Baolu wrote: >> Hi Janusz, >> >> On 8/26/19 4:15 PM, Janusz Krzysztofik wrote: >>> Hi Lu, >>> >>> On Friday, August 23, 2019 3:51:11 AM CEST Lu Baolu wrote: >>>> Hi, >>>> >>>> On 8/22/19 10:29 PM, Janusz Krzysztofik wrote: >>>>> When a perfectly working i915 device is hot unplugged (via sysfs) and >>>>> hot re-plugged again, its dev->archdata.iommu field is not populated >>>>> again with an IOMMU pointer. As a result, the device probe fails on >>>>> DMA mapping error during scratch page setup. >>>>> >>>>> It looks like that happens because devices are not detached from their >>>>> MMUIO bus before they are removed on device unplug. Then, when an >>>>> already registered device/IOMMU association is identified by the >>>>> reinstantiated device's bus and function IDs on IOMMU bus re-attach >>>>> attempt, the device's archdata is not populated with IOMMU information >>>>> and the bad happens. >>>>> >>>>> I'm not sure if this is a proper fix but it works for me so at least it >>>>> confirms correctness of my analysis results, I believe. So far I >>>>> haven't been able to identify a good place where the possibly missing >>>>> IOMMU bus detach on device unplug operation could be added. >>>> >>>> Which kernel version are you testing with? Does it contain below commit? >>>> >>>> commit 458b7c8e0dde12d140e3472b80919cbb9ae793f4 >>>> Author: Lu Baolu <baolu.lu@linux.intel.com> >>>> Date: Thu Aug 1 11:14:58 2019 +0800 >>> >>> I was using an internal branch based on drm-tip which didn't contain this >>> commit yet. Fortunately it has been already merged into drm-tip over last >>> weekend and has effectively fixed the issue. >> >> Thanks for testing this. > > My testing appeared not sufficiently exhaustive. The fix indeed resolved my > initially discovered issue of not being able to rebind the i915 driver to a > re-plugged device, however it brought another, probably more serious problem > to light. > > When an open i915 device is hot unplugged, IOMMU bus notifier now cleans up > IOMMU info for the device on PCI device remove while the i915 driver is still > not released, kept by open file descriptors. Then, on last device close, > cleanup attempts lead to kernel panic raised from intel_unmap() on unresolved > IOMMU domain.
We should avoid kernel panic when a intel_unmap() is called against a non-existent domain. But we shouldn't expect the IOMMU driver not cleaning up the domain info when a device remove notification comes and wait until all file descriptors being closed, right?
Best regards, Baolu
> > With commit 458b7c8e0dde reverted and my fix applied, both late device close > and device re-plug work for me. However, I can realize that's probably still > not a complete solution, possibly missing some protection against reuse of a > removed device other than for cleanup. If you think that's the right way to > go, I can work more on that. > > I've had a look at other drivers and found AMD is using somehow similar > approach. On the other hand, looking at the IOMMU common code I couldn't > identify any arrangement that would support deferred device cleanup. > > If that approach is not acceptable for Intel IOMMU, please suggest a way you'd > like to have it resolved and I can try to implement it. > > Thanks, > Janusz > >> Best regards, >> Lu Baolu >>
| |