lkml.org 
[lkml]   [2022]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [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@marvell.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.
\
 
 \ /
  Last update: 2022-03-04 17:45    [W:0.036 / U:0.388 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site