From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50607) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWP6c-0002FU-FR for qemu-devel@nongnu.org; Thu, 18 Feb 2016 08:58:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aWP6Z-0007Nk-7Y for qemu-devel@nongnu.org; Thu, 18 Feb 2016 08:58:54 -0500 Received: from mail-ig0-x233.google.com ([2607:f8b0:4001:c05::233]:33633) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aWP6Y-0007Ne-WC for qemu-devel@nongnu.org; Thu, 18 Feb 2016 08:58:51 -0500 Received: by mail-ig0-x233.google.com with SMTP id y8so14143904igp.0 for ; Thu, 18 Feb 2016 05:58:50 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <87ziv79ph5.fsf@linaro.org> References: <1454059965-23402-1-git-send-email-a.rigo@virtualopensystems.com> <1454059965-23402-8-git-send-email-a.rigo@virtualopensystems.com> <87ziv79ph5.fsf@linaro.org> Date: Thu, 18 Feb 2016 14:58:50 +0100 Message-ID: From: alvise rigo Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 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: =?UTF-8?B?QWxleCBCZW5uw6ll?= Cc: MTTCG Devel , Claudio Fontana , QEMU Developers , Paolo Bonzini , Jani Kokkonen , VirtualOpenSystems Technical Team , Richard Henderson On Thu, Feb 11, 2016 at 5:33 PM, Alex Benn=C3=A9e = wrote: > > 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 acce= ss made >> + * by a stcond (see softmmu_template.h). */ >> + bool excl_succeeded; >> >> /* Note that this is accessed at the start of every TB via a negati= ve >> offset from AREG0. Leave this field at the end so as to make th= e >> 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 exclusiv= e >> + * helpers. */ >> + >> +#ifdef BIGENDIAN_EXCLUSIVE_HELPERS >> + >> +#define helper_ldlink_name glue(glue(helper_be_ldlink, USUFFIX), MMUSU= FFIX) >> +#define helper_stcond_name glue(glue(helper_be_stcond, SUFFIX), MMUSUF= FIX) >> +#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), MMUSU= FFIX) >> +#define helper_stcond_name glue(glue(helper_le_stcond, SUFFIX), MMUSUF= FIX) >> +#define helper_ld glue(glue(helper_le_ld, USUFFIX), MMUSUFFIX) >> +#define helper_st glue(glue(helper_le_st, SUFFIX), MMUSUFFIX) >> +#else /* DATA_SIZE <=3D 1 */ >> +#define helper_ldlink_name glue(glue(helper_ret_ldlink, USUFFIX), MMUS= UFFIX) >> +#define helper_stcond_name glue(glue(helper_ret_stcond, SUFFIX), MMUSU= FFIX) >> +#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 =3D 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 =3D CPU_GET_CLASS(this); >> + hwaddr hw_addr; >> + unsigned mmu_idx =3D get_mmuidx(oi); >> + >> + /* Use the proper load helper from cpu_ldst.h */ >> + ret =3D helper_ld(env, addr, oi, retaddr); >> + >> + index =3D (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); >> + >> + /* hw_addr =3D hwaddr of the page (i.e. section->mr->ram_addr + xla= t) >> + * plus the offset (i.e. addr & ~TARGET_PAGE_MASK) */ >> + hw_addr =3D (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 i= s 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 !=3D 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). Indeed, it makes more sense to do as you suggested. Thank you, alvise > >> + 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 |=3D TLB_EXCL; >> + >> + /* From now on we are in LL/SC context */ >> + this->ll_sc_context =3D 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 =3D ENV_GET_CPU(env); >> + >> + if (!cpu->ll_sc_context) { >> + ret =3D 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 =3D true; >> + helper_st(env, addr, val, oi, retaddr); >> + >> + if (cpu->excl_succeeded) { >> + ret =3D 0; >> + } else { >> + ret =3D 1; >> + } >> + } >> + >> + /* Unset LL/SC context */ >> + cpu->ll_sc_context =3D false; >> + cpu->excl_succeeded =3D false; >> + cpu->excl_protected_range.begin =3D 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 ad= dr, 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 helpe= rs */ >> +#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 *e= nv, 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_ulon= g addr, >> + TCGMemOpIdx oi, uintptr_t r= etaddr); >> +tcg_target_ulong helper_le_ldlinkuw_mmu(CPUArchState *env, target_ulong= addr, >> + TCGMemOpIdx oi, uintptr_t r= etaddr); >> +tcg_target_ulong helper_le_ldlinkul_mmu(CPUArchState *env, target_ulong= addr, >> + TCGMemOpIdx oi, uintptr_t r= etaddr); >> +uint64_t helper_le_ldlinkq_mmu(CPUArchState *env, target_ulong addr, >> + TCGMemOpIdx oi, uintptr_t r= etaddr); >> +tcg_target_ulong helper_be_ldlinkuw_mmu(CPUArchState *env, target_ulong= addr, >> + TCGMemOpIdx oi, uintptr_t r= etaddr); >> +tcg_target_ulong helper_be_ldlinkul_mmu(CPUArchState *env, target_ulong= addr, >> + TCGMemOpIdx oi, uintptr_t r= etaddr); >> +uint64_t helper_be_ldlinkq_mmu(CPUArchState *env, target_ulong addr, >> + TCGMemOpIdx oi, uintptr_t r= etaddr); >> >> /* Value sign-extended to tcg register size. */ >> tcg_target_ulong helper_ret_ldsb_mmu(CPUArchState *env, target_ulong ad= dr, >> @@ -1010,6 +1025,22 @@ uint32_t helper_be_ldl_cmmu(CPUArchState *env, ta= rget_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 reta= ddr); >> +tcg_target_ulong helper_le_stcondw_mmu(CPUArchState *env, target_ulong = addr, >> + uint16_t val, TCGMemOpIdx oi, uintptr_t ret= addr); >> +tcg_target_ulong helper_le_stcondl_mmu(CPUArchState *env, target_ulong = addr, >> + uint32_t val, TCGMemOpIdx oi, uintptr_t ret= addr); >> +uint64_t helper_le_stcondq_mmu(CPUArchState *env, target_ulong addr, >> + uint64_t val, TCGMemOpIdx oi, uintptr_t ret= addr); >> +tcg_target_ulong helper_be_stcondw_mmu(CPUArchState *env, target_ulong = addr, >> + uint16_t val, TCGMemOpIdx oi, uintptr_t ret= addr); >> +tcg_target_ulong helper_be_stcondl_mmu(CPUArchState *env, target_ulong = addr, >> + uint32_t val, TCGMemOpIdx oi, uintptr_t ret= addr); >> +uint64_t helper_be_stcondq_mmu(CPUArchState *env, target_ulong addr, >> + uint64_t val, TCGMemOpIdx oi, uintptr_t ret= addr); >> + >> >> /* Temporary aliases until backends are converted. */ >> #ifdef TARGET_WORDS_BIGENDIAN > > > -- > Alex Benn=C3=A9e