On Thu, Mar 21, 2024 at 07:26:41PM +0800, Yi Liu wrote: > > yes, the correct way is to undo what have been done before the fail > > device. However, I somehow remember that pasid capability is only > > available when the group is singleton. So iterate all devices of the > > devices just means one device in fact. If this is true, then the > > current code is fine although a bit confusing. Platform devicse don't have that limitation.. It is PCI only. > > > And the whole thing is easier to reason about if an input argument > > > specifies the current attached domain instead of having the driver > > > read it from the xarray. > > > > yep, will correct it as a fix patch. > > Hi Jason, > > It appears there are two solutions here. > > First, only undo the devices that have set_dev_pasid successfully in > the __iommu_set_group_pasid(), so the problematic > __iommu_remove_group_pasid() call at line 3378 [1] would go away. > This also makes the helper more self-contained. Draft patch in [2] > > Second, pass in the domain to remove_dev_pasid(). Draft patch in [3] > > Either of the above two should be able to solve the mistake you mentioned. > BTW. They are orthogonal, so it's also possible to apply both of them. > Which one is your preference then? I would do both because I also think it is not nice that the drivers always have to have the boiler plate to read the xarray in their remove.. Jason
