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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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
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
(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.

Jan


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