From: Jan Kiszka <jan.kiszka@siemens.com>

Obtail the new pgd pointer before releasing the page containing this
value.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

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--;
 	}
-- 
1.7.1
--

[ message continues ]

Reviewed-by: Sheng Yang <sheng@linux.intel.com>

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@intel.com>
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@intel.com>

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?

--
regards
Yang, Sheng
--

[ message continues ]

CONFIG_64BIT is on here, so this change did not make a difference for me.

Jan

[ message continues ]

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
--

[ message continues ]

In fact this one shouldn't affect the result. Wrong guess...

--
regards
--

[ message continues ]

Ping...

I think this fix also qualifies for stable (.35 and .36).

Jan


[ message continues ]

Still not merged?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux
--

[ message continues ]

David, do you plan to pick this one up?

thanks,
-chris
--

[ message continues ]

Hmm, still no reaction. Trying David's Intel address now...

Jan

[ message continues ]