From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH] xen: arm: inject unhandled instruction and data aborts to the guest. Date: Mon, 09 Dec 2013 15:57:55 +0000 Message-ID: <52A5E883.8090109@linaro.org> References: <1386601104-10021-1-git-send-email-ian.campbell@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1386601104-10021-1-git-send-email-ian.campbell@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Andre Przywara , stefano.stabellini@eu.citrix.com, tim@xen.org, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.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 > Cc: Andre Przywara > --- > 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