[Qemu-devel] [PATCH 0/2] Fix qemu-system-aarch64 crash

Richard Henderson posted 2 patches 7 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180630000242.29594-1-richard.henderson@linaro.org
Test checkpatch passed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
include/qom/cpu.h | 6 ++++--
target/arm/cpu.h  | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH 0/2] Fix qemu-system-aarch64 crash
Posted by Richard Henderson 7 years, 7 months ago
The sequence of events was
  (1) Kernel executed a disabled sve insn,
  (2) Undefined Instruction trap went to EL3,
  (3) Lookup of the exception handler saw el3 and returned asidx 1,
  (4) Which hadn't been set up.

So there's definitely a bug with SVE exception routing.
That said...

With just the first patch, the kernel goes into a silly exception loop
which is understandable.  With just the second patch, qemu gets SIGABRT
instead of SIGSEGV, which is definitely easier to debug.

I think I'm in favor of both patches, but you might say we shouldn't
have to have the first one and just apply the second.


r~


Richard Henderson (2):
  target/arm: Always return ARMASIdx_NS when num_ases == 1
  cpu: Assert asidx_from_attrs return value in range

 include/qom/cpu.h | 6 ++++--
 target/arm/cpu.h  | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

-- 
2.17.1


Re: [Qemu-devel] [PATCH 0/2] Fix qemu-system-aarch64 crash
Posted by Peter Maydell 7 years, 7 months ago
On 30 June 2018 at 01:02, Richard Henderson
<richard.henderson@linaro.org> wrote:
> The sequence of events was
>   (1) Kernel executed a disabled sve insn,
>   (2) Undefined Instruction trap went to EL3,
>   (3) Lookup of the exception handler saw el3 and returned asidx 1,
>   (4) Which hadn't been set up.
>
> So there's definitely a bug with SVE exception routing.
> That said...
>
> With just the first patch, the kernel goes into a silly exception loop
> which is understandable.  With just the second patch, qemu gets SIGABRT
> instead of SIGSEGV, which is definitely easier to debug.
>
> I think I'm in favor of both patches, but you might say we shouldn't
> have to have the first one and just apply the second.

I think my vote is for just the second -- a CPU without the
security extensions should never be emitting transactions
with attrs.secure true, so that's a bug we want to track down.
Suitably placed assert()s do a better job of that than sweeping
the problem under the carpet by squashing the attributes
in arm_asidx_from_attrs().

thanks
-- PMM