Re: [PATCH] arm64: errata: Add Ampere erratum AC04_CPU_50 workaround alternative

[Zaid Alali <zaidal@os.amperecomputing.com>]

On Tue, Jan 28, 2025 at 08:34:47AM +0000, Marc Zyngier wrote:
> On Mon, 27 Jan 2025 20:18:29 +0000,
> Zaid Alali <zaidal@os.amperecomputing.com> wrote:
> > 
> > Add an alternative code sequence to work around Ampere erratum
> > AC03_CPU_50 on AmpereOne and Ampere1A.
> > 
> > Due to AC03_CPU_50, when ICC_PMR_EL1 should have a value of 0xf0 a
> > direct read of the register will return a value of 0xf8. An incorrect
> > value from a direct read can only happen with the value 0xf0.
> 
> Under which precise conditions? Does it equally apply to virtual
> interrupts or SCR_EL3.FIQ==0, for which there is no non-secure shift
> (which I can only assume is the source of the erratum)? Does it
> equally affect G0 and G1 interrupts?
>

Hi Marc,

This only occurs when:
When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State,
and ICC_PMR_EL1.Priority==LowestPriorityImplemented==0xf8 (highest priority is 0x00).
 
Does it equally apply to virtual interrupts or SCR_EL3.FIQ==0?
 
Based on this Defect (AArch-21735) and our implementation, it only 
affected ICC_PMR_EL1, therefore does not apply to virtual interrupts.
It also does not apply when SCR_EL3.FIQ==0, as no modification of 
ICC_PMR_EL1 is required. Automatically sends the unshifted version.
 
For which there is no non-secure shift, does it equally affect G0 and G1 interrupts?
 
When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, 
and ICC_PMR_EL1.Priority==LowestPriorityImplemented==0xf8 (highest priority is 0x00):
The ICC_PMR_EL1.Priority read returns an unshifted version (0xf8).
It should have returned 0xf0. HW shifts the priority and inserts a 1b1 ensure
the priority is a Grp1 priority. Should only be visible in HW.
 
When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, and ICC_PMR_EL1.Priority==Grp0:
This issue would not occur. Returned value would be the highest priority, 0x00.
The returned value is correct as per ARM.
 
When SCR_EL3.FIQ==1 and PE is NOT in EL3/Secure State, and 
ICC_PMR_EL1.Priority==Grp1 (but not the lowest priority):
This issue would not occur. Returned value would be the shifted version.
The returned value is correct as per ARM.
 
> > 
> > Note: Currently there are no checks against a value of 0xf0, and that
> > save restore of 0xf8 -> 0xf0 is fine, so this is all future proofing.
> > 
> > Signed-off-by: Zaid Alali <zaidal@os.amperecomputing.com>
> > ---
> >  arch/arm64/Kconfig                  | 16 ++++++++++++++++
> >  arch/arm64/include/asm/arch_gicv3.h |  2 +-
> >  arch/arm64/include/asm/daifflags.h  |  4 ++--
> >  arch/arm64/include/asm/irqflags.h   |  6 +++---
> >  arch/arm64/include/asm/sysreg.h     |  9 +++++++++
> >  arch/arm64/kernel/cpu_errata.c      | 15 +++++++++++++++
> >  arch/arm64/kernel/entry.S           |  4 ++++
> >  arch/arm64/tools/cpucaps            |  1 +
> 
> Please add an entry to Documentation/arch/arm64/silicon-errata.txt.
> 
> >  8 files changed, 51 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index fcdd0ed3eca8..8d6e263d66c7 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -461,6 +461,22 @@ config AMPERE_ERRATUM_AC03_CPU_38
> >  
> >  	  If unsure, say Y.
> >  
> > +config AMPERE_ERRATUM_AC03_CPU_50
> > +	bool "AmpereOne: AC03_CPU_50: Certain checks for ICC_PMR_EL1 that expects the value 0xf0 may read 0xf8 instead"
> > +	default y
> > +	help
> > +	  This option adds an alternative code sequence to work around Ampere
> > +	  erratum AC03_CPU_50 on AmpereOne and Ampere1A.
> > +
> > +	  Due to AC03_CPU_50, when ICC_PMR_EL1 should have a value of 0xf0 a
> > +	  direct read of the register will return a value of 0xf8. An incorrect
> > +	  value from a direct read can only happen with the value 0xf0.
> > +
> > +	  The workaround for the erratum will do logical AND 0xf0 to the
> > +	  value read from ICC_PMR_EL1 register before returning the value.
> > +
> > +	  If unsure, say Y.
> > +
> 
> An alternative for this would simply to prevent the enabling of pNMI
> on this platform.
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.