[PATCH v2] x86/mm: limit deferred TLB flushing to PV domain support

[PATCH v2] x86/mm: limit deferred TLB flushing to PV domain support

[Roger Pau Monne]

The current logic on x86 will mark all domain owned pages as needing a TLB
flush before being re-used.  However such TLB flushing is only strictly
needed when the pages might have been mapped by a PV domain, as those can
keep a reference to the page in the TLB after it has been freed.

Limit the flushing to builds with PV domain support, as tracking whether a
page might have been mapped by a PV domain is not trivial (and possibly not
worth the extra logic).

Signed-off-by: Roger Pau Monné <roger@xenproject.org>
---
Changes since v1:
 - Only avoid the flush if there's no PV domain support.
 - Fix comment.
---
 xen/common/page_alloc.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 62ac89b824de..cbb2af7f64ce 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -1538,8 +1538,11 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
         BUG();
     }
 
-    /* If a page has no owner it will need no safety TLB flush. */
-    pg->u.free.need_tlbflush = (page_get_owner(pg) != NULL);
+    /*
+     * If a page has no owner and there's no PV domain support it will need no
+     * safety TLB flush, there can be no stale TLB entries.
+     */
+    pg->u.free.need_tlbflush = IS_ENABLED(CONFIG_PV) && page_get_owner(pg);
     if ( pg->u.free.need_tlbflush )
         page_set_tlbflush_timestamp(pg);
 
-- 
2.55.0

Re: [PATCH v2] x86/mm: limit deferred TLB flushing to PV domain support

[Jan Beulich]

On 09.09.2026 16:05, Roger Pau Monne wrote:
> --- a/xen/common/page_alloc.c
> +++ b/xen/common/page_alloc.c
> @@ -1538,8 +1538,11 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
>          BUG();
>      }
>  
> -    /* If a page has no owner it will need no safety TLB flush. */
> -    pg->u.free.need_tlbflush = (page_get_owner(pg) != NULL);
> +    /*
> +     * If a page has no owner and there's no PV domain support it will need no
> +     * safety TLB flush, there can be no stale TLB entries.
> +     */
> +    pg->u.free.need_tlbflush = IS_ENABLED(CONFIG_PV) && page_get_owner(pg);
>      if ( pg->u.free.need_tlbflush )
>          page_set_tlbflush_timestamp(pg);

I'm okay with the code change now, but the comment is still concerning me.
All by itself there is no reason why stale TLB entries couldn't also exist
for HVM guests. It's just that (a) only the host TLBs are flushed by
filtered_flush_tlb_mask() and (b) flushes of guest TLBs occur when pages
are removed from their P2Ms (aiui; hopefully true also for Arm). IOW what
the comment says looks to be correct, just that it leaves too much to be
figured out by the reader. At the very least I'd suggest "..., there can
be no stale (host) TLB entries." Thoughts?

Jan

Re: [PATCH v2] x86/mm: limit deferred TLB flushing to PV domain support

[Roger Pau Monné]

On Wed, Sep 09, 2026 at 04:46:01PM +0200, Jan Beulich wrote:
> On 09.09.2026 16:05, Roger Pau Monne wrote:
> > --- a/xen/common/page_alloc.c
> > +++ b/xen/common/page_alloc.c
> > @@ -1538,8 +1538,11 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
> >          BUG();
> >      }
> >  
> > -    /* If a page has no owner it will need no safety TLB flush. */
> > -    pg->u.free.need_tlbflush = (page_get_owner(pg) != NULL);
> > +    /*
> > +     * If a page has no owner and there's no PV domain support it will need no
> > +     * safety TLB flush, there can be no stale TLB entries.
> > +     */
> > +    pg->u.free.need_tlbflush = IS_ENABLED(CONFIG_PV) && page_get_owner(pg);
> >      if ( pg->u.free.need_tlbflush )
> >          page_set_tlbflush_timestamp(pg);
> 
> I'm okay with the code change now, but the comment is still concerning me.
> All by itself there is no reason why stale TLB entries couldn't also exist
> for HVM guests. It's just that (a) only the host TLBs are flushed by
> filtered_flush_tlb_mask() and (b) flushes of guest TLBs occur when pages
> are removed from their P2Ms (aiui; hopefully true also for Arm). IOW what
> the comment says looks to be correct, just that it leaves too much to be
> figured out by the reader. At the very least I'd suggest "..., there can
> be no stale (host) TLB entries." Thoughts?

Hm, I find adding "(host)" to also be slightly confusing, as I would
usually associate host TLB with Xen context TLB state.  Which is also
made more confusing by how PV guests share the page-tables with Xen.

"If a page has no owner and there's no PV domain support it will need
no safety TLB flush.  PV domains are the only domain types that can
keep stale entries on the TLB, as they have (limited) control over the
host MMU and when flushes are performed"

Is this any better?  I'm still not fully convinced, as HVM guests do
have full control over the MMU, it's just that in that case p2m
changes unconditionally lead to flushes.

Thanks, Roger.

Re: [PATCH v2] x86/mm: limit deferred TLB flushing to PV domain support

[Jason Andryuk]

On 2026-09-09 12:28, Roger Pau Monné wrote:
> On Wed, Sep 09, 2026 at 04:46:01PM +0200, Jan Beulich wrote:
>> On 09.09.2026 16:05, Roger Pau Monne wrote:
>>> --- a/xen/common/page_alloc.c
>>> +++ b/xen/common/page_alloc.c
>>> @@ -1538,8 +1538,11 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
>>>           BUG();
>>>       }
>>>   
>>> -    /* If a page has no owner it will need no safety TLB flush. */
>>> -    pg->u.free.need_tlbflush = (page_get_owner(pg) != NULL);
>>> +    /*
>>> +     * If a page has no owner and there's no PV domain support it will need no
>>> +     * safety TLB flush, there can be no stale TLB entries.
>>> +     */
>>> +    pg->u.free.need_tlbflush = IS_ENABLED(CONFIG_PV) && page_get_owner(pg);
>>>       if ( pg->u.free.need_tlbflush )
>>>           page_set_tlbflush_timestamp(pg);
>>
>> I'm okay with the code change now, but the comment is still concerning me.
>> All by itself there is no reason why stale TLB entries couldn't also exist
>> for HVM guests. It's just that (a) only the host TLBs are flushed by
>> filtered_flush_tlb_mask() and (b) flushes of guest TLBs occur when pages
>> are removed from their P2Ms (aiui; hopefully true also for Arm). IOW what
>> the comment says looks to be correct, just that it leaves too much to be
>> figured out by the reader. At the very least I'd suggest "..., there can
>> be no stale (host) TLB entries." Thoughts?
> 
> Hm, I find adding "(host)" to also be slightly confusing, as I would
> usually associate host TLB with Xen context TLB state.  Which is also
> made more confusing by how PV guests share the page-tables with Xen.
> 
> "If a page has no owner and there's no PV domain support it will need
> no safety TLB flush.  PV domains are the only domain types that can
> keep stale entries on the TLB, as they have (limited) control over the
> host MMU and when flushes are performed"
I find "will need no" a little awkward.  Maybe:

"If a page has no owner and there's no PV domain support it does not 
need a safety TLB flush."

or:

"If a page has no owner and there's no PV domain support, then a safety 
TLB flush is not needed."

Regards,
Jason

Re: [PATCH v2] x86/mm: limit deferred TLB flushing to PV domain support

[Jan Beulich]

On 09.09.2026 22:07, Jason Andryuk wrote:
> On 2026-09-09 12:28, Roger Pau Monné wrote:
>> On Wed, Sep 09, 2026 at 04:46:01PM +0200, Jan Beulich wrote:
>>> On 09.09.2026 16:05, Roger Pau Monne wrote:
>>>> --- a/xen/common/page_alloc.c
>>>> +++ b/xen/common/page_alloc.c
>>>> @@ -1538,8 +1538,11 @@ static bool mark_page_free(struct page_info *pg, mfn_t mfn)
>>>>           BUG();
>>>>       }
>>>>   
>>>> -    /* If a page has no owner it will need no safety TLB flush. */
>>>> -    pg->u.free.need_tlbflush = (page_get_owner(pg) != NULL);
>>>> +    /*
>>>> +     * If a page has no owner and there's no PV domain support it will need no
>>>> +     * safety TLB flush, there can be no stale TLB entries.
>>>> +     */
>>>> +    pg->u.free.need_tlbflush = IS_ENABLED(CONFIG_PV) && page_get_owner(pg);
>>>>       if ( pg->u.free.need_tlbflush )
>>>>           page_set_tlbflush_timestamp(pg);
>>>
>>> I'm okay with the code change now, but the comment is still concerning me.
>>> All by itself there is no reason why stale TLB entries couldn't also exist
>>> for HVM guests. It's just that (a) only the host TLBs are flushed by
>>> filtered_flush_tlb_mask() and (b) flushes of guest TLBs occur when pages
>>> are removed from their P2Ms (aiui; hopefully true also for Arm). IOW what
>>> the comment says looks to be correct, just that it leaves too much to be
>>> figured out by the reader. At the very least I'd suggest "..., there can
>>> be no stale (host) TLB entries." Thoughts?
>>
>> Hm, I find adding "(host)" to also be slightly confusing, as I would
>> usually associate host TLB with Xen context TLB state.  Which is also
>> made more confusing by how PV guests share the page-tables with Xen.
>>
>> "If a page has no owner and there's no PV domain support it will need
>> no safety TLB flush.  PV domains are the only domain types that can
>> keep stale entries on the TLB, as they have (limited) control over the
>> host MMU and when flushes are performed"
> I find "will need no" a little awkward.  Maybe:
> 
> "If a page has no owner and there's no PV domain support it does not 
> need a safety TLB flush."
> 
> or:
> 
> "If a page has no owner and there's no PV domain support, then a safety 
> TLB flush is not needed."

I'd be okay with any of these. Then:
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan