From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v2] xen/arm: Correctly support WARN_ON Date: Mon, 08 Sep 2014 12:19:04 -0700 Message-ID: <540E0128.8070506@linaro.org> References: <1407256336-11839-1-git-send-email-julien.grall@linaro.org> <1410181281.3680.3.camel@kazak.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XR4Sz-0002vv-SY for xen-devel@lists.xenproject.org; Mon, 08 Sep 2014 19:19:09 +0000 Received: by mail-qc0-f180.google.com with SMTP id c9so15825937qcz.25 for ; Mon, 08 Sep 2014 12:19:06 -0700 (PDT) In-Reply-To: <1410181281.3680.3.camel@kazak.uk.xensource.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: xen-devel@lists.xenproject.org, tim@xen.org, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org Hi Ian, On 08/09/14 06:01, Ian Campbell wrote: >> + /* PC is always 4-byte align, as Xen is using ARM instruction set */ > > "aligned" Ok. > Is it worth a check here? I presume the nested fault if PC were > misaligned would be pretty exciting, print+goto die would seem > appropriate. I don't think so. The "undefined instruction" is only used when the processor is unable to decode the instruction. If it fails to load the instruction (because a wrong memory address, pc misaligned), Xen would have received a "prefetch abort". >> + instr = *((uint32_t *)regs->pc); >> + if ( instr != BUG_OPCODE ) >> + goto die; >> + >> + if ( do_bug_frame(regs, regs->pc) ) >> + goto die; >> + >> + regs->pc += 4; >> + return; >> + >> +die: >> + do_unexpected_trap("undefined instruction", regs); > > No need to change the case of this message. Ok. >> }> +#ifdef CONFIG_ARM_64 >> +static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr) >> +{ >> + /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive >> + * software breakpoint exception for EL1 and EL0 here >> + */ > > BUG_ON? ;-O Sounds a good plan. >> +die: >> + do_unexpected_trap("undefined breakpoint value", regs); > > Please can you capitilise this to match the other callers. Will do. > >> +/* Many version of GCC doesn't support the asm %c parameter which would > > "versions". > >> + * be preferable to this unpleasantness. We use mergeable string >> + * sections to avoid multiple copies of the string appearing in the >> + * Xen image. > > OOI what is the size increase of the final (stripped) binary with this patch? I compared the file xen/xen and on both arm32/arm64 the binary is smaller of about 2%. I think this is because previously gcc wasn't merge the string when BUG_ON was used in inline function. Regards, -- Julien Grall