Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220514080402.2650181-1-chenhuacai@loongson.cn>
 <20220514080402.2650181-11-chenhuacai@loongson.cn> <3982e7e7-f98e-8d8b-f13b-2bfa10a69b95@xen0n.name>
In-Reply-To: <3982e7e7-f98e-8d8b-f13b-2bfa10a69b95@xen0n.name>
From:   Huacai Chen <chenhuacai@gmail.com>
Date:   Sun, 15 May 2022 21:00:19 +0800
Message-ID: <CAAhV-H4FDk42Ci_PMjn5BSaaUy-X8aVHWMEOj5_np8K8peTd0w@mail.gmail.com>
Subject: Re: [PATCH V10 10/22] LoongArch: Add exception/interrupt handling
To:     WANG Xuerui <kernel@xen0n.name>
Cc:     Huacai Chen <chenhuacai@loongson.cn>,
        Arnd Bergmann <arnd@arndb.de>,
        Andy Lutomirski <luto@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        David Airlie <airlied@linux.ie>,
        Jonathan Corbet <corbet@lwn.net>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        linux-arch <linux-arch@vger.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Xuefeng Li <lixuefeng@loongson.cn>,
        Yanteng Si <siyanteng@loongson.cn>,
        Guo Ren <guoren@kernel.org>,
        Jiaxun Yang <jiaxun.yang@flygoat.com>,
        Stephen Rothwell <sfr@canb.auug.org.au>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi, Xuerui,

On Sun, May 15, 2022 at 5:07 PM WANG Xuerui <kernel@xen0n.name> wrote:
>
> Hi,
>
> On 5/14/22 16:03, Huacai Chen wrote:
> > Add the exception and interrupt handling machanism for basic LoongArch
> > support.
> >
> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn>
> > ---
> >   arch/loongarch/include/asm/branch.h       |  21 +
> >   arch/loongarch/include/asm/bug.h          |  23 +
> >   arch/loongarch/include/asm/entry-common.h |  13 +
> >   arch/loongarch/include/asm/hardirq.h      |  24 +
> >   arch/loongarch/include/asm/hw_irq.h       |  17 +
> >   arch/loongarch/include/asm/irq.h          | 130 ++++
> >   arch/loongarch/include/asm/irq_regs.h     |  27 +
> >   arch/loongarch/include/asm/irqflags.h     |  78 +++
> >   arch/loongarch/include/asm/kdebug.h       |  23 +
> >   arch/loongarch/include/asm/stackframe.h   | 212 ++++++
> >   arch/loongarch/include/asm/stacktrace.h   |  74 +++
> >   arch/loongarch/include/uapi/asm/break.h   |  23 +
> >   arch/loongarch/kernel/access-helper.h     |  13 +
> >   arch/loongarch/kernel/genex.S             |  95 +++
> >   arch/loongarch/kernel/irq.c               | 131 ++++
> >   arch/loongarch/kernel/traps.c             | 755 ++++++++++++++++++++++
> >   16 files changed, 1659 insertions(+)
> >   create mode 100644 arch/loongarch/include/asm/branch.h
> >   create mode 100644 arch/loongarch/include/asm/bug.h
> >   create mode 100644 arch/loongarch/include/asm/entry-common.h
> >   create mode 100644 arch/loongarch/include/asm/hardirq.h
> >   create mode 100644 arch/loongarch/include/asm/hw_irq.h
> >   create mode 100644 arch/loongarch/include/asm/irq.h
> >   create mode 100644 arch/loongarch/include/asm/irq_regs.h
> >   create mode 100644 arch/loongarch/include/asm/irqflags.h
> >   create mode 100644 arch/loongarch/include/asm/kdebug.h
> >   create mode 100644 arch/loongarch/include/asm/stackframe.h
> >   create mode 100644 arch/loongarch/include/asm/stacktrace.h
> >   create mode 100644 arch/loongarch/include/uapi/asm/break.h
> >   create mode 100644 arch/loongarch/kernel/access-helper.h
> >   create mode 100644 arch/loongarch/kernel/genex.S
> >   create mode 100644 arch/loongarch/kernel/irq.c
> >   create mode 100644 arch/loongarch/kernel/traps.c
> This patch mostly looks good, except...
> > (snip)
> >
> > +asmlinkage void cache_parity_error(void)
> > +{
> > +     const int field = 2 * sizeof(unsigned long);
> > +     unsigned int reg_val;
> > +
> > +     /* For the moment, report the problem and hang. */
> > +     pr_err("Cache error exception:\n");
> > +     pr_err("csr_merrera == %0*llx\n", field, csr_readq(LOONGARCH_CSR_MERRERA));
> > +     reg_val = csr_readl(LOONGARCH_CSR_MERRCTL);
> > +     pr_err("csr_merrctl == %08x\n", reg_val);
> > +
> > +     pr_err("Decoded c0_cacheerr: %s cache fault in %s reference.\n",
> > +            reg_val & (1<<30) ? "secondary" : "primary",
> > +            reg_val & (1<<31) ? "data" : "insn");
> > +     if (((current_cpu_data.processor_id & 0xff0000) == PRID_COMP_LOONGSON)) {
> > +             pr_err("Error bits: %s%s%s%s%s%s%s%s\n",
> > +                     reg_val & (1<<29) ? "ED " : "",
> > +                     reg_val & (1<<28) ? "ET " : "",
> > +                     reg_val & (1<<27) ? "ES " : "",
> > +                     reg_val & (1<<26) ? "EE " : "",
> > +                     reg_val & (1<<25) ? "EB " : "",
> > +                     reg_val & (1<<24) ? "EI " : "",
> > +                     reg_val & (1<<23) ? "E1 " : "",
> > +                     reg_val & (1<<22) ? "E0 " : "");
> > +     } else {
> > +             pr_err("Error bits: %s%s%s%s%s%s%s\n",
> > +                     reg_val & (1<<29) ? "ED " : "",
> > +                     reg_val & (1<<28) ? "ET " : "",
> > +                     reg_val & (1<<26) ? "EE " : "",
> > +                     reg_val & (1<<25) ? "EB " : "",
> > +                     reg_val & (1<<24) ? "EI " : "",
> > +                     reg_val & (1<<23) ? "E1 " : "",
> > +                     reg_val & (1<<22) ? "E0 " : "");
> > +     }
> > +     pr_err("IDX: 0x%08x\n", reg_val & ((1<<22)-1));
> > +
> > +     panic("Can't handle the cache error!");
> > +}
>
> ... this function. This implementation is completely wrong, as it's the
> same logic on MIPS, but LoongArch's MERRCTL CSR is not arranged in the
> same way. There are no individual error bits, for example.
>
> You can simply replace this with a direct panic for now, for correctness.
Thank you very much, this is my fault.

Huacai
>
> With this fixed:
>
> Reviewed-by: WANG Xuerui <git@xen0n.name>
>