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

[Julien Grall <julien@xxxxxxx>]

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()) ?
> 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.
> > 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,

--
Julien Grall

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