[RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[Vincent Chen]

With the emergence of more and more RISC-V CPUs, the request for how to
upstream the vendor errata patch may gradually appear. In order to resolve
this issue, this patch introduces the alternative mechanism from ARM64 and
x86 to enable the kernel to patch code at runtime according to the
manufacturer information of the running CPU. The main purpose of this patch
set is to propose a framework to apply vendor's errata solutions. Based on
this framework, it can be ensured that the errata only applies to the
specified CPU cores. Other CPU cores do not be affected. Therefore, some
complicated scenarios are unsupported in this patch set, such as patching
code to the kernel module, doing relocation in patching code, and
heterogeneous CPU topology.

In the "alternative" scheme, Users could use the macro ALTERNATIVE to apply
an errata to the existing code flow. In the macro ALTERNATIVE, users need
to specify the manufacturer information (vendor id, arch id, and implement
id) for this errata. Therefore, kernel will know this errata is suitable
for which CPU core. During the booting procedure, kernel will select the
errata required by the CPU core and then patch it. It means that the kernel
only applies the errata to the specified CPU core. In this case, the
vendor's errata does not affect each other at runtime. The above patching
procedure only occurs during the booting phase, so we only take the
overhead of the "alternative" mechanism once.

This "alternative" mechanism is enabled by default to ensure that all
required errata will be applied. However, users can disable this feature by
the Kconfig "CONFIG_RISCV_ERRATA_ALTERNATIVE".

The last patch is to apply the SiFive CIP-453 errata by this "alternative"
scheme. Therefore, It can be regarded as an example. According to the
results of running this image on the QEMU virt platform, kernel does not
apply this errata at run-time because the CPU manufacturer information
does not match the specified SiFive CPU core. Therefore, this errata does
not affect any CPU core except for the specified SiFive cores.

Vincent Chen (4):
  riscv: Add 3 SBI wrapper functions to get cpu manufacturer information
  riscv: Get CPU manufacturer information
  riscv: Introduce alternative mechanism to apply errata solution
  riscv: sifive: apply errata "cip-453" patch

 arch/riscv/Kconfig                          |   1 +
 arch/riscv/Kconfig.erratas                  |  32 ++++++++
 arch/riscv/Kconfig.socs                     |   1 +
 arch/riscv/Makefile                         |   1 +
 arch/riscv/errata/Makefile                  |   2 +
 arch/riscv/errata/alternative.c             |  69 +++++++++++++++++
 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/alternative-macros.h | 110 ++++++++++++++++++++++++++++
 arch/riscv/include/asm/alternative.h        |  44 +++++++++++
 arch/riscv/include/asm/asm.h                |   1 +
 arch/riscv/include/asm/csr.h                |   3 +
 arch/riscv/include/asm/errata_list.h        |   9 +++
 arch/riscv/include/asm/hwcap.h              |   6 ++
 arch/riscv/include/asm/processor.h          |   2 +
 arch/riscv/include/asm/sbi.h                |   3 +
 arch/riscv/include/asm/sections.h           |   2 +
 arch/riscv/include/asm/soc.h                |   1 +
 arch/riscv/include/asm/vendorid_list.h      |   6 ++
 arch/riscv/kernel/cpufeature.c              |  17 +++++
 arch/riscv/kernel/entry.S                   |  12 ++-
 arch/riscv/kernel/sbi.c                     |  15 ++++
 arch/riscv/kernel/setup.c                   |   2 +
 arch/riscv/kernel/smpboot.c                 |   4 +
 arch/riscv/kernel/soc.c                     |   1 +
 arch/riscv/kernel/vmlinux.lds.S             |  14 ++++
 27 files changed, 448 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/Kconfig.erratas
 create mode 100644 arch/riscv/errata/Makefile
 create mode 100644 arch/riscv/errata/alternative.c
 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
 create mode 100644 arch/riscv/include/asm/alternative-macros.h
 create mode 100644 arch/riscv/include/asm/alternative.h
 create mode 100644 arch/riscv/include/asm/errata_list.h
 create mode 100644 arch/riscv/include/asm/vendorid_list.h

-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC patch 1/4] riscv: Add 3 SBI wrapper functions to get cpu manufacturer information

[Vincent Chen]

Add 3 wrapper functions to get vendor id, architecture id and implement id
from M-mode

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
---
 arch/riscv/include/asm/sbi.h |  3 +++
 arch/riscv/kernel/sbi.c      | 15 +++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 99895d9c3bdd..94a7f664caf4 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -97,6 +97,9 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
 
 void sbi_console_putchar(int ch);
 int sbi_console_getchar(void);
+long sbi_get_vendorid(void);
+long sbi_get_archid(void);
+long sbi_get_impid(void);
 void sbi_set_timer(uint64_t stime_value);
 void sbi_shutdown(void);
 void sbi_clear_ipi(void);
diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
index f4a7db3d309e..33e3a9d6efab 100644
--- a/arch/riscv/kernel/sbi.c
+++ b/arch/riscv/kernel/sbi.c
@@ -547,6 +547,21 @@ static inline long sbi_get_firmware_version(void)
 	return __sbi_base_ecall(SBI_EXT_BASE_GET_IMP_VERSION);
 }
 
+long sbi_get_vendorid(void)
+{
+	return __sbi_base_ecall(SBI_EXT_BASE_GET_MVENDORID);
+}
+
+long sbi_get_archid(void)
+{
+	return __sbi_base_ecall(SBI_EXT_BASE_GET_MARCHID);
+}
+
+long sbi_get_impid(void)
+{
+	return __sbi_base_ecall(SBI_EXT_BASE_GET_MIMPID);
+}
+
 static void sbi_send_cpumask_ipi(const struct cpumask *target)
 {
 	struct cpumask hartid_mask;
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 1/4] riscv: Add 3 SBI wrapper functions to get cpu manufacturer information

[Palmer Dabbelt]

On Sun, 07 Mar 2021 19:58:14 PST (-0800), vincent.chen@sifive.com wrote:
> Add 3 wrapper functions to get vendor id, architecture id and implement id
> from M-mode
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  arch/riscv/include/asm/sbi.h |  3 +++
>  arch/riscv/kernel/sbi.c      | 15 +++++++++++++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> index 99895d9c3bdd..94a7f664caf4 100644
> --- a/arch/riscv/include/asm/sbi.h
> +++ b/arch/riscv/include/asm/sbi.h
> @@ -97,6 +97,9 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
>
>  void sbi_console_putchar(int ch);
>  int sbi_console_getchar(void);
> +long sbi_get_vendorid(void);
> +long sbi_get_archid(void);
> +long sbi_get_impid(void);
>  void sbi_set_timer(uint64_t stime_value);
>  void sbi_shutdown(void);
>  void sbi_clear_ipi(void);
> diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
> index f4a7db3d309e..33e3a9d6efab 100644
> --- a/arch/riscv/kernel/sbi.c
> +++ b/arch/riscv/kernel/sbi.c
> @@ -547,6 +547,21 @@ static inline long sbi_get_firmware_version(void)
>  	return __sbi_base_ecall(SBI_EXT_BASE_GET_IMP_VERSION);
>  }
>
> +long sbi_get_vendorid(void)
> +{
> +	return __sbi_base_ecall(SBI_EXT_BASE_GET_MVENDORID);
> +}
> +
> +long sbi_get_archid(void)
> +{
> +	return __sbi_base_ecall(SBI_EXT_BASE_GET_MARCHID);
> +}
> +
> +long sbi_get_impid(void)
> +{
> +	return __sbi_base_ecall(SBI_EXT_BASE_GET_MIMPID);
> +}

The function names should match the SBI function names, which have an extra "m" 
in them.  Otherwise it's just confusing.

> +
>  static void sbi_send_cpumask_ipi(const struct cpumask *target)
>  {
>  	struct cpumask hartid_mask;

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 1/4] riscv: Add 3 SBI wrapper functions to get cpu manufacturer information

[Vincent Chen]

On Wed, Mar 10, 2021 at 12:39 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Sun, 07 Mar 2021 19:58:14 PST (-0800), vincent.chen@sifive.com wrote:
> > Add 3 wrapper functions to get vendor id, architecture id and implement id
> > from M-mode
> >
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> >  arch/riscv/include/asm/sbi.h |  3 +++
> >  arch/riscv/kernel/sbi.c      | 15 +++++++++++++++
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
> > index 99895d9c3bdd..94a7f664caf4 100644
> > --- a/arch/riscv/include/asm/sbi.h
> > +++ b/arch/riscv/include/asm/sbi.h
> > @@ -97,6 +97,9 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0,
> >
> >  void sbi_console_putchar(int ch);
> >  int sbi_console_getchar(void);
> > +long sbi_get_vendorid(void);
> > +long sbi_get_archid(void);
> > +long sbi_get_impid(void);
> >  void sbi_set_timer(uint64_t stime_value);
> >  void sbi_shutdown(void);
> >  void sbi_clear_ipi(void);
> > diff --git a/arch/riscv/kernel/sbi.c b/arch/riscv/kernel/sbi.c
> > index f4a7db3d309e..33e3a9d6efab 100644
> > --- a/arch/riscv/kernel/sbi.c
> > +++ b/arch/riscv/kernel/sbi.c
> > @@ -547,6 +547,21 @@ static inline long sbi_get_firmware_version(void)
> >       return __sbi_base_ecall(SBI_EXT_BASE_GET_IMP_VERSION);
> >  }
> >
> > +long sbi_get_vendorid(void)
> > +{
> > +     return __sbi_base_ecall(SBI_EXT_BASE_GET_MVENDORID);
> > +}
> > +
> > +long sbi_get_archid(void)
> > +{
> > +     return __sbi_base_ecall(SBI_EXT_BASE_GET_MARCHID);
> > +}
> > +
> > +long sbi_get_impid(void)
> > +{
> > +     return __sbi_base_ecall(SBI_EXT_BASE_GET_MIMPID);
> > +}
>
> The function names should match the SBI function names, which have an extra "m"
> in them.  Otherwise it's just confusing.
>

OK, I will add an extra "m" to them to match the SBI function names.
Thank you for the comments.

> > +
> >  static void sbi_send_cpumask_ipi(const struct cpumask *target)
> >  {
> >       struct cpumask hartid_mask;

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC patch 2/4] riscv: Get CPU manufacturer information

[Vincent Chen]

Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
ID early in boot so we only need to take the SBI call overhead once.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
---
 arch/riscv/include/asm/csr.h       |  3 +++
 arch/riscv/include/asm/hwcap.h     |  6 ++++++
 arch/riscv/include/asm/processor.h |  2 ++
 arch/riscv/include/asm/soc.h       |  1 +
 arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
 arch/riscv/kernel/setup.c          |  2 ++
 arch/riscv/kernel/soc.c            |  1 +
 7 files changed, 32 insertions(+)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index caadfc1d7487..87ac65696871 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -115,6 +115,9 @@
 #define CSR_MIP			0x344
 #define CSR_PMPCFG0		0x3a0
 #define CSR_PMPADDR0		0x3b0
+#define CSR_MVENDORID		0xf11
+#define CSR_MARCHID		0xf12
+#define CSR_MIMPID		0xf13
 #define CSR_MHARTID		0xf14
 
 #ifdef CONFIG_RISCV_M_MODE
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 5ce50468aff1..b7409487c9d2 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
 #define riscv_isa_extension_available(isa_bitmap, ext)	\
 	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
 
+struct cpu_manufacturer_info_t {
+	unsigned long vendor_id;
+	unsigned long arch_id;
+	unsigned long imp_id;
+};
+
 #endif
 
 #endif /* _ASM_RISCV_HWCAP_H */
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index 3a240037bde2..4e11a9621d14 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
 
 extern void riscv_fill_hwcap(void);
 
+void riscv_fill_cpu_manufacturer_info(void);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PROCESSOR_H */
diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
index f494066051a2..03dee6db404c 100644
--- a/arch/riscv/include/asm/soc.h
+++ b/arch/riscv/include/asm/soc.h
@@ -10,6 +10,7 @@
 #include <linux/of.h>
 #include <linux/linkage.h>
 #include <linux/types.h>
+#include <asm/hwcap.h>
 
 #define SOC_EARLY_INIT_DECLARE(name, compat, fn)			\
 	static const struct of_device_id __soc_early_init__##name	\
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index ac202f44a670..389162ee19ea 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -12,6 +12,8 @@
 #include <asm/hwcap.h>
 #include <asm/smp.h>
 #include <asm/switch_to.h>
+#include <asm/sbi.h>
+#include <asm/csr.h>
 
 unsigned long elf_hwcap __read_mostly;
 
@@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
 bool has_fpu __read_mostly;
 #endif
 
+struct cpu_manufacturer_info_t cpu_mfr_info;
+
 /**
  * riscv_isa_extension_base() - Get base extension word
  *
@@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
 		has_fpu = true;
 #endif
 }
+
+void riscv_fill_cpu_manufacturer_info(void)
+{
+#ifndef CONFIG_RISCV_M_MODE
+	cpu_mfr_info.vendor_id = sbi_get_vendorid();
+	cpu_mfr_info.arch_id = sbi_get_archid();
+	cpu_mfr_info.imp_id = sbi_get_impid();
+#else
+	cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
+	cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
+	cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
+#endif
+}
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index e85bacff1b50..03621ce9092c 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
 #endif
 
 	riscv_fill_hwcap();
+
+	riscv_fill_cpu_manufacturer_info();
 }
 
 static int __init topology_init(void)
diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
index a0516172a33c..58f6fd91743a 100644
--- a/arch/riscv/kernel/soc.c
+++ b/arch/riscv/kernel/soc.c
@@ -6,6 +6,7 @@
 #include <linux/libfdt.h>
 #include <linux/pgtable.h>
 #include <asm/soc.h>
+#include <asm/hwcap.h>
 
 /*
  * This is called extremly early, before parse_dtb(), to allow initializing
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Damien Le Moal]

On 2021/03/08 12:59, Vincent Chen wrote:
> Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
> ID early in boot so we only need to take the SBI call overhead once.
> 
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  arch/riscv/include/asm/csr.h       |  3 +++
>  arch/riscv/include/asm/hwcap.h     |  6 ++++++
>  arch/riscv/include/asm/processor.h |  2 ++
>  arch/riscv/include/asm/soc.h       |  1 +
>  arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
>  arch/riscv/kernel/setup.c          |  2 ++
>  arch/riscv/kernel/soc.c            |  1 +
>  7 files changed, 32 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index caadfc1d7487..87ac65696871 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -115,6 +115,9 @@
>  #define CSR_MIP			0x344
>  #define CSR_PMPCFG0		0x3a0
>  #define CSR_PMPADDR0		0x3b0
> +#define CSR_MVENDORID		0xf11
> +#define CSR_MARCHID		0xf12
> +#define CSR_MIMPID		0xf13
>  #define CSR_MHARTID		0xf14
>  
>  #ifdef CONFIG_RISCV_M_MODE
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5ce50468aff1..b7409487c9d2 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>  #define riscv_isa_extension_available(isa_bitmap, ext)	\
>  	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>  
> +struct cpu_manufacturer_info_t {
> +	unsigned long vendor_id;
> +	unsigned long arch_id;
> +	unsigned long imp_id;
> +};
> +
>  #endif
>  
>  #endif /* _ASM_RISCV_HWCAP_H */
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 3a240037bde2..4e11a9621d14 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
>  
>  extern void riscv_fill_hwcap(void);
>  
> +void riscv_fill_cpu_manufacturer_info(void);
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* _ASM_RISCV_PROCESSOR_H */
> diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
> index f494066051a2..03dee6db404c 100644
> --- a/arch/riscv/include/asm/soc.h
> +++ b/arch/riscv/include/asm/soc.h
> @@ -10,6 +10,7 @@
>  #include <linux/of.h>
>  #include <linux/linkage.h>
>  #include <linux/types.h>
> +#include <asm/hwcap.h>
>  
>  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)			\
>  	static const struct of_device_id __soc_early_init__##name	\
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index ac202f44a670..389162ee19ea 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -12,6 +12,8 @@
>  #include <asm/hwcap.h>
>  #include <asm/smp.h>
>  #include <asm/switch_to.h>
> +#include <asm/sbi.h>
> +#include <asm/csr.h>
>  
>  unsigned long elf_hwcap __read_mostly;
>  
> @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>  bool has_fpu __read_mostly;
>  #endif
>  
> +struct cpu_manufacturer_info_t cpu_mfr_info;
> +
>  /**
>   * riscv_isa_extension_base() - Get base extension word
>   *
> @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
>  		has_fpu = true;
>  #endif
>  }
> +
> +void riscv_fill_cpu_manufacturer_info(void)
> +{
> +#ifndef CONFIG_RISCV_M_MODE
> +	cpu_mfr_info.vendor_id = sbi_get_vendorid();
> +	cpu_mfr_info.arch_id = sbi_get_archid();
> +	cpu_mfr_info.imp_id = sbi_get_impid();
> +#else
> +	cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> +	cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> +	cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> +#endif

Why ? reading the registers will work with M-Mode too. It was there before when
we temporarily had the builtin DTB lookup based on vendor/arch/imp (see defunct
soc_lookup_builtin_dtb() in 5.11).

> +}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index e85bacff1b50..03621ce9092c 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
>  #endif
>  
>  	riscv_fill_hwcap();
> +

Nit: I do not think the white libe is really necessary here.

> +	riscv_fill_cpu_manufacturer_info();
>  }
>  
>  static int __init topology_init(void)
> diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> index a0516172a33c..58f6fd91743a 100644
> --- a/arch/riscv/kernel/soc.c
> +++ b/arch/riscv/kernel/soc.c
> @@ -6,6 +6,7 @@
>  #include <linux/libfdt.h>
>  #include <linux/pgtable.h>
>  #include <asm/soc.h>
> +#include <asm/hwcap.h>

Why is this necessary ?

>  
>  /*
>   * This is called extremly early, before parse_dtb(), to allow initializing
> 


-- 
Damien Le Moal
Western Digital Research

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Sean Anderson]

On 3/8/21 6:30 PM, Damien Le Moal wrote:
> On 2021/03/08 12:59, Vincent Chen wrote:
>> Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
>> ID early in boot so we only need to take the SBI call overhead once.
>>
>> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
>> ---
>>   arch/riscv/include/asm/csr.h       |  3 +++
>>   arch/riscv/include/asm/hwcap.h     |  6 ++++++
>>   arch/riscv/include/asm/processor.h |  2 ++
>>   arch/riscv/include/asm/soc.h       |  1 +
>>   arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
>>   arch/riscv/kernel/setup.c          |  2 ++
>>   arch/riscv/kernel/soc.c            |  1 +
>>   7 files changed, 32 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
>> index caadfc1d7487..87ac65696871 100644
>> --- a/arch/riscv/include/asm/csr.h
>> +++ b/arch/riscv/include/asm/csr.h
>> @@ -115,6 +115,9 @@
>>   #define CSR_MIP			0x344
>>   #define CSR_PMPCFG0		0x3a0
>>   #define CSR_PMPADDR0		0x3b0
>> +#define CSR_MVENDORID		0xf11
>> +#define CSR_MARCHID		0xf12
>> +#define CSR_MIMPID		0xf13
>>   #define CSR_MHARTID		0xf14
>>   
>>   #ifdef CONFIG_RISCV_M_MODE
>> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
>> index 5ce50468aff1..b7409487c9d2 100644
>> --- a/arch/riscv/include/asm/hwcap.h
>> +++ b/arch/riscv/include/asm/hwcap.h
>> @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>>   #define riscv_isa_extension_available(isa_bitmap, ext)	\
>>   	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>>   
>> +struct cpu_manufacturer_info_t {
>> +	unsigned long vendor_id;
>> +	unsigned long arch_id;
>> +	unsigned long imp_id;
>> +};
>> +
>>   #endif
>>   
>>   #endif /* _ASM_RISCV_HWCAP_H */
>> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
>> index 3a240037bde2..4e11a9621d14 100644
>> --- a/arch/riscv/include/asm/processor.h
>> +++ b/arch/riscv/include/asm/processor.h
>> @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
>>   
>>   extern void riscv_fill_hwcap(void);
>>   
>> +void riscv_fill_cpu_manufacturer_info(void);
>> +
>>   #endif /* __ASSEMBLY__ */
>>   
>>   #endif /* _ASM_RISCV_PROCESSOR_H */
>> diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
>> index f494066051a2..03dee6db404c 100644
>> --- a/arch/riscv/include/asm/soc.h
>> +++ b/arch/riscv/include/asm/soc.h
>> @@ -10,6 +10,7 @@
>>   #include <linux/of.h>
>>   #include <linux/linkage.h>
>>   #include <linux/types.h>
>> +#include <asm/hwcap.h>
>>   
>>   #define SOC_EARLY_INIT_DECLARE(name, compat, fn)			\
>>   	static const struct of_device_id __soc_early_init__##name	\
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index ac202f44a670..389162ee19ea 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -12,6 +12,8 @@
>>   #include <asm/hwcap.h>
>>   #include <asm/smp.h>
>>   #include <asm/switch_to.h>
>> +#include <asm/sbi.h>
>> +#include <asm/csr.h>
>>   
>>   unsigned long elf_hwcap __read_mostly;
>>   
>> @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>>   bool has_fpu __read_mostly;
>>   #endif
>>   
>> +struct cpu_manufacturer_info_t cpu_mfr_info;
>> +
>>   /**
>>    * riscv_isa_extension_base() - Get base extension word
>>    *
>> @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
>>   		has_fpu = true;
>>   #endif
>>   }
>> +
>> +void riscv_fill_cpu_manufacturer_info(void)
>> +{
>> +#ifndef CONFIG_RISCV_M_MODE
>> +	cpu_mfr_info.vendor_id = sbi_get_vendorid();
>> +	cpu_mfr_info.arch_id = sbi_get_archid();
>> +	cpu_mfr_info.imp_id = sbi_get_impid();
>> +#else
>> +	cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
>> +	cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
>> +	cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
>> +#endif
> 
> Why ? reading the registers will work with M-Mode too. It was there before when
> we temporarily had the builtin DTB lookup based on vendor/arch/imp (see defunct
> soc_lookup_builtin_dtb() in 5.11).

I don't understand what the objection is here. S-mode does not have
these CSRs, so it uses SBI. M-mode does, so it reads the CSRs. This is
the same logic as soc_lookup_builtin_dtb, except it works with S-mode as
well.

--Sean

> 
>> +}
>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>> index e85bacff1b50..03621ce9092c 100644
>> --- a/arch/riscv/kernel/setup.c
>> +++ b/arch/riscv/kernel/setup.c
>> @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
>>   #endif
>>   
>>   	riscv_fill_hwcap();
>> +
> 
> Nit: I do not think the white libe is really necessary here.
> 
>> +	riscv_fill_cpu_manufacturer_info();
>>   }
>>   
>>   static int __init topology_init(void)
>> diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
>> index a0516172a33c..58f6fd91743a 100644
>> --- a/arch/riscv/kernel/soc.c
>> +++ b/arch/riscv/kernel/soc.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/libfdt.h>
>>   #include <linux/pgtable.h>
>>   #include <asm/soc.h>
>> +#include <asm/hwcap.h>
> 
> Why is this necessary ?
> 
>>   
>>   /*
>>    * This is called extremly early, before parse_dtb(), to allow initializing
>>
> 
> 


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Vincent Chen]

> >  #endif /* _ASM_RISCV_PROCESSOR_H */
> > diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
> > index f494066051a2..03dee6db404c 100644
> > --- a/arch/riscv/include/asm/soc.h
> > +++ b/arch/riscv/include/asm/soc.h
> > @@ -10,6 +10,7 @@
> >  #include <linux/of.h>
> >  #include <linux/linkage.h>
> >  #include <linux/types.h>
> > +#include <asm/hwcap.h>
> >
> >  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)                     \
> >       static const struct of_device_id __soc_early_init__##name       \
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index ac202f44a670..389162ee19ea 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -12,6 +12,8 @@
> >  #include <asm/hwcap.h>
> >  #include <asm/smp.h>
> >  #include <asm/switch_to.h>
> > +#include <asm/sbi.h>
> > +#include <asm/csr.h>
> >
> >  unsigned long elf_hwcap __read_mostly;
> >
> > @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> >  bool has_fpu __read_mostly;
> >  #endif
> >
> > +struct cpu_manufacturer_info_t cpu_mfr_info;
> > +
> >  /**
> >   * riscv_isa_extension_base() - Get base extension word
> >   *
> > @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
> >               has_fpu = true;
> >  #endif
> >  }
> > +
> > +void riscv_fill_cpu_manufacturer_info(void)
> > +{
> > +#ifndef CONFIG_RISCV_M_MODE
> > +     cpu_mfr_info.vendor_id = sbi_get_vendorid();
> > +     cpu_mfr_info.arch_id = sbi_get_archid();
> > +     cpu_mfr_info.imp_id = sbi_get_impid();
> > +#else
> > +     cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> > +     cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> > +     cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> > +#endif
>
> Why ? reading the registers will work with M-Mode too. It was there before when
> we temporarily had the builtin DTB lookup based on vendor/arch/imp (see defunct
> soc_lookup_builtin_dtb() in 5.11).
>
Sorry, I cannot fully catch what you mean.
I agree that reading these registers will work with M-Mode, so I used
the macro csr_read() to read these three registers when the kernel run
in the M-mode.
The definition of csr_read() is here:
#define csr_read(csr)                                           \
({                                                              \
        register unsigned long __v;                             \
        __asm__ __volatile__ ("csrr %0, " __ASM_STR(csr)        \
                              : "=r" (__v) :                    \
                              : "memory");                      \
        __v;                                                    \
})
I guess that #if"n"def CONFIG_RISCV_M_MODE may mislead you. #ifndef is
really unfriendly to read. I will change #ifndef to #ifdef in my next
version patch.


> > +}
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index e85bacff1b50..03621ce9092c 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
> >  #endif
> >
> >       riscv_fill_hwcap();
> > +
>
> Nit: I do not think the white libe is really necessary here.
>
OK, I will remove it.
> > +     riscv_fill_cpu_manufacturer_info();
> >  }
> >
> >  static int __init topology_init(void)
> > diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> > index a0516172a33c..58f6fd91743a 100644
> > --- a/arch/riscv/kernel/soc.c
> > +++ b/arch/riscv/kernel/soc.c
> > @@ -6,6 +6,7 @@
> >  #include <linux/libfdt.h>
> >  #include <linux/pgtable.h>
> >  #include <asm/soc.h>
> > +#include <asm/hwcap.h>
>
> Why is this necessary ?

I forgot to remove it when doing code cleanup. Thank you for the
reminder. I will remove it

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Damien Le Moal]

On 2021/03/09 10:25, Vincent Chen wrote:
>>>  #endif /* _ASM_RISCV_PROCESSOR_H */
>>> diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
>>> index f494066051a2..03dee6db404c 100644
>>> --- a/arch/riscv/include/asm/soc.h
>>> +++ b/arch/riscv/include/asm/soc.h
>>> @@ -10,6 +10,7 @@
>>>  #include <linux/of.h>
>>>  #include <linux/linkage.h>
>>>  #include <linux/types.h>
>>> +#include <asm/hwcap.h>
>>>
>>>  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)                     \
>>>       static const struct of_device_id __soc_early_init__##name       \
>>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>>> index ac202f44a670..389162ee19ea 100644
>>> --- a/arch/riscv/kernel/cpufeature.c
>>> +++ b/arch/riscv/kernel/cpufeature.c
>>> @@ -12,6 +12,8 @@
>>>  #include <asm/hwcap.h>
>>>  #include <asm/smp.h>
>>>  #include <asm/switch_to.h>
>>> +#include <asm/sbi.h>
>>> +#include <asm/csr.h>
>>>
>>>  unsigned long elf_hwcap __read_mostly;
>>>
>>> @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>>>  bool has_fpu __read_mostly;
>>>  #endif
>>>
>>> +struct cpu_manufacturer_info_t cpu_mfr_info;
>>> +
>>>  /**
>>>   * riscv_isa_extension_base() - Get base extension word
>>>   *
>>> @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
>>>               has_fpu = true;
>>>  #endif
>>>  }
>>> +
>>> +void riscv_fill_cpu_manufacturer_info(void)
>>> +{
>>> +#ifndef CONFIG_RISCV_M_MODE
>>> +     cpu_mfr_info.vendor_id = sbi_get_vendorid();
>>> +     cpu_mfr_info.arch_id = sbi_get_archid();
>>> +     cpu_mfr_info.imp_id = sbi_get_impid();
>>> +#else
>>> +     cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
>>> +     cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
>>> +     cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
>>> +#endif
>>
>> Why ? reading the registers will work with M-Mode too. It was there before when
>> we temporarily had the builtin DTB lookup based on vendor/arch/imp (see defunct
>> soc_lookup_builtin_dtb() in 5.11).
>>
> Sorry, I cannot fully catch what you mean.
> I agree that reading these registers will work with M-Mode, so I used
> the macro csr_read() to read these three registers when the kernel run
> in the M-mode.
> The definition of csr_read() is here:
> #define csr_read(csr)                                           \
> ({                                                              \
>         register unsigned long __v;                             \
>         __asm__ __volatile__ ("csrr %0, " __ASM_STR(csr)        \
>                               : "=r" (__v) :                    \
>                               : "memory");                      \
>         __v;                                                    \
> })
> I guess that #if"n"def CONFIG_RISCV_M_MODE may mislead you. #ifndef is
> really unfriendly to read. I will change #ifndef to #ifdef in my next
> version patch.

You are right. I totally misread this :) It was my morning and I needed more
coffee ! Apologies for the noise. This looks fine.

> 
> 
>>> +}
>>> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>>> index e85bacff1b50..03621ce9092c 100644
>>> --- a/arch/riscv/kernel/setup.c
>>> +++ b/arch/riscv/kernel/setup.c
>>> @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
>>>  #endif
>>>
>>>       riscv_fill_hwcap();
>>> +
>>
>> Nit: I do not think the white libe is really necessary here.
>>
> OK, I will remove it.
>>> +     riscv_fill_cpu_manufacturer_info();
>>>  }
>>>
>>>  static int __init topology_init(void)
>>> diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
>>> index a0516172a33c..58f6fd91743a 100644
>>> --- a/arch/riscv/kernel/soc.c
>>> +++ b/arch/riscv/kernel/soc.c
>>> @@ -6,6 +6,7 @@
>>>  #include <linux/libfdt.h>
>>>  #include <linux/pgtable.h>
>>>  #include <asm/soc.h>
>>> +#include <asm/hwcap.h>
>>
>> Why is this necessary ?
> 
> I forgot to remove it when doing code cleanup. Thank you for the
> reminder. I will remove it
> 


-- 
Damien Le Moal
Western Digital Research

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Guo Ren]

Hi Vincent,

On Mon, Mar 8, 2021 at 11:58 AM Vincent Chen <vincent.chen@sifive.com> wrote:
>
> Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
> ID early in boot so we only need to take the SBI call overhead once.
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  arch/riscv/include/asm/csr.h       |  3 +++
>  arch/riscv/include/asm/hwcap.h     |  6 ++++++
>  arch/riscv/include/asm/processor.h |  2 ++
>  arch/riscv/include/asm/soc.h       |  1 +
>  arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
>  arch/riscv/kernel/setup.c          |  2 ++
>  arch/riscv/kernel/soc.c            |  1 +
>  7 files changed, 32 insertions(+)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index caadfc1d7487..87ac65696871 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -115,6 +115,9 @@
>  #define CSR_MIP                        0x344
>  #define CSR_PMPCFG0            0x3a0
>  #define CSR_PMPADDR0           0x3b0
> +#define CSR_MVENDORID          0xf11
> +#define CSR_MARCHID            0xf12
> +#define CSR_MIMPID             0xf13
>  #define CSR_MHARTID            0xf14
>
>  #ifdef CONFIG_RISCV_M_MODE
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5ce50468aff1..b7409487c9d2 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>  #define riscv_isa_extension_available(isa_bitmap, ext) \
>         __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>
> +struct cpu_manufacturer_info_t {
> +       unsigned long vendor_id;
> +       unsigned long arch_id;
> +       unsigned long imp_id;
> +};
> +
>  #endif
>
>  #endif /* _ASM_RISCV_HWCAP_H */
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 3a240037bde2..4e11a9621d14 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
>
>  extern void riscv_fill_hwcap(void);
>
> +void riscv_fill_cpu_manufacturer_info(void);
> +
>  #endif /* __ASSEMBLY__ */
>
>  #endif /* _ASM_RISCV_PROCESSOR_H */
> diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
> index f494066051a2..03dee6db404c 100644
> --- a/arch/riscv/include/asm/soc.h
> +++ b/arch/riscv/include/asm/soc.h
> @@ -10,6 +10,7 @@
>  #include <linux/of.h>
>  #include <linux/linkage.h>
>  #include <linux/types.h>
> +#include <asm/hwcap.h>
>
>  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)                       \
>         static const struct of_device_id __soc_early_init__##name       \
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index ac202f44a670..389162ee19ea 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -12,6 +12,8 @@
>  #include <asm/hwcap.h>
>  #include <asm/smp.h>
>  #include <asm/switch_to.h>
> +#include <asm/sbi.h>
> +#include <asm/csr.h>
>
>  unsigned long elf_hwcap __read_mostly;
>
> @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>  bool has_fpu __read_mostly;
>  #endif
>
> +struct cpu_manufacturer_info_t cpu_mfr_info;
> +
>  /**
>   * riscv_isa_extension_base() - Get base extension word
>   *
> @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
>                 has_fpu = true;
>  #endif
>  }
> +
> +void riscv_fill_cpu_manufacturer_info(void)
> +{
> +#ifndef CONFIG_RISCV_M_MODE
> +       cpu_mfr_info.vendor_id = sbi_get_vendorid();
> +       cpu_mfr_info.arch_id = sbi_get_archid();
> +       cpu_mfr_info.imp_id = sbi_get_impid();
> +#else
> +       cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> +       cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> +       cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> +#endif
How about let opensbi emulate csr_read(CSR_MXXX) for S mode, then we
needn't to define new sbi call.

> +}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index e85bacff1b50..03621ce9092c 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
>  #endif
>
>         riscv_fill_hwcap();
> +
> +       riscv_fill_cpu_manufacturer_info();
>  }
>
>  static int __init topology_init(void)
> diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> index a0516172a33c..58f6fd91743a 100644
> --- a/arch/riscv/kernel/soc.c
> +++ b/arch/riscv/kernel/soc.c
> @@ -6,6 +6,7 @@
>  #include <linux/libfdt.h>
>  #include <linux/pgtable.h>
>  #include <asm/soc.h>
> +#include <asm/hwcap.h>
>
>  /*
>   * This is called extremly early, before parse_dtb(), to allow initializing
> --
> 2.7.4
>
How

-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Vincent Chen]

> > +void riscv_fill_cpu_manufacturer_info(void)
> > +{
> > +#ifndef CONFIG_RISCV_M_MODE
> > +       cpu_mfr_info.vendor_id = sbi_get_vendorid();
> > +       cpu_mfr_info.arch_id = sbi_get_archid();
> > +       cpu_mfr_info.imp_id = sbi_get_impid();
> > +#else
> > +       cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> > +       cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> > +       cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> > +#endif
> How about let opensbi emulate csr_read(CSR_MXXX) for S mode, then we
> needn't to define new sbi call.
>
Hi Guo Ren,
The SBI spec has defined 3 SBI calls for S-mode to get vendorid,
archid, and impid, so I do not define new SBI calls here. The 3
functions ( sbi_get_vendorid(), sbi_get_archid(), and sbi_get_impid())
are the new kernel API to issue these 3 SBI calls.


> > +}
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index e85bacff1b50..03621ce9092c 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
> >  #endif
> >
> >         riscv_fill_hwcap();
> > +
> > +       riscv_fill_cpu_manufacturer_info();
> >  }
> >
> >  static int __init topology_init(void)
> > diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> > index a0516172a33c..58f6fd91743a 100644
> > --- a/arch/riscv/kernel/soc.c
> > +++ b/arch/riscv/kernel/soc.c
> > @@ -6,6 +6,7 @@
> >  #include <linux/libfdt.h>
> >  #include <linux/pgtable.h>
> >  #include <asm/soc.h>
> > +#include <asm/hwcap.h>
> >
> >  /*
> >   * This is called extremly early, before parse_dtb(), to allow initializing
> > --
> > 2.7.4
> >
> How
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Anup Patel]

On Tue, Mar 9, 2021 at 6:58 AM Guo Ren <guoren@kernel.org> wrote:
>
> Hi Vincent,
>
> On Mon, Mar 8, 2021 at 11:58 AM Vincent Chen <vincent.chen@sifive.com> wrote:
> >
> > Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
> > ID early in boot so we only need to take the SBI call overhead once.
> >
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> >  arch/riscv/include/asm/csr.h       |  3 +++
> >  arch/riscv/include/asm/hwcap.h     |  6 ++++++
> >  arch/riscv/include/asm/processor.h |  2 ++
> >  arch/riscv/include/asm/soc.h       |  1 +
> >  arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
> >  arch/riscv/kernel/setup.c          |  2 ++
> >  arch/riscv/kernel/soc.c            |  1 +
> >  7 files changed, 32 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> > index caadfc1d7487..87ac65696871 100644
> > --- a/arch/riscv/include/asm/csr.h
> > +++ b/arch/riscv/include/asm/csr.h
> > @@ -115,6 +115,9 @@
> >  #define CSR_MIP                        0x344
> >  #define CSR_PMPCFG0            0x3a0
> >  #define CSR_PMPADDR0           0x3b0
> > +#define CSR_MVENDORID          0xf11
> > +#define CSR_MARCHID            0xf12
> > +#define CSR_MIMPID             0xf13
> >  #define CSR_MHARTID            0xf14
> >
> >  #ifdef CONFIG_RISCV_M_MODE
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index 5ce50468aff1..b7409487c9d2 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
> >  #define riscv_isa_extension_available(isa_bitmap, ext) \
> >         __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
> >
> > +struct cpu_manufacturer_info_t {
> > +       unsigned long vendor_id;
> > +       unsigned long arch_id;
> > +       unsigned long imp_id;
> > +};
> > +
> >  #endif
> >
> >  #endif /* _ASM_RISCV_HWCAP_H */
> > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> > index 3a240037bde2..4e11a9621d14 100644
> > --- a/arch/riscv/include/asm/processor.h
> > +++ b/arch/riscv/include/asm/processor.h
> > @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
> >
> >  extern void riscv_fill_hwcap(void);
> >
> > +void riscv_fill_cpu_manufacturer_info(void);
> > +
> >  #endif /* __ASSEMBLY__ */
> >
> >  #endif /* _ASM_RISCV_PROCESSOR_H */
> > diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
> > index f494066051a2..03dee6db404c 100644
> > --- a/arch/riscv/include/asm/soc.h
> > +++ b/arch/riscv/include/asm/soc.h
> > @@ -10,6 +10,7 @@
> >  #include <linux/of.h>
> >  #include <linux/linkage.h>
> >  #include <linux/types.h>
> > +#include <asm/hwcap.h>
> >
> >  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)                       \
> >         static const struct of_device_id __soc_early_init__##name       \
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index ac202f44a670..389162ee19ea 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -12,6 +12,8 @@
> >  #include <asm/hwcap.h>
> >  #include <asm/smp.h>
> >  #include <asm/switch_to.h>
> > +#include <asm/sbi.h>
> > +#include <asm/csr.h>
> >
> >  unsigned long elf_hwcap __read_mostly;
> >
> > @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> >  bool has_fpu __read_mostly;
> >  #endif
> >
> > +struct cpu_manufacturer_info_t cpu_mfr_info;
> > +
> >  /**
> >   * riscv_isa_extension_base() - Get base extension word
> >   *
> > @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
> >                 has_fpu = true;
> >  #endif
> >  }
> > +
> > +void riscv_fill_cpu_manufacturer_info(void)
> > +{
> > +#ifndef CONFIG_RISCV_M_MODE
> > +       cpu_mfr_info.vendor_id = sbi_get_vendorid();
> > +       cpu_mfr_info.arch_id = sbi_get_archid();
> > +       cpu_mfr_info.imp_id = sbi_get_impid();
> > +#else
> > +       cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> > +       cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> > +       cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> > +#endif
> How about let opensbi emulate csr_read(CSR_MXXX) for S mode, then we
> needn't to define new sbi call.

Accessing M-mode CSRs from the S-mode kernel will only make things
complicated for hypervisors because now hypervisors will also end-up
emulating M-mode CSRs.

Best would be to only access S-mode CSRs and SBI calls from
S-mode kernel.

>
> > +}
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index e85bacff1b50..03621ce9092c 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
> >  #endif
> >
> >         riscv_fill_hwcap();
> > +
> > +       riscv_fill_cpu_manufacturer_info();
> >  }
> >
> >  static int __init topology_init(void)
> > diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> > index a0516172a33c..58f6fd91743a 100644
> > --- a/arch/riscv/kernel/soc.c
> > +++ b/arch/riscv/kernel/soc.c
> > @@ -6,6 +6,7 @@
> >  #include <linux/libfdt.h>
> >  #include <linux/pgtable.h>
> >  #include <asm/soc.h>
> > +#include <asm/hwcap.h>
> >
> >  /*
> >   * This is called extremly early, before parse_dtb(), to allow initializing
> > --
> > 2.7.4
> >
> How
>
> --
> Best Regards
>  Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Guo Ren]

Got it. Thx

On Tue, Mar 9, 2021 at 1:11 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Mar 9, 2021 at 6:58 AM Guo Ren <guoren@kernel.org> wrote:
> >
> > Hi Vincent,
> >
> > On Mon, Mar 8, 2021 at 11:58 AM Vincent Chen <vincent.chen@sifive.com> wrote:
> > >
> > > Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
> > > ID early in boot so we only need to take the SBI call overhead once.
> > >
> > > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > > ---
> > >  arch/riscv/include/asm/csr.h       |  3 +++
> > >  arch/riscv/include/asm/hwcap.h     |  6 ++++++
> > >  arch/riscv/include/asm/processor.h |  2 ++
> > >  arch/riscv/include/asm/soc.h       |  1 +
> > >  arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
> > >  arch/riscv/kernel/setup.c          |  2 ++
> > >  arch/riscv/kernel/soc.c            |  1 +
> > >  7 files changed, 32 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> > > index caadfc1d7487..87ac65696871 100644
> > > --- a/arch/riscv/include/asm/csr.h
> > > +++ b/arch/riscv/include/asm/csr.h
> > > @@ -115,6 +115,9 @@
> > >  #define CSR_MIP                        0x344
> > >  #define CSR_PMPCFG0            0x3a0
> > >  #define CSR_PMPADDR0           0x3b0
> > > +#define CSR_MVENDORID          0xf11
> > > +#define CSR_MARCHID            0xf12
> > > +#define CSR_MIMPID             0xf13
> > >  #define CSR_MHARTID            0xf14
> > >
> > >  #ifdef CONFIG_RISCV_M_MODE
> > > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > > index 5ce50468aff1..b7409487c9d2 100644
> > > --- a/arch/riscv/include/asm/hwcap.h
> > > +++ b/arch/riscv/include/asm/hwcap.h
> > > @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
> > >  #define riscv_isa_extension_available(isa_bitmap, ext) \
> > >         __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
> > >
> > > +struct cpu_manufacturer_info_t {
> > > +       unsigned long vendor_id;
> > > +       unsigned long arch_id;
> > > +       unsigned long imp_id;
> > > +};
> > > +
> > >  #endif
> > >
> > >  #endif /* _ASM_RISCV_HWCAP_H */
> > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> > > index 3a240037bde2..4e11a9621d14 100644
> > > --- a/arch/riscv/include/asm/processor.h
> > > +++ b/arch/riscv/include/asm/processor.h
> > > @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
> > >
> > >  extern void riscv_fill_hwcap(void);
> > >
> > > +void riscv_fill_cpu_manufacturer_info(void);
> > > +
> > >  #endif /* __ASSEMBLY__ */
> > >
> > >  #endif /* _ASM_RISCV_PROCESSOR_H */
> > > diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
> > > index f494066051a2..03dee6db404c 100644
> > > --- a/arch/riscv/include/asm/soc.h
> > > +++ b/arch/riscv/include/asm/soc.h
> > > @@ -10,6 +10,7 @@
> > >  #include <linux/of.h>
> > >  #include <linux/linkage.h>
> > >  #include <linux/types.h>
> > > +#include <asm/hwcap.h>
> > >
> > >  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)                       \
> > >         static const struct of_device_id __soc_early_init__##name       \
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index ac202f44a670..389162ee19ea 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -12,6 +12,8 @@
> > >  #include <asm/hwcap.h>
> > >  #include <asm/smp.h>
> > >  #include <asm/switch_to.h>
> > > +#include <asm/sbi.h>
> > > +#include <asm/csr.h>
> > >
> > >  unsigned long elf_hwcap __read_mostly;
> > >
> > > @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> > >  bool has_fpu __read_mostly;
> > >  #endif
> > >
> > > +struct cpu_manufacturer_info_t cpu_mfr_info;
> > > +
> > >  /**
> > >   * riscv_isa_extension_base() - Get base extension word
> > >   *
> > > @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
> > >                 has_fpu = true;
> > >  #endif
> > >  }
> > > +
> > > +void riscv_fill_cpu_manufacturer_info(void)
> > > +{
> > > +#ifndef CONFIG_RISCV_M_MODE
> > > +       cpu_mfr_info.vendor_id = sbi_get_vendorid();
> > > +       cpu_mfr_info.arch_id = sbi_get_archid();
> > > +       cpu_mfr_info.imp_id = sbi_get_impid();
> > > +#else
> > > +       cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> > > +       cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> > > +       cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> > > +#endif
> > How about let opensbi emulate csr_read(CSR_MXXX) for S mode, then we
> > needn't to define new sbi call.
>
> Accessing M-mode CSRs from the S-mode kernel will only make things
> complicated for hypervisors because now hypervisors will also end-up
> emulating M-mode CSRs.
>
> Best would be to only access S-mode CSRs and SBI calls from
> S-mode kernel.
>
> >
> > > +}
> > > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > > index e85bacff1b50..03621ce9092c 100644
> > > --- a/arch/riscv/kernel/setup.c
> > > +++ b/arch/riscv/kernel/setup.c
> > > @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
> > >  #endif
> > >
> > >         riscv_fill_hwcap();
> > > +
> > > +       riscv_fill_cpu_manufacturer_info();
> > >  }
> > >
> > >  static int __init topology_init(void)
> > > diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> > > index a0516172a33c..58f6fd91743a 100644
> > > --- a/arch/riscv/kernel/soc.c
> > > +++ b/arch/riscv/kernel/soc.c
> > > @@ -6,6 +6,7 @@
> > >  #include <linux/libfdt.h>
> > >  #include <linux/pgtable.h>
> > >  #include <asm/soc.h>
> > > +#include <asm/hwcap.h>
> > >
> > >  /*
> > >   * This is called extremly early, before parse_dtb(), to allow initializing
> > > --
> > > 2.7.4
> > >
> > How
> >
> > --
> > Best Regards
> >  Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Palmer Dabbelt]

On Mon, 08 Mar 2021 21:11:04 PST (-0800), anup@brainfault.org wrote:
> On Tue, Mar 9, 2021 at 6:58 AM Guo Ren <guoren@kernel.org> wrote:
>>
>> Hi Vincent,
>>
>> On Mon, Mar 8, 2021 at 11:58 AM Vincent Chen <vincent.chen@sifive.com> wrote:
>> >
>> > Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
>> > ID early in boot so we only need to take the SBI call overhead once.
>> >
>> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
>> > ---
>> >  arch/riscv/include/asm/csr.h       |  3 +++
>> >  arch/riscv/include/asm/hwcap.h     |  6 ++++++
>> >  arch/riscv/include/asm/processor.h |  2 ++
>> >  arch/riscv/include/asm/soc.h       |  1 +
>> >  arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
>> >  arch/riscv/kernel/setup.c          |  2 ++
>> >  arch/riscv/kernel/soc.c            |  1 +
>> >  7 files changed, 32 insertions(+)
>> >
>> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
>> > index caadfc1d7487..87ac65696871 100644
>> > --- a/arch/riscv/include/asm/csr.h
>> > +++ b/arch/riscv/include/asm/csr.h
>> > @@ -115,6 +115,9 @@
>> >  #define CSR_MIP                        0x344
>> >  #define CSR_PMPCFG0            0x3a0
>> >  #define CSR_PMPADDR0           0x3b0
>> > +#define CSR_MVENDORID          0xf11
>> > +#define CSR_MARCHID            0xf12
>> > +#define CSR_MIMPID             0xf13
>> >  #define CSR_MHARTID            0xf14
>> >
>> >  #ifdef CONFIG_RISCV_M_MODE
>> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
>> > index 5ce50468aff1..b7409487c9d2 100644
>> > --- a/arch/riscv/include/asm/hwcap.h
>> > +++ b/arch/riscv/include/asm/hwcap.h
>> > @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>> >  #define riscv_isa_extension_available(isa_bitmap, ext) \
>> >         __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>> >
>> > +struct cpu_manufacturer_info_t {
>> > +       unsigned long vendor_id;
>> > +       unsigned long arch_id;
>> > +       unsigned long imp_id;
>> > +};
>> > +
>> >  #endif
>> >
>> >  #endif /* _ASM_RISCV_HWCAP_H */
>> > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
>> > index 3a240037bde2..4e11a9621d14 100644
>> > --- a/arch/riscv/include/asm/processor.h
>> > +++ b/arch/riscv/include/asm/processor.h
>> > @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
>> >
>> >  extern void riscv_fill_hwcap(void);
>> >
>> > +void riscv_fill_cpu_manufacturer_info(void);
>> > +
>> >  #endif /* __ASSEMBLY__ */
>> >
>> >  #endif /* _ASM_RISCV_PROCESSOR_H */
>> > diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
>> > index f494066051a2..03dee6db404c 100644
>> > --- a/arch/riscv/include/asm/soc.h
>> > +++ b/arch/riscv/include/asm/soc.h
>> > @@ -10,6 +10,7 @@
>> >  #include <linux/of.h>
>> >  #include <linux/linkage.h>
>> >  #include <linux/types.h>
>> > +#include <asm/hwcap.h>
>> >
>> >  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)                       \
>> >         static const struct of_device_id __soc_early_init__##name       \
>> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> > index ac202f44a670..389162ee19ea 100644
>> > --- a/arch/riscv/kernel/cpufeature.c
>> > +++ b/arch/riscv/kernel/cpufeature.c
>> > @@ -12,6 +12,8 @@
>> >  #include <asm/hwcap.h>
>> >  #include <asm/smp.h>
>> >  #include <asm/switch_to.h>
>> > +#include <asm/sbi.h>
>> > +#include <asm/csr.h>
>> >
>> >  unsigned long elf_hwcap __read_mostly;
>> >
>> > @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>> >  bool has_fpu __read_mostly;
>> >  #endif
>> >
>> > +struct cpu_manufacturer_info_t cpu_mfr_info;
>> > +
>> >  /**
>> >   * riscv_isa_extension_base() - Get base extension word
>> >   *
>> > @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
>> >                 has_fpu = true;
>> >  #endif
>> >  }
>> > +
>> > +void riscv_fill_cpu_manufacturer_info(void)
>> > +{
>> > +#ifndef CONFIG_RISCV_M_MODE
>> > +       cpu_mfr_info.vendor_id = sbi_get_vendorid();
>> > +       cpu_mfr_info.arch_id = sbi_get_archid();
>> > +       cpu_mfr_info.imp_id = sbi_get_impid();
>> > +#else
>> > +       cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
>> > +       cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
>> > +       cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
>> > +#endif
>> How about let opensbi emulate csr_read(CSR_MXXX) for S mode, then we
>> needn't to define new sbi call.
>
> Accessing M-mode CSRs from the S-mode kernel will only make things
> complicated for hypervisors because now hypervisors will also end-up
> emulating M-mode CSRs.

IMO that's not really the problem: hypervisors are going to have to emulate 
CSRs anyway, so adding more isn't a big deal.  The problem is having S-mode 
software depend on M-mode.  More explicitly:

* The RISC-V specs are nominally layered, which means S-mode doesn't include 
  any of the M-mode bits. M-mode CSR accesses are simply illegal S-mode 
  instructions.  I know the CSRs are a bit special in that they're chunked out 
  by privilege mode, but there's nothing preventing the ISA from later 
  reallocating these M-mode CSR bits in S-mode (aside from then having a modal 
  ISA, which is a design anti-goal).
* All behavior in M-mode is implementation defined, including these CSR 
  accesses.  While they're obviously allocated and a behavior is specified by 
  the ISA, there's nothing that says M-mode has to actually implement these in 
  any sane fashion (ie, it can return 0 or shut down or whatever).

So is essence, adding these M-mode CSR accesses into S-mode kernels introduces 
an entirely new ISA extension that we're just making up on the spot and 
assuming will be implemented by firmware.  At a bare minimum we'd need to have 
that defined by a specification, but even then I don't see it as the right way 
to go.

> Best would be to only access S-mode CSRs and SBI calls from
> S-mode kernel.

Agreed.  I'm not opposed to expanding the scope of the M-mode kernels to boot 
on more systems, but users who want S-mode shouldn't end up with M-mode 
dependencies sneaking in.  So in this case, I'm very much in favor of the SBI 
call over accessing an M-mode CSR.

>
>>
>> > +}
>> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
>> > index e85bacff1b50..03621ce9092c 100644
>> > --- a/arch/riscv/kernel/setup.c
>> > +++ b/arch/riscv/kernel/setup.c
>> > @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
>> >  #endif
>> >
>> >         riscv_fill_hwcap();
>> > +
>> > +       riscv_fill_cpu_manufacturer_info();
>> >  }
>> >
>> >  static int __init topology_init(void)
>> > diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
>> > index a0516172a33c..58f6fd91743a 100644
>> > --- a/arch/riscv/kernel/soc.c
>> > +++ b/arch/riscv/kernel/soc.c
>> > @@ -6,6 +6,7 @@
>> >  #include <linux/libfdt.h>
>> >  #include <linux/pgtable.h>
>> >  #include <asm/soc.h>
>> > +#include <asm/hwcap.h>
>> >
>> >  /*
>> >   * This is called extremly early, before parse_dtb(), to allow initializing
>> > --
>> > 2.7.4
>> >
>> How
>>
>> --
>> Best Regards
>>  Guo Ren
>>
>> ML: https://lore.kernel.org/linux-csky/
>>
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Guo Ren]

Got it. Thx for more clarification.

On Wed, Mar 10, 2021 at 11:40 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Mon, 08 Mar 2021 21:11:04 PST (-0800), anup@brainfault.org wrote:
> > On Tue, Mar 9, 2021 at 6:58 AM Guo Ren <guoren@kernel.org> wrote:
> >>
> >> Hi Vincent,
> >>
> >> On Mon, Mar 8, 2021 at 11:58 AM Vincent Chen <vincent.chen@sifive.com> wrote:
> >> >
> >> > Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
> >> > ID early in boot so we only need to take the SBI call overhead once.
> >> >
> >> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> >> > ---
> >> >  arch/riscv/include/asm/csr.h       |  3 +++
> >> >  arch/riscv/include/asm/hwcap.h     |  6 ++++++
> >> >  arch/riscv/include/asm/processor.h |  2 ++
> >> >  arch/riscv/include/asm/soc.h       |  1 +
> >> >  arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
> >> >  arch/riscv/kernel/setup.c          |  2 ++
> >> >  arch/riscv/kernel/soc.c            |  1 +
> >> >  7 files changed, 32 insertions(+)
> >> >
> >> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> >> > index caadfc1d7487..87ac65696871 100644
> >> > --- a/arch/riscv/include/asm/csr.h
> >> > +++ b/arch/riscv/include/asm/csr.h
> >> > @@ -115,6 +115,9 @@
> >> >  #define CSR_MIP                        0x344
> >> >  #define CSR_PMPCFG0            0x3a0
> >> >  #define CSR_PMPADDR0           0x3b0
> >> > +#define CSR_MVENDORID          0xf11
> >> > +#define CSR_MARCHID            0xf12
> >> > +#define CSR_MIMPID             0xf13
> >> >  #define CSR_MHARTID            0xf14
> >> >
> >> >  #ifdef CONFIG_RISCV_M_MODE
> >> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> >> > index 5ce50468aff1..b7409487c9d2 100644
> >> > --- a/arch/riscv/include/asm/hwcap.h
> >> > +++ b/arch/riscv/include/asm/hwcap.h
> >> > @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
> >> >  #define riscv_isa_extension_available(isa_bitmap, ext) \
> >> >         __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
> >> >
> >> > +struct cpu_manufacturer_info_t {
> >> > +       unsigned long vendor_id;
> >> > +       unsigned long arch_id;
> >> > +       unsigned long imp_id;
> >> > +};
> >> > +
> >> >  #endif
> >> >
> >> >  #endif /* _ASM_RISCV_HWCAP_H */
> >> > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> >> > index 3a240037bde2..4e11a9621d14 100644
> >> > --- a/arch/riscv/include/asm/processor.h
> >> > +++ b/arch/riscv/include/asm/processor.h
> >> > @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
> >> >
> >> >  extern void riscv_fill_hwcap(void);
> >> >
> >> > +void riscv_fill_cpu_manufacturer_info(void);
> >> > +
> >> >  #endif /* __ASSEMBLY__ */
> >> >
> >> >  #endif /* _ASM_RISCV_PROCESSOR_H */
> >> > diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
> >> > index f494066051a2..03dee6db404c 100644
> >> > --- a/arch/riscv/include/asm/soc.h
> >> > +++ b/arch/riscv/include/asm/soc.h
> >> > @@ -10,6 +10,7 @@
> >> >  #include <linux/of.h>
> >> >  #include <linux/linkage.h>
> >> >  #include <linux/types.h>
> >> > +#include <asm/hwcap.h>
> >> >
> >> >  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)                       \
> >> >         static const struct of_device_id __soc_early_init__##name       \
> >> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> >> > index ac202f44a670..389162ee19ea 100644
> >> > --- a/arch/riscv/kernel/cpufeature.c
> >> > +++ b/arch/riscv/kernel/cpufeature.c
> >> > @@ -12,6 +12,8 @@
> >> >  #include <asm/hwcap.h>
> >> >  #include <asm/smp.h>
> >> >  #include <asm/switch_to.h>
> >> > +#include <asm/sbi.h>
> >> > +#include <asm/csr.h>
> >> >
> >> >  unsigned long elf_hwcap __read_mostly;
> >> >
> >> > @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
> >> >  bool has_fpu __read_mostly;
> >> >  #endif
> >> >
> >> > +struct cpu_manufacturer_info_t cpu_mfr_info;
> >> > +
> >> >  /**
> >> >   * riscv_isa_extension_base() - Get base extension word
> >> >   *
> >> > @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
> >> >                 has_fpu = true;
> >> >  #endif
> >> >  }
> >> > +
> >> > +void riscv_fill_cpu_manufacturer_info(void)
> >> > +{
> >> > +#ifndef CONFIG_RISCV_M_MODE
> >> > +       cpu_mfr_info.vendor_id = sbi_get_vendorid();
> >> > +       cpu_mfr_info.arch_id = sbi_get_archid();
> >> > +       cpu_mfr_info.imp_id = sbi_get_impid();
> >> > +#else
> >> > +       cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> >> > +       cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> >> > +       cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> >> > +#endif
> >> How about let opensbi emulate csr_read(CSR_MXXX) for S mode, then we
> >> needn't to define new sbi call.
> >
> > Accessing M-mode CSRs from the S-mode kernel will only make things
> > complicated for hypervisors because now hypervisors will also end-up
> > emulating M-mode CSRs.
>
> IMO that's not really the problem: hypervisors are going to have to emulate
> CSRs anyway, so adding more isn't a big deal.  The problem is having S-mode
> software depend on M-mode.  More explicitly:
>
> * The RISC-V specs are nominally layered, which means S-mode doesn't include
>   any of the M-mode bits. M-mode CSR accesses are simply illegal S-mode
>   instructions.  I know the CSRs are a bit special in that they're chunked out
>   by privilege mode, but there's nothing preventing the ISA from later
>   reallocating these M-mode CSR bits in S-mode (aside from then having a modal
>   ISA, which is a design anti-goal).
> * All behavior in M-mode is implementation defined, including these CSR
>   accesses.  While they're obviously allocated and a behavior is specified by
>   the ISA, there's nothing that says M-mode has to actually implement these in
>   any sane fashion (ie, it can return 0 or shut down or whatever).
>
> So is essence, adding these M-mode CSR accesses into S-mode kernels introduces
> an entirely new ISA extension that we're just making up on the spot and
> assuming will be implemented by firmware.  At a bare minimum we'd need to have
> that defined by a specification, but even then I don't see it as the right way
> to go.
>
> > Best would be to only access S-mode CSRs and SBI calls from
> > S-mode kernel.
>
> Agreed.  I'm not opposed to expanding the scope of the M-mode kernels to boot
> on more systems, but users who want S-mode shouldn't end up with M-mode
> dependencies sneaking in.  So in this case, I'm very much in favor of the SBI
> call over accessing an M-mode CSR.
>
> >
> >>
> >> > +}
> >> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> >> > index e85bacff1b50..03621ce9092c 100644
> >> > --- a/arch/riscv/kernel/setup.c
> >> > +++ b/arch/riscv/kernel/setup.c
> >> > @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
> >> >  #endif
> >> >
> >> >         riscv_fill_hwcap();
> >> > +
> >> > +       riscv_fill_cpu_manufacturer_info();
> >> >  }
> >> >
> >> >  static int __init topology_init(void)
> >> > diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> >> > index a0516172a33c..58f6fd91743a 100644
> >> > --- a/arch/riscv/kernel/soc.c
> >> > +++ b/arch/riscv/kernel/soc.c
> >> > @@ -6,6 +6,7 @@
> >> >  #include <linux/libfdt.h>
> >> >  #include <linux/pgtable.h>
> >> >  #include <asm/soc.h>
> >> > +#include <asm/hwcap.h>
> >> >
> >> >  /*
> >> >   * This is called extremly early, before parse_dtb(), to allow initializing
> >> > --
> >> > 2.7.4
> >> >
> >> How
> >>
> >> --
> >> Best Regards
> >>  Guo Ren
> >>
> >> ML: https://lore.kernel.org/linux-csky/
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> linux-riscv@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-riscv



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 2/4] riscv: Get CPU manufacturer information

[Palmer Dabbelt]

On Sun, 07 Mar 2021 19:58:15 PST (-0800), vincent.chen@sifive.com wrote:
> Issue 3 SBI calls to get the vendor ID, architecture ID and implementation
> ID early in boot so we only need to take the SBI call overhead once.
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  arch/riscv/include/asm/csr.h       |  3 +++
>  arch/riscv/include/asm/hwcap.h     |  6 ++++++
>  arch/riscv/include/asm/processor.h |  2 ++
>  arch/riscv/include/asm/soc.h       |  1 +
>  arch/riscv/kernel/cpufeature.c     | 17 +++++++++++++++++
>  arch/riscv/kernel/setup.c          |  2 ++
>  arch/riscv/kernel/soc.c            |  1 +
>  7 files changed, 32 insertions(+)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index caadfc1d7487..87ac65696871 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -115,6 +115,9 @@
>  #define CSR_MIP			0x344
>  #define CSR_PMPCFG0		0x3a0
>  #define CSR_PMPADDR0		0x3b0
> +#define CSR_MVENDORID		0xf11
> +#define CSR_MARCHID		0xf12
> +#define CSR_MIMPID		0xf13
>  #define CSR_MHARTID		0xf14
>
>  #ifdef CONFIG_RISCV_M_MODE
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 5ce50468aff1..b7409487c9d2 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -44,6 +44,12 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit);
>  #define riscv_isa_extension_available(isa_bitmap, ext)	\
>  	__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext)
>
> +struct cpu_manufacturer_info_t {
> +	unsigned long vendor_id;
> +	unsigned long arch_id;
> +	unsigned long imp_id;
> +};
> +
>  #endif
>
>  #endif /* _ASM_RISCV_HWCAP_H */
> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 3a240037bde2..4e11a9621d14 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -72,6 +72,8 @@ int riscv_of_parent_hartid(struct device_node *node);
>
>  extern void riscv_fill_hwcap(void);
>
> +void riscv_fill_cpu_manufacturer_info(void);
> +
>  #endif /* __ASSEMBLY__ */
>
>  #endif /* _ASM_RISCV_PROCESSOR_H */
> diff --git a/arch/riscv/include/asm/soc.h b/arch/riscv/include/asm/soc.h
> index f494066051a2..03dee6db404c 100644
> --- a/arch/riscv/include/asm/soc.h
> +++ b/arch/riscv/include/asm/soc.h
> @@ -10,6 +10,7 @@
>  #include <linux/of.h>
>  #include <linux/linkage.h>
>  #include <linux/types.h>
> +#include <asm/hwcap.h>
>
>  #define SOC_EARLY_INIT_DECLARE(name, compat, fn)			\
>  	static const struct of_device_id __soc_early_init__##name	\
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index ac202f44a670..389162ee19ea 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -12,6 +12,8 @@
>  #include <asm/hwcap.h>
>  #include <asm/smp.h>
>  #include <asm/switch_to.h>
> +#include <asm/sbi.h>
> +#include <asm/csr.h>
>
>  unsigned long elf_hwcap __read_mostly;
>
> @@ -22,6 +24,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
>  bool has_fpu __read_mostly;
>  #endif
>
> +struct cpu_manufacturer_info_t cpu_mfr_info;
> +
>  /**
>   * riscv_isa_extension_base() - Get base extension word
>   *
> @@ -149,3 +153,16 @@ void riscv_fill_hwcap(void)
>  		has_fpu = true;
>  #endif
>  }
> +
> +void riscv_fill_cpu_manufacturer_info(void)
> +{
> +#ifndef CONFIG_RISCV_M_MODE
> +	cpu_mfr_info.vendor_id = sbi_get_vendorid();
> +	cpu_mfr_info.arch_id = sbi_get_archid();
> +	cpu_mfr_info.imp_id = sbi_get_impid();
> +#else
> +	cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID);
> +	cpu_mfr_info.arch_id = csr_read(CSR_MARCHID);
> +	cpu_mfr_info.imp_id = csr_read(CSR_MIMPID);
> +#endif
> +}
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index e85bacff1b50..03621ce9092c 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -278,6 +278,8 @@ void __init setup_arch(char **cmdline_p)
>  #endif
>
>  	riscv_fill_hwcap();
> +
> +	riscv_fill_cpu_manufacturer_info();
>  }
>
>  static int __init topology_init(void)
> diff --git a/arch/riscv/kernel/soc.c b/arch/riscv/kernel/soc.c
> index a0516172a33c..58f6fd91743a 100644
> --- a/arch/riscv/kernel/soc.c
> +++ b/arch/riscv/kernel/soc.c
> @@ -6,6 +6,7 @@
>  #include <linux/libfdt.h>
>  #include <linux/pgtable.h>
>  #include <asm/soc.h>
> +#include <asm/hwcap.h>
>
>  /*
>   * This is called extremly early, before parse_dtb(), to allow initializing

Reviewed-by: Palmer Dabbelt <palmerdabbelt@google.com>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC patch 3/4] riscv: Introduce alternative mechanism to apply errata solution

[Vincent Chen]

Introduce the "alternative" mechanism from ARM64 and x86 to apply the CPU
vendors' errata solution at runtime. The main purpose of this patch is
to provide a framework. Therefore, the implementation is quite basic for
now so that some scenarios could not use this schemei, such as patching
code to a module, relocating the patching code and heterogeneous CPU
topology.

Users could use the macro ALTERNATIVE to apply an errata to the existing
code flow. In the macro ALTERNATIVE, users need to specify the manufacturer
information(vendorid, archid, and impid) for this errata. Therefore, kernel
will know this errata is suitable for which CPU core. During the booting
procedure, kernel will select the errata required by the CPU core and then
patch it. It means that the kernel only applies the errata to the specified
CPU core. In this case the vendor's errata does not affect each other at
runtime. The above patching procedure only occurs during the booting phase,
so we only take the overhead of the "alternative" mechanism once.

This "alternative" mechanism is enabled by default to ensure that all
required errata will be applied. However, users can disable this feature by
the Kconfig "CONFIG_RISCV_ERRATA_ALTERNATIVE".

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
---
 arch/riscv/Kconfig                          |   1 +
 arch/riscv/Kconfig.erratas                  |  12 +++
 arch/riscv/Makefile                         |   1 +
 arch/riscv/errata/Makefile                  |   1 +
 arch/riscv/errata/alternative.c             |  69 +++++++++++++++++
 arch/riscv/include/asm/alternative-macros.h | 110 ++++++++++++++++++++++++++++
 arch/riscv/include/asm/alternative.h        |  44 +++++++++++
 arch/riscv/include/asm/asm.h                |   1 +
 arch/riscv/include/asm/errata_list.h        |   8 ++
 arch/riscv/include/asm/sections.h           |   2 +
 arch/riscv/include/asm/vendorid_list.h      |   6 ++
 arch/riscv/kernel/smpboot.c                 |   4 +
 arch/riscv/kernel/vmlinux.lds.S             |  14 ++++
 13 files changed, 273 insertions(+)
 create mode 100644 arch/riscv/Kconfig.erratas
 create mode 100644 arch/riscv/errata/Makefile
 create mode 100644 arch/riscv/errata/alternative.c
 create mode 100644 arch/riscv/include/asm/alternative-macros.h
 create mode 100644 arch/riscv/include/asm/alternative.h
 create mode 100644 arch/riscv/include/asm/errata_list.h
 create mode 100644 arch/riscv/include/asm/vendorid_list.h

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a998babc1237..2e26251fe1f2 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -204,6 +204,7 @@ config LOCKDEP_SUPPORT
 	def_bool y
 
 source "arch/riscv/Kconfig.socs"
+source "arch/riscv/Kconfig.erratas"
 
 menu "Platform type"
 
diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
new file mode 100644
index 000000000000..4d0bafc536df
--- /dev/null
+++ b/arch/riscv/Kconfig.erratas
@@ -0,0 +1,12 @@
+menu "CPU errata selection"
+
+config RISCV_ERRATA_ALTERNATIVE
+	bool "RISC-V alternative scheme"
+	default y
+	help
+	  This Kconfig allows the kernel to automatically patch the
+	  errata required by the execution platform at run time. The
+	  code patching is performed once in the boot stages. It means
+	  that the overhead from this mechanism is just taken once.
+
+endmenu
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 1368d943f1f3..1f5c03082976 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -87,6 +87,7 @@ KBUILD_IMAGE	:= $(boot)/Image.gz
 head-y := arch/riscv/kernel/head.o
 
 core-y += arch/riscv/
+core-$(CONFIG_RISCV_ERRATA_ALTERNATIVE) += arch/riscv/errata/
 
 libs-y += arch/riscv/lib/
 libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
new file mode 100644
index 000000000000..43e6d5424367
--- /dev/null
+++ b/arch/riscv/errata/Makefile
@@ -0,0 +1 @@
+obj-y	+= alternative.o
diff --git a/arch/riscv/errata/alternative.c b/arch/riscv/errata/alternative.c
new file mode 100644
index 000000000000..052affdce6eb
--- /dev/null
+++ b/arch/riscv/errata/alternative.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * alternative runtime patching
+ * inspired by the ARM64 and x86 version
+ *
+ * Copyright (C) 2021 Sifive.
+ */
+
+#include <linux/init.h>
+#include <linux/cpu.h>
+#include <linux/uaccess.h>
+#include <asm/patch.h>
+#include <asm/alternative.h>
+#include <asm/sections.h>
+
+struct alt_region {
+	struct alt_entry *begin;
+	struct alt_entry *end;
+};
+
+static bool (*errata_checkfunc)(struct alt_entry *alt);
+typedef int (*patch_func_t)(void *addr, const void *insn, size_t size);
+
+static void __apply_alternatives(void *alt_region, void *alt_patch_func)
+{
+	struct alt_entry *alt;
+	struct alt_region *region = alt_region;
+
+	for (alt = region->begin; alt < region->end; alt++) {
+		if (!errata_checkfunc(alt))
+			continue;
+		((patch_func_t)alt_patch_func)(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+	}
+}
+
+static void __init init_alternative(void)
+{
+	struct errata_checkfunc_id *ptr;
+
+	for (ptr = (struct errata_checkfunc_id *)__alt_checkfunc_table;
+	     ptr < (struct errata_checkfunc_id *)__alt_checkfunc_table_end;
+	     ptr++) {
+		if (cpu_mfr_info.vendor_id == ptr->vendor_id)
+			errata_checkfunc = ptr->func;
+	}
+}
+
+/*
+ * This is called very early in the boot process (directly after we run
+ * a feature detect on the boot CPU). No need to worry about other CPUs
+ * here.
+ */
+void __init apply_boot_alternatives(void)
+{
+	struct alt_region region;
+
+	/* If called on non-boot cpu things could go wrong */
+	WARN_ON(smp_processor_id() != 0);
+
+	init_alternative();
+
+	if (!errata_checkfunc)
+		return;
+
+	region.begin = (struct alt_entry *)__alt_start;
+	region.end = (struct alt_entry *)__alt_end;
+	__apply_alternatives(&region, patch_text_nosync);
+}
+
diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
new file mode 100644
index 000000000000..7b6f0c94b1fb
--- /dev/null
+++ b/arch/riscv/include/asm/alternative-macros.h
@@ -0,0 +1,110 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_ALTERNATIVE_MACROS_H
+#define __ASM_ALTERNATIVE_MACROS_H
+
+#ifdef CONFIG_RISCV_ERRATA_ALTERNATIVE
+
+#ifndef __ASSEMBLY__
+
+#include <asm/asm.h>
+#include <linux/stringify.h>
+
+#define ALT_ENTRY(oldptr, altptr, vendor_id, errata_id, altlen) \
+	RISCV_PTR " " oldptr "\n" \
+	RISCV_PTR " " altptr "\n" \
+	REG_ASM " " vendor_id "\n" \
+	REG_ASM " " altlen "\n" \
+	".word " errata_id "\n"
+
+#define __ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, enable) \
+	"886 :\n"							\
+	oldinsn "\n"							\
+	".if " __stringify(enable) " == 1\n"				\
+	"887 :\n"							\
+	".pushsection .alternative, \"a\"\n"				\
+	ALT_ENTRY("886b", "888f", __stringify(vendor_id), __stringify(errata_id), "889f - 888f") \
+	".popsection\n"							\
+	".subsection 1\n"						\
+	"888 :\n"							\
+	altinsn "\n"							\
+	"889 :\n"							\
+	".previous\n"							\
+	".org	. - (887b - 886b) + (889b - 888b)\n"			\
+	".org	. - (889b - 888b) + (887b - 886b)\n"			\
+	".endif\n"
+
+#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)	\
+	__ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, IS_ENABLED(CONFIG_k))
+
+#else /* __ASSEMBLY__ */
+
+.macro ALT_ENTRY oldptr altptr vendor_id errata_id alt_len
+	RISCV_PTR \oldptr
+	RISCV_PTR \altptr
+	REG_ASM \vendor_id
+	REG_ASM \alt_len
+	.word	\errata_id
+.endm
+
+.macro __ALTERNATIVE_CFG insn1 insn2 vendor_id errata_id enable = 1
+886 :
+	\insn1
+	.if \enable
+887 :
+	.pushsection .alternative, "a"
+	ALT_ENTRY 886b, 888f, \vendor_id, \errata_id, 889f - 888f
+	.popsection
+	.subsection 1
+888 :
+	\insn2
+889 :
+	.previous
+	.org    . - (889b - 888b) + (887b - 886b)
+	.org    . - (887b - 886b) + (889b - 888b)
+	.endif
+.endm
+
+#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
+	__ALTERNATIVE_CFG oldinsn, altinsn, vendor_id, errata_id, IS_ENABLED(CONFIG_k)
+
+#endif /* !__ASSEMBLY__ */
+
+#else /* !CONFIG_RISCV_ERRATA_ALTERNATIVE*/
+#ifndef __ASSEMBLY__
+
+#define __ALTERNATIVE_CFG(oldinsn)  \
+	oldinsn "\n"
+
+#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
+	__ALTERNATIVE_CFG(oldinsn)
+
+#else /* __ASSEMBLY__ */
+
+.macro __ALTERNATIVE_CFG insn1
+	\insn1
+.endm
+
+#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
+	__ALTERNATIVE_CFG oldinsn
+
+#endif /* !__ASSEMBLY__ */
+#endif /* CONFIG_RISCV_ERRATA_ALTERNATIVE */
+
+/*
+ * Usage:
+ *	ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)
+ *  in the assembly code. Otherwise,
+ *	asm(ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k));
+ *
+ * oldinsn: The old instruction which will be replaced.
+ * altinsn: The replacement instruction.
+ * vendor_id: The CPU vendor ID.
+ * errata_id: The errata ID.
+ * CONFIG_k: The Kconfig of this errata. The instructions replacement can
+ *           be disabled by this Kconfig. When Kconfig is disabled, the
+ *           oldinsn will always be executed.
+ */
+#define ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)   \
+	_ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)
+
+#endif
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
new file mode 100644
index 000000000000..98a0ea331a27
--- /dev/null
+++ b/arch/riscv/include/asm/alternative.h
@@ -0,0 +1,44 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 Sifive.
+ */
+
+#ifndef __ASM_ALTERNATIVE_H
+#define __ASM_ALTERNATIVE_H
+
+#define ERRATA_STRING_LENGTH_MAX 32
+
+#include <asm/alternative-macros.h>
+
+#ifndef __ASSEMBLY__
+
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/stddef.h>
+#include <asm/hwcap.h>
+
+void __init apply_boot_alternatives(void);
+
+struct alt_entry {
+	void *old_ptr;		 /* address of original instruciton or data  */
+	void *alt_ptr;		 /* address of replacement instruction or data */
+	unsigned long vendor_id; /* cpu vendor id */
+	unsigned long alt_len;   /* The replacement size */
+	unsigned int errata_id;  /* The errata id */
+} __packed;
+
+struct errata_checkfunc_id {
+	unsigned long vendor_id;
+	bool (*func)(struct alt_entry *alt);
+};
+
+extern struct cpu_manufacturer_info_t cpu_mfr_info;
+
+#define REGISTER_ERRATA_CHECKFUNC(checkfunc, vendorid)			  \
+	static const struct errata_checkfunc_id _errata_check_##vendorid  \
+	__used __section(".alt_checkfunc_table")			  \
+	__aligned(__alignof__(struct errata_checkfunc_id)) =		  \
+	{ .vendor_id = vendorid,					  \
+	  .func = checkfunc }
+#endif
+#endif
diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
index 9c992a88d858..e20646aa97e5 100644
--- a/arch/riscv/include/asm/asm.h
+++ b/arch/riscv/include/asm/asm.h
@@ -23,6 +23,7 @@
 #define REG_L		__REG_SEL(ld, lw)
 #define REG_S		__REG_SEL(sd, sw)
 #define REG_SC		__REG_SEL(sc.d, sc.w)
+#define REG_ASM 	__REG_SEL(.dword, .word)
 #define SZREG		__REG_SEL(8, 4)
 #define LGREG		__REG_SEL(3, 2)
 
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
new file mode 100644
index 000000000000..5e5a1fcd90ba
--- /dev/null
+++ b/arch/riscv/include/asm/errata_list.h
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 Sifive.
+ */
+
+#ifdef CONFIG_SOC_SIFIVE
+#define	ERRATA_NUMBER 0
+#endif
diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
index 1595c5b60cfd..d13160f46d4e 100644
--- a/arch/riscv/include/asm/sections.h
+++ b/arch/riscv/include/asm/sections.h
@@ -11,5 +11,7 @@ extern char _start[];
 extern char _start_kernel[];
 extern char __init_data_begin[], __init_data_end[];
 extern char __init_text_begin[], __init_text_end[];
+extern char __alt_checkfunc_table[], __alt_checkfunc_table_end[];
+extern char __alt_start[], __alt_end[];
 
 #endif /* __ASM_SECTIONS_H */
diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
new file mode 100644
index 000000000000..1f3be47decb6
--- /dev/null
+++ b/arch/riscv/include/asm/vendorid_list.h
@@ -0,0 +1,6 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 SiFive
+ */
+
+#define SIFIVE_VENDOR_ID	0x489
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 5e276c25646f..9a408e2942ac 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -32,6 +32,7 @@
 #include <asm/sections.h>
 #include <asm/sbi.h>
 #include <asm/smp.h>
+#include <asm/alternative.h>
 
 #include "head.h"
 
@@ -40,6 +41,9 @@ static DECLARE_COMPLETION(cpu_running);
 void __init smp_prepare_boot_cpu(void)
 {
 	init_cpu_topology();
+#ifdef CONFIG_RISCV_ERRATA_ALTERNATIVE
+	apply_boot_alternatives();
+#endif
 }
 
 void __init smp_prepare_cpus(unsigned int max_cpus)
diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index de03cb22d0e9..6503dfca65b0 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -90,6 +90,20 @@ SECTIONS
 	}
 
 	__init_data_end = .;
+
+	. = ALIGN(8);
+	.alt_checkfunc_table : {
+		__alt_checkfunc_table = .;
+		*(.alt_checkfunc_table)
+		__alt_checkfunc_table_end = .;
+	}
+
+	. = ALIGN(8);
+	.alternative : {
+		__alt_start = .;
+		*(.alternative)
+		__alt_end = .;
+	}
 	__init_end = .;
 
 	/* Start of data section */
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 3/4] riscv: Introduce alternative mechanism to apply errata solution

[Palmer Dabbelt]

On Sun, 07 Mar 2021 19:58:16 PST (-0800), vincent.chen@sifive.com wrote:
> Introduce the "alternative" mechanism from ARM64 and x86 to apply the CPU
> vendors' errata solution at runtime. The main purpose of this patch is
> to provide a framework. Therefore, the implementation is quite basic for
> now so that some scenarios could not use this schemei, such as patching
> code to a module, relocating the patching code and heterogeneous CPU
> topology.
>
> Users could use the macro ALTERNATIVE to apply an errata to the existing
> code flow. In the macro ALTERNATIVE, users need to specify the manufacturer
> information(vendorid, archid, and impid) for this errata. Therefore, kernel
> will know this errata is suitable for which CPU core. During the booting
> procedure, kernel will select the errata required by the CPU core and then
> patch it. It means that the kernel only applies the errata to the specified
> CPU core. In this case the vendor's errata does not affect each other at
> runtime. The above patching procedure only occurs during the booting phase,
> so we only take the overhead of the "alternative" mechanism once.
>
> This "alternative" mechanism is enabled by default to ensure that all
> required errata will be applied. However, users can disable this feature by
> the Kconfig "CONFIG_RISCV_ERRATA_ALTERNATIVE".
>
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> ---
>  arch/riscv/Kconfig                          |   1 +
>  arch/riscv/Kconfig.erratas                  |  12 +++
>  arch/riscv/Makefile                         |   1 +
>  arch/riscv/errata/Makefile                  |   1 +
>  arch/riscv/errata/alternative.c             |  69 +++++++++++++++++
>  arch/riscv/include/asm/alternative-macros.h | 110 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/alternative.h        |  44 +++++++++++
>  arch/riscv/include/asm/asm.h                |   1 +
>  arch/riscv/include/asm/errata_list.h        |   8 ++
>  arch/riscv/include/asm/sections.h           |   2 +
>  arch/riscv/include/asm/vendorid_list.h      |   6 ++
>  arch/riscv/kernel/smpboot.c                 |   4 +
>  arch/riscv/kernel/vmlinux.lds.S             |  14 ++++
>  13 files changed, 273 insertions(+)
>  create mode 100644 arch/riscv/Kconfig.erratas
>  create mode 100644 arch/riscv/errata/Makefile
>  create mode 100644 arch/riscv/errata/alternative.c
>  create mode 100644 arch/riscv/include/asm/alternative-macros.h
>  create mode 100644 arch/riscv/include/asm/alternative.h
>  create mode 100644 arch/riscv/include/asm/errata_list.h
>  create mode 100644 arch/riscv/include/asm/vendorid_list.h
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index a998babc1237..2e26251fe1f2 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -204,6 +204,7 @@ config LOCKDEP_SUPPORT
>  	def_bool y
>
>  source "arch/riscv/Kconfig.socs"
> +source "arch/riscv/Kconfig.erratas"
>
>  menu "Platform type"
>
> diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
> new file mode 100644
> index 000000000000..4d0bafc536df
> --- /dev/null
> +++ b/arch/riscv/Kconfig.erratas
> @@ -0,0 +1,12 @@
> +menu "CPU errata selection"
> +
> +config RISCV_ERRATA_ALTERNATIVE
> +	bool "RISC-V alternative scheme"
> +	default y
> +	help
> +	  This Kconfig allows the kernel to automatically patch the
> +	  errata required by the execution platform at run time. The
> +	  code patching is performed once in the boot stages. It means
> +	  that the overhead from this mechanism is just taken once.
> +
> +endmenu
> diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> index 1368d943f1f3..1f5c03082976 100644
> --- a/arch/riscv/Makefile
> +++ b/arch/riscv/Makefile
> @@ -87,6 +87,7 @@ KBUILD_IMAGE	:= $(boot)/Image.gz
>  head-y := arch/riscv/kernel/head.o
>
>  core-y += arch/riscv/
> +core-$(CONFIG_RISCV_ERRATA_ALTERNATIVE) += arch/riscv/errata/
>
>  libs-y += arch/riscv/lib/
>  libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
> new file mode 100644
> index 000000000000..43e6d5424367
> --- /dev/null
> +++ b/arch/riscv/errata/Makefile
> @@ -0,0 +1 @@
> +obj-y	+= alternative.o
> diff --git a/arch/riscv/errata/alternative.c b/arch/riscv/errata/alternative.c
> new file mode 100644
> index 000000000000..052affdce6eb
> --- /dev/null
> +++ b/arch/riscv/errata/alternative.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * alternative runtime patching
> + * inspired by the ARM64 and x86 version
> + *
> + * Copyright (C) 2021 Sifive.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/cpu.h>
> +#include <linux/uaccess.h>
> +#include <asm/patch.h>
> +#include <asm/alternative.h>
> +#include <asm/sections.h>
> +
> +struct alt_region {
> +	struct alt_entry *begin;
> +	struct alt_entry *end;
> +};
> +
> +static bool (*errata_checkfunc)(struct alt_entry *alt);
> +typedef int (*patch_func_t)(void *addr, const void *insn, size_t size);
> +
> +static void __apply_alternatives(void *alt_region, void *alt_patch_func)
> +{
> +	struct alt_entry *alt;
> +	struct alt_region *region = alt_region;
> +
> +	for (alt = region->begin; alt < region->end; alt++) {
> +		if (!errata_checkfunc(alt))
> +			continue;
> +		((patch_func_t)alt_patch_func)(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> +	}
> +}
> +
> +static void __init init_alternative(void)
> +{
> +	struct errata_checkfunc_id *ptr;
> +
> +	for (ptr = (struct errata_checkfunc_id *)__alt_checkfunc_table;
> +	     ptr < (struct errata_checkfunc_id *)__alt_checkfunc_table_end;
> +	     ptr++) {
> +		if (cpu_mfr_info.vendor_id == ptr->vendor_id)

Of those three new SBI calls this is the only one I see used.  You could get 
rid of the whole global caching thing and just call it once here, which would 
be a bit simpler.

I'm also not convinced that it's worth all this table stuff here.  It's not 
like we have new vendors every day, we could just have a switch statement on 
sbi_get_mvendorid() and stick every vendor's errata probe in there.

At that point this would be simple enough that I'd be inclined to 
unconditionally (at least on S-mode kernels, the M-mode kernels aren't portable 
so who cares) include the errata checking.  I can understand not wanting to 
take all the errata fixing, as some of those could get complex, but if we have 
the check in there we can provide a warning to users who don't have the errata 
fix.

If that gets out of hand or we have non-portable kernels we can always hoist 
the ifdefs to avoid the check functions as well, but I'd prefer that portable 
kernels are at least capable of detecting they've been built in a fashion that 
will subtly break things.

> +			errata_checkfunc = ptr->func;
> +	}
> +}
> +
> +/*
> + * This is called very early in the boot process (directly after we run
> + * a feature detect on the boot CPU). No need to worry about other CPUs
> + * here.
> + */
> +void __init apply_boot_alternatives(void)
> +{
> +	struct alt_region region;
> +
> +	/* If called on non-boot cpu things could go wrong */
> +	WARN_ON(smp_processor_id() != 0);
> +
> +	init_alternative();
> +
> +	if (!errata_checkfunc)
> +		return;
> +
> +	region.begin = (struct alt_entry *)__alt_start;
> +	region.end = (struct alt_entry *)__alt_end;
> +	__apply_alternatives(&region, patch_text_nosync);
> +}
> +
> diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> new file mode 100644
> index 000000000000..7b6f0c94b1fb
> --- /dev/null
> +++ b/arch/riscv/include/asm/alternative-macros.h
> @@ -0,0 +1,110 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_ALTERNATIVE_MACROS_H
> +#define __ASM_ALTERNATIVE_MACROS_H
> +
> +#ifdef CONFIG_RISCV_ERRATA_ALTERNATIVE
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <asm/asm.h>
> +#include <linux/stringify.h>
> +
> +#define ALT_ENTRY(oldptr, altptr, vendor_id, errata_id, altlen) \
> +	RISCV_PTR " " oldptr "\n" \
> +	RISCV_PTR " " altptr "\n" \
> +	REG_ASM " " vendor_id "\n" \
> +	REG_ASM " " altlen "\n" \
> +	".word " errata_id "\n"
> +
> +#define __ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, enable) \
> +	"886 :\n"							\
> +	oldinsn "\n"							\
> +	".if " __stringify(enable) " == 1\n"				\
> +	"887 :\n"							\
> +	".pushsection .alternative, \"a\"\n"				\
> +	ALT_ENTRY("886b", "888f", __stringify(vendor_id), __stringify(errata_id), "889f - 888f") \
> +	".popsection\n"							\
> +	".subsection 1\n"						\
> +	"888 :\n"							\
> +	altinsn "\n"							\
> +	"889 :\n"							\
> +	".previous\n"							\
> +	".org	. - (887b - 886b) + (889b - 888b)\n"			\
> +	".org	. - (889b - 888b) + (887b - 886b)\n"			\
> +	".endif\n"
> +
> +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)	\
> +	__ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, IS_ENABLED(CONFIG_k))
> +
> +#else /* __ASSEMBLY__ */
> +
> +.macro ALT_ENTRY oldptr altptr vendor_id errata_id alt_len
> +	RISCV_PTR \oldptr
> +	RISCV_PTR \altptr
> +	REG_ASM \vendor_id
> +	REG_ASM \alt_len
> +	.word	\errata_id
> +.endm
> +
> +.macro __ALTERNATIVE_CFG insn1 insn2 vendor_id errata_id enable = 1
> +886 :
> +	\insn1
> +	.if \enable
> +887 :
> +	.pushsection .alternative, "a"
> +	ALT_ENTRY 886b, 888f, \vendor_id, \errata_id, 889f - 888f
> +	.popsection
> +	.subsection 1
> +888 :
> +	\insn2
> +889 :
> +	.previous
> +	.org    . - (889b - 888b) + (887b - 886b)
> +	.org    . - (887b - 886b) + (889b - 888b)
> +	.endif
> +.endm
> +
> +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
> +	__ALTERNATIVE_CFG oldinsn, altinsn, vendor_id, errata_id, IS_ENABLED(CONFIG_k)
> +
> +#endif /* !__ASSEMBLY__ */
> +
> +#else /* !CONFIG_RISCV_ERRATA_ALTERNATIVE*/
> +#ifndef __ASSEMBLY__
> +
> +#define __ALTERNATIVE_CFG(oldinsn)  \
> +	oldinsn "\n"
> +
> +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
> +	__ALTERNATIVE_CFG(oldinsn)
> +
> +#else /* __ASSEMBLY__ */
> +
> +.macro __ALTERNATIVE_CFG insn1
> +	\insn1
> +.endm
> +
> +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
> +	__ALTERNATIVE_CFG oldinsn
> +
> +#endif /* !__ASSEMBLY__ */
> +#endif /* CONFIG_RISCV_ERRATA_ALTERNATIVE */
> +
> +/*
> + * Usage:
> + *	ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)
> + *  in the assembly code. Otherwise,
> + *	asm(ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k));
> + *
> + * oldinsn: The old instruction which will be replaced.
> + * altinsn: The replacement instruction.
> + * vendor_id: The CPU vendor ID.
> + * errata_id: The errata ID.
> + * CONFIG_k: The Kconfig of this errata. The instructions replacement can
> + *           be disabled by this Kconfig. When Kconfig is disabled, the
> + *           oldinsn will always be executed.
> + */
> +#define ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)   \
> +	_ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)
> +
> +#endif
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> new file mode 100644
> index 000000000000..98a0ea331a27
> --- /dev/null
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -0,0 +1,44 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 Sifive.
> + */
> +
> +#ifndef __ASM_ALTERNATIVE_H
> +#define __ASM_ALTERNATIVE_H
> +
> +#define ERRATA_STRING_LENGTH_MAX 32
> +
> +#include <asm/alternative-macros.h>
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/stddef.h>
> +#include <asm/hwcap.h>
> +
> +void __init apply_boot_alternatives(void);
> +
> +struct alt_entry {
> +	void *old_ptr;		 /* address of original instruciton or data  */
> +	void *alt_ptr;		 /* address of replacement instruction or data */
> +	unsigned long vendor_id; /* cpu vendor id */
> +	unsigned long alt_len;   /* The replacement size */
> +	unsigned int errata_id;  /* The errata id */
> +} __packed;
> +
> +struct errata_checkfunc_id {
> +	unsigned long vendor_id;
> +	bool (*func)(struct alt_entry *alt);
> +};
> +
> +extern struct cpu_manufacturer_info_t cpu_mfr_info;
> +
> +#define REGISTER_ERRATA_CHECKFUNC(checkfunc, vendorid)			  \
> +	static const struct errata_checkfunc_id _errata_check_##vendorid  \
> +	__used __section(".alt_checkfunc_table")			  \
> +	__aligned(__alignof__(struct errata_checkfunc_id)) =		  \
> +	{ .vendor_id = vendorid,					  \
> +	  .func = checkfunc }
> +#endif
> +#endif
> diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
> index 9c992a88d858..e20646aa97e5 100644
> --- a/arch/riscv/include/asm/asm.h
> +++ b/arch/riscv/include/asm/asm.h
> @@ -23,6 +23,7 @@
>  #define REG_L		__REG_SEL(ld, lw)
>  #define REG_S		__REG_SEL(sd, sw)
>  #define REG_SC		__REG_SEL(sc.d, sc.w)
> +#define REG_ASM 	__REG_SEL(.dword, .word)
>  #define SZREG		__REG_SEL(8, 4)
>  #define LGREG		__REG_SEL(3, 2)
>
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> new file mode 100644
> index 000000000000..5e5a1fcd90ba
> --- /dev/null
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -0,0 +1,8 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 Sifive.
> + */
> +
> +#ifdef CONFIG_SOC_SIFIVE
> +#define	ERRATA_NUMBER 0
> +#endif
> diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
> index 1595c5b60cfd..d13160f46d4e 100644
> --- a/arch/riscv/include/asm/sections.h
> +++ b/arch/riscv/include/asm/sections.h
> @@ -11,5 +11,7 @@ extern char _start[];
>  extern char _start_kernel[];
>  extern char __init_data_begin[], __init_data_end[];
>  extern char __init_text_begin[], __init_text_end[];
> +extern char __alt_checkfunc_table[], __alt_checkfunc_table_end[];
> +extern char __alt_start[], __alt_end[];
>
>  #endif /* __ASM_SECTIONS_H */
> diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
> new file mode 100644
> index 000000000000..1f3be47decb6
> --- /dev/null
> +++ b/arch/riscv/include/asm/vendorid_list.h
> @@ -0,0 +1,6 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2021 SiFive
> + */
> +
> +#define SIFIVE_VENDOR_ID	0x489
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 5e276c25646f..9a408e2942ac 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -32,6 +32,7 @@
>  #include <asm/sections.h>
>  #include <asm/sbi.h>
>  #include <asm/smp.h>
> +#include <asm/alternative.h>
>
>  #include "head.h"
>
> @@ -40,6 +41,9 @@ static DECLARE_COMPLETION(cpu_running);
>  void __init smp_prepare_boot_cpu(void)
>  {
>  	init_cpu_topology();
> +#ifdef CONFIG_RISCV_ERRATA_ALTERNATIVE
> +	apply_boot_alternatives();
> +#endif
>  }
>
>  void __init smp_prepare_cpus(unsigned int max_cpus)
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index de03cb22d0e9..6503dfca65b0 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -90,6 +90,20 @@ SECTIONS
>  	}
>
>  	__init_data_end = .;
> +
> +	. = ALIGN(8);
> +	.alt_checkfunc_table : {
> +		__alt_checkfunc_table = .;
> +		*(.alt_checkfunc_table)
> +		__alt_checkfunc_table_end = .;
> +	}
> +
> +	. = ALIGN(8);
> +	.alternative : {
> +		__alt_start = .;
> +		*(.alternative)
> +		__alt_end = .;
> +	}
>  	__init_end = .;
>
>  	/* Start of data section */

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 3/4] riscv: Introduce alternative mechanism to apply errata solution

[Vincent Chen]

On Wed, Mar 10, 2021 at 12:39 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Sun, 07 Mar 2021 19:58:16 PST (-0800), vincent.chen@sifive.com wrote:
> > Introduce the "alternative" mechanism from ARM64 and x86 to apply the CPU
> > vendors' errata solution at runtime. The main purpose of this patch is
> > to provide a framework. Therefore, the implementation is quite basic for
> > now so that some scenarios could not use this schemei, such as patching
> > code to a module, relocating the patching code and heterogeneous CPU
> > topology.
> >
> > Users could use the macro ALTERNATIVE to apply an errata to the existing
> > code flow. In the macro ALTERNATIVE, users need to specify the manufacturer
> > information(vendorid, archid, and impid) for this errata. Therefore, kernel
> > will know this errata is suitable for which CPU core. During the booting
> > procedure, kernel will select the errata required by the CPU core and then
> > patch it. It means that the kernel only applies the errata to the specified
> > CPU core. In this case the vendor's errata does not affect each other at
> > runtime. The above patching procedure only occurs during the booting phase,
> > so we only take the overhead of the "alternative" mechanism once.
> >
> > This "alternative" mechanism is enabled by default to ensure that all
> > required errata will be applied. However, users can disable this feature by
> > the Kconfig "CONFIG_RISCV_ERRATA_ALTERNATIVE".
> >
> > Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> > ---
> >  arch/riscv/Kconfig                          |   1 +
> >  arch/riscv/Kconfig.erratas                  |  12 +++
> >  arch/riscv/Makefile                         |   1 +
> >  arch/riscv/errata/Makefile                  |   1 +
> >  arch/riscv/errata/alternative.c             |  69 +++++++++++++++++
> >  arch/riscv/include/asm/alternative-macros.h | 110 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/alternative.h        |  44 +++++++++++
> >  arch/riscv/include/asm/asm.h                |   1 +
> >  arch/riscv/include/asm/errata_list.h        |   8 ++
> >  arch/riscv/include/asm/sections.h           |   2 +
> >  arch/riscv/include/asm/vendorid_list.h      |   6 ++
> >  arch/riscv/kernel/smpboot.c                 |   4 +
> >  arch/riscv/kernel/vmlinux.lds.S             |  14 ++++
> >  13 files changed, 273 insertions(+)
> >  create mode 100644 arch/riscv/Kconfig.erratas
> >  create mode 100644 arch/riscv/errata/Makefile
> >  create mode 100644 arch/riscv/errata/alternative.c
> >  create mode 100644 arch/riscv/include/asm/alternative-macros.h
> >  create mode 100644 arch/riscv/include/asm/alternative.h
> >  create mode 100644 arch/riscv/include/asm/errata_list.h
> >  create mode 100644 arch/riscv/include/asm/vendorid_list.h
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index a998babc1237..2e26251fe1f2 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -204,6 +204,7 @@ config LOCKDEP_SUPPORT
> >       def_bool y
> >
> >  source "arch/riscv/Kconfig.socs"
> > +source "arch/riscv/Kconfig.erratas"
> >
> >  menu "Platform type"
> >
> > diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
> > new file mode 100644
> > index 000000000000..4d0bafc536df
> > --- /dev/null
> > +++ b/arch/riscv/Kconfig.erratas
> > @@ -0,0 +1,12 @@
> > +menu "CPU errata selection"
> > +
> > +config RISCV_ERRATA_ALTERNATIVE
> > +     bool "RISC-V alternative scheme"
> > +     default y
> > +     help
> > +       This Kconfig allows the kernel to automatically patch the
> > +       errata required by the execution platform at run time. The
> > +       code patching is performed once in the boot stages. It means
> > +       that the overhead from this mechanism is just taken once.
> > +
> > +endmenu
> > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
> > index 1368d943f1f3..1f5c03082976 100644
> > --- a/arch/riscv/Makefile
> > +++ b/arch/riscv/Makefile
> > @@ -87,6 +87,7 @@ KBUILD_IMAGE        := $(boot)/Image.gz
> >  head-y := arch/riscv/kernel/head.o
> >
> >  core-y += arch/riscv/
> > +core-$(CONFIG_RISCV_ERRATA_ALTERNATIVE) += arch/riscv/errata/
> >
> >  libs-y += arch/riscv/lib/
> >  libs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a
> > diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
> > new file mode 100644
> > index 000000000000..43e6d5424367
> > --- /dev/null
> > +++ b/arch/riscv/errata/Makefile
> > @@ -0,0 +1 @@
> > +obj-y        += alternative.o
> > diff --git a/arch/riscv/errata/alternative.c b/arch/riscv/errata/alternative.c
> > new file mode 100644
> > index 000000000000..052affdce6eb
> > --- /dev/null
> > +++ b/arch/riscv/errata/alternative.c
> > @@ -0,0 +1,69 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * alternative runtime patching
> > + * inspired by the ARM64 and x86 version
> > + *
> > + * Copyright (C) 2021 Sifive.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/cpu.h>
> > +#include <linux/uaccess.h>
> > +#include <asm/patch.h>
> > +#include <asm/alternative.h>
> > +#include <asm/sections.h>
> > +
> > +struct alt_region {
> > +     struct alt_entry *begin;
> > +     struct alt_entry *end;
> > +};
> > +
> > +static bool (*errata_checkfunc)(struct alt_entry *alt);
> > +typedef int (*patch_func_t)(void *addr, const void *insn, size_t size);
> > +
> > +static void __apply_alternatives(void *alt_region, void *alt_patch_func)
> > +{
> > +     struct alt_entry *alt;
> > +     struct alt_region *region = alt_region;
> > +
> > +     for (alt = region->begin; alt < region->end; alt++) {
> > +             if (!errata_checkfunc(alt))
> > +                     continue;
> > +             ((patch_func_t)alt_patch_func)(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > +     }
> > +}
> > +
> > +static void __init init_alternative(void)
> > +{
> > +     struct errata_checkfunc_id *ptr;
> > +
> > +     for (ptr = (struct errata_checkfunc_id *)__alt_checkfunc_table;
> > +          ptr < (struct errata_checkfunc_id *)__alt_checkfunc_table_end;
> > +          ptr++) {
> > +             if (cpu_mfr_info.vendor_id == ptr->vendor_id)
>
> Of those three new SBI calls this is the only one I see used.  You could get
> rid of the whole global caching thing and just call it once here, which would
> be a bit simpler.
>
OK. I will modify it in my next version patch.  Thanks.

> I'm also not convinced that it's worth all this table stuff here.  It's not
> like we have new vendors every day, we could just have a switch statement on
> sbi_get_mvendorid() and stick every vendor's errata probe in there.
>
OK.
> At that point this would be simple enough that I'd be inclined to
> unconditionally (at least on S-mode kernels, the M-mode kernels aren't portable
> so who cares) include the errata checking.  I can understand not wanting to
> take all the errata fixing, as some of those could get complex, but if we have
> the check in there we can provide a warning to users who don't have the errata
> fix.
>
> If that gets out of hand or we have non-portable kernels we can always hoist
> the ifdefs to avoid the check functions as well, but I'd prefer that portable
> kernels are at least capable of detecting they've been built in a fashion that
> will subtly break things.
>
I agreed with it. I will add it in my next version patch. Thanks


> > +                     errata_checkfunc = ptr->func;
> > +     }
> > +}
> > +
> > +/*
> > + * This is called very early in the boot process (directly after we run
> > + * a feature detect on the boot CPU). No need to worry about other CPUs
> > + * here.
> > + */
> > +void __init apply_boot_alternatives(void)
> > +{
> > +     struct alt_region region;
> > +
> > +     /* If called on non-boot cpu things could go wrong */
> > +     WARN_ON(smp_processor_id() != 0);
> > +
> > +     init_alternative();
> > +
> > +     if (!errata_checkfunc)
> > +             return;
> > +
> > +     region.begin = (struct alt_entry *)__alt_start;
> > +     region.end = (struct alt_entry *)__alt_end;
> > +     __apply_alternatives(&region, patch_text_nosync);
> > +}
> > +
> > diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> > new file mode 100644
> > index 000000000000..7b6f0c94b1fb
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/alternative-macros.h
> > @@ -0,0 +1,110 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef __ASM_ALTERNATIVE_MACROS_H
> > +#define __ASM_ALTERNATIVE_MACROS_H
> > +
> > +#ifdef CONFIG_RISCV_ERRATA_ALTERNATIVE
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <asm/asm.h>
> > +#include <linux/stringify.h>
> > +
> > +#define ALT_ENTRY(oldptr, altptr, vendor_id, errata_id, altlen) \
> > +     RISCV_PTR " " oldptr "\n" \
> > +     RISCV_PTR " " altptr "\n" \
> > +     REG_ASM " " vendor_id "\n" \
> > +     REG_ASM " " altlen "\n" \
> > +     ".word " errata_id "\n"
> > +
> > +#define __ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, enable) \
> > +     "886 :\n"                                                       \
> > +     oldinsn "\n"                                                    \
> > +     ".if " __stringify(enable) " == 1\n"                            \
> > +     "887 :\n"                                                       \
> > +     ".pushsection .alternative, \"a\"\n"                            \
> > +     ALT_ENTRY("886b", "888f", __stringify(vendor_id), __stringify(errata_id), "889f - 888f") \
> > +     ".popsection\n"                                                 \
> > +     ".subsection 1\n"                                               \
> > +     "888 :\n"                                                       \
> > +     altinsn "\n"                                                    \
> > +     "889 :\n"                                                       \
> > +     ".previous\n"                                                   \
> > +     ".org   . - (887b - 886b) + (889b - 888b)\n"                    \
> > +     ".org   . - (889b - 888b) + (887b - 886b)\n"                    \
> > +     ".endif\n"
> > +
> > +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)   \
> > +     __ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, IS_ENABLED(CONFIG_k))
> > +
> > +#else /* __ASSEMBLY__ */
> > +
> > +.macro ALT_ENTRY oldptr altptr vendor_id errata_id alt_len
> > +     RISCV_PTR \oldptr
> > +     RISCV_PTR \altptr
> > +     REG_ASM \vendor_id
> > +     REG_ASM \alt_len
> > +     .word   \errata_id
> > +.endm
> > +
> > +.macro __ALTERNATIVE_CFG insn1 insn2 vendor_id errata_id enable = 1
> > +886 :
> > +     \insn1
> > +     .if \enable
> > +887 :
> > +     .pushsection .alternative, "a"
> > +     ALT_ENTRY 886b, 888f, \vendor_id, \errata_id, 889f - 888f
> > +     .popsection
> > +     .subsection 1
> > +888 :
> > +     \insn2
> > +889 :
> > +     .previous
> > +     .org    . - (889b - 888b) + (887b - 886b)
> > +     .org    . - (887b - 886b) + (889b - 888b)
> > +     .endif
> > +.endm
> > +
> > +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
> > +     __ALTERNATIVE_CFG oldinsn, altinsn, vendor_id, errata_id, IS_ENABLED(CONFIG_k)
> > +
> > +#endif /* !__ASSEMBLY__ */
> > +
> > +#else /* !CONFIG_RISCV_ERRATA_ALTERNATIVE*/
> > +#ifndef __ASSEMBLY__
> > +
> > +#define __ALTERNATIVE_CFG(oldinsn)  \
> > +     oldinsn "\n"
> > +
> > +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
> > +     __ALTERNATIVE_CFG(oldinsn)
> > +
> > +#else /* __ASSEMBLY__ */
> > +
> > +.macro __ALTERNATIVE_CFG insn1
> > +     \insn1
> > +.endm
> > +
> > +#define _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k) \
> > +     __ALTERNATIVE_CFG oldinsn
> > +
> > +#endif /* !__ASSEMBLY__ */
> > +#endif /* CONFIG_RISCV_ERRATA_ALTERNATIVE */
> > +
> > +/*
> > + * Usage:
> > + *   ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)
> > + *  in the assembly code. Otherwise,
> > + *   asm(ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k));
> > + *
> > + * oldinsn: The old instruction which will be replaced.
> > + * altinsn: The replacement instruction.
> > + * vendor_id: The CPU vendor ID.
> > + * errata_id: The errata ID.
> > + * CONFIG_k: The Kconfig of this errata. The instructions replacement can
> > + *           be disabled by this Kconfig. When Kconfig is disabled, the
> > + *           oldinsn will always be executed.
> > + */
> > +#define ALTERNATIVE(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)   \
> > +     _ALTERNATIVE_CFG(oldinsn, altinsn, vendor_id, errata_id, CONFIG_k)
> > +
> > +#endif
> > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > new file mode 100644
> > index 000000000000..98a0ea331a27
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/alternative.h
> > @@ -0,0 +1,44 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2021 Sifive.
> > + */
> > +
> > +#ifndef __ASM_ALTERNATIVE_H
> > +#define __ASM_ALTERNATIVE_H
> > +
> > +#define ERRATA_STRING_LENGTH_MAX 32
> > +
> > +#include <asm/alternative-macros.h>
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <linux/init.h>
> > +#include <linux/types.h>
> > +#include <linux/stddef.h>
> > +#include <asm/hwcap.h>
> > +
> > +void __init apply_boot_alternatives(void);
> > +
> > +struct alt_entry {
> > +     void *old_ptr;           /* address of original instruciton or data  */
> > +     void *alt_ptr;           /* address of replacement instruction or data */
> > +     unsigned long vendor_id; /* cpu vendor id */
> > +     unsigned long alt_len;   /* The replacement size */
> > +     unsigned int errata_id;  /* The errata id */
> > +} __packed;
> > +
> > +struct errata_checkfunc_id {
> > +     unsigned long vendor_id;
> > +     bool (*func)(struct alt_entry *alt);
> > +};
> > +
> > +extern struct cpu_manufacturer_info_t cpu_mfr_info;
> > +
> > +#define REGISTER_ERRATA_CHECKFUNC(checkfunc, vendorid)                         \
> > +     static const struct errata_checkfunc_id _errata_check_##vendorid  \
> > +     __used __section(".alt_checkfunc_table")                          \
> > +     __aligned(__alignof__(struct errata_checkfunc_id)) =              \
> > +     { .vendor_id = vendorid,                                          \
> > +       .func = checkfunc }
> > +#endif
> > +#endif
> > diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
> > index 9c992a88d858..e20646aa97e5 100644
> > --- a/arch/riscv/include/asm/asm.h
> > +++ b/arch/riscv/include/asm/asm.h
> > @@ -23,6 +23,7 @@
> >  #define REG_L                __REG_SEL(ld, lw)
> >  #define REG_S                __REG_SEL(sd, sw)
> >  #define REG_SC               __REG_SEL(sc.d, sc.w)
> > +#define REG_ASM      __REG_SEL(.dword, .word)
> >  #define SZREG                __REG_SEL(8, 4)
> >  #define LGREG                __REG_SEL(3, 2)
> >
> > diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> > new file mode 100644
> > index 000000000000..5e5a1fcd90ba
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/errata_list.h
> > @@ -0,0 +1,8 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2021 Sifive.
> > + */
> > +
> > +#ifdef CONFIG_SOC_SIFIVE
> > +#define      ERRATA_NUMBER 0
> > +#endif
> > diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
> > index 1595c5b60cfd..d13160f46d4e 100644
> > --- a/arch/riscv/include/asm/sections.h
> > +++ b/arch/riscv/include/asm/sections.h
> > @@ -11,5 +11,7 @@ extern char _start[];
> >  extern char _start_kernel[];
> >  extern char __init_data_begin[], __init_data_end[];
> >  extern char __init_text_begin[], __init_text_end[];
> > +extern char __alt_checkfunc_table[], __alt_checkfunc_table_end[];
> > +extern char __alt_start[], __alt_end[];
> >
> >  #endif /* __ASM_SECTIONS_H */
> > diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
> > new file mode 100644
> > index 000000000000..1f3be47decb6
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/vendorid_list.h
> > @@ -0,0 +1,6 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2021 SiFive
> > + */
> > +
> > +#define SIFIVE_VENDOR_ID     0x489
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index 5e276c25646f..9a408e2942ac 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -32,6 +32,7 @@
> >  #include <asm/sections.h>
> >  #include <asm/sbi.h>
> >  #include <asm/smp.h>
> > +#include <asm/alternative.h>
> >
> >  #include "head.h"
> >
> > @@ -40,6 +41,9 @@ static DECLARE_COMPLETION(cpu_running);
> >  void __init smp_prepare_boot_cpu(void)
> >  {
> >       init_cpu_topology();
> > +#ifdef CONFIG_RISCV_ERRATA_ALTERNATIVE
> > +     apply_boot_alternatives();
> > +#endif
> >  }
> >
> >  void __init smp_prepare_cpus(unsigned int max_cpus)
> > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > index de03cb22d0e9..6503dfca65b0 100644
> > --- a/arch/riscv/kernel/vmlinux.lds.S
> > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > @@ -90,6 +90,20 @@ SECTIONS
> >       }
> >
> >       __init_data_end = .;
> > +
> > +     . = ALIGN(8);
> > +     .alt_checkfunc_table : {
> > +             __alt_checkfunc_table = .;
> > +             *(.alt_checkfunc_table)
> > +             __alt_checkfunc_table_end = .;
> > +     }
> > +
> > +     . = ALIGN(8);
> > +     .alternative : {
> > +             __alt_start = .;
> > +             *(.alternative)
> > +             __alt_end = .;
> > +     }
> >       __init_end = .;
> >
> >       /* Start of data section */

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC patch 4/4] riscv: sifive: apply errata "cip-453" patch

[Vincent Chen]

Add sign extension to the $badaddr before addressing the instruction page
fault and instruction access fault to workaround the issue "cip-453".

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@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
+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
+		},
+	},
+};
+
+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)
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)
 	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 */
-- 
2.7.4


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 4/4] riscv: sifive: apply errata "cip-453" patch

[Palmer Dabbelt]

On Sun, 07 Mar 2021 19:58:17 PST (-0800), vincent.chen@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@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.

> +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?

> +		},
> +	},
> +};
> +
> +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.

> 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.

>  	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 */

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 4/4] riscv: sifive: apply errata "cip-453" patch

[Vincent Chen]

On Wed, Mar 10, 2021 at 12:39 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Sun, 07 Mar 2021 19:58:17 PST (-0800), vincent.chen@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@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 */

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[Palmer Dabbelt]

On Sun, 07 Mar 2021 19:58:13 PST (-0800), vincent.chen@sifive.com wrote:
> With the emergence of more and more RISC-V CPUs, the request for how to
> upstream the vendor errata patch may gradually appear. In order to resolve
> this issue, this patch introduces the alternative mechanism from ARM64 and
> x86 to enable the kernel to patch code at runtime according to the
> manufacturer information of the running CPU. The main purpose of this patch
> set is to propose a framework to apply vendor's errata solutions. Based on
> this framework, it can be ensured that the errata only applies to the
> specified CPU cores. Other CPU cores do not be affected. Therefore, some
> complicated scenarios are unsupported in this patch set, such as patching
> code to the kernel module, doing relocation in patching code, and
> heterogeneous CPU topology.
>
> In the "alternative" scheme, Users could use the macro ALTERNATIVE to apply
> an errata to the existing code flow. In the macro ALTERNATIVE, users need
> to specify the manufacturer information (vendor id, arch id, and implement
> id) for this errata. Therefore, kernel will know this errata is suitable
> for which CPU core. During the booting procedure, kernel will select the
> errata required by the CPU core and then patch it. It means that the kernel
> only applies the errata to the specified CPU core. In this case, the
> vendor's errata does not affect each other at runtime. The above patching
> procedure only occurs during the booting phase, so we only take the
> overhead of the "alternative" mechanism once.
>
> This "alternative" mechanism is enabled by default to ensure that all
> required errata will be applied. However, users can disable this feature by
> the Kconfig "CONFIG_RISCV_ERRATA_ALTERNATIVE".

This all generally seems OK to me, though I have a few comments in the actual 
patches.

>
> The last patch is to apply the SiFive CIP-453 errata by this "alternative"
> scheme. Therefore, It can be regarded as an example. According to the
> results of running this image on the QEMU virt platform, kernel does not
> apply this errata at run-time because the CPU manufacturer information
> does not match the specified SiFive CPU core. Therefore, this errata does
> not affect any CPU core except for the specified SiFive cores.

Looking at the documentation for that I also saw CIP-1200.  That one seems way 
scarier, and also probably a better driver for building an errata framework as 
it has more than one caller.  Is that in this chip?  It's in a document just 
titled "Errata_FU740-C000_20210205".

>
> Vincent Chen (4):
>   riscv: Add 3 SBI wrapper functions to get cpu manufacturer information
>   riscv: Get CPU manufacturer information
>   riscv: Introduce alternative mechanism to apply errata solution
>   riscv: sifive: apply errata "cip-453" patch
>
>  arch/riscv/Kconfig                          |   1 +
>  arch/riscv/Kconfig.erratas                  |  32 ++++++++
>  arch/riscv/Kconfig.socs                     |   1 +
>  arch/riscv/Makefile                         |   1 +
>  arch/riscv/errata/Makefile                  |   2 +
>  arch/riscv/errata/alternative.c             |  69 +++++++++++++++++
>  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/alternative-macros.h | 110 ++++++++++++++++++++++++++++
>  arch/riscv/include/asm/alternative.h        |  44 +++++++++++
>  arch/riscv/include/asm/asm.h                |   1 +
>  arch/riscv/include/asm/csr.h                |   3 +
>  arch/riscv/include/asm/errata_list.h        |   9 +++
>  arch/riscv/include/asm/hwcap.h              |   6 ++
>  arch/riscv/include/asm/processor.h          |   2 +
>  arch/riscv/include/asm/sbi.h                |   3 +
>  arch/riscv/include/asm/sections.h           |   2 +
>  arch/riscv/include/asm/soc.h                |   1 +
>  arch/riscv/include/asm/vendorid_list.h      |   6 ++
>  arch/riscv/kernel/cpufeature.c              |  17 +++++
>  arch/riscv/kernel/entry.S                   |  12 ++-
>  arch/riscv/kernel/sbi.c                     |  15 ++++
>  arch/riscv/kernel/setup.c                   |   2 +
>  arch/riscv/kernel/smpboot.c                 |   4 +
>  arch/riscv/kernel/soc.c                     |   1 +
>  arch/riscv/kernel/vmlinux.lds.S             |  14 ++++
>  27 files changed, 448 insertions(+), 2 deletions(-)
>  create mode 100644 arch/riscv/Kconfig.erratas
>  create mode 100644 arch/riscv/errata/Makefile
>  create mode 100644 arch/riscv/errata/alternative.c
>  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
>  create mode 100644 arch/riscv/include/asm/alternative-macros.h
>  create mode 100644 arch/riscv/include/asm/alternative.h
>  create mode 100644 arch/riscv/include/asm/errata_list.h
>  create mode 100644 arch/riscv/include/asm/vendorid_list.h

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[Vincent Chen]

On Wed, Mar 10, 2021 at 12:39 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Sun, 07 Mar 2021 19:58:13 PST (-0800), vincent.chen@sifive.com wrote:
> > With the emergence of more and more RISC-V CPUs, the request for how to
> > upstream the vendor errata patch may gradually appear. In order to resolve
> > this issue, this patch introduces the alternative mechanism from ARM64 and
> > x86 to enable the kernel to patch code at runtime according to the
> > manufacturer information of the running CPU. The main purpose of this patch
> > set is to propose a framework to apply vendor's errata solutions. Based on
> > this framework, it can be ensured that the errata only applies to the
> > specified CPU cores. Other CPU cores do not be affected. Therefore, some
> > complicated scenarios are unsupported in this patch set, such as patching
> > code to the kernel module, doing relocation in patching code, and
> > heterogeneous CPU topology.
> >
> > In the "alternative" scheme, Users could use the macro ALTERNATIVE to apply
> > an errata to the existing code flow. In the macro ALTERNATIVE, users need
> > to specify the manufacturer information (vendor id, arch id, and implement
> > id) for this errata. Therefore, kernel will know this errata is suitable
> > for which CPU core. During the booting procedure, kernel will select the
> > errata required by the CPU core and then patch it. It means that the kernel
> > only applies the errata to the specified CPU core. In this case, the
> > vendor's errata does not affect each other at runtime. The above patching
> > procedure only occurs during the booting phase, so we only take the
> > overhead of the "alternative" mechanism once.
> >
> > This "alternative" mechanism is enabled by default to ensure that all
> > required errata will be applied. However, users can disable this feature by
> > the Kconfig "CONFIG_RISCV_ERRATA_ALTERNATIVE".
>
> This all generally seems OK to me, though I have a few comments in the actual
> patches.
>
> >
> > The last patch is to apply the SiFive CIP-453 errata by this "alternative"
> > scheme. Therefore, It can be regarded as an example. According to the
> > results of running this image on the QEMU virt platform, kernel does not
> > apply this errata at run-time because the CPU manufacturer information
> > does not match the specified SiFive CPU core. Therefore, this errata does
> > not affect any CPU core except for the specified SiFive cores.
>
> Looking at the documentation for that I also saw CIP-1200.  That one seems way
> scarier, and also probably a better driver for building an errata framework as
> it has more than one caller.  Is that in this chip?  It's in a document just
> titled "Errata_FU740-C000_20210205".
>
This chip has the CIP-1200 issue, and we had a patch using this
alternative scheme to apply this errata. The code is like below:
 static inline void local_flush_tlb_page(unsigned long addr)
 {
-        __asm__ __volatile__ ("sfence.vma %0" : : "r" (addr) : "memory");
+       __asm__ __volatile__(
+       ALTERNATIVE("sfence.vma %0", "sfence.vma",SIFIVE_VENDOR_ID,
ERRATA_CIP_1200, CONFIG_ERRATA_SIFIVE_CIP_1200)
+       :
+       : "r" (addr)
+       : "memory");
 }
We originally plan to send this errata patch after each vendor accepts
this alternative framework.

> >
> > Vincent Chen (4):
> >   riscv: Add 3 SBI wrapper functions to get cpu manufacturer information
> >   riscv: Get CPU manufacturer information
> >   riscv: Introduce alternative mechanism to apply errata solution
> >   riscv: sifive: apply errata "cip-453" patch
> >
> >  arch/riscv/Kconfig                          |   1 +
> >  arch/riscv/Kconfig.erratas                  |  32 ++++++++
> >  arch/riscv/Kconfig.socs                     |   1 +
> >  arch/riscv/Makefile                         |   1 +
> >  arch/riscv/errata/Makefile                  |   2 +
> >  arch/riscv/errata/alternative.c             |  69 +++++++++++++++++
> >  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/alternative-macros.h | 110 ++++++++++++++++++++++++++++
> >  arch/riscv/include/asm/alternative.h        |  44 +++++++++++
> >  arch/riscv/include/asm/asm.h                |   1 +
> >  arch/riscv/include/asm/csr.h                |   3 +
> >  arch/riscv/include/asm/errata_list.h        |   9 +++
> >  arch/riscv/include/asm/hwcap.h              |   6 ++
> >  arch/riscv/include/asm/processor.h          |   2 +
> >  arch/riscv/include/asm/sbi.h                |   3 +
> >  arch/riscv/include/asm/sections.h           |   2 +
> >  arch/riscv/include/asm/soc.h                |   1 +
> >  arch/riscv/include/asm/vendorid_list.h      |   6 ++
> >  arch/riscv/kernel/cpufeature.c              |  17 +++++
> >  arch/riscv/kernel/entry.S                   |  12 ++-
> >  arch/riscv/kernel/sbi.c                     |  15 ++++
> >  arch/riscv/kernel/setup.c                   |   2 +
> >  arch/riscv/kernel/smpboot.c                 |   4 +
> >  arch/riscv/kernel/soc.c                     |   1 +
> >  arch/riscv/kernel/vmlinux.lds.S             |  14 ++++
> >  27 files changed, 448 insertions(+), 2 deletions(-)
> >  create mode 100644 arch/riscv/Kconfig.erratas
> >  create mode 100644 arch/riscv/errata/Makefile
> >  create mode 100644 arch/riscv/errata/alternative.c
> >  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
> >  create mode 100644 arch/riscv/include/asm/alternative-macros.h
> >  create mode 100644 arch/riscv/include/asm/alternative.h
> >  create mode 100644 arch/riscv/include/asm/errata_list.h
> >  create mode 100644 arch/riscv/include/asm/vendorid_list.h

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[Ruinland ChuanTzu Tsai]

> With the emergence of more and more RISC-V CPUs, the request for how to
> upstream the vendor errata patch may gradually appear. In order to resolve
> this issue, this patch introduces the alternative mechanism from ARM64 and
> x86 to enable the kernel to patch code at runtime according to the
> manufacturer information of the running CPU. The main purpose of this patch
> set is to propose a framework to apply vendor's errata solutions. Based on
> this framework, it can be ensured that the errata only applies to the
> specified CPU cores. Other CPU cores do not be affected. Therefore, some
> complicated scenarios are unsupported in this patch set, such as patching
> code to the kernel module, doing relocation in patching code, and
> heterogeneous CPU topology.
> 
> In the "alternative" scheme, Users could use the macro ALTERNATIVE to apply
> an errata to the existing code flow. In the macro ALTERNATIVE, users need

In general I love the idea of fixing errata dynamically, yet in the past,
vendors sometimes bump into the dilemma on the necessity of using alternative
framework or just plainly "ifdefine"-ing the CONFIG_ERRATUM_XXX to toggle their
fixups.

Quoting from the Qualcomm Falkor E1041 dicssusion threads [1][2] :

"Just do it with an #ifdef"

"There's really no need for this to be an alternative. It makes the kernel
larger and more complex due to all the altinstr data and probing code."

I wonder if there will be similar disputes end up here. And having a conclusion
on what should be resolved by alternative and what shouldn't might be more
complicated since we're having a more diversed group of vendors (or "users" who
can access the errata as you mentioned).

Cordially yours,
Ruinland

[1] https://lore.kernel.org/kvmarm/20171201112457.GE18083@arm.com/
[2] https://patchwork.kernel.org/project/linux-arm-kernel/patch/1512957823-18064-2-git-send-email-shankerd@codeaurora.org/#21249629

> to specify the manufacturer information (vendor id, arch id, and implement
> id) for this errata. Therefore, kernel will know this errata is suitable
> for which CPU core. During the booting procedure, kernel will select the
> errata required by the CPU core and then patch it. It means that the kernel
> only applies the errata to the specified CPU core. In this case, the
> vendor's errata does not affect each other at runtime. The above patching
> procedure only occurs during the booting phase, so we only take the
> overhead of the "alternative" mechanism once.
> 
> This "alternative" mechanism is enabled by default to ensure that all
> required errata will be applied. However, users can disable this feature by
> the Kconfig "CONFIG_RISCV_ERRATA_ALTERNATIVE".
> 
> The last patch is to apply the SiFive CIP-453 errata by this "alternative"
> scheme. Therefore, It can be regarded as an example. According to the
> results of running this image on the QEMU virt platform, kernel does not
> apply this errata at run-time because the CPU manufacturer information
> does not match the specified SiFive CPU core. Therefore, this errata does
> not affect any CPU core except for the specified SiFive cores.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[Vincent Chen]

On Thu, Mar 11, 2021 at 11:51 AM Ruinland ChuanTzu Tsai
<ruinland@andestech.com> wrote:
>
>
> > With the emergence of more and more RISC-V CPUs, the request for how to
> > upstream the vendor errata patch may gradually appear. In order to resolve
> > this issue, this patch introduces the alternative mechanism from ARM64 and
> > x86 to enable the kernel to patch code at runtime according to the
> > manufacturer information of the running CPU. The main purpose of this patch
> > set is to propose a framework to apply vendor's errata solutions. Based on
> > this framework, it can be ensured that the errata only applies to the
> > specified CPU cores. Other CPU cores do not be affected. Therefore, some
> > complicated scenarios are unsupported in this patch set, such as patching
> > code to the kernel module, doing relocation in patching code, and
> > heterogeneous CPU topology.
> >
> > In the "alternative" scheme, Users could use the macro ALTERNATIVE to apply
> > an errata to the existing code flow. In the macro ALTERNATIVE, users need
>
> In general I love the idea of fixing errata dynamically, yet in the past,
> vendors sometimes bump into the dilemma on the necessity of using alternative
> framework or just plainly "ifdefine"-ing the CONFIG_ERRATUM_XXX to toggle their
> fixups.
>
> Quoting from the Qualcomm Falkor E1041 dicssusion threads [1][2] :
>
> "Just do it with an #ifdef"
>
> "There's really no need for this to be an alternative. It makes the kernel
> larger and more complex due to all the altinstr data and probing code."
>
> I wonder if there will be similar disputes end up here. And having a conclusion
> on what should be resolved by alternative and what shouldn't might be more
> complicated since we're having a more diversed group of vendors (or "users" who
> can access the errata as you mentioned).
>
> Cordially yours,
> Ruinland
>
> [1] https://lore.kernel.org/kvmarm/20171201112457.GE18083@arm.com/
> [2] https://patchwork.kernel.org/project/linux-arm-kernel/patch/1512957823-18064-2-git-send-email-shankerd@codeaurora.org/#21249629
>

To be honest, We had a similar discussion in our internal group, and
someone voted for the ifdef method. However, in the end, I chose the
alternative scheme because it will not have any performance impact on
other vendor's core.

For the "#ifdef" mechanism, if we want one kernel image that can
include all vendor's errata, all CONFIG_ERRATA_XXX should be able to
enable at the same time. In this case, the code patching may become
the following pattern.

if (IS_ENABLED(CONFIG_ERRATA_XXX ) && unlikely(has_errata_xxx))
{
      vendor's errata ......
}
The has_errata_xxx is a global variable and its default value is 0. At
runtime, the customized function provided by the vendor should
properly set has_errata_xxx as 1 for the specified CPU cores. Because
the default value of has_errata_xxx for certain cores is 0, It is OK
for all vendor's to enable the CONFIG_ERRATA_XXX. However, this
implementation will cause other vendors to suffer some performance
impact. I used sifive errata cip 1200 as an example:

+        if (IS_ENABLED(CONFIG_ERRATA_SIFIVE_CIP_1200) &&
unlikely(has_errata_sifive_cip_1200)) {
+                asm("sfence.vma\n");
+        } else
+               local_flush_tlb_page(addr);

In this case, except for the particular sifive cores, all riscv cores
will execute local_flush_tlb_page(addr) instead of sfence.vma. The
corresponding assembly code is listed below.

        if (IS_ENABLED(CONFIG_ERRATA_SIFIVE_CIP_1200) &&
unlikely(has_errata_sifive_cip_1200)) {
ffffffe000007416:       012dd797                auipc   a5,0x12dd
ffffffe00000741a:       cfa7c783                lbu     a5,-774(a5) #
ffffffe0012e4110 <has_errata_sifive_cip_1200>
ffffffe00000741e:       e7e9                    bnez
a5,ffffffe0000074e8 <do_page_fault+0x33c>
local_flush_tlb_page():
/home/vincentchen/work/64bit/linux/./arch/riscv/include/asm/tlbflush.h:22
ffffffe000007420:       12098073                sfence.vma      s3

According to the objdump results, before executing
local_flush_tlb_page(), other vendors' cores need to execute three
additional instructions (from 0xffffffe000007416 to
0xffffffe00000741e) due to this errata. The performance impact from
these 3 instructions may be slight. ( 1. The branch predictor may
always do the correct prediction at 0xffffffe00000741e. 2. If this
errata is inserted into a hot spot path, has_errata_sifive_cip_1200
may be stored in the L2 cache.) However, I am still not sure whether
each vendor is willing to suffer the performance impact. Besides, I am
also worried that if we have a lot of errata, the impact on
performance will become significant. Therefore, I finally chose the
alternative scheme.

However, if all vendors are willing to suffer the performance impact
from other vendor's errata, I can change the framework to "#ifdef"
scheme. After all, it may need to take a lot of time to accumulate a
lot of errata to make the performance impact obvious.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[Ruinland ChuanTzu Tsai]

Hi Vincent,

> However, if all vendors are willing to suffer the performance impact
> from other vendor's errata, I can change the framework to "#ifdef"
> scheme. After all, it may need to take a lot of time to accumulate a
> lot of errata to make the performance impact obvious.
I'm not sure if I'm interpreting the alternative framework right, yet I have
doubts about the performance arguments.
Just as the example (CONFIG_QCOM_FALKOR_ERRATUM_E1041) I quoted in the 
previous
mail, in their final decision, the `#ifdef` simply toggles the code in
_compile time_ [1].

I don't know why they chose to enable it by default [2], yet ideally, for
irrelevant processors/SoCs, people should be able to turn the option off
without issues.

Somehow, to my best acknowledgment and as you mentioned, in ARM's ecosystem
(more specifically, the distros), alternative-based errata fixes are all 
enabl-
ed by default, so all the errata workarounds will be generated and gathered
into a special section. This will cause the size of Linux image which 
distros
shipped to become larger and larger.

For ARM, that might not be a problem since most of the IC vendors 
nowadays are
licensing from ARM and few of them are making their microarchitectures.
So mi errata es su errata. (My errata is your errata.)

Yet for RISC-V, we have plenty of players who make their home-grown cores on
the field.
This leads to a dilemma: either to enable all the workarounds by default 
so the
distros could just build one general image which will become bloated or
specially crafted images will be provided lead to a fractured experience for
end-users.

My 2 cents on this issue is that maybe we need to establish a principle
(probably not in a written-down manner) on deciding what goes to alternative
and what shall just be an option disabled by default.

Cordially yours,
Ruinland


[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/include/asm/assembler.h#n610

[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/Kconfig#n805


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC patch 0/4] riscv: introduce alternative mechanism to apply errata patches

[Vincent Chen]

On Mon, Mar 15, 2021 at 12:46 PM Ruinland ChuanTzu Tsai
<ruinland@andestech.com> wrote:
>
> Hi Vincent,
>
> > However, if all vendors are willing to suffer the performance impact
> > from other vendor's errata, I can change the framework to "#ifdef"
> > scheme. After all, it may need to take a lot of time to accumulate a
> > lot of errata to make the performance impact obvious.
> I'm not sure if I'm interpreting the alternative framework right, yet I have
> doubts about the performance arguments.
> Just as the example (CONFIG_QCOM_FALKOR_ERRATUM_E1041) I quoted in the
> previous
> mail, in their final decision, the `#ifdef` simply toggles the code in
> _compile time_ [1].
>
> I don't know why they chose to enable it by default [2], yet ideally, for
> irrelevant processors/SoCs, people should be able to turn the option off
> without issues.
>
> Somehow, to my best acknowledgment and as you mentioned, in ARM's ecosystem
> (more specifically, the distros), alternative-based errata fixes are all
> enabl-
> ed by default, so all the errata workarounds will be generated and gathered
> into a special section. This will cause the size of Linux image which
> distros
> shipped to become larger and larger.
>
> For ARM, that might not be a problem since most of the IC vendors
> nowadays are
> licensing from ARM and few of them are making their microarchitectures.
> So mi errata es su errata. (My errata is your errata.)
>
> Yet for RISC-V, we have plenty of players who make their home-grown cores on
> the field.
> This leads to a dilemma: either to enable all the workarounds by default
> so the
> distros could just build one general image which will become bloated or
> specially crafted images will be provided lead to a fractured experience for
> end-users.
>
> My 2 cents on this issue is that maybe we need to establish a principle
> (probably not in a written-down manner) on deciding what goes to alternative
> and what shall just be an option disabled by default.
>
> Cordially yours,
> Ruinland
>

Hi Ruinland,
I can understand and agree with what you worry about the image size.
However, I have a different idea to resolve this issue.

I prefer the RISC-V default configuration to enable all errata
KCONFIG. It can ensure that the default image can run on all vendors'
platforms without any known hardware issue. For general users, it is
more friendly because they do not need to worry whether missing the
required errata or not. For advanced users such as distros, given the
targeted platforms, they can use Kconfig to disable unneeded errata to
shrink the kernel image. Therefore, they do not ship a bloated kernel
image to their customers.

Back to the current alternative framework, it can support the above
case. Users can only enable particular CONFIG_ERRATA_XXX to generate a
customized image for a specified platform. Because the kernel patches
errata at runtime, it also allows users to enable multiple
CONFIG_ERRATA_XXX at the same time to generate a uniform image run on
multiple RISC-V platforms. Therefore, it reserves the flexibility for
distros or vendors to customize the kernel image.

In contrast to the alternative framework, if we only use "ifdef" to
patch errata at compile-time, one image can run on specified
platforms. In this case, not only distros but also vendors will have
kernel image fragmentation issues. It may increase the maintenance
effort.

Based on the above reason, I do not prefer to use "ifdef" to patch
errata at compile time.

Best regards,
Vincent

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv