[v2,2/5] irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ firmware issues
Message ID | 20230515131353.v2.2.I88dc0a0eb1d9d537de61604cd8994ecc55c0cac1@changeid |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp7175428vqo; Mon, 15 May 2023 13:31:41 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4OcNPGzhV5M3skHbAYpS3OMoAukVUONKX8kT5zGy3xOu9I8jUJ8LDxP9nkq6YLxyMITFiM X-Received: by 2002:a17:902:b78a:b0:1aa:d9c5:9cd5 with SMTP id e10-20020a170902b78a00b001aad9c59cd5mr33820332pls.11.1684182701015; Mon, 15 May 2023 13:31:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1684182701; cv=none; d=google.com; s=arc-20160816; b=Kw7QvpKogGqO/WNcr+saZcGIrqkZTrJVckVj8F7ELFfom/hf9LXKA6ttTfLERdHC+E mle+FkzD7nU2wRIAVMxDQeUF45J5CUg1u4UCMBw0CxdKKl4C7cd3n/N/bAPx+IoNS7j1 tpRpXJkPKeFOMIswUiYUahfbBd/RL22Yv1BHfVFdn1HDv6cvQHt8fx7OrXGT8821IbP0 gCGpWVYclGhN1O8w8J1E5VqNhl/UVWEC00wn3ZTiPXh9CG5wx6EA4KuxEvC97msRMqTg /M5bVcsaxz7dvziyUZcJjBMxc4YjIPa3J6ah966m89nh1NJIhL/+ecf+Js/L099rd1JE k7sw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=M3ZxCx5hi3hTfImTlWZJFg7LTzfHGowr1aBuZvBj8Ek=; b=0whYGdS68XjY3sESzdR/BcGmIkIbQWYKsaRBHLO/4VPTYzYOEuVqpeqwiqmhd3BV5g biaEmpQUyQksBVyukbcAk9wXlTmi+ze44xVC1HMyWzvQKuw2u5nHgK5MCnxBugsVqKGD dFCnlyyIRzaGjM3ur1qH2kkCkWrZo+dy3i5U4csm+by0QZKT3JHlxwOEBqI2ElEyrrKk dvgUH+4ZaHx+C0LkUoWkhPY+SQiNx6+PSpWQQiUbXBJE1LY7z29NlRCm+H4hGEOrsMkM gzbASiSTdkbFtfX3PcIsOhYDzY1KKgglIsL1bjolpw/Fn8L776XWfRvmAWRxJfJRz5w8 uSJg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=Hbdt9iI0; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s14-20020a170902ea0e00b001a64b603189si7443244plg.100.2023.05.15.13.31.05; Mon, 15 May 2023 13:31:40 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@chromium.org header.s=google header.b=Hbdt9iI0; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=chromium.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S245277AbjEOUQR (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Mon, 15 May 2023 16:16:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36674 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S245265AbjEOUQM (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 15 May 2023 16:16:12 -0400 Received: from mail-pg1-x535.google.com (mail-pg1-x535.google.com [IPv6:2607:f8b0:4864:20::535]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D00B710C3 for <linux-kernel@vger.kernel.org>; Mon, 15 May 2023 13:16:10 -0700 (PDT) Received: by mail-pg1-x535.google.com with SMTP id 41be03b00d2f7-53033a0b473so6603160a12.0 for <linux-kernel@vger.kernel.org>; Mon, 15 May 2023 13:16:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1684181770; x=1686773770; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=M3ZxCx5hi3hTfImTlWZJFg7LTzfHGowr1aBuZvBj8Ek=; b=Hbdt9iI0CBNRsRWJDq6LO2fdXD8SaO03ZHaJ8lncZbQVLatn4REqixKNi/L63qD17o HHOoNZBZebHLnkPuM0TGc9DikVIAEVVjUSf5dIZfNglLnIldiCXQvYMyLC6zIJUZi7Rd MuSHzfulUWCXbZfoxjiTNIlH48Va9wDUeEHqg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684181770; x=1686773770; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=M3ZxCx5hi3hTfImTlWZJFg7LTzfHGowr1aBuZvBj8Ek=; b=PK2X1lFyioc8lrBhs3+3Nyjsgg23UJnLpYxo8BkktVbQcq8TlXxdDFEzt9TuqUaLDd 2yf8n6diygVhRz6eM6j6afedlJbFZVWJocUK0MoXPFWPTHQHwI6r+CnQqxZpl54JoTxp jZ7XKbqfwUO3QKRvi4kAUmagmJOb1CZXWiL+pahXQorK8ZFsBUcLJVsQoSRTMYuewHVJ wXTOhLTzCiGcKvzD2elmFkCDLy1ecyHdSp6uk8/uj/7rMD2V0lds+G7MOF1bmUN9CZwM GMc1inf3nvISExm0Sv400nTX5MhLbb6iWeTB0BXvcgTgQqdJi8i9d5p1ak33GaTRV7du xhiQ== X-Gm-Message-State: AC+VfDw/3yl7WFv/2HhY4+3aiHYELf0na7TPkKuhLXnJKLrlnwu5dJZL 0RlLbR7c2ocokGVX4/kjb7YLBA== X-Received: by 2002:a05:6a21:7898:b0:101:167d:8472 with SMTP id bf24-20020a056a21789800b00101167d8472mr32063218pzc.26.1684181770297; Mon, 15 May 2023 13:16:10 -0700 (PDT) Received: from tictac2.mtv.corp.google.com ([2620:15c:9d:2:9f33:9d98:277a:d4cd]) by smtp.gmail.com with ESMTPSA id a19-20020a62bd13000000b0063f0c9eadc7sm7981411pff.200.2023.05.15.13.16.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 15 May 2023 13:16:09 -0700 (PDT) From: Douglas Anderson <dianders@chromium.org> To: Marc Zyngier <maz@kernel.org>, Thomas Gleixner <tglx@linutronix.de>, Rob Herring <robh+dt@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Matthias Brugger <matthias.bgg@gmail.com> Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Allen-KH Cheng <allen-kh.cheng@mediatek.com>, linux-mediatek@lists.infradead.org, Eddie Huang <eddie.huang@mediatek.com>, Hsin-Hsiung Wang <hsin-hsiung.wang@mediatek.com>, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>, wenst@chromium.org, yidilin@chromium.org, Tinghan Shen <tinghan.shen@mediatek.com>, jwerner@chromium.org, Weiyi Lu <weiyi.lu@mediatek.com>, Ben Ho <Ben.Ho@mediatek.com>, Seiya Wang <seiya.wang@mediatek.com>, Douglas Anderson <dianders@chromium.org>, linux-kernel@vger.kernel.org Subject: [PATCH v2 2/5] irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ firmware issues Date: Mon, 15 May 2023 13:13:51 -0700 Message-ID: <20230515131353.v2.2.I88dc0a0eb1d9d537de61604cd8994ecc55c0cac1@changeid> X-Mailer: git-send-email 2.40.1.606.ga4b1b128d6-goog In-Reply-To: <20230515131353.v2.cover@dianders> References: <20230515131353.v2.cover@dianders> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765993559854506365?= X-GMAIL-MSGID: =?utf-8?q?1765993559854506365?= |
Series |
irqchip/gic-v3: Disable pseudo NMIs on Mediatek Chromebooks w/ bad FW
|
|
Commit Message
Doug Anderson
May 15, 2023, 8:13 p.m. UTC
Some Chromebooks with Mediatek SoCs have a problem where the firmware doesn't properly save/restore certain GICR registers. Newer Chromebooks should fix this issue and we may be able to do firmware updates for old Chromebooks. At the moment, the only known issue with these Chromebooks is that we can't enable "pseudo NMIs" since the priority register can be lost. Enabling "pseudo NMIs" on Chromebooks with the problematic firmware causes crashes and freezes. Let's detect devices with this problem and then disable "pseudo NMIs" on them. We'll detect the problem by looking for the presence of the "mediatek,broken-save-restore-fw" property in the GIC device tree node. Any devices with fixed firmware will not have this property. Our detection plan works because we never bake a Chromebook's device tree into firmware. Instead, device trees are always bundled with the kernel. We'll update the device trees of all affected Chromebooks and then we'll never enable "pseudo NMI" on a kernel that is bundled with old device trees. When a firmware update is shipped that fixes this issue it will know to patch the device tree to remove the property. In order to make this work, the quick detection mechanism of the GICv3 code is extended to be able to look for properties in addition to looking at "compatible". Reviewed-by: Julius Werner <jwerner@chromium.org> Signed-off-by: Douglas Anderson <dianders@chromium.org> --- Changes in v2: - mediatek,gicr-save-quirk => mediatek,broken-save-restore-fw drivers/irqchip/irq-gic-common.c | 8 ++++++-- drivers/irqchip/irq-gic-common.h | 1 + drivers/irqchip/irq-gic-v3.c | 20 ++++++++++++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-)
Comments
Il 15/05/23 22:13, Douglas Anderson ha scritto: > Some Chromebooks with Mediatek SoCs have a problem where the firmware > doesn't properly save/restore certain GICR registers. Newer > Chromebooks should fix this issue and we may be able to do firmware > updates for old Chromebooks. At the moment, the only known issue with > these Chromebooks is that we can't enable "pseudo NMIs" since the > priority register can be lost. Enabling "pseudo NMIs" on Chromebooks > with the problematic firmware causes crashes and freezes. > > Let's detect devices with this problem and then disable "pseudo NMIs" > on them. We'll detect the problem by looking for the presence of the > "mediatek,broken-save-restore-fw" property in the GIC device tree > node. Any devices with fixed firmware will not have this property. > > Our detection plan works because we never bake a Chromebook's device > tree into firmware. Instead, device trees are always bundled with the > kernel. We'll update the device trees of all affected Chromebooks and > then we'll never enable "pseudo NMI" on a kernel that is bundled with > old device trees. When a firmware update is shipped that fixes this > issue it will know to patch the device tree to remove the property. > > In order to make this work, the quick detection mechanism of the GICv3 > code is extended to be able to look for properties in addition to > looking at "compatible". > > Reviewed-by: Julius Werner <jwerner@chromium.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> I don't like firmware removing properties from my devicetrees and I'd like this issue to get addressed in another way (use a scratch register? and check it in Linux drivers to determine if the issue is not present: if scratch contains BIT(x), do not parse the quirk) but that's a different discussion which is a bit out of context for this patch, so: Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Hi, On Tue, May 16, 2023 at 6:23 AM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 15/05/23 22:13, Douglas Anderson ha scritto: > > Some Chromebooks with Mediatek SoCs have a problem where the firmware > > doesn't properly save/restore certain GICR registers. Newer > > Chromebooks should fix this issue and we may be able to do firmware > > updates for old Chromebooks. At the moment, the only known issue with > > these Chromebooks is that we can't enable "pseudo NMIs" since the > > priority register can be lost. Enabling "pseudo NMIs" on Chromebooks > > with the problematic firmware causes crashes and freezes. > > > > Let's detect devices with this problem and then disable "pseudo NMIs" > > on them. We'll detect the problem by looking for the presence of the > > "mediatek,broken-save-restore-fw" property in the GIC device tree > > node. Any devices with fixed firmware will not have this property. > > > > Our detection plan works because we never bake a Chromebook's device > > tree into firmware. Instead, device trees are always bundled with the > > kernel. We'll update the device trees of all affected Chromebooks and > > then we'll never enable "pseudo NMI" on a kernel that is bundled with > > old device trees. When a firmware update is shipped that fixes this > > issue it will know to patch the device tree to remove the property. > > > > In order to make this work, the quick detection mechanism of the GICv3 > > code is extended to be able to look for properties in addition to > > looking at "compatible". > > > > Reviewed-by: Julius Werner <jwerner@chromium.org> > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > I don't like firmware removing properties from my devicetrees and I'd like this > issue to get addressed in another way (use a scratch register? and check it in > Linux drivers to determine if the issue is not present: if scratch contains BIT(x), > do not parse the quirk) but that's a different discussion which is a bit out of > context for this patch, so: Any particular reason why? IMO it's actually a fair bit cleaner to have firmware remove a property that's specifically documented for the firmware to remove compared to having firmware adding properties to or otherwise messing with the device tree. For the removal case, it's easy from the device tree git history to find out about the property, when it was added, and that it is expected that some versions of firmware will remove it. IMO having firmware add properties can be a little more mysterious, though that has its place too. In general, though, firmware is expected to be able to be able to touch up the device tree. It puts things in "chosen", adds bits describing the firmware, can add things to the device tree to describe components it is uniquely able to probe (like SDRAM), could enable/disable a component if it has info about their presence, etc. I'm happy to hear other opinions on it, but in my mind having a sideband bit telling us to ignore the quirk is more confusing instead of less confusing. > Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> Thanks!
On Tue, 16 May 2023 14:23:52 +0100, AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > Il 15/05/23 22:13, Douglas Anderson ha scritto: > > Some Chromebooks with Mediatek SoCs have a problem where the firmware > > doesn't properly save/restore certain GICR registers. Newer > > Chromebooks should fix this issue and we may be able to do firmware > > updates for old Chromebooks. At the moment, the only known issue with > > these Chromebooks is that we can't enable "pseudo NMIs" since the > > priority register can be lost. Enabling "pseudo NMIs" on Chromebooks > > with the problematic firmware causes crashes and freezes. > > > > Let's detect devices with this problem and then disable "pseudo NMIs" > > on them. We'll detect the problem by looking for the presence of the > > "mediatek,broken-save-restore-fw" property in the GIC device tree > > node. Any devices with fixed firmware will not have this property. > > > > Our detection plan works because we never bake a Chromebook's device > > tree into firmware. Instead, device trees are always bundled with the > > kernel. We'll update the device trees of all affected Chromebooks and > > then we'll never enable "pseudo NMI" on a kernel that is bundled with > > old device trees. When a firmware update is shipped that fixes this > > issue it will know to patch the device tree to remove the property. > > > > In order to make this work, the quick detection mechanism of the GICv3 > > code is extended to be able to look for properties in addition to > > looking at "compatible". > > > > Reviewed-by: Julius Werner <jwerner@chromium.org> > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > I don't like firmware removing properties from my devicetrees and I'd like this > issue to get addressed in another way (use a scratch register? and check it in > Linux drivers to determine if the issue is not present: if scratch contains BIT(x), > do not parse the quirk) but that's a different discussion which is a bit out of > context for this patch, so: So what you're advocating for is that we have another flag somewhere that says the same thing. Stored where? Accessible how? On top of having to check for DT, ACPI, and SOC_ID interfaces, you want YAFM (Yet Another Fixing Method)? Thanks, but no, thanks. M.
Hi Douglas, On Mon, May 15, 2023 at 10:16 PM Douglas Anderson <dianders@chromium.org> wrote: > Some Chromebooks with Mediatek SoCs have a problem where the firmware > doesn't properly save/restore certain GICR registers. Newer > Chromebooks should fix this issue and we may be able to do firmware > updates for old Chromebooks. At the moment, the only known issue with > these Chromebooks is that we can't enable "pseudo NMIs" since the > priority register can be lost. Enabling "pseudo NMIs" on Chromebooks > with the problematic firmware causes crashes and freezes. > > Let's detect devices with this problem and then disable "pseudo NMIs" > on them. We'll detect the problem by looking for the presence of the > "mediatek,broken-save-restore-fw" property in the GIC device tree > node. Any devices with fixed firmware will not have this property. > > Our detection plan works because we never bake a Chromebook's device > tree into firmware. Instead, device trees are always bundled with the > kernel. We'll update the device trees of all affected Chromebooks and > then we'll never enable "pseudo NMI" on a kernel that is bundled with > old device trees. When a firmware update is shipped that fixes this > issue it will know to patch the device tree to remove the property. > > In order to make this work, the quick detection mechanism of the GICv3 > code is extended to be able to look for properties in addition to > looking at "compatible". > > Reviewed-by: Julius Werner <jwerner@chromium.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v2: > - mediatek,gicr-save-quirk => mediatek,broken-save-restore-fw Thanks for your patch, which is now commit 44bd78dd2b8897f5 ("irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ firmware issues") in v6.4-rc4. This causes enabling an unrelated workaround on R-Car V4H: GIC: enabling workaround for GICv3: Cavium erratum 38539 > --- a/drivers/irqchip/irq-gic-common.c > +++ b/drivers/irqchip/irq-gic-common.c > @@ -16,7 +16,11 @@ void gic_enable_of_quirks(const struct device_node *np, > const struct gic_quirk *quirks, void *data) > { > for (; quirks->desc; quirks++) { > - if (!of_device_is_compatible(np, quirks->compatible)) > + if (quirks->compatible && > + !of_device_is_compatible(np, quirks->compatible)) > + continue; > + if (quirks->property && > + !of_property_read_bool(np, quirks->property)) > continue; Presumably the loop should continue if none of quirks-compatible or quirks->property is set? > if (quirks->init(data)) > pr_info("GIC: enabling workaround for %s\n", > @@ -28,7 +32,7 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, > void *data) > { > for (; quirks->desc; quirks++) { > - if (quirks->compatible) > + if (quirks->compatible || quirks->property) > continue; > if (quirks->iidr != (quirks->mask & iidr)) > continue; > diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h > index 27e3d4ed4f32..3db4592cda1c 100644 > --- a/drivers/irqchip/irq-gic-common.h > +++ b/drivers/irqchip/irq-gic-common.h > @@ -13,6 +13,7 @@ > struct gic_quirk { > const char *desc; > const char *compatible; > + const char *property; > bool (*init)(void *data); > u32 iidr; > u32 mask; > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 6fcee221f201..a605aa79435a 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -39,6 +39,7 @@ > > #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996 (1ULL << 0) > #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539 (1ULL << 1) > +#define FLAGS_WORKAROUND_MTK_GICR_SAVE (1ULL << 2) > > #define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1) > > @@ -1720,6 +1721,15 @@ static bool gic_enable_quirk_msm8996(void *data) > return true; > } > > +static bool gic_enable_quirk_mtk_gicr(void *data) > +{ > + struct gic_chip_data *d = data; > + > + d->flags |= FLAGS_WORKAROUND_MTK_GICR_SAVE; > + > + return true; > +} > + > static bool gic_enable_quirk_cavium_38539(void *data) > { > struct gic_chip_data *d = data; > @@ -1792,6 +1802,11 @@ static const struct gic_quirk gic_quirks[] = { > .compatible = "qcom,msm8996-gic-v3", > .init = gic_enable_quirk_msm8996, > }, > + { > + .desc = "GICv3: Mediatek Chromebook GICR save problem", > + .property = "mediatek,broken-save-restore-fw", > + .init = gic_enable_quirk_mtk_gicr, > + }, > { > .desc = "GICv3: HIP06 erratum 161010803", > .iidr = 0x0204043b, > @@ -1834,6 +1849,11 @@ static void gic_enable_nmi_support(void) > if (!gic_prio_masking_enabled()) > return; > > + if (gic_data.flags & FLAGS_WORKAROUND_MTK_GICR_SAVE) { > + pr_warn("Skipping NMI enable due to firmware issues\n"); > + return; > + } > + > ppi_nmi_refs = kcalloc(gic_data.ppi_nr, sizeof(*ppi_nmi_refs), GFP_KERNEL); > if (!ppi_nmi_refs) > return; > -- > 2.40.1.606.ga4b1b128d6-goog Gr{oetje,eeting}s, Geert
On Tue, 30 May 2023 09:29:02 +0100, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Douglas, > > On Mon, May 15, 2023 at 10:16 PM Douglas Anderson <dianders@chromium.org> wrote: > > Some Chromebooks with Mediatek SoCs have a problem where the firmware > > doesn't properly save/restore certain GICR registers. Newer > > Chromebooks should fix this issue and we may be able to do firmware > > updates for old Chromebooks. At the moment, the only known issue with > > these Chromebooks is that we can't enable "pseudo NMIs" since the > > priority register can be lost. Enabling "pseudo NMIs" on Chromebooks > > with the problematic firmware causes crashes and freezes. > > > > Let's detect devices with this problem and then disable "pseudo NMIs" > > on them. We'll detect the problem by looking for the presence of the > > "mediatek,broken-save-restore-fw" property in the GIC device tree > > node. Any devices with fixed firmware will not have this property. > > > > Our detection plan works because we never bake a Chromebook's device > > tree into firmware. Instead, device trees are always bundled with the > > kernel. We'll update the device trees of all affected Chromebooks and > > then we'll never enable "pseudo NMI" on a kernel that is bundled with > > old device trees. When a firmware update is shipped that fixes this > > issue it will know to patch the device tree to remove the property. > > > > In order to make this work, the quick detection mechanism of the GICv3 > > code is extended to be able to look for properties in addition to > > looking at "compatible". > > > > Reviewed-by: Julius Werner <jwerner@chromium.org> > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > --- > > > > Changes in v2: > > - mediatek,gicr-save-quirk => mediatek,broken-save-restore-fw > > Thanks for your patch, which is now commit 44bd78dd2b8897f5 > ("irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ > firmware issues") in v6.4-rc4. > > This causes enabling an unrelated workaround on R-Car V4H: > > GIC: enabling workaround for GICv3: Cavium erratum 38539 > > > --- a/drivers/irqchip/irq-gic-common.c > > +++ b/drivers/irqchip/irq-gic-common.c > > @@ -16,7 +16,11 @@ void gic_enable_of_quirks(const struct device_node *np, > > const struct gic_quirk *quirks, void *data) > > { > > for (; quirks->desc; quirks++) { > > - if (!of_device_is_compatible(np, quirks->compatible)) > > + if (quirks->compatible && > > + !of_device_is_compatible(np, quirks->compatible)) > > + continue; > > + if (quirks->property && > > + !of_property_read_bool(np, quirks->property)) > > continue; > > Presumably the loop should continue if none of quirks-compatible > or quirks->property is set? Indeed, thanks for pointing that out. Can you give the following hack a go (compile tested only)? diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c index de47b51cdadb..7b591736ab58 100644 --- a/drivers/irqchip/irq-gic-common.c +++ b/drivers/irqchip/irq-gic-common.c @@ -16,6 +16,8 @@ void gic_enable_of_quirks(const struct device_node *np, const struct gic_quirk *quirks, void *data) { for (; quirks->desc; quirks++) { + if (!quirks->compatible && !quirks->property) + continue; if (quirks->compatible && !of_device_is_compatible(np, quirks->compatible)) continue; If that works for you, I'll queue it ASAP. Cheers, M.
Hi Marc, On Tue, May 30, 2023 at 11:46 AM Marc Zyngier <maz@kernel.org> wrote: > On Tue, 30 May 2023 09:29:02 +0100, > Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > On Mon, May 15, 2023 at 10:16 PM Douglas Anderson <dianders@chromium.org> wrote: > > > Some Chromebooks with Mediatek SoCs have a problem where the firmware > > > doesn't properly save/restore certain GICR registers. Newer > > > Chromebooks should fix this issue and we may be able to do firmware > > > updates for old Chromebooks. At the moment, the only known issue with > > > these Chromebooks is that we can't enable "pseudo NMIs" since the > > > priority register can be lost. Enabling "pseudo NMIs" on Chromebooks > > > with the problematic firmware causes crashes and freezes. > > > > > > Let's detect devices with this problem and then disable "pseudo NMIs" > > > on them. We'll detect the problem by looking for the presence of the > > > "mediatek,broken-save-restore-fw" property in the GIC device tree > > > node. Any devices with fixed firmware will not have this property. > > > > > > Our detection plan works because we never bake a Chromebook's device > > > tree into firmware. Instead, device trees are always bundled with the > > > kernel. We'll update the device trees of all affected Chromebooks and > > > then we'll never enable "pseudo NMI" on a kernel that is bundled with > > > old device trees. When a firmware update is shipped that fixes this > > > issue it will know to patch the device tree to remove the property. > > > > > > In order to make this work, the quick detection mechanism of the GICv3 > > > code is extended to be able to look for properties in addition to > > > looking at "compatible". > > > > > > Reviewed-by: Julius Werner <jwerner@chromium.org> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > --- > > > > > > Changes in v2: > > > - mediatek,gicr-save-quirk => mediatek,broken-save-restore-fw > > > > Thanks for your patch, which is now commit 44bd78dd2b8897f5 > > ("irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ > > firmware issues") in v6.4-rc4. > > > > This causes enabling an unrelated workaround on R-Car V4H: > > > > GIC: enabling workaround for GICv3: Cavium erratum 38539 > > > > > --- a/drivers/irqchip/irq-gic-common.c > > > +++ b/drivers/irqchip/irq-gic-common.c > > > @@ -16,7 +16,11 @@ void gic_enable_of_quirks(const struct device_node *np, > > > const struct gic_quirk *quirks, void *data) > > > { > > > for (; quirks->desc; quirks++) { > > > - if (!of_device_is_compatible(np, quirks->compatible)) > > > + if (quirks->compatible && > > > + !of_device_is_compatible(np, quirks->compatible)) > > > + continue; > > > + if (quirks->property && > > > + !of_property_read_bool(np, quirks->property)) > > > continue; > > > > Presumably the loop should continue if none of quirks-compatible > > or quirks->property is set? > > Indeed, thanks for pointing that out. Can you give the following hack > a go (compile tested only)? > > diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c > index de47b51cdadb..7b591736ab58 100644 > --- a/drivers/irqchip/irq-gic-common.c > +++ b/drivers/irqchip/irq-gic-common.c > @@ -16,6 +16,8 @@ void gic_enable_of_quirks(const struct device_node *np, > const struct gic_quirk *quirks, void *data) > { > for (; quirks->desc; quirks++) { > + if (!quirks->compatible && !quirks->property) > + continue; > if (quirks->compatible && > !of_device_is_compatible(np, quirks->compatible)) > continue; > > If that works for you, I'll queue it ASAP. Thanks, that fixes the issue for me on Renesas White-Hawk (R-Car V4H). No regressions on Koelsch (R-Car M2-W) and Salvator-XS (R-Car H3 ES2.0). Tested-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
Hi, On Tue, May 30, 2023 at 2:46 AM Marc Zyngier <maz@kernel.org> wrote: > > On Tue, 30 May 2023 09:29:02 +0100, > Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Hi Douglas, > > > > On Mon, May 15, 2023 at 10:16 PM Douglas Anderson <dianders@chromium.org> wrote: > > > Some Chromebooks with Mediatek SoCs have a problem where the firmware > > > doesn't properly save/restore certain GICR registers. Newer > > > Chromebooks should fix this issue and we may be able to do firmware > > > updates for old Chromebooks. At the moment, the only known issue with > > > these Chromebooks is that we can't enable "pseudo NMIs" since the > > > priority register can be lost. Enabling "pseudo NMIs" on Chromebooks > > > with the problematic firmware causes crashes and freezes. > > > > > > Let's detect devices with this problem and then disable "pseudo NMIs" > > > on them. We'll detect the problem by looking for the presence of the > > > "mediatek,broken-save-restore-fw" property in the GIC device tree > > > node. Any devices with fixed firmware will not have this property. > > > > > > Our detection plan works because we never bake a Chromebook's device > > > tree into firmware. Instead, device trees are always bundled with the > > > kernel. We'll update the device trees of all affected Chromebooks and > > > then we'll never enable "pseudo NMI" on a kernel that is bundled with > > > old device trees. When a firmware update is shipped that fixes this > > > issue it will know to patch the device tree to remove the property. > > > > > > In order to make this work, the quick detection mechanism of the GICv3 > > > code is extended to be able to look for properties in addition to > > > looking at "compatible". > > > > > > Reviewed-by: Julius Werner <jwerner@chromium.org> > > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > > > --- > > > > > > Changes in v2: > > > - mediatek,gicr-save-quirk => mediatek,broken-save-restore-fw > > > > Thanks for your patch, which is now commit 44bd78dd2b8897f5 > > ("irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ > > firmware issues") in v6.4-rc4. > > > > This causes enabling an unrelated workaround on R-Car V4H: > > > > GIC: enabling workaround for GICv3: Cavium erratum 38539 > > > > > --- a/drivers/irqchip/irq-gic-common.c > > > +++ b/drivers/irqchip/irq-gic-common.c > > > @@ -16,7 +16,11 @@ void gic_enable_of_quirks(const struct device_node *np, > > > const struct gic_quirk *quirks, void *data) > > > { > > > for (; quirks->desc; quirks++) { > > > - if (!of_device_is_compatible(np, quirks->compatible)) > > > + if (quirks->compatible && > > > + !of_device_is_compatible(np, quirks->compatible)) > > > + continue; > > > + if (quirks->property && > > > + !of_property_read_bool(np, quirks->property)) > > > continue; > > > > Presumably the loop should continue if none of quirks-compatible > > or quirks->property is set? > > Indeed, thanks for pointing that out. Can you give the following hack > a go (compile tested only)? > > diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c > index de47b51cdadb..7b591736ab58 100644 > --- a/drivers/irqchip/irq-gic-common.c > +++ b/drivers/irqchip/irq-gic-common.c > @@ -16,6 +16,8 @@ void gic_enable_of_quirks(const struct device_node *np, > const struct gic_quirk *quirks, void *data) > { > for (; quirks->desc; quirks++) { > + if (!quirks->compatible && !quirks->property) > + continue; Sorry for missing this and thanks for the fix. Looks like this is already committed, but in case it matters: Reviewed-by: Douglas Anderson <dianders@chromium.org>
diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c index a610821c8ff2..de47b51cdadb 100644 --- a/drivers/irqchip/irq-gic-common.c +++ b/drivers/irqchip/irq-gic-common.c @@ -16,7 +16,11 @@ void gic_enable_of_quirks(const struct device_node *np, const struct gic_quirk *quirks, void *data) { for (; quirks->desc; quirks++) { - if (!of_device_is_compatible(np, quirks->compatible)) + if (quirks->compatible && + !of_device_is_compatible(np, quirks->compatible)) + continue; + if (quirks->property && + !of_property_read_bool(np, quirks->property)) continue; if (quirks->init(data)) pr_info("GIC: enabling workaround for %s\n", @@ -28,7 +32,7 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, void *data) { for (; quirks->desc; quirks++) { - if (quirks->compatible) + if (quirks->compatible || quirks->property) continue; if (quirks->iidr != (quirks->mask & iidr)) continue; diff --git a/drivers/irqchip/irq-gic-common.h b/drivers/irqchip/irq-gic-common.h index 27e3d4ed4f32..3db4592cda1c 100644 --- a/drivers/irqchip/irq-gic-common.h +++ b/drivers/irqchip/irq-gic-common.h @@ -13,6 +13,7 @@ struct gic_quirk { const char *desc; const char *compatible; + const char *property; bool (*init)(void *data); u32 iidr; u32 mask; diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 6fcee221f201..a605aa79435a 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c @@ -39,6 +39,7 @@ #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996 (1ULL << 0) #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539 (1ULL << 1) +#define FLAGS_WORKAROUND_MTK_GICR_SAVE (1ULL << 2) #define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1) @@ -1720,6 +1721,15 @@ static bool gic_enable_quirk_msm8996(void *data) return true; } +static bool gic_enable_quirk_mtk_gicr(void *data) +{ + struct gic_chip_data *d = data; + + d->flags |= FLAGS_WORKAROUND_MTK_GICR_SAVE; + + return true; +} + static bool gic_enable_quirk_cavium_38539(void *data) { struct gic_chip_data *d = data; @@ -1792,6 +1802,11 @@ static const struct gic_quirk gic_quirks[] = { .compatible = "qcom,msm8996-gic-v3", .init = gic_enable_quirk_msm8996, }, + { + .desc = "GICv3: Mediatek Chromebook GICR save problem", + .property = "mediatek,broken-save-restore-fw", + .init = gic_enable_quirk_mtk_gicr, + }, { .desc = "GICv3: HIP06 erratum 161010803", .iidr = 0x0204043b, @@ -1834,6 +1849,11 @@ static void gic_enable_nmi_support(void) if (!gic_prio_masking_enabled()) return; + if (gic_data.flags & FLAGS_WORKAROUND_MTK_GICR_SAVE) { + pr_warn("Skipping NMI enable due to firmware issues\n"); + return; + } + ppi_nmi_refs = kcalloc(gic_data.ppi_nr, sizeof(*ppi_nmi_refs), GFP_KERNEL); if (!ppi_nmi_refs) return;