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

[Jan Beulich <jbeulich@xxxxxxxx>]

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@xxxxxxxx>

Jan