[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <05d97f210e484a07887f1e476fa4cae5@exch03.asrmicro.com>
Date: Thu, 18 May 2023 03:15:54 +0000
From: Yan Zheng(严政) <zhengyan@...micro.com>
To: Marc Zyngier <maz@...nel.org>
CC: "tglx@...utronix.de" <tglx@...utronix.de>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Gao Meitao(高玫涛)
<meitaogao@...micro.com>,
Zhou Qiao(周侨) <qiaozhou@...micro.com>,
Zhang Zhizhou(张治洲)
<zhizhouzhang@...micro.com>
Subject: RE: [PATCH] irqchip/gic-v3: workaround for ASR8601 when reading
mpidr
> -----Original Message-----
> From: Marc Zyngier [mailto:maz@...nel.org]
> Sent: Wednesday, May 17, 2023 8:59 PM
> To: Yan Zheng(严政) <zhengyan@...micro.com>
> Cc: tglx@...utronix.de; linux-kernel@...r.kernel.org; Gao Meitao(高玫涛)
> <meitaogao@...micro.com>; Zhou Qiao(周侨) <qiaozhou@...micro.com>;
> Zhang Zhizhou(张治洲) <zhizhouzhang@...micro.com>
> Subject: Re: [PATCH] irqchip/gic-v3: workaround for ASR8601 when reading
> mpidr
>
> On Wed, 17 May 2023 11:45:22 +0100,
> Yan Zheng(严政) <zhengyan@...micro.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Marc Zyngier [mailto:maz@...nel.org]
> > > Sent: Wednesday, May 17, 2023 4:32 PM
> > > To: Yan Zheng(严政) <zhengyan@...micro.com>
> > > Cc: tglx@...utronix.de; linux-kernel@...r.kernel.org; Gao
> > > Meitao(高玫涛)
> > > <meitaogao@...micro.com>; Zhou Qiao(周侨) <qiaozhou@...micro.com>;
> > > Zhang Zhizhou(张治洲) <zhizhouzhang@...micro.com>
> > > Subject: Re: [PATCH] irqchip/gic-v3: workaround for ASR8601 when
> > > reading mpidr
> > >
> > > On Wed, 17 May 2023 08:55:00 +0100,
> > > zhengyan <zhengyan@...micro.com> wrote:
> > > >
> > > > This patch add workaround for ASR8601, which uses an armv8.2
> > > > processor with a gic-500. ARMv8.2 uses Multiprocessor Affinity
> > > > Register to identify the logical address of the core by
> > > > | cluster | core | thread |.
> > >
> > > Not quite. The ARMv8.2 architecture doesn't say *any* of that. It is
> > > ARM's
> > > *implementations* that follow this scheme.
> > >
> >
> > Really thank you for rapid response,
> > Yes, as arm documents
> > https://developer.arm.com/docuentation/ka002107/latest said
>
> This page doesn't exist.
>
Sorry for my mistake,
https://developer.arm.com/documentation/ka002107/latest
extract from the docs:
The Arm-v8.0 CPUs use affinity 0 for a CPU and affinity 1 for a cluster.
All Arm-v8.2 CPUs use affinity 2 for a cluster, 1 for a CPU, and 0 for a thread.
The GIC-500 does not support affinity 2. Therefore, the GIC-500 cannot route
Shared Peripheral Interrupts (SPIs) or Software Generated Interrupts (SGIs)
correctly for Arm-v8.2 CPUs.
The GIC-500 does not have correct buses to connect the system together.
> > It comes from armv8.2 get 3 types for affinity (arm v8.0 cpus only get
> > 2 types) And it's an implementations issue.
>
> Again, this has nothing to do with the ARMv8.2 architecture. Nor the
> ARMv8.0 architecture. Please read the ARM ARM, which says absolutely
> *nothing* of what the various affinity levels are for.
>
As the extract from pervious part mentioned,
It seems like this issue related to that? I just want to mention that
this issue is cause by HW choice in the commit message.
> >
> > > > However, gic-500 only supports topologies with affinity levels
> > > > less than 2 as
> > > > | cluster | core|.
> > > >
> > > > So it needs this patch to shift the MPIDR values to ensure proper
> > > > functionality
> > > >
> > > > Signed-off-by: zhengyan <zhengyan@...micro.com>
> > > > ---
> > > > drivers/irqchip/irq-gic-v3.c | 28 +++++++++++++++++++++++++++-
> > > > 1 file changed, 27 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/irqchip/irq-gic-v3.c
> > > > b/drivers/irqchip/irq-gic-v3.c index 6fcee221f201..435b98a8641e
> > > > 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_MPIDR_ASR8601 (1ULL << 2)
> > >
> > > What is ASR8601? Is it a system? Or an erratum number? For issues
> > > that are the result of a HW integration issue, please provide an
> > > official erratum number, and update
> Documentation/arm64/silicon-errata.rst.
> > >
> >
> > ASR8601 is our soc's name, and yes it’s a kind of HW integration issue
> > But maybe it’s not an erratum since our HW design is like that,
> > although Arm doesn't recommend this way.
>
> Yes, for a good reason: it doesn't work. So this is *definitely* an erratum, no ifs,
> no buts.
>
Okay, I agree with you. Thanks for your advices.
> > And I would like to add more comments
> > Under the next part before *desc = "GICv3: ASR 8601 MPIDR shift"*
> > Maybe this is a better way? Or add something under Documentation?
>
> Documentation/arm64/silicon-errata.rst is the place to put it. Nowhere else.
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists