> -----Original Message----- > From: Wei Liu [mailto:[email protected]] > Sent: 09 October 2018 14:38 > To: Paul Durrant <[email protected]> > Cc: Wei Liu <[email protected]>; [email protected]; Kevin > Tian <[email protected]>; Jan Beulich <[email protected]>; Roger Pau > Monne <[email protected]> > Subject: Re: [PATCH] x86/vtd: fix IOMMU share PT destruction path > > On Tue, Oct 09, 2018 at 01:58:14PM +0100, Wei Liu wrote: > > On Tue, Oct 09, 2018 at 12:38:41PM +0100, Paul Durrant wrote: > > > > -----Original Message----- > > > > From: Xen-devel [mailto:[email protected]] On > Behalf > > > > Of Paul Durrant > > > > Sent: 09 October 2018 12:23 > > > > To: Wei Liu <[email protected]>; [email protected] > > > > Cc: Kevin Tian <[email protected]>; Wei Liu > <[email protected]>; Jan > > > > Beulich <[email protected]>; Roger Pau Monne <[email protected]> > > > > Subject: Re: [Xen-devel] [PATCH] x86/vtd: fix IOMMU share PT > destruction > > > > path > > > > > > > > > -----Original Message----- > > > > > From: Wei Liu [mailto:[email protected]] > > > > > Sent: 09 October 2018 11:42 > > > > > To: [email protected] > > > > > Cc: Jan Beulich <[email protected]>; Roger Pau Monne > > > > > <[email protected]>; Paul Durrant <[email protected]>; > Wei Liu > > > > > <[email protected]>; Kevin Tian <[email protected]> > > > > > Subject: [PATCH] x86/vtd: fix IOMMU share PT destruction path > > > > > > > > > > Commit 2916951c1 ("mm / iommu: include need_iommu() test in > > > > > iommu_use_hap_pt()") included need_iommu() in iommu_use_hap_pt and > > > > > 91d4eca7add (" mm / iommu: split need_iommu() into has_iommu_pt() > and > > > > > need_iommu_pt_sync()") made things finer grain by spliting > need_iommu > > > > > into three states. > > > > > > > > > > The destruction path can't use iommu_use_hap_pt because at the > point > > > > > platform op is called, IOMMU is already switched to disabled state > and > > > > > the shared PT test would always be false. > > > > > > > > > > > > > This is VT-d specific and the issue could affect AMD too. > > > > > > Yes, it looks like deallocate_iommu_page_tables() will then try to > deallocate page tables that would not have been allocated. This probably > happens to be safe because the root table is allocated on demand, but that > might change in future so I think it better to fix as I suggest below. > > > > > > Paul > > > > > > > Can't you just > > > > switch round the call to teardown and setting IOMMU_status_disabled > in > > > > xen/drivers/passthrough/iommu.c:iommu_teardown()? > > > > OK. I will swap those lines instead. > > Unfortunately swapping those two lines won't work, because for a DomU > that doesn't have passthrough device, IOMMU is always disabled during > its life cycle. Yet mm/p2m.c invokes iommu_share_p2m_table based solely > on if hap is enabled.
OK. This disparity between dom0 and domU set-up is annoying. > > Do you have other suggestions? It seems that we'd better make the > predicates for enabling and disabling match. > Since your patch to the VT-d code works, let's go with it then. Paul > Wei. _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
