On 16.04.2020 15:59, Roger Pau Monne wrote:
> Introduce a specific flag to request a HVM guest linear TLB flush,
> which is an ASID/VPID tickle that forces a guest linear to guest
> physical TLB flush for all HVM guests.
>
> This was previously unconditionally done in each pre_flush call, but
> that's not required: HVM guests not using shadow don't require linear
> TLB flushes as Xen doesn't modify the guest page tables in that case
> (ie: when using HAP).
I'm afraid I'm being confused by this: Even in shadow mode Xen
doesn't modify guest page tables, does it?
> @@ -254,3 +257,14 @@ unsigned int flush_area_local(const void *va, unsigned
> int flags)
>
> return flags;
> }
> +
> +void guest_flush_tlb_mask(const struct domain *d, const cpumask_t *mask)
> +{
> + unsigned int flags = (is_pv_domain(d) || paging_mode_shadow(d) ?
> FLUSH_TLB
> + : 0) |
> + (is_hvm_domain(d) && cpu_has_svm ?
> FLUSH_HVM_ASID_CORE
> + : 0);
Why the is_pv_domain() part of the condition? Afaict for PV
domains you can get here only if they have shadow mode enabled.
> --- a/xen/arch/x86/mm/shadow/private.h
> +++ b/xen/arch/x86/mm/shadow/private.h
> @@ -818,6 +818,12 @@ static inline int sh_check_page_has_no_refs(struct
> page_info *page)
> bool shadow_flush_tlb(bool (*flush_vcpu)(void *ctxt, struct vcpu *v),
> void *ctxt);
>
> +static inline void sh_flush_local(const struct domain *d)
> +{
> + flush_local(FLUSH_TLB |
> + (is_hvm_domain(d) && cpu_has_svm ? FLUSH_HVM_ASID_CORE : 0));
> +}
I think the right side of | wants folding with its counterpart in
guest_flush_tlb_mask(). Doing so would avoid guest_flush_tlb_mask()
getting updated but this one forgotten. Perhaps split out
guest_flush_tlb_flags() from guest_flush_tlb_mask()?
I also think this function should move into multi.c as long as it's
needed only there.
Jan
