diff mbox series

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

Message ID 20240623115704.315645-2-mark.cave-ayland@ilande.co.uk (mailing list archive)
State New, archived
Headers show
Series target/m68k: implement unaligned accesses for m68k CPUs | expand

Commit Message

Mark Cave-Ayland June 23, 2024, 11:57 a.m. UTC
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(+)

Comments

BALATON Zoltan June 23, 2024, 3:11 p.m. UTC | #1
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];
>
Richard Henderson June 23, 2024, 7:47 p.m. UTC | #2
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~
Mark Cave-Ayland June 24, 2024, 5:41 a.m. UTC | #3
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.
Mark Cave-Ayland June 24, 2024, 5:55 a.m. UTC | #4
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.
diff mbox series

Patch

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