[PATCH 0/2] target/m68k: implement unaligned accesses for m68k CPUs

[PATCH 0/2] target/m68k: implement unaligned accesses for m68k CPUs

[Mark Cave-Ayland]

This series implements unaligned accesses for early m68k CPUs that do not
support word/long accesses at byte granularity. Patch 1 implements the
do_unaligned_access function for m68k CPUs, whilst patch 2 is based upon a
prototype patch developed by Laurent as part of Gitlab Issue #2165.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2165


Mark Cave-Ayland (2):
  target/m68k: implement do_unaligned_access callback for m68k CPUs
  target/m68k: pass alignment into TCG memory load/store routines

 target/m68k/cpu.c       |  1 +
 target/m68k/cpu.h       |  4 ++++
 target/m68k/op_helper.c | 11 +++++++++++
 target/m68k/translate.c | 18 +++++++++++++++---
 4 files changed, 31 insertions(+), 3 deletions(-)

-- 
2.39.2

[PATCH 1/2] target/m68k: implement do_unaligned_access callback for m68k CPUs

[Mark Cave-Ayland]

For m68k CPUs that do not support unaligned accesses, any such access should
cause the CPU to raise an Address Error exception.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 target/m68k/cpu.c       |  1 +
 target/m68k/cpu.h       |  4 ++++
 target/m68k/op_helper.c | 11 +++++++++++
 3 files changed, 16 insertions(+)

diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
index efd6bbded8..25e95f9f68 100644
--- a/target/m68k/cpu.c
+++ b/target/m68k/cpu.c
@@ -538,6 +538,7 @@ static const TCGCPUOps m68k_tcg_ops = {
     .cpu_exec_interrupt = m68k_cpu_exec_interrupt,
     .do_interrupt = m68k_cpu_do_interrupt,
     .do_transaction_failed = m68k_cpu_transaction_failed,
+    .do_unaligned_access = m68k_cpu_do_unaligned_access,
 #endif /* !CONFIG_USER_ONLY */
 };
 
diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
index b5bbeedb7a..d4c9531b1c 100644
--- a/target/m68k/cpu.h
+++ b/target/m68k/cpu.h
@@ -590,6 +590,10 @@ void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
                                  unsigned size, MMUAccessType access_type,
                                  int mmu_idx, MemTxAttrs attrs,
                                  MemTxResult response, uintptr_t retaddr);
+G_NORETURN void m68k_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+                                             MMUAccessType access_type,
+                                             int mmu_idx,
+                                             uintptr_t retaddr);
 #endif
 
 #include "exec/cpu-all.h"
diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
index 15bad5dd46..417b691d8d 100644
--- a/target/m68k/op_helper.c
+++ b/target/m68k/op_helper.c
@@ -558,6 +558,17 @@ raise_exception_format2(CPUM68KState *env, int tt, int ilen, uintptr_t raddr)
     cpu_loop_exit(cs);
 }
 
+#if !defined(CONFIG_USER_ONLY)
+G_NORETURN void m68k_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
+                                             MMUAccessType access_type,
+                                             int mmu_idx, uintptr_t retaddr)
+{
+    CPUM68KState *env = cpu_env(cs);
+
+    raise_exception(env, EXCP_ADDRESS);
+}
+#endif
+
 void HELPER(divuw)(CPUM68KState *env, int destr, uint32_t den, int ilen)
 {
     uint32_t num = env->dregs[destr];
-- 
2.39.2

Re: [PATCH 1/2] target/m68k: implement do_unaligned_access callback for m68k CPUs

[BALATON Zoltan]

On Sun, 23 Jun 2024, Mark Cave-Ayland wrote:
> For m68k CPUs that do not support unaligned accesses, any such access should
> cause the CPU to raise an Address Error exception.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> target/m68k/cpu.c       |  1 +
> target/m68k/cpu.h       |  4 ++++
> target/m68k/op_helper.c | 11 +++++++++++
> 3 files changed, 16 insertions(+)
>
> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
> index efd6bbded8..25e95f9f68 100644
> --- a/target/m68k/cpu.c
> +++ b/target/m68k/cpu.c
> @@ -538,6 +538,7 @@ static const TCGCPUOps m68k_tcg_ops = {
>     .cpu_exec_interrupt = m68k_cpu_exec_interrupt,
>     .do_interrupt = m68k_cpu_do_interrupt,
>     .do_transaction_failed = m68k_cpu_transaction_failed,
> +    .do_unaligned_access = m68k_cpu_do_unaligned_access,
> #endif /* !CONFIG_USER_ONLY */

Why is it sysemu only? Shouldn't user mode cpu only emulation do the same? 
I also don't get how this is restricted to pre 68020 CPUs or account for 
differences between data and inst fetch on 20+ but I may be missing 
somerhing as I don't know this code or 68k behaviour well. So this is just 
a question, I'm not saying it's wrong but I don't understand why it's 
right.

Regards,
BALATON Zoltan

> };
>
> diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h
> index b5bbeedb7a..d4c9531b1c 100644
> --- a/target/m68k/cpu.h
> +++ b/target/m68k/cpu.h
> @@ -590,6 +590,10 @@ void m68k_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
>                                  unsigned size, MMUAccessType access_type,
>                                  int mmu_idx, MemTxAttrs attrs,
>                                  MemTxResult response, uintptr_t retaddr);
> +G_NORETURN void m68k_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> +                                             MMUAccessType access_type,
> +                                             int mmu_idx,
> +                                             uintptr_t retaddr);
> #endif
>
> #include "exec/cpu-all.h"
> diff --git a/target/m68k/op_helper.c b/target/m68k/op_helper.c
> index 15bad5dd46..417b691d8d 100644
> --- a/target/m68k/op_helper.c
> +++ b/target/m68k/op_helper.c
> @@ -558,6 +558,17 @@ raise_exception_format2(CPUM68KState *env, int tt, int ilen, uintptr_t raddr)
>     cpu_loop_exit(cs);
> }
>
> +#if !defined(CONFIG_USER_ONLY)
> +G_NORETURN void m68k_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> +                                             MMUAccessType access_type,
> +                                             int mmu_idx, uintptr_t retaddr)
> +{
> +    CPUM68KState *env = cpu_env(cs);
> +
> +    raise_exception(env, EXCP_ADDRESS);
> +}
> +#endif
> +
> void HELPER(divuw)(CPUM68KState *env, int destr, uint32_t den, int ilen)
> {
>     uint32_t num = env->dregs[destr];
>

Re: [PATCH 1/2] target/m68k: implement do_unaligned_access callback for m68k CPUs

[Mark Cave-Ayland]

On 23/06/2024 16:11, BALATON Zoltan wrote:

> On Sun, 23 Jun 2024, Mark Cave-Ayland wrote:
>> For m68k CPUs that do not support unaligned accesses, any such access should
>> cause the CPU to raise an Address Error exception.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> target/m68k/cpu.c       |  1 +
>> target/m68k/cpu.h       |  4 ++++
>> target/m68k/op_helper.c | 11 +++++++++++
>> 3 files changed, 16 insertions(+)
>>
>> diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c
>> index efd6bbded8..25e95f9f68 100644
>> --- a/target/m68k/cpu.c
>> +++ b/target/m68k/cpu.c
>> @@ -538,6 +538,7 @@ static const TCGCPUOps m68k_tcg_ops = {
>>     .cpu_exec_interrupt = m68k_cpu_exec_interrupt,
>>     .do_interrupt = m68k_cpu_do_interrupt,
>>     .do_transaction_failed = m68k_cpu_transaction_failed,
>> +    .do_unaligned_access = m68k_cpu_do_unaligned_access,
>> #endif /* !CONFIG_USER_ONLY */
> 
> Why is it sysemu only? Shouldn't user mode cpu only emulation do the same? I also 
> don't get how this is restricted to pre 68020 CPUs or account for differences between 
> data and inst fetch on 20+ but I may be missing somerhing as I don't know this code 
> or 68k behaviour well. So this is just a question, I'm not saying it's wrong but I 
> don't understand why it's right.

I'm not exactly sure, but I'm guessing that this is handled by the host user code 
since all CPUs that implement do_unaligned_access do so in a block contained within 
#ifndef CONFIG_USER_ONLY ... #endif.


ATB,

Mark.

Re: [PATCH 1/2] target/m68k: implement do_unaligned_access callback for m68k CPUs

[Richard Henderson]

On 6/23/24 04:57, Mark Cave-Ayland wrote:
> +G_NORETURN void m68k_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> +                                             MMUAccessType access_type,
> +                                             int mmu_idx, uintptr_t retaddr)
> +{
> +    CPUM68KState *env = cpu_env(cs);
> +
> +    raise_exception(env, EXCP_ADDRESS);
> +}

Surely the address gets stored in the exception frame somewhere?


r~

Re: [PATCH 1/2] target/m68k: implement do_unaligned_access callback for m68k CPUs

[Mark Cave-Ayland]

On 23/06/2024 20:47, Richard Henderson wrote:

> On 6/23/24 04:57, Mark Cave-Ayland wrote:
>> +G_NORETURN void m68k_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
>> +                                             MMUAccessType access_type,
>> +                                             int mmu_idx, uintptr_t retaddr)
>> +{
>> +    CPUM68KState *env = cpu_env(cs);
>> +
>> +    raise_exception(env, EXCP_ADDRESS);
>> +}
> 
> Surely the address gets stored in the exception frame somewhere?
>  
> r~

Well. There is already some logic there for EXCP_ADDRESS in m68k_interrupt_all() so I 
thought this would just work as-is, but upon closer reading of older CPU datasheets 
it appears there are at least 2 different stack frame formats for CPUs before the 68040.

It looks like I'll have to do a bit more research before posting a v2.


ATB,

Mark.

[PATCH 2/2] target/m68k: pass alignment into TCG memory load/store routines

[Mark Cave-Ayland]

Now that do_unaligned_access has been implemented for 68k CPUs, pass the required
alignment into the TCG memory load/store routines. This allows the TCG memory core
to generate an Address Error exception for unaligned memory accesses if required.

Suggested-by: Laurent Vivier <laurent@vivier.eu>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2165
---
 target/m68k/translate.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/target/m68k/translate.c b/target/m68k/translate.c
index 445966fb6a..661a7b4def 100644
--- a/target/m68k/translate.c
+++ b/target/m68k/translate.c
@@ -303,13 +303,18 @@ static inline TCGv gen_load(DisasContext *s, int opsize, TCGv addr,
                             int sign, int index)
 {
     TCGv tmp = tcg_temp_new_i32();
+    MemOp memop = opsize | (sign ? MO_SIGN : 0) | MO_TE;
 
     switch (opsize) {
     case OS_BYTE:
+        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
+        break;
     case OS_WORD:
     case OS_LONG:
-        tcg_gen_qemu_ld_tl(tmp, addr, index,
-                           opsize | (sign ? MO_SIGN : 0) | MO_TE);
+        if (!m68k_feature(s->env, M68K_FEATURE_UNALIGNED_DATA)) {
+            memop |= MO_ALIGN_2;
+        }
+        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
         break;
     default:
         g_assert_not_reached();
@@ -321,11 +326,18 @@ static inline TCGv gen_load(DisasContext *s, int opsize, TCGv addr,
 static inline void gen_store(DisasContext *s, int opsize, TCGv addr, TCGv val,
                              int index)
 {
+    MemOp memop = opsize | MO_TE;
+
     switch (opsize) {
     case OS_BYTE:
+        tcg_gen_qemu_st_tl(val, addr, index, memop);
+        break;
     case OS_WORD:
     case OS_LONG:
-        tcg_gen_qemu_st_tl(val, addr, index, opsize | MO_TE);
+        if (!m68k_feature(s->env, M68K_FEATURE_UNALIGNED_DATA)) {
+            memop |= MO_ALIGN_2;
+        }
+        tcg_gen_qemu_st_tl(val, addr, index, memop);
         break;
     default:
         g_assert_not_reached();
-- 
2.39.2

Re: [PATCH 2/2] target/m68k: pass alignment into TCG memory load/store routines

[BALATON Zoltan]

On Sun, 23 Jun 2024, Mark Cave-Ayland wrote:
> Now that do_unaligned_access has been implemented for 68k CPUs, pass the required
> alignment into the TCG memory load/store routines. This allows the TCG memory core
> to generate an Address Error exception for unaligned memory accesses if required.
>
> Suggested-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2165
> ---
> target/m68k/translate.c | 18 +++++++++++++++---
> 1 file changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
> index 445966fb6a..661a7b4def 100644
> --- a/target/m68k/translate.c
> +++ b/target/m68k/translate.c
> @@ -303,13 +303,18 @@ static inline TCGv gen_load(DisasContext *s, int opsize, TCGv addr,
>                             int sign, int index)
> {
>     TCGv tmp = tcg_temp_new_i32();
> +    MemOp memop = opsize | (sign ? MO_SIGN : 0) | MO_TE;
>
>     switch (opsize) {
>     case OS_BYTE:
> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
> +        break;
>     case OS_WORD:
>     case OS_LONG:
> -        tcg_gen_qemu_ld_tl(tmp, addr, index,
> -                           opsize | (sign ? MO_SIGN : 0) | MO_TE);
> +        if (!m68k_feature(s->env, M68K_FEATURE_UNALIGNED_DATA)) {
> +            memop |= MO_ALIGN_2;
> +        }
> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);

You could swap the order of these so byte comes last and fall through to 
it from word/long to avoid duplicated line.

Maybe this answers my question about where it's restriced by CPU type. I 
wonder if this check for M68K_FEATURE_UNALIGNED_DATA could be avoded here 
and done by checking it in init and only set the unaligned method for CPUs 
that need it to not add overhead for most CPUs that don't need it.

Regards,
BALATON Zoltan

>         break;
>     default:
>         g_assert_not_reached();
> @@ -321,11 +326,18 @@ static inline TCGv gen_load(DisasContext *s, int opsize, TCGv addr,
> static inline void gen_store(DisasContext *s, int opsize, TCGv addr, TCGv val,
>                              int index)
> {
> +    MemOp memop = opsize | MO_TE;
> +
>     switch (opsize) {
>     case OS_BYTE:
> +        tcg_gen_qemu_st_tl(val, addr, index, memop);
> +        break;
>     case OS_WORD:
>     case OS_LONG:
> -        tcg_gen_qemu_st_tl(val, addr, index, opsize | MO_TE);
> +        if (!m68k_feature(s->env, M68K_FEATURE_UNALIGNED_DATA)) {
> +            memop |= MO_ALIGN_2;
> +        }
> +        tcg_gen_qemu_st_tl(val, addr, index, memop);
>         break;
>     default:
>         g_assert_not_reached();
>

Re: [PATCH 2/2] target/m68k: pass alignment into TCG memory load/store routines

[Richard Henderson]

On 6/23/24 08:23, BALATON Zoltan wrote:
> On Sun, 23 Jun 2024, Mark Cave-Ayland wrote:
>> Now that do_unaligned_access has been implemented for 68k CPUs, pass the required
>> alignment into the TCG memory load/store routines. This allows the TCG memory core
>> to generate an Address Error exception for unaligned memory accesses if required.
>>
>> Suggested-by: Laurent Vivier <laurent@vivier.eu>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2165
>> ---
>> target/m68k/translate.c | 18 +++++++++++++++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
>> index 445966fb6a..661a7b4def 100644
>> --- a/target/m68k/translate.c
>> +++ b/target/m68k/translate.c
>> @@ -303,13 +303,18 @@ static inline TCGv gen_load(DisasContext *s, int opsize, TCGv addr,
>>                             int sign, int index)
>> {
>>     TCGv tmp = tcg_temp_new_i32();
>> +    MemOp memop = opsize | (sign ? MO_SIGN : 0) | MO_TE;
>>
>>     switch (opsize) {
>>     case OS_BYTE:
>> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
>> +        break;
>>     case OS_WORD:
>>     case OS_LONG:
>> -        tcg_gen_qemu_ld_tl(tmp, addr, index,
>> -                           opsize | (sign ? MO_SIGN : 0) | MO_TE);
>> +        if (!m68k_feature(s->env, M68K_FEATURE_UNALIGNED_DATA)) {
>> +            memop |= MO_ALIGN_2;
>> +        }
>> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
> 
> You could swap the order of these so byte comes last and fall through to it from word/long 
> to avoid duplicated line.
> 
> Maybe this answers my question about where it's restriced by CPU type. I wonder if this 
> check for M68K_FEATURE_UNALIGNED_DATA could be avoded here and done by checking it in init 
> and only set the unaligned method for CPUs that need it to not add overhead for most CPUs 
> that don't need it.

No, there's no overhead in having the unaligned method present always.

But swapping the order of case labels, or sinking the tcg_gen_qemu_ld_tl call below the 
switch, is a good idea.


r~

Re: [PATCH 2/2] target/m68k: pass alignment into TCG memory load/store routines

[Mark Cave-Ayland]

On 23/06/2024 16:23, BALATON Zoltan wrote:

> On Sun, 23 Jun 2024, Mark Cave-Ayland wrote:
>> Now that do_unaligned_access has been implemented for 68k CPUs, pass the required
>> alignment into the TCG memory load/store routines. This allows the TCG memory core
>> to generate an Address Error exception for unaligned memory accesses if required.
>>
>> Suggested-by: Laurent Vivier <laurent@vivier.eu>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2165
>> ---
>> target/m68k/translate.c | 18 +++++++++++++++---
>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
>> index 445966fb6a..661a7b4def 100644
>> --- a/target/m68k/translate.c
>> +++ b/target/m68k/translate.c
>> @@ -303,13 +303,18 @@ static inline TCGv gen_load(DisasContext *s, int opsize, TCGv 
>> addr,
>>                             int sign, int index)
>> {
>>     TCGv tmp = tcg_temp_new_i32();
>> +    MemOp memop = opsize | (sign ? MO_SIGN : 0) | MO_TE;
>>
>>     switch (opsize) {
>>     case OS_BYTE:
>> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
>> +        break;
>>     case OS_WORD:
>>     case OS_LONG:
>> -        tcg_gen_qemu_ld_tl(tmp, addr, index,
>> -                           opsize | (sign ? MO_SIGN : 0) | MO_TE);
>> +        if (!m68k_feature(s->env, M68K_FEATURE_UNALIGNED_DATA)) {
>> +            memop |= MO_ALIGN_2;
>> +        }
>> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
> 
> You could swap the order of these so byte comes last and fall through to it from 
> word/long to avoid duplicated line.
> 
> Maybe this answers my question about where it's restriced by CPU type. I wonder if 
> this check for M68K_FEATURE_UNALIGNED_DATA could be avoded here and done by checking 
> it in init and only set the unaligned method for CPUs that need it to not add 
> overhead for most CPUs that don't need it.

I don't think that it matters too much if the method isn't implemented as the logic 
surrounding when to call do_unaligned_access is contained within the TCG core.

I'll have a go at updating the ordering and send a v2 if it looks good.


ATB,

Mark.

Re: [PATCH 2/2] target/m68k: pass alignment into TCG memory load/store routines

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 3124 bytes --]

On Mon, 24 Jun 2024, Mark Cave-Ayland wrote:
> On 23/06/2024 16:23, BALATON Zoltan wrote:
>> On Sun, 23 Jun 2024, Mark Cave-Ayland wrote:
>>> Now that do_unaligned_access has been implemented for 68k CPUs, pass the 
>>> required
>>> alignment into the TCG memory load/store routines. This allows the TCG 
>>> memory core
>>> to generate an Address Error exception for unaligned memory accesses if 
>>> required.
>>> 
>>> Suggested-by: Laurent Vivier <laurent@vivier.eu>
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2165
>>> ---
>>> target/m68k/translate.c | 18 +++++++++++++++---
>>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/target/m68k/translate.c b/target/m68k/translate.c
>>> index 445966fb6a..661a7b4def 100644
>>> --- a/target/m68k/translate.c
>>> +++ b/target/m68k/translate.c
>>> @@ -303,13 +303,18 @@ static inline TCGv gen_load(DisasContext *s, int 
>>> opsize, TCGv addr,
>>>                             int sign, int index)
>>> {
>>>     TCGv tmp = tcg_temp_new_i32();
>>> +    MemOp memop = opsize | (sign ? MO_SIGN : 0) | MO_TE;
>>> 
>>>     switch (opsize) {
>>>     case OS_BYTE:
>>> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
>>> +        break;
>>>     case OS_WORD:
>>>     case OS_LONG:
>>> -        tcg_gen_qemu_ld_tl(tmp, addr, index,
>>> -                           opsize | (sign ? MO_SIGN : 0) | MO_TE);
>>> +        if (!m68k_feature(s->env, M68K_FEATURE_UNALIGNED_DATA)) {
>>> +            memop |= MO_ALIGN_2;
>>> +        }
>>> +        tcg_gen_qemu_ld_tl(tmp, addr, index, memop);
>> 
>> You could swap the order of these so byte comes last and fall through to it 
>> from word/long to avoid duplicated line.
>> 
>> Maybe this answers my question about where it's restriced by CPU type. I 
>> wonder if this check for M68K_FEATURE_UNALIGNED_DATA could be avoded here 
>> and done by checking it in init and only set the unaligned method for CPUs 
>> that need it to not add overhead for most CPUs that don't need it.
>
> I don't think that it matters too much if the method isn't implemented as the 
> logic surrounding when to call do_unaligned_access is contained within the 
> TCG core.

I've seen this after I've sent a patch for PPC where removing a 
conditional from a helper often called for memory access showed it had an 
effect on performance. So I thought adding a conditional here might cause 
some slow down for CPUs that don't need this. But this is compile time so 
maybe it's not that big problem as in a hepler. Yet only adding the 
unaligned method for CPUs then always set MO_ALIGN here avoiding the if 
only calls the method when needed if that works at all. I don't know if 
that's worth it, you could test with some memory intensive benchmark such 
as STREAM benchmark to check if this has any effect.

Regards,
BALATON Zotan

> I'll have a go at updating the ordering and send a v2 if it looks good.
>
>
> ATB,
>
> Mark.
>
>