From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 10C22C77B7E for ; Mon, 29 May 2023 15:02:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229835AbjE2PCV (ORCPT ); Mon, 29 May 2023 11:02:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50394 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229654AbjE2PCT (ORCPT ); Mon, 29 May 2023 11:02:19 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8BF0AAD for ; Mon, 29 May 2023 08:02:17 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 1E38F625F5 for ; Mon, 29 May 2023 15:02:17 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5D00AC433EF; Mon, 29 May 2023 15:02:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1685372536; bh=vagnwYkF/M0s80z8AYqejbdkvgbJQQlnjBPiq81emPY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=npDO2Veeyz+kj399nU8IyHp/kJLssER2ahHR3mH2pLgcUvndMVDkS/8uwXOEL/SOR p3UtckAaps02LL8nC9BdbRYYrQe8ZCNrW39Gs2XGaV8YRTzx6DxJEKrh7/kMGsKHNk 2mYKglSDk//Z99IOVFs1UsgwX/h3qhcwfIjDLcDfCv8T3S43wJmGMftVD/G3kFr9PA iB4YsQ1krMlITGlKtoDHIQnzSakpZCDcDTaySxjC1hBnsNj558YcVIdfpO4Pk/CF2g Df//QLLjwfFx9Kaq5WhwRJtDBRt+5tS4FMjd4moQt70BP4laletBs7ecj8nR0kxXGu O2WdBBWzEN0Jw== Received: from sofa.misterjones.org ([185.219.108.64] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1q3eNy-00162v-6Y; Mon, 29 May 2023 16:02:14 +0100 Date: Mon, 29 May 2023 16:02:13 +0100 Message-ID: <87sfbfji4q.wl-maz@kernel.org> From: Marc Zyngier To: zhengyan Cc: linux-kernel@vger.kernel.org, meitaogao@asrmicro.com, qiaozhou@asrmicro.com, tglx@linutronix.de, zhizhouzhang@asrmicro.com Subject: Re: [PATCH v2] irqchip/gic-v3: workaround for ASR8601 when reading mpidr In-Reply-To: <20230522110643.3063073-1-zhengyan@asrmicro.com> References: <20230517075500.43516-1-zhengyan@asrmicro.com> <20230522110643.3063073-1-zhengyan@asrmicro.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/28.2 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: zhengyan@asrmicro.com, linux-kernel@vger.kernel.org, meitaogao@asrmicro.com, qiaozhou@asrmicro.com, tglx@linutronix.de, zhizhouzhang@asrmicro.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 22 May 2023 12:06:43 +0100, zhengyan wrote: > > This patch add workaround for ASR8601, which uses an armv8.2 > processor with a gic-500. But gic-500 is incompatible with > ARMv8.2 implementations from ARM. > > ARMv8.2 from ARM implementation uses Multiprocessor Affinity > Register to identify the logical address of the core by > | cluster | core | thread |. > However, gic-500 only supports topologies with > affinity levels less than 2 as > | cluster | core|. > > So we need this patch as workaround to shift the MPIDR values > to ensure proper functionality > > Signed-off-by: zhengyan > --- > Documentation/arm64/silicon-errata.rst | 4 ++++ > drivers/irqchip/irq-gic-v3.c | 30 ++++++++++++++++++++++++++ > 2 files changed, 34 insertions(+) > > diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst > index 9e311bc43e05..d6430ade349d 100644 > --- a/Documentation/arm64/silicon-errata.rst > +++ b/Documentation/arm64/silicon-errata.rst > @@ -214,3 +214,7 @@ stable kernels. > +----------------+-----------------+-----------------+-----------------------------+ > | Fujitsu | A64FX | E#010001 | FUJITSU_ERRATUM_010001 | > +----------------+-----------------+-----------------+-----------------------------+ > + > ++----------------+-----------------+-----------------+-----------------------------+ > +| ASR | ASR8601 | #8601001 | N/A | > ++----------------+-----------------+-----------------+-----------------------------+ > diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c > index 6fcee221f201..cf64783dfe70 100644 > --- a/drivers/irqchip/irq-gic-v3.c > +++ b/drivers/irqchip/irq-gic-v3.c > @@ -39,6 +39,9 @@ > > #define FLAGS_WORKAROUND_GICR_WAKER_MSM8996 (1ULL << 0) > #define FLAGS_WORKAROUND_CAVIUM_ERRATUM_38539 (1ULL << 1) > +#define FLAGS_WORKAROUND_ASR_ERRATUM_8601001 (1ULL << 2) > + > +#define ASR8601_AFF_QUIRK(aff) (aff >> 8) This is wrong. There are more than just affinity bits in MPIDR, and you're making a mess of the result. > > #define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1) > > @@ -659,6 +662,9 @@ static u64 gic_mpidr_to_affinity(unsigned long mpidr) > { > u64 aff; > > + if (gic_data.flags & FLAGS_WORKAROUND_ASR_ERRATUM_8601001) > + mpidr = ASR8601_AFF_QUIRK(mpidr); > + > aff = ((u64)MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 | > MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 | > MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 | > @@ -970,6 +976,9 @@ static int __gic_populate_rdist(struct redist_region *region, void __iomem *ptr) > * Convert affinity to a 32bit value that can be matched to > * GICR_TYPER bits [63:32]. > */ > + if (gic_data.flags & FLAGS_WORKAROUND_ASR_ERRATUM_8601001) > + mpidr = ASR8601_AFF_QUIRK(mpidr); It really wasn't what I had in mind when I asked you to place this inside a helper. The whole thing looks horrible, and I really don't want to have to maintain anything like it. I came up with the following patch, which keeps the workaround *in a single spot*. Let me know if that works for you. M. diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 6fcee221f201..b7d69ef4da9f 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_ASR_ERRATUM_8601001 (1ULL << 2) #define GIC_IRQ_TYPE_PARTITION (GIC_IRQ_TYPE_LPI + 1) @@ -655,10 +656,16 @@ static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu) return 0; } -static u64 gic_mpidr_to_affinity(unsigned long mpidr) +static u64 gic_cpu_to_affinity(int cpu) { + u64 mpidr = cpu_logical_map(cpu); u64 aff; + /* ASR8601 needs to have its affinities shifted down... */ + if (unlikely(gic_data.flags & FLAGS_WORKAROUND_ASR_ERRATUM_8601001)) + mpidr = (MPIDR_AFFINITY_LEVEL(mpidr, 1) | + (MPIDR_AFFINITY_LEVEL(mpidr, 2) << 8)); + aff = ((u64)MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 | MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 | MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 | @@ -913,7 +920,7 @@ static void __init gic_dist_init(void) * Set all global interrupts to the boot CPU only. ARE must be * enabled. */ - affinity = gic_mpidr_to_affinity(cpu_logical_map(smp_processor_id())); + affinity = gic_cpu_to_affinity(smp_processor_id()); for (i = 32; i < GIC_LINE_NR; i++) gic_write_irouter(affinity, base + GICD_IROUTER + i * 8); @@ -962,7 +969,7 @@ static int gic_iterate_rdists(int (*fn)(struct redist_region *, void __iomem *)) static int __gic_populate_rdist(struct redist_region *region, void __iomem *ptr) { - unsigned long mpidr = cpu_logical_map(smp_processor_id()); + unsigned long mpidr; u64 typer; u32 aff; @@ -970,6 +977,8 @@ static int __gic_populate_rdist(struct redist_region *region, void __iomem *ptr) * Convert affinity to a 32bit value that can be matched to * GICR_TYPER bits [63:32]. */ + mpidr = gic_cpu_to_affinity(smp_processor_id()); + aff = (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 | MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 | MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 | @@ -1262,9 +1271,11 @@ static u16 gic_compute_target_list(int *base_cpu, const struct cpumask *mask, unsigned long cluster_id) { int next_cpu, cpu = *base_cpu; - unsigned long mpidr = cpu_logical_map(cpu); + unsigned long mpidr; u16 tlist = 0; + mpidr = gic_cpu_to_affinity(cpu); + while (cpu < nr_cpu_ids) { tlist |= 1 << (mpidr & 0xf); @@ -1273,8 +1284,7 @@ static u16 gic_compute_target_list(int *base_cpu, const struct cpumask *mask, goto out; cpu = next_cpu; - mpidr = cpu_logical_map(cpu); - + mpidr = gic_cpu_to_affinity(cpu); if (cluster_id != MPIDR_TO_SGI_CLUSTER_ID(mpidr)) { cpu--; goto out; @@ -1318,7 +1328,7 @@ static void gic_ipi_send_mask(struct irq_data *d, const struct cpumask *mask) dsb(ishst); for_each_cpu(cpu, mask) { - u64 cluster_id = MPIDR_TO_SGI_CLUSTER_ID(cpu_logical_map(cpu)); + u64 cluster_id = MPIDR_TO_SGI_CLUSTER_ID(gic_cpu_to_affinity(cpu)); u16 tlist; tlist = gic_compute_target_list(&cpu, mask, cluster_id); @@ -1376,7 +1386,7 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *mask_val, offset = convert_offset_index(d, GICD_IROUTER, &index); reg = gic_dist_base(d) + offset + (index * 8); - val = gic_mpidr_to_affinity(cpu_logical_map(cpu)); + val = gic_cpu_to_affinity(cpu); gic_write_irouter(val, reg); @@ -1786,6 +1796,15 @@ static bool gic_enable_quirk_nvidia_t241(void *data) return true; } +static bool gic_enable_quirk_asr8601(void *data) +{ + struct gic_chip_data *d = data; + + d->flags |= FLAGS_WORKAROUND_ASR_ERRATUM_8601001; + + return true; +} + static const struct gic_quirk gic_quirks[] = { { .desc = "GICv3: Qualcomm MSM8996 broken firmware", @@ -1823,6 +1842,11 @@ static const struct gic_quirk gic_quirks[] = { .mask = 0xffffffff, .init = gic_enable_quirk_nvidia_t241, }, + { + .desc = "GICv3: ASR erratum 8601001", + .compatible = "asr,asr8601-gic-v3", + .init = gic_enable_quirk_asr8601, + }, { } }; -- Without deviation from the norm, progress is not possible.