* [Qemu-devel] [PATCH v6 0/3] target-mips: Add support for misaligned accesses
@ 2015-05-27 13:28 Yongbok Kim
2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 Yongbok Kim
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Yongbok Kim @ 2015-05-27 13:28 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, leon.alrae, afaerber, rth
This patch set adds support for misaligned memory accesses in MIPS architecture
Release 6 and MIPS SIMD Architecture.
The behaviour, semantics, and architecture specifications of misaligned memory
accesses are described in:
MIPS Architecture For Programmers Volume I-A: Introduction to the MIPS64
Architecture, Appendix B Misaligned Memory Accesses.
Available at http://www.imgtec.com/mips/architectures/mips64.asp
Regards,
Yongbok
v6:
* Rephrased comments (Peter)
v5:
* Rewrote R6 patch to use new MO_UNALIN (Richard)
* Further cleanup to pass caculated address for MSA LD/ ST (Richard)
v4:
* Removed the work-around per the recent TCG change for misaligned accesses
* Added probe_write() (Richard)
* Used helper_ret_*_mmu directly (Richard)
* Removed TLB checking for MSA LD (Richard)
* Removed unnecessary save_cpu_state() calls
v3:
* Rewrote MSA patch
* Work-around is using byte-to-byte accesses and endianness corrections for
R5+MSA. (This replaces the misaligned flag from v2.) (Leon)
* Bug fixes (Leon)
* Separate helper functions for each data formats
v2:
* Removed re-translation in the mips_cpu_do_unaligned_access() (Peter)
* Checks validity only if an access is spanning into two pages in MSA (Leon)
* Introduced misaligned flag to indicate MSA ld/st is ongoing, is used to
allow misaligned accesses in the mips_cpu_do_unaligned_access() callback.
This is crucial to support MSA misaligned accesses in Release 5 cores.
Yongbok Kim (3):
target-mips: Misaligned memory accesses for R6
softmmu: Add probe_write()
target-mips: Misaligned memory accesses for MSA
include/exec/exec-all.h | 2 +
softmmu_template.h | 18 ++++++
target-mips/helper.h | 10 +++-
target-mips/op_helper.c | 135 +++++++++++++++++++++++-------------------
target-mips/translate.c | 72 +++++++++++++++--------
target-mips/translate_init.c | 2 +-
6 files changed, 150 insertions(+), 89 deletions(-)
--
1.7.5.4
^ permalink raw reply [flat|nested] 9+ messages in thread* [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 2015-05-27 13:28 [Qemu-devel] [PATCH v6 0/3] target-mips: Add support for misaligned accesses Yongbok Kim @ 2015-05-27 13:29 ` Yongbok Kim 2015-05-29 12:33 ` Leon Alrae 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 2/3] softmmu: Add probe_write() Yongbok Kim 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA Yongbok Kim 2 siblings, 1 reply; 9+ messages in thread From: Yongbok Kim @ 2015-05-27 13:29 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, leon.alrae, afaerber, rth Release 6 requires misaligned memory access support for all ordinary memory access instructions (for example, LW/SW, LWC1/SWC1). However misaligned support is not provided for certain special memory accesses such as atomics (for example, LL/SC). Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> --- target-mips/translate.c | 45 ++++++++++++++++++++++++++++------------- target-mips/translate_init.c | 2 +- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/target-mips/translate.c b/target-mips/translate.c index fd063a2..a5ea04e 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -1414,6 +1414,7 @@ typedef struct DisasContext { int32_t CP0_Config1; /* Routine used to access memory */ int mem_idx; + TCGMemOp default_tcg_memop_mask; uint32_t hflags, saved_hflags; int bstate; target_ulong btarget; @@ -2081,12 +2082,14 @@ static void gen_ld(DisasContext *ctx, uint32_t opc, switch (opc) { #if defined(TARGET_MIPS64) case OPC_LWU: - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL); + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUL | + ctx->default_tcg_memop_mask); gen_store_gpr(t0, rt); opn = "lwu"; break; case OPC_LD: - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ); + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ | + ctx->default_tcg_memop_mask); gen_store_gpr(t0, rt); opn = "ld"; break; @@ -2143,7 +2146,8 @@ static void gen_ld(DisasContext *ctx, uint32_t opc, t1 = tcg_const_tl(pc_relative_pc(ctx)); gen_op_addr_add(ctx, t0, t0, t1); tcg_temp_free(t1); - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ); + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ | + ctx->default_tcg_memop_mask); gen_store_gpr(t0, rt); opn = "ldpc"; break; @@ -2152,22 +2156,26 @@ static void gen_ld(DisasContext *ctx, uint32_t opc, t1 = tcg_const_tl(pc_relative_pc(ctx)); gen_op_addr_add(ctx, t0, t0, t1); tcg_temp_free(t1); - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL); + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL | + ctx->default_tcg_memop_mask); gen_store_gpr(t0, rt); opn = "lwpc"; break; case OPC_LW: - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL); + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL | + ctx->default_tcg_memop_mask); gen_store_gpr(t0, rt); opn = "lw"; break; case OPC_LH: - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESW); + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESW | + ctx->default_tcg_memop_mask); gen_store_gpr(t0, rt); opn = "lh"; break; case OPC_LHU: - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUW); + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEUW | + ctx->default_tcg_memop_mask); gen_store_gpr(t0, rt); opn = "lhu"; break; @@ -2251,7 +2259,8 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt, switch (opc) { #if defined(TARGET_MIPS64) case OPC_SD: - tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEQ); + tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEQ | + ctx->default_tcg_memop_mask); opn = "sd"; break; case OPC_SDL: @@ -2266,11 +2275,13 @@ static void gen_st (DisasContext *ctx, uint32_t opc, int rt, break; #endif case OPC_SW: - tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUL); + tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUL | + ctx->default_tcg_memop_mask); opn = "sw"; break; case OPC_SH: - tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUW); + tcg_gen_qemu_st_tl(t1, t0, ctx->mem_idx, MO_TEUW | + ctx->default_tcg_memop_mask); opn = "sh"; break; case OPC_SB: @@ -2347,7 +2358,8 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t opc, int ft, case OPC_LWC1: { TCGv_i32 fp0 = tcg_temp_new_i32(); - tcg_gen_qemu_ld_i32(fp0, t0, ctx->mem_idx, MO_TESL); + tcg_gen_qemu_ld_i32(fp0, t0, ctx->mem_idx, MO_TESL | + ctx->default_tcg_memop_mask); gen_store_fpr32(fp0, ft); tcg_temp_free_i32(fp0); } @@ -2357,7 +2369,8 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t opc, int ft, { TCGv_i32 fp0 = tcg_temp_new_i32(); gen_load_fpr32(fp0, ft); - tcg_gen_qemu_st_i32(fp0, t0, ctx->mem_idx, MO_TEUL); + tcg_gen_qemu_st_i32(fp0, t0, ctx->mem_idx, MO_TEUL | + ctx->default_tcg_memop_mask); tcg_temp_free_i32(fp0); } opn = "swc1"; @@ -2365,7 +2378,8 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t opc, int ft, case OPC_LDC1: { TCGv_i64 fp0 = tcg_temp_new_i64(); - tcg_gen_qemu_ld_i64(fp0, t0, ctx->mem_idx, MO_TEQ); + tcg_gen_qemu_ld_i64(fp0, t0, ctx->mem_idx, MO_TEQ | + ctx->default_tcg_memop_mask); gen_store_fpr64(ctx, fp0, ft); tcg_temp_free_i64(fp0); } @@ -2375,7 +2389,8 @@ static void gen_flt_ldst (DisasContext *ctx, uint32_t opc, int ft, { TCGv_i64 fp0 = tcg_temp_new_i64(); gen_load_fpr64(ctx, fp0, ft); - tcg_gen_qemu_st_i64(fp0, t0, ctx->mem_idx, MO_TEQ); + tcg_gen_qemu_st_i64(fp0, t0, ctx->mem_idx, MO_TEQ | + ctx->default_tcg_memop_mask); tcg_temp_free_i64(fp0); } opn = "sdc1"; @@ -19143,6 +19158,8 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb, #else ctx.mem_idx = ctx.hflags & MIPS_HFLAG_KSU; #endif + ctx.default_tcg_memop_mask = (ctx.insn_flags & ISA_MIPS32R6) ? + MO_UNALN : MO_ALIGN; num_insns = 0; max_insns = tb->cflags & CF_COUNT_MASK; if (max_insns == 0) diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index 85a65e7..ec54fef 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -607,7 +607,7 @@ static const mips_def_t mips_defs[] = }, { /* A generic CPU supporting MIPS64 Release 6 ISA. - FIXME: Support IEEE 754-2008 FP and misaligned memory accesses. + FIXME: Support IEEE 754-2008 FP. Eventually this should be replaced by a real CPU model. */ .name = "MIPS64R6-generic", .CP0_PRid = 0x00010000, -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 Yongbok Kim @ 2015-05-29 12:33 ` Leon Alrae 2015-06-01 8:29 ` Yongbok Kim 0 siblings, 1 reply; 9+ messages in thread From: Leon Alrae @ 2015-05-29 12:33 UTC (permalink / raw) To: Yongbok Kim, qemu-devel; +Cc: peter.maydell, afaerber, rth On 27/05/2015 14:29, Yongbok Kim wrote: > @@ -2143,7 +2146,8 @@ static void gen_ld(DisasContext *ctx, uint32_t opc, > t1 = tcg_const_tl(pc_relative_pc(ctx)); > gen_op_addr_add(ctx, t0, t0, t1); > tcg_temp_free(t1); > - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ); > + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ | > + ctx->default_tcg_memop_mask); > gen_store_gpr(t0, rt); > opn = "ldpc"; > break; > @@ -2152,22 +2156,26 @@ static void gen_ld(DisasContext *ctx, uint32_t opc, > t1 = tcg_const_tl(pc_relative_pc(ctx)); > gen_op_addr_add(ctx, t0, t0, t1); > tcg_temp_free(t1); > - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL); > + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL | > + ctx->default_tcg_memop_mask); > gen_store_gpr(t0, rt); > opn = "lwpc"; > break; As I can see in other places you skipped load/store instructions not present in R6 spec (like pre-R6 DSP or microMIPS loads/stores), which probably is fine. However, IIUC these two instructions LWPC and LDPC are from mips16 ASE, so probably you want to skip them as well? (note that for R6 we’ve got R6_OPC_LWPC and R6_OPC_LDPC and they are naturally aligned). Apart from that, Reviewed-by: Leon Alrae <leon.alrae@imgtec.com> BTW these OPC_LWPC and OPC_LDPC are very confusing... I presume these fake instructions were created in order to reuse gen_ldst() function for mips16 M16_OPC_LWPC and I64_LDPC instructions. It would be nice to clean this up at some point (the same as OPC_JALRC). Leon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 2015-05-29 12:33 ` Leon Alrae @ 2015-06-01 8:29 ` Yongbok Kim 0 siblings, 0 replies; 9+ messages in thread From: Yongbok Kim @ 2015-06-01 8:29 UTC (permalink / raw) To: Leon Alrae, qemu-devel; +Cc: peter.maydell, afaerber, rth On 29/05/2015 13:33, Leon Alrae wrote: > On 27/05/2015 14:29, Yongbok Kim wrote: >> @@ -2143,7 +2146,8 @@ static void gen_ld(DisasContext *ctx, uint32_t opc, >> t1 = tcg_const_tl(pc_relative_pc(ctx)); >> gen_op_addr_add(ctx, t0, t0, t1); >> tcg_temp_free(t1); >> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ); >> + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TEQ | >> + ctx->default_tcg_memop_mask); >> gen_store_gpr(t0, rt); >> opn = "ldpc"; >> break; >> @@ -2152,22 +2156,26 @@ static void gen_ld(DisasContext *ctx, uint32_t opc, >> t1 = tcg_const_tl(pc_relative_pc(ctx)); >> gen_op_addr_add(ctx, t0, t0, t1); >> tcg_temp_free(t1); >> - tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL); >> + tcg_gen_qemu_ld_tl(t0, t0, ctx->mem_idx, MO_TESL | >> + ctx->default_tcg_memop_mask); >> gen_store_gpr(t0, rt); >> opn = "lwpc"; >> break; > > As I can see in other places you skipped load/store instructions not > present in R6 spec (like pre-R6 DSP or microMIPS loads/stores), which > probably is fine. However, IIUC these two instructions LWPC and LDPC are > from mips16 ASE, so probably you want to skip them as well? (note that > for R6 we’ve got R6_OPC_LWPC and R6_OPC_LDPC and they are naturally > aligned). Nice finding! > > Apart from that, > > Reviewed-by: Leon Alrae <leon.alrae@imgtec.com> > > BTW these OPC_LWPC and OPC_LDPC are very confusing... I presume these > fake instructions were created in order to reuse gen_ldst() function for > mips16 M16_OPC_LWPC and I64_LDPC instructions. It would be nice to clean > this up at some point (the same as OPC_JALRC). > > Leon > Yes, indeed. I have removed those fictional branch instructions similar to these before, still we have more to remove. Regards, Yongbok ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v6 2/3] softmmu: Add probe_write() 2015-05-27 13:28 [Qemu-devel] [PATCH v6 0/3] target-mips: Add support for misaligned accesses Yongbok Kim 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 Yongbok Kim @ 2015-05-27 13:29 ` Yongbok Kim 2015-05-27 13:42 ` Peter Maydell 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA Yongbok Kim 2 siblings, 1 reply; 9+ messages in thread From: Yongbok Kim @ 2015-05-27 13:29 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, leon.alrae, afaerber, rth Add probe_write() in order to support such functionality that probes if a specified guest virtual address exists in TLB and is writable. The function forces a tlb_fill() if the address does not exist or is not writable, as a result an exception can occur. Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> --- include/exec/exec-all.h | 2 ++ softmmu_template.h | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 0 deletions(-) diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index b58cd47..af51203 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -109,6 +109,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, hwaddr paddr, MemTxAttrs attrs, int prot, int mmu_idx, target_ulong size); void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr); +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, + uintptr_t retaddr); #else static inline void tlb_flush_page(CPUState *cpu, target_ulong addr) { diff --git a/softmmu_template.h b/softmmu_template.h index 9cb1659..1558b8b 100644 --- a/softmmu_template.h +++ b/softmmu_template.h @@ -548,6 +548,24 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr, helper_te_st_name(env, addr, val, oi, GETRA()); } +#if DATA_SIZE == 1 +/* Probe if the specified guest virtual address exists in TLB and is writable, + if not force a tlb_fill(). As a result an exception can occur. */ +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, + uintptr_t retaddr) +{ + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; + + if ((addr & TARGET_PAGE_MASK) + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { + /* TLB entry is for a different page */ + if (!VICTIM_TLB_HIT(addr_write)) { + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); + } + } +} +#endif #endif /* !defined(SOFTMMU_CODE_ACCESS) */ #undef READ_ACCESS_TYPE -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v6 2/3] softmmu: Add probe_write() 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 2/3] softmmu: Add probe_write() Yongbok Kim @ 2015-05-27 13:42 ` Peter Maydell 0 siblings, 0 replies; 9+ messages in thread From: Peter Maydell @ 2015-05-27 13:42 UTC (permalink / raw) To: Yongbok Kim Cc: Richard Henderson, Leon Alrae, QEMU Developers, Andreas Färber On 27 May 2015 at 14:29, Yongbok Kim <yongbok.kim@imgtec.com> wrote: > Add probe_write() in order to support such functionality that probes > if a specified guest virtual address exists in TLB and is writable. > The function forces a tlb_fill() if the address does not exist or is > not writable, as a result an exception can occur. > > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> > --- > include/exec/exec-all.h | 2 ++ > softmmu_template.h | 18 ++++++++++++++++++ > 2 files changed, 20 insertions(+), 0 deletions(-) > > diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h > index b58cd47..af51203 100644 > --- a/include/exec/exec-all.h > +++ b/include/exec/exec-all.h > @@ -109,6 +109,8 @@ void tlb_set_page_with_attrs(CPUState *cpu, target_ulong vaddr, > hwaddr paddr, MemTxAttrs attrs, > int prot, int mmu_idx, target_ulong size); > void tb_invalidate_phys_addr(AddressSpace *as, hwaddr addr); > +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, > + uintptr_t retaddr); > #else > static inline void tlb_flush_page(CPUState *cpu, target_ulong addr) > { > diff --git a/softmmu_template.h b/softmmu_template.h > index 9cb1659..1558b8b 100644 > --- a/softmmu_template.h > +++ b/softmmu_template.h > @@ -548,6 +548,24 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr, > helper_te_st_name(env, addr, val, oi, GETRA()); > } > > +#if DATA_SIZE == 1 > +/* Probe if the specified guest virtual address exists in TLB and is writable, > + if not force a tlb_fill(). As a result an exception can occur. */ /* Probe for whether the specified guest write access is permitted. * If it is not permitted then an exception will be taken in the same * way as if this were a real write access (and we will not return). * Otherwise the function will return, and there will be a valid * entry in the TLB for this access. */ > +void probe_write(CPUArchState *env, target_ulong addr, int mmu_idx, > + uintptr_t retaddr) > +{ > + int index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1); > + target_ulong tlb_addr = env->tlb_table[mmu_idx][index].addr_write; > + > + if ((addr & TARGET_PAGE_MASK) > + != (tlb_addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK))) { > + /* TLB entry is for a different page */ > + if (!VICTIM_TLB_HIT(addr_write)) { > + tlb_fill(ENV_GET_CPU(env), addr, MMU_DATA_STORE, mmu_idx, retaddr); > + } > + } > +} > +#endif > #endif /* !defined(SOFTMMU_CODE_ACCESS) */ > > #undef READ_ACCESS_TYPE > -- -- PMM ^ permalink raw reply [flat|nested] 9+ messages in thread
* [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA 2015-05-27 13:28 [Qemu-devel] [PATCH v6 0/3] target-mips: Add support for misaligned accesses Yongbok Kim 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 Yongbok Kim 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 2/3] softmmu: Add probe_write() Yongbok Kim @ 2015-05-27 13:29 ` Yongbok Kim 2015-05-29 15:57 ` Leon Alrae 2015-05-29 16:07 ` Leon Alrae 2 siblings, 2 replies; 9+ messages in thread From: Yongbok Kim @ 2015-05-27 13:29 UTC (permalink / raw) To: qemu-devel; +Cc: peter.maydell, leon.alrae, afaerber, rth MIPS SIMD Architecture vector loads and stores require misalignment support. MSA Memory access should work as an atomic operation. Therefore, it has to check validity of all addresses for a vector store access if it is spanning into two pages. Separating helper functions for each data format as format is known in translation. To use mmu_idx from cpu_mmu_index() instead of calculating it from hflag. Removing save_cpu_state() call in translation because it is able to use cpu_restore_state() on fault as GETRA() is passed. Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> --- target-mips/helper.h | 10 +++- target-mips/op_helper.c | 135 +++++++++++++++++++++++++--------------------- target-mips/translate.c | 27 ++++++---- 3 files changed, 98 insertions(+), 74 deletions(-) diff --git a/target-mips/helper.h b/target-mips/helper.h index 3bd0b02..bdd5ba5 100644 --- a/target-mips/helper.h +++ b/target-mips/helper.h @@ -931,5 +931,11 @@ DEF_HELPER_4(msa_ftint_u_df, void, env, i32, i32, i32) DEF_HELPER_4(msa_ffint_s_df, void, env, i32, i32, i32) DEF_HELPER_4(msa_ffint_u_df, void, env, i32, i32, i32) -DEF_HELPER_5(msa_ld_df, void, env, i32, i32, i32, s32) -DEF_HELPER_5(msa_st_df, void, env, i32, i32, i32, s32) +#define MSALDST_PROTO(type) \ +DEF_HELPER_3(msa_ld_ ## type, void, env, i32, tl) \ +DEF_HELPER_3(msa_st_ ## type, void, env, i32, tl) +MSALDST_PROTO(b) +MSALDST_PROTO(h) +MSALDST_PROTO(w) +MSALDST_PROTO(d) +#undef MSALDST_PROTO diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c index 73a8e45..f44b9bb 100644 --- a/target-mips/op_helper.c +++ b/target-mips/op_helper.c @@ -3558,72 +3558,83 @@ FOP_CONDN_S(sne, (float32_lt(fst1, fst0, &env->active_fpu.fp_status) /* Element-by-element access macros */ #define DF_ELEMENTS(df) (MSA_WRLEN / DF_BITS(df)) -void helper_msa_ld_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs, - int32_t s10) -{ - wr_t *pwd = &(env->active_fpu.fpr[wd].wr); - target_ulong addr = env->active_tc.gpr[rs] + (s10 << df); - int i; +#if !defined(CONFIG_USER_ONLY) +#define MEMOP_IDX(DF) \ + TCGMemOpIdx oi = make_memop_idx(MO_TE | DF | MO_UNALN, \ + cpu_mmu_index(env)); +#else +#define MEMOP_IDX(DF) +#endif - switch (df) { - case DF_BYTE: - for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) { - pwd->b[i] = do_lbu(env, addr + (i << DF_BYTE), - env->hflags & MIPS_HFLAG_KSU); - } - break; - case DF_HALF: - for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) { - pwd->h[i] = do_lhu(env, addr + (i << DF_HALF), - env->hflags & MIPS_HFLAG_KSU); - } - break; - case DF_WORD: - for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) { - pwd->w[i] = do_lw(env, addr + (i << DF_WORD), - env->hflags & MIPS_HFLAG_KSU); - } - break; - case DF_DOUBLE: - for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) { - pwd->d[i] = do_ld(env, addr + (i << DF_DOUBLE), - env->hflags & MIPS_HFLAG_KSU); - } - break; - } +#define MSA_LD_DF(DF, TYPE, LD_INSN, ...) \ +void helper_msa_ld_ ## TYPE(CPUMIPSState *env, uint32_t wd, \ + target_ulong addr) \ +{ \ + wr_t *pwd = &(env->active_fpu.fpr[wd].wr); \ + wr_t wx; \ + int i; \ + MEMOP_IDX(DF) \ + for (i = 0; i < DF_ELEMENTS(DF); i++) { \ + wx.TYPE[i] = LD_INSN(env, addr + (i << DF), ##__VA_ARGS__); \ + } \ + memcpy(pwd, &wx, sizeof(wr_t)); \ } -void helper_msa_st_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs, - int32_t s10) +#if !defined(CONFIG_USER_ONLY) +MSA_LD_DF(DF_BYTE, b, helper_ret_ldub_mmu, oi, GETRA()) +MSA_LD_DF(DF_HALF, h, helper_ret_lduw_mmu, oi, GETRA()) +MSA_LD_DF(DF_WORD, w, helper_ret_ldul_mmu, oi, GETRA()) +MSA_LD_DF(DF_DOUBLE, d, helper_ret_ldq_mmu, oi, GETRA()) +#else +MSA_LD_DF(DF_BYTE, b, cpu_ldub_data) +MSA_LD_DF(DF_HALF, h, cpu_lduw_data) +MSA_LD_DF(DF_WORD, w, cpu_ldl_data) +MSA_LD_DF(DF_DOUBLE, d, cpu_ldq_data) +#endif + +static inline void ensure_writable_pages(CPUMIPSState *env, + target_ulong addr, + int mmu_idx, + uintptr_t retaddr) { - wr_t *pwd = &(env->active_fpu.fpr[wd].wr); - target_ulong addr = env->active_tc.gpr[rs] + (s10 << df); - int i; +#if !defined(CONFIG_USER_ONLY) +#define MSA_PAGESPAN(x) (unlikely((((x) & ~TARGET_PAGE_MASK) \ + + MSA_WRLEN/8 - 1) >= TARGET_PAGE_SIZE)) + target_ulong page_addr; - switch (df) { - case DF_BYTE: - for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) { - do_sb(env, addr + (i << DF_BYTE), pwd->b[i], - env->hflags & MIPS_HFLAG_KSU); - } - break; - case DF_HALF: - for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) { - do_sh(env, addr + (i << DF_HALF), pwd->h[i], - env->hflags & MIPS_HFLAG_KSU); - } - break; - case DF_WORD: - for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) { - do_sw(env, addr + (i << DF_WORD), pwd->w[i], - env->hflags & MIPS_HFLAG_KSU); - } - break; - case DF_DOUBLE: - for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) { - do_sd(env, addr + (i << DF_DOUBLE), pwd->d[i], - env->hflags & MIPS_HFLAG_KSU); - } - break; + if (MSA_PAGESPAN(addr)) { + /* first page */ + probe_write(env, addr, mmu_idx, retaddr); + /* second page */ + page_addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; + probe_write(env, page_addr, mmu_idx, retaddr); } +#endif +} + +#define MSA_ST_DF(DF, TYPE, ST_INSN, ...) \ +void helper_msa_st_ ## TYPE(CPUMIPSState *env, uint32_t wd, \ + target_ulong addr) \ +{ \ + wr_t *pwd = &(env->active_fpu.fpr[wd].wr); \ + int mmu_idx = cpu_mmu_index(env); \ + int i; \ + MEMOP_IDX(DF) \ + ensure_writable_pages(env, addr, mmu_idx, GETRA()); \ + for (i = 0; i < DF_ELEMENTS(DF); i++) { \ + ST_INSN(env, addr + (i << DF), pwd->TYPE[i], ##__VA_ARGS__); \ + } \ } + +#if !defined(CONFIG_USER_ONLY) +MSA_ST_DF(DF_BYTE, b, helper_ret_stb_mmu, oi, GETRA()) +MSA_ST_DF(DF_HALF, h, helper_ret_stw_mmu, oi, GETRA()) +MSA_ST_DF(DF_WORD, w, helper_ret_stl_mmu, oi, GETRA()) +MSA_ST_DF(DF_DOUBLE, d, helper_ret_stq_mmu, oi, GETRA()) +#else +MSA_ST_DF(DF_BYTE, b, cpu_stb_data) +MSA_ST_DF(DF_HALF, h, cpu_stw_data) +MSA_ST_DF(DF_WORD, w, cpu_stl_data) +MSA_ST_DF(DF_DOUBLE, d, cpu_stq_data) +#endif + diff --git a/target-mips/translate.c b/target-mips/translate.c index a5ea04e..a30b0f8 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -18419,32 +18419,39 @@ static void gen_msa(CPUMIPSState *env, DisasContext *ctx) uint8_t wd = (ctx->opcode >> 6) & 0x1f; uint8_t df = (ctx->opcode >> 0) & 0x3; - TCGv_i32 tdf = tcg_const_i32(df); TCGv_i32 twd = tcg_const_i32(wd); - TCGv_i32 trs = tcg_const_i32(rs); - TCGv_i32 ts10 = tcg_const_i32(s10); + TCGv taddr = tcg_temp_new(); + gen_base_offset_addr(ctx, taddr, rs, s10 << df); switch (MASK_MSA_MINOR(opcode)) { case OPC_LD_B: + gen_helper_msa_ld_b(cpu_env, twd, taddr); + break; case OPC_LD_H: + gen_helper_msa_ld_h(cpu_env, twd, taddr); + break; case OPC_LD_W: + gen_helper_msa_ld_w(cpu_env, twd, taddr); + break; case OPC_LD_D: - save_cpu_state(ctx, 1); - gen_helper_msa_ld_df(cpu_env, tdf, twd, trs, ts10); + gen_helper_msa_ld_d(cpu_env, twd, taddr); break; case OPC_ST_B: + gen_helper_msa_st_b(cpu_env, twd, taddr); + break; case OPC_ST_H: + gen_helper_msa_st_h(cpu_env, twd, taddr); + break; case OPC_ST_W: + gen_helper_msa_st_w(cpu_env, twd, taddr); + break; case OPC_ST_D: - save_cpu_state(ctx, 1); - gen_helper_msa_st_df(cpu_env, tdf, twd, trs, ts10); + gen_helper_msa_st_d(cpu_env, twd, taddr); break; } tcg_temp_free_i32(twd); - tcg_temp_free_i32(tdf); - tcg_temp_free_i32(trs); - tcg_temp_free_i32(ts10); + tcg_temp_free(taddr); } break; default: -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA Yongbok Kim @ 2015-05-29 15:57 ` Leon Alrae 2015-05-29 16:07 ` Leon Alrae 1 sibling, 0 replies; 9+ messages in thread From: Leon Alrae @ 2015-05-29 15:57 UTC (permalink / raw) To: Yongbok Kim, qemu-devel; +Cc: peter.maydell, afaerber, rth On 27/05/2015 14:29, Yongbok Kim wrote: > MIPS SIMD Architecture vector loads and stores require misalignment support. > MSA Memory access should work as an atomic operation. Therefore, it has to > check validity of all addresses for a vector store access if it is spanning > into two pages. > > Separating helper functions for each data format as format is known in > translation. > To use mmu_idx from cpu_mmu_index() instead of calculating it from hflag. > Removing save_cpu_state() call in translation because it is able to use > cpu_restore_state() on fault as GETRA() is passed. > > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> > --- > target-mips/helper.h | 10 +++- > target-mips/op_helper.c | 135 +++++++++++++++++++++++++--------------------- > target-mips/translate.c | 27 ++++++---- > 3 files changed, 98 insertions(+), 74 deletions(-) > > diff --git a/target-mips/helper.h b/target-mips/helper.h > index 3bd0b02..bdd5ba5 100644 > --- a/target-mips/helper.h > +++ b/target-mips/helper.h > @@ -931,5 +931,11 @@ DEF_HELPER_4(msa_ftint_u_df, void, env, i32, i32, i32) > DEF_HELPER_4(msa_ffint_s_df, void, env, i32, i32, i32) > DEF_HELPER_4(msa_ffint_u_df, void, env, i32, i32, i32) > > -DEF_HELPER_5(msa_ld_df, void, env, i32, i32, i32, s32) > -DEF_HELPER_5(msa_st_df, void, env, i32, i32, i32, s32) > +#define MSALDST_PROTO(type) \ > +DEF_HELPER_3(msa_ld_ ## type, void, env, i32, tl) \ > +DEF_HELPER_3(msa_st_ ## type, void, env, i32, tl) > +MSALDST_PROTO(b) > +MSALDST_PROTO(h) > +MSALDST_PROTO(w) > +MSALDST_PROTO(d) > +#undef MSALDST_PROTO > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c > index 73a8e45..f44b9bb 100644 > --- a/target-mips/op_helper.c > +++ b/target-mips/op_helper.c > @@ -3558,72 +3558,83 @@ FOP_CONDN_S(sne, (float32_lt(fst1, fst0, &env->active_fpu.fp_status) > /* Element-by-element access macros */ > #define DF_ELEMENTS(df) (MSA_WRLEN / DF_BITS(df)) > > -void helper_msa_ld_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs, > - int32_t s10) > -{ > - wr_t *pwd = &(env->active_fpu.fpr[wd].wr); > - target_ulong addr = env->active_tc.gpr[rs] + (s10 << df); > - int i; > +#if !defined(CONFIG_USER_ONLY) > +#define MEMOP_IDX(DF) \ > + TCGMemOpIdx oi = make_memop_idx(MO_TE | DF | MO_UNALN, \ > + cpu_mmu_index(env)); > +#else > +#define MEMOP_IDX(DF) > +#endif > > - switch (df) { > - case DF_BYTE: > - for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) { > - pwd->b[i] = do_lbu(env, addr + (i << DF_BYTE), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_HALF: > - for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) { > - pwd->h[i] = do_lhu(env, addr + (i << DF_HALF), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_WORD: > - for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) { > - pwd->w[i] = do_lw(env, addr + (i << DF_WORD), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_DOUBLE: > - for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) { > - pwd->d[i] = do_ld(env, addr + (i << DF_DOUBLE), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - } > +#define MSA_LD_DF(DF, TYPE, LD_INSN, ...) \ > +void helper_msa_ld_ ## TYPE(CPUMIPSState *env, uint32_t wd, \ > + target_ulong addr) \ > +{ \ > + wr_t *pwd = &(env->active_fpu.fpr[wd].wr); \ > + wr_t wx; \ > + int i; \ > + MEMOP_IDX(DF) \ > + for (i = 0; i < DF_ELEMENTS(DF); i++) { \ > + wx.TYPE[i] = LD_INSN(env, addr + (i << DF), ##__VA_ARGS__); \ > + } \ > + memcpy(pwd, &wx, sizeof(wr_t)); \ > } > > -void helper_msa_st_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs, > - int32_t s10) > +#if !defined(CONFIG_USER_ONLY) > +MSA_LD_DF(DF_BYTE, b, helper_ret_ldub_mmu, oi, GETRA()) > +MSA_LD_DF(DF_HALF, h, helper_ret_lduw_mmu, oi, GETRA()) > +MSA_LD_DF(DF_WORD, w, helper_ret_ldul_mmu, oi, GETRA()) > +MSA_LD_DF(DF_DOUBLE, d, helper_ret_ldq_mmu, oi, GETRA()) > +#else > +MSA_LD_DF(DF_BYTE, b, cpu_ldub_data) > +MSA_LD_DF(DF_HALF, h, cpu_lduw_data) > +MSA_LD_DF(DF_WORD, w, cpu_ldl_data) > +MSA_LD_DF(DF_DOUBLE, d, cpu_ldq_data) > +#endif > + > +static inline void ensure_writable_pages(CPUMIPSState *env, > + target_ulong addr, > + int mmu_idx, > + uintptr_t retaddr) > { > - wr_t *pwd = &(env->active_fpu.fpr[wd].wr); > - target_ulong addr = env->active_tc.gpr[rs] + (s10 << df); > - int i; > +#if !defined(CONFIG_USER_ONLY) > +#define MSA_PAGESPAN(x) (unlikely((((x) & ~TARGET_PAGE_MASK) \ > + + MSA_WRLEN/8 - 1) >= TARGET_PAGE_SIZE)) > + target_ulong page_addr; nit: I find the beginning of this simple function somewhat hard to read. How about moving this macro definition outside the body? > > - switch (df) { > - case DF_BYTE: > - for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) { > - do_sb(env, addr + (i << DF_BYTE), pwd->b[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_HALF: > - for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) { > - do_sh(env, addr + (i << DF_HALF), pwd->h[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_WORD: > - for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) { > - do_sw(env, addr + (i << DF_WORD), pwd->w[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_DOUBLE: > - for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) { > - do_sd(env, addr + (i << DF_DOUBLE), pwd->d[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > + if (MSA_PAGESPAN(addr)) { nit: Wouldn't "unlikely()" fit better here rather than hiding it in MSA_PAGESPAN()? > + /* first page */ > + probe_write(env, addr, mmu_idx, retaddr); > + /* second page */ > + page_addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; > + probe_write(env, page_addr, mmu_idx, retaddr); > } > +#endif > +} > + > +#define MSA_ST_DF(DF, TYPE, ST_INSN, ...) \ > +void helper_msa_st_ ## TYPE(CPUMIPSState *env, uint32_t wd, \ > + target_ulong addr) \ > +{ \ > + wr_t *pwd = &(env->active_fpu.fpr[wd].wr); \ > + int mmu_idx = cpu_mmu_index(env); \ > + int i; \ > + MEMOP_IDX(DF) \ > + ensure_writable_pages(env, addr, mmu_idx, GETRA()); \ > + for (i = 0; i < DF_ELEMENTS(DF); i++) { \ > + ST_INSN(env, addr + (i << DF), pwd->TYPE[i], ##__VA_ARGS__); \ > + } \ > } > + > +#if !defined(CONFIG_USER_ONLY) > +MSA_ST_DF(DF_BYTE, b, helper_ret_stb_mmu, oi, GETRA()) > +MSA_ST_DF(DF_HALF, h, helper_ret_stw_mmu, oi, GETRA()) > +MSA_ST_DF(DF_WORD, w, helper_ret_stl_mmu, oi, GETRA()) > +MSA_ST_DF(DF_DOUBLE, d, helper_ret_stq_mmu, oi, GETRA()) > +#else > +MSA_ST_DF(DF_BYTE, b, cpu_stb_data) > +MSA_ST_DF(DF_HALF, h, cpu_stw_data) > +MSA_ST_DF(DF_WORD, w, cpu_stl_data) > +MSA_ST_DF(DF_DOUBLE, d, cpu_stq_data) > +#endif > + When I applied this patch, git complained: /home/lea/wrk/qemu/.git/rebase-apply/patch:174: new blank line at EOF. + warning: 1 line adds whitespace errors. Generally I'm wondering if this code would look better if helper_msa_ld_* and helper_msa_st_* definitions weren't common for linux-user and system. Yes, we would duplicate some bits, but we would avoid things like ##__VA_ARGS__, empty MEMOP_IDX(DF) and empty ensure_writable_pages(). It's up to you. Thanks, Leon ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA Yongbok Kim 2015-05-29 15:57 ` Leon Alrae @ 2015-05-29 16:07 ` Leon Alrae 1 sibling, 0 replies; 9+ messages in thread From: Leon Alrae @ 2015-05-29 16:07 UTC (permalink / raw) To: Yongbok Kim, qemu-devel; +Cc: peter.maydell, afaerber, rth On 27/05/2015 14:29, Yongbok Kim wrote: > MIPS SIMD Architecture vector loads and stores require misalignment support. > MSA Memory access should work as an atomic operation. Therefore, it has to > check validity of all addresses for a vector store access if it is spanning > into two pages. > > Separating helper functions for each data format as format is known in > translation. > To use mmu_idx from cpu_mmu_index() instead of calculating it from hflag. > Removing save_cpu_state() call in translation because it is able to use > cpu_restore_state() on fault as GETRA() is passed. > > Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com> > --- > target-mips/helper.h | 10 +++- > target-mips/op_helper.c | 135 +++++++++++++++++++++++++--------------------- > target-mips/translate.c | 27 ++++++---- > 3 files changed, 98 insertions(+), 74 deletions(-) > > diff --git a/target-mips/helper.h b/target-mips/helper.h > index 3bd0b02..bdd5ba5 100644 > --- a/target-mips/helper.h > +++ b/target-mips/helper.h > @@ -931,5 +931,11 @@ DEF_HELPER_4(msa_ftint_u_df, void, env, i32, i32, i32) > DEF_HELPER_4(msa_ffint_s_df, void, env, i32, i32, i32) > DEF_HELPER_4(msa_ffint_u_df, void, env, i32, i32, i32) > > -DEF_HELPER_5(msa_ld_df, void, env, i32, i32, i32, s32) > -DEF_HELPER_5(msa_st_df, void, env, i32, i32, i32, s32) > +#define MSALDST_PROTO(type) \ > +DEF_HELPER_3(msa_ld_ ## type, void, env, i32, tl) \ > +DEF_HELPER_3(msa_st_ ## type, void, env, i32, tl) > +MSALDST_PROTO(b) > +MSALDST_PROTO(h) > +MSALDST_PROTO(w) > +MSALDST_PROTO(d) > +#undef MSALDST_PROTO > diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c > index 73a8e45..f44b9bb 100644 > --- a/target-mips/op_helper.c > +++ b/target-mips/op_helper.c > @@ -3558,72 +3558,83 @@ FOP_CONDN_S(sne, (float32_lt(fst1, fst0, &env->active_fpu.fp_status) > /* Element-by-element access macros */ > #define DF_ELEMENTS(df) (MSA_WRLEN / DF_BITS(df)) > > -void helper_msa_ld_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs, > - int32_t s10) > -{ > - wr_t *pwd = &(env->active_fpu.fpr[wd].wr); > - target_ulong addr = env->active_tc.gpr[rs] + (s10 << df); > - int i; > +#if !defined(CONFIG_USER_ONLY) > +#define MEMOP_IDX(DF) \ > + TCGMemOpIdx oi = make_memop_idx(MO_TE | DF | MO_UNALN, \ > + cpu_mmu_index(env)); > +#else > +#define MEMOP_IDX(DF) > +#endif > > - switch (df) { > - case DF_BYTE: > - for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) { > - pwd->b[i] = do_lbu(env, addr + (i << DF_BYTE), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_HALF: > - for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) { > - pwd->h[i] = do_lhu(env, addr + (i << DF_HALF), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_WORD: > - for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) { > - pwd->w[i] = do_lw(env, addr + (i << DF_WORD), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_DOUBLE: > - for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) { > - pwd->d[i] = do_ld(env, addr + (i << DF_DOUBLE), > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - } > +#define MSA_LD_DF(DF, TYPE, LD_INSN, ...) \ > +void helper_msa_ld_ ## TYPE(CPUMIPSState *env, uint32_t wd, \ > + target_ulong addr) \ > +{ \ > + wr_t *pwd = &(env->active_fpu.fpr[wd].wr); \ > + wr_t wx; \ > + int i; \ > + MEMOP_IDX(DF) \ > + for (i = 0; i < DF_ELEMENTS(DF); i++) { \ > + wx.TYPE[i] = LD_INSN(env, addr + (i << DF), ##__VA_ARGS__); \ > + } \ > + memcpy(pwd, &wx, sizeof(wr_t)); \ > } > > -void helper_msa_st_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs, > - int32_t s10) > +#if !defined(CONFIG_USER_ONLY) > +MSA_LD_DF(DF_BYTE, b, helper_ret_ldub_mmu, oi, GETRA()) > +MSA_LD_DF(DF_HALF, h, helper_ret_lduw_mmu, oi, GETRA()) > +MSA_LD_DF(DF_WORD, w, helper_ret_ldul_mmu, oi, GETRA()) > +MSA_LD_DF(DF_DOUBLE, d, helper_ret_ldq_mmu, oi, GETRA()) > +#else > +MSA_LD_DF(DF_BYTE, b, cpu_ldub_data) > +MSA_LD_DF(DF_HALF, h, cpu_lduw_data) > +MSA_LD_DF(DF_WORD, w, cpu_ldl_data) > +MSA_LD_DF(DF_DOUBLE, d, cpu_ldq_data) > +#endif > + > +static inline void ensure_writable_pages(CPUMIPSState *env, > + target_ulong addr, > + int mmu_idx, > + uintptr_t retaddr) > { > - wr_t *pwd = &(env->active_fpu.fpr[wd].wr); > - target_ulong addr = env->active_tc.gpr[rs] + (s10 << df); > - int i; > +#if !defined(CONFIG_USER_ONLY) > +#define MSA_PAGESPAN(x) (unlikely((((x) & ~TARGET_PAGE_MASK) \ > + + MSA_WRLEN/8 - 1) >= TARGET_PAGE_SIZE)) > + target_ulong page_addr; nit: I find the beginning of this simple function somewhat hard to read. How about moving this macro definition outside the function? > > - switch (df) { > - case DF_BYTE: > - for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) { > - do_sb(env, addr + (i << DF_BYTE), pwd->b[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_HALF: > - for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) { > - do_sh(env, addr + (i << DF_HALF), pwd->h[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_WORD: > - for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) { > - do_sw(env, addr + (i << DF_WORD), pwd->w[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > - case DF_DOUBLE: > - for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) { > - do_sd(env, addr + (i << DF_DOUBLE), pwd->d[i], > - env->hflags & MIPS_HFLAG_KSU); > - } > - break; > + if (MSA_PAGESPAN(addr)) { nit: Wouldn't "unlikely" fit better here rather than hiding it in MSA_PAGESPAN()? > + /* first page */ > + probe_write(env, addr, mmu_idx, retaddr); > + /* second page */ > + page_addr = (addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE; > + probe_write(env, page_addr, mmu_idx, retaddr); > } > +#endif > +} > + > +#define MSA_ST_DF(DF, TYPE, ST_INSN, ...) \ > +void helper_msa_st_ ## TYPE(CPUMIPSState *env, uint32_t wd, \ > + target_ulong addr) \ > +{ \ > + wr_t *pwd = &(env->active_fpu.fpr[wd].wr); \ > + int mmu_idx = cpu_mmu_index(env); \ > + int i; \ > + MEMOP_IDX(DF) \ > + ensure_writable_pages(env, addr, mmu_idx, GETRA()); \ > + for (i = 0; i < DF_ELEMENTS(DF); i++) { \ > + ST_INSN(env, addr + (i << DF), pwd->TYPE[i], ##__VA_ARGS__); \ > + } \ > } > + > +#if !defined(CONFIG_USER_ONLY) > +MSA_ST_DF(DF_BYTE, b, helper_ret_stb_mmu, oi, GETRA()) > +MSA_ST_DF(DF_HALF, h, helper_ret_stw_mmu, oi, GETRA()) > +MSA_ST_DF(DF_WORD, w, helper_ret_stl_mmu, oi, GETRA()) > +MSA_ST_DF(DF_DOUBLE, d, helper_ret_stq_mmu, oi, GETRA()) > +#else > +MSA_ST_DF(DF_BYTE, b, cpu_stb_data) > +MSA_ST_DF(DF_HALF, h, cpu_stw_data) > +MSA_ST_DF(DF_WORD, w, cpu_stl_data) > +MSA_ST_DF(DF_DOUBLE, d, cpu_stq_data) > +#endif > + git complained: /home/lea/wrk/qemu/.git/rebase-apply/patch:174: new blank line at EOF. + warning: 1 line adds whitespace errors. Generally I'm wondering if this code would look better if helper_msa_ld_* and helper_msa_st_* definitions weren't common for linux-user and system. Yes, we would duplicate some bits, but we would avoid things like ##__VA_ARGS__, empty MEMOP_IDX(DF) and empty ensure_writable_pages(). It's up to you. Thanks, Leon > diff --git a/target-mips/translate.c b/target-mips/translate.c > index a5ea04e..a30b0f8 100644 > --- a/target-mips/translate.c > +++ b/target-mips/translate.c > @@ -18419,32 +18419,39 @@ static void gen_msa(CPUMIPSState *env, DisasContext *ctx) > uint8_t wd = (ctx->opcode >> 6) & 0x1f; > uint8_t df = (ctx->opcode >> 0) & 0x3; > > - TCGv_i32 tdf = tcg_const_i32(df); > TCGv_i32 twd = tcg_const_i32(wd); > - TCGv_i32 trs = tcg_const_i32(rs); > - TCGv_i32 ts10 = tcg_const_i32(s10); > + TCGv taddr = tcg_temp_new(); > + gen_base_offset_addr(ctx, taddr, rs, s10 << df); > > switch (MASK_MSA_MINOR(opcode)) { > case OPC_LD_B: > + gen_helper_msa_ld_b(cpu_env, twd, taddr); > + break; > case OPC_LD_H: > + gen_helper_msa_ld_h(cpu_env, twd, taddr); > + break; > case OPC_LD_W: > + gen_helper_msa_ld_w(cpu_env, twd, taddr); > + break; > case OPC_LD_D: > - save_cpu_state(ctx, 1); > - gen_helper_msa_ld_df(cpu_env, tdf, twd, trs, ts10); > + gen_helper_msa_ld_d(cpu_env, twd, taddr); > break; > case OPC_ST_B: > + gen_helper_msa_st_b(cpu_env, twd, taddr); > + break; > case OPC_ST_H: > + gen_helper_msa_st_h(cpu_env, twd, taddr); > + break; > case OPC_ST_W: > + gen_helper_msa_st_w(cpu_env, twd, taddr); > + break; > case OPC_ST_D: > - save_cpu_state(ctx, 1); > - gen_helper_msa_st_df(cpu_env, tdf, twd, trs, ts10); > + gen_helper_msa_st_d(cpu_env, twd, taddr); > break; > } > > tcg_temp_free_i32(twd); > - tcg_temp_free_i32(tdf); > - tcg_temp_free_i32(trs); > - tcg_temp_free_i32(ts10); > + tcg_temp_free(taddr); > } > break; > default: > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2015-06-01 10:55 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-27 13:28 [Qemu-devel] [PATCH v6 0/3] target-mips: Add support for misaligned accesses Yongbok Kim 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 1/3] target-mips: Misaligned memory accesses for R6 Yongbok Kim 2015-05-29 12:33 ` Leon Alrae 2015-06-01 8:29 ` Yongbok Kim 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 2/3] softmmu: Add probe_write() Yongbok Kim 2015-05-27 13:42 ` Peter Maydell 2015-05-27 13:29 ` [Qemu-devel] [PATCH v6 3/3] target-mips: Misaligned memory accesses for MSA Yongbok Kim 2015-05-29 15:57 ` Leon Alrae 2015-05-29 16:07 ` Leon Alrae
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).