Re: [PATCH] xen: arm: inject unhandled instruction and data aborts to the guest.

[Julien Grall <julien.grall@linaro.org>]

On 12/09/2013 02:58 PM, Ian Campbell wrote:
> Currently an unhandled data abort in guest context leads to us killing the
> guest and an unhandled instruction abort in guest context leads to us killing
> the host!
> 
> Andre pointed out that an unhandled data abort can be caused by e.g. dmidecode
> looking for things which are not there in the guests physical address space.
> Propagating the fault to the guest allows it to properly SIGSEGV the
> processes.
> 
> A guest kernel can trivially jump to an unmapped physical address which would
> cause an instruction abort. Killing the host for that is obviously bad.
> Instead inject the exception so the guest kernel can SIGSEGV or panic() etc as
> it deems appropriate.
> 
> Tested on arm64 (Mustang) and arm32 (Midway) with a dom0 kernel late_initcall
> which either dereferences or jumps to address 0, provoking both behaviours and
> resulting correctly in a guest kernel panic. Also tested on fast models with a
> 32-bit dom0 on a 64-bit hypervisor, which behaved correctly.
> 
> In addition tested on both platforms with a userspace program which either
> calls to or dereferences address 0. The process is correctly killed with SEGV.
> 
> Lastly tested on Mustang with a 32-bit version of the userspace test on a
> 64-bit dom0 kernel.
> 
> I think that covers all the cases.
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
> Cc: Andre Przywara <andre.przywara@calxeda.com>
> ---
> Release wise this is a(n important) bug fix.
> ---
>  xen/arch/arm/traps.c            |  199 +++++++++++++++++++++++++++++++++------
>  xen/include/asm-arm/cpregs.h    |    1 +
>  xen/include/asm-arm/processor.h |   15 ++-
>  xen/include/asm-arm/regs.h      |    2 +
>  4 files changed, 184 insertions(+), 33 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 2a85d37..e4e7f83 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -264,6 +264,8 @@ static void cpsr_switch_mode(struct cpu_user_regs *regs, int mode)
>  
>      regs->cpsr |= mode;
>      regs->cpsr |= PSR_IRQ_MASK;
> +    if (mode == PSR_MODE_ABT)

if ( ... )

The patch looks good to me:
Acked-by: Julien Grall <julien.grall@linaro.org>


-- 
Julien Grall