From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45713) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTuBB-0001hE-VZ for qemu-devel@nongnu.org; Thu, 11 Feb 2016 11:33:22 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aTuB7-0000Ui-RY for qemu-devel@nongnu.org; Thu, 11 Feb 2016 11:33:17 -0500 Received: from mail-wm0-x229.google.com ([2a00:1450:400c:c09::229]:38529) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aTuB7-0000UZ-Dv for qemu-devel@nongnu.org; Thu, 11 Feb 2016 11:33:13 -0500 Received: by mail-wm0-x229.google.com with SMTP id p63so75911811wmp.1 for ; Thu, 11 Feb 2016 08:33:13 -0800 (PST) References: <1454059965-23402-1-git-send-email-a.rigo@virtualopensystems.com> <1454059965-23402-8-git-send-email-a.rigo@virtualopensystems.com> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: <1454059965-23402-8-git-send-email-a.rigo@virtualopensystems.com> Date: Thu, 11 Feb 2016 16:33:10 +0000 Message-ID: <87ziv79ph5.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC v7 07/16] softmmu: Add helpers for a new slowpath List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alvise Rigo Cc: mttcg@listserver.greensocs.com, claudio.fontana@huawei.com, qemu-devel@nongnu.org, pbonzini@redhat.com, jani.kokkonen@huawei.com, tech@virtualopensystems.com, rth@twiddle.net Alvise Rigo writes: > The new helpers rely on the legacy ones to perform the actual read/write. > > The LoadLink helper (helper_ldlink_name) prepares the way for the > following StoreCond operation. It sets the linked address and the size > of the access. The LoadLink helper also updates the TLB entry of the > page involved in the LL/SC to all vCPUs by forcing a TLB flush, so that > the following accesses made by all the vCPUs will follow the slow path. > > The StoreConditional helper (helper_stcond_name) returns 1 if the > store has to fail due to a concurrent access to the same page by > another vCPU. A 'concurrent access' can be a store made by *any* vCPU > (although, some implementations allow stores made by the CPU that issued > the LoadLink). > > Suggested-by: Jani Kokkonen > Suggested-by: Claudio Fontana > Signed-off-by: Alvise Rigo > --- > cputlb.c | 3 ++ > include/qom/cpu.h | 5 ++ > softmmu_llsc_template.h | 133 ++++++++++++++++++++++++++++++++++++++++++++++++ > softmmu_template.h | 12 +++++ > tcg/tcg.h | 31 +++++++++++ > 5 files changed, 184 insertions(+) > create mode 100644 softmmu_llsc_template.h > > diff --git a/cputlb.c b/cputlb.c > index f6fb161..ce6d720 100644 > --- a/cputlb.c > +++ b/cputlb.c > @@ -476,6 +476,8 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) > > #define MMUSUFFIX _mmu > > +/* Generates LoadLink/StoreConditional helpers in softmmu_template.h */ > +#define GEN_EXCLUSIVE_HELPERS > #define SHIFT 0 > #include "softmmu_template.h" > > @@ -488,6 +490,7 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr) > #define SHIFT 3 > #include "softmmu_template.h" > #undef MMUSUFFIX > +#undef GEN_EXCLUSIVE_HELPERS > > #define MMUSUFFIX _cmmu > #undef GETPC_ADJ > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 682c81d..6f6c1c0 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -351,10 +351,15 @@ struct CPUState { > */ > bool throttle_thread_scheduled; > > + /* Used by the atomic insn translation backend. */ > + bool ll_sc_context; > /* vCPU's exclusive addresses range. > * The address is set to EXCLUSIVE_RESET_ADDR if the vCPU is not > * in the middle of a LL/SC. */ > struct Range excl_protected_range; > + /* Used to carry the SC result but also to flag a normal store access made > + * by a stcond (see softmmu_template.h). */ > + bool excl_succeeded; > > /* Note that this is accessed at the start of every TB via a negative > offset from AREG0. Leave this field at the end so as to make the > diff --git a/softmmu_llsc_template.h b/softmmu_llsc_template.h > new file mode 100644 > index 0000000..101f5e8 > --- /dev/null > +++ b/softmmu_llsc_template.h > @@ -0,0 +1,133 @@ > +/* > + * Software MMU support (esclusive load/store operations) > + * > + * Generate helpers used by TCG for qemu_ldlink/stcond ops. > + * > + * Included from softmmu_template.h only. > + * > + * Copyright (c) 2015 Virtual Open Systems > + * > + * Authors: > + * Alvise Rigo > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library; if not, see . > + */ > + > +/* This template does not generate together the le and be version, but only one > + * of the two depending on whether BIGENDIAN_EXCLUSIVE_HELPERS has been set. > + * The same nomenclature as softmmu_template.h is used for the exclusive > + * helpers. */ > + > +#ifdef BIGENDIAN_EXCLUSIVE_HELPERS > + > +#define helper_ldlink_name glue(glue(helper_be_ldlink, USUFFIX), MMUSUFFIX) > +#define helper_stcond_name glue(glue(helper_be_stcond, SUFFIX), MMUSUFFIX) > +#define helper_ld glue(glue(helper_be_ld, USUFFIX), MMUSUFFIX) > +#define helper_st glue(glue(helper_be_st, SUFFIX), MMUSUFFIX) > + > +#else /* LE helpers + 8bit helpers (generated only once for both LE end BE) */ > + > +#if DATA_SIZE > 1 > +#define helper_ldlink_name glue(glue(helper_le_ldlink, USUFFIX), MMUSUFFIX) > +#define helper_stcond_name glue(glue(helper_le_stcond, SUFFIX), MMUSUFFIX) > +#define helper_ld glue(glue(helper_le_ld, USUFFIX), MMUSUFFIX) > +#define helper_st glue(glue(helper_le_st, SUFFIX), MMUSUFFIX) > +#else /* DATA_SIZE <= 1 */ > +#define helper_ldlink_name glue(glue(helper_ret_ldlink, USUFFIX), MMUSUFFIX) > +#define helper_stcond_name glue(glue(helper_ret_stcond, SUFFIX), MMUSUFFIX) > +#define helper_ld glue(glue(helper_ret_ld, USUFFIX), MMUSUFFIX) > +#define helper_st glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX) > +#endif > + > +#endif > + > +WORD_TYPE helper_ldlink_name(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr) > +{ > + WORD_TYPE ret; > + int index; > + CPUState *cpu, *this = ENV_GET_CPU(env); I'd rename this to this_cpu and move the *cpu definition to inside the if {} where it is used so no confusion occurs. > + CPUClass *cc = CPU_GET_CLASS(this); > + hwaddr hw_addr; > + unsigned mmu_idx = get_mmuidx(oi); > + > + /* Use the proper load helper from cpu_ldst.h */ > + ret = helper_ld(env, addr, oi, retaddr); > + > + index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > + > + /* hw_addr = hwaddr of the page (i.e. section->mr->ram_addr + xlat) > + * plus the offset (i.e. addr & ~TARGET_PAGE_MASK) */ > + hw_addr = (env->iotlb[mmu_idx][index].addr & TARGET_PAGE_MASK) + addr; > + if (likely(!(env->tlb_table[mmu_idx][index].addr_read & TLB_MMIO))) { > + /* If all the vCPUs have the EXCL bit set for this page there is no need > + * to request any flush. */ > + if (!cpu_physical_memory_is_excl(hw_addr)) { > + cpu_physical_memory_set_excl(hw_addr); > + CPU_FOREACH(cpu) { > + if (current_cpu != cpu) { Why use current_cpu is we have this_cpu? I'd argue the check should use what we've been passed over global (and future TLS value). > + tlb_flush(cpu, 1); > + } > + } > + } > + } else { > + hw_error("EXCL accesses to MMIO regions not supported yet."); > + } > + > + cc->cpu_set_excl_protected_range(this, hw_addr, DATA_SIZE); > + > + /* For this vCPU, just update the TLB entry, no need to flush. */ > + env->tlb_table[mmu_idx][index].addr_write |= TLB_EXCL; > + > + /* From now on we are in LL/SC context */ > + this->ll_sc_context = true; > + > + return ret; > +} > + > +WORD_TYPE helper_stcond_name(CPUArchState *env, target_ulong addr, > + DATA_TYPE val, TCGMemOpIdx oi, > + uintptr_t retaddr) > +{ > + WORD_TYPE ret; > + CPUState *cpu = ENV_GET_CPU(env); > + > + if (!cpu->ll_sc_context) { > + ret = 1; > + } else { > + /* We set it preventively to true to distinguish the following legacy > + * access as one made by the store conditional wrapper. If the store > + * conditional does not succeed, the value will be set to 0.*/ > + cpu->excl_succeeded = true; > + helper_st(env, addr, val, oi, retaddr); > + > + if (cpu->excl_succeeded) { > + ret = 0; > + } else { > + ret = 1; > + } > + } > + > + /* Unset LL/SC context */ > + cpu->ll_sc_context = false; > + cpu->excl_succeeded = false; > + cpu->excl_protected_range.begin = EXCLUSIVE_RESET_ADDR; > + > + return ret; > +} > + > +#undef helper_ldlink_name > +#undef helper_stcond_name > +#undef helper_ld > +#undef helper_st > diff --git a/softmmu_template.h b/softmmu_template.h > index 6279437..4332db2 100644 > --- a/softmmu_template.h > +++ b/softmmu_template.h > @@ -622,6 +622,18 @@ void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, > #endif > #endif /* !defined(SOFTMMU_CODE_ACCESS) */ > > +#ifdef GEN_EXCLUSIVE_HELPERS > + > +#if DATA_SIZE > 1 /* The 8-bit helpers are generate along with LE helpers */ > +#define BIGENDIAN_EXCLUSIVE_HELPERS > +#include "softmmu_llsc_template.h" > +#undef BIGENDIAN_EXCLUSIVE_HELPERS > +#endif > + > +#include "softmmu_llsc_template.h" > + > +#endif /* !defined(GEN_EXCLUSIVE_HELPERS) */ > + > #undef READ_ACCESS_TYPE > #undef SHIFT > #undef DATA_TYPE > diff --git a/tcg/tcg.h b/tcg/tcg.h > index a696922..3e050a4 100644 > --- a/tcg/tcg.h > +++ b/tcg/tcg.h > @@ -968,6 +968,21 @@ tcg_target_ulong helper_be_ldul_mmu(CPUArchState *env, target_ulong addr, > TCGMemOpIdx oi, uintptr_t retaddr); > uint64_t helper_be_ldq_mmu(CPUArchState *env, target_ulong addr, > TCGMemOpIdx oi, uintptr_t retaddr); > +/* Exclusive variants */ > +tcg_target_ulong helper_ret_ldlinkub_mmu(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_le_ldlinkuw_mmu(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_le_ldlinkul_mmu(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr); > +uint64_t helper_le_ldlinkq_mmu(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_be_ldlinkuw_mmu(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_be_ldlinkul_mmu(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr); > +uint64_t helper_be_ldlinkq_mmu(CPUArchState *env, target_ulong addr, > + TCGMemOpIdx oi, uintptr_t retaddr); > > /* Value sign-extended to tcg register size. */ > tcg_target_ulong helper_ret_ldsb_mmu(CPUArchState *env, target_ulong addr, > @@ -1010,6 +1025,22 @@ uint32_t helper_be_ldl_cmmu(CPUArchState *env, target_ulong addr, > TCGMemOpIdx oi, uintptr_t retaddr); > uint64_t helper_be_ldq_cmmu(CPUArchState *env, target_ulong addr, > TCGMemOpIdx oi, uintptr_t retaddr); > +/* Exclusive variants */ > +tcg_target_ulong helper_ret_stcondb_mmu(CPUArchState *env, target_ulong addr, > + uint8_t val, TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_le_stcondw_mmu(CPUArchState *env, target_ulong addr, > + uint16_t val, TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_le_stcondl_mmu(CPUArchState *env, target_ulong addr, > + uint32_t val, TCGMemOpIdx oi, uintptr_t retaddr); > +uint64_t helper_le_stcondq_mmu(CPUArchState *env, target_ulong addr, > + uint64_t val, TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_be_stcondw_mmu(CPUArchState *env, target_ulong addr, > + uint16_t val, TCGMemOpIdx oi, uintptr_t retaddr); > +tcg_target_ulong helper_be_stcondl_mmu(CPUArchState *env, target_ulong addr, > + uint32_t val, TCGMemOpIdx oi, uintptr_t retaddr); > +uint64_t helper_be_stcondq_mmu(CPUArchState *env, target_ulong addr, > + uint64_t val, TCGMemOpIdx oi, uintptr_t retaddr); > + > > /* Temporary aliases until backends are converted. */ > #ifdef TARGET_WORDS_BIGENDIAN -- Alex Bennée