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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 30.09.15 at 02:02, <yunhong.jiang@xxxxxxxxx> wrote:

> 
>> -----Original Message-----
>> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
>> bounces@xxxxxxxxxxxxx] On Behalf Of Jan Beulich
>> Sent: Tuesday, September 29, 2015 12:59 AM
>> To: xen-devel
>> Cc: George Dunlap; Andrew Cooper; Tian, Kevin; Keir Fraser; Nakajima, Jun
>> Subject: Re: [Xen-devel] [PATCH 1/3] x86/p2m: tighten conditions of IOMMU
>> mapping updates
>> 
>> >>> 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>
>> > ---
>> > 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?
>> 
>> Okay, I think I finally figured it myself (with the help of the partial
>> patch presented yesterday): There is no need for more flushing
>> in the current model, where we only ever split large pages or fully
>> replace sub-hierarchies with large pages (accompanied by a suitable
>> flush already). More flushing would only be needed if we were to
>> add code to re-establish large pages if an intermediate page table
>> re-gained a state where this would be possible (quite desirable
>> in a situation where log-dirty mode got temporarily enabled on a
>> domain, but I'm not sure how common an operation that would be).
>> When splitting large pages, all the hardware can have cached are
>> - leaf entries the size of the page being split
> 
> I'm not sure if there is an offset-1 issue. 

offset-1 ?

> Basically I think we depends on the "Invalidation Hint" being cleared so 
> that all the paging structure cache are cleared. That should work if there is 
> no split happen.
> However, in the split situation, I suspect that the entry that was split is 
> not covered by the ADDR/AM combination since the AM, based on  
> iommu_flush_iotlb_psi(), is in fact the "order" parameter for 
> ept_set_entry(). I'm not sure if I missed anything. 

IH is always clear in Xen right now. And the address covers both -
leaf entries as well as intermediate ones. Yet as explained before
we don't even care about intermediate ones in the split case.

Jan


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