On Wed, May 11, 2016 at 3:26 AM, Xu, Quan <quan...@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
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;
}
}
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
