Re: [PATCH 3/3] tools/perf/arch/powerc: Add get_arch_regnum for powerpc
Hi Athira and Namhyung,
On 03/09/2024 03:25 PM, Athira Rajeev wrote:
The function get_dwarf_regnum() returns a DWARF register number
from a register name string. This calls arch specific function
get_arch_regnum to return register number for corresponding arch.
Add mappings for register name to register number in powerpc code:
arch/powerpc/util/dwarf-regs.c
Signed-off-by: Athira Rajeev
---
tools/perf/arch/powerpc/util/dwarf-regs.c | 29 +++
1 file changed, 29 insertions(+)
I found commit 3eee606757ad ("perf dwarf-regs: Add get_dwarf_regnum()")
for x86, would you be able to share how to test these changes? What is
the difference with and without the patches?
Thanks,
Tiezhu
Re: [RFC PATCH] asm-generic: Unify uapi bitsperlong.h
On 06/08/2023 08:56 PM, Arnd Bergmann wrote: On Thu, Jun 8, 2023, at 09:04, Tiezhu Yang wrote: On 05/09/2023 05:37 PM, Arnd Bergmann wrote: On Tue, May 9, 2023, at 09:05, Tiezhu Yang wrote: I think we are completely safe on the architectures that were added since the linux-3.x days (arm64, riscv, csky, openrisc, loongarch, nios2, and hexagon), but for the older ones there is a regression risk. Especially on targets that are not that actively maintained (sparc, alpha, ia64, sh, ...) there is a good chance that users are stuck on ancient toolchains. It's probably also a safe assumption that anyone with an older libc version won't be using the latest kernel headers, so I think we can still do this across architectures if both glibc and musl already require a compiler that is new enough, or alternatively if we know that the kernel headers require a new compiler for other reasons and nobody has complained. For glibc, it looks the minimum compiler version was raised from gcc-5 to gcc-8 four years ago, so we should be fine. In musl, the documentation states that at least gcc-3.4 or clang-3.2 are required, which probably predate the __SIZEOF_LONG__ macro. On the other hand, musl was only released in 2011, and building musl itself explicitly does not require kernel uapi headers, so this may not be too critical. There is also uClibc, but I could not find any minimum supported compiler version for that. Most commonly, this one is used for cross-build environments, so it's also less likely to have libc/gcc/headers being wildly out of sync. Not sure. Arnd [1] https://sourceware.org/pipermail/libc-alpha/2019-January/101010.html Thanks Arnd for the detailed reply. Any more comments? What should I do in the next step? I think the summary is "it's probably fine", but I don't know for sure, and it may not be worth the benefit. Thank you, it is very clear now. Maybe you can prepare a v2 that only does this for the newer architectures I mentioned above, with and an explanation and link to my above reply in the file comments? Only arm64, riscv and loongarch belong to the newer architectures which are related with this change, I am not sure it is necessary to "unify" uapi bitsperlong.h for them. Anyway, let me try, I will send a new version, maybe this is going to progress in the right direction. Thanks, Tiezhu
Re: [RFC PATCH] asm-generic: Unify uapi bitsperlong.h
Hi all, On 05/09/2023 05:37 PM, Arnd Bergmann wrote: On Tue, May 9, 2023, at 09:05, Tiezhu Yang wrote: Now we specify the minimal version of GCC as 5.1 and Clang/LLVM as 11.0.0 in Documentation/process/changes.rst, __CHAR_BIT__ and __SIZEOF_LONG__ are usable, just define __BITS_PER_LONG as (__CHAR_BIT__ * __SIZEOF_LONG__) in asm-generic uapi bitsperlong.h, simpler, works everywhere. Remove all the arch specific uapi bitsperlong.h which will be generated as arch/*/include/generated/uapi/asm/bitsperlong.h. Suggested-by: Xi Ruoyao Link: https://lore.kernel.org/all/d3e255e4746de44c9903c4433616d44ffcf18d1b.ca...@xry111.site/ Signed-off-by: Tiezhu Yang I originally introduced the bitsperlong.h header, and I'd love to see it removed if it's no longer needed. Your patch certainly seems like it does this well. There is one minor obstacle to this, which is that the compiler requirements for uapi headers are not the same as for kernel internal code. In particular, the uapi headers may be included by user space code that is built with an older compiler version, or with a compiler that is not gcc or clang. I think we are completely safe on the architectures that were added since the linux-3.x days (arm64, riscv, csky, openrisc, loongarch, nios2, and hexagon), but for the older ones there is a regression risk. Especially on targets that are not that actively maintained (sparc, alpha, ia64, sh, ...) there is a good chance that users are stuck on ancient toolchains. It's probably also a safe assumption that anyone with an older libc version won't be using the latest kernel headers, so I think we can still do this across architectures if both glibc and musl already require a compiler that is new enough, or alternatively if we know that the kernel headers require a new compiler for other reasons and nobody has complained. For glibc, it looks the minimum compiler version was raised from gcc-5 to gcc-8 four years ago, so we should be fine. In musl, the documentation states that at least gcc-3.4 or clang-3.2 are required, which probably predate the __SIZEOF_LONG__ macro. On the other hand, musl was only released in 2011, and building musl itself explicitly does not require kernel uapi headers, so this may not be too critical. There is also uClibc, but I could not find any minimum supported compiler version for that. Most commonly, this one is used for cross-build environments, so it's also less likely to have libc/gcc/headers being wildly out of sync. Not sure. Arnd [1] https://sourceware.org/pipermail/libc-alpha/2019-January/101010.html Thanks Arnd for the detailed reply. Any more comments? What should I do in the next step? Thanks, Tiezhu
[RFC PATCH] asm-generic: Unify uapi bitsperlong.h
Now we specify the minimal version of GCC as 5.1 and Clang/LLVM as 11.0.0 in Documentation/process/changes.rst, __CHAR_BIT__ and __SIZEOF_LONG__ are usable, just define __BITS_PER_LONG as (__CHAR_BIT__ * __SIZEOF_LONG__) in asm-generic uapi bitsperlong.h, simpler, works everywhere. Remove all the arch specific uapi bitsperlong.h which will be generated as arch/*/include/generated/uapi/asm/bitsperlong.h. Suggested-by: Xi Ruoyao Link: https://lore.kernel.org/all/d3e255e4746de44c9903c4433616d44ffcf18d1b.ca...@xry111.site/ Signed-off-by: Tiezhu Yang --- This is based on 6.4-rc1 arch/alpha/include/uapi/asm/bitsperlong.h | 9 arch/arm64/include/uapi/asm/bitsperlong.h | 24 --- arch/ia64/include/uapi/asm/bitsperlong.h | 9 arch/loongarch/include/uapi/asm/bitsperlong.h | 9 arch/mips/include/uapi/asm/bitsperlong.h | 9 arch/parisc/include/uapi/asm/bitsperlong.h | 13 --- arch/powerpc/include/uapi/asm/bitsperlong.h| 13 --- arch/riscv/include/uapi/asm/bitsperlong.h | 14 --- arch/s390/include/uapi/asm/bitsperlong.h | 14 --- arch/sparc/include/uapi/asm/bitsperlong.h | 14 --- arch/x86/include/uapi/asm/bitsperlong.h| 14 --- include/uapi/asm-generic/bitsperlong.h | 11 + tools/arch/alpha/include/uapi/asm/bitsperlong.h| 9 tools/arch/arm64/include/uapi/asm/bitsperlong.h| 24 --- tools/arch/hexagon/include/uapi/asm/bitsperlong.h | 27 -- tools/arch/ia64/include/uapi/asm/bitsperlong.h | 9 .../arch/loongarch/include/uapi/asm/bitsperlong.h | 9 .../arch/microblaze/include/uapi/asm/bitsperlong.h | 2 -- tools/arch/mips/include/uapi/asm/bitsperlong.h | 9 tools/arch/parisc/include/uapi/asm/bitsperlong.h | 15 tools/arch/powerpc/include/uapi/asm/bitsperlong.h | 13 --- tools/arch/riscv/include/uapi/asm/bitsperlong.h| 14 --- tools/arch/s390/include/uapi/asm/bitsperlong.h | 13 --- tools/arch/sparc/include/uapi/asm/bitsperlong.h| 13 --- tools/arch/x86/include/uapi/asm/bitsperlong.h | 13 --- tools/include/uapi/asm-generic/bitsperlong.h | 12 ++ tools/include/uapi/asm/bitsperlong.h | 24 --- 27 files changed, 3 insertions(+), 356 deletions(-) delete mode 100644 arch/alpha/include/uapi/asm/bitsperlong.h delete mode 100644 arch/arm64/include/uapi/asm/bitsperlong.h delete mode 100644 arch/ia64/include/uapi/asm/bitsperlong.h delete mode 100644 arch/loongarch/include/uapi/asm/bitsperlong.h delete mode 100644 arch/mips/include/uapi/asm/bitsperlong.h delete mode 100644 arch/parisc/include/uapi/asm/bitsperlong.h delete mode 100644 arch/powerpc/include/uapi/asm/bitsperlong.h delete mode 100644 arch/riscv/include/uapi/asm/bitsperlong.h delete mode 100644 arch/s390/include/uapi/asm/bitsperlong.h delete mode 100644 arch/sparc/include/uapi/asm/bitsperlong.h delete mode 100644 arch/x86/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/alpha/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/arm64/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/hexagon/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/ia64/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/loongarch/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/microblaze/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/mips/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/parisc/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/powerpc/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/riscv/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/s390/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/sparc/include/uapi/asm/bitsperlong.h delete mode 100644 tools/arch/x86/include/uapi/asm/bitsperlong.h delete mode 100644 tools/include/uapi/asm/bitsperlong.h diff --git a/arch/alpha/include/uapi/asm/bitsperlong.h b/arch/alpha/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 6c5bf7d..000 --- a/arch/alpha/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,9 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -#ifndef __ASM_ALPHA_BITSPERLONG_H -#define __ASM_ALPHA_BITSPERLONG_H - -#define __BITS_PER_LONG 64 - -#include - -#endif /* __ASM_ALPHA_BITSPERLONG_H */ diff --git a/arch/arm64/include/uapi/asm/bitsperlong.h b/arch/arm64/include/uapi/asm/bitsperlong.h deleted file mode 100644 index 485d60be..000 --- a/arch/arm64/include/uapi/asm/bitsperlong.h +++ /dev/null @@ -1,24 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ -/* - * Copyright (C) 2012 ARM Ltd. - * - * This program is free software; you can redistribute it and/or modify
[PATCH] selftests: powerpc: Use "grep -E" instead of "egrep"
The latest version of grep claims the egrep is now obsolete so the build
now contains warnings that look like:
egrep: warning: egrep is obsolescent; using grep -E
fix this using "grep -E" instead.
sed -i "s/egrep/grep -E/g" `grep egrep -rwl tools/testing/selftests/powerpc`
Here are the steps to install the latest grep:
wget http://ftp.gnu.org/gnu/grep/grep-3.8.tar.gz
tar xf grep-3.8.tar.gz
cd grep-3.8 && ./configure && make
sudo make install
export PATH=/usr/local/bin:$PATH
Signed-off-by: Tiezhu Yang
---
As Shuah suggested, this patch should go through powerpc/linux.git
tools/testing/selftests/powerpc/scripts/hmi.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/powerpc/scripts/hmi.sh
b/tools/testing/selftests/powerpc/scripts/hmi.sh
index dcdb392..bcc7b6b 100755
--- a/tools/testing/selftests/powerpc/scripts/hmi.sh
+++ b/tools/testing/selftests/powerpc/scripts/hmi.sh
@@ -36,7 +36,7 @@ trap "ppc64_cpu --smt-snooze-delay=100" 0 1
# for each chip+core combination
# todo - less fragile parsing
-egrep -o 'OCC: Chip [0-9a-f]+ Core [0-9a-f]' < /sys/firmware/opal/msglog |
+grep -E -o 'OCC: Chip [0-9a-f]+ Core [0-9a-f]' < /sys/firmware/opal/msglog |
while read chipcore; do
chip=$(echo "$chipcore"|awk '{print $3}')
core=$(echo "$chipcore"|awk '{print $5}')
--
2.1.0
[PATCH] powerpc: Use "grep -E" instead of "egrep"
The latest version of grep claims the egrep is now obsolete so the build
now contains warnings that look like:
egrep: warning: egrep is obsolescent; using grep -E
fix this up by moving the related file to use "grep -E" instead.
sed -i "s/egrep/grep -E/g" `grep egrep -rwl arch/powerpc`
Here are the steps to install the latest grep:
wget http://ftp.gnu.org/gnu/grep/grep-3.8.tar.gz
tar xf grep-3.8.tar.gz
cd grep-3.8 && ./configure && make
sudo make install
export PATH=/usr/local/bin:$PATH
Signed-off-by: Tiezhu Yang
---
arch/powerpc/boot/wrapper | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index 5bdd4dd..a86ae11 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -581,7 +581,7 @@ ps3)
# reached, then enter the system reset vector of the partially decompressed
# image. No warning is issued.
rm -f "$odir"/{otheros,otheros-too-big}.bld
-size=$(${CROSS}nm --no-sort --radix=d "$ofile" | egrep ' _end$' | cut -d'
' -f1)
+size=$(${CROSS}nm --no-sort --radix=d "$ofile" | grep -E ' _end$' | cut
-d' ' -f1)
bld="otheros.bld"
if [ $size -gt $((0x100)) ]; then
bld="otheros-too-big.bld"
--
2.1.0
Re: [PATCH v2 0/2] kdump: simplify code
On 12/13/2021 10:43 PM, Matthew Wilcox wrote: On Mon, Dec 13, 2021 at 08:30:33AM +, David Laight wrote: From: Matthew Wilcox Sent: 12 December 2021 11:48 On Sat, Dec 11, 2021 at 05:53:46PM +, David Laight wrote: From: Tiezhu Yang Sent: 11 December 2021 03:33 v2: -- add copy_to_user_or_kernel() in lib/usercopy.c -- define userbuf as bool type Instead of having a flag to indicate whether the buffer is user or kernel, would it be better to have two separate buffer pointers. One for a user space buffer, the other for a kernel space buffer. Exactly one of the buffers should always be NULL. No. You should be using an iov_iter instead. See https://lore.kernel.org/all/ya4bdb0ubjczh...@casper.infradead.org/ for a start on this. iov_iter gets horribly expensive... Oh, right. Reading the kcore is a high-performance path, my mistake. Hi, Thank you for your discussions. The intention of this patchset is to simplify the related code with no functional changes and no side effects. At this moment, if you are OK, I will send v3 used with inline function copy_to_user_or_kernel() to keep it simple, maybe other more changes can be done in the future if no any side effect. The v3 will contain the following three patches to make the changes more clear: kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel() kdump: crashdump: use copy_to_user_or_kernel() to simplify code kdump: vmcore: crashdump: make variable type of userbuf as bool
[PATCH v2 1/2] kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel()
In arch/*/kernel/crash_dump*.c, there exist many similar code
about copy_oldmem_page(), remove copy_to() in fs/proc/vmcore.c
and add copy_to_user_or_kernel() in lib/usercopy.c, then we can
use copy_to_user_or_kernel() to simplify the related code.
Signed-off-by: Tiezhu Yang
---
fs/proc/vmcore.c| 28 +++-
include/linux/uaccess.h | 1 +
lib/usercopy.c | 15 +++
3 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 509f851..f67fd77 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -238,22 +238,8 @@ copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
size_t csize,
return copy_oldmem_page(pfn, buf, csize, offset, userbuf);
}
-/*
- * Copy to either kernel or user space
- */
-static int copy_to(void *target, void *src, size_t size, int userbuf)
-{
- if (userbuf) {
- if (copy_to_user((char __user *) target, src, size))
- return -EFAULT;
- } else {
- memcpy(target, src, size);
- }
- return 0;
-}
-
#ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP
-static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf)
+static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, bool userbuf)
{
struct vmcoredd_node *dump;
u64 offset = 0;
@@ -266,7 +252,7 @@ static int vmcoredd_copy_dumps(void *dst, u64 start, size_t
size, int userbuf)
if (start < offset + dump->size) {
tsz = min(offset + (u64)dump->size - start, (u64)size);
buf = dump->buf + start - offset;
- if (copy_to(dst, buf, tsz, userbuf)) {
+ if (copy_to_user_or_kernel(dst, buf, tsz, userbuf)) {
ret = -EFAULT;
goto out_unlock;
}
@@ -330,7 +316,7 @@ static int vmcoredd_mmap_dumps(struct vm_area_struct *vma,
unsigned long dst,
* returned otherwise number of bytes read are returned.
*/
static ssize_t __read_vmcore(char *buffer, size_t buflen, loff_t *fpos,
-int userbuf)
+bool userbuf)
{
ssize_t acc = 0, tmp;
size_t tsz;
@@ -347,7 +333,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen,
loff_t *fpos,
/* Read ELF core header */
if (*fpos < elfcorebuf_sz) {
tsz = min(elfcorebuf_sz - (size_t)*fpos, buflen);
- if (copy_to(buffer, elfcorebuf + *fpos, tsz, userbuf))
+ if (copy_to_user_or_kernel(buffer, elfcorebuf + *fpos, tsz,
userbuf))
return -EFAULT;
buflen -= tsz;
*fpos += tsz;
@@ -395,7 +381,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen,
loff_t *fpos,
/* Read remaining elf notes */
tsz = min(elfcorebuf_sz + elfnotes_sz - (size_t)*fpos, buflen);
kaddr = elfnotes_buf + *fpos - elfcorebuf_sz - vmcoredd_orig_sz;
- if (copy_to(buffer, kaddr, tsz, userbuf))
+ if (copy_to_user_or_kernel(buffer, kaddr, tsz, userbuf))
return -EFAULT;
buflen -= tsz;
@@ -435,7 +421,7 @@ static ssize_t __read_vmcore(char *buffer, size_t buflen,
loff_t *fpos,
static ssize_t read_vmcore(struct file *file, char __user *buffer,
size_t buflen, loff_t *fpos)
{
- return __read_vmcore((__force char *) buffer, buflen, fpos, 1);
+ return __read_vmcore((__force char *) buffer, buflen, fpos, true);
}
/*
@@ -461,7 +447,7 @@ static vm_fault_t mmap_vmcore_fault(struct vm_fault *vmf)
if (!PageUptodate(page)) {
offset = (loff_t) index << PAGE_SHIFT;
buf = __va((page_to_pfn(page) << PAGE_SHIFT));
- rc = __read_vmcore(buf, PAGE_SIZE, , 0);
+ rc = __read_vmcore(buf, PAGE_SIZE, , false);
if (rc < 0) {
unlock_page(page);
put_page(page);
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index ac03940..a25e682e 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -283,6 +283,7 @@ __copy_from_user_inatomic_nocache(void *to, const void
__user *from,
#endif /* ARCH_HAS_NOCACHE_UACCESS */
extern __must_check int check_zeroed_user(const void __user *from, size_t
size);
+extern __must_check int copy_to_user_or_kernel(void *target, void *src, size_t
size, bool userbuf);
/**
* copy_struct_from_user: copy a struct from userspace
diff --git a/lib/usercopy.c b/lib/usercopy.c
index 7413dd3..7431b1b 100644
--- a/lib/usercopy.c
+++ b/lib/usercopy.c
@@ -90,3 +90,18 @@ int check_zeroed_user(const void __user *from, size_t size)
return -EFAULT;
}
EXPORT_SYMBOL(check_zeroed_user);
+
+/*
+ * Copy to either
[PATCH v2 2/2] kdump: crashdump: use copy_to_user_or_kernel() to simplify code
Use copy_to_user_or_kernel() to simplify the related code about
copy_oldmem_page() in arch/*/kernel/crash_dump*.c files.
Signed-off-by: Tiezhu Yang
---
arch/arm/kernel/crash_dump.c | 12 +++-
arch/arm64/kernel/crash_dump.c | 12 +++-
arch/ia64/kernel/crash_dump.c| 12 +---
arch/mips/kernel/crash_dump.c| 11 +++
arch/powerpc/kernel/crash_dump.c | 11 ---
arch/riscv/kernel/crash_dump.c | 11 +++
arch/sh/kernel/crash_dump.c | 11 +++
arch/x86/kernel/crash_dump_32.c | 11 +++
arch/x86/kernel/crash_dump_64.c | 15 +--
fs/proc/vmcore.c | 4 ++--
include/linux/crash_dump.h | 8
11 files changed, 38 insertions(+), 80 deletions(-)
diff --git a/arch/arm/kernel/crash_dump.c b/arch/arm/kernel/crash_dump.c
index 53cb924..a27c5df 100644
--- a/arch/arm/kernel/crash_dump.c
+++ b/arch/arm/kernel/crash_dump.c
@@ -29,7 +29,7 @@
*/
ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
size_t csize, unsigned long offset,
-int userbuf)
+bool userbuf)
{
void *vaddr;
@@ -40,14 +40,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
if (!vaddr)
return -ENOMEM;
- if (userbuf) {
- if (copy_to_user(buf, vaddr + offset, csize)) {
- iounmap(vaddr);
- return -EFAULT;
- }
- } else {
- memcpy(buf, vaddr + offset, csize);
- }
+ if (copy_to_user_or_kernel(buf, vaddr + offset, csize, userbuf))
+ csize = -EFAULT;
iounmap(vaddr);
return csize;
diff --git a/arch/arm64/kernel/crash_dump.c b/arch/arm64/kernel/crash_dump.c
index 58303a9..d22988f 100644
--- a/arch/arm64/kernel/crash_dump.c
+++ b/arch/arm64/kernel/crash_dump.c
@@ -27,7 +27,7 @@
*/
ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
size_t csize, unsigned long offset,
-int userbuf)
+bool userbuf)
{
void *vaddr;
@@ -38,14 +38,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
if (!vaddr)
return -ENOMEM;
- if (userbuf) {
- if (copy_to_user((char __user *)buf, vaddr + offset, csize)) {
- memunmap(vaddr);
- return -EFAULT;
- }
- } else {
- memcpy(buf, vaddr + offset, csize);
- }
+ if (copy_to_user_or_kernel(buf, vaddr + offset, csize, userbuf))
+ csize = -EFAULT;
memunmap(vaddr);
diff --git a/arch/ia64/kernel/crash_dump.c b/arch/ia64/kernel/crash_dump.c
index 0ed3c3d..12128f8 100644
--- a/arch/ia64/kernel/crash_dump.c
+++ b/arch/ia64/kernel/crash_dump.c
@@ -33,19 +33,17 @@
*/
ssize_t
copy_oldmem_page(unsigned long pfn, char *buf,
- size_t csize, unsigned long offset, int userbuf)
+ size_t csize, unsigned long offset, bool userbuf)
{
void *vaddr;
if (!csize)
return 0;
+
vaddr = __va(pfn<
static ssize_t __copy_oldmem_page(unsigned long pfn, char *buf, size_t csize,
- unsigned long offset, int userbuf,
+ unsigned long offset, bool userbuf,
bool encrypted)
{
void *vaddr;
@@ -29,13 +29,8 @@ static ssize_t __copy_oldmem_page(unsigned long pfn, char
*buf, size_t csize,
if (!vaddr)
return -ENOMEM;
- if (userbuf) {
- if (copy_to_user((void __user *)buf, vaddr + offset, csize)) {
- iounmap((void __iomem *)vaddr);
- return -EFAULT;
- }
- } else
- memcpy(buf, vaddr + offset, csize);
+ if (copy_to_user_or_kernel(buf, vaddr + offset, csize, userbuf))
+ csize = -EFAULT;
set_iounmap_nonlazy();
iounmap((void __iomem *)vaddr);
@@ -56,7 +51,7 @@ static ssize_t __copy_oldmem_page(unsigned long pfn, char
*buf, size_t csize,
* mapped in the current kernel. We stitch up a pte, similar to kmap_atomic.
*/
ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t csize,
-unsigned long offset, int userbuf)
+unsigned long offset, bool userbuf)
{
return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, false);
}
@@ -67,7 +62,7 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf, size_t
csize,
* machines.
*/
ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf, size_t csize,
- unsigned long offset, int userbuf)
+ unsigned long offset, bool userbuf)
{
return __copy_oldmem_page(pfn, buf, csize, offset, userbuf, true);
}
diff --git a/fs/proc/vmcore.c b/fs/p
[PATCH v2 0/2] kdump: simplify code
v2: -- add copy_to_user_or_kernel() in lib/usercopy.c -- define userbuf as bool type Tiezhu Yang (2): kdump: vmcore: remove copy_to() and add copy_to_user_or_kernel() kdump: crashdump: use copy_to_user_or_kernel() to simplify code arch/arm/kernel/crash_dump.c | 12 +++- arch/arm64/kernel/crash_dump.c | 12 +++- arch/ia64/kernel/crash_dump.c| 12 +--- arch/mips/kernel/crash_dump.c| 11 +++ arch/powerpc/kernel/crash_dump.c | 11 --- arch/riscv/kernel/crash_dump.c | 11 +++ arch/sh/kernel/crash_dump.c | 11 +++ arch/x86/kernel/crash_dump_32.c | 11 +++ arch/x86/kernel/crash_dump_64.c | 15 +-- fs/proc/vmcore.c | 32 +--- include/linux/crash_dump.h | 8 include/linux/uaccess.h | 1 + lib/usercopy.c | 15 +++ 13 files changed, 61 insertions(+), 101 deletions(-) -- 2.1.0
Re: [PATCH 1/2] kdump: vmcore: move copy_to() from vmcore.c to uaccess.h
On 12/11/2021 12:59 AM, Andrew Morton wrote:
On Fri, 10 Dec 2021 21:36:00 +0800 Tiezhu Yang wrote:
In arch/*/kernel/crash_dump*.c, there exist similar code about
copy_oldmem_page(), move copy_to() from vmcore.c to uaccess.h,
and then we can use copy_to() to simplify the related code.
...
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -238,20 +238,6 @@ copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
size_t csize,
return copy_oldmem_page(pfn, buf, csize, offset, userbuf);
}
-/*
- * Copy to either kernel or user space
- */
-static int copy_to(void *target, void *src, size_t size, int userbuf)
-{
- if (userbuf) {
- if (copy_to_user((char __user *) target, src, size))
- return -EFAULT;
- } else {
- memcpy(target, src, size);
- }
- return 0;
-}
-
#ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP
static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf)
{
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index ac03940..4a6c3e4 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -201,6 +201,20 @@ copy_to_user(void __user *to, const void *from, unsigned
long n)
return n;
}
+/*
+ * Copy to either kernel or user space
+ */
+static inline int copy_to(void *target, void *src, size_t size, int userbuf)
+{
+ if (userbuf) {
+ if (copy_to_user((char __user *) target, src, size))
+ return -EFAULT;
+ } else {
+ memcpy(target, src, size);
+ }
+ return 0;
+}
+
Ordinarily I'd say "this is too large to be inlined". But the function
has only a single callsite per architecture so inlining it won't cause
bloat at present.
But hopefully copy_to() will get additional callers in the future, in
which case it shouldn't be inlined. So I'm thinking it would be best
to start out with this as a regular non-inlined function, in
lib/usercopy.c.
Also, copy_to() is a very poor name for a globally-visible helper
function. Better would be copy_to_user_or_kernel(), although that's
perhaps a bit long.
And the `userbuf' arg should have type bool, yes?
Hi Andrew,
Thank you very much for your reply and suggestion, I agree with you,
I will send v2 later.
Thanks,
Tiezhu
[PATCH 1/2] kdump: vmcore: move copy_to() from vmcore.c to uaccess.h
In arch/*/kernel/crash_dump*.c, there exist similar code about
copy_oldmem_page(), move copy_to() from vmcore.c to uaccess.h,
and then we can use copy_to() to simplify the related code.
Signed-off-by: Tiezhu Yang
---
fs/proc/vmcore.c| 14 --
include/linux/uaccess.h | 14 ++
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 509f851..c5976a8 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -238,20 +238,6 @@ copy_oldmem_page_encrypted(unsigned long pfn, char *buf,
size_t csize,
return copy_oldmem_page(pfn, buf, csize, offset, userbuf);
}
-/*
- * Copy to either kernel or user space
- */
-static int copy_to(void *target, void *src, size_t size, int userbuf)
-{
- if (userbuf) {
- if (copy_to_user((char __user *) target, src, size))
- return -EFAULT;
- } else {
- memcpy(target, src, size);
- }
- return 0;
-}
-
#ifdef CONFIG_PROC_VMCORE_DEVICE_DUMP
static int vmcoredd_copy_dumps(void *dst, u64 start, size_t size, int userbuf)
{
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index ac03940..4a6c3e4 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -201,6 +201,20 @@ copy_to_user(void __user *to, const void *from, unsigned
long n)
return n;
}
+/*
+ * Copy to either kernel or user space
+ */
+static inline int copy_to(void *target, void *src, size_t size, int userbuf)
+{
+ if (userbuf) {
+ if (copy_to_user((char __user *) target, src, size))
+ return -EFAULT;
+ } else {
+ memcpy(target, src, size);
+ }
+ return 0;
+}
+
#ifndef copy_mc_to_kernel
/*
* Without arch opt-in this generic copy_mc_to_kernel() will not handle
--
2.1.0
[PATCH 0/2] kdump: simplify code
Tiezhu Yang (2): kdump: vmcore: move copy_to() from vmcore.c to uaccess.h kdump: crashdump: use copy_to() to simplify the related code arch/arm/kernel/crash_dump.c | 10 ++ arch/arm64/kernel/crash_dump.c | 10 ++ arch/ia64/kernel/crash_dump.c| 10 -- arch/mips/kernel/crash_dump.c| 9 ++--- arch/powerpc/kernel/crash_dump.c | 7 ++- arch/riscv/kernel/crash_dump.c | 9 ++--- arch/sh/kernel/crash_dump.c | 9 ++--- arch/x86/kernel/crash_dump_32.c | 9 ++--- arch/x86/kernel/crash_dump_64.c | 9 ++--- fs/proc/vmcore.c | 14 -- include/linux/uaccess.h | 14 ++ 11 files changed, 34 insertions(+), 76 deletions(-) -- 2.1.0
[PATCH 2/2] kdump: crashdump: use copy_to() to simplify the related code
Use copy_to() to simplify the related code about copy_oldmem_page()
in arch/*/kernel/crash_dump*.c files.
Signed-off-by: Tiezhu Yang
---
arch/arm/kernel/crash_dump.c | 10 ++
arch/arm64/kernel/crash_dump.c | 10 ++
arch/ia64/kernel/crash_dump.c| 10 --
arch/mips/kernel/crash_dump.c| 9 ++---
arch/powerpc/kernel/crash_dump.c | 7 ++-
arch/riscv/kernel/crash_dump.c | 9 ++---
arch/sh/kernel/crash_dump.c | 9 ++---
arch/x86/kernel/crash_dump_32.c | 9 ++---
arch/x86/kernel/crash_dump_64.c | 9 ++---
9 files changed, 20 insertions(+), 62 deletions(-)
diff --git a/arch/arm/kernel/crash_dump.c b/arch/arm/kernel/crash_dump.c
index 53cb924..6491f1d 100644
--- a/arch/arm/kernel/crash_dump.c
+++ b/arch/arm/kernel/crash_dump.c
@@ -40,14 +40,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
if (!vaddr)
return -ENOMEM;
- if (userbuf) {
- if (copy_to_user(buf, vaddr + offset, csize)) {
- iounmap(vaddr);
- return -EFAULT;
- }
- } else {
- memcpy(buf, vaddr + offset, csize);
- }
+ if (copy_to(buf, vaddr + offset, csize, userbuf))
+ csize = -EFAULT;
iounmap(vaddr);
return csize;
diff --git a/arch/arm64/kernel/crash_dump.c b/arch/arm64/kernel/crash_dump.c
index 58303a9..496e6a5 100644
--- a/arch/arm64/kernel/crash_dump.c
+++ b/arch/arm64/kernel/crash_dump.c
@@ -38,14 +38,8 @@ ssize_t copy_oldmem_page(unsigned long pfn, char *buf,
if (!vaddr)
return -ENOMEM;
- if (userbuf) {
- if (copy_to_user((char __user *)buf, vaddr + offset, csize)) {
- memunmap(vaddr);
- return -EFAULT;
- }
- } else {
- memcpy(buf, vaddr + offset, csize);
- }
+ if (copy_to(buf, vaddr + offset, csize, userbuf))
+ csize = -EFAULT;
memunmap(vaddr);
diff --git a/arch/ia64/kernel/crash_dump.c b/arch/ia64/kernel/crash_dump.c
index 0ed3c3d..20f4c4e 100644
--- a/arch/ia64/kernel/crash_dump.c
+++ b/arch/ia64/kernel/crash_dump.c
@@ -39,13 +39,11 @@ copy_oldmem_page(unsigned long pfn, char *buf,
if (!csize)
return 0;
+
vaddr = __va(pfn<
Re: [PATCH bpf-next v2] bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33
On 09/14/2021 03:30 PM, Daniel Borkmann wrote: On 9/11/21 3:56 AM, Tiezhu Yang wrote: [...] With this patch, it does not change the current limit 33, MAX_TAIL_CALL_CNT can reflect the actual max tail call count, the tailcall selftests can work well, and also the above failed testcase in test_bpf can be fixed for the interpreter (all archs) and the JIT (all archs except for x86). # uname -m x86_64 # echo 1 > /proc/sys/net/core/bpf_jit_enable # modprobe test_bpf # dmesg | grep -w FAIL Tail call error path, max count reached jited:1 ret 33 != 34 FAIL Could you also state in here which archs you have tested with this change? I presume /every/ arch which has a JIT? OK, will do it in v3. I have tested on x86 and mips. Signed-off-by: Tiezhu Yang --- v2: -- fix the typos in the commit message and update the commit message. -- fix the failed tailcall selftests for x86 jit. I am not quite sure the change on x86 is proper, with this change, tailcall selftests passed, but tailcall limit test in test_bpf.ko failed, I do not know the reason now, I think this is another issue, maybe someone more versed in x86 jit could take a look. There should be a series from Johan coming today with regards to test_bpf.ko that will fix the "tail call error path, max count reached" test which had an assumption in that R0 would always be valid for the fall-through and could be passed to the bpf_exit insn whereas it is not guaranteed and verifier, for example, forbids a subsequent access to R0 w/o reinit. For your testing, I would suggested to recheck once this series is out. I will test the following patch on x86 and mips: [PATCH bpf v4 13/14] bpf/tests: Fix error in tail call limit tests [...] diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index 0fe6aac..74a9e61 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -402,7 +402,7 @@ static int get_pop_bytes(bool *callee_regs_used) * ... bpf_tail_call(void *ctx, struct bpf_array *array, u64 index) ... * if (index >= array->map.max_entries) * goto out; - * if (++tail_call_cnt > MAX_TAIL_CALL_CNT) + * if (tail_call_cnt++ == MAX_TAIL_CALL_CNT) Why such inconsistency to e.g. above with arm64 case but also compared to x86 32 bit which uses JAE? If so, we should cleanly follow the reference implementation (== interpreter) _everywhere_ and _not_ introduce additional variants/implementations across JITs. In order tokeep consistencyand make as few changes as possible, I will modify the check condition as follows: #define MAX_TAIL_CALL_CNT 33 (1) for x86, arm64, ... (0 ~ 32) tcc = 0; if (tcc == MAX_TAIL_CALL_CNT) goto out; tcc++; (2) for mips, riscv (33 ~ 1) tcc = MAX_TAIL_CALL_CNT; if (tcc == 0) goto out; tcc--; [...]
[PATCH bpf-next v2] bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33
In the current code, the actual max tail call count is 33 which is greater
than MAX_TAIL_CALL_CNT (defined as 32), the actual limit is not consistent
with the meaning of MAX_TAIL_CALL_CNT, there is some confusion and need to
spend some time to think about the reason at the first glance.
We can see the historical evolution from commit 04fd61ab36ec ("bpf: allow
bpf programs to tail-call other bpf programs") and commit f9dabe016b63
("bpf: Undo off-by-one in interpreter tail call count limit").
In order to avoid changing existing behavior, the actual limit is 33 now,
this is reasonable.
After commit 874be05f525e ("bpf, tests: Add tail call test suite"), we can
see there exists failed testcase.
On all archs when CONFIG_BPF_JIT_ALWAYS_ON is not set:
# echo 0 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:0 ret 34 != 33 FAIL
On some archs:
# echo 1 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:1 ret 34 != 33 FAIL
So it is necessary to change the value of MAX_TAIL_CALL_CNT from 32 to 33,
then do some small changes of the related code.
With this patch, it does not change the current limit 33, MAX_TAIL_CALL_CNT
can reflect the actual max tail call count, the tailcall selftests can work
well, and also the above failed testcase in test_bpf can be fixed for the
interpreter (all archs) and the JIT (all archs except for x86).
# uname -m
x86_64
# echo 1 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:1 ret 33 != 34 FAIL
Signed-off-by: Tiezhu Yang
---
v2:
-- fix the typos in the commit message and update the commit message.
-- fix the failed tailcall selftests for x86 jit.
I am not quite sure the change on x86 is proper, with this change,
tailcall selftests passed, but tailcall limit test in test_bpf.ko
failed, I do not know the reason now, I think this is another issue,
maybe someone more versed in x86 jit could take a look.
arch/arm/net/bpf_jit_32.c | 11 ++-
arch/arm64/net/bpf_jit_comp.c | 7 ---
arch/mips/net/ebpf_jit.c | 4 ++--
arch/powerpc/net/bpf_jit_comp32.c | 4 ++--
arch/powerpc/net/bpf_jit_comp64.c | 12 ++--
arch/riscv/net/bpf_jit_comp32.c | 4 ++--
arch/riscv/net/bpf_jit_comp64.c | 4 ++--
arch/sparc/net/bpf_jit_comp_64.c | 8
arch/x86/net/bpf_jit_comp.c | 10 +-
include/linux/bpf.h | 2 +-
kernel/bpf/core.c | 4 ++--
11 files changed, 36 insertions(+), 34 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a951276..39d9ae9 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1180,18 +1180,19 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
/* tmp2[0] = array, tmp2[1] = index */
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
-* goto out;
+ /*
* tail_call_cnt++;
+* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+* goto out;
*/
+ tc = arm_bpf_get_reg64(tcc, tmp, ctx);
+ emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
+ emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
lo = (u32)MAX_TAIL_CALL_CNT;
hi = (u32)((u64)MAX_TAIL_CALL_CNT >> 32);
- tc = arm_bpf_get_reg64(tcc, tmp, ctx);
emit(ARM_CMP_I(tc[0], hi), ctx);
_emit(ARM_COND_EQ, ARM_CMP_I(tc[1], lo), ctx);
_emit(ARM_COND_HI, ARM_B(jmp_offset), ctx);
- emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
- emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
arm_bpf_put_reg64(tcc, tmp, ctx);
/* prog = array->ptrs[index]
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 41c23f4..5d6c843 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -286,14 +286,15 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
emit(A64_CMP(0, r3, tmp), ctx);
emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
-* goto out;
+ /*
* tail_call_cnt++;
+* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+* goto out;
*/
+ emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
emit_a64_mov_i64(tmp, MAX_TAIL_CALL_CNT, ctx);
emit(A64_CMP(1, tcc, tmp), ctx);
emit(A64_B_(A64_COND_HI, jmp_offset), ctx);
- emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
/* prog = array->ptrs[index];
* if (prog == NULL)
diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c
index 3a73e93..029fc34 100644
--- a/arch/mips/net/ebpf_jit.c
+++ b/arch/mips/net/ebpf_jit.c
@@ -617,14 +617,14 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx, int
this_idx)
b_off = b_imm(this_idx + 1, c
[PATCH bpf-next] bpf: Change value of MAX_TAIL_CALL_CNT from 32 to 33
In the current code, the actual max tail call count is 33 which is greater
than MAX_TAIL_CALL_CNT (defined as 32), the actual limit is not consistent
with the meaning of MAX_TAIL_CALL_CNT, there is some confusion and need to
spend some time to think the reason at the first glance.
We can see the historical evolution from commit 04fd61ab36ec ("bpf: allow
bpf programs to tail-call other bpf programs") and commit f9dabe016b63
("bpf: Undo off-by-one in interpreter tail call count limit").
In order to avoid changing existing behavior, the actual limit is 33 now,
this is resonable.
After commit 874be05f525e ("bpf, tests: Add tail call test suite"), we can
see there exists failed testcase.
On all archs when CONFIG_BPF_JIT_ALWAYS_ON is not set:
# echo 0 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:0 ret 34 != 33 FAIL
On some archs:
# echo 1 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:1 ret 34 != 33 FAIL
So it is necessary to change the value of MAX_TAIL_CALL_CNT from 32 to 33,
then do some small changes of the related code.
With this patch, it does not change the current limit, MAX_TAIL_CALL_CNT
can reflect the actual max tail call count, and the above failed testcase
can be fixed.
Signed-off-by: Tiezhu Yang
---
arch/arm/net/bpf_jit_32.c | 11 ++-
arch/arm64/net/bpf_jit_comp.c | 7 ---
arch/mips/net/ebpf_jit.c | 4 ++--
arch/powerpc/net/bpf_jit_comp32.c | 4 ++--
arch/powerpc/net/bpf_jit_comp64.c | 12 ++--
arch/riscv/net/bpf_jit_comp32.c | 4 ++--
arch/riscv/net/bpf_jit_comp64.c | 4 ++--
arch/sparc/net/bpf_jit_comp_64.c | 8
include/linux/bpf.h | 2 +-
kernel/bpf/core.c | 4 ++--
10 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a951276..39d9ae9 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1180,18 +1180,19 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
/* tmp2[0] = array, tmp2[1] = index */
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
-* goto out;
+ /*
* tail_call_cnt++;
+* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+* goto out;
*/
+ tc = arm_bpf_get_reg64(tcc, tmp, ctx);
+ emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
+ emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
lo = (u32)MAX_TAIL_CALL_CNT;
hi = (u32)((u64)MAX_TAIL_CALL_CNT >> 32);
- tc = arm_bpf_get_reg64(tcc, tmp, ctx);
emit(ARM_CMP_I(tc[0], hi), ctx);
_emit(ARM_COND_EQ, ARM_CMP_I(tc[1], lo), ctx);
_emit(ARM_COND_HI, ARM_B(jmp_offset), ctx);
- emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
- emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
arm_bpf_put_reg64(tcc, tmp, ctx);
/* prog = array->ptrs[index]
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 41c23f4..5d6c843 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -286,14 +286,15 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
emit(A64_CMP(0, r3, tmp), ctx);
emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
-* goto out;
+ /*
* tail_call_cnt++;
+* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+* goto out;
*/
+ emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
emit_a64_mov_i64(tmp, MAX_TAIL_CALL_CNT, ctx);
emit(A64_CMP(1, tcc, tmp), ctx);
emit(A64_B_(A64_COND_HI, jmp_offset), ctx);
- emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
/* prog = array->ptrs[index];
* if (prog == NULL)
diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c
index 3a73e93..029fc34 100644
--- a/arch/mips/net/ebpf_jit.c
+++ b/arch/mips/net/ebpf_jit.c
@@ -617,14 +617,14 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx, int
this_idx)
b_off = b_imm(this_idx + 1, ctx);
emit_instr(ctx, bne, MIPS_R_AT, MIPS_R_ZERO, b_off);
/*
-* if (TCC-- < 0)
+* if (--TCC < 0)
* goto out;
*/
/* Delay slot */
tcc_reg = (ctx->flags & EBPF_TCC_IN_V1) ? MIPS_R_V1 : MIPS_R_S4;
emit_instr(ctx, daddiu, MIPS_R_T5, tcc_reg, -1);
b_off = b_imm(this_idx + 1, ctx);
- emit_instr(ctx, bltz, tcc_reg, b_off);
+ emit_instr(ctx, bltz, MIPS_R_T5, b_off);
/*
* prog = array->ptrs[index];
* if (prog == NULL)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c
b/arch/powerpc/net/bpf_jit_comp32.c
index beb12cb..b5585ad 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -221,12 +221,12 @@ stat
[RFC PATCH bpf-next] bpf: Make actual max tail call count as MAX_TAIL_CALL_CNT
In the current code, the actual max tail call count is 33 which is greater
than MAX_TAIL_CALL_CNT, this is not consistent with the intended meaning
in the commit 04fd61ab36ec ("bpf: allow bpf programs to tail-call other
bpf programs"):
"The chain of tail calls can form unpredictable dynamic loops therefore
tail_call_cnt is used to limit the number of calls and currently is set
to 32."
Additionally, after commit 874be05f525e ("bpf, tests: Add tail call test
suite"), we can see there exists failed testcase.
On all archs when CONFIG_BPF_JIT_ALWAYS_ON is not set:
# echo 0 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:0 ret 34 != 33 FAIL
On some archs:
# echo 1 > /proc/sys/net/core/bpf_jit_enable
# modprobe test_bpf
# dmesg | grep -w FAIL
Tail call error path, max count reached jited:1 ret 34 != 33 FAIL
with this patch, make the actual max tail call count as MAX_TAIL_CALL_CNT,
at the same time, the above failed testcase can be fixed.
Signed-off-by: Tiezhu Yang
---
Hi all,
This is a RFC patch, if I am wrong or I missed something,
please let me know, thank you!
arch/arm/net/bpf_jit_32.c | 11 ++-
arch/arm64/net/bpf_jit_comp.c | 7 ---
arch/mips/net/ebpf_jit.c | 4 ++--
arch/powerpc/net/bpf_jit_comp32.c | 4 ++--
arch/powerpc/net/bpf_jit_comp64.c | 12 ++--
arch/riscv/net/bpf_jit_comp32.c | 4 ++--
arch/riscv/net/bpf_jit_comp64.c | 4 ++--
arch/sparc/net/bpf_jit_comp_64.c | 8
kernel/bpf/core.c | 4 ++--
9 files changed, 30 insertions(+), 28 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index a951276..39d9ae9 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -1180,18 +1180,19 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
/* tmp2[0] = array, tmp2[1] = index */
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
-* goto out;
+ /*
* tail_call_cnt++;
+* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+* goto out;
*/
+ tc = arm_bpf_get_reg64(tcc, tmp, ctx);
+ emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
+ emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
lo = (u32)MAX_TAIL_CALL_CNT;
hi = (u32)((u64)MAX_TAIL_CALL_CNT >> 32);
- tc = arm_bpf_get_reg64(tcc, tmp, ctx);
emit(ARM_CMP_I(tc[0], hi), ctx);
_emit(ARM_COND_EQ, ARM_CMP_I(tc[1], lo), ctx);
_emit(ARM_COND_HI, ARM_B(jmp_offset), ctx);
- emit(ARM_ADDS_I(tc[1], tc[1], 1), ctx);
- emit(ARM_ADC_I(tc[0], tc[0], 0), ctx);
arm_bpf_put_reg64(tcc, tmp, ctx);
/* prog = array->ptrs[index]
diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c
index 41c23f4..5d6c843 100644
--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
@@ -286,14 +286,15 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx)
emit(A64_CMP(0, r3, tmp), ctx);
emit(A64_B_(A64_COND_CS, jmp_offset), ctx);
- /* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
-* goto out;
+ /*
* tail_call_cnt++;
+* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
+* goto out;
*/
+ emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
emit_a64_mov_i64(tmp, MAX_TAIL_CALL_CNT, ctx);
emit(A64_CMP(1, tcc, tmp), ctx);
emit(A64_B_(A64_COND_HI, jmp_offset), ctx);
- emit(A64_ADD_I(1, tcc, tcc, 1), ctx);
/* prog = array->ptrs[index];
* if (prog == NULL)
diff --git a/arch/mips/net/ebpf_jit.c b/arch/mips/net/ebpf_jit.c
index 3a73e93..029fc34 100644
--- a/arch/mips/net/ebpf_jit.c
+++ b/arch/mips/net/ebpf_jit.c
@@ -617,14 +617,14 @@ static int emit_bpf_tail_call(struct jit_ctx *ctx, int
this_idx)
b_off = b_imm(this_idx + 1, ctx);
emit_instr(ctx, bne, MIPS_R_AT, MIPS_R_ZERO, b_off);
/*
-* if (TCC-- < 0)
+* if (--TCC < 0)
* goto out;
*/
/* Delay slot */
tcc_reg = (ctx->flags & EBPF_TCC_IN_V1) ? MIPS_R_V1 : MIPS_R_S4;
emit_instr(ctx, daddiu, MIPS_R_T5, tcc_reg, -1);
b_off = b_imm(this_idx + 1, ctx);
- emit_instr(ctx, bltz, tcc_reg, b_off);
+ emit_instr(ctx, bltz, MIPS_R_T5, b_off);
/*
* prog = array->ptrs[index];
* if (prog == NULL)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c
b/arch/powerpc/net/bpf_jit_comp32.c
index beb12cb..b5585ad 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -221,12 +221,12 @@ static void bpf_jit_emit_tail_call(u32 *image, struct
codegen_context *ctx, u32
PPC_BCC(COND_GE, out);
/*
+* tail_call_cnt++;
* if (tail_call_cnt > MAX_TAIL_CALL_CNT)
* goto out;
*/
- EMIT(PPC_RAW_CMPLWI(_R0, MAX_TAIL_CA
