Hi Qian, On Thu, Jun 25, 2020 at 11:37:20AM -0400, Qian Cai wrote: > On Thu, Jun 25, 2020 at 04:52:27PM +0200, Joerg Roedel wrote: > > - u64 pt_root = atomic64_read(&domain->pt_root); > > + unsigned long pt_root = domain->pt_root; > > The pt_root might be reload later in case of register pressure where the > compiler decides to not store it as a stack variable, so it needs > smp_rmb() here to match to the smp_wmb() in > amd_iommu_domain_set_pt_root() to make the load visiable to all CPUs. > > Then, smp_rmb/wmb() wouldn't be able to deal with data races, so it > needs, > > unsigned long pt_root = READ_ONCE(domain->pt_root); > > > > > pgtable->root = (u64 *)(pt_root & PAGE_MASK); > > pgtable->mode = pt_root & 7; /* lowest 3 bits encode pgtable mode */ > > @@ -164,7 +164,13 @@ static void amd_iommu_domain_get_pgtable(struct > > protection_domain *domain, > > > > static void amd_iommu_domain_set_pt_root(struct protection_domain *domain, > > u64 root) > > { > > - atomic64_set(&domain->pt_root, root); > > + domain->pt_root = root; > > WRITE_ONCE(domain->pt_root, root);
Thanks for your review. I addressed your comments and will send an updated version shortly. Regards, Joerg _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu