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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.03.2020 10:01, Roger Pau Monné wrote:
> On Fri, Mar 20, 2020 at 08:21:19AM +0100, Jan Beulich wrote:
>> On 19.03.2020 20:07, Julien Grall wrote:
>>> 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?
> 
> No, I think from Julien's proposal (if I understood it correctly)
> flush_tlb_all_guests_mask would do what flush_tlb_mask currently does
> previous to this patch (flush Xen's TLBs + HVM ASIDs).
> 
>>>> 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).
> 
> I don't have a strong opinion re naming, are you OK with the names
> proposed above?

Well, no - imo a function named e.g. flush_tlb_all_guests_cpumask() is
not supposed to flush any host TLBs. But I'll also reply to Julien's
subsequent reply in a minute.

Jan

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