Re: [PATCH for-4.14 v2] x86/tlb: fix assisted flush usage

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jun 23, 2020 at 04:08:06PM +0100, Julien Grall wrote:
> Hi Roger,
> 
> On 23/06/2020 15:50, Roger Pau Monne wrote:
> > diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
> > index 9b62087be1..f360166f00 100644
> > --- a/xen/include/xen/mm.h
> > +++ b/xen/include/xen/mm.h
> > @@ -639,7 +639,8 @@ static inline void accumulate_tlbflush(bool 
> > *need_tlbflush,
> >       }
> >   }
> > -static inline void filtered_flush_tlb_mask(uint32_t tlbflush_timestamp)
> > +static inline void filtered_flush_tlb_mask(uint32_t tlbflush_timestamp,
> > +                                           bool sync)
> 
> I read the commit message and went through the code, but it is still not
> clear what "sync" means in a non-architectural way.
> 
> As an Arm developper, I would assume this means we don't wait for the TLB
> flush to complete. But I am sure this would result to some unexpected
> behavior.

No, when we return from filtered_flush_tlb_mask the flush has been
performed (either with sync or without), but I understand the
confusion given the parameter name.

> So can you explain on non-x86 words what this really mean?

sync (in this context) means to force the usage of an IPI (if built
with PV or shadow paging support) in order to perform the flush.
AFAICT on Arm you always avoid an IPI when performing a flush, and
that's fine because you don't have PV or shadow, and then you don't
require this. Also I'm not sure Arm has the concept of a spurious
page fault.

I could rename to force_ipi (or require_ipi) if that's better?

Roger.