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

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

On Fri, Jun 26, 2020 at 10:38:11AM +0100, Julien Grall wrote:
> Hi Roger,
> 
> Sorry I didn't manage to answer to your question before you sent v3.
> 
> On 25/06/2020 12:30, Roger Pau Monne wrote:
> > diff --git a/xen/include/asm-arm/flushtlb.h b/xen/include/asm-arm/flushtlb.h
> > index ab1aae5c90..7ae0543885 100644
> > --- a/xen/include/asm-arm/flushtlb.h
> > +++ b/xen/include/asm-arm/flushtlb.h
> > @@ -27,6 +27,7 @@ static inline void page_set_tlbflush_timestamp(struct 
> > page_info *page)
> >   /* Flush specified CPUs' TLBs */
> >   void flush_tlb_mask(const cpumask_t *mask);
> > +#define flush_tlb_mask_sync flush_tlb_mask
> 
> Dropping the parameter 'sync' from filtered_flush_tlb_mask() is a nice
> improvement, but it unfortunately doesn't fully address my concern.
> 
> After this patch there is exactly one use of flush_tlb_mask() in common code
> (see grant_table.c). But without looking at the x86 code, it is not clear
> why this requires a different flush compare to the two other sites.

It's not dealing with page allocation or page type changes directly,
and hence doesn't need to use an IPI in order to prevent races with
spurious_page_fault.

> IOW, if I want to modify the common code in the future, how do I know which
> flush to call?

Unless you modify one of the specific areas mentioned above (page
allocation or page type changes) you should use flush_tlb_mask.

This is not ideal, and my aim will be to be able to use the assisted
flush everywhere if possible, so I would really like to get rid of the
interrupt disabling done in spurious_page_fault and this model where
x86 relies on blocking interrupts in order to prevent page type
changes or page freeing.

Such change however doesn't feel appropriate for a release freeze
period, and hence went with something smaller that restores the
previous behavior. Another option is to just disable assisted flushes
for the time being and re-enable them when a suitable solution is
found.

Roger.