Re: [PATCH] x86/mm: limit deferred TLB flushing to PV owned pages

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

On Wed, Sep 09, 2026 at 11:21:51AM +0200, Jan Beulich wrote:
> On 09.09.2026 09:24, Roger Pau Monne wrote:
> > The current logic on x86 will mark all domain owned pages as needed a TLB
> > flush before being re-used.  However such TLB flushing is only strictly
> > needed for PV domain owned pages, as those can keep a reference to the page
> > in the TLB after it has been freed.
> 
> What about HVM-owned ones which a PV domain has grant- or foreign-mapped?

I've looked at grant pages, and that's handled correctly, a TLB flush
is strictly done when the pages are unmapped, so there are no stale
references in the receiver TLB one the grant is released (see
gnttab_flush_tlb()).

However I cannot find any forced TLB flush for foreign mappings, I
assume this is fine because foreign mappings are not controlled by the
source domain, and hence there's no need to forcefully purge any TLB
references.  However there isn't much that can be done here: forcing a
flush on unmap in do_mmu_update() itself would be a high performance
penalty.

> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -1501,6 +1501,7 @@ bool scrub_free_pages(void)
> >  
> >  static bool mark_page_free(struct page_info *pg, mfn_t mfn)
> >  {
> > +    const struct domain *owner = page_get_owner(pg);
> >      bool pg_offlined = false;
> >  
> >      ASSERT(mfn_x(mfn) == mfn_x(page_to_mfn(pg)));
> > @@ -1539,7 +1540,7 @@ static bool mark_page_free(struct page_info *pg, 
> > mfn_t mfn)
> >      }
> >  
> >      /* If a page has no owner it will need no safety TLB flush. */
> > -    pg->u.free.need_tlbflush = (page_get_owner(pg) != NULL);
> > +    pg->u.free.need_tlbflush = owner && is_pv_domain(owner);

I guess I will need to adjust this to:

pg->u.free.need_tlbflush = owner && IS_ENABLED(CONFIG_PV);

As keeping track of whether a page has been ever mapped by a PV domain
seems overly complicated, and not worth it.

> >      if ( pg->u.free.need_tlbflush )
> >          page_set_tlbflush_timestamp(pg);
> 
> Imo whichever change it is going to be here, it definitely also requires
> the comment to be kept in sync.

Ops, yes, should adjust the comment.

Thanks, Roger.