diff mbox

[v4,03/10] IOMMU/MMU: enhance the call trees of IOMMU unmapping and mapping

Message ID CAFLBxZZp+ikHPq9e-YMCG2Hj+-bUtzApNYZ+cwDM8s9Edc7Raw@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

George Dunlap May 11, 2016, 8:45 a.m. UTC 
On Wed, May 11, 2016 at 3:26 AM, Xu, Quan <quan.xu@intel.com> wrote:
> Agreed. Thanks for your careful checking.
>
> Check it again --
> 1. then I am no need to check 'rc' as below:
>
>       if ( rc == 0 && p2m_is_hostp2m(p2m) && need_iommu(d) &&
>            need_modify_vtd_table )
>      {
> +                        if ( !rc )
> +                            rc = ret;
> ...
>
> +                    if ( !rc && unlikely(ret) )
> +                        rc = ret;
>      }

Actually, in the case of iommu_map_page(), you can just use rc
directly and not bother using ret at all.  (In the case of
iommu_unmap_page(), you still need to use ret and check that rc != 0
to make sure you get the first error.)

Something like applying the attached patch (not built or nitpicked for
style, just for clarity).

 -George

Comments

Quan Xu May 11, 2016, 8:58 a.m. UTC | #1 
On May 11, 2016 4:46 PM, George Dunlap <George.Dunlap@eu.citrix.com> wrote:
> On Wed, May 11, 2016 at 3:26 AM, Xu, Quan <quan.xu@intel.com> wrote:

> > Agreed. Thanks for your careful checking.

> >

> > Check it again --

> > 1. then I am no need to check 'rc' as below:

> >

> >       if ( rc == 0 && p2m_is_hostp2m(p2m) && need_iommu(d) &&

> >            need_modify_vtd_table )

> >      {

> > +                        if ( !rc )

> > +                            rc = ret;

> > ...

> >

> > +                    if ( !rc && unlikely(ret) )

> > +                        rc = ret;

> >      }

> 

> Actually, in the case of iommu_map_page(), you can just use rc directly and

> not bother using ret at all.  (In the case of iommu_unmap_page(), you still

> need to use ret and check that rc != 0 to make sure you get the first error.)

> 

> Something like applying the attached patch (not built or nitpicked for style,

> just for clarity).

> 


Got it, thanks.

Quan
diff mbox

Patch

diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index 814cb72..f8360c6 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -828,7 +828,7 @@  out:
         ept_sync_domain(p2m);
 
     /* For host p2m, may need to change VT-d page table.*/
-    if ( entry_written && p2m_is_hostp2m(p2m) && need_iommu(d) &&
+    if ( rc == 0  && p2m_is_hostp2m(p2m) && need_iommu(d) &&
          need_modify_vtd_table )
     {
         if ( iommu_hap_pt_share )
@@ -838,16 +838,13 @@  out:
             if ( iommu_flags )
                 for ( i = 0; i < (1 << order); i++ )
                 {
-                    ret = iommu_map_page(d, gfn + i, mfn_x(mfn) + i, iommu_flags);
+                    rc = iommu_map_page(d, gfn + i, mfn_x(mfn) + i, iommu_flags);
 
-                    if ( unlikely(ret) )
+                    if ( unlikely(rc) )
                     {
                         while ( i-- )
                             iommu_unmap_page(p2m->domain, gfn + i);
 
-                        if ( !rc )
-                            rc = ret;
-
                         break;
                     }
                 }