From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48162) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9rP0-0005dl-N9 for qemu-devel@nongnu.org; Mon, 06 Jun 2016 06:04:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b9rOu-0003B0-K3 for qemu-devel@nongnu.org; Mon, 06 Jun 2016 06:04:57 -0400 Received: from mail-lf0-x241.google.com ([2a00:1450:4010:c07::241]:33862) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b9rOu-0003Aw-C2 for qemu-devel@nongnu.org; Mon, 06 Jun 2016 06:04:52 -0400 Received: by mail-lf0-x241.google.com with SMTP id k192so4459702lfb.1 for ; Mon, 06 Jun 2016 03:04:52 -0700 (PDT) References: <87inxmitp3.fsf@linaro.org> From: Sergey Fedorov Message-ID: <57554AC2.2080705@gmail.com> Date: Mon, 6 Jun 2016 13:04:50 +0300 MIME-Version: 1.0 In-Reply-To: <87inxmitp3.fsf@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC v1 08/12] cputlb: introduce tlb_flush_* async work. List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?Q?Alex_Benn=c3=a9e?= Cc: mttcg@listserver.greensocs.com, fred.konrad@greensocs.com, a.rigo@virtualopensystems.com, cota@braap.org, qemu-devel@nongnu.org, mark.burton@greensocs.com, pbonzini@redhat.com, jan.kiszka@siemens.com, rth@twiddle.net, peter.maydell@linaro.org, claudio.fontana@huawei.com, Peter Crosthwaite On 06/06/16 11:54, Alex Bennée wrote: > Sergey Fedorov writes: > >> On 15/04/16 17:23, Alex Bennée wrote: >>> diff --git a/cputlb.c b/cputlb.c >>> index 1412049..42a3b07 100644 >>> --- a/cputlb.c >>> +++ b/cputlb.c (snip) >>> @@ -89,6 +81,34 @@ void tlb_flush(CPUState *cpu, int flush_global) >>> env->tlb_flush_addr = -1; >>> env->tlb_flush_mask = 0; >>> tlb_flush_count++; >>> + /* atomic_mb_set(&cpu->pending_tlb_flush, 0); */ >>> +} >>> + >>> +static void tlb_flush_global_async_work(CPUState *cpu, void *opaque) >>> +{ >>> + tlb_flush_nocheck(cpu, GPOINTER_TO_INT(opaque)); >>> +} >>> + >>> +/* NOTE: >>> + * If flush_global is true (the usual case), flush all tlb entries. >>> + * If flush_global is false, flush (at least) all tlb entries not >>> + * marked global. >>> + * >>> + * Since QEMU doesn't currently implement a global/not-global flag >>> + * for tlb entries, at the moment tlb_flush() will also flush all >>> + * tlb entries in the flush_global == false case. This is OK because >>> + * CPU architectures generally permit an implementation to drop >>> + * entries from the TLB at any time, so flushing more entries than >>> + * required is only an efficiency issue, not a correctness issue. >>> + */ >>> +void tlb_flush(CPUState *cpu, int flush_global) >>> +{ >>> + if (cpu->created) { >> Why do we check for 'cpu->created' here? Any why don't do that in >> tlb_flush_page_all()? > A bunch of random stuff gets kicked off at start-up which was getting in > the way (c.f. arm_cpu_reset and watch/breakpoints). tlb_flush() is > rather liberally sprinkled around the init code of various CPUs. Wouldn't we race if we call tlb_flush() with no BQL held? We could just queue an async job since we force each CPU thread to flush its work queue before starting execution anyway. Kind regards, Sergey > >>> + async_run_on_cpu(cpu, tlb_flush_global_async_work, >>> + GINT_TO_POINTER(flush_global)); >>> + } else { >>> + tlb_flush_nocheck(cpu, flush_global); >>> + } >>> } >>> >>> static inline void v_tlb_flush_by_mmuidx(CPUState *cpu, va_list argp) >>>