Re: [Xen-devel] [PATCH 1/3] x86/p2m: tighten conditions of IOMMU mapping updates

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Sep 29, 2015 at 04:47:00AM -0600, Jan Beulich wrote:
> >>> On 23.09.15 at 16:00, <wei.liu2@xxxxxxxxxx> wrote:
> > On Tue, Sep 22, 2015 at 08:15:39AM -0600, Jan Beulich wrote:
> >> >>> On 21.09.15 at 16:02, <JBeulich@xxxxxxxx> wrote:
> >> > In the EPT case permission changes should also result in updates or
> >> > TLB flushes.
> >> > 
> >> > In the NPT case the old MFN does not depend on the new entry being
> >> > valid (but solely on the old one), and the need to update or TLB-flush
> >> > again also depends on permission changes.
> >> > 
> >> > In the shadow mode case, iommu_hap_pt_share should be ignored.
> >> > 
> >> > Furthermore in the NPT/shadow case old intermediate page tables must
> >> > be freed only after IOMMU side updates/flushes have got carried out.
> >> > 
> >> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> >> 
> >> Wei,
> >> 
> >> I'm sorry, I forgot to Cc you on the patch and state that I think this
> >> should be considered for 4.6.
> > 
> > While I think having this issue fixed for 4.6 would be nice,  I would
> > like to ...
> > 
> >> > ---
> >> > In addition to the fixes here it looks to me as if both EPT and
> >> > NPT/shadow code lack invalidation of IOMMU side paging structure
> >> > caches, i.e. further changes may be needed. Am I overlooking something?
> > 
> > have this question answered. Having a "fix" that doesn't actually fix
> > the issue is not very useful...
> 
> So I suppose you saw my reasoning of there not actually being any
> further flushing necessary right now. Following the discussion with

Yes, I saw it. I'm convinced.

> George I split off and applied the EPT side of this, which I'd like to
> see go into 4.6 too. The NPT/shadow side will need a little more work

EPT side:

Release-acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>

> (including further splitting the original patch), partly depending on
> a decision on whether to keep the dead page table sharing code in
> p2m-pt.c.
> 
> I did already split out the intermediate page table freeing patch.
> The only other bug is the incomplete checking of iommu_hap_pt_share
> in shadow mode, everything else would be cleanup only no matter
> what route we go. The only thing is that this broken check would
> go away altogether if we decided to drop the pt-share logic, but I
> suppose for backporting purposes it would be worthwhile to have
> that fix separated regardless of further direction.
> 

I will reply separately to your other two patches.

Wei.

> Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel