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.
Jan
