Hi Douglas,
On Mon, May 15, 2023 at 10:16 PM Douglas Anderson <dianders@xxxxxxxxxxxx> 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@xxxxxxxxxxxx>
> Signed-off-by: Douglas Anderson <dianders@xxxxxxxxxxxx>
> ---
>
> 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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
