[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1/2] IOMMU/MMU: Adjust top level functions for VT-d Device-TLB flush error.
On Thu, Mar 17, 2016 at 6:54 AM, Quan Xu <quan.xu@xxxxxxxxx> wrote: > diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c > index c997b53..526548e 100644 > --- a/xen/arch/x86/mm.c > +++ b/xen/arch/x86/mm.c > @@ -2467,7 +2467,7 @@ static int __get_page_type(struct page_info *page, > unsigned long type, > int preemptible) > { > unsigned long nx, x, y = page->u.inuse.type_info; > - int rc = 0; > + int rc = 0, ret = 0; > > ASSERT(!(type & ~(PGT_type_mask | PGT_pae_xen_l2))); > > @@ -2578,11 +2578,11 @@ static int __get_page_type(struct page_info *page, > unsigned long type, > if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) ) > { > if ( (x & PGT_type_mask) == PGT_writable_page ) > - iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page))); > + ret = iommu_unmap_page(d, mfn_to_gmfn(d, page_to_mfn(page))); > else if ( type == PGT_writable_page ) > - iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)), > - page_to_mfn(page), > - IOMMUF_readable|IOMMUF_writable); > + ret = iommu_map_page(d, mfn_to_gmfn(d, page_to_mfn(page)), > + page_to_mfn(page), > + IOMMUF_readable|IOMMUF_writable); > } > } > > @@ -2599,6 +2599,9 @@ static int __get_page_type(struct page_info *page, > unsigned long type, > if ( (x & PGT_partial) && !(nx & PGT_partial) ) > put_page(page); > > + if ( !rc ) > + rc = ret; > + What's this about? If the iommu_[un]map_page() operation times out, we still go through with calling alloc_page_type(); and if alloc_page_type() fails we return its failure value, but if it succeeds, we return the error from iommu_[un]map_page()? > return rc; > } > > diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c > index 3cb6868..f9bcce7 100644 > --- a/xen/arch/x86/mm/p2m-ept.c > +++ b/xen/arch/x86/mm/p2m-ept.c > @@ -830,7 +830,15 @@ out: > { > if ( iommu_flags ) > for ( i = 0; i < (1 << order); i++ ) > - 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 ( rc ) > + { > + while ( i-- > 0 ) > + iommu_unmap_page(d, gfn + i); This won't unmap gfn+0 (since it will break out when i == 0 without calling unmap). If i were signed, we could make the conditional ">= 0"; but unfortunately it's unsigned, so you'll have to do something more complicated. While we're at it, is it worth adding "unlikely()" to the if() condition here? > + break; > + } > + } > else > for ( i = 0; i < (1 << order); i++ ) > iommu_unmap_page(d, gfn + i); > diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c > index 3d80612..c33b753 100644 > --- a/xen/arch/x86/mm/p2m-pt.c > +++ b/xen/arch/x86/mm/p2m-pt.c > @@ -680,8 +680,16 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long > gfn, mfn_t mfn, > } > else if ( iommu_pte_flags ) > for ( i = 0; i < (1UL << page_order); i++ ) > - iommu_map_page(p2m->domain, gfn + i, mfn_x(mfn) + i, > - iommu_pte_flags); > + { > + rc = iommu_map_page(p2m->domain, gfn + i, mfn_x(mfn) + i, > + iommu_pte_flags); > + if ( rc ) > + { > + while ( i-- > 0 ) > + iommu_unmap_page(p2m->domain, gfn + i); Same with gfn+0. -George _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |