x86/mm: limit deferred TLB flushing to PV owned pages

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

Posted by Roger Pau Monne 2 weeks, 1 day ago

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.

Limit the requirement of a flush to pages that are owned by PV domains.

Signed-off-by: Roger Pau Monné <roger@xenproject.org>
---
 xen/common/page_alloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 62ac89b824de..be202f0b8d1e 100644
--- 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);
     if ( pg->u.free.need_tlbflush )
         page_set_tlbflush_timestamp(pg);
 
-- 
2.55.0

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

Posted by Jan Beulich 2 weeks, 1 day ago

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?

> --- 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);
>      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.

Jan

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

Posted by Roger Pau Monné 2 weeks, 1 day ago

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.

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

Posted by Jan Beulich 2 weeks, 1 day ago

On 09.09.2026 15:09, Roger Pau Monné wrote:
> 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.

Right, and hence ...

>>> --- 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.

... "yes" here as well.

Jan

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

Posted by Andrew Cooper 2 weeks, 1 day ago

On 09/09/2026 10:21 am, 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?

If a patch is grant or foreign mapped, then it can't be in the process
of being freed.

I presume you mean "had been grant/foreign mapped previously", and with
that, I agree with your concern.

~Andrew