From patchwork Tue Sep 27 06:50:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leon Alrae X-Patchwork-Id: 675410 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sjs2v1yyLz9stY for ; Tue, 27 Sep 2016 16:52:20 +1000 (AEST) Received: from localhost ([::1]:48374 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bomFT-0005sb-Ed for incoming@patchwork.ozlabs.org; Tue, 27 Sep 2016 02:52:15 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59745) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bomEP-0004Pm-KH for qemu-devel@nongnu.org; Tue, 27 Sep 2016 02:51:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bomEM-0000CV-TK for qemu-devel@nongnu.org; Tue, 27 Sep 2016 02:51:08 -0400 Received: from mailapp02.imgtec.com ([217.156.133.132]:42733 helo=mailapp01.imgtec.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bomEM-0000C6-KJ for qemu-devel@nongnu.org; Tue, 27 Sep 2016 02:51:06 -0400 Received: from hhmail02.hh.imgtec.org (unknown [10.100.10.20]) by Forcepoint Email with ESMTPS id 68AEEAE33CE3F; Tue, 27 Sep 2016 07:51:02 +0100 (IST) Received: from hhmipssw204.hh.imgtec.org (10.100.21.121) by hhmail02.hh.imgtec.org (10.100.10.20) with Microsoft SMTP Server (TLS) id 14.3.294.0; Tue, 27 Sep 2016 07:51:03 +0100 From: Leon Alrae To: Date: Tue, 27 Sep 2016 07:50:48 +0100 Message-ID: <1474959049-7061-2-git-send-email-leon.alrae@imgtec.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1474959049-7061-1-git-send-email-leon.alrae@imgtec.com> References: <1474959049-7061-1-git-send-email-leon.alrae@imgtec.com> MIME-Version: 1.0 X-Originating-IP: [10.100.21.121] X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 217.156.133.132 Subject: [Qemu-devel] [PATCH v3 1/2] target-mips: compare virtual addresses in LL/SC sequence X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: aurelien@aurel32.net, rth@twiddle.net Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Until now we have been comparing physical addresses in LL/SC sequence. Unfortunately that means that on each SC we have to do the address translation which is a quite complex operation. If we could get rid of it then it would allow us to throw away SC helpers and benefit from having common implementation of SC in user and system mode (currently we have 2 separate implementations selected by #ifdef CONFIG_USER_ONLY). Given our LL/SC emulation is already very simplified (as we only compare the address and value), using virtual addresses instead of physical does not seem to be a gross violation. Correct guest software should not rely on LL/SC if they accesses the same physical address via different virtual addresses or if page mapping gets changed between LL/SC due to manipulating tlb entries. MIPS Instruction Set Manual clearly says that an RMW sequence must use the same address in the LL and SC (virtual address, physical address, cacheability and coherency attributes must be identical). Otherwise the result of the SC is not predictable. This patch takes advantage of this fact and removes the virtual -> physical address translation from SC helper. lladdr served as Coprocessor 0 LLAddr register which captures physical address of the most recent LL instruction, and also lladdr was used for comparison with following SC physical address. This patch changes the meaning of lladdr - now it will only keep the virtual address of the most recent LL. Additionally we introduce CP0_LLAddr which is the actual Coperocessor 0 LLAddr register that guest can access. Signed-off-by: Leon Alrae --- target-mips/cpu.h | 3 ++- target-mips/machine.c | 7 ++++--- target-mips/op_helper.c | 29 +++++++++++++++++------------ target-mips/translate.c | 4 ++-- 4 files changed, 25 insertions(+), 18 deletions(-) diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 5182dc7..78555b9 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -494,10 +494,11 @@ struct CPUMIPSState { #define CP0C5_NFExists 0 int32_t CP0_Config6; int32_t CP0_Config7; + uint64_t CP0_LLAddr; uint64_t CP0_MAAR[MIPS_MAAR_MAX]; int32_t CP0_MAARI; /* XXX: Maybe make LLAddr per-TC? */ - uint64_t lladdr; + target_ulong lladdr; /* LL virtual address compared against SC */ target_ulong llval; target_ulong llnewval; target_ulong llreg; diff --git a/target-mips/machine.c b/target-mips/machine.c index a27f2f1..deebdf2 100644 --- a/target-mips/machine.c +++ b/target-mips/machine.c @@ -206,8 +206,8 @@ const VMStateDescription vmstate_tlb = { const VMStateDescription vmstate_mips_cpu = { .name = "cpu", - .version_id = 8, - .minimum_version_id = 8, + .version_id = 9, + .minimum_version_id = 9, .post_load = cpu_post_load, .fields = (VMStateField[]) { /* Active TC */ @@ -274,9 +274,10 @@ const VMStateDescription vmstate_mips_cpu = { VMSTATE_INT32(env.CP0_Config3, MIPSCPU), VMSTATE_INT32(env.CP0_Config6, MIPSCPU), VMSTATE_INT32(env.CP0_Config7, MIPSCPU), + VMSTATE_UINT64(env.CP0_LLAddr, MIPSCPU), VMSTATE_UINT64_ARRAY(env.CP0_MAAR, MIPSCPU, MIPS_MAAR_MAX), VMSTATE_INT32(env.CP0_MAARI, MIPSCPU), - VMSTATE_UINT64(env.lladdr, MIPSCPU), + VMSTATE_UINTTL(env.lladdr, MIPSCPU), VMSTATE_UINTTL_ARRAY(env.CP0_WatchLo, MIPSCPU, 8), VMSTATE_INT32_ARRAY(env.CP0_WatchHi, MIPSCPU, 8), VMSTATE_UINTTL(env.CP0_XContext, MIPSCPU), diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c index ea2f2ab..e0c9842 100644 --- a/target-mips/op_helper.c +++ b/target-mips/op_helper.c @@ -271,15 +271,15 @@ static inline hwaddr do_translate_address(CPUMIPSState *env, target_ulong address, int rw, uintptr_t retaddr) { - hwaddr lladdr; + hwaddr paddr; CPUState *cs = CPU(mips_env_get_cpu(env)); - lladdr = cpu_mips_translate_address(env, address, rw); + paddr = cpu_mips_translate_address(env, address, rw); - if (lladdr == -1LL) { + if (paddr == -1LL) { cpu_loop_exit_restore(cs, retaddr); } else { - return lladdr; + return paddr; } } @@ -290,7 +290,8 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx) \ env->CP0_BadVAddr = arg; \ do_raise_exception(env, EXCP_AdEL, GETPC()); \ } \ - env->lladdr = do_translate_address(env, arg, 0, GETPC()); \ + env->CP0_LLAddr = do_translate_address(env, arg, 0, GETPC()); \ + env->lladdr = arg; \ env->llval = do_##insn(env, arg, mem_idx, GETPC()); \ return env->llval; \ } @@ -310,7 +311,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1, \ env->CP0_BadVAddr = arg2; \ do_raise_exception(env, EXCP_AdES, GETPC()); \ } \ - if (do_translate_address(env, arg2, 1, GETPC()) == env->lladdr) { \ + if (arg2 == env->lladdr) { \ tmp = do_##ld_insn(env, arg2, mem_idx, GETPC()); \ if (tmp == env->llval) { \ do_##st_insn(env, arg2, arg1, mem_idx, GETPC()); \ @@ -885,7 +886,7 @@ target_ulong helper_mftc0_status(CPUMIPSState *env) target_ulong helper_mfc0_lladdr(CPUMIPSState *env) { - return (int32_t)(env->lladdr >> env->CP0_LLAddr_shift); + return (int32_t)(env->CP0_LLAddr >> env->CP0_LLAddr_shift); } target_ulong helper_mfc0_maar(CPUMIPSState *env) @@ -961,7 +962,7 @@ target_ulong helper_dmfc0_tcschefback(CPUMIPSState *env) target_ulong helper_dmfc0_lladdr(CPUMIPSState *env) { - return env->lladdr >> env->CP0_LLAddr_shift; + return env->CP0_LLAddr >> env->CP0_LLAddr_shift; } target_ulong helper_dmfc0_maar(CPUMIPSState *env) @@ -1189,7 +1190,8 @@ void helper_mtc0_tcrestart(CPUMIPSState *env, target_ulong arg1) { env->active_tc.PC = arg1; env->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS); - env->lladdr = 0ULL; + env->CP0_LLAddr = 0; + env->lladdr = 0; /* MIPS16 not implemented. */ } @@ -1201,12 +1203,14 @@ void helper_mttc0_tcrestart(CPUMIPSState *env, target_ulong arg1) if (other_tc == other->current_tc) { other->active_tc.PC = arg1; other->active_tc.CP0_TCStatus &= ~(1 << CP0TCSt_TDS); - other->lladdr = 0ULL; + other->CP0_LLAddr = 0; + other->lladdr = 0; /* MIPS16 not implemented. */ } else { other->tcs[other_tc].PC = arg1; other->tcs[other_tc].CP0_TCStatus &= ~(1 << CP0TCSt_TDS); - other->lladdr = 0ULL; + other->CP0_LLAddr = 0; + other->lladdr = 0; /* MIPS16 not implemented. */ } } @@ -1591,7 +1595,7 @@ void helper_mtc0_lladdr(CPUMIPSState *env, target_ulong arg1) { target_long mask = env->CP0_LLAddr_rw_bitmask; arg1 = arg1 << env->CP0_LLAddr_shift; - env->lladdr = (env->lladdr & ~mask) | (arg1 & mask); + env->CP0_LLAddr = (env->CP0_LLAddr & ~mask) | (arg1 & mask); } #define MTC0_MAAR_MASK(env) \ @@ -2277,6 +2281,7 @@ static inline void exception_return(CPUMIPSState *env) void helper_eret(CPUMIPSState *env) { exception_return(env); + env->CP0_LLAddr = 1; env->lladdr = 1; } diff --git a/target-mips/translate.c b/target-mips/translate.c index d829738..5d0732f 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -4826,7 +4826,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel) case 17: switch (sel) { case 0: - gen_mfhc0_load64(arg, offsetof(CPUMIPSState, lladdr), + gen_mfhc0_load64(arg, offsetof(CPUMIPSState, CP0_LLAddr), ctx->CP0_LLAddr_shift); rn = "LLAddr"; break; @@ -20114,7 +20114,7 @@ void mips_cpu_dump_state(CPUState *cs, FILE *f, fprintf_function cpu_fprintf, env->CP0_Status, env->CP0_Cause, env->CP0_EPC); cpu_fprintf(f, " Config0 0x%08x Config1 0x%08x LLAddr 0x%016" PRIx64 "\n", - env->CP0_Config0, env->CP0_Config1, env->lladdr); + env->CP0_Config0, env->CP0_Config1, env->CP0_LLAddr); cpu_fprintf(f, " Config2 0x%08x Config3 0x%08x\n", env->CP0_Config2, env->CP0_Config3); cpu_fprintf(f, " Config4 0x%08x Config5 0x%08x\n",