Re: [PATCH] x86/vtd: fix IOMMU share PT destruction path

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 09 October 2018 14:38
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>; xen-devel@lists.xenproject.org; Kevin
> Tian <kevin.tian@intel.com>; Jan Beulich <JBeulich@suse.com>; Roger Pau
> Monne <roger.pau@citrix.com>
> 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:xen-devel-bounces@lists.xenproject.org] On
> Behalf
> > > > Of Paul Durrant
> > > > Sent: 09 October 2018 12:23
> > > > To: Wei Liu <wei.liu2@citrix.com>; xen-devel@lists.xenproject.org
> > > > Cc: Kevin Tian <kevin.tian@intel.com>; Wei Liu
> <wei.liu2@citrix.com>; Jan
> > > > Beulich <JBeulich@suse.com>; Roger Pau Monne <roger.pau@citrix.com>
> > > > Subject: Re: [Xen-devel] [PATCH] x86/vtd: fix IOMMU share PT
> destruction
> > > > path
> > > >
> > > > > -----Original Message-----
> > > > > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > > > > Sent: 09 October 2018 11:42
> > > > > To: xen-devel@lists.xenproject.org
> > > > > Cc: Jan Beulich <JBeulich@suse.com>; Roger Pau Monne
> > > > > <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>;
> Wei Liu
> > > > > <wei.liu2@citrix.com>; Kevin Tian <kevin.tian@intel.com>
> > > > > 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
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel