Re: [EXT] Re: [PATCH V2] irqchip/gic-v3: Workaround Marvell erratum 38545 when reading IAR

[Marc Zyngier]

Hi Linu,

On Fri, 04 Mar 2022 13:25:42 +0000,
Linu Cherian <lcherian@xxxxxxxxxxx> wrote:
> 
> Hi Marc,
> 
> > >  static inline u64 gic_read_iar_cavium_thunderx(void)
> > >  {
> > > -	u64 irqstat;
> > > +	u64 irqstat, apr;
> > >
> > > +	apr = read_sysreg_s(SYS_ICC_AP1R0_EL1);
> > 
> > Why only AP1R0? Does the HW only support 5 bits of priority? If it supports
> > more, you need to check all the registers that may contain an active priority
> > (0xa0 for a standard interrupt, 0x20 for a pNMI).
> > 
> 
> Yes correct. HW supports only 5 bits of priority groups.
> Will note this in the comment.

Thanks.

> 
> > >  	nops(8);
> > >  	irqstat = read_sysreg_s(SYS_ICC_IAR1_EL1);
> > >  	nops(4);
> > >  	mb();
> > >
> > > -	return irqstat;
> > > +	if (likely(apr != read_sysreg_s(SYS_ICC_AP1R0_EL1)))
> > > +		return irqstat;
> > > +
> > > +	return 0x3ff;
> > 
> > This should be ICC_IAR1_EL1_SPURIOUS.
> 
> Looks like we need fixes like below in couple of files to make use
> of this macro.
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 5bc01e62c08a..d02b7339d21a 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -18,7 +18,7 @@
>  #include <linux/kvm_types.h>
>  #include <linux/percpu.h>
>  #include <linux/psci.h>
> -#include <asm/arch_gicv3.h>
> +#include <linux/irqchip/arm-gic-v3.h>
> 
> Should I consider fixing these ? 
> At least  its builds fine for me with similar header fixes.

Ah, crap. I'd like to avoid dragging too much of the linux/*.h into
asm/*.h, as this eventually leads to a pretty terrible mess. Never
mind then. I'll look into fixing it independently, and we'll live with
the 0x3ff for now.

> > > +#define CAVIUM_CPU_PART_THUNDERX_OTX_GEN 0x0A0
> > 
> > Is this an actual part number? What does 'GEN' stand for?
> > 
> 
> No, this is not an actual part number. GEN was meant to be generic
> to cover a group of part numbers.

The problem with that is that it eventually clashes with part numbers
that are allocated later, and your old kernel tries to apply a
workaround on the new HW... Sticking to the actual parts is a lot
safer.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.