On Tuesday 02 November 2010 15:46:11 Jan Kiszka wrote: > Am 02.11.2010 08:31, Sheng Yang wrote: > > On Tuesday 02 November 2010 15:05:51 Jan Kiszka wrote: > >> From: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> > >> > >> Obtail the new pgd pointer before releasing the page containing this > >> value. > >> > >> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> > >> --- > >> > >> Who is taking care of this? The kvm tree? > >> > >> drivers/pci/intel-iommu.c | 2 +- > >> 1 files changed, 1 insertions(+), 1 deletions(-) > >> > >> diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c > >> index 4789f8e..35463dd 100644 > >> --- a/drivers/pci/intel-iommu.c > >> +++ b/drivers/pci/intel-iommu.c > >> @@ -3627,9 +3627,9 @@ static int intel_iommu_attach_device(struct > >> iommu_domain *domain, > >> > >> pte = dmar_domain->pgd; > >> if (dma_pte_present(pte)) { > >> > >> - free_pgtable_page(dmar_domain->pgd); > >> > >> dmar_domain->pgd = (struct dma_pte *) > >> > >> phys_to_virt(dma_pte_addr(pte)); > >> > >> + free_pgtable_page(pte); > >> > >> } > >> dmar_domain->agaw--; > >> > >> } > > > > Reviewed-by: Sheng Yang <sheng@xxxxxxxxxxxxxxx> > > > > CC iommu mailing list and David. > > > > OK, Jan, I got your meaning now. And it's not the exactly swap. :) > > > > I think the old code is safe, seems it's broken(exposed) by: > > > > commit 1a8bd481bfba30515b54368d90a915db3faf302f > > Author: David Woodhouse <David.Woodhouse@xxxxxxxxx> > > Date: Tue Aug 10 01:38:53 2010 +0100 > > > > intel-iommu: Fix 32-bit build warning with __cmpxchg() > > > > drivers/pci/intel-iommu.c: In function 'dma_pte_addr': > > drivers/pci/intel-iommu.c:239: warning: passing argument 1 of > > '__cmpxchg64' > > > > from incompatible pointer typ > > > > It seems that __cmpxchg64() now cares about the type of its pointer > > argument, so give it a (uint64_t *) instead of a pointer to a > > structure which contains only that. > > > > Signed-off-by: David Woodhouse <David.Woodhouse@xxxxxxxxx> > > > > diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c > > index c9171be..603cdc0 100644 > > --- a/drivers/pci/intel-iommu.c > > +++ b/drivers/pci/intel-iommu.c > > @@ -236,7 +236,7 @@ static inline u64 dma_pte_addr(struct dma_pte *pte) > > > > return pte->val & VTD_PAGE_MASK; > > > > #else > > > > /* Must have a full atomic 64-bit read */ > > > > - return __cmpxchg64(pte, 0ULL, 0ULL) & VTD_PAGE_MASK; > > + return __cmpxchg64(&pte->val, 0ULL, 0ULL) & VTD_PAGE_MASK; > > > > #endif > > } > > > > Seems here is the only affected code? > > CONFIG_64BIT is on here, so this change did not make a difference for me. Oh... Then it would due to most VT-d machine wouldn't run into while (iommu->agaw < dmar_domain->agaw). We have routing test for VT-d devices assignment, but seems we don't use this kind of VT-d machine for testing. -- regards Yang, Sheng > > Jan -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html