[PATCH v2 1/3] RISC-V: pi: Force hidden visibility for all symbol references

[PATCH v2 1/3] RISC-V: pi: Force hidden visibility for all symbol references

[Jesse Taube]

Eliminate all GOT entries in the .pi section, by forcing hidden
visibility for all symbol references, which informs the compiler that
such references will be resolved at link time without the need for
allocating GOT entries.

Include linux/hidden.h in Makefile, like arm64, for the
hidden visibility attribute.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/kernel/pi/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
index 50bc5ef7dd2f..1ef7584be0c3 100644
--- a/arch/riscv/kernel/pi/Makefile
+++ b/arch/riscv/kernel/pi/Makefile
@@ -5,6 +5,7 @@ KBUILD_CFLAGS	:= $(subst $(CC_FLAGS_FTRACE),,$(KBUILD_CFLAGS)) -fpie \
 		   -Os -DDISABLE_BRANCH_PROFILING $(DISABLE_STACKLEAK_PLUGIN) \
 		   $(call cc-option,-mbranch-protection=none) \
 		   -I$(srctree)/scripts/dtc/libfdt -fno-stack-protector \
+		   -include $(srctree)/include/linux/hidden.h \
 		   -D__DISABLE_EXPORTS -ffreestanding \
 		   -fno-asynchronous-unwind-tables -fno-unwind-tables \
 		   $(call cc-option,-fno-addrsig)
-- 
2.45.2

[PATCH v2 2/3] RISC-V: pi: Add kernel/pi/pi.h

[Jesse Taube]

Add pi.h header for declarations of the kernel/pi prefixed functions
and any other related declarations.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/kernel/pi/cmdline_early.c | 10 ++--------
 arch/riscv/kernel/pi/fdt_early.c     |  7 +------
 arch/riscv/kernel/pi/pi.h            | 17 +++++++++++++++++
 3 files changed, 20 insertions(+), 14 deletions(-)
 create mode 100644 arch/riscv/kernel/pi/pi.h

diff --git a/arch/riscv/kernel/pi/cmdline_early.c b/arch/riscv/kernel/pi/cmdline_early.c
index f6d4dedffb84..fbcdc9e4e143 100644
--- a/arch/riscv/kernel/pi/cmdline_early.c
+++ b/arch/riscv/kernel/pi/cmdline_early.c
@@ -6,15 +6,9 @@
 #include <asm/pgtable.h>
 #include <asm/setup.h>
 
-static char early_cmdline[COMMAND_LINE_SIZE];
+#include "pi.h"
 
-/*
- * Declare the functions that are exported (but prefixed) here so that LLVM
- * does not complain it lacks the 'static' keyword (which, if added, makes
- * LLVM complain because the function is actually unused in this file).
- */
-u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
-bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
+static char early_cmdline[COMMAND_LINE_SIZE];
 
 static char *get_early_cmdline(uintptr_t dtb_pa)
 {
diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
index 899610e042ab..40ee299702bf 100644
--- a/arch/riscv/kernel/pi/fdt_early.c
+++ b/arch/riscv/kernel/pi/fdt_early.c
@@ -3,12 +3,7 @@
 #include <linux/init.h>
 #include <linux/libfdt.h>
 
-/*
- * Declare the functions that are exported (but prefixed) here so that LLVM
- * does not complain it lacks the 'static' keyword (which, if added, makes
- * LLVM complain because the function is actually unused in this file).
- */
-u64 get_kaslr_seed(uintptr_t dtb_pa);
+#include "pi.h"
 
 u64 get_kaslr_seed(uintptr_t dtb_pa)
 {
diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
new file mode 100644
index 000000000000..65da99466baf
--- /dev/null
+++ b/arch/riscv/kernel/pi/pi.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _RISCV_PI_H_
+#define _RISCV_PI_H_
+
+#include <linux/types.h>
+
+/*
+ * The folowing functions are exported (but prefixed) declare them here so
+ * that LLVM does not complain it lacks the 'static' keyword (which, if
+ * added, makes LLVM complain because the function is unused).
+ */
+
+u64 get_kaslr_seed(uintptr_t dtb_pa);
+bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
+u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
+
+#endif /* _RISCV_PI_H_ */
-- 
2.45.2

Re: [PATCH v2 2/3] RISC-V: pi: Add kernel/pi/pi.h

[Alexandre Ghiti]

Hi Jesse,

On Wed, Jun 26, 2024 at 7:17 PM Jesse Taube <jesse@rivosinc.com> wrote:
>
> Add pi.h header for declarations of the kernel/pi prefixed functions
> and any other related declarations.
>
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
>  arch/riscv/kernel/pi/cmdline_early.c | 10 ++--------
>  arch/riscv/kernel/pi/fdt_early.c     |  7 +------
>  arch/riscv/kernel/pi/pi.h            | 17 +++++++++++++++++
>  3 files changed, 20 insertions(+), 14 deletions(-)
>  create mode 100644 arch/riscv/kernel/pi/pi.h
>
> diff --git a/arch/riscv/kernel/pi/cmdline_early.c b/arch/riscv/kernel/pi/cmdline_early.c
> index f6d4dedffb84..fbcdc9e4e143 100644
> --- a/arch/riscv/kernel/pi/cmdline_early.c
> +++ b/arch/riscv/kernel/pi/cmdline_early.c
> @@ -6,15 +6,9 @@
>  #include <asm/pgtable.h>
>  #include <asm/setup.h>
>
> -static char early_cmdline[COMMAND_LINE_SIZE];
> +#include "pi.h"
>
> -/*
> - * Declare the functions that are exported (but prefixed) here so that LLVM
> - * does not complain it lacks the 'static' keyword (which, if added, makes
> - * LLVM complain because the function is actually unused in this file).
> - */
> -u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
> -bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
> +static char early_cmdline[COMMAND_LINE_SIZE];
>
>  static char *get_early_cmdline(uintptr_t dtb_pa)
>  {
> diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
> index 899610e042ab..40ee299702bf 100644
> --- a/arch/riscv/kernel/pi/fdt_early.c
> +++ b/arch/riscv/kernel/pi/fdt_early.c
> @@ -3,12 +3,7 @@
>  #include <linux/init.h>
>  #include <linux/libfdt.h>
>
> -/*
> - * Declare the functions that are exported (but prefixed) here so that LLVM
> - * does not complain it lacks the 'static' keyword (which, if added, makes
> - * LLVM complain because the function is actually unused in this file).
> - */
> -u64 get_kaslr_seed(uintptr_t dtb_pa);
> +#include "pi.h"
>
>  u64 get_kaslr_seed(uintptr_t dtb_pa)
>  {
> diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
> new file mode 100644
> index 000000000000..65da99466baf
> --- /dev/null
> +++ b/arch/riscv/kernel/pi/pi.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _RISCV_PI_H_
> +#define _RISCV_PI_H_
> +
> +#include <linux/types.h>
> +
> +/*
> + * The folowing functions are exported (but prefixed) declare them here so

s/folowing/following

And it lacks something between "(but prefixed)" and "declare": what
was wrong with the initial comment?

> + * that LLVM does not complain it lacks the 'static' keyword (which, if
> + * added, makes LLVM complain because the function is unused).
> + */
> +
> +u64 get_kaslr_seed(uintptr_t dtb_pa);
> +bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
> +u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
> +
> +#endif /* _RISCV_PI_H_ */
> --
> 2.45.2
>

Suggested-by: Charlie Jenkins <charlie@rivosinc.com>
Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex

[PATCH v2 3/3] RISC-V: Use Zkr to seed KASLR base address

[Jesse Taube]

Parse the device tree for Zkr in isa string.
If Zkr is present, use it to seed the kernel base address.

Signed-off-by: Jesse Taube <jesse@rivosinc.com>
---
 arch/riscv/kernel/pi/Makefile           |  2 +-
 arch/riscv/kernel/pi/archrandom_early.c | 30 ++++++++
 arch/riscv/kernel/pi/fdt_early.c        | 94 +++++++++++++++++++++++++
 arch/riscv/kernel/pi/pi.h               |  3 +
 arch/riscv/mm/init.c                    |  5 +-
 5 files changed, 132 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/kernel/pi/archrandom_early.c

diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
index 1ef7584be0c3..dba902f2a538 100644
--- a/arch/riscv/kernel/pi/Makefile
+++ b/arch/riscv/kernel/pi/Makefile
@@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
 $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
 	$(call if_changed_rule,cc_o_c)
 
-obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
+obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o
 extra-y		:= $(patsubst %.pi.o,%.o,$(obj-y))
diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
new file mode 100644
index 000000000000..c6261165e8a6
--- /dev/null
+++ b/arch/riscv/kernel/pi/archrandom_early.c
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <asm/csr.h>
+#include <linux/processor.h>
+
+#include "pi.h"
+
+/*
+ * To avoid rewriting code include asm/archrandom.h and create macros
+ * for the functions that won't be included.
+ */
+#undef riscv_has_extension_unlikely
+#define riscv_has_extension_likely(...) false
+#undef pr_err_once
+#define pr_err_once(...)
+
+#include <asm/archrandom.h>
+
+u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
+{
+	unsigned long seed = 0;
+
+	if (!early_isa_str((const void *)dtb_pa, "zkr"))
+		return 0;
+
+	if (!csr_seed_long(&seed))
+		return 0;
+
+	return seed;
+}
diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
index 40ee299702bf..ba76197b44d1 100644
--- a/arch/riscv/kernel/pi/fdt_early.c
+++ b/arch/riscv/kernel/pi/fdt_early.c
@@ -23,3 +23,97 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
 	*prop = 0;
 	return ret;
 }
+
+/* Based off of fdt_stringlist_contains */
+static int isa_string_contains(const char *strlist, int listlen, const char *str)
+{
+	int len = strlen(str);
+	const char *p;
+
+	while (listlen >= len) {
+		if (strncasecmp(str, strlist, len) == 0)
+			return 1;
+		p = memchr(strlist, '_', listlen);
+		if (!p)
+			p = memchr(strlist, '\0', listlen);
+		if (!p)
+			return 0; /* malformed strlist.. */
+		listlen -= (p - strlist) + 1;
+		strlist = p + 1;
+	}
+
+	return 0;
+}
+
+/* Based off of fdt_nodename_eq_ */
+static int fdt_node_name_eq(const void *fdt, int offset,
+			    const char *s)
+{
+	int olen;
+	int len = strlen(s);
+	const char *p = fdt_get_name(fdt, offset, &olen);
+
+	if (!p || olen < len)
+		/* short match */
+		return 0;
+
+	if (memcmp(p, s, len) != 0)
+		return 0;
+
+	if (p[len] == '\0')
+		return 1;
+	else if (!memchr(s, '@', len) && (p[len] == '@'))
+		return 1;
+	else
+		return 0;
+}
+
+/*
+ * Returns true if the extension is in the isa string
+ * Returns false if the extension is not found
+ */
+static bool get_ext_named(const void *fdt, int node, const char *name)
+{
+	const void *prop;
+	int len;
+
+	prop = fdt_getprop(fdt, node, "riscv,isa-base", &len);
+	if (prop && isa_string_contains(prop, len, name))
+		return true;
+
+	prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
+	if (prop && isa_string_contains(prop, len, name))
+		return true;
+
+	prop = fdt_getprop(fdt, node, "riscv,isa", &len);
+	if (prop && isa_string_contains(prop, len, name))
+		return true;
+
+	return false;
+}
+
+/*
+ * Returns true if the extension is in the isa string on all cpus
+ * Returns false if the extension is not found
+ */
+bool early_isa_str(const void *fdt, const char *ext_name)
+{
+	int node, parent;
+	bool ret = false;
+
+	parent = fdt_path_offset(fdt, "/cpus");
+	if (parent < 0)
+		return false;
+
+	fdt_for_each_subnode(node, fdt, parent) {
+		if (!fdt_node_name_eq(fdt, node, "cpu"))
+			continue;
+
+		if (!get_ext_named(fdt, node, ext_name))
+			return false;
+
+		ret = true;
+	}
+
+	return ret;
+}
diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
index 65da99466baf..26e7e5f84a30 100644
--- a/arch/riscv/kernel/pi/pi.h
+++ b/arch/riscv/kernel/pi/pi.h
@@ -4,6 +4,8 @@
 
 #include <linux/types.h>
 
+bool early_isa_str(const void *fdt, const char *ext_name);
+
 /*
  * The folowing functions are exported (but prefixed) declare them here so
  * that LLVM does not complain it lacks the 'static' keyword (which, if
@@ -11,6 +13,7 @@
  */
 
 u64 get_kaslr_seed(uintptr_t dtb_pa);
+u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
 bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
 u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
 
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index 9940171c79f0..bfb068dc4a64 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
 #ifdef CONFIG_RANDOMIZE_BASE
 extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
 extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
+extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
 
 static int __init print_nokaslr(char *p)
 {
@@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
 
 #ifdef CONFIG_RANDOMIZE_BASE
 	if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
-		u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
+		u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
 		u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
 		u32 nr_pos;
 
+		if (kaslr_seed == 0)
+			kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
 		/*
 		 * Compute the number of positions available: we are limited
 		 * by the early page table that only has one PUD and we must
-- 
2.45.2

Re: [PATCH v2 3/3] RISC-V: Use Zkr to seed KASLR base address

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 8878 bytes --]

Hey Jesse,

On Wed, Jun 26, 2024 at 01:16:52PM -0400, Jesse Taube wrote:
> Parse the device tree for Zkr in isa string.
> If Zkr is present, use it to seed the kernel base address.
> 
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
>  arch/riscv/kernel/pi/Makefile           |  2 +-
>  arch/riscv/kernel/pi/archrandom_early.c | 30 ++++++++
>  arch/riscv/kernel/pi/fdt_early.c        | 94 +++++++++++++++++++++++++
>  arch/riscv/kernel/pi/pi.h               |  3 +
>  arch/riscv/mm/init.c                    |  5 +-
>  5 files changed, 132 insertions(+), 2 deletions(-)
>  create mode 100644 arch/riscv/kernel/pi/archrandom_early.c
> 
> diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
> index 1ef7584be0c3..dba902f2a538 100644
> --- a/arch/riscv/kernel/pi/Makefile
> +++ b/arch/riscv/kernel/pi/Makefile
> @@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
>  $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
>  	$(call if_changed_rule,cc_o_c)
>  
> -obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
> +obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o
>  extra-y		:= $(patsubst %.pi.o,%.o,$(obj-y))
> diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
> new file mode 100644
> index 000000000000..c6261165e8a6
> --- /dev/null
> +++ b/arch/riscv/kernel/pi/archrandom_early.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <asm/csr.h>
> +#include <linux/processor.h>
> +
> +#include "pi.h"
> +
> +/*
> + * To avoid rewriting code include asm/archrandom.h and create macros
> + * for the functions that won't be included.
> + */
> +#undef riscv_has_extension_unlikely
> +#define riscv_has_extension_likely(...) false
> +#undef pr_err_once
> +#define pr_err_once(...)
> +
> +#include <asm/archrandom.h>
> +
> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
> +{
> +	unsigned long seed = 0;
> +
> +	if (!early_isa_str((const void *)dtb_pa, "zkr"))
> +		return 0;
> +
> +	if (!csr_seed_long(&seed))
> +		return 0;
> +
> +	return seed;
> +}
> diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
> index 40ee299702bf..ba76197b44d1 100644
> --- a/arch/riscv/kernel/pi/fdt_early.c
> +++ b/arch/riscv/kernel/pi/fdt_early.c
> @@ -23,3 +23,97 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
>  	*prop = 0;
>  	return ret;
>  }
> +
> +/* Based off of fdt_stringlist_contains */
> +static int isa_string_contains(const char *strlist, int listlen, const char *str)

The variable names here are needlessly confusing IMO. The function also
returns a bool, not an int.

> +{
> +	int len = strlen(str);
> +	const char *p;
> +
> +	while (listlen >= len) {
> +		if (strncasecmp(str, strlist, len) == 0)
> +			return 1;

How does this handle searching a devicetree containing "rv64ima_zksed_zkr"
for the extension zks? Hint: https://godbolt.org/z/YfhTqe54e
I think this works for fdt_stringlist_contains() because it also
compares the null chars - which you're not doing so I think this also
brakes for something like riscv,isa-extensions = "rv64ima\0zksed\0zkr"
while searching for zks.

> +		p = memchr(strlist, '_', listlen);

Or how does this handle searching "rv64imafdczkr" for zkr? It's gonna
run right off the end of the string without finding anything, right?

Handling "riscv,isa" is not trivial, but at least the search for extension
approach here skips dealing with some of what has to be done in the "real"
parser with the version numbers...

Maybe we just say screw "riscv,isa", as it's deprecated anyway, and only
add this new feature for "riscv,isa-extensions" which is far simpler to
parse and can be done using off-the-shelf fdt functions?

If not, then I think we should use fdt_stringlist_contains verbatim for
"riscv,isa-extensions" and introduce a custom function for "riscv,isa"
only.

> +		if (!p)
> +			p = memchr(strlist, '\0', listlen);
> +		if (!p)
> +			return 0; /* malformed strlist.. */
> +		listlen -= (p - strlist) + 1;
> +		strlist = p + 1;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Based off of fdt_nodename_eq_ */

Why can't we just use fdt_nodename_eq?

> +static int fdt_node_name_eq(const void *fdt, int offset,
> +			    const char *s)
> +{
> +	int olen;
> +	int len = strlen(s);
> +	const char *p = fdt_get_name(fdt, offset, &olen);
> +
> +	if (!p || olen < len)
> +		/* short match */
> +		return 0;
> +
> +	if (memcmp(p, s, len) != 0)
> +		return 0;
> +
> +	if (p[len] == '\0')
> +		return 1;
> +	else if (!memchr(s, '@', len) && (p[len] == '@'))
> +		return 1;
> +	else
> +		return 0;
> +}
> +
> +/*
> + * Returns true if the extension is in the isa string
> + * Returns false if the extension is not found
> + */
> +static bool get_ext_named(const void *fdt, int node, const char *name)

Could you rename this function please? Having something named "get" that
returns a bool, and not an "ext_named" is odd - and it'd be self
explanatory in that case. Maybe it can just be moved into the sole caller
and isn't needed?

> +{
> +	const void *prop;
> +	int len;
> +
> +	prop = fdt_getprop(fdt, node, "riscv,isa-base", &len);
> +	if (prop && isa_string_contains(prop, len, name))
> +		return true;

This shouldn't be here, there'll not be an extension in this property.

> +	prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
> +	if (prop && isa_string_contains(prop, len, name))
> +		return true;
> +
> +	prop = fdt_getprop(fdt, node, "riscv,isa", &len);
> +	if (prop && isa_string_contains(prop, len, name))
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * Returns true if the extension is in the isa string on all cpus

Shouldn't we only be checking CPUs that are not disabled or reserved,
rather than all CPUs? To use Zkr for KASLR this is kinda irrelevant
since really we only care about whether or not the boot CPU has Zkr,
but in general we only want to consider CPUs that we can actually use.
For example, if you did this for FPU support with mpfs.dtsi, you'd get
told that the F/D extensions were not present cos hart 0 doesn't have
them, even though it's disabled and will not be used by Linux.

> + * Returns false if the extension is not found
> + */
> +bool early_isa_str(const void *fdt, const char *ext_name)

Could you try to match the naming of the stuff used outside of pi?
Maybe early_isa_ext_available()?

Thanks for the update,
Conor.

> +{
> +	int node, parent;
> +	bool ret = false;
> +
> +	parent = fdt_path_offset(fdt, "/cpus");
> +	if (parent < 0)
> +		return false;
> +
> +	fdt_for_each_subnode(node, fdt, parent) {
> +		if (!fdt_node_name_eq(fdt, node, "cpu"))
> +			continue;
> +
> +		if (!get_ext_named(fdt, node, ext_name))
> +			return false;
> +
> +		ret = true;
> +	}
> +
> +	return ret;
> +}
> diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
> index 65da99466baf..26e7e5f84a30 100644
> --- a/arch/riscv/kernel/pi/pi.h
> +++ b/arch/riscv/kernel/pi/pi.h
> @@ -4,6 +4,8 @@
>  
>  #include <linux/types.h>
>  
> +bool early_isa_str(const void *fdt, const char *ext_name);
> +
>  /*
>   * The folowing functions are exported (but prefixed) declare them here so
>   * that LLVM does not complain it lacks the 'static' keyword (which, if
> @@ -11,6 +13,7 @@
>   */
>  
>  u64 get_kaslr_seed(uintptr_t dtb_pa);
> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>  bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>  u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
>  
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 9940171c79f0..bfb068dc4a64 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
>  #ifdef CONFIG_RANDOMIZE_BASE
>  extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>  extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
> +extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>  
>  static int __init print_nokaslr(char *p)
>  {
> @@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>  
>  #ifdef CONFIG_RANDOMIZE_BASE
>  	if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
> -		u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
> +		u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
>  		u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
>  		u32 nr_pos;
>  
> +		if (kaslr_seed == 0)
> +			kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
>  		/*
>  		 * Compute the number of positions available: we are limited
>  		 * by the early page table that only has one PUD and we must
> -- 
> 2.45.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] RISC-V: Use Zkr to seed KASLR base address

[Jesse Taube]

On 6/27/24 07:45, Conor Dooley wrote:
> Hey Jesse,
> 
> On Wed, Jun 26, 2024 at 01:16:52PM -0400, Jesse Taube wrote:
>> Parse the device tree for Zkr in isa string.
>> If Zkr is present, use it to seed the kernel base address.
>>
>> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
>> ---
>>   arch/riscv/kernel/pi/Makefile           |  2 +-
>>   arch/riscv/kernel/pi/archrandom_early.c | 30 ++++++++
>>   arch/riscv/kernel/pi/fdt_early.c        | 94 +++++++++++++++++++++++++
>>   arch/riscv/kernel/pi/pi.h               |  3 +
>>   arch/riscv/mm/init.c                    |  5 +-
>>   5 files changed, 132 insertions(+), 2 deletions(-)
>>   create mode 100644 arch/riscv/kernel/pi/archrandom_early.c
>>
>> diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
>> index 1ef7584be0c3..dba902f2a538 100644
>> --- a/arch/riscv/kernel/pi/Makefile
>> +++ b/arch/riscv/kernel/pi/Makefile
>> @@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
>>   $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
>>   	$(call if_changed_rule,cc_o_c)
>>   
>> -obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
>> +obj-y		:= cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o
>>   extra-y		:= $(patsubst %.pi.o,%.o,$(obj-y))
>> diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
>> new file mode 100644
>> index 000000000000..c6261165e8a6
>> --- /dev/null
>> +++ b/arch/riscv/kernel/pi/archrandom_early.c
>> @@ -0,0 +1,30 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +
>> +#include <asm/csr.h>
>> +#include <linux/processor.h>
>> +
>> +#include "pi.h"
>> +
>> +/*
>> + * To avoid rewriting code include asm/archrandom.h and create macros
>> + * for the functions that won't be included.
>> + */
>> +#undef riscv_has_extension_unlikely
>> +#define riscv_has_extension_likely(...) false
>> +#undef pr_err_once
>> +#define pr_err_once(...)
>> +
>> +#include <asm/archrandom.h>
>> +
>> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
>> +{
>> +	unsigned long seed = 0;
>> +
>> +	if (!early_isa_str((const void *)dtb_pa, "zkr"))
>> +		return 0;
>> +
>> +	if (!csr_seed_long(&seed))
>> +		return 0;
>> +
>> +	return seed;
>> +}
>> diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
>> index 40ee299702bf..ba76197b44d1 100644
>> --- a/arch/riscv/kernel/pi/fdt_early.c
>> +++ b/arch/riscv/kernel/pi/fdt_early.c
>> @@ -23,3 +23,97 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
>>   	*prop = 0;
>>   	return ret;
>>   }
>> +
>> +/* Based off of fdt_stringlist_contains */
>> +static int isa_string_contains(const char *strlist, int listlen, const char *str)
> 
> The variable names here are needlessly confusing IMO. The function also
> returns a bool, not an int.
> 
>> +{
>> +	int len = strlen(str);
>> +	const char *p;
>> +
>> +	while (listlen >= len) {
>> +		if (strncasecmp(str, strlist, len) == 0)
>> +			return 1;
> 
> How does this handle searching a devicetree containing "rv64ima_zksed_zkr"
> for the extension zks? Hint: https://godbolt.org/z/YfhTqe54e
> I think this works for fdt_stringlist_contains() because it also
> compares the null chars - which you're not doing so I think this also
> brakes for something like riscv,isa-extensions = "rv64ima\0zksed\0zkr"
> while searching for zks.
> 
>> +		p = memchr(strlist, '_', listlen);
> 
> Or how does this handle searching "rv64imafdczkr" for zkr? It's gonna
> run right off the end of the string without finding anything, right?

Yes...

Is that a valid isa,string? I will try to copy how cpufeature.c as close 
as posible.

> 
> Handling "riscv,isa" is not trivial, but at least the search for extension
> approach here skips dealing with some of what has to be done in the "real"
> parser with the version numbers...
> 
> Maybe we just say screw "riscv,isa", as it's deprecated anyway, and only.
I think it's important to have.

> add this new feature for "riscv,isa-extensions" which is far simpler to
> parse and can be done using off-the-shelf fdt functions?
> 
> If not, then I think we should use fdt_stringlist_contains verbatim for
> "riscv,isa-extensions".

Ok I had a notion that riscv,isa-extensions could be upercase they 
cant/wont. I will use fdt_stringlist_contains.

> and introduce a custom function for "riscv,isa"
> only.

That was my original thought I will do that.

> 
>> +		if (!p)
>> +			p = memchr(strlist, '\0', listlen);
>> +		if (!p)
>> +			return 0; /* malformed strlist.. */
>> +		listlen -= (p - strlist) + 1;
>> +		strlist = p + 1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/* Based off of fdt_nodename_eq_ */
> 
> Why can't we just use fdt_nodename_eq?

Because fdt_nodename_eq_ is static.
I will change the comment to "copy of fdt_nodename_eq_".
Oddly there is `of_node_name_eq` but not `fdt_nodename_eq`

> 
>> +static int fdt_node_name_eq(const void *fdt, int offset,
>> +			    const char *s)
>> +{
>> +	int olen;
>> +	int len = strlen(s);
>> +	const char *p = fdt_get_name(fdt, offset, &olen);
>> +
>> +	if (!p || olen < len)
>> +		/* short match */
>> +		return 0;
>> +
>> +	if (memcmp(p, s, len) != 0)
>> +		return 0;
>> +
>> +	if (p[len] == '\0')
>> +		return 1;
>> +	else if (!memchr(s, '@', len) && (p[len] == '@'))
>> +		return 1;
>> +	else
>> +		return 0;
>> +}
>> +
>> +/*
>> + * Returns true if the extension is in the isa string
>> + * Returns false if the extension is not found
>> + */
>> +static bool get_ext_named(const void *fdt, int node, const char *name)
> 
> Could you rename this function please?

Yes.

> Having something named "get" that
> returns a bool, and not an "ext_named" is odd - and it'd be self
> explanatory in that case. Maybe it can just be moved into the sole caller
> and isn't needed?

I have it as a seperate function if in the future there needed to be a 
per cpu check.

>> +{
>> +	const void *prop;
>> +	int len;
>> +
>> +	prop = fdt_getprop(fdt, node, "riscv,isa-base", &len);
>> +	if (prop && isa_string_contains(prop, len, name))
>> +		return true;
> 
> This shouldn't be here, there'll not be an extension in this property.
Will remove.

> 
>> +	prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
>> +	if (prop && isa_string_contains(prop, len, name))
>> +		return true;
>> +
>> +	prop = fdt_getprop(fdt, node, "riscv,isa", &len);
>> +	if (prop && isa_string_contains(prop, len, name))
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +/*
>> + * Returns true if the extension is in the isa string on all cpus
> 
> Shouldn't we only be checking CPUs that are not disabled or reserved,
> rather than all CPUs?

Its way easier to just check all the cpus rather then make sure we are 
runing on one thats has the extention. I will add a continue for 
dissabled/reserved cpus.

> To use Zkr for KASLR this is kinda irrelevant
> since really we only care about whether or not the boot CPU has Zkr,
> but in general we only want to consider CPUs that we can actually use.
> For example, if you did this for FPU support with mpfs.dtsi, you'd get
> told that the F/D extensions were not present cos hart 0

Can we assume that the boot hart is always 0?

> doesn't have
> them, even though it's disabled and will not be used by Linux.
> 
>> + * Returns false if the extension is not found
>> + */
>> +bool early_isa_str(const void *fdt, const char *ext_name)
> 
> Could you try to match the naming of the stuff used outside of pi?
> Maybe early_isa_ext_available()?

Yes.

Thanks,
Jesse Taube

> 
> Thanks for the update,
> Conor.
> 
>> +{
>> +	int node, parent;
>> +	bool ret = false;
>> +
>> +	parent = fdt_path_offset(fdt, "/cpus");
>> +	if (parent < 0)
>> +		return false;
>> +
>> +	fdt_for_each_subnode(node, fdt, parent) {
>> +		if (!fdt_node_name_eq(fdt, node, "cpu"))
>> +			continue;
>> +
>> +		if (!get_ext_named(fdt, node, ext_name))
>> +			return false;
>> +
>> +		ret = true;
>> +	}
>> +
>> +	return ret;
>> +}
>> diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
>> index 65da99466baf..26e7e5f84a30 100644
>> --- a/arch/riscv/kernel/pi/pi.h
>> +++ b/arch/riscv/kernel/pi/pi.h
>> @@ -4,6 +4,8 @@
>>   
>>   #include <linux/types.h>
>>   
>> +bool early_isa_str(const void *fdt, const char *ext_name);
>> +
>>   /*
>>    * The folowing functions are exported (but prefixed) declare them here so
>>    * that LLVM does not complain it lacks the 'static' keyword (which, if
>> @@ -11,6 +13,7 @@
>>    */
>>   
>>   u64 get_kaslr_seed(uintptr_t dtb_pa);
>> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>>   bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>>   u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
>>   
>> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>> index 9940171c79f0..bfb068dc4a64 100644
>> --- a/arch/riscv/mm/init.c
>> +++ b/arch/riscv/mm/init.c
>> @@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
>>   #ifdef CONFIG_RANDOMIZE_BASE
>>   extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>>   extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
>> +extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>>   
>>   static int __init print_nokaslr(char *p)
>>   {
>> @@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>>   
>>   #ifdef CONFIG_RANDOMIZE_BASE
>>   	if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
>> -		u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
>> +		u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
>>   		u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
>>   		u32 nr_pos;
>>   
>> +		if (kaslr_seed == 0)
>> +			kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
>>   		/*
>>   		 * Compute the number of positions available: we are limited
>>   		 * by the early page table that only has one PUD and we must
>> -- 
>> 2.45.2
>>

Re: [PATCH v2 3/3] RISC-V: Use Zkr to seed KASLR base address

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4320 bytes --]

On Thu, Jun 27, 2024 at 01:55:19PM -0400, Jesse Taube wrote:
> On 6/27/24 07:45, Conor Dooley wrote:
> > On Wed, Jun 26, 2024 at 01:16:52PM -0400, Jesse Taube wrote:

> > > +/* Based off of fdt_stringlist_contains */
> > > +static int isa_string_contains(const char *strlist, int listlen, const char *str)
> > > +{
> > > +	int len = strlen(str);
> > > +	const char *p;
> > > +
> > > +	while (listlen >= len) {
> > > +		if (strncasecmp(str, strlist, len) == 0)
> > > +			return 1;
> > 
> > How does this handle searching a devicetree containing "rv64ima_zksed_zkr"
> > for the extension zks? Hint: https://godbolt.org/z/YfhTqe54e
> > I think this works for fdt_stringlist_contains() because it also
> > compares the null chars - which you're not doing so I think this also
> > brakes for something like riscv,isa-extensions = "rv64ima\0zksed\0zkr"
> > while searching for zks.
> > 
> > > +		p = memchr(strlist, '_', listlen);
> > 
> > Or how does this handle searching "rv64imafdczkr" for zkr? It's gonna
> > run right off the end of the string without finding anything, right?
> 
> Yes...
> 
> Is that a valid isa,string?

It is. I wish I had just not allowed it, but I was more naive then and
figured we should allow whatever the spec did. Technically versioning of
the extension isn't allowed, but in the wild people do put it in, so I
believe that a parser shouldn't break when it encounters versioning,
even if the regex for the property doesn't permit them:
^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[0-9a-z])+)?(?:_[hsxz](?:[0-9a-z])+)*$

> I will try to copy how cpufeature.c as close as
> posible.

The comments there should be fairly understandable, but that parser has
a different goal to the one here, so you should be able to make things
simpler. I hope at least.

> > Handling "riscv,isa" is not trivial, but at least the search for extension
> > approach here skips dealing with some of what has to be done in the "real"
> > parser with the version numbers...
> > 
> > Maybe we just say screw "riscv,isa", as it's deprecated anyway, and only.
> I think it's important to have.
> 
> > add this new feature for "riscv,isa-extensions" which is far simpler to
> > parse and can be done using off-the-shelf fdt functions?
> > 
> > If not, then I think we should use fdt_stringlist_contains verbatim for
> > "riscv,isa-extensions".
> 
> Ok I had a notion that riscv,isa-extensions could be upercase they
> cant/wont. I will use fdt_stringlist_contains.
> 
> > and introduce a custom function for "riscv,isa"
> > only.
> 
> That was my original thought I will do that.
> 
> > 
> > > +		if (!p)
> > > +			p = memchr(strlist, '\0', listlen);
> > > +		if (!p)
> > > +			return 0; /* malformed strlist.. */
> > > +		listlen -= (p - strlist) + 1;
> > > +		strlist = p + 1;
> > > +	}
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/* Based off of fdt_nodename_eq_ */
> > 
> > Why can't we just use fdt_nodename_eq?
> 
> Because fdt_nodename_eq_ is static.
> I will change the comment to "copy of fdt_nodename_eq_".
> Oddly there is `of_node_name_eq` but not `fdt_nodename_eq`

of_node_name_eq comes from the kernel, fdt_nodename_eq comes from
libfdt. I figure the former cannot be used this early since we've not
extracted the dtb and parsed it yet...

> > > +/*
> > > + * Returns true if the extension is in the isa string on all cpus
> > 
> > Shouldn't we only be checking CPUs that are not disabled or reserved,
> > rather than all CPUs?
> 
> Its way easier to just check all the cpus rather then make sure we are
> runing on one thats has the extention. I will add a continue for
> dissabled/reserved cpus.
> 
> > To use Zkr for KASLR this is kinda irrelevant
> > since really we only care about whether or not the boot CPU has Zkr,
> > but in general we only want to consider CPUs that we can actually use.
> > For example, if you did this for FPU support with mpfs.dtsi, you'd get
> > told that the F/D extensions were not present cos hart 0
> 
> Can we assume that the boot hart is always 0?

Nah, You cannot assume that the boot hart is hart 0, in the example I gave
here hart 0 is not available to Linux.

> > doesn't have
> > them, even though it's disabled and will not be used by Linux.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/3] RISC-V: Use Zkr to seed KASLR base address

[Alexandre Ghiti]

Hi Jesse,

On Wed, Jun 26, 2024 at 7:17 PM Jesse Taube <jesse@rivosinc.com> wrote:
>
> Parse the device tree for Zkr in isa string.
> If Zkr is present, use it to seed the kernel base address.

It would be nice to explain why we finally decided to only use the Zkr
extension in !EFI platform, if @Ard Biesheuvel agrees, I guess you can
even quote him: "On an ACPI+EFI system, you have to be able to trust
the firmware. And you have to use the available abstractions.
Otherwise, there is no point."

>
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>
> ---
>  arch/riscv/kernel/pi/Makefile           |  2 +-
>  arch/riscv/kernel/pi/archrandom_early.c | 30 ++++++++
>  arch/riscv/kernel/pi/fdt_early.c        | 94 +++++++++++++++++++++++++
>  arch/riscv/kernel/pi/pi.h               |  3 +
>  arch/riscv/mm/init.c                    |  5 +-
>  5 files changed, 132 insertions(+), 2 deletions(-)
>  create mode 100644 arch/riscv/kernel/pi/archrandom_early.c
>
> diff --git a/arch/riscv/kernel/pi/Makefile b/arch/riscv/kernel/pi/Makefile
> index 1ef7584be0c3..dba902f2a538 100644
> --- a/arch/riscv/kernel/pi/Makefile
> +++ b/arch/riscv/kernel/pi/Makefile
> @@ -33,5 +33,5 @@ $(obj)/string.o: $(srctree)/lib/string.c FORCE
>  $(obj)/ctype.o: $(srctree)/lib/ctype.c FORCE
>         $(call if_changed_rule,cc_o_c)
>
> -obj-y          := cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o
> +obj-y          := cmdline_early.pi.o fdt_early.pi.o string.pi.o ctype.pi.o lib-fdt.pi.o lib-fdt_ro.pi.o archrandom_early.pi.o
>  extra-y                := $(patsubst %.pi.o,%.o,$(obj-y))
> diff --git a/arch/riscv/kernel/pi/archrandom_early.c b/arch/riscv/kernel/pi/archrandom_early.c
> new file mode 100644
> index 000000000000..c6261165e8a6
> --- /dev/null
> +++ b/arch/riscv/kernel/pi/archrandom_early.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <asm/csr.h>
> +#include <linux/processor.h>
> +
> +#include "pi.h"
> +
> +/*
> + * To avoid rewriting code include asm/archrandom.h and create macros
> + * for the functions that won't be included.
> + */
> +#undef riscv_has_extension_unlikely
> +#define riscv_has_extension_likely(...) false
> +#undef pr_err_once
> +#define pr_err_once(...)
> +
> +#include <asm/archrandom.h>
> +
> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa)
> +{
> +       unsigned long seed = 0;
> +
> +       if (!early_isa_str((const void *)dtb_pa, "zkr"))
> +               return 0;
> +
> +       if (!csr_seed_long(&seed))
> +               return 0;
> +
> +       return seed;
> +}
> diff --git a/arch/riscv/kernel/pi/fdt_early.c b/arch/riscv/kernel/pi/fdt_early.c
> index 40ee299702bf..ba76197b44d1 100644
> --- a/arch/riscv/kernel/pi/fdt_early.c
> +++ b/arch/riscv/kernel/pi/fdt_early.c
> @@ -23,3 +23,97 @@ u64 get_kaslr_seed(uintptr_t dtb_pa)
>         *prop = 0;
>         return ret;
>  }
> +
> +/* Based off of fdt_stringlist_contains */
> +static int isa_string_contains(const char *strlist, int listlen, const char *str)
> +{
> +       int len = strlen(str);
> +       const char *p;
> +
> +       while (listlen >= len) {
> +               if (strncasecmp(str, strlist, len) == 0)
> +                       return 1;
> +               p = memchr(strlist, '_', listlen);
> +               if (!p)
> +                       p = memchr(strlist, '\0', listlen);
> +               if (!p)
> +                       return 0; /* malformed strlist.. */
> +               listlen -= (p - strlist) + 1;
> +               strlist = p + 1;
> +       }
> +
> +       return 0;
> +}
> +
> +/* Based off of fdt_nodename_eq_ */
> +static int fdt_node_name_eq(const void *fdt, int offset,
> +                           const char *s)
> +{
> +       int olen;
> +       int len = strlen(s);
> +       const char *p = fdt_get_name(fdt, offset, &olen);
> +
> +       if (!p || olen < len)
> +               /* short match */
> +               return 0;
> +
> +       if (memcmp(p, s, len) != 0)
> +               return 0;
> +
> +       if (p[len] == '\0')
> +               return 1;
> +       else if (!memchr(s, '@', len) && (p[len] == '@'))
> +               return 1;
> +       else
> +               return 0;
> +}
> +
> +/*
> + * Returns true if the extension is in the isa string
> + * Returns false if the extension is not found
> + */
> +static bool get_ext_named(const void *fdt, int node, const char *name)
> +{
> +       const void *prop;
> +       int len;
> +
> +       prop = fdt_getprop(fdt, node, "riscv,isa-base", &len);
> +       if (prop && isa_string_contains(prop, len, name))
> +               return true;
> +
> +       prop = fdt_getprop(fdt, node, "riscv,isa-extensions", &len);
> +       if (prop && isa_string_contains(prop, len, name))
> +               return true;
> +
> +       prop = fdt_getprop(fdt, node, "riscv,isa", &len);
> +       if (prop && isa_string_contains(prop, len, name))
> +               return true;
> +
> +       return false;
> +}
> +
> +/*
> + * Returns true if the extension is in the isa string on all cpus
> + * Returns false if the extension is not found
> + */
> +bool early_isa_str(const void *fdt, const char *ext_name)
> +{
> +       int node, parent;
> +       bool ret = false;
> +
> +       parent = fdt_path_offset(fdt, "/cpus");
> +       if (parent < 0)
> +               return false;
> +
> +       fdt_for_each_subnode(node, fdt, parent) {
> +               if (!fdt_node_name_eq(fdt, node, "cpu"))
> +                       continue;
> +
> +               if (!get_ext_named(fdt, node, ext_name))
> +                       return false;
> +
> +               ret = true;
> +       }
> +
> +       return ret;
> +}
> diff --git a/arch/riscv/kernel/pi/pi.h b/arch/riscv/kernel/pi/pi.h
> index 65da99466baf..26e7e5f84a30 100644
> --- a/arch/riscv/kernel/pi/pi.h
> +++ b/arch/riscv/kernel/pi/pi.h
> @@ -4,6 +4,8 @@
>
>  #include <linux/types.h>
>
> +bool early_isa_str(const void *fdt, const char *ext_name);
> +
>  /*
>   * The folowing functions are exported (but prefixed) declare them here so
>   * that LLVM does not complain it lacks the 'static' keyword (which, if
> @@ -11,6 +13,7 @@
>   */
>
>  u64 get_kaslr_seed(uintptr_t dtb_pa);
> +u64 get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>  bool set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>  u64 set_satp_mode_from_cmdline(uintptr_t dtb_pa);
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index 9940171c79f0..bfb068dc4a64 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -1025,6 +1025,7 @@ static void __init pt_ops_set_late(void)
>  #ifdef CONFIG_RANDOMIZE_BASE
>  extern bool __init __pi_set_nokaslr_from_cmdline(uintptr_t dtb_pa);
>  extern u64 __init __pi_get_kaslr_seed(uintptr_t dtb_pa);
> +extern u64 __init __pi_get_kaslr_seed_zkr(const uintptr_t dtb_pa);
>
>  static int __init print_nokaslr(char *p)
>  {
> @@ -1045,10 +1046,12 @@ asmlinkage void __init setup_vm(uintptr_t dtb_pa)
>
>  #ifdef CONFIG_RANDOMIZE_BASE
>         if (!__pi_set_nokaslr_from_cmdline(dtb_pa)) {
> -               u64 kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
> +               u64 kaslr_seed = __pi_get_kaslr_seed_zkr(dtb_pa);
>                 u32 kernel_size = (uintptr_t)(&_end) - (uintptr_t)(&_start);
>                 u32 nr_pos;
>
> +               if (kaslr_seed == 0)
> +                       kaslr_seed = __pi_get_kaslr_seed(dtb_pa);
>                 /*
>                  * Compute the number of positions available: we are limited
>                  * by the early page table that only has one PUD and we must
> --
> 2.45.2
>

Thanks,

Alex

Re: [PATCH v2 1/3] RISC-V: pi: Force hidden visibility for all symbol references

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 523 bytes --]

On Wed, Jun 26, 2024 at 01:16:50PM -0400, Jesse Taube wrote:
> Eliminate all GOT entries in the .pi section, by forcing hidden
> visibility for all symbol references, which informs the compiler that
> such references will be resolved at link time without the need for
> allocating GOT entries.
> 
> Include linux/hidden.h in Makefile, like arm64, for the
> hidden visibility attribute.
> 
> Signed-off-by: Jesse Taube <jesse@rivosinc.com>

Did you forget to add --cover-letter to your format-patch incantation?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] RISC-V: pi: Force hidden visibility for all symbol references

[Alexandre Ghiti]

Hi Jesse,

On Thu, Jun 27, 2024 at 12:35 PM Conor Dooley
<conor.dooley@microchip.com> wrote:
>
> On Wed, Jun 26, 2024 at 01:16:50PM -0400, Jesse Taube wrote:
> > Eliminate all GOT entries in the .pi section, by forcing hidden
> > visibility for all symbol references, which informs the compiler that
> > such references will be resolved at link time without the need for
> > allocating GOT entries.
> >
> > Include linux/hidden.h in Makefile, like arm64, for the
> > hidden visibility attribute.
> >
> > Signed-off-by: Jesse Taube <jesse@rivosinc.com>
>
> Did you forget to add --cover-letter to your format-patch incantation?

I admittedly don't understand all the details but it makes the GOT
entry disappear and arm64 uses it too, so:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex