[RFC patch 4/4] riscv: sifive: apply errata "cip-453" patch
Vincent Chen
vincent.chen at sifive.com
Fri Mar 12 03:50:07 GMT 2021
On Wed, Mar 10, 2021 at 12:39 PM Palmer Dabbelt <palmer at dabbelt.com> wrote:
>
> On Sun, 07 Mar 2021 19:58:17 PST (-0800), vincent.chen at sifive.com wrote:
> > Add sign extension to the $badaddr before addressing the instruction page
> > fault and instruction access fault to workaround the issue "cip-453".
>
> The documentation I see quite explicitly says that this bug doesn't manifest in
> Linux. IDK if I'm just reading an old one, but if not that should be fixed as
> presumably it does actually manifest?
>
> > To avoid affecting the existing code sequence, this patch will creates two
> > trampolines to add sign extension to the $badaddr. By the "alternative"
> > mechanism, these two trampolines will replace the original exception
> > handler of instruction page fault and instruction access fault in the
> > excp_vect_table. In this case, only the specific SiFive CPU core jumps to
> > the do_page_fault and do_trap_insn_fault through these two trampolines.
> > Other CPUs are not affected.
> >
> > Signed-off-by: Vincent Chen <vincent.chen at sifive.com>
> > ---
> > arch/riscv/Kconfig.erratas | 20 +++++++++++
> > arch/riscv/Kconfig.socs | 1 +
> > arch/riscv/errata/Makefile | 1 +
> > arch/riscv/errata/sifive/Makefile | 2 ++
> > arch/riscv/errata/sifive/errata.c | 56 +++++++++++++++++++++++++++++++
> > arch/riscv/errata/sifive/errata_cip_453.S | 34 +++++++++++++++++++
> > arch/riscv/include/asm/errata_list.h | 3 +-
> > arch/riscv/kernel/entry.S | 12 +++++--
> > 8 files changed, 126 insertions(+), 3 deletions(-)
> > create mode 100644 arch/riscv/errata/sifive/Makefile
> > create mode 100644 arch/riscv/errata/sifive/errata.c
> > create mode 100644 arch/riscv/errata/sifive/errata_cip_453.S
> >
> > diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
> > index 4d0bafc536df..907fa6b13ee2 100644
> > --- a/arch/riscv/Kconfig.erratas
> > +++ b/arch/riscv/Kconfig.erratas
> > @@ -9,4 +9,24 @@ config RISCV_ERRATA_ALTERNATIVE
> > code patching is performed once in the boot stages. It means
> > that the overhead from this mechanism is just taken once.
> >
> > +config ERRATA_SIFIVE
> > + bool "SiFive errata"
> > + help
> > + All SiFive errata Kconfig depend on this Kconfig. Please say "Y"
> > + here if your platform uses SiFive CPU cores.
> > +
> > + Otherwise, please say "N" here to avoid unnecessary overhead.
> > +
> > +config ERRATA_SIFIVE_CIP_453
> > + bool "Apply SiFive errata CIP-453"
> > + depends on ERRATA_SIFIVE
> > + depends on RISCV_ERRATA_ALTERNATIVE
> > + default y
> > + help
> > + This will apply the SiFive CIP-453 errata to add sign extension
> > + to the $badaddr when exception type is instruction page fault
> > + and instruction access fault.
> > +
> > + If you don't know what to do here, say "Y".
> > +
> > endmenu
> > diff --git a/arch/riscv/Kconfig.socs b/arch/riscv/Kconfig.socs
> > index 7efcece8896c..b9eda857fc87 100644
> > --- a/arch/riscv/Kconfig.socs
> > +++ b/arch/riscv/Kconfig.socs
> > @@ -7,6 +7,7 @@ config SOC_SIFIVE
> > select CLK_SIFIVE
> > select CLK_SIFIVE_PRCI
> > select SIFIVE_PLIC
> > + select ERRATA_SIFIVE
> > help
> > This enables support for SiFive SoC platform hardware.
> >
> > diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
> > index 43e6d5424367..b8f8740a3e44 100644
> > --- a/arch/riscv/errata/Makefile
> > +++ b/arch/riscv/errata/Makefile
> > @@ -1 +1,2 @@
> > obj-y += alternative.o
> > +obj-$(CONFIG_ERRATA_SIFIVE) += sifive/
> > diff --git a/arch/riscv/errata/sifive/Makefile b/arch/riscv/errata/sifive/Makefile
> > new file mode 100644
> > index 000000000000..b7f4cd7ee185
> > --- /dev/null
> > +++ b/arch/riscv/errata/sifive/Makefile
> > @@ -0,0 +1,2 @@
> > +obj-y += altern_ops.o
> > +obj-y += errata_cip_453.o
> > diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
> > new file mode 100644
> > index 000000000000..0b0a9af42a55
> > --- /dev/null
> > +++ b/arch/riscv/errata/sifive/errata.c
> > @@ -0,0 +1,56 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2021 Sifive.
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/string.h>
> > +#include <linux/bug.h>
> > +#include <asm/alternative.h>
> > +#include <asm/vendorid_list.h>
> > +#include <asm/errata_list.h>
> > +
> > +#define MAX_ERRATA_IMPID 5
>
> Why 5? I only see 2.
>
This is my mistake. I forgot to modify it after I completed some
testing. I will modify it.
> > +struct errata_info_t {
> > + char name[ERRATA_STRING_LENGTH_MAX];
> > + unsigned long arch_id;
> > + unsigned long imp_id[MAX_ERRATA_IMPID];
> > +} errata_info;
> > +
> > +struct errata_info_t sifive_errata_list[ERRATA_NUMBER] = {
> > + {
> > + .name = "cip-453",
> > + .arch_id = 0x8000000000000007,
> > + .imp_id = {
> > + 0x20181004, 0x00200504
>
> Is there any way to figure these out from the documentation?
>
Sorry. Currently, there is no public document to record the information.
> > + },
> > + },
> > +};
> > +
> > +static inline bool __init cpu_info_cmp(struct errata_info_t *errata, struct alt_entry *alt)
> > +{
> > + int i;
> > +
> > + if (errata->arch_id != cpu_mfr_info.arch_id)
> > + return false;
> > +
> > + for (i = 0; i < MAX_ERRATA_IMPID && errata->imp_id[i]; i++)
> > + if (errata->imp_id[i] == cpu_mfr_info.imp_id)
> > + return true;
> > +
> > + return false;
> > +}
> > +
> > +static bool __init sifive_errata_check(struct alt_entry *alt)
> > +{
> > + if (cpu_mfr_info.vendor_id != alt->vendor_id)
> > + return false;
> > +
> > + if (likely(alt->errata_id < ERRATA_NUMBER))
> > + return cpu_info_cmp(&sifive_errata_list[alt->errata_id], alt);
> > +
> > + WARN_ON(1);
> > + return false;
> > +}
> > +
> > +REGISTER_ERRATA_CHECKFUNC(sifive_errata_check, SIFIVE_VENDOR_ID);
> > diff --git a/arch/riscv/errata/sifive/errata_cip_453.S b/arch/riscv/errata/sifive/errata_cip_453.S
> > new file mode 100644
> > index 000000000000..34d0fe26172e
> > --- /dev/null
> > +++ b/arch/riscv/errata/sifive/errata_cip_453.S
> > @@ -0,0 +1,34 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2021 SiFive
> > + */
> > +
> > +#include <linux/linkage.h>
> > +#include <asm/asm.h>
> > +#include <asm/asm-offsets.h>
> > +#include <asm/alternative.h>
> > +
> > +.macro ADD_SIGN_EXT pt_reg badaddr tmp_reg
> > + REG_L \badaddr, PT_BADADDR(\pt_reg)
> > + li \tmp_reg,1
> > + slli \tmp_reg,\tmp_reg,0x26
> > + and \tmp_reg,\tmp_reg,\badaddr
> > + beqz \tmp_reg, 1f
> > + li \tmp_reg,-1
> > + slli \tmp_reg,\tmp_reg,0x27
> > + or \badaddr,\tmp_reg,\badaddr
> > + REG_S \badaddr, PT_BADADDR(\pt_reg)
> > +1:
> > +.endm
> > +
> > +ENTRY(do_page_fault_trampoline)
> > + ADD_SIGN_EXT a0, t0, t1
> > + la t0, do_page_fault
> > + jr t0
> > +END(do_page_fault_trampoline)
> > +
> > +ENTRY(do_trap_insn_fault_trampoline)
> > + ADD_SIGN_EXT a0, t0, t1
> > + la t0, do_trap_insn_fault
> > + jr t0
> > +END(do_trap_insn_fault_trampoline)
>
> These should get names that are more specific to the errata in question, as
> they're global symbols.
>
OK
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > index 5e5a1fcd90ba..d3752c8eff1f 100644
> > --- a/arch/riscv/include/asm/errata_list.h
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -4,5 +4,6 @@
> > */
> >
> > #ifdef CONFIG_SOC_SIFIVE
> > -#define ERRATA_NUMBER 0
> > +#define ERRATA_CIP_453 0
> > +#define ERRATA_NUMBER 1
> > #endif
> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> > index 744f3209c48d..821e86ee67e4 100644
> > --- a/arch/riscv/kernel/entry.S
> > +++ b/arch/riscv/kernel/entry.S
> > @@ -12,6 +12,9 @@
> > #include <asm/unistd.h>
> > #include <asm/thread_info.h>
> > #include <asm/asm-offsets.h>
> > +#include <asm/alternative.h>
> > +#include <asm/vendorid_list.h>
> > +#include <asm/errata_list.h>
> >
> > #if !IS_ENABLED(CONFIG_PREEMPTION)
> > .set resume_kernel, restore_all
> > @@ -450,7 +453,9 @@ ENDPROC(__switch_to)
> > /* Exception vector table */
> > ENTRY(excp_vect_table)
> > RISCV_PTR do_trap_insn_misaligned
> > - RISCV_PTR do_trap_insn_fault
> > + ALTERNATIVE(__stringify(RISCV_PTR do_trap_insn_fault),
> > + __stringify(RISCV_PTR do_trap_insn_fault_trampoline),
> > + SIFIVE_VENDOR_ID, ERRATA_CIP_453, CONFIG_ERRATA_SIFIVE_CIP_453)
>
> You've only got the instruction patching function in the errata framework, but
> this is data. It doesn't really matter, as we fence before the other CPUs come
> online, but a comment in there that both are OK would be good.
>
OK. I got it. Thanks
> > RISCV_PTR do_trap_insn_illegal
> > RISCV_PTR do_trap_break
> > RISCV_PTR do_trap_load_misaligned
> > @@ -461,7 +466,10 @@ ENTRY(excp_vect_table)
> > RISCV_PTR do_trap_ecall_s
> > RISCV_PTR do_trap_unknown
> > RISCV_PTR do_trap_ecall_m
> > - RISCV_PTR do_page_fault /* instruction page fault */
> > + /* instruciton page fault */
> > + ALTERNATIVE(__stringify(RISCV_PTR do_page_fault),
> > + __stringify(RISCV_PTR do_page_fault_trampoline),
> > + SIFIVE_VENDOR_ID, ERRATA_CIP_453, CONFIG_ERRATA_SIFIVE_CIP_453)
> > RISCV_PTR do_page_fault /* load page fault */
> > RISCV_PTR do_trap_unknown
> > RISCV_PTR do_page_fault /* store page fault */
More information about the linux-riscv
mailing list