| Message ID | 20240125062739.1339782-6-debug@rivosinc.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | riscv control-flow integrity for usermode | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
Yo, Series is RFC, so not gonna review it in depth, just wanted to comment on this particular patch. On Wed, Jan 24, 2024 at 10:21:30PM -0800, debug@rivosinc.com wrote: > From: Deepak Gupta <debug@rivosinc.com> > > This patch adds support for detecting zicfiss and zicfilp. zicfiss and zicfilp > stands for unprivleged integer spec extension for shadow stack and branch > tracking on indirect branches, respectively. > > This patch looks for zicfiss and zicfilp in device tree and accordinlgy lights > up bit in cpu feature bitmap. Furthermore this patch adds detection utility > functions to return whether shadow stack or landing pads are supported by > cpu. > > Signed-off-by: Deepak Gupta <debug@rivosinc.com> > --- > arch/riscv/include/asm/cpufeature.h | 18 ++++++++++++++++++ > arch/riscv/include/asm/hwcap.h | 2 ++ > arch/riscv/include/asm/processor.h | 1 + > arch/riscv/kernel/cpufeature.c | 2 ++ > 4 files changed, 23 insertions(+) > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > index a418c3112cd6..216190731c55 100644 > --- a/arch/riscv/include/asm/cpufeature.h > +++ b/arch/riscv/include/asm/cpufeature.h > @@ -133,4 +133,22 @@ static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsi > return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); > } > > +static inline bool cpu_supports_shadow_stack(void) > +{ > +#ifdef CONFIG_RISCV_USER_CFI In passing, I don't see any reason for not using IS_ENABLED() here. > + return riscv_isa_extension_available(NULL, ZICFISS); > +#else > + return false; > +#endif > +} > + > +static inline bool cpu_supports_indirect_br_lp_instr(void) > +{ > +#ifdef CONFIG_RISCV_USER_CFI > + return riscv_isa_extension_available(NULL, ZICFILP); > +#else > + return false; > +#endif > +} > + > #endif > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h > index 06d30526ef3b..918165cfb4fa 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -57,6 +57,8 @@ > #define RISCV_ISA_EXT_ZIHPM 42 > #define RISCV_ISA_EXT_SMSTATEEN 43 > #define RISCV_ISA_EXT_ZICOND 44 > +#define RISCV_ISA_EXT_ZICFISS 45 > +#define RISCV_ISA_EXT_ZICFILP 46 > > #define RISCV_ISA_EXT_MAX 64 > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h > index f19f861cda54..ee2f51787ff8 100644 > --- a/arch/riscv/include/asm/processor.h > +++ b/arch/riscv/include/asm/processor.h > @@ -13,6 +13,7 @@ > #include <vdso/processor.h> > > #include <asm/ptrace.h> > +#include <asm/hwcap.h> > > #ifdef CONFIG_64BIT > #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index 98623393fd1f..16624bc9a46b 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -185,6 +185,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { > __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), > __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT), > __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), > + __RISCV_ISA_EXT_DATA(zicfiss, RISCV_ISA_EXT_ZICFISS), > + __RISCV_ISA_EXT_DATA(zicfilp, RISCV_ISA_EXT_ZICFILP), Anything you add to this array, you need to document in a dt-binding. Also, you added these in the wrong place. There's a massive comment before the array describing the order entries must be in, please take a look. Thanks, Conor. > }; > > const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext); > -- > 2.43.0 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, Jan 25, 2024 at 05:59:24PM +0000, Conor Dooley wrote: >Yo, > >Series is RFC, so not gonna review it in depth, just wanted to comment >on this particular patch. > >On Wed, Jan 24, 2024 at 10:21:30PM -0800, debug@rivosinc.com wrote: >> From: Deepak Gupta <debug@rivosinc.com> >> >> This patch adds support for detecting zicfiss and zicfilp. zicfiss and zicfilp >> stands for unprivleged integer spec extension for shadow stack and branch >> tracking on indirect branches, respectively. >> >> This patch looks for zicfiss and zicfilp in device tree and accordinlgy lights >> up bit in cpu feature bitmap. Furthermore this patch adds detection utility >> functions to return whether shadow stack or landing pads are supported by >> cpu. >> >> Signed-off-by: Deepak Gupta <debug@rivosinc.com> >> --- >> arch/riscv/include/asm/cpufeature.h | 18 ++++++++++++++++++ >> arch/riscv/include/asm/hwcap.h | 2 ++ >> arch/riscv/include/asm/processor.h | 1 + >> arch/riscv/kernel/cpufeature.c | 2 ++ >> 4 files changed, 23 insertions(+) >> >> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h >> index a418c3112cd6..216190731c55 100644 >> --- a/arch/riscv/include/asm/cpufeature.h >> +++ b/arch/riscv/include/asm/cpufeature.h >> @@ -133,4 +133,22 @@ static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsi >> return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); >> } >> >> +static inline bool cpu_supports_shadow_stack(void) >> +{ >> +#ifdef CONFIG_RISCV_USER_CFI > >In passing, I don't see any reason for not using IS_ENABLED() here. No reason. I should probably do that. More readable. Thanks. > >> + return riscv_isa_extension_available(NULL, ZICFISS); >> +#else >> + return false; >> +#endif >> +} >> + >> +static inline bool cpu_supports_indirect_br_lp_instr(void) >> +{ >> +#ifdef CONFIG_RISCV_USER_CFI >> + return riscv_isa_extension_available(NULL, ZICFILP); >> +#else >> + return false; >> +#endif >> +} >> + >> #endif >> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h >> index 06d30526ef3b..918165cfb4fa 100644 >> --- a/arch/riscv/include/asm/hwcap.h >> +++ b/arch/riscv/include/asm/hwcap.h >> @@ -57,6 +57,8 @@ >> #define RISCV_ISA_EXT_ZIHPM 42 >> #define RISCV_ISA_EXT_SMSTATEEN 43 >> #define RISCV_ISA_EXT_ZICOND 44 >> +#define RISCV_ISA_EXT_ZICFISS 45 >> +#define RISCV_ISA_EXT_ZICFILP 46 >> >> #define RISCV_ISA_EXT_MAX 64 >> >> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h >> index f19f861cda54..ee2f51787ff8 100644 >> --- a/arch/riscv/include/asm/processor.h >> +++ b/arch/riscv/include/asm/processor.h >> @@ -13,6 +13,7 @@ >> #include <vdso/processor.h> >> >> #include <asm/ptrace.h> >> +#include <asm/hwcap.h> >> >> #ifdef CONFIG_64BIT >> #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) >> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c >> index 98623393fd1f..16624bc9a46b 100644 >> --- a/arch/riscv/kernel/cpufeature.c >> +++ b/arch/riscv/kernel/cpufeature.c >> @@ -185,6 +185,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { >> __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), >> __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT), >> __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), >> + __RISCV_ISA_EXT_DATA(zicfiss, RISCV_ISA_EXT_ZICFISS), >> + __RISCV_ISA_EXT_DATA(zicfilp, RISCV_ISA_EXT_ZICFILP), > >Anything you add to this array, you need to document in a dt-binding. You mean Documentation/devicetree/bindings/riscv/extensions.yaml (or possibly any other yaml if needed?) >Also, you added these in the wrong place. There's a massive comment >before the array describing the order entries must be in, please take a >look. I see the comment. In my defense, looks like I missed it when I was rebasing. Will fix it. > >Thanks, >Conor. > > >> }; >> >> const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext); >> -- >> 2.43.0 >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Thu, Jan 25, 2024 at 10:26:19AM -0800, Deepak Gupta wrote: > > Anything you add to this array, you need to document in a dt-binding. > > You mean Documentation/devicetree/bindings/riscv/extensions.yaml > (or possibly any other yaml if needed?) That one alone should be okay. > > Also, you added these in the wrong place. There's a massive comment > > before the array describing the order entries must be in, please take a > > look. > > I see the comment. > In my defense, looks like I missed it when I was rebasing. No worries.
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index a418c3112cd6..216190731c55 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -133,4 +133,22 @@ static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsi return __riscv_isa_extension_available(hart_isa[cpu].isa, ext); } +static inline bool cpu_supports_shadow_stack(void) +{ +#ifdef CONFIG_RISCV_USER_CFI + return riscv_isa_extension_available(NULL, ZICFISS); +#else + return false; +#endif +} + +static inline bool cpu_supports_indirect_br_lp_instr(void) +{ +#ifdef CONFIG_RISCV_USER_CFI + return riscv_isa_extension_available(NULL, ZICFILP); +#else + return false; +#endif +} + #endif diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index 06d30526ef3b..918165cfb4fa 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -57,6 +57,8 @@ #define RISCV_ISA_EXT_ZIHPM 42 #define RISCV_ISA_EXT_SMSTATEEN 43 #define RISCV_ISA_EXT_ZICOND 44 +#define RISCV_ISA_EXT_ZICFISS 45 +#define RISCV_ISA_EXT_ZICFILP 46 #define RISCV_ISA_EXT_MAX 64 diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h index f19f861cda54..ee2f51787ff8 100644 --- a/arch/riscv/include/asm/processor.h +++ b/arch/riscv/include/asm/processor.h @@ -13,6 +13,7 @@ #include <vdso/processor.h> #include <asm/ptrace.h> +#include <asm/hwcap.h> #ifdef CONFIG_64BIT #define DEFAULT_MAP_WINDOW (UL(1) << (MMAP_VA_BITS - 1)) diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 98623393fd1f..16624bc9a46b 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -185,6 +185,8 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = { __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT), __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT), + __RISCV_ISA_EXT_DATA(zicfiss, RISCV_ISA_EXT_ZICFISS), + __RISCV_ISA_EXT_DATA(zicfilp, RISCV_ISA_EXT_ZICFILP), }; const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);