From nobody Tue Feb 10 06:43:59 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=linaro.org ARC-Seal: i=1; a=rsa-sha256; t=1613223311; cv=none; d=zohomail.com; s=zohoarc; b=NW08DcmGIY2yNjhZRhrvyTQWMRODEAecuwlYGJXHXGO2WpVLnl02lbREXTy18q0bAGs470NiwqZZ5yk3FJ7J///QeodNRkl+//TXgXJUdxqsCac7o2u69WLjj3dFPXWrzGeVdoYlstaHiNEZCKpiLggVWwldQIzpVEo9As4tc80= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1613223311; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=et68Ul0EK68HfckjM4rQ5ALpeTI2NFkiLkwfcenYZPU=; b=h/otXvOZmKUhhx/RDngcGyeX+Bm7ouK6V5UY7sH83nIv/DGy6gfi/xCtbKFBOKRJijHrcfkkjjTPwK+FGlVXi39f8J5P4Srufprpx2SlNDVup/tU5Rhoh6fBwACloZ3+7mg03RXz5TqNwtzKIbi84ZULFPJ0g1iOV6IottLOnYU= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1613223310882671.7069321382738; Sat, 13 Feb 2021 05:35:10 -0800 (PST) Received: from localhost ([::1]:57592 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lAv4n-0003sM-OM for importer@patchew.org; Sat, 13 Feb 2021 08:35:09 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:47154) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lAuaY-00025V-UW for qemu-devel@nongnu.org; Sat, 13 Feb 2021 08:03:55 -0500 Received: from mail-wm1-x32d.google.com ([2a00:1450:4864:20::32d]:37797) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lAuaT-00068W-IE for qemu-devel@nongnu.org; Sat, 13 Feb 2021 08:03:54 -0500 Received: by mail-wm1-x32d.google.com with SMTP id m1so2512599wml.2 for ; Sat, 13 Feb 2021 05:03:47 -0800 (PST) Received: from zen.linaroharston ([51.148.130.216]) by smtp.gmail.com with ESMTPSA id r1sm14426134wrl.95.2021.02.13.05.03.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 13 Feb 2021 05:03:40 -0800 (PST) Received: from zen.lan (localhost [127.0.0.1]) by zen.linaroharston (Postfix) with ESMTP id 72FD71FFAA; Sat, 13 Feb 2021 13:03:27 +0000 (GMT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=et68Ul0EK68HfckjM4rQ5ALpeTI2NFkiLkwfcenYZPU=; b=YqFH+HvL/pKIPfhNf6VuN5Y3pwIbblowslx6gE5Gluua7r0tcbXEkSS8h8wLMshI0v l4ga6Y72v6Ml9T9M83gE9UPdUnuld/9f+Xf5tyOqz82XPe08PnV+l2bhmZnymVmjyImI iSVeWDI/3qyBYIt3X4yQ5OuVxPwMrHJIQp20FlmHkdBHAnOVitSRz38ecPpWnSWe2Nyo TBqLJxovCHuwLfx05/v6M5eK5ituqQMboZ2743RtFy6A9BjlhIaDdJq6yUYbWuIJqQOK sH9jiA4LAZ8+fi1Nrs9S7xR4FwleU952EEFdDZef1iCHCjcPUzeeHuvu9NhfNY/fKuyZ TcZQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=et68Ul0EK68HfckjM4rQ5ALpeTI2NFkiLkwfcenYZPU=; b=tq1xSBthZkSehGt9M6NFCWJpdBImV2Yx8Fdu8iXfC5vjfpKzmr6z6AOAmUo1o4zPaD mWma7+ZWmZBNd26o8nKNUzQBTWaegH6hFm+JBns2EF7WRJpPNmgNrUjxmOQ2B35+LweO IiXVvuWzd14GLKCra5PF7rbTMfRbR1kBTreu+EthszT2IqkV4vf7e5IEYX2tIt+bhUM5 F/cZmLzfM9eET0P9S3wlnUOf1Zsk5OWS2UbSFv0OTDBR2KKVafFn0LMjBegOHJQWHPdQ b1r4xBwQqj4ifMQi0nGL8uVyszCkscV0g2/fOeao7Zk3kU1kRdMeXSjHBIj4Bc/wjW6E t5EA== X-Gm-Message-State: AOAM532cojykh5OZNlUHlE1eDK0JTyDBHY5sc+GdU7J7Ly1eIOe2z5CJ KUK4yZRHQ59LqNguxfNGifDLeQ== X-Google-Smtp-Source: ABdhPJwHlNmj6GGEJTEjn4+lPFCUvg2wCWHyaJNMPspzCPlptoGQGd3pL5pfOZSEJ+Wrugdi6I08QQ== X-Received: by 2002:a05:600c:3229:: with SMTP id r41mr2179155wmp.164.1613221426372; Sat, 13 Feb 2021 05:03:46 -0800 (PST) From: =?UTF-8?q?Alex=20Benn=C3=A9e?= To: qemu-devel@nongnu.org Subject: [PATCH v3 20/23] accel/tcg: allow plugin instrumentation to be disable via cflags Date: Sat, 13 Feb 2021 13:03:22 +0000 Message-Id: <20210213130325.14781-21-alex.bennee@linaro.org> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20210213130325.14781-1-alex.bennee@linaro.org> References: <20210213130325.14781-1-alex.bennee@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=2a00:1450:4864:20::32d; envelope-from=alex.bennee@linaro.org; helo=mail-wm1-x32d.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Richard Henderson , robhenry@microsoft.com, mahmoudabdalghany@outlook.com, aaron@os.amperecomputing.com, cota@braap.org, Paolo Bonzini , kuhn.chenqun@huawei.com, =?UTF-8?q?Alex=20Benn=C3=A9e?= Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) When icount is enabled and we recompile an MMIO access we end up double counting the instruction execution. To avoid this we introduce the CF_MEMI cflag which only allows memory instrumentation for the next TB (which won't yet have been counted). As this is part of the hashed compile flags we will only execute the generated TB while coming out of a cpu_io_recompile. While we are at it delete the old TODO. We might as well keep the translation handy as it's likely you will repeatedly hit it on each MMIO access. Reported-by: Aaron Lindsay Signed-off-by: Alex Benn=C3=A9e Reviewed-by: Richard Henderson Message-Id: <20210210221053.18050-21-alex.bennee@linaro.org> Tested-by: Aaron Lindsay --- v3 - s/CF_NOINSTR/CF_MEMI_ONY/ - Limit instrumentation at API call sites instead of skipping altogether - clean-up commit log message --- include/exec/exec-all.h | 6 +++--- include/exec/plugin-gen.h | 4 ++-- include/qemu/plugin.h | 4 ++++ accel/tcg/plugin-gen.c | 3 ++- accel/tcg/translate-all.c | 18 +++++++++--------- accel/tcg/translator.c | 5 ++++- plugins/api.c | 36 +++++++++++++++++++++++++----------- 7 files changed, 49 insertions(+), 27 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index e08179de34..77a2dc044d 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -454,14 +454,14 @@ struct TranslationBlock { uint32_t cflags; /* compile flags */ #define CF_COUNT_MASK 0x00007fff #define CF_LAST_IO 0x00008000 /* Last insn may be an IO access. */ +#define CF_MEMI_ONLY 0x00010000 /* Only instrument memory ops */ #define CF_USE_ICOUNT 0x00020000 #define CF_INVALID 0x00040000 /* TB is stale. Set with @jmp_lock held = */ #define CF_PARALLEL 0x00080000 /* Generate code for a parallel context = */ #define CF_CLUSTER_MASK 0xff000000 /* Top 8 bits are cluster ID */ #define CF_CLUSTER_SHIFT 24 -/* cflags' mask for hashing/comparison */ -#define CF_HASH_MASK \ - (CF_COUNT_MASK | CF_LAST_IO | CF_USE_ICOUNT | CF_PARALLEL | CF_CLUSTER= _MASK) +/* cflags' mask for hashing/comparison, basically ignore CF_INVALID */ +#define CF_HASH_MASK (~CF_INVALID) =20 /* Per-vCPU dynamic tracing state used to generate this TB */ uint32_t trace_vcpu_dstate; diff --git a/include/exec/plugin-gen.h b/include/exec/plugin-gen.h index 4834a9e2f4..b1b72b5d90 100644 --- a/include/exec/plugin-gen.h +++ b/include/exec/plugin-gen.h @@ -19,7 +19,7 @@ struct DisasContextBase; =20 #ifdef CONFIG_PLUGIN =20 -bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb); +bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb, bool s= upress); void plugin_gen_tb_end(CPUState *cpu); void plugin_gen_insn_start(CPUState *cpu, const struct DisasContextBase *d= b); void plugin_gen_insn_end(void); @@ -41,7 +41,7 @@ static inline void plugin_insn_append(const void *from, s= ize_t size) #else /* !CONFIG_PLUGIN */ =20 static inline -bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb) +bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb, bool s= upress) { return false; } diff --git a/include/qemu/plugin.h b/include/qemu/plugin.h index 841deed79c..c5a79a89f0 100644 --- a/include/qemu/plugin.h +++ b/include/qemu/plugin.h @@ -92,6 +92,7 @@ struct qemu_plugin_dyn_cb { }; }; =20 +/* Internal context for instrumenting an instruction */ struct qemu_plugin_insn { GByteArray *data; uint64_t vaddr; @@ -99,6 +100,7 @@ struct qemu_plugin_insn { GArray *cbs[PLUGIN_N_CB_TYPES][PLUGIN_N_CB_SUBTYPES]; bool calls_helpers; bool mem_helper; + bool mem_only; }; =20 /* @@ -128,6 +130,7 @@ static inline struct qemu_plugin_insn *qemu_plugin_insn= _alloc(void) return insn; } =20 +/* Internal context for this TranslationBlock */ struct qemu_plugin_tb { GPtrArray *insns; size_t n; @@ -135,6 +138,7 @@ struct qemu_plugin_tb { uint64_t vaddr2; void *haddr1; void *haddr2; + bool mem_only; GArray *cbs[PLUGIN_N_CB_SUBTYPES]; }; =20 diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c index 8a1bb801e0..c3dc3effe7 100644 --- a/accel/tcg/plugin-gen.c +++ b/accel/tcg/plugin-gen.c @@ -842,7 +842,7 @@ static void plugin_gen_inject(const struct qemu_plugin_= tb *plugin_tb) pr_ops(); } =20 -bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb) +bool plugin_gen_tb_start(CPUState *cpu, const TranslationBlock *tb, bool m= em_only) { struct qemu_plugin_tb *ptb =3D tcg_ctx->plugin_tb; bool ret =3D false; @@ -855,6 +855,7 @@ bool plugin_gen_tb_start(CPUState *cpu, const Translati= onBlock *tb) ptb->vaddr2 =3D -1; get_page_addr_code_hostp(cpu->env_ptr, tb->pc, &ptb->haddr1); ptb->haddr2 =3D NULL; + ptb->mem_only =3D mem_only; =20 plugin_gen_empty_callback(PLUGIN_GEN_FROM_TB); } diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c index 0666f9ef14..fdf88dc1c3 100644 --- a/accel/tcg/translate-all.c +++ b/accel/tcg/translate-all.c @@ -2399,7 +2399,8 @@ void tb_check_watchpoint(CPUState *cpu, uintptr_t ret= addr) } =20 #ifndef CONFIG_USER_ONLY -/* in deterministic execution mode, instructions doing device I/Os +/* + * In deterministic execution mode, instructions doing device I/Os * must be at the end of the TB. * * Called by softmmu_template.h, with iothread mutex not held. @@ -2430,19 +2431,18 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t reta= ddr) n =3D 2; } =20 - /* Generate a new TB executing the I/O insn. */ - cpu->cflags_next_tb =3D curr_cflags() | CF_LAST_IO | n; + /* + * Exit the loop and potentially generate a new TB executing the + * just the I/O insns. We also limit instrumentation to memory + * operations only (which execute after completion) so we don't + * double instrument the instruction. + */ + cpu->cflags_next_tb =3D curr_cflags() | CF_MEMI_ONLY | CF_LAST_IO | n; =20 qemu_log_mask_and_addr(CPU_LOG_EXEC, tb->pc, "cpu_io_recompile: rewound execution of TB to " TARGET_FMT_lx "\n", tb->pc); =20 - /* TODO: If env->pc !=3D tb->pc (i.e. the faulting instruction was not - * the first in the TB) then we end up generating a whole new TB and - * repeating the fault, which is horribly inefficient. - * Better would be to execute just this insn uncached, or generate a - * second new TB. - */ cpu_loop_exit_noexc(cpu); } =20 diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index a49a794065..2dfc27102f 100644 --- a/accel/tcg/translator.c +++ b/accel/tcg/translator.c @@ -58,7 +58,8 @@ void translator_loop(const TranslatorOps *ops, DisasConte= xtBase *db, ops->tb_start(db, cpu); tcg_debug_assert(db->is_jmp =3D=3D DISAS_NEXT); /* no early exit */ =20 - plugin_enabled =3D plugin_gen_tb_start(cpu, tb); + plugin_enabled =3D plugin_gen_tb_start(cpu, tb, + tb_cflags(db->tb) & CF_MEMI_ONLY); =20 while (true) { db->num_insns++; @@ -100,6 +101,8 @@ void translator_loop(const TranslatorOps *ops, DisasCon= textBase *db, gen_io_start(); ops->translate_insn(db, cpu); } else { + /* we should only see CF_MEMI_ONLY for io_recompile */ + tcg_debug_assert(!(tb_cflags(db->tb) & CF_MEMI_ONLY)); ops->translate_insn(db, cpu); } =20 diff --git a/plugins/api.c b/plugins/api.c index 5dc8e6f934..0b04380d57 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -84,15 +84,19 @@ void qemu_plugin_register_vcpu_tb_exec_cb(struct qemu_p= lugin_tb *tb, enum qemu_plugin_cb_flags flags, void *udata) { - plugin_register_dyn_cb__udata(&tb->cbs[PLUGIN_CB_REGULAR], - cb, flags, udata); + if (!tb->mem_only) { + plugin_register_dyn_cb__udata(&tb->cbs[PLUGIN_CB_REGULAR], + cb, flags, udata); + } } =20 void qemu_plugin_register_vcpu_tb_exec_inline(struct qemu_plugin_tb *tb, enum qemu_plugin_op op, void *ptr, uint64_t imm) { - plugin_register_inline_op(&tb->cbs[PLUGIN_CB_INLINE], 0, op, ptr, imm); + if (!tb->mem_only) { + plugin_register_inline_op(&tb->cbs[PLUGIN_CB_INLINE], 0, op, ptr, = imm); + } } =20 void qemu_plugin_register_vcpu_insn_exec_cb(struct qemu_plugin_insn *insn, @@ -100,20 +104,27 @@ void qemu_plugin_register_vcpu_insn_exec_cb(struct qe= mu_plugin_insn *insn, enum qemu_plugin_cb_flags flag= s, void *udata) { - plugin_register_dyn_cb__udata(&insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_REG= ULAR], - cb, flags, udata); + if (!insn->mem_only) { + plugin_register_dyn_cb__udata(&insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB= _REGULAR], + cb, flags, udata); + } } =20 void qemu_plugin_register_vcpu_insn_exec_inline(struct qemu_plugin_insn *i= nsn, enum qemu_plugin_op op, void *ptr, uint64_t imm) { - plugin_register_inline_op(&insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INLINE], - 0, op, ptr, imm); + if (!insn->mem_only) { + plugin_register_inline_op(&insn->cbs[PLUGIN_CB_INSN][PLUGIN_CB_INL= INE], + 0, op, ptr, imm); + } } =20 =20 - +/* + * We always plant memory instrumentation because they don't finalise until + * after the operation has complete. + */ void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugin_insn *insn, qemu_plugin_vcpu_mem_cb_t cb, enum qemu_plugin_cb_flags flags, @@ -121,7 +132,7 @@ void qemu_plugin_register_vcpu_mem_cb(struct qemu_plugi= n_insn *insn, void *udata) { plugin_register_vcpu_mem_cb(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_REGULA= R], - cb, flags, rw, udata); + cb, flags, rw, udata); } =20 void qemu_plugin_register_vcpu_mem_inline(struct qemu_plugin_insn *insn, @@ -130,7 +141,7 @@ void qemu_plugin_register_vcpu_mem_inline(struct qemu_p= lugin_insn *insn, uint64_t imm) { plugin_register_inline_op(&insn->cbs[PLUGIN_CB_MEM][PLUGIN_CB_INLINE], - rw, op, ptr, imm); + rw, op, ptr, imm); } =20 void qemu_plugin_register_vcpu_tb_trans_cb(qemu_plugin_id_t id, @@ -181,10 +192,13 @@ uint64_t qemu_plugin_tb_vaddr(const struct qemu_plugi= n_tb *tb) struct qemu_plugin_insn * qemu_plugin_tb_get_insn(const struct qemu_plugin_tb *tb, size_t idx) { + struct qemu_plugin_insn *insn; if (unlikely(idx >=3D tb->n)) { return NULL; } - return g_ptr_array_index(tb->insns, idx); + insn =3D g_ptr_array_index(tb->insns, idx); + insn->mem_only =3D tb->mem_only; + return insn; } =20 /* --=20 2.20.1