[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.2 03/43] target/arm: Implement do_unaligned_access for
From: |
Peter Maydell |
Subject: |
Re: [PATCH for-6.2 03/43] target/arm: Implement do_unaligned_access for user-only |
Date: |
Thu, 29 Jul 2021 14:14:16 +0100 |
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