[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v5 4/7] x86/tlb: introduce a flush guests TLB flag



On 19.02.2020 18:43, Roger Pau Monne wrote:
> Introduce a specific flag to request a HVM guest TLB flush, which is
> an ASID/VPID tickle that forces a linear TLB flush for all HVM guests.

Here and below, what do you mean by "linear"? I guess you mean
TLBs holding translations from guest linear to guest physical,
but I think this could do with then also saying so, even if it's
more words.

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

This explains the correctness in one direction. What about the
removal of this from the switch_cr3_cr4() path? And what about
our safety assumptions from the ticking of tlbflush_clock,
where we then imply that pages e.g. about to be freed can't
have any translations left in any TLBs anymore?

> --- a/xen/include/asm-x86/flushtlb.h
> +++ b/xen/include/asm-x86/flushtlb.h
> @@ -105,6 +105,8 @@ void switch_cr3_cr4(unsigned long cr3, unsigned long cr4);
>  #define FLUSH_VCPU_STATE 0x1000
>   /* Flush the per-cpu root page table */
>  #define FLUSH_ROOT_PGTBL 0x2000
> + /* Flush all HVM guests linear TLB (using ASID/VPID) */
> +#define FLUSH_GUESTS_TLB 0x4000

For one, the "all" is pretty misleading. A single such request
doesn't do this for all vCPU-s of all HVM guests, does it? I'm
also struggling with the 'S' in "GUESTS" - why is it not just
"GUEST"? I admit the names of the involved functions
(hvm_flush_guest_tlbs(), hvm_asid_flush_core()) are somewhat
misleading, as they don't actually do any flushing, they merely
arrange for what is in the TLB to no longer be able to be used,
so giving this a suitable name is "historically" complicated.
What if we did away with the hvm_flush_guest_tlbs() wrapper,
naming the constant here then after hvm_asid_flush_core(), e.g.
FLUSH_ASID_CORE?

I also think this constant might better be zero when !HVM.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.