lkml.org 
[lkml]   [2018]   [Sep]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC PATCH v2] irqchip/gic-v3: Add quirk for msm8996 secured registers
From
Date
On 04/09/18 15:05, Srinivas Kandagatla 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@linaro.org>
> ---
> 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.

Well, we still need something. Specially given that this seems to affect
more than just the piece of HW you mention (see Craig's follow-up). This
is quite important, given that the IIDR stuff doesn't seem to work at
all (see below).

>
> 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 */

I just realized that this is the exact same ID as QDF2400, which is a
perfectly functional platform. So on top of being fairly broken, the
firmware also hijacks the ID of another implementation. I don't know if
I should laugh or cry. Needless to say, I'm not going to willingly break
a perfectly working platform.

> + .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();
>

Given the above, I'm not taking this patch any time soon. Please
consider alternative methods such as a different (non-)compatible string
to flag affected implementations.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

\
 
 \ /
  Last update: 2018-09-11 16:14    [W:0.037 / U:29.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site