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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87k0d91xcf.wl-maz@kernel.org>
Date:   Fri, 04 Mar 2022 16:44:32 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Linu Cherian <lcherian@...vell.com>
Cc:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "will@...nel.org" <will@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linuc.decode@...il.com" <linuc.decode@...il.com>
Subject: Re: [EXT] Re: [PATCH V2] irqchip/gic-v3: Workaround Marvell erratum 38545 when reading IAR

Hi Linu,

On Fri, 04 Mar 2022 13:25:42 +0000,
Linu Cherian <lcherian@...vell.com> 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.