Re: [RFC PATCH v2] irqchip/gic-v3: Add quirk for msm8996 secured registers

[Craig Tatlor]

On 4 September 2018 15:05:38 BST, Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx> wrote:
>Access to GICR_WAKER is restricted on msm8996 SoC in Hypervisor.
>Its been more than 2 years of wait for this to be fixed, which has
>no hopes to be fixed. This change was introduced for the "lead device"
>on msm8996 platform. It looks like all publicly available msm8996
>devices have this implementation.
>
>So add a quirk to not access this register on msm8996.
>
>With this quirk MSM8996 can at least boot out of mainline,
>which can help community to work with boards based on MSM8996.
>
>Without this patch Qualcomm DB820c board reboots when GICR_WAKER
>is accessed.
>
>Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@xxxxxxxxxx>
>---
>Hi Marc,
>
>There is no errata associated with this quirk, so I could not add
>any entries into Documentation/arm64/silicon-errata.txt Or add
>any number to the quirk description.
>
>Changes since v1:
>- renamed gic_v3 references to gic
>- added full mask for iidr register match
>
>thanks,
>srini
>
> drivers/irqchip/irq-gic-v3.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/irqchip/irq-gic-v3.c
>b/drivers/irqchip/irq-gic-v3.c
>index d5912f1ec884..406d4a44c887 100644
>--- a/drivers/irqchip/irq-gic-v3.c
>+++ b/drivers/irqchip/irq-gic-v3.c
>@@ -41,6 +41,8 @@
> 
> #include "irq-gic-common.h"
> 
>+#define FLAGS_WORKAROUND_GICR_WAKER_MSM8996	(1ULL << 0)
>+
> struct redist_region {
> 	void __iomem		*redist_base;
> 	phys_addr_t		phys_base;
>@@ -55,6 +57,7 @@ struct gic_chip_data {
> 	struct irq_domain	*domain;
> 	u64			redist_stride;
> 	u32			nr_redist_regions;
>+	u64			flags;
> 	bool			has_rss;
> 	unsigned int		irq_nr;
> 	struct partition_desc	*ppi_descs[16];
>@@ -139,6 +142,9 @@ static void gic_enable_redist(bool enable)
> 	u32 count = 1000000;	/* 1s! */
> 	u32 val;
> 
>+	if (gic_data.flags & FLAGS_WORKAROUND_GICR_WAKER_MSM8996)
>+		return;
>+
> 	rbase = gic_data_rdist_rd_base();
> 
> 	val = readl_relaxed(rbase + GICR_WAKER);
>@@ -1068,13 +1074,31 @@ static const struct irq_domain_ops
>partition_domain_ops = {
> 	.select = gic_irq_domain_select,
> };
> 
>+static bool __maybe_unused gic_enable_quirk_msm8996(void *data)
>+{
>+	struct gic_chip_data *d = data;
>+
>+	d->flags |= FLAGS_WORKAROUND_GICR_WAKER_MSM8996;
>+
>+	return true;
>+}
>+
>+static const struct gic_quirk gic_quirks[] = {
>+	{
>+		.desc	= "GICv3: Qualcomm MSM8996 skip GICR_WAKER Read/Write",
>+		.iidr	= 0x00001070,	/* MSM8996 */
>+		.mask	= 0xffffffff,
>+		.init	= gic_enable_quirk_msm8996,
>+	},
>+};
>+
> static int __init gic_init_bases(void __iomem *dist_base,
> 				 struct redist_region *rdist_regs,
> 				 u32 nr_redist_regions,
> 				 u64 redist_stride,
> 				 struct fwnode_handle *handle)
> {
>-	u32 typer;
>+	u32 typer, iidr;
> 	int gic_irqs;
> 	int err;
> 
>@@ -1130,6 +1154,9 @@ static int __init gic_init_bases(void __iomem
>*dist_base,
> 	if (IS_ENABLED(CONFIG_ARM_GIC_V3_ITS) && gic_dist_supports_lpis())
> 		its_init(handle, &gic_data.rdists, gic_data.domain);
> 
>+	iidr = readl_relaxed(dist_base + GICD_IIDR);
>+	gic_enable_quirks(iidr, gic_quirks, &gic_data);
>+
> 	gic_smp_init();
> 	gic_dist_init();
> 	gic_cpu_init();

This bug also affects sdm660/630 and probably 8998 but I cant verify so it would probably make sense to loosen up the naming for the quirk.
--