2022-05-13 13:57:25

by Heiko Stuebner

[permalink] [raw]
Subject: [PATCH v10 00/12] riscv: support for Svpbmt and D1 memory types

Svpbmt is an extension defining "Supervisor-mode: page-based memory types"
for things like non-cacheable pages or I/O memory pages.


So this is my 2nd try at implementing Svpbmt (and the diverging D1 memory
types) using the alternatives framework.

This includes a number of changes to the alternatives mechanism itself.
The biggest one being the move to a more central location, as I expect
in the future, nearly every chip needing some sort of patching, be it
either for erratas or for optional features (svpbmt or others).

Detection of the svpbmt functionality is done via Atish's isa extension
handling series [0] and thus does not need any dt-parsing of its own
anymore.

The series also introduces support for the memory types of the D1
which are implemented differently to svpbmt. But when patching anyway
it's pretty clean to add the D1 variant via ALTERNATIVE_2 to the same
location.

The only slightly bigger difference is that the "normal" type is not 0
as with svpbmt, so kernel patches for this PMA type need to be applied
even before the MMU is brought up, so the series introduces a separate
stage for that.


In theory this series is 2 parts:
- alternatives improvements
- svpbmt+d1

I picked the recipient list from the previous versions, hopefully
I didn't forget anybody.

I tested the series on:
- qemu-rv32 + buildroot rootfs
- qemu-rv64 + debian roots
- Allwinner D1-Nezha
- BeagleV - it at least reached the same point as without the series

I also ran Palmers CI environment on 5.18-rc6 + this series and
it passed with all testcases now.


changes in v10:
- add received review-tags
- put early patching behind a kconfig symbol
- adapt compiler flags of sources in use by early patching
similar to other riscv arch-parts.
This fixes the medlow cmodel issue on rv32 and also issues
with Kasan.

changes in v9:
- rebase onto 5.18-rc1
- drop the sbi null-ptr patch
While I still think this to be non-ideal as is, it isn't really
necessary for svpbmt support anymore
- merge cpufeature + svpbmt patch, as otherwise some empty shells
cause build warnings when a bisection stops between these two
patches
- address review comments from Christoph Hellwig:
- keep alternatives optional, they now get selected by its
users (erratas and also the newly introduced svpbmt kconfig)
- wrap long lines and keep things below 80 characters
- restyle svpbmt + thead errata assembly
- introduce a helper for the repeated calls to
(val & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT

changes in v8:
- rebase onto 5.17-final + isa extension series
We're halfway through the merge-window, so this series
should be merge after that
- adapt to fix limiting alternatives to non-xip-kernels
- add .norelax option for alternatives
- fix unused cpu_apply_errata in thead errata
- don't use static globals to store cpu-manufacturer data
as it makes machines hang if done too early

changes in v7:
- fix typo in patch1 (Atish)
- moved to Atish's isa-extension framework
- and therefore move regular boot-alternatives directly behind fill_hwcaps
- change T-Head errata Kconfig text (Atish)

changes in v6:
- rebase onto 5.17-rc1
- handle sbi null-ptr differently
- improve commit messages
- use riscv,mmu as property name

changes in v5:
- move to use alternatives for runtime-patching
- add D1 variant


[0] https://lore.kernel.org/r/[email protected]

Heiko Stuebner (12):
riscv: integrate alternatives better into the main architecture
riscv: allow different stages with alternatives
riscv: implement module alternatives
riscv: implement ALTERNATIVE_2 macro
riscv: extend concatenated alternatives-lines to the same length
riscv: prevent compressed instructions in alternatives
riscv: move boot alternatives to after fill_hwcap
riscv: Fix accessing pfn bits in PTEs for non-32bit variants
riscv: add RISC-V Svpbmt extension support
riscv: remove FIXMAP_PAGE_IO and fall back to its default value
riscv: don't use global static vars to store alternative data
riscv: add memory-type errata for T-Head

arch/riscv/Kconfig | 28 +++++
arch/riscv/Kconfig.erratas | 34 ++++--
arch/riscv/Kconfig.socs | 1 -
arch/riscv/Makefile | 2 +-
arch/riscv/errata/Makefile | 2 +-
arch/riscv/errata/alternative.c | 75 ------------
arch/riscv/errata/sifive/errata.c | 20 ++-
arch/riscv/errata/thead/Makefile | 11 ++
arch/riscv/errata/thead/errata.c | 82 +++++++++++++
arch/riscv/include/asm/alternative-macros.h | 129 +++++++++++++++-----
arch/riscv/include/asm/alternative.h | 25 +++-
arch/riscv/include/asm/errata_list.h | 59 +++++++++
arch/riscv/include/asm/fixmap.h | 2 -
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/include/asm/pgtable-32.h | 17 +++
arch/riscv/include/asm/pgtable-64.h | 79 +++++++++++-
arch/riscv/include/asm/pgtable-bits.h | 10 --
arch/riscv/include/asm/pgtable.h | 55 +++++++--
arch/riscv/include/asm/vendorid_list.h | 1 +
arch/riscv/kernel/Makefile | 15 +++
arch/riscv/kernel/alternative.c | 118 ++++++++++++++++++
arch/riscv/kernel/cpu.c | 1 +
arch/riscv/kernel/cpufeature.c | 80 +++++++++++-
arch/riscv/kernel/module.c | 29 +++++
arch/riscv/kernel/setup.c | 2 +
arch/riscv/kernel/smpboot.c | 4 -
arch/riscv/kernel/traps.c | 2 +-
arch/riscv/mm/init.c | 1 +
28 files changed, 724 insertions(+), 161 deletions(-)
delete mode 100644 arch/riscv/errata/alternative.c
create mode 100644 arch/riscv/errata/thead/Makefile
create mode 100644 arch/riscv/errata/thead/errata.c
create mode 100644 arch/riscv/kernel/alternative.c

--
2.35.1



2022-05-14 00:09:18

by Heiko Stuebner

[permalink] [raw]
Subject: [PATCH 05/12] riscv: extend concatenated alternatives-lines to the same length

ALT_NEW_CONTENT already uses same-length assembler lines, so
extend this to the other elements as well.

This makes it more readable when these elements need to be extended
in the future.

Signed-off-by: Heiko Stuebner <[email protected]>
Reviewed-by: Philipp Tomsich <[email protected]>
---
arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
index 9e04cd53afc8..8c2bbc7bbe50 100644
--- a/arch/riscv/include/asm/alternative-macros.h
+++ b/arch/riscv/include/asm/alternative-macros.h
@@ -62,14 +62,14 @@
#include <asm/asm.h>
#include <linux/stringify.h>

-#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \
- RISCV_PTR " " oldptr "\n" \
- RISCV_PTR " " newptr "\n" \
- REG_ASM " " vendor_id "\n" \
- REG_ASM " " newlen "\n" \
+#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \
+ RISCV_PTR " " oldptr "\n" \
+ RISCV_PTR " " newptr "\n" \
+ REG_ASM " " vendor_id "\n" \
+ REG_ASM " " newlen "\n" \
".word " errata_id "\n"

-#define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
+#define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
".if " __stringify(enable) " == 1\n" \
".pushsection .alternative, \"a\"\n" \
ALT_ENTRY("886b", "888f", __stringify(vendor_id), __stringify(errata_id), "889f - 888f") \
@@ -83,10 +83,10 @@
".org . - (889b - 888b) + (887b - 886b)\n" \
".endif\n"

-#define __ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, enable) \
- "886 :\n" \
- old_c "\n" \
- "887 :\n" \
+#define __ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, enable) \
+ "886 :\n" \
+ old_c "\n" \
+ "887 :\n" \
ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c)

#define _ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, CONFIG_k) \
--
2.35.1


2022-05-14 00:28:03

by Heiko Stuebner

[permalink] [raw]
Subject: Re: [PATCH v10 00/12] riscv: support for Svpbmt and D1 memory types

Am Freitag, 13. Mai 2022, 05:32:49 CEST schrieb Palmer Dabbelt:
> On Wed, 11 May 2022 12:29:09 PDT (-0700), [email protected] wrote:
> > Svpbmt is an extension defining "Supervisor-mode: page-based memory types"
> > for things like non-cacheable pages or I/O memory pages.
> >
> >
> > So this is my 2nd try at implementing Svpbmt (and the diverging D1 memory
> > types) using the alternatives framework.
> >
> > This includes a number of changes to the alternatives mechanism itself.
> > The biggest one being the move to a more central location, as I expect
> > in the future, nearly every chip needing some sort of patching, be it
> > either for erratas or for optional features (svpbmt or others).
> >
> > Detection of the svpbmt functionality is done via Atish's isa extension
> > handling series [0] and thus does not need any dt-parsing of its own
> > anymore.
> >
> > The series also introduces support for the memory types of the D1
> > which are implemented differently to svpbmt. But when patching anyway
> > it's pretty clean to add the D1 variant via ALTERNATIVE_2 to the same
> > location.
> >
> > The only slightly bigger difference is that the "normal" type is not 0
> > as with svpbmt, so kernel patches for this PMA type need to be applied
> > even before the MMU is brought up, so the series introduces a separate
> > stage for that.
> >
> >
> > In theory this series is 2 parts:
> > - alternatives improvements
> > - svpbmt+d1
> >
> > I picked the recipient list from the previous versions, hopefully
> > I didn't forget anybody.
> >
> > I tested the series on:
> > - qemu-rv32 + buildroot rootfs
> > - qemu-rv64 + debian roots
> > - Allwinner D1-Nezha
> > - BeagleV - it at least reached the same point as without the series
>
> IMO that's fine, it's also broken due to issues around non-coherence but
> it has an entirely different way of handling things than.
>
> > I also ran Palmers CI environment on 5.18-rc6 + this series and
> > it passed with all testcases now.
>
> Thanks, I know that's a bit of a mess. If I ever get some time I'll try
> and clean it up, but it keeps finding issues so I'm sort of stuck with
> it for now.

No worries, once I got it to run, it was easy to use, though needs
quite a bit of time to build everything.

But now that I made it to run, I'll try to use in the future as well :-) .

> As expected it now passes locally, so I've put this on
> for-next. I hadn't noticed your testing was on rc6, I put this on top
> of rc1 -- that's what I usually do for merge window stuff, but if
> there's something specific between rc1 and rc6 this depends on then LMK
> and I'll sort it out.

In the past I also had it run on -rc1 without any issues.
Just after refreshing with with your recent changes, it was back at
5.17-something and then I went to 5.18-rc6 as base.

So there wasn't any real reason for -rc6 except was the most current
release :-)

Heiko

> > changes in v10:
> > - add received review-tags
> > - put early patching behind a kconfig symbol
> > - adapt compiler flags of sources in use by early patching
> > similar to other riscv arch-parts.
> > This fixes the medlow cmodel issue on rv32 and also issues
> > with Kasan.
> >
> > changes in v9:
> > - rebase onto 5.18-rc1
> > - drop the sbi null-ptr patch
> > While I still think this to be non-ideal as is, it isn't really
> > necessary for svpbmt support anymore
> > - merge cpufeature + svpbmt patch, as otherwise some empty shells
> > cause build warnings when a bisection stops between these two
> > patches
> > - address review comments from Christoph Hellwig:
> > - keep alternatives optional, they now get selected by its
> > users (erratas and also the newly introduced svpbmt kconfig)
> > - wrap long lines and keep things below 80 characters
> > - restyle svpbmt + thead errata assembly
> > - introduce a helper for the repeated calls to
> > (val & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT
> >
> > changes in v8:
> > - rebase onto 5.17-final + isa extension series
> > We're halfway through the merge-window, so this series
> > should be merge after that
> > - adapt to fix limiting alternatives to non-xip-kernels
> > - add .norelax option for alternatives
> > - fix unused cpu_apply_errata in thead errata
> > - don't use static globals to store cpu-manufacturer data
> > as it makes machines hang if done too early
> >
> > changes in v7:
> > - fix typo in patch1 (Atish)
> > - moved to Atish's isa-extension framework
> > - and therefore move regular boot-alternatives directly behind fill_hwcaps
> > - change T-Head errata Kconfig text (Atish)
> >
> > changes in v6:
> > - rebase onto 5.17-rc1
> > - handle sbi null-ptr differently
> > - improve commit messages
> > - use riscv,mmu as property name
> >
> > changes in v5:
> > - move to use alternatives for runtime-patching
> > - add D1 variant
> >
> >
> > [0] https://lore.kernel.org/r/[email protected]
> >
> > Heiko Stuebner (12):
> > riscv: integrate alternatives better into the main architecture
> > riscv: allow different stages with alternatives
> > riscv: implement module alternatives
> > riscv: implement ALTERNATIVE_2 macro
> > riscv: extend concatenated alternatives-lines to the same length
> > riscv: prevent compressed instructions in alternatives
> > riscv: move boot alternatives to after fill_hwcap
> > riscv: Fix accessing pfn bits in PTEs for non-32bit variants
> > riscv: add RISC-V Svpbmt extension support
> > riscv: remove FIXMAP_PAGE_IO and fall back to its default value
> > riscv: don't use global static vars to store alternative data
> > riscv: add memory-type errata for T-Head
> >
> > arch/riscv/Kconfig | 28 +++++
> > arch/riscv/Kconfig.erratas | 34 ++++--
> > arch/riscv/Kconfig.socs | 1 -
> > arch/riscv/Makefile | 2 +-
> > arch/riscv/errata/Makefile | 2 +-
> > arch/riscv/errata/alternative.c | 75 ------------
> > arch/riscv/errata/sifive/errata.c | 20 ++-
> > arch/riscv/errata/thead/Makefile | 11 ++
> > arch/riscv/errata/thead/errata.c | 82 +++++++++++++
> > arch/riscv/include/asm/alternative-macros.h | 129 +++++++++++++++-----
> > arch/riscv/include/asm/alternative.h | 25 +++-
> > arch/riscv/include/asm/errata_list.h | 59 +++++++++
> > arch/riscv/include/asm/fixmap.h | 2 -
> > arch/riscv/include/asm/hwcap.h | 1 +
> > arch/riscv/include/asm/pgtable-32.h | 17 +++
> > arch/riscv/include/asm/pgtable-64.h | 79 +++++++++++-
> > arch/riscv/include/asm/pgtable-bits.h | 10 --
> > arch/riscv/include/asm/pgtable.h | 55 +++++++--
> > arch/riscv/include/asm/vendorid_list.h | 1 +
> > arch/riscv/kernel/Makefile | 15 +++
> > arch/riscv/kernel/alternative.c | 118 ++++++++++++++++++
> > arch/riscv/kernel/cpu.c | 1 +
> > arch/riscv/kernel/cpufeature.c | 80 +++++++++++-
> > arch/riscv/kernel/module.c | 29 +++++
> > arch/riscv/kernel/setup.c | 2 +
> > arch/riscv/kernel/smpboot.c | 4 -
> > arch/riscv/kernel/traps.c | 2 +-
> > arch/riscv/mm/init.c | 1 +
> > 28 files changed, 724 insertions(+), 161 deletions(-)
> > delete mode 100644 arch/riscv/errata/alternative.c
> > create mode 100644 arch/riscv/errata/thead/Makefile
> > create mode 100644 arch/riscv/errata/thead/errata.c
> > create mode 100644 arch/riscv/kernel/alternative.c
>





2022-05-14 00:47:32

by Palmer Dabbelt

[permalink] [raw]
Subject: Re: [PATCH v10 00/12] riscv: support for Svpbmt and D1 memory types

On Wed, 11 May 2022 12:29:09 PDT (-0700), [email protected] wrote:
> Svpbmt is an extension defining "Supervisor-mode: page-based memory types"
> for things like non-cacheable pages or I/O memory pages.
>
>
> So this is my 2nd try at implementing Svpbmt (and the diverging D1 memory
> types) using the alternatives framework.
>
> This includes a number of changes to the alternatives mechanism itself.
> The biggest one being the move to a more central location, as I expect
> in the future, nearly every chip needing some sort of patching, be it
> either for erratas or for optional features (svpbmt or others).
>
> Detection of the svpbmt functionality is done via Atish's isa extension
> handling series [0] and thus does not need any dt-parsing of its own
> anymore.
>
> The series also introduces support for the memory types of the D1
> which are implemented differently to svpbmt. But when patching anyway
> it's pretty clean to add the D1 variant via ALTERNATIVE_2 to the same
> location.
>
> The only slightly bigger difference is that the "normal" type is not 0
> as with svpbmt, so kernel patches for this PMA type need to be applied
> even before the MMU is brought up, so the series introduces a separate
> stage for that.
>
>
> In theory this series is 2 parts:
> - alternatives improvements
> - svpbmt+d1
>
> I picked the recipient list from the previous versions, hopefully
> I didn't forget anybody.
>
> I tested the series on:
> - qemu-rv32 + buildroot rootfs
> - qemu-rv64 + debian roots
> - Allwinner D1-Nezha
> - BeagleV - it at least reached the same point as without the series

IMO that's fine, it's also broken due to issues around non-coherence but
it has an entirely different way of handling things than.

> I also ran Palmers CI environment on 5.18-rc6 + this series and
> it passed with all testcases now.

Thanks, I know that's a bit of a mess. If I ever get some time I'll try
and clean it up, but it keeps finding issues so I'm sort of stuck with
it for now. As expected it now passes locally, so I've put this on
for-next. I hadn't noticed your testing was on rc6, I put this on top
of rc1 -- that's what I usually do for merge window stuff, but if
there's something specific between rc1 and rc6 this depends on then LMK
and I'll sort it out.

> changes in v10:
> - add received review-tags
> - put early patching behind a kconfig symbol
> - adapt compiler flags of sources in use by early patching
> similar to other riscv arch-parts.
> This fixes the medlow cmodel issue on rv32 and also issues
> with Kasan.
>
> changes in v9:
> - rebase onto 5.18-rc1
> - drop the sbi null-ptr patch
> While I still think this to be non-ideal as is, it isn't really
> necessary for svpbmt support anymore
> - merge cpufeature + svpbmt patch, as otherwise some empty shells
> cause build warnings when a bisection stops between these two
> patches
> - address review comments from Christoph Hellwig:
> - keep alternatives optional, they now get selected by its
> users (erratas and also the newly introduced svpbmt kconfig)
> - wrap long lines and keep things below 80 characters
> - restyle svpbmt + thead errata assembly
> - introduce a helper for the repeated calls to
> (val & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT
>
> changes in v8:
> - rebase onto 5.17-final + isa extension series
> We're halfway through the merge-window, so this series
> should be merge after that
> - adapt to fix limiting alternatives to non-xip-kernels
> - add .norelax option for alternatives
> - fix unused cpu_apply_errata in thead errata
> - don't use static globals to store cpu-manufacturer data
> as it makes machines hang if done too early
>
> changes in v7:
> - fix typo in patch1 (Atish)
> - moved to Atish's isa-extension framework
> - and therefore move regular boot-alternatives directly behind fill_hwcaps
> - change T-Head errata Kconfig text (Atish)
>
> changes in v6:
> - rebase onto 5.17-rc1
> - handle sbi null-ptr differently
> - improve commit messages
> - use riscv,mmu as property name
>
> changes in v5:
> - move to use alternatives for runtime-patching
> - add D1 variant
>
>
> [0] https://lore.kernel.org/r/[email protected]
>
> Heiko Stuebner (12):
> riscv: integrate alternatives better into the main architecture
> riscv: allow different stages with alternatives
> riscv: implement module alternatives
> riscv: implement ALTERNATIVE_2 macro
> riscv: extend concatenated alternatives-lines to the same length
> riscv: prevent compressed instructions in alternatives
> riscv: move boot alternatives to after fill_hwcap
> riscv: Fix accessing pfn bits in PTEs for non-32bit variants
> riscv: add RISC-V Svpbmt extension support
> riscv: remove FIXMAP_PAGE_IO and fall back to its default value
> riscv: don't use global static vars to store alternative data
> riscv: add memory-type errata for T-Head
>
> arch/riscv/Kconfig | 28 +++++
> arch/riscv/Kconfig.erratas | 34 ++++--
> arch/riscv/Kconfig.socs | 1 -
> arch/riscv/Makefile | 2 +-
> arch/riscv/errata/Makefile | 2 +-
> arch/riscv/errata/alternative.c | 75 ------------
> arch/riscv/errata/sifive/errata.c | 20 ++-
> arch/riscv/errata/thead/Makefile | 11 ++
> arch/riscv/errata/thead/errata.c | 82 +++++++++++++
> arch/riscv/include/asm/alternative-macros.h | 129 +++++++++++++++-----
> arch/riscv/include/asm/alternative.h | 25 +++-
> arch/riscv/include/asm/errata_list.h | 59 +++++++++
> arch/riscv/include/asm/fixmap.h | 2 -
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/asm/pgtable-32.h | 17 +++
> arch/riscv/include/asm/pgtable-64.h | 79 +++++++++++-
> arch/riscv/include/asm/pgtable-bits.h | 10 --
> arch/riscv/include/asm/pgtable.h | 55 +++++++--
> arch/riscv/include/asm/vendorid_list.h | 1 +
> arch/riscv/kernel/Makefile | 15 +++
> arch/riscv/kernel/alternative.c | 118 ++++++++++++++++++
> arch/riscv/kernel/cpu.c | 1 +
> arch/riscv/kernel/cpufeature.c | 80 +++++++++++-
> arch/riscv/kernel/module.c | 29 +++++
> arch/riscv/kernel/setup.c | 2 +
> arch/riscv/kernel/smpboot.c | 4 -
> arch/riscv/kernel/traps.c | 2 +-
> arch/riscv/mm/init.c | 1 +
> 28 files changed, 724 insertions(+), 161 deletions(-)
> delete mode 100644 arch/riscv/errata/alternative.c
> create mode 100644 arch/riscv/errata/thead/Makefile
> create mode 100644 arch/riscv/errata/thead/errata.c
> create mode 100644 arch/riscv/kernel/alternative.c

2022-05-14 01:01:56

by Heiko Stuebner

[permalink] [raw]
Subject: [PATCH 11/12] riscv: don't use global static vars to store alternative data

Right now the code uses a global struct to store vendor-ids
and another global variable to store the vendor-patch-function.

There exist specific cases where we'll need to patch the kernel
at an even earlier stage, where trying to write to a static
variable might actually result in hangs.

Also collecting the vendor-information consists of 3 sbi-ecalls
(or csr-reads) which is pretty negligible in the context of
booting a kernel.

So rework the code to not rely on static variables and instead
collect the vendor-information when a round of alternatives is
to be applied.

Signed-off-by: Heiko Stuebner <[email protected]>
Reviewed-by: Guo Ren <[email protected]>
Reviewed-by: Philipp Tomsich <[email protected]>
---
arch/riscv/kernel/alternative.c | 51 ++++++++++++++++-----------------
1 file changed, 24 insertions(+), 27 deletions(-)

diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
index e6c9de9f9ba6..27f722ae452b 100644
--- a/arch/riscv/kernel/alternative.c
+++ b/arch/riscv/kernel/alternative.c
@@ -16,41 +16,35 @@
#include <asm/sbi.h>
#include <asm/csr.h>

-static struct cpu_manufacturer_info_t {
+struct cpu_manufacturer_info_t {
unsigned long vendor_id;
unsigned long arch_id;
unsigned long imp_id;
-} cpu_mfr_info;
+ void (*vendor_patch_func)(struct alt_entry *begin, struct alt_entry *end,
+ unsigned long archid, unsigned long impid,
+ unsigned int stage);
+};

-static void (*vendor_patch_func)(struct alt_entry *begin, struct alt_entry *end,
- unsigned long archid, unsigned long impid,
- unsigned int stage) __initdata_or_module;
-
-static inline void __init riscv_fill_cpu_mfr_info(void)
+static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_info_t *cpu_mfr_info)
{
#ifdef CONFIG_RISCV_M_MODE
- 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);
+ 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);
#else
- cpu_mfr_info.vendor_id = sbi_get_mvendorid();
- cpu_mfr_info.arch_id = sbi_get_marchid();
- cpu_mfr_info.imp_id = sbi_get_mimpid();
+ cpu_mfr_info->vendor_id = sbi_get_mvendorid();
+ cpu_mfr_info->arch_id = sbi_get_marchid();
+ cpu_mfr_info->imp_id = sbi_get_mimpid();
#endif
-}
-
-static void __init init_alternative(void)
-{
- riscv_fill_cpu_mfr_info();

- switch (cpu_mfr_info.vendor_id) {
+ switch (cpu_mfr_info->vendor_id) {
#ifdef CONFIG_ERRATA_SIFIVE
case SIFIVE_VENDOR_ID:
- vendor_patch_func = sifive_errata_patch_func;
+ cpu_mfr_info->vendor_patch_func = sifive_errata_patch_func;
break;
#endif
default:
- vendor_patch_func = NULL;
+ cpu_mfr_info->vendor_patch_func = NULL;
}
}

@@ -63,14 +57,19 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin,
struct alt_entry *end,
unsigned int stage)
{
+ struct cpu_manufacturer_info_t cpu_mfr_info;
+
+ riscv_fill_cpu_mfr_info(&cpu_mfr_info);
+
riscv_cpufeature_patch_func(begin, end, stage);

- if (!vendor_patch_func)
+ if (!cpu_mfr_info.vendor_patch_func)
return;

- vendor_patch_func(begin, end,
- cpu_mfr_info.arch_id, cpu_mfr_info.imp_id,
- stage);
+ cpu_mfr_info.vendor_patch_func(begin, end,
+ cpu_mfr_info.arch_id,
+ cpu_mfr_info.imp_id,
+ stage);
}

void __init apply_boot_alternatives(void)
@@ -78,8 +77,6 @@ void __init apply_boot_alternatives(void)
/* If called on non-boot cpu things could go wrong */
WARN_ON(smp_processor_id() != 0);

- init_alternative();
-
_apply_alternatives((struct alt_entry *)__alt_start,
(struct alt_entry *)__alt_end,
RISCV_ALTERNATIVES_BOOT);
--
2.35.1


2022-05-16 08:55:12

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 05/12] riscv: extend concatenated alternatives-lines to the same length

Reviewed-by: Guo Ren <[email protected]>

On Thu, May 12, 2022 at 3:29 AM Heiko Stuebner <[email protected]> wrote:
>
> ALT_NEW_CONTENT already uses same-length assembler lines, so
> extend this to the other elements as well.
>
> This makes it more readable when these elements need to be extended
> in the future.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> Reviewed-by: Philipp Tomsich <[email protected]>
> ---
> arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> index 9e04cd53afc8..8c2bbc7bbe50 100644
> --- a/arch/riscv/include/asm/alternative-macros.h
> +++ b/arch/riscv/include/asm/alternative-macros.h
> @@ -62,14 +62,14 @@
> #include <asm/asm.h>
> #include <linux/stringify.h>
>
> -#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \
> - RISCV_PTR " " oldptr "\n" \
> - RISCV_PTR " " newptr "\n" \
> - REG_ASM " " vendor_id "\n" \
> - REG_ASM " " newlen "\n" \
> +#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \
> + RISCV_PTR " " oldptr "\n" \
> + RISCV_PTR " " newptr "\n" \
> + REG_ASM " " vendor_id "\n" \
> + REG_ASM " " newlen "\n" \
> ".word " errata_id "\n"
>
> -#define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
> +#define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
> ".if " __stringify(enable) " == 1\n" \
> ".pushsection .alternative, \"a\"\n" \
> ALT_ENTRY("886b", "888f", __stringify(vendor_id), __stringify(errata_id), "889f - 888f") \
> @@ -83,10 +83,10 @@
> ".org . - (889b - 888b) + (887b - 886b)\n" \
> ".endif\n"
>
> -#define __ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, enable) \
> - "886 :\n" \
> - old_c "\n" \
> - "887 :\n" \
> +#define __ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, enable) \
> + "886 :\n" \
> + old_c "\n" \
> + "887 :\n" \
> ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c)
>
> #define _ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, CONFIG_k) \
> --
> 2.35.1
>


--
Best Regards
Guo Ren

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

2022-05-16 16:27:11

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 11/12] riscv: don't use global static vars to store alternative data

On Wed, May 11, 2022 at 09:29:20PM +0200, Heiko Stuebner wrote:
> Right now the code uses a global struct to store vendor-ids
> and another global variable to store the vendor-patch-function.
>
> There exist specific cases where we'll need to patch the kernel
> at an even earlier stage, where trying to write to a static
> variable might actually result in hangs.
>
> Also collecting the vendor-information consists of 3 sbi-ecalls
> (or csr-reads) which is pretty negligible in the context of
> booting a kernel.
>
> So rework the code to not rely on static variables and instead
> collect the vendor-information when a round of alternatives is
> to be applied.
>
> Signed-off-by: Heiko Stuebner <[email protected]>
> Reviewed-by: Guo Ren <[email protected]>
> Reviewed-by: Philipp Tomsich <[email protected]>
> ---
> arch/riscv/kernel/alternative.c | 51 ++++++++++++++++-----------------
> 1 file changed, 24 insertions(+), 27 deletions(-)
>
> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> index e6c9de9f9ba6..27f722ae452b 100644
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -16,41 +16,35 @@
> #include <asm/sbi.h>
> #include <asm/csr.h>
>
> -static struct cpu_manufacturer_info_t {
> +struct cpu_manufacturer_info_t {
> unsigned long vendor_id;
> unsigned long arch_id;
> unsigned long imp_id;
> -} cpu_mfr_info;
> + void (*vendor_patch_func)(struct alt_entry *begin, struct alt_entry *end,
> + unsigned long archid, unsigned long impid,
> + unsigned int stage);

Please drop the confusing vendor_ prefix for the function pointer
while you're at it. The vendor id is just one of three inputs for
the patching.

Otherwise this looks good:

Reviewed-by: Christoph Hellwig <[email protected]>