Re: [Xen-devel] [Patch RFC 00/13] VT-d Asynchronous Device-TLB Flush for ATS Device

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.10.15 at 13:09, <quan.xu@xxxxxxxxx> wrote:
> On 11.10.2015 at 2:25, <tim@xxxxxxx> wrote:
>> At 17:02 +0000 on 07 Oct (1444237344), Xu, Quan wrote:
>> > Q2: how do you know when to drop them?
>> >    - log (or something) when the IOMMU entry is removed/overwritten; and
>> >    - drop the entry when the flush completes.
>> >
>> >    -- We can add a new page_list_entry structure per page_info, and Add the
>> page with the new page_list_entry structure to per domain page list, when
>> >     the IOMMU entry is removed/overwritten; and drop the entry when the
>> flush completes.
>> 
>> As Jan says, I think that might be too much overhead -- a new page_list_entry
>> would be a pretty big increase to page_info.
>> (And potentially might not be enough if a page ever needs to be on two 
> lists? Or
>> can we make sure that never happens?)
>> 
>> Storing the list of to-be-flushed MFNs separately sounds better.  The 
>> question 
> is
>> how to make sure we don't run out of memory to store that list.  Maybe have 
> a
>> pool allocated that's big enough for sensible use, and fail p2m updates with
>> -EAGAIN when we run out?
>> 
> Ignore this method. It is not a good idea.
> One question: do two lists refer to page_list and arch.relmem_list?
> It is might a challenge to make sure we don't run out of memory, if it store 
> additional relevant information.
> An aggressive method:
> --Remove this page from domain page_list / arch.relmem_list.  
> ... [like steal_page(). Swizzle the owner -- page_set_owner(page, NULL);  
> Unlink from original owner -- page_list_del(page, &d->page_list)] Then,( take 
> a 
> reference for _scheme_B__,) add this page to a per domain page list(), and 
> put this page when the flush completes.

I'm not following - are you suggesting to remove the page from the
guest temporarily? That's certainly no a valid approach.

> I am afraid that this method may cause some potential issues. i.e. make xen 
> crash in some corner cases ..

And such would preclude its use too.

>> > __scheme B__
>> > Q1: - when to take the references?
>> >
>> >     take the reference when the IOMMU entry is _ removed/overwritten_;
>> >     in detail:
>> >      --iommu_unmap_page(), or
>> >      --ept_set_entry() [Once IOMMU shares EPT page table.]
>> 
>> Hmm.  That might be practical, taking a typecount of whatever type the page
>> happens to be at the time.  I would prefer scheme A though, if it can be 
> made
>> to work.  It fits better with the usual way refcounts are used, and it's 
> more likely
>> to be a good long-term fix.  This is the second time we've had to add
>> refcounting for an edge case, and I suspect it won't be the last.
> 
> I know you prefer __scheme_A__(I think Jan prefers __scheme_A__ too.  Jan, 
> correct me, if I am wrong :) )
> which fits better with the usual way refcounts are used. But __scheme_A__ 
> would be difficult for buy-in by my team (Obviously, why spend so many effort 
> for such a small issue? why does __scheme_B__ not accept?) I think, 
> __scheme_A__ is also a tricky solution.
> 
> 
>  The IOMMU entry associated with the freed portion of GPA should not be 
> reallocated for another purpose until the flush completes.
>  I think __scheme_A__ is complex to keep a log of all relevant pending 
> derefs, and to be processed when the flush completes;
> 
> optimized __scheme_A__:
> It could keep a log of the reference only when the IOMMU entry is _ 
> removed/overwritten_.(if the IOMMU entry is not _ removed/overwritten_, it is 
> safe.).

And I'm afraid this is too vague for me to understand - what do you
want to track under what conditions?

Jan


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