| Message ID | 1623961837-12540-1-git-send-email-amhetre@nvidia.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | iommu/arm-smmu: Fix races in iommu domain/group creation | expand |
On 6/18/2021 2:00 AM, Ashish Mhetre wrote: > Multiple iommu domains and iommu groups are getting created for the devices > sharing same SID. It is expected for devices sharing same SID to be in same > iommu group and same iommu domain. > This is leading to context faults when one device is accessing IOVA from > other device which shouldn't be the case for devices sharing same SID. > Fix this by protecting iommu domain and iommu group creation with mutexes. > > Ashish Mhetre (1): > iommu: Fix race condition during default domain allocation > > Krishna Reddy (1): > iommu/arm-smmu: Fix race condition during iommu_group creation > > drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 +++++- > drivers/iommu/iommu.c | 2 ++ > 2 files changed, 7 insertions(+), 1 deletion(-) > Hi, Can you please help in reviewing this V2 change? Please let me know if anymore changes are required. Thanks, Ashish Mhetre
On Fri, Jun 18, 2021 at 02:00:35AM +0530, Ashish Mhetre wrote: > Multiple iommu domains and iommu groups are getting created for the devices > sharing same SID. It is expected for devices sharing same SID to be in same > iommu group and same iommu domain. > This is leading to context faults when one device is accessing IOVA from > other device which shouldn't be the case for devices sharing same SID. > Fix this by protecting iommu domain and iommu group creation with mutexes. Robin -- any chance you could take a look at these, please? You had some comments on the first version which convinced me that they are needed, but I couldn't tell whether you wanted to solve this a different way or not. Cheers, Will
On 2021-08-02 16:16, Will Deacon wrote: > On Fri, Jun 18, 2021 at 02:00:35AM +0530, Ashish Mhetre wrote: >> Multiple iommu domains and iommu groups are getting created for the devices >> sharing same SID. It is expected for devices sharing same SID to be in same >> iommu group and same iommu domain. >> This is leading to context faults when one device is accessing IOVA from >> other device which shouldn't be the case for devices sharing same SID. >> Fix this by protecting iommu domain and iommu group creation with mutexes. > > Robin -- any chance you could take a look at these, please? You had some > comments on the first version which convinced me that they are needed, > but I couldn't tell whether you wanted to solve this a different way or not. Sorry, I was lamenting that this came to light due to the of_iommu_configure() flow being yucky, but that wasn't meant to imply that there aren't - or couldn't be in future - better reasons for iommu_probe_device() to be robust against concurrency anyway. I do think these are legitimate fixes to make in their own right, even if the current need might get swept back under the rug in future. I would say, however, that the commit messages seem to focus too much on the wrong details and aren't overly useful, and patch #2 is missing Ashish's sign-off. Thanks, Robin.
On Mon, Aug 02, 2021 at 04:46:37PM +0100, Robin Murphy wrote: > On 2021-08-02 16:16, Will Deacon wrote: > > On Fri, Jun 18, 2021 at 02:00:35AM +0530, Ashish Mhetre wrote: > > > Multiple iommu domains and iommu groups are getting created for the devices > > > sharing same SID. It is expected for devices sharing same SID to be in same > > > iommu group and same iommu domain. > > > This is leading to context faults when one device is accessing IOVA from > > > other device which shouldn't be the case for devices sharing same SID. > > > Fix this by protecting iommu domain and iommu group creation with mutexes. > > > > Robin -- any chance you could take a look at these, please? You had some > > comments on the first version which convinced me that they are needed, > > but I couldn't tell whether you wanted to solve this a different way or not. > > Sorry, I was lamenting that this came to light due to the > of_iommu_configure() flow being yucky, but that wasn't meant to imply that > there aren't - or couldn't be in future - better reasons for > iommu_probe_device() to be robust against concurrency anyway. I do think > these are legitimate fixes to make in their own right, even if the current > need might get swept back under the rug in future. > > I would say, however, that the commit messages seem to focus too much on the > wrong details and aren't overly useful, and patch #2 is missing Ashish's > sign-off. Ashish -- please can you send a v3 fixing these issues? Will