Re: [PATCH v2] arm64: errata: Add NXP iMX8QM workaround for A53 Cache coherency issue

From: Mark Rutland <mark.rutland@arm.com>
To: "Ivan T. Ivanov" <iivanov@suse.de>
Cc: Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Mark Brown <broonie@kernel.org>, Shawn Guo <shawnguo@kernel.org>,
	Dong Aisheng <aisheng.dong@nxp.com>, Frank Li <frank.li@nxp.com>,
	Jason Liu <jason.hui.liu@nxp.com>,
	linux-arm-kernel@lists.infradead.org, linux-imx@nxp.com
Subject: Re: [PATCH v2] arm64: errata: Add NXP iMX8QM workaround for A53 Cache coherency issue
Date: Thu, 8 Jun 2023 15:16:16 +0100	[thread overview]
Message-ID: <ZIHisLL9FXlbuMLJ@FVFF77S0Q05N> (raw)
In-Reply-To: <m7vnrbgmkiqd3iinhsqiumvmquukju6t4n662monifeeecs24l@sjwihbs4fp5h>

On Thu, Jun 08, 2023 at 04:39:29PM +0300, Ivan T. Ivanov wrote:
> On 06-02 11:34, Will Deacon wrote:
> > On Thu, Apr 20, 2023 at 02:29:52PM +0300, Ivan T. Ivanov wrote:
> > > According to NXP errata document[1] i.MX8QuadMax SoC suffers from
> > > serious cache coherence issue. It was also mentioned in initial
> > > support[2] for imx8qm mek machine.
> > > 
> > > I chose to use an ALTERNATIVE() framework, instead downstream solution[3],
> > > for this issue with the hope to reduce effect of this fix on unaffected
> > > platforms.

> > > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> > > index 4a79ba100799..265b6334291b 100644
> > > --- a/arch/arm64/kernel/traps.c
> > > +++ b/arch/arm64/kernel/traps.c
> > > @@ -556,6 +556,11 @@ static void user_cache_maint_handler(unsigned long esr, struct pt_regs *regs)
> > >  		__user_cache_maint("dc civac", address, ret);
> > >  		break;
> > >  	case ESR_ELx_SYS64_ISS_CRM_IC_IVAU:	/* IC IVAU */
> > > +		if (cpus_have_final_cap(ARM64_WORKAROUND_NXP_ERR050104)) {
> > > +			asm volatile("ic ialluis");
> > 
> > Hmm, one oddity here is that you can pass a faulting address and not see
> > the fault. It looks like that's already IMP DEF, so it's probably ok, but
> > might be worth a comment.
> 
> I am not sure what should be expected behavior, but I could
> add comment, sure.

Another option is to make this:

	 case ESR_ELx_SYS64_ISS_CRM_IC_IVAU:     /* IC IVAU */
	 	__user_cache_maint("ic ivau", address, ret)
		if (cpus_have_final_cap(ARM64_WORKAROUND_NXP_ERR050104) && !ret)
			asm volatile("ic ialluis");
		break;

Which'll ensure that if the regular IC IVAU faults we'll handle that, and if
not we'll do the IC IALLUIS.

I think that looks a bit cleaner, too.

> > Finally, how come you don't need to upgrade I-cache invalidation by-VA
> > in the kernel? It looks like you're only handling operations trapped
> > from EL0.
> 
> Hm, I was thinking that __tlbi() is taking care for this or you mean 
> something else, like locations in assembler.h?

The __tlbi macro handles only TLBI instructions.

The trap handler above *only* handles IC instructions trapped from userspace;
we have IC IVAU instructions elsewhere in the kernel (e.g.
arch/arm64/mm/cache.S).

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel