| Message ID | 20200127181115.82709-5-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: improve assisted tlb flush and use it in guest mode | expand |
On Mon, Jan 27, 2020 at 07:11:12PM +0100, Roger Pau Monne wrote: > The current implementation of the hypervisor assisted flush for HAP is > extremely inefficient. > > First of all there's no need to call paging_update_cr3, as the only > relevant part of that function when doing a flush is the ASID vCPU > flush, so just call that function directly. > > Since hvm_asid_flush_vcpu is protected against concurrent callers by > using atomic operations there's no need anymore to pause the affected > vCPUs. > > Finally the global TLB flush performed by flush_tlb_mask is also not > necessary, since we only want to flush the guest TLB state it's enough > to trigger a vmexit on the pCPUs currently holding any vCPU state, as > such vmexit will already perform an ASID/VPID update, and thus clear > the guest TLB. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/mm/hap/hap.c | 48 +++++++++++++++++---------------------- > 1 file changed, 21 insertions(+), 27 deletions(-) > > diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c > index 6894c1aa38..401eaf8026 100644 > --- a/xen/arch/x86/mm/hap/hap.c > +++ b/xen/arch/x86/mm/hap/hap.c > @@ -669,32 +669,24 @@ static void hap_update_cr3(struct vcpu *v, int do_locking, bool noflush) > hvm_update_guest_cr3(v, noflush); > } > > +static void do_flush(void *data) I think the name is misleading, because this function doesn't flush the TLB itself. We're relying on the side effect of vmexit to flush. I don't have suggestion for a good name, though, so this comment is not blocking. > +{ > + cpumask_t *mask = data; > + unsigned int cpu = smp_processor_id(); > + > + ASSERT(cpumask_test_cpu(cpu, mask)); > + cpumask_clear_cpu(cpu, mask); > +} > + > bool hap_flush_tlb(bool (*flush_vcpu)(void *ctxt, struct vcpu *v), > void *ctxt) > { > static DEFINE_PER_CPU(cpumask_t, flush_cpumask); > cpumask_t *mask = &this_cpu(flush_cpumask); > struct domain *d = current->domain; > + unsigned int this_cpu = smp_processor_id(); > struct vcpu *v; > > - /* Avoid deadlock if more than one vcpu tries this at the same time. */ > - if ( !spin_trylock(&d->hypercall_deadlock_mutex) ) > - return false; > - > - /* Pause all other vcpus. */ > - for_each_vcpu ( d, v ) > - if ( v != current && flush_vcpu(ctxt, v) ) > - vcpu_pause_nosync(v); > - > - /* Now that all VCPUs are signalled to deschedule, we wait... */ > - for_each_vcpu ( d, v ) > - if ( v != current && flush_vcpu(ctxt, v) ) > - while ( !vcpu_runnable(v) && v->is_running ) > - cpu_relax(); > - > - /* All other vcpus are paused, safe to unlock now. */ > - spin_unlock(&d->hypercall_deadlock_mutex); > - > cpumask_clear(mask); > > /* Flush paging-mode soft state (e.g., va->gfn cache; PAE PDPE cache). */ > @@ -705,20 +697,22 @@ bool hap_flush_tlb(bool (*flush_vcpu)(void *ctxt, struct vcpu *v), > if ( !flush_vcpu(ctxt, v) ) > continue; > > - paging_update_cr3(v, false); > + hvm_asid_flush_vcpu(v); > > cpu = read_atomic(&v->dirty_cpu); > - if ( is_vcpu_dirty_cpu(cpu) ) > + if ( cpu != this_cpu && is_vcpu_dirty_cpu(cpu) ) > __cpumask_set_cpu(cpu, mask); > } > > - /* Flush TLBs on all CPUs with dirty vcpu state. */ > - flush_tlb_mask(mask); > - > - /* Done. */ > - for_each_vcpu ( d, v ) > - if ( v != current && flush_vcpu(ctxt, v) ) > - vcpu_unpause(v); > + /* > + * Trigger a vmexit on all pCPUs with dirty vCPU state in order to force an > + * ASID/VPIT change and hence accomplish a guest TLB flush. Note that vCPUs > + * not currently running will already be flushed when scheduled because of > + * the ASID tickle done in the loop above. > + */ VPIT -> VPID Reviewed-by: Wei Liu <wl@xen.org> > + on_selected_cpus(mask, do_flush, mask, 0); > + while ( !cpumask_empty(mask) ) > + cpu_relax(); > > return true; > } > -- > 2.25.0 >
diff --git a/xen/arch/x86/mm/hap/hap.c b/xen/arch/x86/mm/hap/hap.c index 6894c1aa38..401eaf8026 100644 --- a/xen/arch/x86/mm/hap/hap.c +++ b/xen/arch/x86/mm/hap/hap.c @@ -669,32 +669,24 @@ static void hap_update_cr3(struct vcpu *v, int do_locking, bool noflush) hvm_update_guest_cr3(v, noflush); } +static void do_flush(void *data) +{ + cpumask_t *mask = data; + unsigned int cpu = smp_processor_id(); + + ASSERT(cpumask_test_cpu(cpu, mask)); + cpumask_clear_cpu(cpu, mask); +} + bool hap_flush_tlb(bool (*flush_vcpu)(void *ctxt, struct vcpu *v), void *ctxt) { static DEFINE_PER_CPU(cpumask_t, flush_cpumask); cpumask_t *mask = &this_cpu(flush_cpumask); struct domain *d = current->domain; + unsigned int this_cpu = smp_processor_id(); struct vcpu *v; - /* Avoid deadlock if more than one vcpu tries this at the same time. */ - if ( !spin_trylock(&d->hypercall_deadlock_mutex) ) - return false; - - /* Pause all other vcpus. */ - for_each_vcpu ( d, v ) - if ( v != current && flush_vcpu(ctxt, v) ) - vcpu_pause_nosync(v); - - /* Now that all VCPUs are signalled to deschedule, we wait... */ - for_each_vcpu ( d, v ) - if ( v != current && flush_vcpu(ctxt, v) ) - while ( !vcpu_runnable(v) && v->is_running ) - cpu_relax(); - - /* All other vcpus are paused, safe to unlock now. */ - spin_unlock(&d->hypercall_deadlock_mutex); - cpumask_clear(mask); /* Flush paging-mode soft state (e.g., va->gfn cache; PAE PDPE cache). */ @@ -705,20 +697,22 @@ bool hap_flush_tlb(bool (*flush_vcpu)(void *ctxt, struct vcpu *v), if ( !flush_vcpu(ctxt, v) ) continue; - paging_update_cr3(v, false); + hvm_asid_flush_vcpu(v); cpu = read_atomic(&v->dirty_cpu); - if ( is_vcpu_dirty_cpu(cpu) ) + if ( cpu != this_cpu && is_vcpu_dirty_cpu(cpu) ) __cpumask_set_cpu(cpu, mask); } - /* Flush TLBs on all CPUs with dirty vcpu state. */ - flush_tlb_mask(mask); - - /* Done. */ - for_each_vcpu ( d, v ) - if ( v != current && flush_vcpu(ctxt, v) ) - vcpu_unpause(v); + /* + * Trigger a vmexit on all pCPUs with dirty vCPU state in order to force an + * ASID/VPIT change and hence accomplish a guest TLB flush. Note that vCPUs + * not currently running will already be flushed when scheduled because of + * the ASID tickle done in the loop above. + */ + on_selected_cpus(mask, do_flush, mask, 0); + while ( !cpumask_empty(mask) ) + cpu_relax(); return true; }
The current implementation of the hypervisor assisted flush for HAP is extremely inefficient. First of all there's no need to call paging_update_cr3, as the only relevant part of that function when doing a flush is the ASID vCPU flush, so just call that function directly. Since hvm_asid_flush_vcpu is protected against concurrent callers by using atomic operations there's no need anymore to pause the affected vCPUs. Finally the global TLB flush performed by flush_tlb_mask is also not necessary, since we only want to flush the guest TLB state it's enough to trigger a vmexit on the pCPUs currently holding any vCPU state, as such vmexit will already perform an ASID/VPID update, and thus clear the guest TLB. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/mm/hap/hap.c | 48 +++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 27 deletions(-)