| Message ID | 20150617124205.3316.81361.stgit@PASHA-ISP |
|---|---|
| State | New |
| Headers | show |
On 17/06/2015 14:42, 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.dovgaluk@ispras.ru> > --- > include/exec/cpu_ldst_template.h | 42 +++++++++++++++++++++++++++++++------- > include/exec/exec-all.h | 27 ++++++++++++++++++++++++ > softmmu_template.h | 18 ++++++++++++++++ > 3 files changed, 79 insertions(+), 8 deletions(-) > > diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h > index 95ab750..1847816 100644 > --- a/include/exec/cpu_ldst_template.h > +++ b/include/exec/cpu_ldst_template.h > @@ -62,7 +62,9 @@ > /* 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) Would it make sense to call these helper_cpu_ld##USUFFIX##MEMSUFFIX? > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index 856e698..b3aefde 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -350,6 +350,33 @@ struct MemoryRegion *iotlb_to_region(CPUState *cpu, > void tlb_fill(CPUState *cpu, target_ulong addr, int is_write, int mmu_idx, > uintptr_t retaddr); > > +uint8_t helper_call_ldb_cmmu(CPUArchState *env, target_ulong addr, > + int mmu_idx, uintptr_t retaddr); Here we already have helper_ret_ldb_cmmu, so the new function is only needed if DATA_SIZE != 1. > +uint16_t helper_call_ldw_cmmu(CPUArchState *env, target_ulong addr, > + int mmu_idx, uintptr_t retaddr); What about helper_ret_ldw_cmmu for consistency with the DATA_SIZE == 1 case? Paolo
> From: Paolo Bonzini [mailto:pbonzini@redhat.com] > On 17/06/2015 14:42, 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.dovgaluk@ispras.ru> > > --- > > include/exec/cpu_ldst_template.h | 42 +++++++++++++++++++++++++++++++------- > > include/exec/exec-all.h | 27 ++++++++++++++++++++++++ > > softmmu_template.h | 18 ++++++++++++++++ > > 3 files changed, 79 insertions(+), 8 deletions(-) > > > > diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h > > index 95ab750..1847816 100644 > > --- a/include/exec/cpu_ldst_template.h > > +++ b/include/exec/cpu_ldst_template.h > > @@ -62,7 +62,9 @@ > > /* 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) > > Would it make sense to call these helper_cpu_ld##USUFFIX##MEMSUFFIX? I don't want to use 'helper' prefix, because helper functions are usually called directly from TB. Pavel Dovgalyuk
On 18/06/2015 07:17, Pavel Dovgaluk wrote: >>> > > >>> > > 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) >> > >> > Would it make sense to call these helper_cpu_ld##USUFFIX##MEMSUFFIX? > I don't want to use 'helper' prefix, because helper functions are > usually called directly from TB. True, but in the end these have the same functionality as helpers, just they're indirectly called from other helpers. Paolo
On 2015-06-18 10:16, Paolo Bonzini wrote: > > > On 18/06/2015 07:17, Pavel Dovgaluk wrote: > >>> > > > >>> > > 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) > >> > > >> > Would it make sense to call these helper_cpu_ld##USUFFIX##MEMSUFFIX? > > I don't want to use 'helper' prefix, because helper functions are > > usually called directly from TB. > > True, but in the end these have the same functionality as helpers, just > they're indirectly called from other helpers. Not fully. The idea is that the helpers are non-inline functions handling the slow path. The cpu_ld##USUFFIX##MEMSUFFIX are inline functions handling the fast path, and calling the helpers for the slow path. That allows for example GCC to optimize the fast path when the cpu_ld functions are used in a loop.
> From: Paolo Bonzini [mailto:pbonzini@redhat.com] > On 17/06/2015 14:42, 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.dovgaluk@ispras.ru> > > --- > > include/exec/cpu_ldst_template.h | 42 +++++++++++++++++++++++++++++++------- > > include/exec/exec-all.h | 27 ++++++++++++++++++++++++ > > softmmu_template.h | 18 ++++++++++++++++ > > 3 files changed, 79 insertions(+), 8 deletions(-) > > > > diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h > > index 95ab750..1847816 100644 > > --- a/include/exec/cpu_ldst_template.h > > +++ b/include/exec/cpu_ldst_template.h > > @@ -62,7 +62,9 @@ > > /* 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) > > Would it make sense to call these helper_cpu_ld##USUFFIX##MEMSUFFIX? > > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > > index 856e698..b3aefde 100644 > > --- a/include/exec/exec-all.h > > +++ b/include/exec/exec-all.h > > @@ -350,6 +350,33 @@ struct MemoryRegion *iotlb_to_region(CPUState *cpu, > > void tlb_fill(CPUState *cpu, target_ulong addr, int is_write, int mmu_idx, > > uintptr_t retaddr); > > > > +uint8_t helper_call_ldb_cmmu(CPUArchState *env, target_ulong addr, > > + int mmu_idx, uintptr_t retaddr); > > Here we already have helper_ret_ldb_cmmu, so the new function is only > needed if DATA_SIZE != 1. > > > +uint16_t helper_call_ldw_cmmu(CPUArchState *env, target_ulong addr, > > + int mmu_idx, uintptr_t retaddr); > > What about helper_ret_ldw_cmmu for consistency with the DATA_SIZE == 1 case? tcg.h breaks these definitions: /* 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_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 #else Pavel Dovgalyuk
On 18/06/2015 11:24, Pavel Dovgaluk wrote: >>> > > +uint16_t helper_call_ldw_cmmu(CPUArchState *env, target_ulong addr, >>> > > + int mmu_idx, uintptr_t retaddr); >> > >> > What about helper_ret_ldw_cmmu for consistency with the DATA_SIZE == 1 case? > tcg.h breaks these definitions: > > /* 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_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 > #else Isn't this exactly the same as your helper_call_ldw_cmmu? Paolo
> From: Paolo Bonzini [mailto:pbonzini@redhat.com] > On 18/06/2015 11:24, Pavel Dovgaluk wrote: > >>> > > +uint16_t helper_call_ldw_cmmu(CPUArchState *env, target_ulong addr, > >>> > > + int mmu_idx, uintptr_t retaddr); > >> > > >> > What about helper_ret_ldw_cmmu for consistency with the DATA_SIZE == 1 case? > > tcg.h breaks these definitions: > > > > /* 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_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 > > #else > > Isn't this exactly the same as your helper_call_ldw_cmmu? Yes, but I can't compile it yet. Pavel Dovgalyuk
On 18/06/2015 11:33, Pavel Dovgaluk wrote: > > > /* 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_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 > > > #else > > > > Isn't this exactly the same as your helper_call_ldw_cmmu? > > Yes, but I can't compile it yet. I'm not sure what's the problem. Can you just move this part of tcg/tcg.h to another header file? Paolo
diff --git a/include/exec/cpu_ldst_template.h b/include/exec/cpu_ldst_template.h index 95ab750..1847816 100644 --- a/include/exec/cpu_ldst_template.h +++ b/include/exec/cpu_ldst_template.h @@ -62,7 +62,9 @@ /* 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 +76,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_call_ld, SUFFIX), MMUSUFFIX)(env, addr, + mmu_idx, retaddr); } else { uintptr_t hostaddr = addr + env->tlb_table[mmu_idx][page_index].addend; res = glue(glue(ld, USUFFIX), _p)((uint8_t *)hostaddr); @@ -82,9 +85,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 +106,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_call_ld, SUFFIX), + 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 +127,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 +140,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_call_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 diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index 856e698..b3aefde 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -350,6 +350,33 @@ struct MemoryRegion *iotlb_to_region(CPUState *cpu, void tlb_fill(CPUState *cpu, target_ulong addr, int is_write, int mmu_idx, uintptr_t retaddr); +uint8_t helper_call_ldb_cmmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); +uint16_t helper_call_ldw_cmmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); +uint32_t helper_call_ldl_cmmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); +uint64_t helper_call_ldq_cmmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); + +uint8_t helper_call_ldb_mmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); +uint16_t helper_call_ldw_mmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); +uint32_t helper_call_ldl_mmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); +uint64_t helper_call_ldq_mmu(CPUArchState *env, target_ulong addr, + int mmu_idx, uintptr_t retaddr); + +void helper_call_stb_mmu(CPUArchState *env, target_ulong addr, + uint8_t val, int mmu_idx, uintptr_t retaddr); +void helper_call_stw_mmu(CPUArchState *env, target_ulong addr, + uint16_t val, int mmu_idx, uintptr_t retaddr); +void helper_call_stl_mmu(CPUArchState *env, target_ulong addr, + uint32_t val, int mmu_idx, uintptr_t retaddr); +void helper_call_stq_mmu(CPUArchState *env, target_ulong addr, + uint64_t val, int mmu_idx, uintptr_t retaddr); + #endif #if defined(CONFIG_USER_ONLY) diff --git a/softmmu_template.h b/softmmu_template.h index 39f571b..7d267b4 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -343,6 +343,15 @@ glue(glue(helper_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr, return helper_te_ld_name (env, addr, oi, GETRA()); } +DATA_TYPE +glue(glue(helper_call_ld, SUFFIX), MMUSUFFIX)(CPUArchState *env, + target_ulong addr, + int mmu_idx, + uintptr_t retaddr) +{ + return helper_te_ld_name(env, addr, mmu_idx, retaddr); +} + #ifndef SOFTMMU_CODE_ACCESS /* Provide signed versions of the load routines as well. We can of course @@ -548,6 +557,15 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr, helper_te_st_name(env, addr, val, oi, GETRA()); } +void +glue(glue(helper_call_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, + target_ulong addr, + DATA_TYPE val, int mmu_idx, + uintptr_t retaddr) +{ + helper_te_st_name(env, addr, val, mmu_idx, retaddr); +} + #endif /* !defined(SOFTMMU_CODE_ACCESS) */ #undef READ_ACCESS_TYPE
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.dovgaluk@ispras.ru> --- include/exec/cpu_ldst_template.h | 42 +++++++++++++++++++++++++++++++------- include/exec/exec-all.h | 27 ++++++++++++++++++++++++ softmmu_template.h | 18 ++++++++++++++++ 3 files changed, 79 insertions(+), 8 deletions(-)