From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH] xen/arm: Init traps very early Date: Wed, 28 May 2014 15:09:47 +0100 Message-ID: <5385EE2B.6030109@linaro.org> References: <1401283983-19843-1-git-send-email-julien.grall@linaro.org> <5385E956.1040104@citrix.com> <5385EB80.5050905@linaro.org> <5385ECFD.5080702@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1WpeYB-0007Tp-SM for xen-devel@lists.xenproject.org; Wed, 28 May 2014 14:09:52 +0000 Received: by mail-wi0-f173.google.com with SMTP id bs8so3862733wib.12 for ; Wed, 28 May 2014 07:09:50 -0700 (PDT) In-Reply-To: <5385ECFD.5080702@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: Andrew Cooper Cc: naresh.bhat@linaro.org, xen-devel@lists.xenproject.org, stefano.stabellini@citrix.com, ian.campbell@citrix.com, tim@xen.org List-Id: xen-devel@lists.xenproject.org On 05/28/2014 03:04 PM, Andrew Cooper wrote: > On 28/05/14 14:58, Julien Grall wrote: >> Hi Andrew, >> >> On 05/28/2014 02:49 PM, Andrew Cooper wrote: >>> On 28/05/14 14:33, Julien Grall wrote: >>>> The function init_traps setups the handler taken when Xen hits a BUG_ON/ASSERT. >>>> >>>> If an error happen before init_traps is called, we loose the backtrace. >>>> >>>> As the function doesn't require any specific setup, we can call it just >>>> after Xen has jumped in C code. >>>> >>>> Signed-off-by: Julien Grall >>>> --- >>>> xen/arch/arm/setup.c | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> >>>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c >>>> index b9ce7a9..5bf8686 100644 >>>> --- a/xen/arch/arm/setup.c >>>> +++ b/xen/arch/arm/setup.c >>>> @@ -666,6 +666,7 @@ void __init start_xen(unsigned long boot_phys_offset, >>>> struct domain *dom0; >>>> >>>> setup_cache(); >>>> + init_traps(); >>> Having recently redone this in x86, it must be after >>> set_processor_id(0), set_current() for debug sanity, and after >>> percpu_init_areas() for future proofing. >> Even though it doesn't harm to call before (the stack will be >> corrupted), I agree to call init_traps after set_processor_id(0), and >> therefore percpu_init_areas. >> >> But we don't need to have set_current correctly set up. The trap entries >> won't save anything about the guest if the exception are taken from HYP >> mode. >> > > set_current((struct vcpu *)0xfffff000); /* debug sanity */ > > This is designed to force a pagefault if a trap handler tries to use > current before current is a valid vcpu. It is a debugging measure > rather than a functional one, and does pick up issues. At this stage if we hit a trap it's mostly for a data abort, alignment issue, undefined instruction. *none* of them if using current. I prefer to move init_traps earlier and get useful call stack 95% of the time, rather than loosing it for the sack of debug sanity. It's very painful to debug when the ASSERT is in the header and can be called on multiple place. Regards, -- Julien Grall