Re: [PATCH rfc] hvf: arm: Inject SEA when executing insn in invalid memory range

["Alex Bennée" <alex.bennee@linaro.org>]

Zenghui Yu <zenghui.yu@linux.dev> writes:

> It seems that hvf doesn't deal with the abort generated when guest tries to
> execute instructions outside of the valid physical memory range, for
> unknown reason. The abort is forwarded to userspace and QEMU doesn't handle
> it either, which ends up with faulting on the same instruction infinitely.
>
> This was noticed by the kvm-unit-tests/selftest-vectors-kernel failure:
>
>   timeout -k 1s --foreground 90s /opt/homebrew/bin/qemu-system-aarch64 \
>     -nodefaults -machine virt -accel hvf -cpu host \
>     -device virtio-serial-device -device virtconsole,chardev=ctd \
>     -chardev testdev,id=ctd -device pci-testdev -display none \
>     -serial stdio -kernel arm/selftest.flat -smp 1 -append
>   vectors-kernel

Have you got patches for teaching kvm-unit-tests about hvf or are you
running these all manually? I tried building on the Mac we have but it
failed the build and the docs only mention x86.

>
>   PASS: selftest: vectors-kernel: und
>   PASS: selftest: vectors-kernel: svc
>   qemu-system-aarch64: 0xffffc000: unhandled exception ec=0x20
>   qemu-system-aarch64: 0xffffc000: unhandled exception ec=0x20
>   qemu-system-aarch64: 0xffffc000: unhandled exception ec=0x20

I think this is running:

  static bool check_pabt(void)
  {
          enum vector v = check_vector_prep();

          install_exception_handler(v, ESR_EL1_EC_IABT_EL1, pabt_handler);

          test_exception("adrp	x9, check_pabt_invalid_paddr\n"
                         "add	x9, x9, :lo12:check_pabt_invalid_paddr\n"
                         "ldr	x9, [x9]\n",
                         "blr	x9\n",
                         "", "x9", "x30");

          install_exception_handler(v, ESR_EL1_EC_IABT_EL1, NULL);

          return pabt_works;
  }

which is expecting 0x21 - instruction abort at the same exception level.
I wonder why there is the difference.


>   [...]
>
> It's apparent that the guest is braindead and it's unsure what prevents hvf
> from injecting an abort directly in that case. Try to deal with the insane
> guest in QEMU by injecting an SEA back into it in the EC_INSNABORT
> emulation path.
>
> Signed-off-by: Zenghui Yu <zenghui.yu@linux.dev>
> ---
>  target/arm/hvf/hvf.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index aabc7d32c1..54d6ea469c 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -2332,9 +2332,32 @@ static int hvf_handle_exception(CPUState *cpu, hv_vcpu_exit_exception_t *excp)
>          bool ea = (syndrome >> 9) & 1;
>          bool s1ptw = (syndrome >> 7) & 1;
>          uint32_t ifsc = (syndrome >> 0) & 0x3f;
> +        uint64_t ipa = excp->physical_address;
> +        AddressSpace *as = cpu_get_address_space(cpu, ARMASIdx_NS);
> +        hwaddr xlat;
> +        MemoryRegion *mr;
> +
> +        cpu_synchronize_state(cpu);
>  
>          trace_hvf_insn_abort(env->pc, set, fnv, ea, s1ptw, ifsc);
>  
> +        /*
> +         * TODO: If s1ptw, this is an error in the guest os page tables.
> +         * Inject the exception into the guest.
> +         */
> +        assert(!s1ptw);
> +
> +        mr = address_space_translate(as, ipa, &xlat, NULL, false,
> +                                     MEMTXATTRS_UNSPECIFIED);
> +        if (unlikely(!memory_region_is_ram(mr))) {
> +            uint32_t syn;
> +
> +            /* inject an SEA back into the guest */
> +            syn = syn_insn_abort(arm_current_el(env) == 1, ea, false, 0x10);
> +            hvf_raise_exception(cpu, EXCP_PREFETCH_ABORT, syn, 1);
> +            break;
> +        }
> +
>          /* fall through */
>      }
>      default:

I need the check the exception paths for KVM and TCG. I guess for the
KVM case it is all in the kernel?

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro