Re: [Xen-devel] [PATCH v7 1/3] x86/tlb: introduce a flush HVM ASIDs flag

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.03.2020 20:07, Julien Grall wrote:
> Hi,
> 
> On 19/03/2020 18:43, Roger Pau Monné wrote:
>> On Thu, Mar 19, 2020 at 06:07:44PM +0000, Julien Grall wrote:
>>>
>>>
>>> On 19/03/2020 17:38, Roger Pau Monné wrote:
>>>> On Thu, Mar 19, 2020 at 04:21:23PM +0000, Julien Grall wrote:
>>>>   >> Why can't you keep flush_tlb_mask() here?
>>>>
>>>> Because filtered_flush_tlb_mask is used in populate_physmap, and
>>>> changes to the phymap require an ASID flush on AMD hardware.
>>>
>>> I am afraid this does not yet explain me why flush_tlb_mask() could not be
>>> updated so it flush the ASID on AMD hardware.
>>
>> Current behavior previous to this patch is to flush the ASIDs on
>> every TLB flush.
>>
>> flush_tlb_mask is too widely used on x86 in places where there's no
>> need to flush the ASIDs. This prevents using assisted flushes (by L0)
>> when running nested, since those assisted flushes performed by L0
>> don't flush the L2 guests TLBs.
>>
>> I could keep current behavior and leave flush_tlb_mask also flushing the
>> ASIDs, but that seems wrong as the function doesn't have anything in
>> it's name that suggests it also flushes the in-guest TLBs for HVM.
> 
> I agree the name is confusing, I had to look at the implementation to 
> understand what it does.
> 
> How about renaming (or introducing) the function to 
> flush_tlb_all_guests_mask() or flush_tlb_all_guests_cpumask()) ?

And this would then flush _only_ guest TLBs?

>> I would rather prefer the default to be to not flush the
>> ASIDs, so that users need to specify so by passing the flag to
>> flusk_mask.
> That's x86 choice. For common, I would rather no introduce those flags until 
> we have another arch that make use of it.

The flags should perhaps indeed remain x86-specific, but suitable
wrappers usable from common code should exist (as you suggest
below).

Jan

>>> This would actually match the behavior of flush_tlb_mask() on Arm where all
>>> the guest TLBs would be removed.
>>
>> That's how it used to be previous to this patch, and the whole point
>> is to split the ASID flushes into a separate flag, so it's not done
>> for every TLB flush.
> 
> Well, tlb_flush_mask() is only implemented for the benefit of common code. It 
> has no other users on Arm.
> 
> It feels to me that we want an helper that will nuke all the guest TLBs on a 
> given set of CPUs (see above for some name suggestion).
> 
> On x86, you could implement it using flush_mask(). On Arm, this could be a 
> rename of the existing function flush_tlb_mask().
> 
> Cheers,
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel