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

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(+)

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];
>

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~

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.

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.