On Tue, Jan 14, 2025 at 11:21:13PM -0500, Yury Norov wrote: > > +static int iommufd_sw_msi_install(struct iommufd_ctx *ictx, > > + struct iommufd_hwpt_paging *hwpt_paging, > > + struct iommufd_sw_msi_map *msi_map) > > +{ > > + unsigned long iova; > > + > > + lockdep_assert_held(&ictx->sw_msi_lock); > > + > > + iova = msi_map->sw_msi_start + msi_map->pgoff * PAGE_SIZE; > > + if (!test_bit(msi_map->id, hwpt_paging->present_sw_msi.bitmap)) { > > + int rc; > > + > > + rc = iommu_map(hwpt_paging->common.domain, iova, > > + msi_map->msi_addr, PAGE_SIZE, > > + IOMMU_WRITE | IOMMU_READ | IOMMU_MMIO, > > + GFP_KERNEL_ACCOUNT); > > + if (rc) > > + return rc; > > + set_bit(msi_map->id, hwpt_paging->present_sw_msi.bitmap); > > + } > > + return 0; > > +} > > So, does sw_msi_lock protect the present_sw_msi bitmap? If so, you > should use non-atomic __set_bit(). Yes, that is a good point Thanks, Jason