Skip to content

Commit

Permalink
target/mips: compare virtual addresses in LL/SC sequence
Browse files Browse the repository at this point in the history
Do only virtual addresses comaprisons in LL/SC sequence emulations.

Until this patch, physical addresses had been compared in SC part of
LL/SC sequence, even though such comparisons could be avoided. Getting
rid of them allows throwing away SC helpers and having common SC
implementations in user and system mode, avoiding the need for two
separate implementations selected by #ifdef CONFIG_USER_ONLY.

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, CP0_LLAddr field is introduced which
is the actual Coperocessor 0 LLAddr register that guest can access.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
Signed-off-by: Miodrag Dinic <miodrag.dinic@imgtec.com>
Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
Acked-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
  • Loading branch information
Leon Alrae authored and AMarkovic committed Feb 14, 2019
1 parent 7e40746 commit c7c7e1e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 18 deletions.
3 changes: 2 additions & 1 deletion target/mips/cpu.h
Expand Up @@ -867,13 +867,14 @@ 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? */
/*
* CP0 Register 17
*/
uint64_t lladdr;
target_ulong lladdr; /* LL virtual address compared against SC */
target_ulong llval;
target_ulong llnewval;
uint64_t llval_wp;
Expand Down
7 changes: 4 additions & 3 deletions target/mips/machine.c
Expand Up @@ -214,8 +214,8 @@ const VMStateDescription vmstate_tlb = {

const VMStateDescription vmstate_mips_cpu = {
.name = "cpu",
.version_id = 17,
.minimum_version_id = 17,
.version_id = 18,
.minimum_version_id = 18,
.post_load = cpu_post_load,
.fields = (VMStateField[]) {
/* Active TC */
Expand Down Expand Up @@ -293,9 +293,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),
Expand Down
29 changes: 17 additions & 12 deletions target/mips/op_helper.c
Expand Up @@ -349,15 +349,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;
}
}

Expand All @@ -370,7 +370,8 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg, int mem_idx) \
} \
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; \
}
Expand All @@ -392,7 +393,7 @@ target_ulong helper_##name(CPUMIPSState *env, target_ulong arg1, \
} \
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()); \
Expand Down Expand Up @@ -987,7 +988,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)
Expand Down Expand Up @@ -1063,7 +1064,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)
Expand Down Expand Up @@ -1299,7 +1300,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. */
}

Expand All @@ -1311,12 +1313,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. */
}
}
Expand Down Expand Up @@ -1868,7 +1872,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) \
Expand Down Expand Up @@ -2566,6 +2570,7 @@ static inline void exception_return(CPUMIPSState *env)
void helper_eret(CPUMIPSState *env)
{
exception_return(env);
env->CP0_LLAddr = 1;
env->lladdr = 1;
}

Expand Down
4 changes: 2 additions & 2 deletions target/mips/translate.c
Expand Up @@ -6612,7 +6612,7 @@ static void gen_mfhc0(DisasContext *ctx, TCGv arg, int reg, int sel)
case CP0_REGISTER_17:
switch (sel) {
case 0:
gen_mfhc0_load64(arg, offsetof(CPUMIPSState, lladdr),
gen_mfhc0_load64(arg, offsetof(CPUMIPSState, CP0_LLAddr),
ctx->CP0_LLAddr_shift);
register_name = "LLAddr";
break;
Expand Down Expand Up @@ -29795,7 +29795,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",
Expand Down

0 comments on commit c7c7e1e

Please sign in to comment.