From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] AMD IOMMU: don't free page table prematurely Date: Mon, 26 May 2014 16:55:01 +0100 Message-ID: <538363D5.9070206@citrix.com> References: <538330AB0200007800015B2F@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1007172589587719997==" Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WoxEu-0003zT-Hx for xen-devel@lists.xenproject.org; Mon, 26 May 2014 15:55:04 +0000 In-Reply-To: <538330AB0200007800015B2F@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , xen-devel Cc: Aravind Gopalakrishnan , suravee.suthikulpanit@amd.com List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --===============1007172589587719997== Content-Type: multipart/alternative; boundary="------------080909090701080500080406" This is a multi-part message in MIME format. --------------080909090701080500080406 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit On 26/05/2014 11:16, Jan Beulich wrote: > iommu_merge_pages() still wants to look at the next level page table, > the TLB flush necessary before freeing too happens in that function, > and if it fails no free should happen at all. Hence the freeing must > be done after that function returned successfully, not before it's > being called. > > Signed-off-by: Jan Beulich Reviewed-by: Andrew Cooper > > --- a/xen/drivers/passthrough/amd/iommu_map.c > +++ b/xen/drivers/passthrough/amd/iommu_map.c > @@ -691,8 +691,6 @@ int amd_iommu_map_page(struct domain *d, > if ( !iommu_update_pde_count(d, pt_mfn[merge_level], > gfn, mfn, merge_level) ) > break; > - /* Deallocate lower level page table */ > - free_amd_iommu_pgtable(mfn_to_page(pt_mfn[merge_level - 1])); > > if ( iommu_merge_pages(d, pt_mfn[merge_level], gfn, > flags, merge_level) ) > @@ -703,6 +701,9 @@ int amd_iommu_map_page(struct domain *d, > domain_crash(d); > return -EFAULT; > } > + > + /* Deallocate lower level page table */ > + free_amd_iommu_pgtable(mfn_to_page(pt_mfn[merge_level - 1])); > } > > out: > > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel --------------080909090701080500080406 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit
On 26/05/2014 11:16, Jan Beulich wrote:
iommu_merge_pages() still wants to look at the next level page table,
the TLB flush necessary before freeing too happens in that function,
and if it fails no free should happen at all. Hence the freeing must
be done after that function returned successfully, not before it's
being called.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>


--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -691,8 +691,6 @@ int amd_iommu_map_page(struct domain *d,
         if ( !iommu_update_pde_count(d, pt_mfn[merge_level],
                                      gfn, mfn, merge_level) )
             break;
-        /* Deallocate lower level page table */
-        free_amd_iommu_pgtable(mfn_to_page(pt_mfn[merge_level - 1]));
 
         if ( iommu_merge_pages(d, pt_mfn[merge_level], gfn, 
                                flags, merge_level) )
@@ -703,6 +701,9 @@ int amd_iommu_map_page(struct domain *d,
             domain_crash(d);
             return -EFAULT;
         }
+
+        /* Deallocate lower level page table */
+        free_amd_iommu_pgtable(mfn_to_page(pt_mfn[merge_level - 1]));
     }
 
 out:






_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

--------------080909090701080500080406-- --===============1007172589587719997== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel --===============1007172589587719997==--