On Wed, Aug 09, 2023 at 03:38:30PM +0200, Marek Szyprowski wrote:
> On 09.08.2023 14:15, Jason Gunthorpe wrote:
> > On Wed, Aug 09, 2023 at 04:01:42AM +0000, Tian, Kevin wrote:
> >>> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> >>> Sent: Wednesday, August 9, 2023 1:27 AM
> >>>
> >>> @@ -1800,11 +1801,18 @@ struct probe_iommu_args {
> >>> static int probe_iommu_group(struct device *dev, void *data)
> >>> {
> >>> struct probe_iommu_args *args = data;
> >>> + bool need_lock;
> >>> int ret;
> >>>
> >>> - device_lock(dev);
> >>> + /* Probing the iommu itself is always done under the device_lock */
> >>> + need_lock = !args->iommu || args->iommu->hwdev != dev;
> >>> +
> >> is !args->iommu a valid case?
> > Hmm, not any more, it used to happen in an earlier version
> >
> >> btw probably a dumb question. Why do we continue to probe the
> >> iommu device itself instead of skipping it? The group is a concept
> >> for devices which require DMA protection from iommu instead of
> >> for the iommu device itself...
> > Yeah, that is how I originally did it, but since the locking appeared
> > here I thought it would be safer to just continue to invoke probe as
> > we have always done. I don't know for sure that there isn't some
> > driver that relies on this for some reason.
> >
> > eg it might change the group layouts or something.
>
> Well, I don't think that there is any driver doing such things, but the
> only way to check is simply change the behavior an wait for complaints.
My other concern is that we don't fully block iommu devices from being
self probed. They can still be probed during rescan and during 2nd
calls to bus_iommu_probe().
But OK, it makes more logical sense to just block it so lets try that.
Jason
