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

[Ivan T. Ivanov]

On 06-08 15:16, Mark Rutland wrote:
> 
> 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.

What I am afraid is that "ic ivau address" will do cache invalidation of a 
random address, because of wrong address wiring. So the end result will not
be what should be expected.

> 
> > > 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;

Yep, I get this.

> we have IC IVAU instructions elsewhere in the kernel (e.g.
> arch/arm64/mm/cache.S).

But I have missed this one :-)

I think that this is working because these are used only 
in operations witch work up to the Point of Unification,
thus not messing up with caches of the rest of PE.

Regards,
Ivan