Re: [PATCH v9 1/3] x86/tlb: introduce a flush HVM ASIDs flag

[Wei Liu <wl@xxxxxxx>]

On Thu, Apr 09, 2020 at 01:54:40PM +0200, Jan Beulich wrote:
> On 06.04.2020 12:57, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/mm/hap/hap.c
> > +++ b/xen/arch/x86/mm/hap/hap.c
> > @@ -118,7 +118,7 @@ int hap_track_dirty_vram(struct domain *d,
> >              p2m_change_type_range(d, begin_pfn, begin_pfn + nr,
> >                                    p2m_ram_rw, p2m_ram_logdirty);
> >  
> > -            flush_tlb_mask(d->dirty_cpumask);
> > +            hap_flush_tlb_mask(d->dirty_cpumask);
> >  
> >              memset(dirty_bitmap, 0xff, size); /* consider all pages dirty 
> > */
> >          }
> > @@ -205,7 +205,7 @@ static int hap_enable_log_dirty(struct domain *d, 
> > bool_t log_global)
> >           * to be read-only, or via hardware-assisted log-dirty.
> >           */
> >          p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_logdirty);
> > -        flush_tlb_mask(d->dirty_cpumask);
> > +        hap_flush_tlb_mask(d->dirty_cpumask);
> >      }
> >      return 0;
> >  }
> > @@ -234,7 +234,7 @@ static void hap_clean_dirty_bitmap(struct domain *d)
> >       * be read-only, or via hardware-assisted log-dirty.
> >       */
> >      p2m_change_entry_type_global(d, p2m_ram_rw, p2m_ram_logdirty);
> > -    flush_tlb_mask(d->dirty_cpumask);
> > +    hap_flush_tlb_mask(d->dirty_cpumask);
> >  }
> >  
> >  /************************************************/
> > @@ -798,7 +798,7 @@ hap_write_p2m_entry(struct p2m_domain *p2m, unsigned 
> > long gfn, l1_pgentry_t *p,
> >  
> >      safe_write_pte(p, new);
> >      if ( old_flags & _PAGE_PRESENT )
> > -        flush_tlb_mask(d->dirty_cpumask);
> > +        hap_flush_tlb_mask(d->dirty_cpumask);
> >  
> >      paging_unlock(d);
> >  
> 
> Following up on my earlier mail about paging_log_dirty_range(), I'm
> now of the opinion that all of these flushes should go away too. I
> can only assume that they got put where they are when HAP code was
> cloned from the shadow one. These are only p2m operations, and hence
> p2m level TLB flushing is all that's needed here.
> 
> > --- a/xen/arch/x86/mm/hap/nested_hap.c
> > +++ b/xen/arch/x86/mm/hap/nested_hap.c
> > @@ -84,7 +84,7 @@ nestedp2m_write_p2m_entry(struct p2m_domain *p2m, 
> > unsigned long gfn,
> >      safe_write_pte(p, new);
> >  
> >      if (old_flags & _PAGE_PRESENT)
> > -        flush_tlb_mask(p2m->dirty_cpumask);
> > +        hap_flush_tlb_mask(p2m->dirty_cpumask);
> 
> Same here then presumably.
> 
> As suggested in my earlier reply, the plain removals of flush
> invocations would probably better be split out into a separate
> patch.
> 
> > --- a/xen/arch/x86/mm/hap/private.h
> > +++ b/xen/arch/x86/mm/hap/private.h
> > @@ -47,4 +47,9 @@ unsigned long hap_p2m_ga_to_gfn_4_levels(struct vcpu *v,
> >      struct p2m_domain *p2m, unsigned long cr3,
> >      paddr_t ga, uint32_t *pfec, unsigned int *page_order);
> >  
> > +static inline void hap_flush_tlb_mask(const cpumask_t *mask)
> > +{
> > +    flush_mask(mask, FLUSH_HVM_ASID_CORE);
> > +}
> 
> With the above introduction of this would then become unnecessary.

I had planned to make the required adjustment(s) and commit v9 today.
But seeing your comment it appears v10 is warranted.

Wei.

> 
> Jan