From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3C62AC433E1 for ; Wed, 26 Aug 2020 08:43:14 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F07F720707 for ; Wed, 26 Aug 2020 08:43:13 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="Abh5Z7to"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=alien8.de header.i=@alien8.de header.b="bkN50tGU" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org F07F720707 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=alien8.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.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:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=W7HD45wuL/oC5gmSZc0cgDQU2HsCad1ximuviLYqCmY=; b=Abh5Z7toHv+ZzTgBwYasNgm3k ODOSP5YafqtPeDj24+BFHe+P6G8ltwd1TI3ahXzLBgiYwJ/bfvTl1SAKbZv/A3Le/7ZFTS3GbfH+Q /RxzbEzBGB87l/EG3rJ1ztM4Of6PtjaqTnoSh8l0ighfmGwsvEhq2cH6Tr+yAXqpoX3epkB6pfuAq b3T2y4UUlA8qwse9+ze7FWkRAWpmRuoSSsEuLknbiCnkIs4wThyYwaCfkxH3nNdxW1dRw4A2Z4HFs yoykoYd6x4Q+/rbcPD+Z2iWow/swW1Sb8dN2R8qteLN0j5ZqkuGBQlmhK5EfJOjKfjz7kt7PJdFL5 yyRe78xUg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kAr07-0007Re-7n; Wed, 26 Aug 2020 08:41:47 +0000 Received: from mail.skyhub.de ([5.9.137.197]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kAr03-0007QS-91 for linux-arm-kernel@lists.infradead.org; Wed, 26 Aug 2020 08:41:45 +0000 Received: from zn.tnic (p200300ec2f0cee0024a26a8aa2aa63ac.dip0.t-ipconnect.de [IPv6:2003:ec:2f0c:ee00:24a2:6a8a:a2aa:63ac]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.skyhub.de (SuperMail on ZX Spectrum 128k) with ESMTPSA id 217AC1EC0301; Wed, 26 Aug 2020 10:41:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alien8.de; s=dkim; t=1598431300; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:in-reply-to:in-reply-to: references:references; bh=ZE7uC6CHQpyqIIxTbxn2FFeWPLzDl3FgM/mrD0u5pCU=; b=bkN50tGUkGAgffHjLGrW+4LjYaJ8cW/UveGXB6ARooAPae1UOhHppKsn+NGMis0EToMYcl GzoHU8jq7tVV87BEwDAoBRRy1rMQcoPwvLhggS2g2pH+K5yc/WFcfEVCt5YrRR5JPQpUfr 3zHUd/7KKb4BlO0G4qf558YFKmswLXU= Date: Wed, 26 Aug 2020 10:41:35 +0200 From: Borislav Petkov To: Sascha Hauer , James Morse Subject: Re: [PATCH 1/2] drivers/edac: Add L1 and L2 error detection for A53 and A57 Message-ID: <20200826084135.GA22390@zn.tnic> References: <20200813075721.27981-1-s.hauer@pengutronix.de> <20200813075721.27981-2-s.hauer@pengutronix.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20200813075721.27981-2-s.hauer@pengutronix.de> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200826_044143_454114_6AD8A964 X-CRM114-Status: GOOD ( 24.54 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Tony Luck , Robert Richter , kernel@pengutronix.de, York Sun , Mauro Carvalho Chehab , linux-arm-kernel@lists.infradead.org, linux-edac@vger.kernel.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Aug 13, 2020 at 09:57:20AM +0200, Sascha Hauer wrote: > The Cortex A53 and A57 cores have error detection capabilities for the > L1/L2 Caches, this patch adds a driver for them. > > Unfortunately there is no robust way to inject errors into the caches, > so this driver doesn't contain any code to actually test it. It has > been tested though with code taken from an older version of this driver > found here: https://lkml.org/lkml/2018/3/14/1203. For reasons stated > in this thread the error injection code is not suitable for mainline, > so it is removed from the driver. > > Signed-off-by: Sascha Hauer > --- > .../bindings/edac/arm,cortex-a5x-edac.yaml | 32 +++ > drivers/edac/Kconfig | 6 + > drivers/edac/Makefile | 1 + > drivers/edac/cortex_arm64_l1_l2.c | 208 ++++++++++++++++++ > 4 files changed, 247 insertions(+) > create mode 100644 Documentation/devicetree/bindings/edac/arm,cortex-a5x-edac.yaml > create mode 100644 drivers/edac/cortex_arm64_l1_l2.c Just nitpicks below. James'd need to look at this too before it goes anywhere. Checkpatch is trying to tell me something here: WARNING: DT compatible string "arm,cortex-a53-edac" appears un-documented -- check ./Documentation/devicetree/bindings/ #296: FILE: drivers/edac/cortex_arm64_l1_l2.c:190: + { .compatible = "arm,cortex-a53-edac" }, WARNING: DT compatible string "arm,cortex-a57-edac" appears un-documented -- check ./Documentation/devicetree/bindings/ #297: FILE: drivers/edac/cortex_arm64_l1_l2.c:191: + { .compatible = "arm,cortex-a57-edac" }, for 2/2 too: WARNING: DT compatible string "arm,cortex-a53-edac" appears un-documented -- check ./Documentation/devicetree/bindings/ #39: FILE: arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi:842: + compatible = "arm,cortex-a53-edac"; WARNING: DT compatible string "arm,cortex-a57-edac" appears un-documented -- check ./Documentation/devicetree/bindings/ #56: FILE: arch/arm64/boot/dts/freescale/fsl-ls1046a.dtsi:805: + compatible = "arm,cortex-a57-edac"; False positive or valid? ... > +static void read_errors(void *data) > +{ > + u64 cpumerr, l2merr; > + int cpu = smp_processor_id(); > + char msg[MESSAGE_SIZE]; > + struct edac_device_ctl_info *edac_ctl = data; Please sort function local variables declaration in a reverse christmas tree order: longest_variable_name; shorter_var_name; even_shorter; i; Check your other functions too pls. > + /* cpumerrsr_el1 */ > + asm volatile("mrs %0, s3_1_c15_c2_2" : "=r" (cpumerr)); > + asm volatile("msr s3_1_c15_c2_2, %0" :: "r" (0)); > + > + if (cpumerr & CPUMERRSR_EL1_VALID) { > + const char *str; > + int fatal = (cpumerr & CPUMERRSR_EL1_FATAL) != 0; Don't need "!= 0" and fatal can be bool. > + switch (FIELD_GET(CPUMERRSR_EL1_RAMID, cpumerr)) { > + case L1_I_TAG_RAM: > + str = "L1-I Tag RAM"; > + break; > + case L1_I_DATA_RAM: > + str = "L1-I Data RAM"; > + break; > + case L1_D_TAG_RAM: > + str = "L1-D Tag RAM"; > + break; > + case L1_D_DATA_RAM: > + str = "L1-D Data RAM"; > + break; > + case L1_D_DIRTY_RAM: > + str = "L1 Dirty RAM"; > + break; > + case TLB_RAM: > + str = "TLB RAM"; > + break; > + default: > + str = "unknown"; > + break; > + } > + > + snprintf(msg, MESSAGE_SIZE, "%s %s error(s) on CPU %d", > + str, fatal ? "fatal" : "correctable", cpu); > + > + if (fatal) > + edac_device_handle_ue(edac_ctl, 0, 0, msg); > + else > + edac_device_handle_ce(edac_ctl, 0, 0, msg); > + } > + > + /* l2merrsr_el1 */ > + asm volatile("mrs %0, s3_1_c15_c2_3" : "=r" (l2merr)); > + asm volatile("msr s3_1_c15_c2_3, %0" :: "r" (0)); > + > + if (l2merr & L2MERRSR_EL1_VALID) { > + int fatal = (l2merr & L2MERRSR_EL1_FATAL) != 0; See above. > + > + snprintf(msg, MESSAGE_SIZE, "L2 %s error(s) on CPU %d", > + fatal ? "fatal" : "correctable", cpu); > + if (fatal) > + edac_device_handle_ue(edac_ctl, 0, 1, msg); > + else > + edac_device_handle_ce(edac_ctl, 0, 1, msg); > + } > +} ... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel