[Qemu-devel] [PATCH v5 0/3] target-mips: Add support for misaligned accesses

[Qemu-devel] [PATCH v5 0/3] target-mips: Add support for misaligned accesses

[Yongbok Kim]

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

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           |   20 ++++++
 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, 152 insertions(+), 89 deletions(-)

-- 
1.7.5.4

[Qemu-devel] [PATCH v5 1/3] target-mips: Misaligned memory accesses for R6

[Yongbok Kim]

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

[Qemu-devel] [PATCH v5 2/3] softmmu: Add probe_write()

[Yongbok Kim]

Add probe_write() forces a tlb_fill if the specified guest virtual
index isn't in the TCG softmmu TLB.

Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 include/exec/exec-all.h |    2 ++
 softmmu_template.h      |   20 ++++++++++++++++++++
 2 files changed, 22 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..1a1de4a 100644
--- a/softmmu_template.h
+++ b/softmmu_template.h
@@ -548,6 +548,26 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
     helper_te_st_name(env, addr, val, oi, GETRA());
 }
 
+#if DATA_SIZE == 1
+/*
+ * Force a tlb_fill if the specified guest virtual index isn't in the TCG
+ * softmmu TLB.
+ */
+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

Re: [Qemu-devel] [PATCH v5 2/3] softmmu: Add probe_write()

[Peter Maydell]

On 26 May 2015 at 17:49, Yongbok Kim <yongbok.kim@imgtec.com> wrote:
> Add probe_write() forces a tlb_fill if the specified guest virtual
> index isn't in the TCG softmmu TLB.

Surely the point is not to fill the TLB but to raise an
exception if the address is not writable?

> +#if DATA_SIZE == 1
> +/*
> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG
> + * softmmu TLB.
> + */
> +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) */

I think this code does what you want, but the comments are
rather misleading...

-- PMM

Re: [Qemu-devel] [PATCH v5 2/3] softmmu: Add probe_write()

[Richard Henderson]

On 05/26/2015 09:53 AM, Peter Maydell wrote:
> On 26 May 2015 at 17:49, Yongbok Kim <yongbok.kim@imgtec.com> wrote:
>> Add probe_write() forces a tlb_fill if the specified guest virtual
>> index isn't in the TCG softmmu TLB.
> 
> Surely the point is not to fill the TLB but to raise an
> exception if the address is not writable?
> 
>> +#if DATA_SIZE == 1
>> +/*
>> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG
>> + * softmmu TLB.
>> + */
>> +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) */
> 
> I think this code does what you want, but the comments are
> rather misleading...

Indeed.  The fact that the TLB gets loaded is merely a happy side-effect.


r~

Re: [Qemu-devel] [PATCH v5 2/3] softmmu: Add probe_write()

[Yongbok Kim]

On 26/05/2015 18:33, Richard Henderson wrote:
> On 05/26/2015 09:53 AM, Peter Maydell wrote:
>> On 26 May 2015 at 17:49, Yongbok Kim <yongbok.kim@imgtec.com> wrote:
>>> Add probe_write() forces a tlb_fill if the specified guest virtual
>>> index isn't in the TCG softmmu TLB.
>>
>> Surely the point is not to fill the TLB but to raise an
>> exception if the address is not writable?
>>
>>> +#if DATA_SIZE == 1
>>> +/*
>>> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG
>>> + * softmmu TLB.
>>> + */
>>> +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) */
>>
>> I think this code does what you want, but the comments are
>> rather misleading...
> 
> Indeed.  The fact that the TLB gets loaded is merely a happy side-effect.
> 
> 
> r~
> 

Agreed, I will change the description.

Regards,
Yongbok

Re: [Qemu-devel] [PATCH v5 2/3] softmmu: Add probe_write()

[Andreas Färber]

Am 26.05.2015 um 18:49 schrieb Yongbok Kim:
> Add probe_write() forces a tlb_fill if the specified guest virtual
> index isn't in the TCG softmmu TLB.
> 
> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
> ---
>  include/exec/exec-all.h |    2 ++
>  softmmu_template.h      |   20 ++++++++++++++++++++
>  2 files changed, 22 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..1a1de4a 100644
> --- a/softmmu_template.h
> +++ b/softmmu_template.h
> @@ -548,6 +548,26 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
>      helper_te_st_name(env, addr, val, oi, GETRA());
>  }
>  
> +#if DATA_SIZE == 1
> +/*
> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG
> + * softmmu TLB.
> + */
> +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);

Isn't the use of ENV_GET_CPU() here contradicting Peter C.'s series?

Regards,
Andreas

> +        }
> +    }
> +}
> +#endif
>  #endif /* !defined(SOFTMMU_CODE_ACCESS) */
>  
>  #undef READ_ACCESS_TYPE

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)

Re: [Qemu-devel] [PATCH v5 2/3] softmmu: Add probe_write()

[Paolo Bonzini]

On 26/05/2015 18:58, Andreas Färber wrote:
>> > +#if DATA_SIZE == 1
>> > +/*
>> > + * Force a tlb_fill if the specified guest virtual index isn't in the TCG
>> > + * softmmu TLB.
>> > + */
>> > +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);
> Isn't the use of ENV_GET_CPU() here contradicting Peter C.'s series?

No, I don't think so.

This function has a genuine need to access env's fields, so it is okay
for it to accept CPUArchState*, especially because it is called from the
TCG innards where everything is based on the CPUArchState* anyway.

The functions touched by Peter's series (gdbserver_fork, tb_flush,
tcg_cpu_exec, cpu_exec_init) don't, so they should accept CPUState*.

In other words, it's okay to go from env to cpu, but you should do it as
soon as possible.

Paolo

Re: [Qemu-devel] [PATCH v5 2/3] softmmu: Add probe_write()

[Peter Crosthwaite]

On Tue, May 26, 2015 at 9:58 AM, Andreas Färber <afaerber@suse.de> wrote:
> Am 26.05.2015 um 18:49 schrieb Yongbok Kim:
>> Add probe_write() forces a tlb_fill if the specified guest virtual
>> index isn't in the TCG softmmu TLB.
>>
>> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
>> ---
>>  include/exec/exec-all.h |    2 ++
>>  softmmu_template.h      |   20 ++++++++++++++++++++
>>  2 files changed, 22 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..1a1de4a 100644
>> --- a/softmmu_template.h
>> +++ b/softmmu_template.h
>> @@ -548,6 +548,26 @@ glue(glue(helper_st, SUFFIX), MMUSUFFIX)(CPUArchState *env, target_ulong addr,
>>      helper_te_st_name(env, addr, val, oi, GETRA());
>>  }
>>
>> +#if DATA_SIZE == 1
>> +/*
>> + * Force a tlb_fill if the specified guest virtual index isn't in the TCG
>> + * softmmu TLB.
>> + */
>> +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);
>
> Isn't the use of ENV_GET_CPU() here contradicting Peter C.'s series?
>

Actually no this one is OK. softmmu_template is in the "write off"
bucket out outside my definition of core-code still. I am looking into
paolos approach of cputlb, exec-all and translate-all being a
multi-compiled template. Files inside the template are free to use
ENV_GET_CPU.

Regards,
Peter

> Regards,
> Andreas
>
>> +        }
>> +    }
>> +}
>> +#endif
>>  #endif /* !defined(SOFTMMU_CODE_ACCESS) */
>>
>>  #undef READ_ACCESS_TYPE
>
> --
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
> 21284 (AG Nürnberg)
>

[Qemu-devel] [PATCH v5 3/3] target-mips: Misaligned memory accesses for MSA

[Yongbok Kim]

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