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

["Xu, Quan" <quan.xu@xxxxxxxxx>]

>> >>>On 08.10.2015 at 16:52 <JBeulich@xxxxxxxx> wrote:
> >>> On 07.10.15 at 19:02, <quan.xu@xxxxxxxxx> wrote:
> > __scheme A__
> > Q1: - when to take the references?
> >     take the reference when the IOMMU entry is _created_;
> >     in detail:
> >      --iommu_map_page(), or
> >      --ept_set_entry() [Once IOMMU shares EPT page table.]
> >
> > That leaves one question:
> >     -- how to do with hot-plug ATS device pass-through? As the EPT
> > doesn't aware IOMMU will share EPT page table when EPT page table was
> _created_.
> 
> How is it not? iommu_hap_pt_share is a global option, and hence even for a
> domain which does not (yet) require an IOMMU these references could be
> acquired proactively.
> 

I mean that the 'need_iommu(d)' is '0'. If I extend to take an additional 
reference for normal HVM guest, I think it is tricky and very challenge to 
manage this additional reference.
that's why I prefer __scheme B__..



> > 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.
> 
> Please be very careful when considering to grow the size of struct page_info -
> due to the amount of its instances this would have a quite measurable effect 
> on
> the memory overhead of memory management. Since the number of pages
> currently having a flush pending shouldn't be that high at any point in time, 
> I
> don't think such increased overhead would be justifiable.
> 

Make sense. Agreed.
I can remove this page from domain  [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 and add this page to a per domain page list, and put 
this page when the flush completes.
I was afraid that this solution may cause potential issues. i.e. make xen crash 
in some corner case ..


> > Q3: what to do about mappings of other domains' memory (i.e. grant and
> > foreign mappings).
> >    Between two domains, now I have only one idea to fix this tricky
> > issue -- waitqueue.
> >    I.e. grant.
> >     For gnttab_transfer /gnttab_unmap , wait on a waitqueue before
> > updating grant flag, until the Device-TLB flush is completed.
> >     For grant-mapped, it is safe as the modification of gnttab_unmap.
> 
> Hmm, wouldn't grant transfers already be taken care of by the extra 
> references?
> See steal_page(). Perhaps the ordering between its invocation and
> guest_physmap_remove_page() would need to be switched (with the latter
> getting undone if steal_page() fails). The waiting for the flush to complete 
> could -
> afaics - be done by using the grant-ops' inherent batching (and hence easy
> availability of continuations). But I admit there's some hand waiving here 
> without
> closer inspection...
> 

I think the extra references can NOT fix the security issue between two domains.
i.e. If domA transfers the ownership of a page to domB, but the domA still take 
extra references of the page. I think it is not correct.

IMO the guide to fix the security issue between two domains -- a two-phase 
operation.
 --First--make sure one domain can NOT read/write/DMA with this page.
 --Second-- then the other domain do everything for this page.

Waitqueue is useful for this case.
I know this is still an ugly solution. - afaics - make sure the hypervisor side 
implementation is correct.

> > __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.]
> >
> >     * Make sure IOMMU page should not be reallocated for
> >      another purpose until the appropriate invalidations have been
> >      performed.
> >     * in this case, it does not matter hot-plug ATS device
> > pass-through or ATS device assigned in domain initialization.
> >
> > Q2 / Q3:
> >     The same as above __scheme A__ Q2/Q3.
> >
> > One question: is __scheme B__ safe? If it is safe, I prefer __scheme B__..
> 
> While at the first glance this looks like a neat idea - 


I think this is safe and a good solution.
I hope you can review into the __scheme B__. I need _Acked-by_ you and Tim 
Deegan.



> what do you do if obtaining
> the reference fails? Not touching the EPT entry may be possible (but not 
> nice),
> but not doing the IOMMU side update when the EPT one already succeeded
> would seem very cumbersome (as you'd need to roll back the EPT side update).
> Maybe a two-phase operation (obtain references for IOMMU, update EPT,
> update IOMMU) could solve this (and could then actually be independent of
> whether page tables are shared).


Good. if the __scheme B__ is correct, I will take it into consideration.

Jan, thanks very much!


-Quan


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