| Message ID | 20210729004647.282017-4-richard.henderson@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Unaligned accesses for user-only | expand |
On Thu, 29 Jul 2021 at 01:47, Richard Henderson <richard.henderson@linaro.org> wrote: > > Cc: qemu-arm@nongnu.org > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/aarch64/cpu_loop.c | 4 ++++ > linux-user/arm/cpu_loop.c | 43 +++++++++++++++++++++++++++-------- > target/arm/cpu.c | 2 +- > target/arm/cpu_tcg.c | 2 +- > 4 files changed, 40 insertions(+), 11 deletions(-) > > diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c > index ee72a1c20f..998831f87f 100644 > --- a/linux-user/aarch64/cpu_loop.c > +++ b/linux-user/aarch64/cpu_loop.c > @@ -137,6 +137,10 @@ void cpu_loop(CPUARMState *env) > case 0x11: /* Synchronous Tag Check Fault */ > info.si_code = TARGET_SEGV_MTESERR; > break; > + case 0x21: /* Alignment fault */ > + info.si_signo = TARGET_SIGBUS; > + info.si_code = TARGET_BUS_ADRALN; > + break; > default: > g_assert_not_reached(); > } > diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c > index 69632d15be..da7da6a0c1 100644 > --- a/linux-user/arm/cpu_loop.c > +++ b/linux-user/arm/cpu_loop.c > @@ -23,6 +23,7 @@ > #include "elf.h" > #include "cpu_loop-common.h" > #include "semihosting/common-semi.h" > +#include "target/arm/syndrome.h" Not a huge fan of linux-user files pulling in target/arm headers, but I guess we do it already in aarch64/cpu_loop.c. (Though that is afaict the only other place ATM...) > > #define get_user_code_u32(x, gaddr, env) \ > ({ abi_long __r = get_user_u32((x), (gaddr)); \ > @@ -286,9 +287,8 @@ void cpu_loop(CPUARMState *env) > { > CPUState *cs = env_cpu(env); > int trapnr; > - unsigned int n, insn; > + unsigned int n, insn, ec, fsc; > target_siginfo_t info; > - uint32_t addr; > abi_ulong ret; > > for(;;) { > @@ -437,15 +437,40 @@ void cpu_loop(CPUARMState *env) > break; > case EXCP_PREFETCH_ABORT: > case EXCP_DATA_ABORT: > - addr = env->exception.vaddress; > - { > - info.si_signo = TARGET_SIGSEGV; > - info.si_errno = 0; > - /* XXX: check env->error_code */ > + info.si_signo = TARGET_SIGSEGV; > + info.si_errno = 0; > + info._sifields._sigfault._addr = env->exception.vaddress; > + /* > + * We should only arrive here with EC in {DATAABORT, INSNABORT}, > + * and short-form FSC, which then tells us to look at the FSR. > + * ??? arm_cpu_reset never sets TTBCR_EAE, so we always get > + * short-form FSC. > + */ > + ec = syn_get_ec(env->exception.syndrome); > + assert(ec == EC_DATAABORT || ec == EC_INSNABORT); > + fsc = extract32(env->exception.syndrome, 0, 6); > + assert(fsc == 0x3f); > + switch (env->exception.fsr & 0x1f) { > + case 0x1: /* Alignment */ > + info.si_signo = TARGET_SIGBUS; > + info.si_code = TARGET_BUS_ADRALN; > + break; > + case 0x3: /* Access flag fault, level 1 */ > + case 0x6: /* Access flag fault, level 2 */ > + case 0x9: /* Domain fault, level 1 */ > + case 0xb: /* Domain fault, level 2 */ > + case 0xd: /* Permision fault, level 1 */ > + case 0xf: /* Permision fault, level 2 */ > + info.si_code = TARGET_SEGV_ACCERR; > + break; > + case 0x5: /* Translation fault, level 1 */ > + case 0x7: /* Translation fault, level 2 */ > info.si_code = TARGET_SEGV_MAPERR; > - info._sifields._sigfault._addr = addr; > - queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info); > + break; > + default: > + g_assert_not_reached(); > } > + queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info); > break; It's slightly sad that we start off with a nicely symbolic ArmMMUFaultInfo type enum value, carefully encode it into a numeric value and then have to switch on the numeric value here, but I can see why we end up this way... Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
On 7/29/21 3:14 AM, Peter Maydell wrote: > On Thu, 29 Jul 2021 at 01:47, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> Cc: qemu-arm@nongnu.org >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> linux-user/aarch64/cpu_loop.c | 4 ++++ >> linux-user/arm/cpu_loop.c | 43 +++++++++++++++++++++++++++-------- >> target/arm/cpu.c | 2 +- >> target/arm/cpu_tcg.c | 2 +- >> 4 files changed, 40 insertions(+), 11 deletions(-) >> >> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c >> index ee72a1c20f..998831f87f 100644 >> --- a/linux-user/aarch64/cpu_loop.c >> +++ b/linux-user/aarch64/cpu_loop.c >> @@ -137,6 +137,10 @@ void cpu_loop(CPUARMState *env) >> case 0x11: /* Synchronous Tag Check Fault */ >> info.si_code = TARGET_SEGV_MTESERR; >> break; >> + case 0x21: /* Alignment fault */ >> + info.si_signo = TARGET_SIGBUS; >> + info.si_code = TARGET_BUS_ADRALN; >> + break; >> default: >> g_assert_not_reached(); >> } >> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c >> index 69632d15be..da7da6a0c1 100644 >> --- a/linux-user/arm/cpu_loop.c >> +++ b/linux-user/arm/cpu_loop.c >> @@ -23,6 +23,7 @@ >> #include "elf.h" >> #include "cpu_loop-common.h" >> #include "semihosting/common-semi.h" >> +#include "target/arm/syndrome.h" > > Not a huge fan of linux-user files pulling in target/arm headers, but > I guess we do it already in aarch64/cpu_loop.c. (Though that is afaict > the only other place ATM...) > >> >> #define get_user_code_u32(x, gaddr, env) \ >> ({ abi_long __r = get_user_u32((x), (gaddr)); \ >> @@ -286,9 +287,8 @@ void cpu_loop(CPUARMState *env) >> { >> CPUState *cs = env_cpu(env); >> int trapnr; >> - unsigned int n, insn; >> + unsigned int n, insn, ec, fsc; >> target_siginfo_t info; >> - uint32_t addr; >> abi_ulong ret; >> >> for(;;) { >> @@ -437,15 +437,40 @@ void cpu_loop(CPUARMState *env) >> break; >> case EXCP_PREFETCH_ABORT: >> case EXCP_DATA_ABORT: >> - addr = env->exception.vaddress; >> - { >> - info.si_signo = TARGET_SIGSEGV; >> - info.si_errno = 0; >> - /* XXX: check env->error_code */ >> + info.si_signo = TARGET_SIGSEGV; >> + info.si_errno = 0; >> + info._sifields._sigfault._addr = env->exception.vaddress; >> + /* >> + * We should only arrive here with EC in {DATAABORT, INSNABORT}, >> + * and short-form FSC, which then tells us to look at the FSR. >> + * ??? arm_cpu_reset never sets TTBCR_EAE, so we always get >> + * short-form FSC. >> + */ >> + ec = syn_get_ec(env->exception.syndrome); >> + assert(ec == EC_DATAABORT || ec == EC_INSNABORT); >> + fsc = extract32(env->exception.syndrome, 0, 6); >> + assert(fsc == 0x3f); >> + switch (env->exception.fsr & 0x1f) { >> + case 0x1: /* Alignment */ >> + info.si_signo = TARGET_SIGBUS; >> + info.si_code = TARGET_BUS_ADRALN; >> + break; >> + case 0x3: /* Access flag fault, level 1 */ >> + case 0x6: /* Access flag fault, level 2 */ >> + case 0x9: /* Domain fault, level 1 */ >> + case 0xb: /* Domain fault, level 2 */ >> + case 0xd: /* Permision fault, level 1 */ >> + case 0xf: /* Permision fault, level 2 */ >> + info.si_code = TARGET_SEGV_ACCERR; >> + break; >> + case 0x5: /* Translation fault, level 1 */ >> + case 0x7: /* Translation fault, level 2 */ >> info.si_code = TARGET_SEGV_MAPERR; >> - info._sifields._sigfault._addr = addr; >> - queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info); >> + break; >> + default: >> + g_assert_not_reached(); >> } >> + queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info); >> break; > > It's slightly sad that we start off with a nicely symbolic > ArmMMUFaultInfo type enum value, carefully encode it into a > numeric value and then have to switch on the numeric value here, > but I can see why we end up this way... We don't have to leave it that way. We could move the ARMMMUFaultInfo out of internals.h, create special user-only copies of arm_cpu_tlb_fill and arm_cpu_do_unaligned_access, create a new function to raise the MTE exception, and place the proper enumeraor into env->error_code instead of the hw syndrome. What we have seemed cleaner on the target/arm/ side at the time. r~
diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c index ee72a1c20f..998831f87f 100644 --- a/linux-user/aarch64/cpu_loop.c +++ b/linux-user/aarch64/cpu_loop.c @@ -137,6 +137,10 @@ void cpu_loop(CPUARMState *env) case 0x11: /* Synchronous Tag Check Fault */ info.si_code = TARGET_SEGV_MTESERR; break; + case 0x21: /* Alignment fault */ + info.si_signo = TARGET_SIGBUS; + info.si_code = TARGET_BUS_ADRALN; + break; default: g_assert_not_reached(); } diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c index 69632d15be..da7da6a0c1 100644 --- a/linux-user/arm/cpu_loop.c +++ b/linux-user/arm/cpu_loop.c @@ -23,6 +23,7 @@ #include "elf.h" #include "cpu_loop-common.h" #include "semihosting/common-semi.h" +#include "target/arm/syndrome.h" #define get_user_code_u32(x, gaddr, env) \ ({ abi_long __r = get_user_u32((x), (gaddr)); \ @@ -286,9 +287,8 @@ void cpu_loop(CPUARMState *env) { CPUState *cs = env_cpu(env); int trapnr; - unsigned int n, insn; + unsigned int n, insn, ec, fsc; target_siginfo_t info; - uint32_t addr; abi_ulong ret; for(;;) { @@ -437,15 +437,40 @@ void cpu_loop(CPUARMState *env) break; case EXCP_PREFETCH_ABORT: case EXCP_DATA_ABORT: - addr = env->exception.vaddress; - { - info.si_signo = TARGET_SIGSEGV; - info.si_errno = 0; - /* XXX: check env->error_code */ + info.si_signo = TARGET_SIGSEGV; + info.si_errno = 0; + info._sifields._sigfault._addr = env->exception.vaddress; + /* + * We should only arrive here with EC in {DATAABORT, INSNABORT}, + * and short-form FSC, which then tells us to look at the FSR. + * ??? arm_cpu_reset never sets TTBCR_EAE, so we always get + * short-form FSC. + */ + ec = syn_get_ec(env->exception.syndrome); + assert(ec == EC_DATAABORT || ec == EC_INSNABORT); + fsc = extract32(env->exception.syndrome, 0, 6); + assert(fsc == 0x3f); + switch (env->exception.fsr & 0x1f) { + case 0x1: /* Alignment */ + info.si_signo = TARGET_SIGBUS; + info.si_code = TARGET_BUS_ADRALN; + break; + case 0x3: /* Access flag fault, level 1 */ + case 0x6: /* Access flag fault, level 2 */ + case 0x9: /* Domain fault, level 1 */ + case 0xb: /* Domain fault, level 2 */ + case 0xd: /* Permision fault, level 1 */ + case 0xf: /* Permision fault, level 2 */ + info.si_code = TARGET_SEGV_ACCERR; + break; + case 0x5: /* Translation fault, level 1 */ + case 0x7: /* Translation fault, level 2 */ info.si_code = TARGET_SEGV_MAPERR; - info._sifields._sigfault._addr = addr; - queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info); + break; + default: + g_assert_not_reached(); } + queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info); break; case EXCP_DEBUG: case EXCP_BKPT: diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 2866dd7658..de0d968d76 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1987,11 +1987,11 @@ static const struct TCGCPUOps arm_tcg_ops = { .cpu_exec_interrupt = arm_cpu_exec_interrupt, .tlb_fill = arm_cpu_tlb_fill, .debug_excp_handler = arm_debug_excp_handler, + .do_unaligned_access = arm_cpu_do_unaligned_access, #if !defined(CONFIG_USER_ONLY) .do_interrupt = arm_cpu_do_interrupt, .do_transaction_failed = arm_cpu_do_transaction_failed, - .do_unaligned_access = arm_cpu_do_unaligned_access, .adjust_watchpoint_address = arm_adjust_watchpoint_address, .debug_check_watchpoint = arm_debug_check_watchpoint, .debug_check_breakpoint = arm_debug_check_breakpoint, diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c index ed444bf436..1b91fdc890 100644 --- a/target/arm/cpu_tcg.c +++ b/target/arm/cpu_tcg.c @@ -904,11 +904,11 @@ static const struct TCGCPUOps arm_v7m_tcg_ops = { .cpu_exec_interrupt = arm_v7m_cpu_exec_interrupt, .tlb_fill = arm_cpu_tlb_fill, .debug_excp_handler = arm_debug_excp_handler, + .do_unaligned_access = arm_cpu_do_unaligned_access, #if !defined(CONFIG_USER_ONLY) .do_interrupt = arm_v7m_cpu_do_interrupt, .do_transaction_failed = arm_cpu_do_transaction_failed, - .do_unaligned_access = arm_cpu_do_unaligned_access, .adjust_watchpoint_address = arm_adjust_watchpoint_address, .debug_check_watchpoint = arm_debug_check_watchpoint, .debug_check_breakpoint = arm_debug_check_breakpoint,
Cc: qemu-arm@nongnu.org Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/aarch64/cpu_loop.c | 4 ++++ linux-user/arm/cpu_loop.c | 43 +++++++++++++++++++++++++++-------- target/arm/cpu.c | 2 +- target/arm/cpu_tcg.c | 2 +- 4 files changed, 40 insertions(+), 11 deletions(-)