diff mbox series

[v4,2/6] RISC-V: Enable cbo.zero in usermode

Message ID 20230918131518.56803-10-ajones@ventanamicro.com (mailing list archive)
State Accepted
Commit 43c16d51a19b0ba2ed66978d5924d486ec1e42bc
Headers show
Series RISC-V: Enable cbo.zero in usermode | expand

Checks

Context Check Description
conchuod/cover_letter success Series has a cover letter
conchuod/tree_selection success Guessed tree name to be for-next at HEAD 0bb80ecc33a8
conchuod/fixes_present success Fixes tag not required for -next series
conchuod/maintainers_pattern success MAINTAINERS pattern errors before the patch: 5 and now 5
conchuod/verify_signedoff success Signed-off-by tag matches author and committer
conchuod/kdoc success Errors and warnings before: 0 this patch: 0
conchuod/build_rv64_clang_allmodconfig success Errors and warnings before: 2154 this patch: 2154
conchuod/module_param success Was 0 now: 0
conchuod/build_rv64_gcc_allmodconfig success Errors and warnings before: 1414 this patch: 1414
conchuod/build_rv32_defconfig success Build OK
conchuod/dtb_warn_rv64 success Errors and warnings before: 25 this patch: 25
conchuod/header_inline success No static functions without inline keyword in header files
conchuod/checkpatch warning CHECK: Consider using #include <linux/cpufeature.h> instead of <asm/cpufeature.h>
conchuod/build_rv64_nommu_k210_defconfig success Build OK
conchuod/verify_fixes success No Fixes tag
conchuod/build_rv64_nommu_virt_defconfig success Build OK
conchuod/patch-2-test-13 success .github/scripts/patches/verify_signedoff.sh
conchuod/vmtest-for-next-PR warning PR summary
conchuod/patch-2-test-1 success .github/scripts/patches/build_rv32_defconfig.sh
conchuod/patch-2-test-2 success .github/scripts/patches/build_rv64_clang_allmodconfig.sh
conchuod/patch-2-test-3 success .github/scripts/patches/build_rv64_gcc_allmodconfig.sh
conchuod/patch-2-test-4 success .github/scripts/patches/build_rv64_nommu_k210_defconfig.sh
conchuod/patch-2-test-5 success .github/scripts/patches/build_rv64_nommu_virt_defconfig.sh
conchuod/patch-2-test-6 warning .github/scripts/patches/checkpatch.sh
conchuod/patch-2-test-7 success .github/scripts/patches/dtb_warn_rv64.sh
conchuod/patch-2-test-8 success .github/scripts/patches/header_inline.sh
conchuod/patch-2-test-9 success .github/scripts/patches/kdoc.sh
conchuod/patch-2-test-10 success .github/scripts/patches/module_param.sh
conchuod/patch-2-test-11 success .github/scripts/patches/verify_fixes.sh
conchuod/patch-2-test-12 success .github/scripts/patches/verify_signedoff.sh

Commit Message

Andrew Jones Sept. 18, 2023, 1:15 p.m. UTC
When Zicboz is present, enable its instruction (cbo.zero) in
usermode by setting its respective senvcfg bit. We don't bother
trying to set this bit per-task, which would also require an
interface for tasks to request enabling and/or disabling. Instead,
permanently set the bit for each hart which has the extension when
bringing it online.

This patch also introduces riscv_cpu_has_extension_[un]likely()
functions to check a specific hart's ISA bitmap for extensions.
Prior to checking the specific hart's bitmap in these functions
we try the bitmap which represents the LCD of extensions, but only
when we know it will use its optimized, alternatives path by gating
its call on CONFIG_RISCV_ALTERNATIVE. When alternatives are used, the
compiler ensures that the invocation of the LCD search becomes a
constant true or false. When it's true, even the new functions will
completely vanish from their callsites. OTOH, when the LCD check is
false, we need to do a search of the hart's ISA bitmap. Had we also
checked the LCD bitmap without the use of alternatives, then we would
have ended up with two bitmap searches instead of one.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/cpufeature.h |  1 +
 arch/riscv/include/asm/csr.h        |  1 +
 arch/riscv/include/asm/hwcap.h      | 16 ++++++++++++++++
 arch/riscv/kernel/cpufeature.c      |  6 ++++++
 arch/riscv/kernel/setup.c           |  4 ++++
 arch/riscv/kernel/smpboot.c         |  4 ++++
 6 files changed, 32 insertions(+)

Comments

Palmer Dabbelt Sept. 21, 2023, 1:30 p.m. UTC | #1
On Mon, 18 Sep 2023 06:15:21 PDT (-0700), ajones@ventanamicro.com wrote:
> When Zicboz is present, enable its instruction (cbo.zero) in
> usermode by setting its respective senvcfg bit. We don't bother
> trying to set this bit per-task, which would also require an
> interface for tasks to request enabling and/or disabling. Instead,
> permanently set the bit for each hart which has the extension when
> bringing it online.
>
> This patch also introduces riscv_cpu_has_extension_[un]likely()
> functions to check a specific hart's ISA bitmap for extensions.
> Prior to checking the specific hart's bitmap in these functions
> we try the bitmap which represents the LCD of extensions, but only
> when we know it will use its optimized, alternatives path by gating
> its call on CONFIG_RISCV_ALTERNATIVE. When alternatives are used, the
> compiler ensures that the invocation of the LCD search becomes a
> constant true or false. When it's true, even the new functions will
> completely vanish from their callsites. OTOH, when the LCD check is
> false, we need to do a search of the hart's ISA bitmap. Had we also
> checked the LCD bitmap without the use of alternatives, then we would
> have ended up with two bitmap searches instead of one.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  arch/riscv/include/asm/cpufeature.h |  1 +
>  arch/riscv/include/asm/csr.h        |  1 +
>  arch/riscv/include/asm/hwcap.h      | 16 ++++++++++++++++
>  arch/riscv/kernel/cpufeature.c      |  6 ++++++
>  arch/riscv/kernel/setup.c           |  4 ++++
>  arch/riscv/kernel/smpboot.c         |  4 ++++
>  6 files changed, 32 insertions(+)
>
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index d0345bd659c9..13b7d35648a9 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -31,5 +31,6 @@ DECLARE_PER_CPU(long, misaligned_access_speed);
>  extern struct riscv_isainfo hart_isa[NR_CPUS];
>
>  void check_unaligned_access(int cpu);
> +void riscv_user_isa_enable(void);
>
>  #endif
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 777cb8299551..5fba25db82d2 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -275,6 +275,7 @@
>  #define CSR_SIE			0x104
>  #define CSR_STVEC		0x105
>  #define CSR_SCOUNTEREN		0x106
> +#define CSR_SENVCFG		0x10a
>  #define CSR_SSCRATCH		0x140
>  #define CSR_SEPC		0x141
>  #define CSR_SCAUSE		0x142
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index b7b58258f6c7..31774bcdf1c6 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -70,6 +70,7 @@
>  #ifndef __ASSEMBLY__
>
>  #include <linux/jump_label.h>
> +#include <asm/cpufeature.h>
>
>  unsigned long riscv_get_elf_hwcap(void);
>
> @@ -137,6 +138,21 @@ riscv_has_extension_unlikely(const unsigned long ext)
>  	return true;
>  }
>
> +static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> +{
> +	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> +		return true;
> +
> +	return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> +}
> +
> +static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
> +{
> +	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> +		return true;
> +
> +	return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> +}
>  #endif
>
>  #endif /* _ASM_RISCV_HWCAP_H */
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index f9ac2717bc7d..8ad6da03ee34 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -653,6 +653,12 @@ static int check_unaligned_access_boot_cpu(void)
>
>  arch_initcall(check_unaligned_access_boot_cpu);
>
> +void riscv_user_isa_enable(void)
> +{
> +	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> +		csr_set(CSR_SENVCFG, ENVCFG_CBZE);

It looks like CBZE didn't actually get ratified?  The CMO extension says

    _The CMO extensions rely on state in {csrname} CSRs that will be defined in a
    future update to the privileged architecture. If this CSR update is not
    ratified, the CMO extension will define its own CSRs._

but the privileged spec says

    The definition of the CBZE field will be furnished by the forthcoming
    Zicboz extension. Its allocation within `senvcfg` may change prior to
    the ratification of that extension.

Is there some ratified spec that actually defines this?

> +}
> +
>  #ifdef CONFIG_RISCV_ALTERNATIVE
>  /*
>   * Alternative patch sites consider 48 bits when determining when to patch
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index e600aab116a4..8fd6c02353d4 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -26,6 +26,7 @@
>  #include <asm/acpi.h>
>  #include <asm/alternative.h>
>  #include <asm/cacheflush.h>
> +#include <asm/cpufeature.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/early_ioremap.h>
>  #include <asm/pgtable.h>
> @@ -314,10 +315,13 @@ void __init setup_arch(char **cmdline_p)
>  	riscv_fill_hwcap();
>  	init_rt_signal_env();
>  	apply_boot_alternatives();
> +
>  	if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
>  	    riscv_isa_extension_available(NULL, ZICBOM))
>  		riscv_noncoherent_supported();
>  	riscv_set_dma_cache_alignment();
> +
> +	riscv_user_isa_enable();
>  }
>
>  static int __init topology_init(void)
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index 1b8da4e40a4d..d1b0a6fc3adf 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -25,6 +25,8 @@
>  #include <linux/of.h>
>  #include <linux/sched/task_stack.h>
>  #include <linux/sched/mm.h>
> +
> +#include <asm/cpufeature.h>
>  #include <asm/cpu_ops.h>
>  #include <asm/cpufeature.h>
>  #include <asm/irq.h>
> @@ -253,6 +255,8 @@ asmlinkage __visible void smp_callin(void)
>  			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
>  	}
>
> +	riscv_user_isa_enable();
> +
>  	/*
>  	 * Remote TLB flushes are ignored while the CPU is offline, so emit
>  	 * a local TLB flush right now just in case.
Anup Patel Sept. 21, 2023, 2:17 p.m. UTC | #2
On Thu, Sep 21, 2023 at 7:00 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Mon, 18 Sep 2023 06:15:21 PDT (-0700), ajones@ventanamicro.com wrote:
> > When Zicboz is present, enable its instruction (cbo.zero) in
> > usermode by setting its respective senvcfg bit. We don't bother
> > trying to set this bit per-task, which would also require an
> > interface for tasks to request enabling and/or disabling. Instead,
> > permanently set the bit for each hart which has the extension when
> > bringing it online.
> >
> > This patch also introduces riscv_cpu_has_extension_[un]likely()
> > functions to check a specific hart's ISA bitmap for extensions.
> > Prior to checking the specific hart's bitmap in these functions
> > we try the bitmap which represents the LCD of extensions, but only
> > when we know it will use its optimized, alternatives path by gating
> > its call on CONFIG_RISCV_ALTERNATIVE. When alternatives are used, the
> > compiler ensures that the invocation of the LCD search becomes a
> > constant true or false. When it's true, even the new functions will
> > completely vanish from their callsites. OTOH, when the LCD check is
> > false, we need to do a search of the hart's ISA bitmap. Had we also
> > checked the LCD bitmap without the use of alternatives, then we would
> > have ended up with two bitmap searches instead of one.
> >
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  arch/riscv/include/asm/cpufeature.h |  1 +
> >  arch/riscv/include/asm/csr.h        |  1 +
> >  arch/riscv/include/asm/hwcap.h      | 16 ++++++++++++++++
> >  arch/riscv/kernel/cpufeature.c      |  6 ++++++
> >  arch/riscv/kernel/setup.c           |  4 ++++
> >  arch/riscv/kernel/smpboot.c         |  4 ++++
> >  6 files changed, 32 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> > index d0345bd659c9..13b7d35648a9 100644
> > --- a/arch/riscv/include/asm/cpufeature.h
> > +++ b/arch/riscv/include/asm/cpufeature.h
> > @@ -31,5 +31,6 @@ DECLARE_PER_CPU(long, misaligned_access_speed);
> >  extern struct riscv_isainfo hart_isa[NR_CPUS];
> >
> >  void check_unaligned_access(int cpu);
> > +void riscv_user_isa_enable(void);
> >
> >  #endif
> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> > index 777cb8299551..5fba25db82d2 100644
> > --- a/arch/riscv/include/asm/csr.h
> > +++ b/arch/riscv/include/asm/csr.h
> > @@ -275,6 +275,7 @@
> >  #define CSR_SIE                      0x104
> >  #define CSR_STVEC            0x105
> >  #define CSR_SCOUNTEREN               0x106
> > +#define CSR_SENVCFG          0x10a
> >  #define CSR_SSCRATCH         0x140
> >  #define CSR_SEPC             0x141
> >  #define CSR_SCAUSE           0x142
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index b7b58258f6c7..31774bcdf1c6 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -70,6 +70,7 @@
> >  #ifndef __ASSEMBLY__
> >
> >  #include <linux/jump_label.h>
> > +#include <asm/cpufeature.h>
> >
> >  unsigned long riscv_get_elf_hwcap(void);
> >
> > @@ -137,6 +138,21 @@ riscv_has_extension_unlikely(const unsigned long ext)
> >       return true;
> >  }
> >
> > +static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> > +{
> > +     if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > +             return true;
> > +
> > +     return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > +}
> > +
> > +static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
> > +{
> > +     if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > +             return true;
> > +
> > +     return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > +}
> >  #endif
> >
> >  #endif /* _ASM_RISCV_HWCAP_H */
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index f9ac2717bc7d..8ad6da03ee34 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -653,6 +653,12 @@ static int check_unaligned_access_boot_cpu(void)
> >
> >  arch_initcall(check_unaligned_access_boot_cpu);
> >
> > +void riscv_user_isa_enable(void)
> > +{
> > +     if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> > +             csr_set(CSR_SENVCFG, ENVCFG_CBZE);
>
> It looks like CBZE didn't actually get ratified?  The CMO extension says
>
>     _The CMO extensions rely on state in {csrname} CSRs that will be defined in a
>     future update to the privileged architecture. If this CSR update is not
>     ratified, the CMO extension will define its own CSRs._
>
> but the privileged spec says
>
>     The definition of the CBZE field will be furnished by the forthcoming
>     Zicboz extension. Its allocation within `senvcfg` may change prior to
>     the ratification of that extension.
>
> Is there some ratified spec that actually defines this?

These bits are defined in the "Chapter 3. Control and Status Register State"
page12 of the ratified RISC-V CMO specification.
(https://github.com/riscv/riscv-CMOs/blob/master/specifications/cmobase-v1.0.1.pdf)

Regards,
Anup

>
> > +}
> > +
> >  #ifdef CONFIG_RISCV_ALTERNATIVE
> >  /*
> >   * Alternative patch sites consider 48 bits when determining when to patch
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index e600aab116a4..8fd6c02353d4 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -26,6 +26,7 @@
> >  #include <asm/acpi.h>
> >  #include <asm/alternative.h>
> >  #include <asm/cacheflush.h>
> > +#include <asm/cpufeature.h>
> >  #include <asm/cpu_ops.h>
> >  #include <asm/early_ioremap.h>
> >  #include <asm/pgtable.h>
> > @@ -314,10 +315,13 @@ void __init setup_arch(char **cmdline_p)
> >       riscv_fill_hwcap();
> >       init_rt_signal_env();
> >       apply_boot_alternatives();
> > +
> >       if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
> >           riscv_isa_extension_available(NULL, ZICBOM))
> >               riscv_noncoherent_supported();
> >       riscv_set_dma_cache_alignment();
> > +
> > +     riscv_user_isa_enable();
> >  }
> >
> >  static int __init topology_init(void)
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index 1b8da4e40a4d..d1b0a6fc3adf 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -25,6 +25,8 @@
> >  #include <linux/of.h>
> >  #include <linux/sched/task_stack.h>
> >  #include <linux/sched/mm.h>
> > +
> > +#include <asm/cpufeature.h>
> >  #include <asm/cpu_ops.h>
> >  #include <asm/cpufeature.h>
> >  #include <asm/irq.h>
> > @@ -253,6 +255,8 @@ asmlinkage __visible void smp_callin(void)
> >                       elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> >       }
> >
> > +     riscv_user_isa_enable();
> > +
> >       /*
> >        * Remote TLB flushes are ignored while the CPU is offline, so emit
> >        * a local TLB flush right now just in case.
Anup Patel Sept. 21, 2023, 2:22 p.m. UTC | #3
On Thu, Sep 21, 2023 at 7:00 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Mon, 18 Sep 2023 06:15:21 PDT (-0700), ajones@ventanamicro.com wrote:
> > When Zicboz is present, enable its instruction (cbo.zero) in
> > usermode by setting its respective senvcfg bit. We don't bother
> > trying to set this bit per-task, which would also require an
> > interface for tasks to request enabling and/or disabling. Instead,
> > permanently set the bit for each hart which has the extension when
> > bringing it online.
> >
> > This patch also introduces riscv_cpu_has_extension_[un]likely()
> > functions to check a specific hart's ISA bitmap for extensions.
> > Prior to checking the specific hart's bitmap in these functions
> > we try the bitmap which represents the LCD of extensions, but only
> > when we know it will use its optimized, alternatives path by gating
> > its call on CONFIG_RISCV_ALTERNATIVE. When alternatives are used, the
> > compiler ensures that the invocation of the LCD search becomes a
> > constant true or false. When it's true, even the new functions will
> > completely vanish from their callsites. OTOH, when the LCD check is
> > false, we need to do a search of the hart's ISA bitmap. Had we also
> > checked the LCD bitmap without the use of alternatives, then we would
> > have ended up with two bitmap searches instead of one.
> >
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> >  arch/riscv/include/asm/cpufeature.h |  1 +
> >  arch/riscv/include/asm/csr.h        |  1 +
> >  arch/riscv/include/asm/hwcap.h      | 16 ++++++++++++++++
> >  arch/riscv/kernel/cpufeature.c      |  6 ++++++
> >  arch/riscv/kernel/setup.c           |  4 ++++
> >  arch/riscv/kernel/smpboot.c         |  4 ++++
> >  6 files changed, 32 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> > index d0345bd659c9..13b7d35648a9 100644
> > --- a/arch/riscv/include/asm/cpufeature.h
> > +++ b/arch/riscv/include/asm/cpufeature.h
> > @@ -31,5 +31,6 @@ DECLARE_PER_CPU(long, misaligned_access_speed);
> >  extern struct riscv_isainfo hart_isa[NR_CPUS];
> >
> >  void check_unaligned_access(int cpu);
> > +void riscv_user_isa_enable(void);
> >
> >  #endif
> > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> > index 777cb8299551..5fba25db82d2 100644
> > --- a/arch/riscv/include/asm/csr.h
> > +++ b/arch/riscv/include/asm/csr.h
> > @@ -275,6 +275,7 @@
> >  #define CSR_SIE                      0x104
> >  #define CSR_STVEC            0x105
> >  #define CSR_SCOUNTEREN               0x106
> > +#define CSR_SENVCFG          0x10a
> >  #define CSR_SSCRATCH         0x140
> >  #define CSR_SEPC             0x141
> >  #define CSR_SCAUSE           0x142
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index b7b58258f6c7..31774bcdf1c6 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -70,6 +70,7 @@
> >  #ifndef __ASSEMBLY__
> >
> >  #include <linux/jump_label.h>
> > +#include <asm/cpufeature.h>
> >
> >  unsigned long riscv_get_elf_hwcap(void);
> >
> > @@ -137,6 +138,21 @@ riscv_has_extension_unlikely(const unsigned long ext)
> >       return true;
> >  }
> >
> > +static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> > +{
> > +     if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > +             return true;
> > +
> > +     return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > +}
> > +
> > +static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
> > +{
> > +     if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > +             return true;
> > +
> > +     return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > +}
> >  #endif
> >
> >  #endif /* _ASM_RISCV_HWCAP_H */
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index f9ac2717bc7d..8ad6da03ee34 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -653,6 +653,12 @@ static int check_unaligned_access_boot_cpu(void)
> >
> >  arch_initcall(check_unaligned_access_boot_cpu);
> >
> > +void riscv_user_isa_enable(void)
> > +{
> > +     if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> > +             csr_set(CSR_SENVCFG, ENVCFG_CBZE);
>
> It looks like CBZE didn't actually get ratified?  The CMO extension says
>
>     _The CMO extensions rely on state in {csrname} CSRs that will be defined in a
>     future update to the privileged architecture. If this CSR update is not
>     ratified, the CMO extension will define its own CSRs._
>
> but the privileged spec says
>
>     The definition of the CBZE field will be furnished by the forthcoming
>     Zicboz extension. Its allocation within `senvcfg` may change prior to
>     the ratification of that extension.
>
> Is there some ratified spec that actually defines this?

I see what you mean. The integration of these bits into the RISC-V
priv spec has not happened hence we have the non-normative text
in the CMO specification. The bit positions in xenvcfg CSRs can't
change because it's already part of ratified CMO specification and
there are organizations (like Ventana) who have implemented these
bits.

Regards,
Anup

>
> > +}
> > +
> >  #ifdef CONFIG_RISCV_ALTERNATIVE
> >  /*
> >   * Alternative patch sites consider 48 bits when determining when to patch
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index e600aab116a4..8fd6c02353d4 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -26,6 +26,7 @@
> >  #include <asm/acpi.h>
> >  #include <asm/alternative.h>
> >  #include <asm/cacheflush.h>
> > +#include <asm/cpufeature.h>
> >  #include <asm/cpu_ops.h>
> >  #include <asm/early_ioremap.h>
> >  #include <asm/pgtable.h>
> > @@ -314,10 +315,13 @@ void __init setup_arch(char **cmdline_p)
> >       riscv_fill_hwcap();
> >       init_rt_signal_env();
> >       apply_boot_alternatives();
> > +
> >       if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
> >           riscv_isa_extension_available(NULL, ZICBOM))
> >               riscv_noncoherent_supported();
> >       riscv_set_dma_cache_alignment();
> > +
> > +     riscv_user_isa_enable();
> >  }
> >
> >  static int __init topology_init(void)
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index 1b8da4e40a4d..d1b0a6fc3adf 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -25,6 +25,8 @@
> >  #include <linux/of.h>
> >  #include <linux/sched/task_stack.h>
> >  #include <linux/sched/mm.h>
> > +
> > +#include <asm/cpufeature.h>
> >  #include <asm/cpu_ops.h>
> >  #include <asm/cpufeature.h>
> >  #include <asm/irq.h>
> > @@ -253,6 +255,8 @@ asmlinkage __visible void smp_callin(void)
> >                       elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> >       }
> >
> > +     riscv_user_isa_enable();
> > +
> >       /*
> >        * Remote TLB flushes are ignored while the CPU is offline, so emit
> >        * a local TLB flush right now just in case.
Andrew Jones Oct. 10, 2023, 5:03 p.m. UTC | #4
On Thu, Sep 21, 2023 at 07:52:10PM +0530, Anup Patel wrote:
> On Thu, Sep 21, 2023 at 7:00 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >
> > On Mon, 18 Sep 2023 06:15:21 PDT (-0700), ajones@ventanamicro.com wrote:
...
> > > +void riscv_user_isa_enable(void)
> > > +{
> > > +     if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> > > +             csr_set(CSR_SENVCFG, ENVCFG_CBZE);
> >
> > It looks like CBZE didn't actually get ratified?  The CMO extension says
> >
> >     _The CMO extensions rely on state in {csrname} CSRs that will be defined in a
> >     future update to the privileged architecture. If this CSR update is not
> >     ratified, the CMO extension will define its own CSRs._
> >
> > but the privileged spec says
> >
> >     The definition of the CBZE field will be furnished by the forthcoming
> >     Zicboz extension. Its allocation within `senvcfg` may change prior to
> >     the ratification of that extension.
> >
> > Is there some ratified spec that actually defines this?
> 
> I see what you mean. The integration of these bits into the RISC-V
> priv spec has not happened hence we have the non-normative text
> in the CMO specification. The bit positions in xenvcfg CSRs can't
> change because it's already part of ratified CMO specification and
> there are organizations (like Ventana) who have implemented these
> bits.
> 

Hi Palmer,

Does Anup's response satisfy the criteria for this? I'm wondering what, if
anything, I should do with this series. (The hwprobe-which-cpus series
is currently based on this series, because of the hwprobe selftests
changes, but I guess I could rework that.)

Thanks,
drew
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
index d0345bd659c9..13b7d35648a9 100644
--- a/arch/riscv/include/asm/cpufeature.h
+++ b/arch/riscv/include/asm/cpufeature.h
@@ -31,5 +31,6 @@  DECLARE_PER_CPU(long, misaligned_access_speed);
 extern struct riscv_isainfo hart_isa[NR_CPUS];
 
 void check_unaligned_access(int cpu);
+void riscv_user_isa_enable(void);
 
 #endif
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 777cb8299551..5fba25db82d2 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -275,6 +275,7 @@ 
 #define CSR_SIE			0x104
 #define CSR_STVEC		0x105
 #define CSR_SCOUNTEREN		0x106
+#define CSR_SENVCFG		0x10a
 #define CSR_SSCRATCH		0x140
 #define CSR_SEPC		0x141
 #define CSR_SCAUSE		0x142
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index b7b58258f6c7..31774bcdf1c6 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -70,6 +70,7 @@ 
 #ifndef __ASSEMBLY__
 
 #include <linux/jump_label.h>
+#include <asm/cpufeature.h>
 
 unsigned long riscv_get_elf_hwcap(void);
 
@@ -137,6 +138,21 @@  riscv_has_extension_unlikely(const unsigned long ext)
 	return true;
 }
 
+static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
+{
+	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
+		return true;
+
+	return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
+}
+
+static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
+{
+	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
+		return true;
+
+	return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
+}
 #endif
 
 #endif /* _ASM_RISCV_HWCAP_H */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index f9ac2717bc7d..8ad6da03ee34 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -653,6 +653,12 @@  static int check_unaligned_access_boot_cpu(void)
 
 arch_initcall(check_unaligned_access_boot_cpu);
 
+void riscv_user_isa_enable(void)
+{
+	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
+		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
+}
+
 #ifdef CONFIG_RISCV_ALTERNATIVE
 /*
  * Alternative patch sites consider 48 bits when determining when to patch
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index e600aab116a4..8fd6c02353d4 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -26,6 +26,7 @@ 
 #include <asm/acpi.h>
 #include <asm/alternative.h>
 #include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
 #include <asm/early_ioremap.h>
 #include <asm/pgtable.h>
@@ -314,10 +315,13 @@  void __init setup_arch(char **cmdline_p)
 	riscv_fill_hwcap();
 	init_rt_signal_env();
 	apply_boot_alternatives();
+
 	if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
 	    riscv_isa_extension_available(NULL, ZICBOM))
 		riscv_noncoherent_supported();
 	riscv_set_dma_cache_alignment();
+
+	riscv_user_isa_enable();
 }
 
 static int __init topology_init(void)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 1b8da4e40a4d..d1b0a6fc3adf 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -25,6 +25,8 @@ 
 #include <linux/of.h>
 #include <linux/sched/task_stack.h>
 #include <linux/sched/mm.h>
+
+#include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
 #include <asm/cpufeature.h>
 #include <asm/irq.h>
@@ -253,6 +255,8 @@  asmlinkage __visible void smp_callin(void)
 			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
 	}
 
+	riscv_user_isa_enable();
+
 	/*
 	 * Remote TLB flushes are ignored while the CPU is offline, so emit
 	 * a local TLB flush right now just in case.