On 2015-06-25 14:28, Pavel Dovgaluk wrote: > > From: Aurelien Jarno [mailto:aurel...@aurel32.net] > > On 2015-06-18 16:28, Pavel Dovgalyuk wrote: > > > This patch introduces several helpers to pass return address > > > which points to the TB. Correct return address allows correct > > > restoring of the guest PC and icount. These functions should be used when > > > helpers embedded into TB invoke memory operations. > > > > > > Signed-off-by: Pavel Dovgalyuk <pavel.dovga...@ispras.ru> > > > --- > > > include/exec/cpu_ldst_template.h | 48 > > > ++++++++++++++++++++++++++++++++------ > > > softmmu_template.h | 6 ----- > > > tcg/tcg.h | 23 ++++++++++++++++++ > > > 3 files changed, 63 insertions(+), 14 deletions(-) > > > > > > diff --git a/include/exec/cpu_ldst_template.h > > > b/include/exec/cpu_ldst_template.h > > > index 95ab750..35df753 100644 > > > --- a/include/exec/cpu_ldst_template.h > > > +++ b/include/exec/cpu_ldst_template.h > > > @@ -54,15 +54,21 @@ > > > #ifdef SOFTMMU_CODE_ACCESS > > > #define ADDR_READ addr_code > > > #define MMUSUFFIX _cmmu > > > +#define URETSUFFIX SUFFIX > > > +#define SRETSUFFIX SUFFIX > > > #else > > > #define ADDR_READ addr_read > > > #define MMUSUFFIX _mmu > > > +#define URETSUFFIX USUFFIX > > > +#define SRETSUFFIX glue(s, SUFFIX) > > > #endif > > > > > > /* generic load/store macros */ > > > > > > static inline RES_TYPE > > > -glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong > > > ptr) > > > +glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(CPUArchState *env, > > > + target_ulong ptr, > > > + uintptr_t retaddr) > > > { > > > int page_index; > > > RES_TYPE res; > > > @@ -74,7 +80,8 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState > > > *env, target_ulong > > ptr) > > > mmu_idx = CPU_MMU_INDEX; > > > if (unlikely(env->tlb_table[mmu_idx][page_index].ADDR_READ != > > > (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) { > > > - res = glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(env, addr, > > > mmu_idx); > > > + res = glue(glue(helper_ret_ld, URETSUFFIX), MMUSUFFIX)(env, addr, > > > + mmu_idx, > > > retaddr); > > > > helper_ret_ld doesn't take a int for the mmu_idx, but rather a > > TCGMemOpIdx (we should add more type checking). They differ by the fact > > that TCGMemOpIdx also encodes the size of the operation (plus the > > alignement, but that's out of scope here). This currently doesn't make a > > difference, but it's better to pass the correct value in case we change > > the code later. > > Ok. > > > > } else { > > > uintptr_t hostaddr = addr + > > > env->tlb_table[mmu_idx][page_index].addend; > > > res = glue(glue(ld, USUFFIX), _p)((uint8_t *)hostaddr); > > > @@ -82,9 +89,17 @@ glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState > > > *env, target_ulong > > ptr) > > > return res; > > > } > > > > > > +static inline RES_TYPE > > > +glue(glue(cpu_ld, USUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong > > > ptr) > > > +{ > > > + return glue(glue(glue(cpu_ld, USUFFIX), MEMSUFFIX), _ra)(env, ptr, > > > 0); > > > +} > > > + > > > #if DATA_SIZE <= 2 > > > static inline int > > > -glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong > > > ptr) > > > +glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env, > > > + target_ulong ptr, > > > + uintptr_t retaddr) > > > { > > > int res, page_index; > > > target_ulong addr; > > > @@ -95,14 +110,20 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState > > > *env, target_ulong > > ptr) > > > mmu_idx = CPU_MMU_INDEX; > > > if (unlikely(env->tlb_table[mmu_idx][page_index].ADDR_READ != > > > (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) { > > > - res = (DATA_STYPE)glue(glue(helper_ld, SUFFIX), > > > - MMUSUFFIX)(env, addr, mmu_idx); > > > + res = (DATA_STYPE)glue(glue(helper_ret_ld, SRETSUFFIX), > > > + MMUSUFFIX)(env, addr, mmu_idx, retaddr); > > > } else { > > > uintptr_t hostaddr = addr + > > > env->tlb_table[mmu_idx][page_index].addend; > > > res = glue(glue(lds, SUFFIX), _p)((uint8_t *)hostaddr); > > > } > > > return res; > > > } > > > + > > > +static inline int > > > +glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong > > > ptr) > > > +{ > > > + return glue(glue(glue(cpu_lds, SUFFIX), MEMSUFFIX), _ra)(env, ptr, > > > 0); > > > +} > > > #endif > > > > > > #ifndef SOFTMMU_CODE_ACCESS > > > @@ -110,8 +131,9 @@ glue(glue(cpu_lds, SUFFIX), MEMSUFFIX)(CPUArchState > > > *env, target_ulong > > ptr) > > > /* generic store macro */ > > > > > > static inline void > > > -glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong > > > ptr, > > > - RES_TYPE v) > > > +glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(CPUArchState *env, > > > + target_ulong ptr, > > > + RES_TYPE v, uintptr_t > > > retaddr) > > > { > > > int page_index; > > > target_ulong addr; > > > @@ -122,13 +144,21 @@ glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState > > > *env, target_ulong > > ptr, > > > mmu_idx = CPU_MMU_INDEX; > > > if (unlikely(env->tlb_table[mmu_idx][page_index].addr_write != > > > (addr & (TARGET_PAGE_MASK | (DATA_SIZE - 1))))) { > > > - glue(glue(helper_st, SUFFIX), MMUSUFFIX)(env, addr, v, mmu_idx); > > > + glue(glue(helper_ret_st, SUFFIX), MMUSUFFIX)(env, addr, v, > > > mmu_idx, > > > + retaddr); > > > } else { > > > uintptr_t hostaddr = addr + > > > env->tlb_table[mmu_idx][page_index].addend; > > > glue(glue(st, SUFFIX), _p)((uint8_t *)hostaddr, v); > > > } > > > } > > > > > > +static inline void > > > +glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState *env, target_ulong > > > ptr, > > > + RES_TYPE v) > > > +{ > > > + glue(glue(glue(cpu_st, SUFFIX), MEMSUFFIX), _ra)(env, ptr, v, 0); > > > +} > > > + > > > #endif /* !SOFTMMU_CODE_ACCESS */ > > > > > > #undef RES_TYPE > > > @@ -139,3 +169,5 @@ glue(glue(cpu_st, SUFFIX), MEMSUFFIX)(CPUArchState > > > *env, target_ulong > > ptr, > > > #undef DATA_SIZE > > > #undef MMUSUFFIX > > > #undef ADDR_READ > > > +#undef URETSUFFIX > > > +#undef SRETSUFFIX > > > diff --git a/softmmu_template.h b/softmmu_template.h > > > index 39f571b..1d59e55 100644 > > > --- a/softmmu_template.h > > > +++ b/softmmu_template.h > > > @@ -165,9 +165,6 @@ static inline DATA_TYPE glue(io_read, > > > SUFFIX)(CPUArchState *env, > > > } > > > #endif > > > > > > -#ifdef SOFTMMU_CODE_ACCESS > > > -static __attribute__((unused)) > > > -#endif > > > WORD_TYPE helper_le_ld_name(CPUArchState *env, target_ulong addr, > > > TCGMemOpIdx oi, uintptr_t retaddr) > > > { > > > @@ -252,9 +249,6 @@ WORD_TYPE helper_le_ld_name(CPUArchState *env, > > > target_ulong addr, > > > } > > > > > > #if DATA_SIZE > 1 > > > -#ifdef SOFTMMU_CODE_ACCESS > > > -static __attribute__((unused)) > > > -#endif > > > WORD_TYPE helper_be_ld_name(CPUArchState *env, target_ulong addr, > > > TCGMemOpIdx oi, uintptr_t retaddr) > > > { > > > diff --git a/tcg/tcg.h b/tcg/tcg.h > > > index 8098f82..31bd419 100644 > > > --- a/tcg/tcg.h > > > +++ b/tcg/tcg.h > > > @@ -981,25 +981,48 @@ void helper_be_stl_mmu(CPUArchState *env, > > > target_ulong addr, uint32_t > > val, > > > void helper_be_stq_mmu(CPUArchState *env, target_ulong addr, uint64_t > > > val, > > > TCGMemOpIdx oi, uintptr_t retaddr); > > > > > > +uint8_t helper_ret_ldb_cmmu(CPUArchState *env, target_ulong addr, > > > + TCGMemOpIdx oi, uintptr_t retaddr); > > > +uint16_t helper_le_ldw_cmmu(CPUArchState *env, target_ulong addr, > > > + TCGMemOpIdx oi, uintptr_t retaddr); > > > +uint32_t helper_le_ldl_cmmu(CPUArchState *env, target_ulong addr, > > > + TCGMemOpIdx oi, uintptr_t retaddr); > > > +uint64_t helper_le_ldq_cmmu(CPUArchState *env, target_ulong addr, > > > + TCGMemOpIdx oi, uintptr_t retaddr); > > > +uint16_t helper_be_ldw_cmmu(CPUArchState *env, target_ulong addr, > > > + TCGMemOpIdx oi, uintptr_t retaddr); > > > +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); > > > + > > > /* Temporary aliases until backends are converted. */ > > > #ifdef TARGET_WORDS_BIGENDIAN > > > # define helper_ret_ldsw_mmu helper_be_ldsw_mmu > > > # define helper_ret_lduw_mmu helper_be_lduw_mmu > > > # define helper_ret_ldsl_mmu helper_be_ldsl_mmu > > > # define helper_ret_ldul_mmu helper_be_ldul_mmu > > > +# define helper_ret_ldl_mmu helper_be_ldul_mmu > > > # define helper_ret_ldq_mmu helper_be_ldq_mmu > > > # define helper_ret_stw_mmu helper_be_stw_mmu > > > # define helper_ret_stl_mmu helper_be_stl_mmu > > > # define helper_ret_stq_mmu helper_be_stq_mmu > > > +# define helper_ret_ldw_cmmu helper_be_ldw_cmmu > > > +# define helper_ret_ldl_cmmu helper_be_ldl_cmmu > > > +# define helper_ret_ldq_cmmu helper_be_ldq_cmmu > > > > As said in the comment above, these are temporary aliases that will > > disappear at some point. It's probably better to not rely on them and > > call helper_te_ld_name/helper_te_st_name directly. > > These functions cannot be called directly. > helper_te_ld_name/helper_te_st_name are defined in softmmu_template.h and not > declared anywhere. > cpu_ldst.h can be included without softmmu_template.h or before it and does > not see these #defines.
Ok, I found it's a pitty that we add more temporary code, but OTOH I understand it's not your job to fix that. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
