Re: [PATCH v2 2/5] irqchip/gic-v3: Disable pseudo NMIs on Mediatek devices w/ firmware issues

On Tue, 30 May 2023 09:29:02 +0100,
Geert Uytterhoeven [off-list ref] wrote:

Hi Douglas,

On Mon, May 15, 2023 at 10:16 PM Douglas Anderson [off-list ref] wrote:

quoted

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

quoted

--- 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.

-- 
Without deviation from the norm, progress is not possible.

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