Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 04 Sep 2018 17:19:48 +0100
User-Agent: K-9 Mail for Android
In-Reply-To: <20180904140538.16602-1-srinivas.kandagatla@linaro.org>
References: <20180904140538.16602-1-srinivas.kandagatla@linaro.org>
MIME-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: quoted-printable
Subject: Re: [RFC PATCH v2] irqchip/gic-v3: Add quirk for msm8996 secured registers
To:     Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        marc.zyngier@arm.com
CC:     sudeep.holla@arm.com, tglx@linutronix.de, jason@lakedaemon.net,
        linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
        rnayak@codeaurora.org, sboyd@kernel.org,
        bjorn.andersson@linaro.org, nicolas.dechesne@linaro.org
From:   Craig Tatlor <ctatlor97@gmail.com>
Message-ID: <A0BB4E01-1CF6-49B1-9B5C-27FAC1DF821A@gmail.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


On 4 September 2018 15:05:38 BST, Srinivas Kandagatla <srinivas=2Ekandagat=
la@linaro=2Eorg> wrote:
>Access to GICR_WAKER is restricted on msm8996 SoC in Hypervisor=2E
>Its been more than 2 years of wait for this to be fixed, which has
>no hopes to be fixed=2E This change was introduced for the "lead device"
>on msm8996 platform=2E It looks like all publicly available msm8996
>devices have this implementation=2E
>
>So add a quirk to not access this register on msm8996=2E
>
>With this quirk MSM8996 can at least boot out of mainline,
>which can help community to work with boards based on MSM8996=2E
>
>Without this patch Qualcomm DB820c board reboots when GICR_WAKER
>is accessed=2E
>
>Signed-off-by: Srinivas Kandagatla <srinivas=2Ekandagatla@linaro=2Eorg>
>---
>Hi Marc,
>
>There is no errata associated with this quirk, so I could not add
>any entries into Documentation/arm64/silicon-errata=2Etxt Or add
>any number to the quirk description=2E
>
>Changes since v1:
>- renamed gic_v3 references to gic
>- added full mask for iidr register match
>
>thanks,
>srini
>
> drivers/irqchip/irq-gic-v3=2Ec | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/irqchip/irq-gic-v3=2Ec
>b/drivers/irqchip/irq-gic-v3=2Ec
>index d5912f1ec884=2E=2E406d4a44c887 100644
>--- a/drivers/irqchip/irq-gic-v3=2Ec
>+++ b/drivers/irqchip/irq-gic-v3=2Ec
>@@ -41,6 +41,8 @@
>=20
> #include "irq-gic-common=2Eh"
>=20
>+#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 =3D 1000000;	/* 1s! */
> 	u32 val;
>=20
>+	if (gic_data=2Eflags & FLAGS_WORKAROUND_GICR_WAKER_MSM8996)
>+		return;
>+
> 	rbase =3D gic_data_rdist_rd_base();
>=20
> 	val =3D readl_relaxed(rbase + GICR_WAKER);
>@@ -1068,13 +1074,31 @@ static const struct irq_domain_ops
>partition_domain_ops =3D {
> 	=2Eselect =3D gic_irq_domain_select,
> };
>=20
>+static bool __maybe_unused gic_enable_quirk_msm8996(void *data)
>+{
>+	struct gic_chip_data *d =3D data;
>+
>+	d->flags |=3D FLAGS_WORKAROUND_GICR_WAKER_MSM8996;
>+
>+	return true;
>+}
>+
>+static const struct gic_quirk gic_quirks[] =3D {
>+	{
>+		=2Edesc	=3D "GICv3: Qualcomm MSM8996 skip GICR_WAKER Read/Write",
>+		=2Eiidr	=3D 0x00001070,	/* MSM8996 */
>+		=2Emask	=3D 0xffffffff,
>+		=2Einit	=3D 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;
>=20
>@@ -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=2Erdists, gic_data=2Edomain);
>=20
>+	iidr =3D 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=2E
--