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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Oct 09, 2018 at 12:38:41PM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Xen-devel [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx] On Behalf
> > Of Paul Durrant
> > Sent: 09 October 2018 12:23
> > To: Wei Liu <wei.liu2@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>; Jan
> > Beulich <JBeulich@xxxxxxxx>; Roger Pau Monne <roger.pau@xxxxxxxxxx>
> > Subject: Re: [Xen-devel] [PATCH] x86/vtd: fix IOMMU share PT destruction
> > path
> > 
> > > -----Original Message-----
> > > From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> > > Sent: 09 October 2018 11:42
> > > To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> > > Cc: Jan Beulich <JBeulich@xxxxxxxx>; Roger Pau Monne
> > > <roger.pau@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Wei Liu
> > > <wei.liu2@xxxxxxxxxx>; Kevin Tian <kevin.tian@xxxxxxxxx>
> > > 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.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel