[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <mhng-c1c0b3c0-f358-40ce-ba1d-e33d2b85d9cc@palmer-ri-x1c9>
Date: Wed, 05 Feb 2025 07:44:03 -0800 (PST)
From: Palmer Dabbelt <palmer@...belt.com>
To: arikalo@...il.com
CC: linux-riscv@...ts.infradead.org, Paul Walmsley <paul.walmsley@...ive.com>,
aou@...s.berkeley.edu, ajones@...tanamicro.com, christoph.muellner@...ll.eu, arikalo@...il.com,
linux-kernel@...r.kernel.org, djordje.todorovic@...cgroup.com
Subject: Re: [PATCH v3] riscv: Fix the PAUSE Opcode for MIPS P8700.
On Wed, 29 Jan 2025 05:17:03 PST (-0800), arikalo@...il.com wrote:
> From: Djordje Todorovic <djordje.todorovic@...cgroup.com>
>
> The riscv MIPS P8700 uses a different opcode for PAUSE.
> It is a ‘hint’ encoding of the SLLI instruction, with rd=0, rs1=0 and
> imm=5. It will behave as a NOP instruction if no additional behavior
> beyond that of SLLI is implemented.
We need to expose it to userspace, then, as it's a user-visible erratum
-- really it's just a totally different custom extension than
Zihintpause, but I don't think it really makes much of a difference what
we call it.
If you have other custom stuff it's probably a good time to expose
those, too, as then it'll be easier for userspace to sort out.
> Add ERRATA_MIPS and ERRATA_MIPS_P8700_PAUSE_OPCODE configs.
> Handle errata for MIPS CPUs.
>
> Signed-off-by: Djordje Todorovic <djordje.todorovic@...cgroup.com>
> Signed-off-by: Aleksandar Rikalo <arikalo@...il.com>
> Signed-off-by: Raj Vishwanathan4 <rvishwanathan@...s.com>
> ---
> arch/riscv/Kconfig.errata | 23 +++++++++++
> arch/riscv/errata/Makefile | 1 +
> arch/riscv/errata/mips/Makefile | 5 +++
> arch/riscv/errata/mips/errata.c | 41 +++++++++++++++++++
> arch/riscv/include/asm/alternative.h | 3 ++
> arch/riscv/include/asm/cmpxchg.h | 3 +-
> arch/riscv/include/asm/errata_list.h | 20 ++++++++-
> arch/riscv/include/asm/vdso/processor.h | 4 +-
> arch/riscv/include/asm/vendorid_list.h | 1 +
> arch/riscv/kernel/alternative.c | 5 +++
> arch/riscv/kernel/entry.S | 2 +
> arch/riscv/mm/init.c | 1 +
> tools/arch/riscv/include/asm/vdso/processor.h | 5 ++-
> 13 files changed, 110 insertions(+), 4 deletions(-)
> create mode 100644 arch/riscv/errata/mips/Makefile
> create mode 100644 arch/riscv/errata/mips/errata.c
>
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index 2acc7d876e1f..1589528fdc30 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -21,6 +21,29 @@ config ERRATA_ANDES_CMO
>
> If you don't know what to do here, say "Y".
>
> +config ERRATA_MIPS
> + bool "MIPS errata"
> + depends on RISCV_ALTERNATIVE
> + help
> + All MIPS errata Kconfig depend on this Kconfig. Disabling
> + this Kconfig will disable all MIPS errata. Please say "Y"
> + here if your platform uses MIPS CPU cores.
> +
> + Otherwise, please say "N" here to avoid unnecessary overhead.
> +
> +config ERRATA_MIPS_P8700_PAUSE_OPCODE
> + bool "Fix the PAUSE Opcode for MIPS P8700"
> + depends on ERRATA_MIPS && 64BIT
> + default n
> + help
> + The RISCV MIPS P8700 uses a different opcode for PAUSE.
> + It is a 'hint' encoding of the SLLI instruction,
> + with rd=0, rs1=0 and imm=5. It will behave as a NOP
> + instruction if no additional behavior beyond that of
> + SLLI is implemented.
> +
> + If you are not using the P8700 processor, say n.
> +
> config ERRATA_SIFIVE
> bool "SiFive errata"
> depends on RISCV_ALTERNATIVE
> diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
> index f0da9d7b39c3..156cafb338c1 100644
> --- a/arch/riscv/errata/Makefile
> +++ b/arch/riscv/errata/Makefile
> @@ -9,5 +9,6 @@ endif
> endif
>
> obj-$(CONFIG_ERRATA_ANDES) += andes/
> +obj-$(CONFIG_ERRATA_MIPS) += mips/
> obj-$(CONFIG_ERRATA_SIFIVE) += sifive/
> obj-$(CONFIG_ERRATA_THEAD) += thead/
> diff --git a/arch/riscv/errata/mips/Makefile b/arch/riscv/errata/mips/Makefile
> new file mode 100644
> index 000000000000..6278c389b801
> --- /dev/null
> +++ b/arch/riscv/errata/mips/Makefile
> @@ -0,0 +1,5 @@
> +ifdef CONFIG_RISCV_ALTERNATIVE_EARLY
> +CFLAGS_errata.o := -mcmodel=medany
> +endif
> +
> +obj-y += errata.o
> diff --git a/arch/riscv/errata/mips/errata.c b/arch/riscv/errata/mips/errata.c
> new file mode 100644
> index 000000000000..998bbcaa50d1
> --- /dev/null
> +++ b/arch/riscv/errata/mips/errata.c
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 MIPS.
> + */
> +
> +#include <linux/memory.h>
> +#include <linux/module.h>
> +#include <asm/text-patching.h>
> +#include <asm/alternative.h>
> +#include <asm/errata_list.h>
> +#include <asm/vendorid_list.h>
> +#include <asm/vendor_extensions.h>
> +
> +void mips_errata_patch_func(struct alt_entry *begin,
> + struct alt_entry *end,
> + unsigned long archid,
> + unsigned long impid,
> + unsigned int stage)
> +{
> + struct alt_entry *alt;
> +
> + BUILD_BUG_ON(ERRATA_MIPS_NUMBER >= RISCV_VENDOR_EXT_ALTERNATIVES_BASE);
> +
> + if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> + return;
> +
> + for (alt = begin; alt < end; alt++) {
> + if (alt->vendor_id != MIPS_VENDOR_ID)
> + continue;
> +
> + if (alt->patch_id >= ERRATA_MIPS_NUMBER) {
> + WARN(1, "MIPS errata id:%d not in kernel errata list\n",
> + alt->patch_id);
> + continue;
> + }
> +
> + mutex_lock(&text_mutex);
> + patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt), alt->alt_len);
> + mutex_unlock(&text_mutex);
> + }
> +}
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 3c2b59b25017..bc3ada8190a9 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -48,6 +48,9 @@ struct alt_entry {
> void andes_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> unsigned long archid, unsigned long impid,
> unsigned int stage);
> +void mips_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> + unsigned long archid, unsigned long impid,
> + unsigned int stage);
> void sifive_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> unsigned long archid, unsigned long impid,
> unsigned int stage);
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 4cadc56220fe..2f5705f49dcc 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -13,6 +13,7 @@
> #include <asm/hwcap.h>
> #include <asm/insn-def.h>
> #include <asm/cpufeature-macros.h>
> +#include <asm/errata_list.h>
>
> #define __arch_xchg_masked(sc_sfx, swap_sfx, prepend, sc_append, \
> swap_append, r, p, n) \
> @@ -404,7 +405,7 @@ static __always_inline void __cmpwait(volatile void *ptr,
> return;
>
> no_zawrs:
> - asm volatile(RISCV_PAUSE : : : "memory");
> + ALT_RISCV_PAUSE();
> }
>
> #define __cmpwait_relaxed(ptr, val) \
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 7c8a71a526a3..68c46ccb9b7c 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -5,7 +5,6 @@
> #ifndef ASM_ERRATA_LIST_H
> #define ASM_ERRATA_LIST_H
>
> -#include <asm/alternative.h>
> #include <asm/csr.h>
> #include <asm/insn-def.h>
> #include <asm/hwcap.h>
> @@ -28,6 +27,11 @@
> #define ERRATA_THEAD_NUMBER 2
> #endif
>
> +#ifdef CONFIG_ERRATA_MIPS
> +#define ERRATA_MIPS_P8700_PAUSE_OPCODE 0
> +#define ERRATA_MIPS_NUMBER 1
> +#endif
> +
> #ifdef __ASSEMBLY__
>
> #define ALT_INSN_FAULT(x) \
> @@ -58,6 +62,20 @@ asm(ALTERNATIVE("sfence.vma %0, %1", "sfence.vma", SIFIVE_VENDOR_ID, \
> ERRATA_SIFIVE_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200) \
> : : "r" (addr), "r" (asid) : "memory")
>
> +#ifdef CONFIG_ERRATA_MIPS_P8700_PAUSE_OPCODE
> +#define ALT_RISCV_PAUSE() \
> +asm volatile(ALTERNATIVE( \
> + RISCV_PAUSE, /* Original RISC‑V pause insn */ \
> + ".4byte 0x00501013", /* Replacement for MIPS P8700 */ \
> + MIPS_VENDOR_ID, /* Vendor ID to match */ \
> + ERRATA_MIPS_P8700_PAUSE_OPCODE, /* patch_id */ \
> + CONFIG_ERRATA_MIPS_P8700_PAUSE_OPCODE) \
> + : /* no outputs */ : /* no inputs */ : "memory")
> +#else
> +#define ALT_RISCV_PAUSE() \
> + asm volatile(RISCV_PAUSE : : : "memory")
> +#endif
> +
> /*
> * _val is marked as "will be overwritten", so need to set it to 0
> * in the default case.
> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
> index 8f383f05a290..8f749552ecfe 100644
> --- a/arch/riscv/include/asm/vdso/processor.h
> +++ b/arch/riscv/include/asm/vdso/processor.h
> @@ -5,6 +5,8 @@
> #ifndef __ASSEMBLY__
>
> #include <asm/barrier.h>
> +
> +#include <asm/errata_list.h>
> #include <asm/insn-def.h>
>
> static inline void cpu_relax(void)
> @@ -19,7 +21,7 @@ static inline void cpu_relax(void)
> * Reduce instruction retirement.
> * This assumes the PC changes.
> */
> - __asm__ __volatile__ (RISCV_PAUSE);
> + ALT_RISCV_PAUSE();
> barrier();
> }
>
> diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
> index a5150cdf34d8..3b09874d7a6d 100644
> --- a/arch/riscv/include/asm/vendorid_list.h
> +++ b/arch/riscv/include/asm/vendorid_list.h
> @@ -9,5 +9,6 @@
> #define MICROCHIP_VENDOR_ID 0x029
> #define SIFIVE_VENDOR_ID 0x489
> #define THEAD_VENDOR_ID 0x5b7
> +#define MIPS_VENDOR_ID 0x722
>
> #endif
> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> index 7eb3cb1215c6..7642704c7f18 100644
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -47,6 +47,11 @@ static void riscv_fill_cpu_mfr_info(struct cpu_manufacturer_info_t *cpu_mfr_info
> cpu_mfr_info->patch_func = andes_errata_patch_func;
> break;
> #endif
> +#ifdef CONFIG_ERRATA_MIPS
> + case MIPS_VENDOR_ID:
> + cpu_mfr_info->patch_func = mips_errata_patch_func;
> + break;
> +#endif
> #ifdef CONFIG_ERRATA_SIFIVE
> case SIFIVE_VENDOR_ID:
> cpu_mfr_info->patch_func = sifive_errata_patch_func;
> diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
> index 33a5a9f2a0d4..2007d9de7f27 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -15,6 +15,8 @@
> #include <asm/thread_info.h>
> #include <asm/asm-offsets.h>
> #include <asm/errata_list.h>
> +#include <asm/alternative.h>
> +
> #include <linux/sizes.h>
>
> .section .irqentry.text, "ax"
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 722178ae3488..ee8a2c5b40f8 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -35,6 +35,7 @@
> #include <asm/soc.h>
> #include <asm/sparsemem.h>
> #include <asm/tlbflush.h>
> +#include <asm/alternative.h>
>
> #include "../kernel/head.h"
>
> diff --git a/tools/arch/riscv/include/asm/vdso/processor.h b/tools/arch/riscv/include/asm/vdso/processor.h
> index 662aca039848..880f26a24f69 100644
> --- a/tools/arch/riscv/include/asm/vdso/processor.h
> +++ b/tools/arch/riscv/include/asm/vdso/processor.h
> @@ -14,7 +14,10 @@ static inline void cpu_relax(void)
> __asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
> #endif
>
> -#ifdef CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE
> +#ifdef CONFIG_ERRATA_MIPS_P8700_PAUSE_OPCODE
> + /* MIPS P8700 pause opcode */
> + __asm__ __volatile__ (".4byte 0x00501013");
> +#elif CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE
> /*
> * Reduce instruction retirement.
> * This assumes the PC changes.
Powered by blists - more mailing lists