[PATCH bpf-next] riscv, bpf: fix some compiler error

[PATCH bpf-next] riscv, bpf: fix some compiler error

[Tong Tiangen]

This patch fix two compile errors:
1. when CONFIG_BPF_JIT and CONFIG_ARCH_32I is open, There is the following
compiler error:
  error: undefined symbol: rv_bpf_fixup_exception

2. when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following
compiler error (W=1):
  error: no previous prototype for 'rv_bpf_fixup_exception'

In this patch, asm/extable.h is introduced,  the rv_bpf_fixup_exception
function declaration is added to this file. in addition, the definition of
exception_table_entry is moved from asm-generic/extable.h to this file.

Fixes: 252c765bd764 ("riscv, bpf: Add BPF exception tables")
Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
---
 arch/riscv/include/asm/Kbuild    |  1 -
 arch/riscv/include/asm/extable.h | 49 ++++++++++++++++++++++++++++++++
 arch/riscv/include/asm/uaccess.h | 13 ---------
 arch/riscv/mm/extable.c          |  8 +-----
 4 files changed, 50 insertions(+), 21 deletions(-)
 create mode 100644 arch/riscv/include/asm/extable.h

diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index 445ccc97305a..57b86fd9916c 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 generic-y += early_ioremap.h
-generic-y += extable.h
 generic-y += flat.h
 generic-y += kvm_para.h
 generic-y += user.h
diff --git a/arch/riscv/include/asm/extable.h b/arch/riscv/include/asm/extable.h
new file mode 100644
index 000000000000..aa0332b053fb
--- /dev/null
+++ b/arch/riscv/include/asm/extable.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_EXTABLE_H
+#define __ASM_EXTABLE_H
+
+/*
+ * The exception table consists of pairs of addresses: the first is the
+ * address of an instruction that is allowed to fault, and the second is
+ * the address at which the program should continue.  No registers are
+ * modified, so it is entirely up to the continuation code to figure out
+ * what to do.
+ *
+ * All the routines below use bits of fixup code that are out of line
+ * with the main instruction path.  This means when everything is well,
+ * we don't even have to jump over them.  Further, they do not intrude
+ * on our cache or tlb entries.
+ */
+struct exception_table_entry {
+	unsigned long insn, fixup;
+};
+
+struct pt_regs;
+int fixup_exception(struct pt_regs *regs);
+
+#if defined(CONFIG_MMU)
+static inline bool rv_in_bpf_jit(struct pt_regs *regs)
+{
+	if (!IS_ENABLED(CONFIG_BPF_JIT) || !IS_ENABLED(CONFIG_64BIT))
+		return false;
+
+	return regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END;
+}
+#else
+static inline bool rv_in_bpf_jit(struct pt_regs *regs)
+{
+	return false;
+}
+#endif
+
+#if defined(CONFIG_BPF_JIT) && defined(CONFIG_64BIT)
+int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
+#else
+static inline int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
+					 struct pt_regs *regs)
+{
+	return 0;
+}
+#endif
+
+#endif
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index f314ff44c48d..96ea91dc0e9c 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -56,19 +56,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
 	return size <= TASK_SIZE && addr <= TASK_SIZE - size;
 }
 
-/*
- * The exception table consists of pairs of addresses: the first is the
- * address of an instruction that is allowed to fault, and the second is
- * the address at which the program should continue.  No registers are
- * modified, so it is entirely up to the continuation code to figure out
- * what to do.
- *
- * All the routines below use bits of fixup code that are out of line
- * with the main instruction path.  This means when everything is well,
- * we don't even have to jump over them.  Further, they do not intrude
- * on our cache or tlb entries.
- */
-
 #define __LSW	0
 #define __MSW	1
 
diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
index 18bf338303b6..264f465db5bb 100644
--- a/arch/riscv/mm/extable.c
+++ b/arch/riscv/mm/extable.c
@@ -11,10 +11,6 @@
 #include <linux/module.h>
 #include <linux/uaccess.h>
 
-#ifdef CONFIG_BPF_JIT
-int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
-#endif
-
 int fixup_exception(struct pt_regs *regs)
 {
 	const struct exception_table_entry *fixup;
@@ -23,10 +19,8 @@ int fixup_exception(struct pt_regs *regs)
 	if (!fixup)
 		return 0;
 
-#ifdef CONFIG_BPF_JIT
-	if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
+	if (rv_in_bpf_jit(regs))
 		return rv_bpf_fixup_exception(fixup, regs);
-#endif
 
 	regs->epc = fixup->fixup;
 	return 1;
-- 
2.25.1

[PATCH bpf-next] riscv, bpf: fix some compiler error

[Tong Tiangen]

This patch fix two compile errors:
1. when CONFIG_BPF_JIT and CONFIG_ARCH_32I is open, There is the following
compiler error:
  error: undefined symbol: rv_bpf_fixup_exception

2. when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following
compiler error (W=1):
  error: no previous prototype for 'rv_bpf_fixup_exception'

In this patch, asm/extable.h is introduced,  the rv_bpf_fixup_exception
function declaration is added to this file. in addition, the definition of
exception_table_entry is moved from asm-generic/extable.h to this file.

Fixes: 252c765bd764 ("riscv, bpf: Add BPF exception tables")
Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
---
 arch/riscv/include/asm/Kbuild    |  1 -
 arch/riscv/include/asm/extable.h | 49 ++++++++++++++++++++++++++++++++
 arch/riscv/include/asm/uaccess.h | 13 ---------
 arch/riscv/mm/extable.c          |  8 +-----
 4 files changed, 50 insertions(+), 21 deletions(-)
 create mode 100644 arch/riscv/include/asm/extable.h

diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
index 445ccc97305a..57b86fd9916c 100644
--- a/arch/riscv/include/asm/Kbuild
+++ b/arch/riscv/include/asm/Kbuild
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 generic-y += early_ioremap.h
-generic-y += extable.h
 generic-y += flat.h
 generic-y += kvm_para.h
 generic-y += user.h
diff --git a/arch/riscv/include/asm/extable.h b/arch/riscv/include/asm/extable.h
new file mode 100644
index 000000000000..aa0332b053fb
--- /dev/null
+++ b/arch/riscv/include/asm/extable.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_EXTABLE_H
+#define __ASM_EXTABLE_H
+
+/*
+ * The exception table consists of pairs of addresses: the first is the
+ * address of an instruction that is allowed to fault, and the second is
+ * the address at which the program should continue.  No registers are
+ * modified, so it is entirely up to the continuation code to figure out
+ * what to do.
+ *
+ * All the routines below use bits of fixup code that are out of line
+ * with the main instruction path.  This means when everything is well,
+ * we don't even have to jump over them.  Further, they do not intrude
+ * on our cache or tlb entries.
+ */
+struct exception_table_entry {
+	unsigned long insn, fixup;
+};
+
+struct pt_regs;
+int fixup_exception(struct pt_regs *regs);
+
+#if defined(CONFIG_MMU)
+static inline bool rv_in_bpf_jit(struct pt_regs *regs)
+{
+	if (!IS_ENABLED(CONFIG_BPF_JIT) || !IS_ENABLED(CONFIG_64BIT))
+		return false;
+
+	return regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END;
+}
+#else
+static inline bool rv_in_bpf_jit(struct pt_regs *regs)
+{
+	return false;
+}
+#endif
+
+#if defined(CONFIG_BPF_JIT) && defined(CONFIG_64BIT)
+int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
+#else
+static inline int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
+					 struct pt_regs *regs)
+{
+	return 0;
+}
+#endif
+
+#endif
diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
index f314ff44c48d..96ea91dc0e9c 100644
--- a/arch/riscv/include/asm/uaccess.h
+++ b/arch/riscv/include/asm/uaccess.h
@@ -56,19 +56,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
 	return size <= TASK_SIZE && addr <= TASK_SIZE - size;
 }
 
-/*
- * The exception table consists of pairs of addresses: the first is the
- * address of an instruction that is allowed to fault, and the second is
- * the address at which the program should continue.  No registers are
- * modified, so it is entirely up to the continuation code to figure out
- * what to do.
- *
- * All the routines below use bits of fixup code that are out of line
- * with the main instruction path.  This means when everything is well,
- * we don't even have to jump over them.  Further, they do not intrude
- * on our cache or tlb entries.
- */
-
 #define __LSW	0
 #define __MSW	1
 
diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
index 18bf338303b6..264f465db5bb 100644
--- a/arch/riscv/mm/extable.c
+++ b/arch/riscv/mm/extable.c
@@ -11,10 +11,6 @@
 #include <linux/module.h>
 #include <linux/uaccess.h>
 
-#ifdef CONFIG_BPF_JIT
-int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
-#endif
-
 int fixup_exception(struct pt_regs *regs)
 {
 	const struct exception_table_entry *fixup;
@@ -23,10 +19,8 @@ int fixup_exception(struct pt_regs *regs)
 	if (!fixup)
 		return 0;
 
-#ifdef CONFIG_BPF_JIT
-	if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
+	if (rv_in_bpf_jit(regs))
 		return rv_bpf_fixup_exception(fixup, regs);
-#endif
 
 	regs->epc = fixup->fixup;
 	return 1;
-- 
2.25.1


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

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[Björn Töpel]

On Tue, 2 Nov 2021 at 15:40, Tong Tiangen <tongtiangen@huawei.com> wrote:
>
> This patch fix two compile errors:
> 1. when CONFIG_BPF_JIT and CONFIG_ARCH_32I is open, There is the following
> compiler error:
>   error: undefined symbol: rv_bpf_fixup_exception
>

Good catch for the RV32!

> 2. when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following
> compiler error (W=1):
>   error: no previous prototype for 'rv_bpf_fixup_exception'
>
> In this patch, asm/extable.h is introduced,  the rv_bpf_fixup_exception
> function declaration is added to this file. in addition, the definition of
> exception_table_entry is moved from asm-generic/extable.h to this file.
>

This is way too complicated. More below.

> Fixes: 252c765bd764 ("riscv, bpf: Add BPF exception tables")
> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
> ---
>  arch/riscv/include/asm/Kbuild    |  1 -
>  arch/riscv/include/asm/extable.h | 49 ++++++++++++++++++++++++++++++++
>  arch/riscv/include/asm/uaccess.h | 13 ---------
>  arch/riscv/mm/extable.c          |  8 +-----
>  4 files changed, 50 insertions(+), 21 deletions(-)
>  create mode 100644 arch/riscv/include/asm/extable.h
>
> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
> index 445ccc97305a..57b86fd9916c 100644
> --- a/arch/riscv/include/asm/Kbuild
> +++ b/arch/riscv/include/asm/Kbuild
> @@ -1,6 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  generic-y += early_ioremap.h
> -generic-y += extable.h
>  generic-y += flat.h
>  generic-y += kvm_para.h
>  generic-y += user.h
> diff --git a/arch/riscv/include/asm/extable.h b/arch/riscv/include/asm/extable.h
> new file mode 100644
> index 000000000000..aa0332b053fb
> --- /dev/null
> +++ b/arch/riscv/include/asm/extable.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_EXTABLE_H
> +#define __ASM_EXTABLE_H
> +
> +/*
> + * The exception table consists of pairs of addresses: the first is the
> + * address of an instruction that is allowed to fault, and the second is
> + * the address at which the program should continue.  No registers are
> + * modified, so it is entirely up to the continuation code to figure out
> + * what to do.
> + *
> + * All the routines below use bits of fixup code that are out of line
> + * with the main instruction path.  This means when everything is well,
> + * we don't even have to jump over them.  Further, they do not intrude
> + * on our cache or tlb entries.
> + */
> +struct exception_table_entry {
> +       unsigned long insn, fixup;
> +};
> +
> +struct pt_regs;
> +int fixup_exception(struct pt_regs *regs);
> +
> +#if defined(CONFIG_MMU)
> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
> +{
> +       if (!IS_ENABLED(CONFIG_BPF_JIT) || !IS_ENABLED(CONFIG_64BIT))
> +               return false;
> +
> +       return regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END;
> +}
> +#else
> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
> +{
> +       return false;
> +}
> +#endif
> +
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_64BIT)
> +int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
> +#else
> +static inline int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
> +                                        struct pt_regs *regs)
> +{
> +       return 0;
> +}
> +#endif
> +
> +#endif
> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> index f314ff44c48d..96ea91dc0e9c 100644
> --- a/arch/riscv/include/asm/uaccess.h
> +++ b/arch/riscv/include/asm/uaccess.h
> @@ -56,19 +56,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
>         return size <= TASK_SIZE && addr <= TASK_SIZE - size;
>  }
>
> -/*
> - * The exception table consists of pairs of addresses: the first is the
> - * address of an instruction that is allowed to fault, and the second is
> - * the address at which the program should continue.  No registers are
> - * modified, so it is entirely up to the continuation code to figure out
> - * what to do.
> - *
> - * All the routines below use bits of fixup code that are out of line
> - * with the main instruction path.  This means when everything is well,
> - * we don't even have to jump over them.  Further, they do not intrude
> - * on our cache or tlb entries.
> - */
> -
>  #define __LSW  0
>  #define __MSW  1
>
> diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
> index 18bf338303b6..264f465db5bb 100644
> --- a/arch/riscv/mm/extable.c
> +++ b/arch/riscv/mm/extable.c
> @@ -11,10 +11,6 @@
>  #include <linux/module.h>
>  #include <linux/uaccess.h>
>
> -#ifdef CONFIG_BPF_JIT
> -int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
> -#endif
> -
>  int fixup_exception(struct pt_regs *regs)
>  {
>         const struct exception_table_entry *fixup;
> @@ -23,10 +19,8 @@ int fixup_exception(struct pt_regs *regs)
>         if (!fixup)
>                 return 0;
>
> -#ifdef CONFIG_BPF_JIT
> -       if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
> +       if (rv_in_bpf_jit(regs))
>                 return rv_bpf_fixup_exception(fixup, regs);
> -#endif
>

The only changes that are needed are:
1. Simply gate with CONFIG_BPF_JIT && CONFIG_ARCH_RV64I, instead of of
CONFIG_BPF_JIT

2. Forward declaration of the rv_bpf_fixup_exception() in bpf_jit_comp64.c.


Björn

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[Björn Töpel]

On Tue, 2 Nov 2021 at 15:40, Tong Tiangen <tongtiangen@huawei.com> wrote:
>
> This patch fix two compile errors:
> 1. when CONFIG_BPF_JIT and CONFIG_ARCH_32I is open, There is the following
> compiler error:
>   error: undefined symbol: rv_bpf_fixup_exception
>

Good catch for the RV32!

> 2. when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following
> compiler error (W=1):
>   error: no previous prototype for 'rv_bpf_fixup_exception'
>
> In this patch, asm/extable.h is introduced,  the rv_bpf_fixup_exception
> function declaration is added to this file. in addition, the definition of
> exception_table_entry is moved from asm-generic/extable.h to this file.
>

This is way too complicated. More below.

> Fixes: 252c765bd764 ("riscv, bpf: Add BPF exception tables")
> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
> ---
>  arch/riscv/include/asm/Kbuild    |  1 -
>  arch/riscv/include/asm/extable.h | 49 ++++++++++++++++++++++++++++++++
>  arch/riscv/include/asm/uaccess.h | 13 ---------
>  arch/riscv/mm/extable.c          |  8 +-----
>  4 files changed, 50 insertions(+), 21 deletions(-)
>  create mode 100644 arch/riscv/include/asm/extable.h
>
> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
> index 445ccc97305a..57b86fd9916c 100644
> --- a/arch/riscv/include/asm/Kbuild
> +++ b/arch/riscv/include/asm/Kbuild
> @@ -1,6 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  generic-y += early_ioremap.h
> -generic-y += extable.h
>  generic-y += flat.h
>  generic-y += kvm_para.h
>  generic-y += user.h
> diff --git a/arch/riscv/include/asm/extable.h b/arch/riscv/include/asm/extable.h
> new file mode 100644
> index 000000000000..aa0332b053fb
> --- /dev/null
> +++ b/arch/riscv/include/asm/extable.h
> @@ -0,0 +1,49 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_EXTABLE_H
> +#define __ASM_EXTABLE_H
> +
> +/*
> + * The exception table consists of pairs of addresses: the first is the
> + * address of an instruction that is allowed to fault, and the second is
> + * the address at which the program should continue.  No registers are
> + * modified, so it is entirely up to the continuation code to figure out
> + * what to do.
> + *
> + * All the routines below use bits of fixup code that are out of line
> + * with the main instruction path.  This means when everything is well,
> + * we don't even have to jump over them.  Further, they do not intrude
> + * on our cache or tlb entries.
> + */
> +struct exception_table_entry {
> +       unsigned long insn, fixup;
> +};
> +
> +struct pt_regs;
> +int fixup_exception(struct pt_regs *regs);
> +
> +#if defined(CONFIG_MMU)
> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
> +{
> +       if (!IS_ENABLED(CONFIG_BPF_JIT) || !IS_ENABLED(CONFIG_64BIT))
> +               return false;
> +
> +       return regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END;
> +}
> +#else
> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
> +{
> +       return false;
> +}
> +#endif
> +
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_64BIT)
> +int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
> +#else
> +static inline int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
> +                                        struct pt_regs *regs)
> +{
> +       return 0;
> +}
> +#endif
> +
> +#endif
> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
> index f314ff44c48d..96ea91dc0e9c 100644
> --- a/arch/riscv/include/asm/uaccess.h
> +++ b/arch/riscv/include/asm/uaccess.h
> @@ -56,19 +56,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
>         return size <= TASK_SIZE && addr <= TASK_SIZE - size;
>  }
>
> -/*
> - * The exception table consists of pairs of addresses: the first is the
> - * address of an instruction that is allowed to fault, and the second is
> - * the address at which the program should continue.  No registers are
> - * modified, so it is entirely up to the continuation code to figure out
> - * what to do.
> - *
> - * All the routines below use bits of fixup code that are out of line
> - * with the main instruction path.  This means when everything is well,
> - * we don't even have to jump over them.  Further, they do not intrude
> - * on our cache or tlb entries.
> - */
> -
>  #define __LSW  0
>  #define __MSW  1
>
> diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
> index 18bf338303b6..264f465db5bb 100644
> --- a/arch/riscv/mm/extable.c
> +++ b/arch/riscv/mm/extable.c
> @@ -11,10 +11,6 @@
>  #include <linux/module.h>
>  #include <linux/uaccess.h>
>
> -#ifdef CONFIG_BPF_JIT
> -int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
> -#endif
> -
>  int fixup_exception(struct pt_regs *regs)
>  {
>         const struct exception_table_entry *fixup;
> @@ -23,10 +19,8 @@ int fixup_exception(struct pt_regs *regs)
>         if (!fixup)
>                 return 0;
>
> -#ifdef CONFIG_BPF_JIT
> -       if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
> +       if (rv_in_bpf_jit(regs))
>                 return rv_bpf_fixup_exception(fixup, regs);
> -#endif
>

The only changes that are needed are:
1. Simply gate with CONFIG_BPF_JIT && CONFIG_ARCH_RV64I, instead of of
CONFIG_BPF_JIT

2. Forward declaration of the rv_bpf_fixup_exception() in bpf_jit_comp64.c.


Björn

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

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[tongtiangen]

On 2021/11/2 23:45, Björn Töpel wrote:
> On Tue, 2 Nov 2021 at 15:40, Tong Tiangen <tongtiangen@huawei.com> wrote:
>>
>> This patch fix two compile errors:
>> 1. when CONFIG_BPF_JIT and CONFIG_ARCH_32I is open, There is the following
>> compiler error:
>>   error: undefined symbol: rv_bpf_fixup_exception
>>
>
> Good catch for the RV32!
>
>> 2. when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following
>> compiler error (W=1):
>>   error: no previous prototype for 'rv_bpf_fixup_exception'
>>
>> In this patch, asm/extable.h is introduced,  the rv_bpf_fixup_exception
>> function declaration is added to this file. in addition, the definition of
>> exception_table_entry is moved from asm-generic/extable.h to this file.
>>
>
> This is way too complicated. More below.
>
>> Fixes: 252c765bd764 ("riscv, bpf: Add BPF exception tables")
>> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
>> ---
>>  arch/riscv/include/asm/Kbuild    |  1 -
>>  arch/riscv/include/asm/extable.h | 49 ++++++++++++++++++++++++++++++++
>>  arch/riscv/include/asm/uaccess.h | 13 ---------
>>  arch/riscv/mm/extable.c          |  8 +-----
>>  4 files changed, 50 insertions(+), 21 deletions(-)
>>  create mode 100644 arch/riscv/include/asm/extable.h
>>
>> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
>> index 445ccc97305a..57b86fd9916c 100644
>> --- a/arch/riscv/include/asm/Kbuild
>> +++ b/arch/riscv/include/asm/Kbuild
>> @@ -1,6 +1,5 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>  generic-y += early_ioremap.h
>> -generic-y += extable.h
>>  generic-y += flat.h
>>  generic-y += kvm_para.h
>>  generic-y += user.h
>> diff --git a/arch/riscv/include/asm/extable.h b/arch/riscv/include/asm/extable.h
>> new file mode 100644
>> index 000000000000..aa0332b053fb
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/extable.h
>> @@ -0,0 +1,49 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __ASM_EXTABLE_H
>> +#define __ASM_EXTABLE_H
>> +
>> +/*
>> + * The exception table consists of pairs of addresses: the first is the
>> + * address of an instruction that is allowed to fault, and the second is
>> + * the address at which the program should continue.  No registers are
>> + * modified, so it is entirely up to the continuation code to figure out
>> + * what to do.
>> + *
>> + * All the routines below use bits of fixup code that are out of line
>> + * with the main instruction path.  This means when everything is well,
>> + * we don't even have to jump over them.  Further, they do not intrude
>> + * on our cache or tlb entries.
>> + */
>> +struct exception_table_entry {
>> +       unsigned long insn, fixup;
>> +};
>> +
>> +struct pt_regs;
>> +int fixup_exception(struct pt_regs *regs);
>> +
>> +#if defined(CONFIG_MMU)
>> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
>> +{
>> +       if (!IS_ENABLED(CONFIG_BPF_JIT) || !IS_ENABLED(CONFIG_64BIT))
>> +               return false;
>> +
>> +       return regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END;
>> +}
>> +#else
>> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
>> +{
>> +       return false;
>> +}
>> +#endif
>> +
>> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_64BIT)
>> +int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
>> +#else
>> +static inline int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
>> +                                        struct pt_regs *regs)
>> +{
>> +       return 0;
>> +}
>> +#endif
>> +
>> +#endif
>> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
>> index f314ff44c48d..96ea91dc0e9c 100644
>> --- a/arch/riscv/include/asm/uaccess.h
>> +++ b/arch/riscv/include/asm/uaccess.h
>> @@ -56,19 +56,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
>>         return size <= TASK_SIZE && addr <= TASK_SIZE - size;
>>  }
>>
>> -/*
>> - * The exception table consists of pairs of addresses: the first is the
>> - * address of an instruction that is allowed to fault, and the second is
>> - * the address at which the program should continue.  No registers are
>> - * modified, so it is entirely up to the continuation code to figure out
>> - * what to do.
>> - *
>> - * All the routines below use bits of fixup code that are out of line
>> - * with the main instruction path.  This means when everything is well,
>> - * we don't even have to jump over them.  Further, they do not intrude
>> - * on our cache or tlb entries.
>> - */
>> -
>>  #define __LSW  0
>>  #define __MSW  1
>>
>> diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
>> index 18bf338303b6..264f465db5bb 100644
>> --- a/arch/riscv/mm/extable.c
>> +++ b/arch/riscv/mm/extable.c
>> @@ -11,10 +11,6 @@
>>  #include <linux/module.h>
>>  #include <linux/uaccess.h>
>>
>> -#ifdef CONFIG_BPF_JIT
>> -int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
>> -#endif
>> -
>>  int fixup_exception(struct pt_regs *regs)
>>  {
>>         const struct exception_table_entry *fixup;
>> @@ -23,10 +19,8 @@ int fixup_exception(struct pt_regs *regs)
>>         if (!fixup)
>>                 return 0;
>>
>> -#ifdef CONFIG_BPF_JIT
>> -       if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
>> +       if (rv_in_bpf_jit(regs))
>>                 return rv_bpf_fixup_exception(fixup, regs);
>> -#endif
>>
>
> The only changes that are needed are:
> 1. Simply gate with CONFIG_BPF_JIT && CONFIG_ARCH_RV64I, instead of of
> CONFIG_BPF_JIT
>
> 2. Forward declaration of the rv_bpf_fixup_exception() in bpf_jit_comp64.c.
>
Hi Björn:
 From the perspective of development, introduce asm/extable.h is also prepare for the
subsequent modification of exception_table_entry, such as:
   1. https://lkml.org/lkml/2021/10/20/591
   2. https://lore.kernel.org/linux-arm-kernel/20211019160219.5202-11-mark.rutland@arm.com/

Therefore, the prototype declarations and definitions related to kernel config are placed in head file,
which can avoid compiler error and simplify the rendering of functions.

Thanks.
Tong.

>
> Björn
> .
>

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[tongtiangen]

On 2021/11/2 23:45, Björn Töpel wrote:
> On Tue, 2 Nov 2021 at 15:40, Tong Tiangen <tongtiangen@huawei.com> wrote:
>>
>> This patch fix two compile errors:
>> 1. when CONFIG_BPF_JIT and CONFIG_ARCH_32I is open, There is the following
>> compiler error:
>>   error: undefined symbol: rv_bpf_fixup_exception
>>
>
> Good catch for the RV32!
>
>> 2. when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following
>> compiler error (W=1):
>>   error: no previous prototype for 'rv_bpf_fixup_exception'
>>
>> In this patch, asm/extable.h is introduced,  the rv_bpf_fixup_exception
>> function declaration is added to this file. in addition, the definition of
>> exception_table_entry is moved from asm-generic/extable.h to this file.
>>
>
> This is way too complicated. More below.
>
>> Fixes: 252c765bd764 ("riscv, bpf: Add BPF exception tables")
>> Signed-off-by: Tong Tiangen <tongtiangen@huawei.com>
>> ---
>>  arch/riscv/include/asm/Kbuild    |  1 -
>>  arch/riscv/include/asm/extable.h | 49 ++++++++++++++++++++++++++++++++
>>  arch/riscv/include/asm/uaccess.h | 13 ---------
>>  arch/riscv/mm/extable.c          |  8 +-----
>>  4 files changed, 50 insertions(+), 21 deletions(-)
>>  create mode 100644 arch/riscv/include/asm/extable.h
>>
>> diff --git a/arch/riscv/include/asm/Kbuild b/arch/riscv/include/asm/Kbuild
>> index 445ccc97305a..57b86fd9916c 100644
>> --- a/arch/riscv/include/asm/Kbuild
>> +++ b/arch/riscv/include/asm/Kbuild
>> @@ -1,6 +1,5 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>  generic-y += early_ioremap.h
>> -generic-y += extable.h
>>  generic-y += flat.h
>>  generic-y += kvm_para.h
>>  generic-y += user.h
>> diff --git a/arch/riscv/include/asm/extable.h b/arch/riscv/include/asm/extable.h
>> new file mode 100644
>> index 000000000000..aa0332b053fb
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/extable.h
>> @@ -0,0 +1,49 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +#ifndef __ASM_EXTABLE_H
>> +#define __ASM_EXTABLE_H
>> +
>> +/*
>> + * The exception table consists of pairs of addresses: the first is the
>> + * address of an instruction that is allowed to fault, and the second is
>> + * the address at which the program should continue.  No registers are
>> + * modified, so it is entirely up to the continuation code to figure out
>> + * what to do.
>> + *
>> + * All the routines below use bits of fixup code that are out of line
>> + * with the main instruction path.  This means when everything is well,
>> + * we don't even have to jump over them.  Further, they do not intrude
>> + * on our cache or tlb entries.
>> + */
>> +struct exception_table_entry {
>> +       unsigned long insn, fixup;
>> +};
>> +
>> +struct pt_regs;
>> +int fixup_exception(struct pt_regs *regs);
>> +
>> +#if defined(CONFIG_MMU)
>> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
>> +{
>> +       if (!IS_ENABLED(CONFIG_BPF_JIT) || !IS_ENABLED(CONFIG_64BIT))
>> +               return false;
>> +
>> +       return regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END;
>> +}
>> +#else
>> +static inline bool rv_in_bpf_jit(struct pt_regs *regs)
>> +{
>> +       return false;
>> +}
>> +#endif
>> +
>> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_64BIT)
>> +int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
>> +#else
>> +static inline int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
>> +                                        struct pt_regs *regs)
>> +{
>> +       return 0;
>> +}
>> +#endif
>> +
>> +#endif
>> diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h
>> index f314ff44c48d..96ea91dc0e9c 100644
>> --- a/arch/riscv/include/asm/uaccess.h
>> +++ b/arch/riscv/include/asm/uaccess.h
>> @@ -56,19 +56,6 @@ static inline int __access_ok(unsigned long addr, unsigned long size)
>>         return size <= TASK_SIZE && addr <= TASK_SIZE - size;
>>  }
>>
>> -/*
>> - * The exception table consists of pairs of addresses: the first is the
>> - * address of an instruction that is allowed to fault, and the second is
>> - * the address at which the program should continue.  No registers are
>> - * modified, so it is entirely up to the continuation code to figure out
>> - * what to do.
>> - *
>> - * All the routines below use bits of fixup code that are out of line
>> - * with the main instruction path.  This means when everything is well,
>> - * we don't even have to jump over them.  Further, they do not intrude
>> - * on our cache or tlb entries.
>> - */
>> -
>>  #define __LSW  0
>>  #define __MSW  1
>>
>> diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
>> index 18bf338303b6..264f465db5bb 100644
>> --- a/arch/riscv/mm/extable.c
>> +++ b/arch/riscv/mm/extable.c
>> @@ -11,10 +11,6 @@
>>  #include <linux/module.h>
>>  #include <linux/uaccess.h>
>>
>> -#ifdef CONFIG_BPF_JIT
>> -int rv_bpf_fixup_exception(const struct exception_table_entry *ex, struct pt_regs *regs);
>> -#endif
>> -
>>  int fixup_exception(struct pt_regs *regs)
>>  {
>>         const struct exception_table_entry *fixup;
>> @@ -23,10 +19,8 @@ int fixup_exception(struct pt_regs *regs)
>>         if (!fixup)
>>                 return 0;
>>
>> -#ifdef CONFIG_BPF_JIT
>> -       if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
>> +       if (rv_in_bpf_jit(regs))
>>                 return rv_bpf_fixup_exception(fixup, regs);
>> -#endif
>>
>
> The only changes that are needed are:
> 1. Simply gate with CONFIG_BPF_JIT && CONFIG_ARCH_RV64I, instead of of
> CONFIG_BPF_JIT
>
> 2. Forward declaration of the rv_bpf_fixup_exception() in bpf_jit_comp64.c.
>
Hi Björn:
 From the perspective of development, introduce asm/extable.h is also prepare for the
subsequent modification of exception_table_entry, such as:
   1. https://lkml.org/lkml/2021/10/20/591
   2. https://lore.kernel.org/linux-arm-kernel/20211019160219.5202-11-mark.rutland@arm.com/

Therefore, the prototype declarations and definitions related to kernel config are placed in head file,
which can avoid compiler error and simplify the rendering of functions.

Thanks.
Tong.

>
> Björn
> .
>

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

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[Björn Töpel]

On Wed, 3 Nov 2021 at 04:06, tongtiangen <tongtiangen@huawei.com> wrote:
>
[...]
> >
> Hi Björn:
>  From the perspective of development, introduce asm/extable.h is also prepare for the
> subsequent modification of exception_table_entry, such as:
>    1. https://lkml.org/lkml/2021/10/20/591
>    2. https://lore.kernel.org/linux-arm-kernel/20211019160219.5202-11-mark.rutland@arm.com/
>
> Therefore, the prototype declarations and definitions related to kernel config are placed in head file,
> which can avoid compiler error and simplify the rendering of functions.
>

Sure, but *this* patch is about getting the broken RV32 build to work,
aimed for the bpf tree. Moving the extable.h is unrelated, and should
not be done here. IMO it would be better to have this patch small/easy
to read. I can't really see how this patch helps, when merging with
Jisheng's work.

...and I still think that:
--8<--
diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
index 18bf338303b6..ddb7d3b99e89 100644
--- a/arch/riscv/mm/extable.c
+++ b/arch/riscv/mm/extable.c
@@ -11,7 +11,7 @@
 #include <linux/module.h>
 #include <linux/uaccess.h>

-#ifdef CONFIG_BPF_JIT
+#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
 int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
struct pt_regs *regs);
 #endif

@@ -23,7 +23,7 @@ int fixup_exception(struct pt_regs *regs)
        if (!fixup)
                return 0;

-#ifdef CONFIG_BPF_JIT
+#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
        if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
                return rv_bpf_fixup_exception(fixup, regs);
 #endif
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 2ca345c7b0bf..6372a235522d 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -459,6 +459,8 @@ static int emit_call(bool fixed, u64 addr, struct
rv_jit_context *ctx)
 #define BPF_FIXUP_OFFSET_MASK   GENMASK(26, 0)
 #define BPF_FIXUP_REG_MASK      GENMASK(31, 27)

+int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
+                          struct pt_regs *regs);
 int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
                                struct pt_regs *regs)
 {
-->8--

is much simpler.



Thoughts?
Björn




> Thanks.
> Tong.
>
> >
> > Björn
> > .
> >

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[Björn Töpel]

On Wed, 3 Nov 2021 at 04:06, tongtiangen <tongtiangen@huawei.com> wrote:
>
[...]
> >
> Hi Björn:
>  From the perspective of development, introduce asm/extable.h is also prepare for the
> subsequent modification of exception_table_entry, such as:
>    1. https://lkml.org/lkml/2021/10/20/591
>    2. https://lore.kernel.org/linux-arm-kernel/20211019160219.5202-11-mark.rutland@arm.com/
>
> Therefore, the prototype declarations and definitions related to kernel config are placed in head file,
> which can avoid compiler error and simplify the rendering of functions.
>

Sure, but *this* patch is about getting the broken RV32 build to work,
aimed for the bpf tree. Moving the extable.h is unrelated, and should
not be done here. IMO it would be better to have this patch small/easy
to read. I can't really see how this patch helps, when merging with
Jisheng's work.

...and I still think that:
--8<--
diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
index 18bf338303b6..ddb7d3b99e89 100644
--- a/arch/riscv/mm/extable.c
+++ b/arch/riscv/mm/extable.c
@@ -11,7 +11,7 @@
 #include <linux/module.h>
 #include <linux/uaccess.h>

-#ifdef CONFIG_BPF_JIT
+#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
 int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
struct pt_regs *regs);
 #endif

@@ -23,7 +23,7 @@ int fixup_exception(struct pt_regs *regs)
        if (!fixup)
                return 0;

-#ifdef CONFIG_BPF_JIT
+#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
        if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
                return rv_bpf_fixup_exception(fixup, regs);
 #endif
diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 2ca345c7b0bf..6372a235522d 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -459,6 +459,8 @@ static int emit_call(bool fixed, u64 addr, struct
rv_jit_context *ctx)
 #define BPF_FIXUP_OFFSET_MASK   GENMASK(26, 0)
 #define BPF_FIXUP_REG_MASK      GENMASK(31, 27)

+int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
+                          struct pt_regs *regs);
 int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
                                struct pt_regs *regs)
 {
-->8--

is much simpler.



Thoughts?
Björn




> Thanks.
> Tong.
>
> >
> > Björn
> > .
> >

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

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[tongtiangen]

On 2021/11/3 14:10, Björn Töpel wrote:
> On Wed, 3 Nov 2021 at 04:06, tongtiangen <tongtiangen@huawei.com> wrote:
>>
> [...]
>>>
>> Hi Björn:
>>  From the perspective of development, introduce asm/extable.h is also prepare for the
>> subsequent modification of exception_table_entry, such as:
>>    1. https://lkml.org/lkml/2021/10/20/591
>>    2. https://lore.kernel.org/linux-arm-kernel/20211019160219.5202-11-mark.rutland@arm.com/
>>
>> Therefore, the prototype declarations and definitions related to kernel config are placed in head file,
>> which can avoid compiler error and simplify the rendering of functions.
>>
>
> Sure, but *this* patch is about getting the broken RV32 build to work,
> aimed for the bpf tree. Moving the extable.h is unrelated, and should
> not be done here. IMO it would be better to have this patch small/easy
> to read. I can't really see how this patch helps, when merging with
> Jisheng's work.
>
> ...and I still think that:
> --8<--
> diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
> index 18bf338303b6..ddb7d3b99e89 100644
> --- a/arch/riscv/mm/extable.c
> +++ b/arch/riscv/mm/extable.c
> @@ -11,7 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/uaccess.h>
>
> -#ifdef CONFIG_BPF_JIT
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
>  int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
> struct pt_regs *regs);
>  #endif
>
> @@ -23,7 +23,7 @@ int fixup_exception(struct pt_regs *regs)
>         if (!fixup)
>                 return 0;
>
> -#ifdef CONFIG_BPF_JIT
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
>         if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
>                 return rv_bpf_fixup_exception(fixup, regs);
>  #endif
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 2ca345c7b0bf..6372a235522d 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -459,6 +459,8 @@ static int emit_call(bool fixed, u64 addr, struct
> rv_jit_context *ctx)
>  #define BPF_FIXUP_OFFSET_MASK   GENMASK(26, 0)
>  #define BPF_FIXUP_REG_MASK      GENMASK(31, 27)
>
> +int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
> +                          struct pt_regs *regs);
>  int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
>                                 struct pt_regs *regs)
>  {
> -->8--
>
> is much simpler.

Adding a function declaration in bpf_jit_comp64.c file cannot fix this compiler error:

....
when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following compiler error (W=1):
   error: no previous prototype for 'rv_bpf_fixup_exception'
....

To fix this compiler error, you need to make a declaration in a header file, which is also
the reason for introducing extable.h.

Before making this patch, I thought about this change, but on the whole, I think the modification
scheme of adding header files moved me.;-)

>
>
>
> Thoughts?
> Björn
>
>
>
>
>> Thanks.
>> Tong.
>>
>>>
>>> Björn
>>> .
>>>
> .
>

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[tongtiangen]

On 2021/11/3 14:10, Björn Töpel wrote:
> On Wed, 3 Nov 2021 at 04:06, tongtiangen <tongtiangen@huawei.com> wrote:
>>
> [...]
>>>
>> Hi Björn:
>>  From the perspective of development, introduce asm/extable.h is also prepare for the
>> subsequent modification of exception_table_entry, such as:
>>    1. https://lkml.org/lkml/2021/10/20/591
>>    2. https://lore.kernel.org/linux-arm-kernel/20211019160219.5202-11-mark.rutland@arm.com/
>>
>> Therefore, the prototype declarations and definitions related to kernel config are placed in head file,
>> which can avoid compiler error and simplify the rendering of functions.
>>
>
> Sure, but *this* patch is about getting the broken RV32 build to work,
> aimed for the bpf tree. Moving the extable.h is unrelated, and should
> not be done here. IMO it would be better to have this patch small/easy
> to read. I can't really see how this patch helps, when merging with
> Jisheng's work.
>
> ...and I still think that:
> --8<--
> diff --git a/arch/riscv/mm/extable.c b/arch/riscv/mm/extable.c
> index 18bf338303b6..ddb7d3b99e89 100644
> --- a/arch/riscv/mm/extable.c
> +++ b/arch/riscv/mm/extable.c
> @@ -11,7 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/uaccess.h>
>
> -#ifdef CONFIG_BPF_JIT
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
>  int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
> struct pt_regs *regs);
>  #endif
>
> @@ -23,7 +23,7 @@ int fixup_exception(struct pt_regs *regs)
>         if (!fixup)
>                 return 0;
>
> -#ifdef CONFIG_BPF_JIT
> +#if defined(CONFIG_BPF_JIT) && defined(CONFIG_ARCH_RV64I)
>         if (regs->epc >= BPF_JIT_REGION_START && regs->epc < BPF_JIT_REGION_END)
>                 return rv_bpf_fixup_exception(fixup, regs);
>  #endif
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 2ca345c7b0bf..6372a235522d 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -459,6 +459,8 @@ static int emit_call(bool fixed, u64 addr, struct
> rv_jit_context *ctx)
>  #define BPF_FIXUP_OFFSET_MASK   GENMASK(26, 0)
>  #define BPF_FIXUP_REG_MASK      GENMASK(31, 27)
>
> +int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
> +                          struct pt_regs *regs);
>  int rv_bpf_fixup_exception(const struct exception_table_entry *ex,
>                                 struct pt_regs *regs)
>  {
> -->8--
>
> is much simpler.

Adding a function declaration in bpf_jit_comp64.c file cannot fix this compiler error:

....
when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following compiler error (W=1):
   error: no previous prototype for 'rv_bpf_fixup_exception'
....

To fix this compiler error, you need to make a declaration in a header file, which is also
the reason for introducing extable.h.

Before making this patch, I thought about this change, but on the whole, I think the modification
scheme of adding header files moved me.;-)

>
>
>
> Thoughts?
> Björn
>
>
>
>
>> Thanks.
>> Tong.
>>
>>>
>>> Björn
>>> .
>>>
> .
>

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

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[Björn Töpel]

On Wed, 3 Nov 2021 at 08:26, tongtiangen <tongtiangen@huawei.com> wrote:
>

[...]

>
> Adding a function declaration in bpf_jit_comp64.c file cannot fix this compiler error:
>

AFAIK, there are two issues:

1. https://lore.kernel.org/llvm/202110290334.2zdMyRq4-lkp@intel.com/
2. https://lore.kernel.org/llvm/202111020610.9oy9Rr0G-lkp@intel.com/

1 is a warning when W=1 is enabled (missing prototype from -Wmissing-prototypes)
2 is an error, since the function is not defined when building CONFIG_ARCH_RV32I

You are trying to address both issues in this patch.

> ....
> when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following compiler error (W=1):
>    error: no previous prototype for 'rv_bpf_fixup_exception'
> ....
>
> To fix this compiler error, you need to make a declaration in a header file, which is also
> the reason for introducing extable.h.
>

No, you don't need the header file. The forward declaration is
sufficient to get rid of the warning, and the adding CONFIG_ARCH_RV64I
fixes the RV32I build.


Björn

Re: [PATCH bpf-next] riscv, bpf: fix some compiler error

[Björn Töpel]

On Wed, 3 Nov 2021 at 08:26, tongtiangen <tongtiangen@huawei.com> wrote:
>

[...]

>
> Adding a function declaration in bpf_jit_comp64.c file cannot fix this compiler error:
>

AFAIK, there are two issues:

1. https://lore.kernel.org/llvm/202110290334.2zdMyRq4-lkp@intel.com/
2. https://lore.kernel.org/llvm/202111020610.9oy9Rr0G-lkp@intel.com/

1 is a warning when W=1 is enabled (missing prototype from -Wmissing-prototypes)
2 is an error, since the function is not defined when building CONFIG_ARCH_RV32I

You are trying to address both issues in this patch.

> ....
> when CONFIG_BPF_JIT and CONFIG_ARCH_64I is open, There is the following compiler error (W=1):
>    error: no previous prototype for 'rv_bpf_fixup_exception'
> ....
>
> To fix this compiler error, you need to make a declaration in a header file, which is also
> the reason for introducing extable.h.
>

No, you don't need the header file. The forward declaration is
sufficient to get rid of the warning, and the adding CONFIG_ARCH_RV64I
fixes the RV32I build.


Björn

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