Re: [Xen-devel] [PATCH v2] x86/mm: also flush TLB when putting writable foreign page reference

[Tim Deegan <tim@xxxxxxx>]

At 05:59 -0600 on 25 Apr (1493099950), Jan Beulich wrote:
> >>> On 25.04.17 at 12:59, <tim@xxxxxxx> wrote:
> > Hi,
> > 
> > At 02:59 -0600 on 25 Apr (1493089158), Jan Beulich wrote:
> >> Jann's explanation of the problem:
> >> 
> >> "start situation:
> >>  - domain A and domain B are PV domains
> >>  - domain A and B both have currently scheduled vCPUs, and the vCPUs
> >>    are not scheduled away
> >>  - domain A has XSM_TARGET access to domain B
> >>  - page X is owned by domain B and has no mappings
> >>  - page X is zeroed
> >> 
> >>  steps:
> >>  - domain A uses do_mmu_update() to map page X in domain A as writable
> >>  - domain A accesses page X through the new PTE, creating a TLB entry
> >>  - domain A removes its mapping of page X
> >>    - type count of page X goes to 0
> >>    - tlbflush_timestamp of page X is bumped
> >>  - domain B maps page X as L1 pagetable
> >>    - type of page X changes to PGT_l1_page_table
> >>    - TLB flush is forced using domain_dirty_cpumask of domain B
> >>    - page X is mapped as L1 pagetable in domain B
> >> 
> >>  At this point, domain B's vCPUs are guaranteed to have no
> >>  incorrectly-typed stale TLB entries for page X, but AFAICS domain A's
> >>  vCPUs can still have stale TLB entries that map page X as writable,
> >>  permitting domain A to control a live pagetable of domain B."
> > 
> > AIUI this patch solves the problem by immediately flushing domain A's
> > TLB entries at the point where domain A removes its mapping of page X.
> > 
> > Could we, instead, bitwise OR domain A's domain_dirty_cpumask into
> > domain B's domain_dirty_cpumask at the same point?
> > 
> > Then when domain B flushes TLBs in the last step (in __get_page_type())
> > it will catch any stale TLB entries from domain A as well.  But in the
> > (hopefully common) case where there's a delay between domain A's
> > __put_page_type() and domain B's __get_page_type(), the usual TLB
> > timestamp filtering will suppress some of the IPIs/flushes.
> 
> Oh, I see. Yes, I think this would be fine. However, we don't have
> a suitable cpumask accessor allowing us to do this ORing atomically,
> so we'd have to open code it.

Probably better to build the accessor than to open code here. :)

> Do you think such a slightly ugly approach would be worth it here?
> Foreign mappings shouldn't be _that_ performance critical..

I have no real idea, though there are quite a lot of them in domain
building/migration.  I can imagine a busy multi-vcpu dom0 could
generate a lot of IPIs, almost all of which could be merged.

> And then, considering that this will result in time stamp based filtering
> again, I'm no longer sure I was right to agree with Jann on the flush
> here needing to be unconditional. Regardless of page table owner
> matching page owner, the time stamp stored for the page will always
> be applicable (it's a global property). So we wouldn't even need to
> OR in the whole dirty mask here, but could already pre-filter (or if we
> stayed with the flush-on-put approach, then v1 would have been
> correct).

I don't think so.  The page's timestamp is set when its typecount
falls to zero, which hasn't happened yet -- we hold a typecount
ourselves here.

In theory we could filter the bits we're adding against a local
timestamp, but that would have to be tlbflush_current_time()
because the TLB entries we care about are live right now, and
filtering against that is (probably) a noop.

Tim.

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