From patchwork Tue Mar 20 12:43:48 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marc Zyngier X-Patchwork-Id: 10297017 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 97E10602B3 for ; Tue, 20 Mar 2018 12:44:30 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 866542845E for ; Tue, 20 Mar 2018 12:44:30 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7917228D21; Tue, 20 Mar 2018 12:44:30 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id A42D52845E for ; Tue, 20 Mar 2018 12:44:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=g+M+TfQ0jGP5JyBSS875IR2d8WTAon6wgKQZWKRcly0=; b=VLsZ0oXSjVIaXl 3w3WGaFvV2Aa2BvHAWwTSmq7j99ic7lg9lmkSauQbHM70x9IwC2iD47/X76kaTVlAk3rFUAW7QXH8 02j9bv3MUmvNKu2jYLt0+DXLfWYmBAVerwwLrebiDXcbAzpPj5l58E1EEjzvj3ZBtAYjHq58YxR7S CnyOGtz6+2I+gIkq14Rn8O8xagIm5lmYep7b/X7s5v00IBEiy8Vw4Az5pGgbtdifM3tWeuz+m6qLV Jr0ITnGaSbFNrj2Ace93Syx4CDNNr7J3DM1CmZoeWSN8qbZ+n/ZhnEnUB3UrI3cnA5DW7Te8D7YF4 40mhjCdYVAXnplZ4cFgg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1eyGcs-0007dY-5X; Tue, 20 Mar 2018 12:44:26 +0000 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70] helo=foss.arm.com) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1eyGcV-0007NZ-2f; Tue, 20 Mar 2018 12:44:05 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8565D1529; Tue, 20 Mar 2018 05:43:52 -0700 (PDT) Received: from [10.1.206.75] (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 95A5F3F24A; Tue, 20 Mar 2018 05:43:51 -0700 (PDT) Subject: Re: Failed to boot ARM64 boards for recent linux-next To: Shawn Lin References: <077cecdd-7982-dd33-454f-e38cc571366c@arm.com> <9cff515e-fcb9-a1c0-329b-a18c3bed0071@rock-chips.com> <2dfe4f47-8163-d7a4-c9cc-d60b9f0e71a8@arm.com> <1d4d0797-1630-b09d-943d-1a1cac178700@rock-chips.com> From: Marc Zyngier Organization: ARM Ltd Message-ID: <46f5f8b2-44bb-d1ab-4949-9cc4a96a7e1b@arm.com> Date: Tue, 20 Mar 2018 12:43:48 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <1d4d0797-1630-b09d-943d-1a1cac178700@rock-chips.com> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180320_054403_265559_3CCA47ED X-CRM114-Status: GOOD ( 32.46 ) X-BeenThere: linux-rockchip@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: Upstream kernel work for Rockchip platforms List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "open list:ARM/Rockchip SoC..." , Jeffy Chen , Robin Murphy , "linux-arm-kernel@lists.infradead.org" Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+patchwork-linux-rockchip=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 20/03/18 10:06, Shawn Lin wrote: > Hi Marc, > > 在 2018/3/20 17:56, Marc Zyngier 写道: >> On 20/03/18 09:32, Shawn Lin wrote: >>> Hi Marc, >>> >>> On 2018/3/20 17:01, Marc Zyngier wrote: >>>> Hi Shawn, >>>> >>>> On 20/03/18 08:48, Shawn Lin wrote: >>>>> Hi Marc, >>>>> >>>>> I was able to boot my RK3399 board with in linux-next-20180314, >>>>> but not today. My bisect robot shows me it was introduced by >>>>> >>>>> commit d6062a6d62c643a06c393745d032da3e6441d4bd >>>>> Author: Marc Zyngier >>>>> Date: Fri Mar 9 14:53:19 2018 +0000 >>>>> >>>>> irqchip/gic-v3: Reset APgRn registers at boot time >>>>> >>>>> Booting a crash kernel while in an interrupt handler is likely >>>>> to leave the Active Priority Registers with some state that >>>>> is not relevant to the new kernel, and is likely to lead >>>>> to erratic behaviours such as interrupts not firing as their >>>>> priority is already active. >>>>> >>>>> As a sanity measure, wipe the APRs clean on startup. We make >>>>> sure to wipe both group 0 and 1 registers in order to avoid >>>>> any surprise. >>>>> >>>>> >>>>> The panic log is here: >>>>> https://paste.ubuntu.com/p/7WrJJDG6JQ/ >>>>> >>>>> Is it a known issue or is there a coming patch for that? >>>> >>>> Interesting. No, that wasn't the intention, but I may have missed a key >>>> detail (group 0 access traps to EL3 if SCR_EL3.FIQ==1). Can you have a >>>> go at the following hack, just to narrow it down: >>>> >>>> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c >>>> index 5bb7bb22f1c1..f8ff43b1d4f8 100644 >>>> --- a/drivers/irqchip/irq-gic-v3.c >>>> +++ b/drivers/irqchip/irq-gic-v3.c >>>> @@ -570,16 +570,12 @@ static void gic_cpu_sys_reg_init(void) >>>> switch(val + 1) { >>>> case 8: >>>> case 7: >>>> - write_gicreg(0, ICC_AP0R3_EL1); >>>> write_gicreg(0, ICC_AP1R3_EL1); >>>> - write_gicreg(0, ICC_AP0R2_EL1); >>>> write_gicreg(0, ICC_AP1R2_EL1); >>>> case 6: >>>> - write_gicreg(0, ICC_AP0R1_EL1); >>>> write_gicreg(0, ICC_AP1R1_EL1); >>>> case 5: >>>> case 4: >>>> - write_gicreg(0, ICC_AP0R0_EL1); >>>> write_gicreg(0, ICC_AP1R0_EL1); >>>> } >>>> >>>> Let me know if that helps. >>>> >>> >>> It works for me. Thanks! >> OK. Would you mind testing a much more complete patch? > > Hmm.. the more complete patch doesn't work for me. Right. Slightly more complicated than I though. This new one should be much better (or at least I hope...). Please let me know if that helps. M. From f5cca850375f22d850816d86747a5e89e67344b0 Mon Sep 17 00:00:00 2001 From: Marc Zyngier Date: Tue, 20 Mar 2018 09:46:42 +0000 Subject: [PATCH] irqchip/gic-v3: Check availability of Group0 before resetting AP0Rn We now try to reset the Active Priority Registers at boot time, without checking if we actually have access to them. Bad move. If the secure side has set SCR_EL3.FIQ=1, we'll trap to EL3, and the firmware may not be please to get such an exception. Instead, let's use PMR to find out if its value gets affected by SCR_EL3.FIQ being set. We use the fact that when SCR_EL3.FIQ is set, the LSB of the priority is lost due to the shifting back and forth of the actual priority. If we read back a 0, we know that Group0 is unavailable. Fixes: d6062a6d62c6 ("irqchip/gic-v3: Reset APgRn registers at boot time") Reported-by: Shawn Lin Signed-off-by: Marc Zyngier --- arch/arm/include/asm/arch_gicv3.h | 6 +---- arch/arm64/include/asm/arch_gicv3.h | 5 ---- drivers/irqchip/irq-gic-v3.c | 48 +++++++++++++++++++++++++++++-------- 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/arch/arm/include/asm/arch_gicv3.h b/arch/arm/include/asm/arch_gicv3.h index 27288bdbd840..0bd530702118 100644 --- a/arch/arm/include/asm/arch_gicv3.h +++ b/arch/arm/include/asm/arch_gicv3.h @@ -137,6 +137,7 @@ static inline u64 read_ ## a64(void) \ return val; \ } +CPUIF_MAP(ICC_PMR, ICC_PMR_EL1) CPUIF_MAP(ICC_AP0R0, ICC_AP0R0_EL1) CPUIF_MAP(ICC_AP0R1, ICC_AP0R1_EL1) CPUIF_MAP(ICC_AP0R2, ICC_AP0R2_EL1) @@ -206,11 +207,6 @@ static inline u32 gic_read_iar(void) return irqstat; } -static inline void gic_write_pmr(u32 val) -{ - write_sysreg(val, ICC_PMR); -} - static inline void gic_write_ctlr(u32 val) { write_sysreg(val, ICC_CTLR); diff --git a/arch/arm64/include/asm/arch_gicv3.h b/arch/arm64/include/asm/arch_gicv3.h index 9becba9ab392..e278f94df0c9 100644 --- a/arch/arm64/include/asm/arch_gicv3.h +++ b/arch/arm64/include/asm/arch_gicv3.h @@ -76,11 +76,6 @@ static inline u64 gic_read_iar_cavium_thunderx(void) return irqstat; } -static inline void gic_write_pmr(u32 val) -{ - write_sysreg_s(val, SYS_ICC_PMR_EL1); -} - static inline void gic_write_ctlr(u32 val) { write_sysreg_s(val, SYS_ICC_CTLR_EL1); diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 5bb7bb22f1c1..62c6ea4523b8 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -532,7 +532,8 @@ static void gic_cpu_sys_reg_init(void) int i, cpu = smp_processor_id(); u64 mpidr = cpu_logical_map(cpu); u64 need_rss = MPIDR_RS(mpidr); - u32 val; + bool group0; + u32 val, pribits; /* * Need to check that the SRE bit has actually been set. If @@ -544,8 +545,28 @@ static void gic_cpu_sys_reg_init(void) if (!gic_enable_sre()) pr_err("GIC: unable to set SRE (disabled at EL2), panic ahead\n"); + pribits = gic_read_ctlr(); + pribits &= ICC_CTLR_EL1_PRI_BITS_MASK; + pribits >>= ICC_CTLR_EL1_PRI_BITS_SHIFT; + pribits++; + + /* + * Let's find out if Group0 is under control of EL3 or not by + * setting the highest possible, non-zero priority in PMR. + * + * If SCR_EL3.FIQ is set, the priority gets shifted down in + * order for the CPU interface to set bit 7, and keep the + * actual priority in the non-secure range. In the process, it + * looses the least significant bit and the actual priority + * becomes 0x80. Reading it back returns 0, indicating that + * we're don't have access to Group0. + */ + write_gicreg(BIT(8 - pribits), ICC_PMR_EL1); + val = read_gicreg(ICC_PMR_EL1); + group0 = val != 0; + /* Set priority mask register */ - gic_write_pmr(DEFAULT_PMR_VALUE); + write_gicreg(DEFAULT_PMR_VALUE, ICC_PMR_EL1); /* * Some firmwares hand over to the kernel with the BPR changed from @@ -563,23 +584,30 @@ static void gic_cpu_sys_reg_init(void) gic_write_ctlr(ICC_CTLR_EL1_EOImode_drop_dir); } - val = gic_read_ctlr(); - val &= ICC_CTLR_EL1_PRI_BITS_MASK; - val >>= ICC_CTLR_EL1_PRI_BITS_SHIFT; + /* Always wack Group0 before Group1 */ + if (group0) { + switch(pribits) { + case 8: + case 7: + write_gicreg(0, ICC_AP0R3_EL1); + write_gicreg(0, ICC_AP0R2_EL1); + case 6: + write_gicreg(0, ICC_AP0R1_EL1); + case 5: + case 4: + write_gicreg(0, ICC_AP0R0_EL1); + } + } - switch(val + 1) { + switch(pribits) { case 8: case 7: - write_gicreg(0, ICC_AP0R3_EL1); write_gicreg(0, ICC_AP1R3_EL1); - write_gicreg(0, ICC_AP0R2_EL1); write_gicreg(0, ICC_AP1R2_EL1); case 6: - write_gicreg(0, ICC_AP0R1_EL1); write_gicreg(0, ICC_AP1R1_EL1); case 5: case 4: - write_gicreg(0, ICC_AP0R0_EL1); write_gicreg(0, ICC_AP1R0_EL1); }