All of lore.kernel.org
 help / color / mirror / Atom feed
From: Douglas Anderson <dianders@chromium.org>
To: Marc Zyngier <maz@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Chen-Yu Tsai <wenst@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>,
	Amit Daniel Kachhap <amit.kachhap@arm.com>,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@collabora.com>,
	James Morse <james.morse@arm.com>,
	Joey Gouly <joey.gouly@arm.com>, Mark Brown <broonie@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org
Subject: [PATCH 1/3] arm64: Disable GiC priorities on Mediatek devices w/ firmware issues
Date: Fri,  6 Oct 2023 15:15:51 -0700	[thread overview]
Message-ID: <20231006151547.1.Ide945748593cffd8ff0feb9ae22b795935b944d6@changeid> (raw)

In commit 44bd78dd2b88 ("irqchip/gic-v3: Disable pseudo NMIs on
Mediatek devices w/ firmware issues") we added a method for detecting
Mediatek devices with broken firmware and disabled pseudo-NMI. While
that worked, it didn't address the problem at a deep enough level.

The fundamental issue with this broken firmware is that it's not
saving and restoring several important GICR registers. The current
list is believed to be:
* GICR_NUM_IPRIORITYR
* GICR_CTLR
* GICR_ISPENDR0
* GICR_ISACTIVER0
* GICR_NSACR

Pseudo-NMI didn't work because it was the only thing (currently) in
the kernel that relied on the broken registers, so forcing pseudo-NMI
off was an effective fix. However, it could be observed that calling
system_uses_irq_prio_masking() on these systems still returned
"true". That caused confusion and led to the need for
commit a07a59415217 ("arm64: smp: avoid NMI IPIs with broken MediaTek
FW"). It's worried that the incorrect value returned by
system_uses_irq_prio_masking() on these systems will continue to
confuse future developers.

Let's fix the issue a little more completely by disabling IRQ
priorities at a deeper level in the kernel. Once we do this we can
revert some of the other bits of code dealing with this quirk.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arm64/kernel/cpufeature.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 2806a2850e78..e35efab8efa9 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2094,9 +2094,30 @@ static int __init early_enable_pseudo_nmi(char *p)
 }
 early_param("irqchip.gicv3_pseudo_nmi", early_enable_pseudo_nmi);
 
+static bool are_gic_priorities_broken(void)
+{
+	bool is_broken = false;
+	struct device_node *np;
+
+	/*
+	 * Detect broken Mediatek firmware that doesn't properly save and
+	 * restore GIC priorities.
+	 */
+	np = of_find_compatible_node(NULL, NULL, "arm,gic-v3");
+	if (np) {
+		is_broken = of_property_read_bool(np, "mediatek,broken-save-restore-fw");
+		of_node_put(np);
+	}
+
+	return is_broken;
+}
+
 static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry,
 				   int scope)
 {
+	if (are_gic_priorities_broken())
+		return false;
+
 	/*
 	 * ARM64_HAS_GIC_CPUIF_SYSREGS has a lower index, and is a boot CPU
 	 * feature, so will be detected earlier.
-- 
2.42.0.609.gbb76f46606-goog


WARNING: multiple messages have this Message-ID (diff)
From: Douglas Anderson <dianders@chromium.org>
To: Marc Zyngier <maz@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Chen-Yu Tsai <wenst@chromium.org>
Cc: Douglas Anderson <dianders@chromium.org>,
	Amit Daniel Kachhap <amit.kachhap@arm.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	James Morse <james.morse@arm.com>,
	Joey Gouly <joey.gouly@arm.com>, Mark Brown <broonie@kernel.org>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org
Subject: [PATCH 1/3] arm64: Disable GiC priorities on Mediatek devices w/ firmware issues
Date: Fri,  6 Oct 2023 15:15:51 -0700	[thread overview]
Message-ID: <20231006151547.1.Ide945748593cffd8ff0feb9ae22b795935b944d6@changeid> (raw)

In commit 44bd78dd2b88 ("irqchip/gic-v3: Disable pseudo NMIs on
Mediatek devices w/ firmware issues") we added a method for detecting
Mediatek devices with broken firmware and disabled pseudo-NMI. While
that worked, it didn't address the problem at a deep enough level.

The fundamental issue with this broken firmware is that it's not
saving and restoring several important GICR registers. The current
list is believed to be:
* GICR_NUM_IPRIORITYR
* GICR_CTLR
* GICR_ISPENDR0
* GICR_ISACTIVER0
* GICR_NSACR

Pseudo-NMI didn't work because it was the only thing (currently) in
the kernel that relied on the broken registers, so forcing pseudo-NMI
off was an effective fix. However, it could be observed that calling
system_uses_irq_prio_masking() on these systems still returned
"true". That caused confusion and led to the need for
commit a07a59415217 ("arm64: smp: avoid NMI IPIs with broken MediaTek
FW"). It's worried that the incorrect value returned by
system_uses_irq_prio_masking() on these systems will continue to
confuse future developers.

Let's fix the issue a little more completely by disabling IRQ
priorities at a deeper level in the kernel. Once we do this we can
revert some of the other bits of code dealing with this quirk.

Signed-off-by: Douglas Anderson <dianders@chromium.org>
---

 arch/arm64/kernel/cpufeature.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 2806a2850e78..e35efab8efa9 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2094,9 +2094,30 @@ static int __init early_enable_pseudo_nmi(char *p)
 }
 early_param("irqchip.gicv3_pseudo_nmi", early_enable_pseudo_nmi);
 
+static bool are_gic_priorities_broken(void)
+{
+	bool is_broken = false;
+	struct device_node *np;
+
+	/*
+	 * Detect broken Mediatek firmware that doesn't properly save and
+	 * restore GIC priorities.
+	 */
+	np = of_find_compatible_node(NULL, NULL, "arm,gic-v3");
+	if (np) {
+		is_broken = of_property_read_bool(np, "mediatek,broken-save-restore-fw");
+		of_node_put(np);
+	}
+
+	return is_broken;
+}
+
 static bool can_use_gic_priorities(const struct arm64_cpu_capabilities *entry,
 				   int scope)
 {
+	if (are_gic_priorities_broken())
+		return false;
+
 	/*
 	 * ARM64_HAS_GIC_CPUIF_SYSREGS has a lower index, and is a boot CPU
 	 * feature, so will be detected earlier.
-- 
2.42.0.609.gbb76f46606-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

             reply	other threads:[~2023-10-06 22:16 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-06 22:15 Douglas Anderson [this message]
2023-10-06 22:15 ` [PATCH 1/3] arm64: Disable GiC priorities on Mediatek devices w/ firmware issues Douglas Anderson
2023-10-06 22:15 ` [PATCH 2/3] Revert "arm64: smp: avoid NMI IPIs with broken MediaTek FW" Douglas Anderson
2023-10-06 22:15   ` Douglas Anderson
2023-10-18 11:03   ` Mark Rutland
2023-10-18 11:03     ` Mark Rutland
2023-10-06 22:15 ` [PATCH 3/3] irqchip/gic-v3: Remove Mediatek pseudo-NMI firmware quirk handling Douglas Anderson
2023-10-06 22:15   ` Douglas Anderson
2023-10-18 11:08   ` Mark Rutland
2023-10-18 11:08     ` Mark Rutland
2023-10-30 23:01     ` Doug Anderson
2023-10-30 23:01       ` Doug Anderson
2023-11-07 11:37       ` Marc Zyngier
2023-11-07 11:37         ` Marc Zyngier
2023-11-07 13:10         ` Catalin Marinas
2023-11-07 13:10           ` Catalin Marinas
2023-10-18 11:01 ` [PATCH 1/3] arm64: Disable GiC priorities on Mediatek devices w/ firmware issues Mark Rutland
2023-10-18 11:01   ` Mark Rutland
2023-10-30 23:19   ` Doug Anderson
2023-10-30 23:19     ` Doug Anderson
2023-11-07 10:18     ` Will Deacon
2023-11-07 10:18       ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231006151547.1.Ide945748593cffd8ff0feb9ae22b795935b944d6@changeid \
    --to=dianders@chromium.org \
    --cc=amit.kachhap@arm.com \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=broonie@kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=james.morse@arm.com \
    --cc=joey.gouly@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=mark.rutland@arm.com \
    --cc=matthias.bgg@gmail.com \
    --cc=maz@kernel.org \
    --cc=wenst@chromium.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.