Hi,
On 16/02/2021 17:16, Peter Maydell wrote:
> From: Richard Henderson <richard.hender...@linaro.org>
>
> A proper syndrome is required to fill in the proper si_code.
> Use page_get_flags to determine permission vs translation for user-only.
>
> Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> Message-id: 20210212184902.1251044-27-richard.hender...@linaro.org
> Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> ---
> linux-user/aarch64/cpu_loop.c | 24 +++++++++++++++++++++---
> target/arm/tlb_helper.c | 15 +++++++++------
> 2 files changed, 30 insertions(+), 9 deletions(-)
While I was testing my next linux-user pull request I found this patch breaks
something.
Following LTP tests are broken:
mmap05
mprotect02
mprotect03
mprotect04
shmat01
with arm64/sid, arm64/trusty, arm64/bionic
Bisecting only using mmap05 test I find this patch.
Symptoms are:
$ sudo unshare --time --ipc --uts --pid --fork --kill-child --mount
--mount-proc --root
chroot/arm64/sid /opt/ltp/testcases/bin/mmap05
**
ERROR:../../../Projects/qemu/linux-user/aarch64/cpu_loop.c:141:cpu_loop: code
should not
be reached
Bail out!
ERROR:../../../Projects/qemu/linux-user/aarch64/cpu_loop.c:141:cpu_loop: code
should not be reached
qemu:handle_cpu_signal received signal outside vCPU context @ pc=0x7f45c1cd9706
Expected result is:
mmap05 1 TPASS : Got SIGSEGV as expected
Thanks,
Laurent
>
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index 42b9c15f536..4e43906e66a 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -23,6 +23,7 @@
> #include "cpu_loop-common.h"
> #include "qemu/guest-random.h"
> #include "hw/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)); \
> @@ -76,7 +77,7 @@
> void cpu_loop(CPUARMState *env)
> {
> CPUState *cs = env_cpu(env);
> - int trapnr;
> + int trapnr, ec, fsc;
> abi_long ret;
> target_siginfo_t info;
>
> @@ -117,9 +118,26 @@ void cpu_loop(CPUARMState *env)
> case EXCP_DATA_ABORT:
> info.si_signo = TARGET_SIGSEGV;
> info.si_errno = 0;
> - /* XXX: check env->error_code */
> - info.si_code = TARGET_SEGV_MAPERR;
> info._sifields._sigfault._addr = env->exception.vaddress;
> +
> + /* We should only arrive here with EC in {DATAABORT, INSNABORT}.
> */
> + ec = syn_get_ec(env->exception.syndrome);
> + assert(ec == EC_DATAABORT || ec == EC_INSNABORT);
> +
> + /* Both EC have the same format for FSC, or close enough. */
> + fsc = extract32(env->exception.syndrome, 0, 6);
> + switch (fsc) {
> + case 0x04 ... 0x07: /* Translation fault, level {0-3} */
> + info.si_code = TARGET_SEGV_MAPERR;
> + break;
> + case 0x09 ... 0x0b: /* Access flag fault, level {1-3} */
> + case 0x0d ... 0x0f: /* Permission fault, level {1-3} */
> + info.si_code = TARGET_SEGV_ACCERR;
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +
> queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> break;
> case EXCP_DEBUG:
> diff --git a/target/arm/tlb_helper.c b/target/arm/tlb_helper.c
> index df85079d9f0..9609333cbdf 100644
> --- a/target/arm/tlb_helper.c
> +++ b/target/arm/tlb_helper.c
> @@ -154,21 +154,24 @@ bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int
> size,
> bool probe, uintptr_t retaddr)
> {
> ARMCPU *cpu = ARM_CPU(cs);
> + ARMMMUFaultInfo fi = {};
>
> #ifdef CONFIG_USER_ONLY
> - cpu->env.exception.vaddress = address;
> - if (access_type == MMU_INST_FETCH) {
> - cs->exception_index = EXCP_PREFETCH_ABORT;
> + int flags = page_get_flags(useronly_clean_ptr(address));
> + if (flags & PAGE_VALID) {
> + fi.type = ARMFault_Permission;
> } else {
> - cs->exception_index = EXCP_DATA_ABORT;
> + fi.type = ARMFault_Translation;
> }
> - cpu_loop_exit_restore(cs, retaddr);
> +
> + /* now we have a real cpu fault */
> + cpu_restore_state(cs, retaddr, true);
> + arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi);
> #else
> hwaddr phys_addr;
> target_ulong page_size;
> int prot, ret;
> MemTxAttrs attrs = {};
> - ARMMMUFaultInfo fi = {};
> ARMCacheAttrs cacheattrs = {};
>
> /*
>
