[PATCH v6 0/9] Add jump table support for objtool on LoongArch

[PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Tiezhu Yang]

This version is based on tip/tip.git objtool/core branch [1], add some weak
and arch-specific functions to make the generic code more readable, tested
with the latest upstream mainline Binutils, GCC and Clang.

The first 6 patches are preparation for patch #7 to enable jump table for
objtool on LoongArch, the last 2 patches are small enough to fix objtool
warnings "funcA() falls through to next function funcB()", one is under
arch/loongarch and the other is under drm/amd/display.

v6:
  -- Add arch_reloc_size() for x86 and ppc.
  -- Call arch_reloc_size() directly in add_jump_table().
  -- Refine arch_adjust_offset() for LoongArch.
  -- Rename arch_adjust_offset() to arch_jump_table_sym_offset(). 
  -- Get each table size of rodata in time for switch table.
  -- Update the commit message to make it more clear.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core

Tiezhu Yang (9):
  objtool: Handle various symbol types of rodata
  objtool: Handle different entry size of rodata
  objtool: Handle PC relative relocation type
  objtool: Handle unreachable entry of rodata
  objtool/LoongArch: Add support for switch table
  objtool/LoongArch: Add support for goto table
  LoongArch: Enable jump table for objtool
  LoongArch: Convert unreachable() to BUG()
  drm/amd/display: Mark dc_fixpt_from_fraction() noinline

 arch/loongarch/Kconfig                        |   3 +
 arch/loongarch/Makefile                       |   6 +-
 arch/loongarch/kernel/machine_kexec.c         |   4 +-
 .../drm/amd/display/dc/basics/fixpt31_32.c    |   2 +-
 tools/objtool/arch/loongarch/decode.c         |  28 ++-
 .../objtool/arch/loongarch/include/arch/elf.h |   7 +
 tools/objtool/arch/loongarch/special.c        | 159 +++++++++++++++++-
 tools/objtool/arch/powerpc/decode.c           |  15 ++
 tools/objtool/arch/x86/decode.c               |  13 ++
 tools/objtool/check.c                         |  28 ++-
 tools/objtool/include/objtool/arch.h          |   3 +
 11 files changed, 251 insertions(+), 17 deletions(-)

-- 
2.42.0

[PATCH v6 1/9] objtool: Handle various symbol types of rodata

[Tiezhu Yang]

In the relocation section ".rela.rodata" of each .o file compiled with
LoongArch toolchain, there are various symbol types such as STT_NOTYPE,
STT_OBJECT, STT_FUNC in addition to the usual STT_SECTION, it needs to
use reloc symbol offset instead of reloc addend to find the destination
instruction in find_jump_table() and add_jump_table().

For the most part, an absolute relocation type is used for rodata. In the
case of STT_SECTION, reloc->sym->offset is always zero, and for the other
symbol types, reloc_addend(reloc) is always zero, thus it can use a simple
statement "reloc->sym->offset + reloc_addend(reloc)" to obtain the symbol
offset for various symbol types.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/check.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e92c5564d9ca..f64435ad3514 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1953,6 +1953,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 	unsigned int prev_offset = 0;
 	struct reloc *reloc = table;
 	struct alternative *alt;
+	unsigned long sym_offset;
 
 	/*
 	 * Each @reloc is a switch table relocation which points to the target
@@ -1970,12 +1971,13 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
 			break;
 
+		sym_offset = reloc->sym->offset + reloc_addend(reloc);
+
 		/* Detect function pointers from contiguous objects: */
-		if (reloc->sym->sec == pfunc->sec &&
-		    reloc_addend(reloc) == pfunc->offset)
+		if (reloc->sym->sec == pfunc->sec && sym_offset == pfunc->offset)
 			break;
 
-		dest_insn = find_insn(file, reloc->sym->sec, reloc_addend(reloc));
+		dest_insn = find_insn(file, reloc->sym->sec, sym_offset);
 		if (!dest_insn)
 			break;
 
@@ -2013,6 +2015,7 @@ static void find_jump_table(struct objtool_file *file, struct symbol *func,
 	struct reloc *table_reloc;
 	struct instruction *dest_insn, *orig_insn = insn;
 	unsigned long table_size;
+	unsigned long sym_offset;
 
 	/*
 	 * Backward search using the @first_jump_src links, these help avoid
@@ -2036,7 +2039,10 @@ static void find_jump_table(struct objtool_file *file, struct symbol *func,
 		table_reloc = arch_find_switch_table(file, insn, &table_size);
 		if (!table_reloc)
 			continue;
-		dest_insn = find_insn(file, table_reloc->sym->sec, reloc_addend(table_reloc));
+
+		sym_offset = table_reloc->sym->offset + reloc_addend(table_reloc);
+
+		dest_insn = find_insn(file, table_reloc->sym->sec, sym_offset);
 		if (!dest_insn || !insn_func(dest_insn) || insn_func(dest_insn)->pfunc != func)
 			continue;
 
-- 
2.42.0

[PATCH v6 2/9] objtool: Handle different entry size of rodata

[Tiezhu Yang]

In the most cases, the entry size of rodata is 8 bytes because the
relocation type is 64 bit. There are also 32 bit relocation types,
the entry size of rodata should be 4 bytes in this case.

Add an arch-specific function arch_reloc_size() to assign the entry
size of rodata for x86, powerpc and LoongArch.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/decode.c | 11 +++++++++++
 tools/objtool/arch/powerpc/decode.c   | 15 +++++++++++++++
 tools/objtool/arch/x86/decode.c       | 13 +++++++++++++
 tools/objtool/check.c                 |  2 +-
 tools/objtool/include/objtool/arch.h  |  2 ++
 5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index 69b66994f2a1..b64205b89f6b 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -363,3 +363,14 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state)
 	state->cfa.base = CFI_SP;
 	state->cfa.offset = 0;
 }
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+	switch (reloc_type(reloc)) {
+	case R_LARCH_32:
+	case R_LARCH_32_PCREL:
+		return 4;
+	default:
+		return 8;
+	}
+}
diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 53b55690f320..3c95dd74fca0 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -106,3 +106,18 @@ void arch_initial_func_cfi_state(struct cfi_init_state *state)
 	state->regs[CFI_RA].base = CFI_CFA;
 	state->regs[CFI_RA].offset = 0;
 }
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+	switch (reloc_type(reloc)) {
+	case R_PPC_REL32:
+	case R_PPC64_REL32:
+	case R_PPC_ADDR32:
+	case R_PPC_UADDR32:
+	case R_PPC_PLT32:
+	case R_PPC_PLTREL32:
+		return 4;
+	default:
+		return 8;
+	}
+}
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index fe1362c34564..fb9691a34d92 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -852,3 +852,16 @@ bool arch_is_embedded_insn(struct symbol *sym)
 	return !strcmp(sym->name, "retbleed_return_thunk") ||
 	       !strcmp(sym->name, "srso_safe_ret");
 }
+
+unsigned int arch_reloc_size(struct reloc *reloc)
+{
+	switch (reloc_type(reloc)) {
+	case R_X86_64_32:
+	case R_X86_64_32S:
+	case R_X86_64_PC32:
+	case R_X86_64_PLT32:
+		return 4;
+	default:
+		return 8;
+	}
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f64435ad3514..d8668ae0f599 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1968,7 +1968,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 			break;
 
 		/* Make sure the table entries are consecutive: */
-		if (prev_offset && reloc_offset(reloc) != prev_offset + 8)
+		if (prev_offset && reloc_offset(reloc) != prev_offset + arch_reloc_size(reloc))
 			break;
 
 		sym_offset = reloc->sym->offset + reloc_addend(reloc);
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index d63b46a19f39..396f7c6c81c0 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -97,4 +97,6 @@ int arch_rewrite_retpolines(struct objtool_file *file);
 
 bool arch_pc_relative_reloc(struct reloc *reloc);
 
+unsigned int arch_reloc_size(struct reloc *reloc);
+
 #endif /* _ARCH_H */
-- 
2.42.0

[PATCH] objtool: remove duplicate case value R_PPC64_REL32

[Kexy Biscuit]

In arch/powerpc/include/uapi/asm/elf.h, R_PPC64_REL32 is defined as a
macro to R_PPC_REL32, makes the case value here being duplicate and
creates the following error...

arch/powerpc/decode.c: In function ‘arch_reloc_size’:
arch/powerpc/decode.c:114:9: error: duplicate case value
  114 |         case R_PPC64_REL32:
      |         ^~~~
arch/powerpc/decode.c:113:9: note: previously used here
  113 |         case R_PPC_REL32:
      |         ^~~~

Remove the duplicate case value to fix the error.

Fixes: "FROMLIST: objtool: Handle different entry size of rodata"
Signed-off-by: Kexy Biscuit <kexybiscuit@aosc.io>
---
This patch is required for this series to build on powerpc, however I'm
not sure if it's the preferred way... Please advise.

 tools/objtool/arch/powerpc/decode.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/powerpc/decode.c
index 3c95dd74fca0..7c0bf2429067 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -111,7 +111,6 @@ unsigned int arch_reloc_size(struct reloc *reloc)
 {
 	switch (reloc_type(reloc)) {
 	case R_PPC_REL32:
-	case R_PPC64_REL32:
 	case R_PPC_ADDR32:
 	case R_PPC_UADDR32:
 	case R_PPC_PLT32:
-- 
2.48.1

Re: [PATCH] objtool: remove duplicate case value R_PPC64_REL32

[Josh Poimboeuf]

On Tue, Feb 11, 2025 at 10:19:57PM +0800, Kexy Biscuit wrote:
> In arch/powerpc/include/uapi/asm/elf.h, R_PPC64_REL32 is defined as a
> macro to R_PPC_REL32, makes the case value here being duplicate and
> creates the following error...
> 
> arch/powerpc/decode.c: In function ‘arch_reloc_size’:
> arch/powerpc/decode.c:114:9: error: duplicate case value
>   114 |         case R_PPC64_REL32:
>       |         ^~~~
> arch/powerpc/decode.c:113:9: note: previously used here
>   113 |         case R_PPC_REL32:
>       |         ^~~~
> 
> Remove the duplicate case value to fix the error.
> 
> Fixes: "FROMLIST: objtool: Handle different entry size of rodata"
> Signed-off-by: Kexy Biscuit <kexybiscuit@aosc.io>
> ---
> This patch is required for this series to build on powerpc, however I'm
> not sure if it's the preferred way... Please advise.

If there are no objections, I'll squash this into the original patch to
avoid breaking bisection.

-- 
Josh

[PATCH v6 3/9] objtool: Handle PC relative relocation type

[Tiezhu Yang]

For the most part, an absolute relocation type is used for rodata.
In the case of STT_SECTION, reloc->sym->offset is always zero, for
the other symbol types, reloc_addend(reloc) is always zero, thus it
can use a simple statement "reloc->sym->offset + reloc_addend(reloc)"
to obtain the symbol offset for various symbol types.

When compiling on LoongArch, there exist PC relative relocation types
for rodata, it needs to calculate the symbol offset with "S + A - PC"
according to the spec of "ELF for the LoongArch Architecture".

If there is only one jump table in the rodata, the "PC" is the entry
address which is equal with the value of reloc_offset(reloc), at this
time, reloc_offset(table) is 0.

If there are many jump tables in the rodata, the "PC" is the offset
of the jump table's base address which is equal with the value of
reloc_offset(reloc) - reloc_offset(table).

So for LoongArch, if the relocation type is PC relative, it can use a
statement "reloc_offset(reloc) - reloc_offset(table)" to get the "PC"
value when calculating the symbol offset with "S + A - PC" for one or
many jump tables in the rodata.

Add an arch-specific function arch_jump_table_sym_offset() to assign
the symbol offset, for the most part that is an absolute relocation,
the default value is "reloc->sym->offset + reloc_addend(reloc)" in
the weak definition, it can be overridden by each architecture that
has different requirements.

Link: https://github.com/loongson/la-abi-specs/blob/release/laelf.adoc
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/decode.c           | 17 +++++++++++++----
 tools/objtool/arch/loongarch/include/arch/elf.h |  7 +++++++
 tools/objtool/check.c                           |  7 ++++++-
 tools/objtool/include/objtool/arch.h            |  1 +
 4 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/tools/objtool/arch/loongarch/decode.c b/tools/objtool/arch/loongarch/decode.c
index b64205b89f6b..02e490555966 100644
--- a/tools/objtool/arch/loongarch/decode.c
+++ b/tools/objtool/arch/loongarch/decode.c
@@ -5,10 +5,7 @@
 #include <asm/inst.h>
 #include <asm/orc_types.h>
 #include <linux/objtool_types.h>
-
-#ifndef EM_LOONGARCH
-#define EM_LOONGARCH	258
-#endif
+#include <arch/elf.h>
 
 int arch_ftrace_match(char *name)
 {
@@ -374,3 +371,15 @@ unsigned int arch_reloc_size(struct reloc *reloc)
 		return 8;
 	}
 }
+
+unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table)
+{
+	switch (reloc_type(reloc)) {
+	case R_LARCH_32_PCREL:
+	case R_LARCH_64_PCREL:
+		return reloc->sym->offset + reloc_addend(reloc) -
+		       (reloc_offset(reloc) - reloc_offset(table));
+	default:
+		return reloc->sym->offset + reloc_addend(reloc);
+	}
+}
diff --git a/tools/objtool/arch/loongarch/include/arch/elf.h b/tools/objtool/arch/loongarch/include/arch/elf.h
index 9623d663220e..ec79062c9554 100644
--- a/tools/objtool/arch/loongarch/include/arch/elf.h
+++ b/tools/objtool/arch/loongarch/include/arch/elf.h
@@ -18,6 +18,13 @@
 #ifndef R_LARCH_32_PCREL
 #define R_LARCH_32_PCREL	99
 #endif
+#ifndef R_LARCH_64_PCREL
+#define R_LARCH_64_PCREL	109
+#endif
+
+#ifndef EM_LOONGARCH
+#define EM_LOONGARCH		258
+#endif
 
 #define R_NONE			R_LARCH_NONE
 #define R_ABS32			R_LARCH_32
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index d8668ae0f599..cff7416b207e 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1943,6 +1943,11 @@ static int add_special_section_alts(struct objtool_file *file)
 	return ret;
 }
 
+__weak unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table)
+{
+	return reloc->sym->offset + reloc_addend(reloc);
+}
+
 static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 			  struct reloc *next_table)
 {
@@ -1971,7 +1976,7 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		if (prev_offset && reloc_offset(reloc) != prev_offset + arch_reloc_size(reloc))
 			break;
 
-		sym_offset = reloc->sym->offset + reloc_addend(reloc);
+		sym_offset = arch_jump_table_sym_offset(reloc, table);
 
 		/* Detect function pointers from contiguous objects: */
 		if (reloc->sym->sec == pfunc->sec && sym_offset == pfunc->offset)
diff --git a/tools/objtool/include/objtool/arch.h b/tools/objtool/include/objtool/arch.h
index 396f7c6c81c0..089a1acc48a8 100644
--- a/tools/objtool/include/objtool/arch.h
+++ b/tools/objtool/include/objtool/arch.h
@@ -98,5 +98,6 @@ int arch_rewrite_retpolines(struct objtool_file *file);
 bool arch_pc_relative_reloc(struct reloc *reloc);
 
 unsigned int arch_reloc_size(struct reloc *reloc);
+unsigned long arch_jump_table_sym_offset(struct reloc *reloc, struct reloc *table);
 
 #endif /* _ARCH_H */
-- 
2.42.0

[PATCH v6 4/9] objtool: Handle unreachable entry of rodata

[Tiezhu Yang]

When compiling with Clang on LoongArch, there exists unreachable entry of
rodata which points to a position after the function return instruction,
this is generated by compiler to fill the non-existent switch case, just
skip the entry when parsing the relocation section of rodata.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/check.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index cff7416b207e..654cffcf9512 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1986,9 +1986,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
 		if (!dest_insn)
 			break;
 
-		/* Make sure the destination is in the same function: */
-		if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
-			break;
+		if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc) {
+			prev_offset = reloc_offset(reloc);
+			continue;
+		}
 
 		alt = malloc(sizeof(*alt));
 		if (!alt) {
-- 
2.42.0

Re: [PATCH v6 4/9] objtool: Handle unreachable entry of rodata

[Josh Poimboeuf]

On Tue, Dec 17, 2024 at 09:09:00AM +0800, Tiezhu Yang wrote:
> When compiling with Clang on LoongArch, there exists unreachable entry of
> rodata which points to a position after the function return instruction,
> this is generated by compiler to fill the non-existent switch case, just
> skip the entry when parsing the relocation section of rodata.
> 
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  tools/objtool/check.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index cff7416b207e..654cffcf9512 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1986,9 +1986,10 @@ static int add_jump_table(struct objtool_file *file, struct instruction *insn,
>  		if (!dest_insn)
>  			break;
>  
> -		/* Make sure the destination is in the same function: */
> -		if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc)
> -			break;
> +		if (!insn_func(dest_insn) || insn_func(dest_insn)->pfunc != pfunc) {
> +			prev_offset = reloc_offset(reloc);
> +			continue;
> +		}

This patch can be dropped, this is already fixed in the -tip tree:

  3724062ca2b1 ("objtool: Ignore dangling jump table entries")

-- 
Josh

[PATCH v6 5/9] objtool/LoongArch: Add support for switch table

[Tiezhu Yang]

The objtool program need to analysis the control flow of each object file
generated by compiler toolchain, it needs to know all the locations that
a branch instruction may jump into, if a jump table is used, objtool has
to correlate the jump instruction with the table.

On x86 (which is the only port supported by objtool before LoongArch),
there is a relocation type on the jump instruction and directly points
to the table. But on LoongArch, the relocation is on another kind of
instruction prior to the jump instruction, and also with scheduling it
is not very easy to tell the offset of that instruction from the jump
instruction. Furthermore, because LoongArch has -fsection-anchors (often
enabled at -O1 or above) the relocation may actually points to a section
anchor instead of the table itself.

The good news is that after continuous analysis and discussion, at last
a GCC patch "LoongArch: Add support to annotate tablejump" and a Clang
patch "[LoongArch] Add options for annotate tablejump" have been merged
into the upstream mainline, the compiler changes make life much easier
for switch table support of objtool on LoongArch.

By now, there is an additional section ".discard.tablejump_annotate" to
store the jump info as pairs of addresses, each pair contains the address
of jump instruction and the address of jump table.

In order to find switch table, it is easy to parse the relocation section
".rela.discard.tablejump_annotate" to get table_sec and table_offset, the
rest process is somehow like x86.

Additionally, it needs to get each table size. When compiling on LoongArch,
there are unsorted table offsets of rodata if there exist many jump tables,
it will get the wrong table end and find the wrong table jump destination
instructions in add_jump_table().

Sort the rodata table offset by parsing ".rela.discard.tablejump_annotate"
and then get each table size of rodata corresponded with each table jump
instruction, it is used to check the table end and will break the process
when parsing ".rela.rodata" to avoid getting the wrong jump destination
instructions.

Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=0ee028f55640
Link: https://github.com/llvm/llvm-project/commit/4c2c17756739
Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/special.c | 131 ++++++++++++++++++++++++-
 1 file changed, 130 insertions(+), 1 deletion(-)

diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 87230ed570fd..4fa6877d5b2b 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
+#include <string.h>
 #include <objtool/special.h>
+#include <objtool/warn.h>
 
 bool arch_support_alt_relocation(struct special_alt *special_alt,
 				 struct instruction *insn,
@@ -8,9 +10,136 @@ bool arch_support_alt_relocation(struct special_alt *special_alt,
 	return false;
 }
 
+struct table_info {
+	struct list_head jump_info;
+	unsigned long insn_offset;
+	unsigned long rodata_offset;
+};
+
+static void get_rodata_table_size_by_table_annotate(struct objtool_file *file,
+						    struct instruction *insn,
+						    unsigned long *table_size)
+{
+	struct section *rsec;
+	struct reloc *reloc;
+	struct list_head table_list;
+	struct table_info *orig_table;
+	struct table_info *next_table;
+	unsigned long tmp_insn_offset;
+	unsigned long tmp_rodata_offset;
+
+	rsec = find_section_by_name(file->elf, ".rela.discard.tablejump_annotate");
+	if (!rsec)
+		return;
+
+	INIT_LIST_HEAD(&table_list);
+
+	for_each_reloc(rsec, reloc) {
+		orig_table = malloc(sizeof(struct table_info));
+		if (!orig_table) {
+			WARN("malloc failed");
+			return;
+		}
+
+		orig_table->insn_offset = reloc->sym->offset + reloc_addend(reloc);
+		reloc++;
+		orig_table->rodata_offset = reloc->sym->offset + reloc_addend(reloc);
+
+		list_add_tail(&orig_table->jump_info, &table_list);
+
+		if (reloc_idx(reloc) + 1 == sec_num_entries(rsec))
+			break;
+	}
+
+	list_for_each_entry(orig_table, &table_list, jump_info) {
+		next_table = list_next_entry(orig_table, jump_info);
+		list_for_each_entry_from(next_table, &table_list, jump_info) {
+			if (next_table->rodata_offset < orig_table->rodata_offset) {
+				tmp_insn_offset = next_table->insn_offset;
+				tmp_rodata_offset = next_table->rodata_offset;
+				next_table->insn_offset = orig_table->insn_offset;
+				next_table->rodata_offset = orig_table->rodata_offset;
+				orig_table->insn_offset = tmp_insn_offset;
+				orig_table->rodata_offset = tmp_rodata_offset;
+			}
+		}
+	}
+
+	list_for_each_entry(orig_table, &table_list, jump_info) {
+		if (insn->offset == orig_table->insn_offset) {
+			next_table = list_next_entry(orig_table, jump_info);
+			if (&next_table->jump_info == &table_list) {
+				*table_size = 0;
+				return;
+			}
+
+			while (next_table->rodata_offset == orig_table->rodata_offset) {
+				next_table = list_next_entry(next_table, jump_info);
+				if (&next_table->jump_info == &table_list) {
+					*table_size = 0;
+					return;
+				}
+			}
+
+			*table_size = next_table->rodata_offset - orig_table->rodata_offset;
+		}
+	}
+}
+
+static struct reloc *find_reloc_by_table_annotate(struct objtool_file *file,
+						  struct instruction *insn,
+						  unsigned long *table_size)
+{
+	struct section *rsec;
+	struct reloc *reloc;
+	unsigned long offset;
+
+	rsec = find_section_by_name(file->elf, ".rela.discard.tablejump_annotate");
+	if (!rsec)
+		return NULL;
+
+	for_each_reloc(rsec, reloc) {
+		if (reloc->sym->sec->rodata)
+			continue;
+
+		if (strcmp(insn->sec->name, reloc->sym->sec->name))
+			continue;
+
+		offset = reloc->sym->offset + reloc_addend(reloc);
+		if (insn->offset == offset) {
+			get_rodata_table_size_by_table_annotate(file, insn, table_size);
+			reloc++;
+			return reloc;
+		}
+	}
+
+	return NULL;
+}
+
 struct reloc *arch_find_switch_table(struct objtool_file *file,
 				     struct instruction *insn,
 				     unsigned long *table_size)
 {
-	return NULL;
+	struct reloc *annotate_reloc;
+	struct reloc *rodata_reloc;
+	struct section *table_sec;
+	unsigned long table_offset;
+
+	annotate_reloc = find_reloc_by_table_annotate(file, insn, table_size);
+	if (!annotate_reloc)
+		return NULL;
+
+	table_sec = annotate_reloc->sym->sec;
+	table_offset = annotate_reloc->sym->offset + reloc_addend(annotate_reloc);
+
+	/*
+	 * Each table entry has a rela associated with it.  The rela
+	 * should reference text in the same function as the original
+	 * instruction.
+	 */
+	rodata_reloc = find_reloc_by_dest(file->elf, table_sec, table_offset);
+	if (!rodata_reloc)
+		return NULL;
+
+	return rodata_reloc;
 }
-- 
2.42.0

[PATCH v6 6/9] objtool/LoongArch: Add support for goto table

[Tiezhu Yang]

The objtool program need to analysis the control flow of each object file
generated by compiler toolchain, it needs to know all the locations that
a branch instruction may jump into, if a jump table is used, objtool has
to correlate the jump instruction with the table.

On x86 (which is the only port supported by objtool before LoongArch),
there is a relocation type on the jump instruction and directly points
to the table. But on LoongArch, the relocation is on another kind of
instruction prior to the jump instruction, and also with scheduling it
is not very easy to tell the offset of that instruction from the jump
instruction. Furthermore, because LoongArch has -fsection-anchors (often
enabled at -O1 or above) the relocation may actually points to a section
anchor instead of the table itself.

For the jump table of switch cases, a GCC patch "LoongArch: Add support
to annotate tablejump" and a Clang patch "[LoongArch] Add options for
annotate tablejump" have been merged into the upstream mainline, it can
parse the additional section ".discard.tablejump_annotate" which stores
the jump info as pairs of addresses, each pair contains the address of
jump instruction and the address of jump table.

For the jump table of computed gotos, it is indeed not easy to implement
in the compiler, especially if there is more than one computed goto in a
function such as ___bpf_prog_run(). objdump kernel/bpf/core.o shows that
there are many table jump instructions in ___bpf_prog_run(), but there are
no relocations on the table jump instructions and to the table directly on
LoongArch.

Without the help of compiler, in order to figure out the address of goto
table for the special case of ___bpf_prog_run(), since the instruction
sequence is relatively single and stable, it makes sense to add a helper
find_reloc_of_rodata_c_jump_table() to find the relocation which points
to the section ".rodata..c_jump_table".

If find_reloc_by_table_annotate() failed, it means there is no relocation
info of switch table address in ".rela.discard.tablejump_annotate", then
objtool may find the relocation info of goto table ".rodata..c_jump_table"
with find_reloc_of_rodata_c_jump_table().

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 tools/objtool/arch/loongarch/special.c | 32 ++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/objtool/arch/loongarch/special.c b/tools/objtool/arch/loongarch/special.c
index 4fa6877d5b2b..27c6473608f3 100644
--- a/tools/objtool/arch/loongarch/special.c
+++ b/tools/objtool/arch/loongarch/special.c
@@ -116,6 +116,30 @@ static struct reloc *find_reloc_by_table_annotate(struct objtool_file *file,
 	return NULL;
 }
 
+static struct reloc *find_reloc_of_rodata_c_jump_table(struct section *sec,
+						       unsigned long offset,
+						       unsigned long *table_size)
+{
+	struct section *rsec;
+	struct reloc *reloc;
+
+	rsec = sec->rsec;
+	if (!rsec)
+		return NULL;
+
+	for_each_reloc(rsec, reloc) {
+		if (reloc_offset(reloc) > offset)
+			break;
+
+		if (!strncmp(reloc->sym->sec->name, ".rodata..c_jump_table", 21)) {
+			*table_size = 0;
+			return reloc;
+		}
+	}
+
+	return NULL;
+}
+
 struct reloc *arch_find_switch_table(struct objtool_file *file,
 				     struct instruction *insn,
 				     unsigned long *table_size)
@@ -126,8 +150,12 @@ struct reloc *arch_find_switch_table(struct objtool_file *file,
 	unsigned long table_offset;
 
 	annotate_reloc = find_reloc_by_table_annotate(file, insn, table_size);
-	if (!annotate_reloc)
-		return NULL;
+	if (!annotate_reloc) {
+		annotate_reloc = find_reloc_of_rodata_c_jump_table(
+				 insn->sec, insn->offset, table_size);
+		if (!annotate_reloc)
+			return NULL;
+	}
 
 	table_sec = annotate_reloc->sym->sec;
 	table_offset = annotate_reloc->sym->offset + reloc_addend(annotate_reloc);
-- 
2.42.0

[PATCH v6 7/9] LoongArch: Enable jump table for objtool

[Tiezhu Yang]

For now, it is time to remove -fno-jump-tables to enable jump table for
objtool if the compiler has -mannotate-tablejump, otherwise it is better
to remain -fno-jump-tables to keep compatibility with older compilers.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/Kconfig  | 3 +++
 arch/loongarch/Makefile | 6 +++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index dae3a9104ca6..66980d847f4e 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -287,6 +287,9 @@ config AS_HAS_LBT_EXTENSION
 config AS_HAS_LVZ_EXTENSION
 	def_bool $(as-instr,hvcl 0)
 
+config CC_HAS_ANNOTATE_TABLEJUMP
+	def_bool $(cc-option,-mannotate-tablejump)
+
 menu "Kernel type and options"
 
 source "kernel/Kconfig.hz"
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index 567bd122a9ee..0304eabbe606 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -101,7 +101,11 @@ KBUILD_AFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)
 KBUILD_CFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)-mthin-add-sub)
 
 ifdef CONFIG_OBJTOOL
-KBUILD_CFLAGS			+= -fno-jump-tables
+ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
+KBUILD_CFLAGS			+= -mannotate-tablejump
+else
+KBUILD_CFLAGS			+= -fno-jump-tables # keep compatibility with older compilers
+endif
 endif
 
 KBUILD_RUSTFLAGS		+= --target=loongarch64-unknown-none-softfloat -Ccode-model=small
-- 
2.42.0

[tip: objtool/core] LoongArch: Enable jump table for objtool

[tip-bot2 for Tiezhu Yang]

The following commit has been merged into the objtool/core branch of tip:

Commit-ID:     e20ab7d454ee8d1e0e8b9ff73a7c87e84c666b2f
Gitweb:        https://git.kernel.org/tip/e20ab7d454ee8d1e0e8b9ff73a7c87e84c666b2f
Author:        Tiezhu Yang <yangtiezhu@loongson.cn>
AuthorDate:    Tue, 17 Dec 2024 09:09:03 +08:00
Committer:     Josh Poimboeuf <jpoimboe@kernel.org>
CommitterDate: Wed, 12 Mar 2025 15:43:39 -07:00

LoongArch: Enable jump table for objtool

For now, it is time to remove -fno-jump-tables to enable jump table for
objtool if the compiler has -mannotate-tablejump, otherwise it is better
to remain -fno-jump-tables to keep compatibility with older compilers.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
Link: https://lore.kernel.org/r/20241217010905.13054-8-yangtiezhu@loongson.cn
Acked-by: Huacai Chen <chenhuacai@loongson.cn>
Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 arch/loongarch/Kconfig  | 3 +++
 arch/loongarch/Makefile | 6 +++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 2b8bd27..15aaa2e 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -291,6 +291,9 @@ config AS_HAS_LBT_EXTENSION
 config AS_HAS_LVZ_EXTENSION
 	def_bool $(as-instr,hvcl 0)
 
+config CC_HAS_ANNOTATE_TABLEJUMP
+	def_bool $(cc-option,-mannotate-tablejump)
+
 menu "Kernel type and options"
 
 source "kernel/Kconfig.hz"
diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index 567bd12..0304eab 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -101,7 +101,11 @@ KBUILD_AFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)
 KBUILD_CFLAGS			+= $(call cc-option,-mthin-add-sub) $(call cc-option,-Wa$(comma)-mthin-add-sub)
 
 ifdef CONFIG_OBJTOOL
-KBUILD_CFLAGS			+= -fno-jump-tables
+ifdef CONFIG_CC_HAS_ANNOTATE_TABLEJUMP
+KBUILD_CFLAGS			+= -mannotate-tablejump
+else
+KBUILD_CFLAGS			+= -fno-jump-tables # keep compatibility with older compilers
+endif
 endif
 
 KBUILD_RUSTFLAGS		+= --target=loongarch64-unknown-none-softfloat -Ccode-model=small

[PATCH v6 8/9] LoongArch: Convert unreachable() to BUG()

[Tiezhu Yang]

When compiling on LoongArch, there exists the following objtool
warning in arch/loongarch/kernel/machine_kexec.o:

  kexec_reboot() falls through to next function crash_shutdown_secondary()

Avoid unreachable() as it can (and will in the absence of UBSAN)
generate fallthrough code. Use BUG() so we get a "break	BRK_BUG"
trap (with unreachable annotation).

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 arch/loongarch/kernel/machine_kexec.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/loongarch/kernel/machine_kexec.c b/arch/loongarch/kernel/machine_kexec.c
index 8ae641dc53bb..f9381800e291 100644
--- a/arch/loongarch/kernel/machine_kexec.c
+++ b/arch/loongarch/kernel/machine_kexec.c
@@ -126,14 +126,14 @@ void kexec_reboot(void)
 	/* All secondary cpus go to kexec_smp_wait */
 	if (smp_processor_id() > 0) {
 		relocated_kexec_smp_wait(NULL);
-		unreachable();
+		BUG();
 	}
 #endif
 
 	do_kexec = (void *)reboot_code_buffer;
 	do_kexec(efi_boot, cmdline_ptr, systable_ptr, start_addr, first_ind_entry);
 
-	unreachable();
+	BUG();
 }
 
 
-- 
2.42.0

[PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Tiezhu Yang]

When compiling with Clang on LoongArch, there exists the following objtool
warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:

  dc_fixpt_recip() falls through to next function dc_fixpt_sinc()

This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
by Clang, given dc_fixpt_from_fraction() is not a simple function, just
mark it noinline to avoid the above issue.

Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
---
 drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
index 88d3f9d7dd55..b40c6a21460d 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
@@ -68,7 +68,7 @@ static inline unsigned long long complete_integer_division_u64(
 #define GET_FRACTIONAL_PART(x) \
 	(FRACTIONAL_PART_MASK & (x))
 
-struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
+noinline struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
 {
 	struct fixed31_32 res;
 
-- 
2.42.0

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Huacai Chen]

Hi, Tiezhu,

On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> When compiling with Clang on LoongArch, there exists the following objtool
> warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
>
>   dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
>
> This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
> by Clang, given dc_fixpt_from_fraction() is not a simple function, just
> mark it noinline to avoid the above issue.
I don't know whether drm maintainers can accept this, because it looks
like a workaround. Yes, uninline this function "solve" a problem and
seems reasonable in this case because the function is "not simple",
but from another point of view, you may hide a type of bug.


Huacai

>
> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
> ---
>  drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> index 88d3f9d7dd55..b40c6a21460d 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> @@ -68,7 +68,7 @@ static inline unsigned long long complete_integer_division_u64(
>  #define GET_FRACTIONAL_PART(x) \
>         (FRACTIONAL_PART_MASK & (x))
>
> -struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
> +noinline struct fixed31_32 dc_fixpt_from_fraction(long long numerator, long long denominator)
>  {
>         struct fixed31_32 res;
>
> --
> 2.42.0
>

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Josh Poimboeuf]

On Wed, Dec 18, 2024 at 10:36:00PM +0800, Huacai Chen wrote:
> Hi, Tiezhu,
> 
> On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> >
> > When compiling with Clang on LoongArch, there exists the following objtool
> > warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
> >
> >   dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
> >
> > This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
> > by Clang, given dc_fixpt_from_fraction() is not a simple function, just
> > mark it noinline to avoid the above issue.
> I don't know whether drm maintainers can accept this, because it looks
> like a workaround. Yes, uninline this function "solve" a problem and
> seems reasonable in this case because the function is "not simple",
> but from another point of view, you may hide a type of bug.

Agreed, it sounds like there's definitely a bug which this patch is
papering over.

-- 
Josh

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Alex Deucher]

On Wed, Dec 18, 2024 at 2:18 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Wed, Dec 18, 2024 at 10:36:00PM +0800, Huacai Chen wrote:
> > Hi, Tiezhu,
> >
> > On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
> > >
> > > When compiling with Clang on LoongArch, there exists the following objtool
> > > warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
> > >
> > >   dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
> > >
> > > This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
> > > by Clang, given dc_fixpt_from_fraction() is not a simple function, just
> > > mark it noinline to avoid the above issue.
> > I don't know whether drm maintainers can accept this, because it looks
> > like a workaround. Yes, uninline this function "solve" a problem and
> > seems reasonable in this case because the function is "not simple",
> > but from another point of view, you may hide a type of bug.
>
> Agreed, it sounds like there's definitely a bug which this patch is
> papering over.

Yes, agreed.

Alex

>
> --
> Josh

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Tiezhu Yang]

On 12/19/2024 03:22 AM, Alex Deucher wrote:
> On Wed, Dec 18, 2024 at 2:18 PM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>>
>> On Wed, Dec 18, 2024 at 10:36:00PM +0800, Huacai Chen wrote:
>>> Hi, Tiezhu,
>>>
>>> On Tue, Dec 17, 2024 at 9:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>>>>
>>>> When compiling with Clang on LoongArch, there exists the following objtool
>>>> warning in drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.o:
>>>>
>>>>   dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
>>>>
>>>> This is because dc_fixpt_from_fraction() is inlined in dc_fixpt_recip()
>>>> by Clang, given dc_fixpt_from_fraction() is not a simple function, just
>>>> mark it noinline to avoid the above issue.
>>> I don't know whether drm maintainers can accept this, because it looks
>>> like a workaround. Yes, uninline this function "solve" a problem and
>>> seems reasonable in this case because the function is "not simple",
>>> but from another point of view, you may hide a type of bug.
>>
>> Agreed, it sounds like there's definitely a bug which this patch is
>> papering over.
>
> Yes, agreed.

Additional Info:

In order to avoid the effect of this series, I tested with kernel
6.13-rc3 without this series again.

--------------------------------------------------------------------------
Here are the test result.

1. For LoongArch

"dc_fixpt_recip() falls through to next function dc_fixpt_sinc()"
objtool warning only appears compiled with the latest mainline LLVM,
there is no this issue compiled with the latest release version such
as LLVM 19.1.6.

(1) objdump info with LLVM release version 19.1.6:

$ clang --version | head -1
clang version 19.1.6

There is an jump instruction "b" at the end of dc_fixpt_recip(), it
maybe jumps to a position and then steps to the return instruction, so
there is no "falls through" objtool warning.

0000000000000200 <dc_fixpt_recip>:
      200:       40009480        beqz            $a0, 148        # 294 
<dc_fixpt_recip+0x94>
      204:       0049fc85        srai.d          $a1, $a0, 0x3f
      208:       00159486        xor             $a2, $a0, $a1
      20c:       001194c5        sub.d           $a1, $a2, $a1
      210:       03800007        ori             $a3, $zero, 0x0
      214:       16000027        lu32i.d         $a3, 1
      218:       002314e6        div.du          $a2, $a3, $a1
      21c:       001d94c8        mul.d           $a4, $a2, $a1
      220:       0011a0e9        sub.d           $a5, $a3, $a4
      224:       02bf8008        addi.w          $a4, $zero, -32
      228:       03400000        andi            $zero, $zero, 0x0
      22c:       03400000        andi            $zero, $zero, 0x0
      230:       00410529        slli.d          $a5, $a5, 0x1
      234:       004104c6        slli.d          $a2, $a2, 0x1
      238:       0012952a        sltu            $a6, $a5, $a1
      23c:       03c0054a        xori            $a6, $a6, 0x1
      240:       001328ab        maskeqz         $a7, $a1, $a6
      244:       0011ad29        sub.d           $a5, $a5, $a7
      248:       00df0108        bstrpick.d      $a4, $a4, 0x1f, 0x0
      24c:       02c00508        addi.d          $a4, $a4, 1
      250:       00149d0b        and             $a7, $a4, $a3
      254:       001528c6        or              $a2, $a2, $a6
      258:       43ffd97f        beqz            $a7, -40        # 230 
<dc_fixpt_recip+0x30>
      25c:       00410527        slli.d          $a3, $a5, 0x1
      260:       001294e5        sltu            $a1, $a3, $a1
      264:       03c004a5        xori            $a1, $a1, 0x1
      268:       02bffc07        addi.w          $a3, $zero, -1
      26c:       031ffce7        lu52i.d         $a3, $a3, 2047
      270:       00159ca7        xor             $a3, $a1, $a3
      274:       680030e6        bltu            $a3, $a2, 48    # 2a4 
<dc_fixpt_recip+0xa4>
      278:       001094c5        add.d           $a1, $a2, $a1
      27c:       00119406        sub.d           $a2, $zero, $a1
      280:       02000084        slti            $a0, $a0, 0
      284:       001390a5        masknez         $a1, $a1, $a0
      288:       001310c4        maskeqz         $a0, $a2, $a0
      28c:       00151484        or              $a0, $a0, $a1
      290:       4c000020        jirl            $zero, $ra, 0
      294:       00150005        or              $a1, $zero, $zero
      298:       002a0001        break           0x1
      29c:       002a0001        break           0x1
      2a0:       53ff73ff        b               -144    # 210 
<dc_fixpt_recip+0x10>
      2a4:       002a0001        break           0x1
      2a8:       53ffd3ff        b               -48     # 278 
<dc_fixpt_recip+0x78>
      2ac:       03400000        andi            $zero, $zero, 0x0
      2b0:       03400000        andi            $zero, $zero, 0x0
      2b4:       03400000        andi            $zero, $zero, 0x0
      2b8:       03400000        andi            $zero, $zero, 0x0
      2bc:       03400000        andi            $zero, $zero, 0x0

00000000000002c0 <dc_fixpt_sinc>:
      2c0:       0049fc85        srai.d          $a1, $a0, 0x3f
      2c4:       00159486        xor             $a2, $a0, $a1
      2c8:       1490fda7        lu12i.w         $a3, 296941
      2cc:       039444e7        ori             $a3, $a3, 0x511
      2d0:       001194c6        sub.d           $a2, $a2, $a1
      2d4:       001500e5        or              $a1, $a3, $zero
      2d8:       160000c5        lu32i.d         $a1, 6
      2dc:       001500c9        or              $a5, $a2, $zero
      2e0:       00150088        or              $a4, $a0, $zero
      2e4:       6000a8c5        blt             $a2, $a1, 168   # 38c 
<dc_fixpt_sinc+0xcc>

(2) objdump info with LLVM mainline version 20.0.0git:

$ clang --version | head -1
clang version 20.0.0git (https://github.com/llvm/llvm-project.git 
8daf4f16fa08b5d876e98108721dd1743a360326)

There is "break" instruction at the end of dc_fixpt_recip(), its offset
is "2a0", this "break" instruction is not dead end due to the offset
"2a4" is in the relocation section '.rela.discard.reachable', that is
to say, dc_fixpt_recip() doesn't end with a return instruction or an
unconditional jump, so objtool determined that the function can fall
through into the next function, thus there is "falls through" objtool
warning.

0000000000000200 <dc_fixpt_recip>:
      200:       40009c80        beqz            $a0, 156        # 29c 
<dc_fixpt_recip+0x9c>
      204:       0049fc85        srai.d          $a1, $a0, 0x3f
      208:       00159486        xor             $a2, $a0, $a1
      20c:       001194c5        sub.d           $a1, $a2, $a1
      210:       03800007        ori             $a3, $zero, 0x0
      214:       16000027        lu32i.d         $a3, 1
      218:       002314e6        div.du          $a2, $a3, $a1
      21c:       001d94c8        mul.d           $a4, $a2, $a1
      220:       0011a0e9        sub.d           $a5, $a3, $a4
      224:       02bf8008        addi.w          $a4, $zero, -32
      228:       03400000        andi            $zero, $zero, 0x0
      22c:       03400000        andi            $zero, $zero, 0x0
      230:       00410529        slli.d          $a5, $a5, 0x1
      234:       004104c6        slli.d          $a2, $a2, 0x1
      238:       0012952a        sltu            $a6, $a5, $a1
      23c:       03c0054a        xori            $a6, $a6, 0x1
      240:       001328ab        maskeqz         $a7, $a1, $a6
      244:       0011ad29        sub.d           $a5, $a5, $a7
      248:       00df0108        bstrpick.d      $a4, $a4, 0x1f, 0x0
      24c:       02c00508        addi.d          $a4, $a4, 1
      250:       00149d0b        and             $a7, $a4, $a3
      254:       001528c6        or              $a2, $a2, $a6
      258:       43ffd97f        beqz            $a7, -40        # 230 
<dc_fixpt_recip+0x30>
      25c:       00410527        slli.d          $a3, $a5, 0x1
      260:       001294e5        sltu            $a1, $a3, $a1
      264:       03c004a5        xori            $a1, $a1, 0x1
      268:       02bffc07        addi.w          $a3, $zero, -1
      26c:       031ffce7        lu52i.d         $a3, $a3, 2047
      270:       00159ca7        xor             $a3, $a1, $a3
      274:       680020e6        bltu            $a3, $a2, 32    # 294 
<dc_fixpt_recip+0x94>
      278:       001094c5        add.d           $a1, $a2, $a1
      27c:       00119406        sub.d           $a2, $zero, $a1
      280:       02000084        slti            $a0, $a0, 0
      284:       001390a5        masknez         $a1, $a1, $a0
      288:       001310c4        maskeqz         $a0, $a2, $a0
      28c:       00151484        or              $a0, $a0, $a1
      290:       4c000020        jirl            $zero, $ra, 0
      294:       002a0001        break           0x1
      298:       53ffe3ff        b               -32     # 278 
<dc_fixpt_recip+0x78>
      29c:       002a0001        break           0x1
      2a0:       002a0001        break           0x1
      2a4:       03400000        andi            $zero, $zero, 0x0
      2a8:       03400000        andi            $zero, $zero, 0x0
      2ac:       03400000        andi            $zero, $zero, 0x0
      2b0:       03400000        andi            $zero, $zero, 0x0
      2b4:       03400000        andi            $zero, $zero, 0x0
      2b8:       03400000        andi            $zero, $zero, 0x0
      2bc:       03400000        andi            $zero, $zero, 0x0

00000000000002c0 <dc_fixpt_sinc>:
      2c0:       0049fc85        srai.d          $a1, $a0, 0x3f
      2c4:       00159486        xor             $a2, $a0, $a1
      2c8:       001194c6        sub.d           $a2, $a2, $a1
      2cc:       1490fda5        lu12i.w         $a1, 296941
      2d0:       039444a5        ori             $a1, $a1, 0x511
      2d4:       160000c5        lu32i.d         $a1, 6
      2d8:       001500c7        or              $a3, $a2, $zero
      2dc:       00150088        or              $a4, $a0, $zero
      2e0:       600090c5        blt             $a2, $a1, 144   # 370 
<dc_fixpt_sinc+0xb0>

2. For x86

I tested with LLVM 19.1.6 and the latest mainline LLVM, the test result
is same with LoongArch.

(1) objdump info with LLVM release version 19.1.6:

$ clang --version | head -1
clang version 19.1.6

There is an jump instruction "jmp" at the end of dc_fixpt_recip(), it
maybe jumps to a position and then steps to the return instruction, so
there is no "falls through" objtool warning.

0000000000000290 <dc_fixpt_recip>:
      290:       f3 0f 1e fa             endbr64
      294:       e8 00 00 00 00          call   299 <dc_fixpt_recip+0x9>
      299:       48 85 ff                test   %rdi,%rdi
      29c:       0f 84 a8 00 00 00       je     34a <dc_fixpt_recip+0xba>
      2a2:       48 89 f9                mov    %rdi,%rcx
      2a5:       48 f7 d9                neg    %rcx
      2a8:       48 0f 48 cf             cmovs  %rdi,%rcx
      2ac:       53                      push   %rbx
      2ad:       48 b8 00 00 00 00 01    movabs $0x100000000,%rax
      2b4:       00 00 00
      2b7:       31 d2                   xor    %edx,%edx
      2b9:       48 f7 f1                div    %rcx
      2bc:       be e0 ff ff ff          mov    $0xffffffe0,%esi
      2c1:       eb 1b                   jmp    2de <dc_fixpt_recip+0x4e>
      2c3:       45 88 c8                mov    %r9b,%r8b
      2c6:       4d 01 c0                add    %r8,%r8
      2c9:       4d 8d 04 80             lea    (%r8,%rax,4),%r8
      2cd:       48 29 da                sub    %rbx,%rdx
      2d0:       45 88 da                mov    %r11b,%r10b
      2d3:       4c 89 d0                mov    %r10,%rax
      2d6:       4c 09 c0                or     %r8,%rax
      2d9:       83 c6 02                add    $0x2,%esi
      2dc:       74 31                   je     30f <dc_fixpt_recip+0x7f>
      2de:       48 01 d2                add    %rdx,%rdx
      2e1:       45 31 c0                xor    %r8d,%r8d
      2e4:       41 ba 00 00 00 00       mov    $0x0,%r10d
      2ea:       48 39 ca                cmp    %rcx,%rdx
      2ed:       41 0f 93 c1             setae  %r9b
      2f1:       72 03                   jb     2f6 <dc_fixpt_recip+0x66>
      2f3:       49 89 ca                mov    %rcx,%r10
      2f6:       4c 29 d2                sub    %r10,%rdx
      2f9:       48 01 d2                add    %rdx,%rdx
      2fc:       45 31 d2                xor    %r10d,%r10d
      2ff:       48 89 cb                mov    %rcx,%rbx
      302:       48 39 ca                cmp    %rcx,%rdx
      305:       41 0f 93 c3             setae  %r11b
      309:       73 b8                   jae    2c3 <dc_fixpt_recip+0x33>
      30b:       31 db                   xor    %ebx,%ebx
      30d:       eb b4                   jmp    2c3 <dc_fixpt_recip+0x33>
      30f:       48 01 d2                add    %rdx,%rdx
      312:       48 be fe ff ff ff ff    movabs $0x7ffffffffffffffe,%rsi
      319:       ff ff 7f
      31c:       4c 8d 46 01             lea    0x1(%rsi),%r8
      320:       48 39 ca                cmp    %rcx,%rdx
      323:       4c 0f 43 c6             cmovae %rsi,%r8
      327:       4c 39 c0                cmp    %r8,%rax
      32a:       77 29                   ja     355 <dc_fixpt_recip+0xc5>
      32c:       48 39 ca                cmp    %rcx,%rdx
      32f:       48 83 d8 ff             sbb    $0xffffffffffffffff,%rax
      333:       48 89 c1                mov    %rax,%rcx
      336:       48 f7 d9                neg    %rcx
      339:       48 85 ff                test   %rdi,%rdi
      33c:       48 0f 49 c8             cmovns %rax,%rcx
      340:       48 89 c8                mov    %rcx,%rax
      343:       5b                      pop    %rbx
      344:       2e e9 00 00 00 00       cs jmp 34a <dc_fixpt_recip+0xba>
      34a:       0f 0b                   ud2
      34c:       0f 0b                   ud2
      34e:       31 c9                   xor    %ecx,%ecx
      350:       e9 57 ff ff ff          jmp    2ac <dc_fixpt_recip+0x1c>
      355:       0f 0b                   ud2
      357:       eb d3                   jmp    32c <dc_fixpt_recip+0x9c>
      359:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

0000000000000360 <.Ltmp40>:
      360:       90                      nop
      361:       90                      nop
      362:       90                      nop
      363:       90                      nop
      364:       90                      nop
      365:       90                      nop
      366:       90                      nop
      367:       90                      nop
      368:       90                      nop
      369:       90                      nop
      36a:       90                      nop
      36b:       90                      nop
      36c:       90                      nop
      36d:       90                      nop
      36e:       90                      nop
      36f:       90                      nop

0000000000000370 <dc_fixpt_sinc>:
      370:       f3 0f 1e fa             endbr64
      374:       e8 00 00 00 00          call   379 <dc_fixpt_sinc+0x9>

(2) objdump info with LLVM mainline version 20.0.0git:

$ clang --version | head -1
clang version 20.0.0git (https://github.com/llvm/llvm-project.git 
8daf4f16fa08b5d876e98108721dd1743a360326)

There is "ud2" instruction at the end of dc_fixpt_recip(), its offset
is "350", this "ud2" instruction is not dead end due to the offset "352"
is in the relocation section '.rela.discard.reachable', that is to say,
dc_fixpt_recip() doesn't end with a return instruction or an
unconditional jump, so objtool determined that the function can fall
through into the next function, thus there is "falls through" objtool
warning.

0000000000000290 <dc_fixpt_recip>:
      290:       f3 0f 1e fa             endbr64
      294:       e8 00 00 00 00          call   299 <dc_fixpt_recip+0x9>
      299:       48 85 ff                test   %rdi,%rdi
      29c:       0f 84 ac 00 00 00       je     34e <dc_fixpt_recip+0xbe>
      2a2:       53                      push   %rbx
      2a3:       48 89 f9                mov    %rdi,%rcx
      2a6:       48 f7 d9                neg    %rcx
      2a9:       48 0f 48 cf             cmovs  %rdi,%rcx
      2ad:       48 b8 00 00 00 00 01    movabs $0x100000000,%rax
      2b4:       00 00 00
      2b7:       31 d2                   xor    %edx,%edx
      2b9:       48 f7 f1                div    %rcx
      2bc:       be e0 ff ff ff          mov    $0xffffffe0,%esi
      2c1:       eb 1b                   jmp    2de <dc_fixpt_recip+0x4e>
      2c3:       45 88 c8                mov    %r9b,%r8b
      2c6:       4d 01 c0                add    %r8,%r8
      2c9:       4d 8d 04 80             lea    (%r8,%rax,4),%r8
      2cd:       48 29 da                sub    %rbx,%rdx
      2d0:       45 88 da                mov    %r11b,%r10b
      2d3:       4c 89 d0                mov    %r10,%rax
      2d6:       4c 09 c0                or     %r8,%rax
      2d9:       83 c6 02                add    $0x2,%esi
      2dc:       74 31                   je     30f <dc_fixpt_recip+0x7f>
      2de:       48 01 d2                add    %rdx,%rdx
      2e1:       45 31 c0                xor    %r8d,%r8d
      2e4:       41 ba 00 00 00 00       mov    $0x0,%r10d
      2ea:       48 39 ca                cmp    %rcx,%rdx
      2ed:       41 0f 93 c1             setae  %r9b
      2f1:       72 03                   jb     2f6 <dc_fixpt_recip+0x66>
      2f3:       49 89 ca                mov    %rcx,%r10
      2f6:       4c 29 d2                sub    %r10,%rdx
      2f9:       48 01 d2                add    %rdx,%rdx
      2fc:       45 31 d2                xor    %r10d,%r10d
      2ff:       48 89 cb                mov    %rcx,%rbx
      302:       48 39 ca                cmp    %rcx,%rdx
      305:       41 0f 93 c3             setae  %r11b
      309:       73 b8                   jae    2c3 <dc_fixpt_recip+0x33>
      30b:       31 db                   xor    %ebx,%ebx
      30d:       eb b4                   jmp    2c3 <dc_fixpt_recip+0x33>
      30f:       48 01 d2                add    %rdx,%rdx
      312:       48 be fe ff ff ff ff    movabs $0x7ffffffffffffffe,%rsi
      319:       ff ff 7f
      31c:       4c 8d 46 01             lea    0x1(%rsi),%r8
      320:       48 39 ca                cmp    %rcx,%rdx
      323:       4c 0f 43 c6             cmovae %rsi,%r8
      327:       4c 39 c0                cmp    %r8,%rax
      32a:       77 1e                   ja     34a <dc_fixpt_recip+0xba>
      32c:       48 39 ca                cmp    %rcx,%rdx
      32f:       48 83 d8 ff             sbb    $0xffffffffffffffff,%rax
      333:       48 89 c1                mov    %rax,%rcx
      336:       48 f7 d9                neg    %rcx
      339:       48 85 ff                test   %rdi,%rdi
      33c:       48 0f 49 c8             cmovns %rax,%rcx
      340:       48 89 c8                mov    %rcx,%rax
      343:       5b                      pop    %rbx
      344:       2e e9 00 00 00 00       cs jmp 34a <dc_fixpt_recip+0xba>
      34a:       0f 0b                   ud2
      34c:       eb de                   jmp    32c <dc_fixpt_recip+0x9c>
      34e:       0f 0b                   ud2
      350:       0f 0b                   ud2
      352:       66 66 66 66 66 2e 0f    data16 data16 data16 data16 cs 
nopw 0x0(%rax,%rax,1)
      359:       1f 84 00 00 00 00 00

0000000000000360 <.Ltmp40>:
      360:       90                      nop
      361:       90                      nop
      362:       90                      nop
      363:       90                      nop
      364:       90                      nop
      365:       90                      nop
      366:       90                      nop
      367:       90                      nop
      368:       90                      nop
      369:       90                      nop
      36a:       90                      nop
      36b:       90                      nop
      36c:       90                      nop
      36d:       90                      nop
      36e:       90                      nop
      36f:       90                      nop

0000000000000370 <dc_fixpt_sinc>:
      370:       f3 0f 1e fa             endbr64
      374:       e8 00 00 00 00          call   379 <dc_fixpt_sinc+0x9>
--------------------------------------------------------------------------

In my opinion, if there is a bug, then it is a generic rather than
arch-specific bug.

The root cause is related with LLVM or Linux kernel? I am not sure
because objtool only checks the object info and reports the warning
"falls through" if the check conditions are true.

tools/objtool/check.c
static int validate_branch(struct objtool_file *file, struct symbol *func,
                            struct instruction *insn, struct insn_state 
state)
{
         struct alternative *alt;
         struct instruction *next_insn, *prev_insn = NULL;
         struct section *sec;
         u8 visited;
         int ret;

         sec = insn->sec;

         while (1) {
                 next_insn = next_insn_to_validate(file, insn);

                 if (func && insn_func(insn) && func != 
insn_func(insn)->pfunc) {
                         /* Ignore KCFI type preambles, which always 
fall through */
                         if (!strncmp(func->name, "__cfi_", 6) ||
                             !strncmp(func->name, "__pfx_", 6))
                                 return 0;

                         WARN("%s() falls through to next function %s()",
                              func->name, insn_func(insn)->name);
                         return 1;
                 }
         ...
}

How to silence the warning when compiling with the latest mainline
LLVM with a proper way? Modify LLVM code or tools/objtool/check.c
or drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c?

Anyway, I think this patch can be independent of this series and can
be sent separately after the "real bug" is fixed, please ignore it now.

Thanks,
Tiezhu

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Peter Zijlstra]

On Fri, Dec 20, 2024 at 01:02:18PM +0800, Tiezhu Yang wrote:

> 2. For x86
> 
> I tested with LLVM 19.1.6 and the latest mainline LLVM, the test result
> is same with LoongArch.

Debian's clang-19 is 19.1.5.

> (1) objdump info with LLVM release version 19.1.6:

Please always use -r, that's ever so much more readable.

> $ clang --version | head -1
> clang version 19.1.6
> 
> There is an jump instruction "jmp" at the end of dc_fixpt_recip(), it
> maybe jumps to a position and then steps to the return instruction, so
> there is no "falls through" objtool warning.
> 
> 0000000000000290 <dc_fixpt_recip>:
>      290:       f3 0f 1e fa             endbr64
>      294:       e8 00 00 00 00          call   299 <dc_fixpt_recip+0x9>
>      299:       48 85 ff                test   %rdi,%rdi
>      29c:       0f 84 a8 00 00 00       je     34a <dc_fixpt_recip+0xba>
>      2a2:       48 89 f9                mov    %rdi,%rcx
>      2a5:       48 f7 d9                neg    %rcx
>      2a8:       48 0f 48 cf             cmovs  %rdi,%rcx
>      2ac:       53                      push   %rbx
>      2ad:       48 b8 00 00 00 00 01    movabs $0x100000000,%rax
>      2b4:       00 00 00
>      2b7:       31 d2                   xor    %edx,%edx
>      2b9:       48 f7 f1                div    %rcx
>      2bc:       be e0 ff ff ff          mov    $0xffffffe0,%esi
>      2c1:       eb 1b                   jmp    2de <dc_fixpt_recip+0x4e>
>      2c3:       45 88 c8                mov    %r9b,%r8b
>      2c6:       4d 01 c0                add    %r8,%r8
>      2c9:       4d 8d 04 80             lea    (%r8,%rax,4),%r8
>      2cd:       48 29 da                sub    %rbx,%rdx
>      2d0:       45 88 da                mov    %r11b,%r10b
>      2d3:       4c 89 d0                mov    %r10,%rax
>      2d6:       4c 09 c0                or     %r8,%rax
>      2d9:       83 c6 02                add    $0x2,%esi
>      2dc:       74 31                   je     30f <dc_fixpt_recip+0x7f>
>      2de:       48 01 d2                add    %rdx,%rdx
>      2e1:       45 31 c0                xor    %r8d,%r8d
>      2e4:       41 ba 00 00 00 00       mov    $0x0,%r10d
>      2ea:       48 39 ca                cmp    %rcx,%rdx
>      2ed:       41 0f 93 c1             setae  %r9b
>      2f1:       72 03                   jb     2f6 <dc_fixpt_recip+0x66>
>      2f3:       49 89 ca                mov    %rcx,%r10
>      2f6:       4c 29 d2                sub    %r10,%rdx
>      2f9:       48 01 d2                add    %rdx,%rdx
>      2fc:       45 31 d2                xor    %r10d,%r10d
>      2ff:       48 89 cb                mov    %rcx,%rbx
>      302:       48 39 ca                cmp    %rcx,%rdx
>      305:       41 0f 93 c3             setae  %r11b
>      309:       73 b8                   jae    2c3 <dc_fixpt_recip+0x33>
>      30b:       31 db                   xor    %ebx,%ebx
>      30d:       eb b4                   jmp    2c3 <dc_fixpt_recip+0x33>
>      30f:       48 01 d2                add    %rdx,%rdx
>      312:       48 be fe ff ff ff ff    movabs $0x7ffffffffffffffe,%rsi
>      319:       ff ff 7f
>      31c:       4c 8d 46 01             lea    0x1(%rsi),%r8
>      320:       48 39 ca                cmp    %rcx,%rdx
>      323:       4c 0f 43 c6             cmovae %rsi,%r8
>      327:       4c 39 c0                cmp    %r8,%rax
>      32a:       77 29                   ja     355 <dc_fixpt_recip+0xc5>
>      32c:       48 39 ca                cmp    %rcx,%rdx
>      32f:       48 83 d8 ff             sbb    $0xffffffffffffffff,%rax
>      333:       48 89 c1                mov    %rax,%rcx
>      336:       48 f7 d9                neg    %rcx
>      339:       48 85 ff                test   %rdi,%rdi
>      33c:       48 0f 49 c8             cmovns %rax,%rcx
>      340:       48 89 c8                mov    %rcx,%rax
>      343:       5b                      pop    %rbx
>      344:       2e e9 00 00 00 00       cs jmp 34a <dc_fixpt_recip+0xba>
>      34a:       0f 0b                   ud2
>      34c:       0f 0b                   ud2
>      34e:       31 c9                   xor    %ecx,%ecx
>      350:       e9 57 ff ff ff          jmp    2ac <dc_fixpt_recip+0x1c>
>      355:       0f 0b                   ud2
>      357:       eb d3                   jmp    32c <dc_fixpt_recip+0x9c>
>      359:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)

I had to puzzle a bit to get that double ud2 -- not all configs do that.

Also, curse the DRM Makefiles, you can't do:

  make drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.s

:-(

They are the two ASSERT()s on line 217 and 54 respectively. They end up
asserting the same value twice, so that makes sense.

> $ clang --version | head -1
> clang version 20.0.0git (https://github.com/llvm/llvm-project.git
> 8daf4f16fa08b5d876e98108721dd1743a360326)

So I didn't have a recent build at hand.. so I've not validated the
below.

> There is "ud2" instruction at the end of dc_fixpt_recip(), its offset
> is "350", this "ud2" instruction is not dead end due to the offset "352"
> is in the relocation section '.rela.discard.reachable', that is to say,
> dc_fixpt_recip() doesn't end with a return instruction or an
> unconditional jump, so objtool determined that the function can fall
> through into the next function, thus there is "falls through" objtool
> warning.
> 
> 0000000000000290 <dc_fixpt_recip>:
>      290:       f3 0f 1e fa             endbr64
>      294:       e8 00 00 00 00          call   299 <dc_fixpt_recip+0x9>
>      299:       48 85 ff                test   %rdi,%rdi
>      29c:       0f 84 ac 00 00 00       je     34e <dc_fixpt_recip+0xbe>
>      2a2:       53                      push   %rbx
>      2a3:       48 89 f9                mov    %rdi,%rcx
>      2a6:       48 f7 d9                neg    %rcx
>      2a9:       48 0f 48 cf             cmovs  %rdi,%rcx
>      2ad:       48 b8 00 00 00 00 01    movabs $0x100000000,%rax
>      2b4:       00 00 00
>      2b7:       31 d2                   xor    %edx,%edx
>      2b9:       48 f7 f1                div    %rcx
>      2bc:       be e0 ff ff ff          mov    $0xffffffe0,%esi
>      2c1:       eb 1b                   jmp    2de <dc_fixpt_recip+0x4e>
>      2c3:       45 88 c8                mov    %r9b,%r8b
>      2c6:       4d 01 c0                add    %r8,%r8
>      2c9:       4d 8d 04 80             lea    (%r8,%rax,4),%r8
>      2cd:       48 29 da                sub    %rbx,%rdx
>      2d0:       45 88 da                mov    %r11b,%r10b
>      2d3:       4c 89 d0                mov    %r10,%rax
>      2d6:       4c 09 c0                or     %r8,%rax
>      2d9:       83 c6 02                add    $0x2,%esi
>      2dc:       74 31                   je     30f <dc_fixpt_recip+0x7f>
>      2de:       48 01 d2                add    %rdx,%rdx
>      2e1:       45 31 c0                xor    %r8d,%r8d
>      2e4:       41 ba 00 00 00 00       mov    $0x0,%r10d
>      2ea:       48 39 ca                cmp    %rcx,%rdx
>      2ed:       41 0f 93 c1             setae  %r9b
>      2f1:       72 03                   jb     2f6 <dc_fixpt_recip+0x66>
>      2f3:       49 89 ca                mov    %rcx,%r10
>      2f6:       4c 29 d2                sub    %r10,%rdx
>      2f9:       48 01 d2                add    %rdx,%rdx
>      2fc:       45 31 d2                xor    %r10d,%r10d
>      2ff:       48 89 cb                mov    %rcx,%rbx
>      302:       48 39 ca                cmp    %rcx,%rdx
>      305:       41 0f 93 c3             setae  %r11b
>      309:       73 b8                   jae    2c3 <dc_fixpt_recip+0x33>
>      30b:       31 db                   xor    %ebx,%ebx
>      30d:       eb b4                   jmp    2c3 <dc_fixpt_recip+0x33>
>      30f:       48 01 d2                add    %rdx,%rdx
>      312:       48 be fe ff ff ff ff    movabs $0x7ffffffffffffffe,%rsi
>      319:       ff ff 7f
>      31c:       4c 8d 46 01             lea    0x1(%rsi),%r8
>      320:       48 39 ca                cmp    %rcx,%rdx
>      323:       4c 0f 43 c6             cmovae %rsi,%r8
>      327:       4c 39 c0                cmp    %r8,%rax
>      32a:       77 1e                   ja     34a <dc_fixpt_recip+0xba>
>      32c:       48 39 ca                cmp    %rcx,%rdx
>      32f:       48 83 d8 ff             sbb    $0xffffffffffffffff,%rax
>      333:       48 89 c1                mov    %rax,%rcx
>      336:       48 f7 d9                neg    %rcx
>      339:       48 85 ff                test   %rdi,%rdi
>      33c:       48 0f 49 c8             cmovns %rax,%rcx
>      340:       48 89 c8                mov    %rcx,%rax
>      343:       5b                      pop    %rbx
>      344:       2e e9 00 00 00 00       cs jmp 34a <dc_fixpt_recip+0xba>
>      34a:       0f 0b                   ud2
>      34c:       eb de                   jmp    32c <dc_fixpt_recip+0x9c>
>      34e:       0f 0b                   ud2
>      350:       0f 0b                   ud2
>      352:       66 66 66 66 66 2e 0f    data16 data16 data16 data16 cs nopw
> 0x0(%rax,%rax,1)
>      359:       1f 84 00 00 00 00 00

If you put them size-by-side, you'll see it's more or less the same
code-gen (trivial differences), but now it just stops code-gen, where
previously it would continue.

So this really is a compiler problem, this needs no annotation, it's
straight up broken.

Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
suspect clang figured that out and invokes UB on us and just stops
code-gen.

Nathan, Nick, don't we have a compiler flag that forces __builtin_trap()
whenever clang pulls something like this? I think UBSAN does this, but
we really shouldn't pull in the whole of that for sanity.

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Nathan Chancellor]

On Fri, Dec 20, 2024 at 11:31:00AM +0100, Peter Zijlstra wrote:
> Also, curse the DRM Makefiles, you can't do:
> 
>   make drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.s

Small tip: You can get the path of the target by building
drivers/gpu/drm/amd/amdgpu/ and finding it in the output. In this case,
it'd be

  $ make drivers/gpu/drm/amd/amdgpu/../display/dc/basics/fixpt31_32.s

Not excusing that it does not work as it should but sometimes you have
to work with what you can *shrug*

> > $ clang --version | head -1
> > clang version 20.0.0git (https://github.com/llvm/llvm-project.git
> > 8daf4f16fa08b5d876e98108721dd1743a360326)
> 
> So I didn't have a recent build at hand.. so I've not validated the
> below.
...
> If you put them size-by-side, you'll see it's more or less the same
> code-gen (trivial differences), but now it just stops code-gen, where
> previously it would continue.
> 
> So this really is a compiler problem, this needs no annotation, it's
> straight up broken.
> 
> Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
> suspect clang figured that out and invokes UB on us and just stops
> code-gen.

Yeah, I think your analysis is spot on, as this was introduced by a
change in clang from a few months ago according to my bisect:

https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448

Since the ASSERT does not do anything to prevent the divide by zero (it
just flags it with WARN_ON) and the rest of the code doesn't either, I
assume that the codegen stops as soon as it encounters the unreachable
that change created from the path where divide by zero would occur via

  dc_fixpt_recip() ->
    dc_fixpt_from_fraction() ->
      complete_integer_division_u64() ->
        div64_u64_rem()

Shouldn't callers of division functions harden them against dividing by
zero?

> Nathan, Nick, don't we have a compiler flag that forces __builtin_trap()
> whenever clang pulls something like this? I think UBSAN does this, but
> we really shouldn't pull in the whole of that for sanity.

Right, I think that LLVM has a hidden flag for this:

  -mllvm -trap-unreachable

That makes this particular warning disappear.

It isn't the greatest because '-mllvm' flags need to be passed along to
the linker for LTO but that's easy enough to deal with. I know we have
talked about enabling that flag in the past but I cannot remember why we
decided against it (maybe code size concerns and other optimization
restrictions)? It looks like GCC has a similar flag,
-funreachable-traps.

Cheers,
Nathan

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Xi Ruoyao]

On Fri, 2024-12-20 at 15:34 -0700, Nathan Chancellor wrote:
> > Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
> > suspect clang figured that out and invokes UB on us and just stops
> > code-gen.
> 
> Yeah, I think your analysis is spot on, as this was introduced by a
> change in clang from a few months ago according to my bisect:
> 
> https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448
> 
> Since the ASSERT does not do anything to prevent the divide by zero (it
> just flags it with WARN_ON) and the rest of the code doesn't either, I
> assume that the codegen stops as soon as it encounters the unreachable
> that change created from the path where divide by zero would occur via
> 
>   dc_fixpt_recip() ->
>     dc_fixpt_from_fraction() ->
>       complete_integer_division_u64() ->
>         div64_u64_rem()
> 
> Shouldn't callers of division functions harden them against dividing by
> zero?

Yes I think it'd be the correct solution.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Tiezhu Yang]

On 12/21/2024 03:40 PM, Xi Ruoyao wrote:
> On Fri, 2024-12-20 at 15:34 -0700, Nathan Chancellor wrote:
>>> Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
>>> suspect clang figured that out and invokes UB on us and just stops
>>> code-gen.
>>
>> Yeah, I think your analysis is spot on, as this was introduced by a
>> change in clang from a few months ago according to my bisect:
>>
>> https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448
>>
>> Since the ASSERT does not do anything to prevent the divide by zero (it
>> just flags it with WARN_ON) and the rest of the code doesn't either, I
>> assume that the codegen stops as soon as it encounters the unreachable
>> that change created from the path where divide by zero would occur via
>>
>>   dc_fixpt_recip() ->
>>     dc_fixpt_from_fraction() ->
>>       complete_integer_division_u64() ->
>>         div64_u64_rem()
>>
>> Shouldn't callers of division functions harden them against dividing by
>> zero?
>
> Yes I think it'd be the correct solution.

Thank you all. Do you mean like this?

--- >8 ---

diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c 
b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
index 88d3f9d7dd55..848d8e67304a 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
@@ -79,11 +79,13 @@ struct fixed31_32 dc_fixpt_from_fraction(long long 
numerator, long long denomina
         unsigned long long arg2_value = arg2_negative ? -denominator : 
denominator;

         unsigned long long remainder;
+       unsigned long long res_value;

         /* determine integer part */

-       unsigned long long res_value = complete_integer_division_u64(
-               arg1_value, arg2_value, &remainder);
+       ASSERT(arg2_value);
+
+       res_value = complete_integer_division_u64(arg1_value, 
arg2_value, &remainder);

         ASSERT(res_value <= LONG_MAX);

@@ -214,8 +216,6 @@ struct fixed31_32 dc_fixpt_recip(struct fixed31_32 arg)
          * Good idea to use Newton's method
          */

-       ASSERT(arg.value);
-
         return dc_fixpt_from_fraction(
                 dc_fixpt_one.value,
                 arg.value);

With the above changes, there is no "falls through" objtool warning
compiled with both clang 19 and the latest mainline clang 20.

If you are OK with it, I will send a separate formal patch to handle
this issue after doing some more testing.

Thanks,
Tiezhu

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Nathan Chancellor]

On Sun, Dec 22, 2024 at 12:27:47PM +0800, Tiezhu Yang wrote:
> On 12/21/2024 03:40 PM, Xi Ruoyao wrote:
> > On Fri, 2024-12-20 at 15:34 -0700, Nathan Chancellor wrote:
> > > > Now, the thing is, these ASSERT()s are checking for divide-by-zero, I
> > > > suspect clang figured that out and invokes UB on us and just stops
> > > > code-gen.
> > > 
> > > Yeah, I think your analysis is spot on, as this was introduced by a
> > > change in clang from a few months ago according to my bisect:
> > > 
> > > https://github.com/llvm/llvm-project/commit/37932643abab699e8bb1def08b7eb4eae7ff1448
> > > 
> > > Since the ASSERT does not do anything to prevent the divide by zero (it
> > > just flags it with WARN_ON) and the rest of the code doesn't either, I
> > > assume that the codegen stops as soon as it encounters the unreachable
> > > that change created from the path where divide by zero would occur via
> > > 
> > >   dc_fixpt_recip() ->
> > >     dc_fixpt_from_fraction() ->
> > >       complete_integer_division_u64() ->
> > >         div64_u64_rem()
> > > 
> > > Shouldn't callers of division functions harden them against dividing by
> > > zero?
> > 
> > Yes I think it'd be the correct solution.
> 
> Thank you all. Do you mean like this?
> 
> --- >8 ---
> 
> diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> index 88d3f9d7dd55..848d8e67304a 100644
> --- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> +++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
> @@ -79,11 +79,13 @@ struct fixed31_32 dc_fixpt_from_fraction(long long
> numerator, long long denomina
>         unsigned long long arg2_value = arg2_negative ? -denominator :
> denominator;
> 
>         unsigned long long remainder;
> +       unsigned long long res_value;
> 
>         /* determine integer part */
> 
> -       unsigned long long res_value = complete_integer_division_u64(
> -               arg1_value, arg2_value, &remainder);
> +       ASSERT(arg2_value);
> +
> +       res_value = complete_integer_division_u64(arg1_value, arg2_value,
> &remainder);
> 
>         ASSERT(res_value <= LONG_MAX);
> 
> @@ -214,8 +216,6 @@ struct fixed31_32 dc_fixpt_recip(struct fixed31_32 arg)
>          * Good idea to use Newton's method
>          */
> 
> -       ASSERT(arg.value);
> -
>         return dc_fixpt_from_fraction(
>                 dc_fixpt_one.value,
>                 arg.value);
> 
> With the above changes, there is no "falls through" objtool warning
> compiled with both clang 19 and the latest mainline clang 20.

I am somewhat surprised that changes anything because the ASSERT is not
stopping control flow so I would expect the same problem as before. I
guess it does not happen perhaps due to inlining differences? I looked
at this code briefly when I sent my initial message and I was not sure
where such a check should exist. It does not look like these functions
really do any sort of error handling.

> If you are OK with it, I will send a separate formal patch to handle
> this issue after doing some more testing.

It may still be worth doing this to get some initial thoughts from the
AMD DRM folks.

Cheers,
Nathan

Re: [PATCH v6 9/9] drm/amd/display: Mark dc_fixpt_from_fraction() noinline

[Tiezhu Yang]

On 12/24/2024 05:46 AM, Nathan Chancellor wrote:
> On Sun, Dec 22, 2024 at 12:27:47PM +0800, Tiezhu Yang wrote:

...

>> With the above changes, there is no "falls through" objtool warning
>> compiled with both clang 19 and the latest mainline clang 20.
>
> I am somewhat surprised that changes anything because the ASSERT is not
> stopping control flow so I would expect the same problem as before. I
> guess it does not happen perhaps due to inlining differences? I looked

It is weird and I think it is not the correct way.

> at this code briefly when I sent my initial message and I was not sure
> where such a check should exist. It does not look like these functions
> really do any sort of error handling.
>
>> If you are OK with it, I will send a separate formal patch to handle
>> this issue after doing some more testing.
>
> It may still be worth doing this to get some initial thoughts from the
> AMD DRM folks.

I think the correct way is:

Keep the current ASSERT for the aim of debugging, just add BUG() to
stop control flow if the divisor is zero.

--- >8 ---

diff --git a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c 
b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
index 88d3f9d7dd55..e15391e36b40 100644
--- a/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/basics/fixpt31_32.c
@@ -52,6 +52,7 @@ static inline unsigned long long 
complete_integer_division_u64(
         unsigned long long result;

         ASSERT(divisor);
+       BUG_ON(!divisor);

         result = div64_u64_rem(dividend, divisor, remainder);

diff --git a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c 
b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
index 131f1e3949d3..ce2036950808 100644
--- a/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
+++ b/drivers/gpu/drm/amd/display/dc/spl/spl_fixpt31_32.c
@@ -30,6 +30,7 @@ static inline unsigned long long 
spl_complete_integer_division_u64(
         unsigned long long result;

         SPL_ASSERT(divisor);
+       BUG_ON(!divisor);

         result = spl_div64_u64_rem(dividend, divisor, remainder);

It looks reasonable and works well both on x86 and LoongArch, there are
no the following objtool warnings:

   dc_fixpt_recip() falls through to next function dc_fixpt_sinc()
   spl_fixpt_recip() falls through to next function spl_fixpt_sinc()

If no more comments, I will send a separate formal patch for your
review in the next week.

Thanks,
Tiezhu

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Huacai Chen]

Hi, Josh and Peter,

I think this series (except the last patch, but that one can be a
separate one) is good enough now, right? If so, I think there is some
ways to get it upstream:

1) I merge objtool/core from tip.git to the loongarch tree, then apply
this whole series with your acked-by;
2) You apply the first 4 patches to tip.git, and then I apply others
to the loongarch tree (still need to merge objtool/core before apply
to avoid build issues).

I prefer the first method, but I can also accept other ways.

Huacai

On Tue, Dec 17, 2024 at 9:09 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
>
> This version is based on tip/tip.git objtool/core branch [1], add some weak
> and arch-specific functions to make the generic code more readable, tested
> with the latest upstream mainline Binutils, GCC and Clang.
>
> The first 6 patches are preparation for patch #7 to enable jump table for
> objtool on LoongArch, the last 2 patches are small enough to fix objtool
> warnings "funcA() falls through to next function funcB()", one is under
> arch/loongarch and the other is under drm/amd/display.
>
> v6:
>   -- Add arch_reloc_size() for x86 and ppc.
>   -- Call arch_reloc_size() directly in add_jump_table().
>   -- Refine arch_adjust_offset() for LoongArch.
>   -- Rename arch_adjust_offset() to arch_jump_table_sym_offset().
>   -- Get each table size of rodata in time for switch table.
>   -- Update the commit message to make it more clear.
>
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
>
> Tiezhu Yang (9):
>   objtool: Handle various symbol types of rodata
>   objtool: Handle different entry size of rodata
>   objtool: Handle PC relative relocation type
>   objtool: Handle unreachable entry of rodata
>   objtool/LoongArch: Add support for switch table
>   objtool/LoongArch: Add support for goto table
>   LoongArch: Enable jump table for objtool
>   LoongArch: Convert unreachable() to BUG()
>   drm/amd/display: Mark dc_fixpt_from_fraction() noinline
>
>  arch/loongarch/Kconfig                        |   3 +
>  arch/loongarch/Makefile                       |   6 +-
>  arch/loongarch/kernel/machine_kexec.c         |   4 +-
>  .../drm/amd/display/dc/basics/fixpt31_32.c    |   2 +-
>  tools/objtool/arch/loongarch/decode.c         |  28 ++-
>  .../objtool/arch/loongarch/include/arch/elf.h |   7 +
>  tools/objtool/arch/loongarch/special.c        | 159 +++++++++++++++++-
>  tools/objtool/arch/powerpc/decode.c           |  15 ++
>  tools/objtool/arch/x86/decode.c               |  13 ++
>  tools/objtool/check.c                         |  28 ++-
>  tools/objtool/include/objtool/arch.h          |   3 +
>  11 files changed, 251 insertions(+), 17 deletions(-)
>
> --
> 2.42.0
>

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Tiezhu Yang]

Hi Josh and Peter,

On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> This version is based on tip/tip.git objtool/core branch [1], add some weak
> and arch-specific functions to make the generic code more readable, tested
> with the latest upstream mainline Binutils, GCC and Clang.

...

> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
>
> Tiezhu Yang (9):
>   objtool: Handle various symbol types of rodata
>   objtool: Handle different entry size of rodata
>   objtool: Handle PC relative relocation type
>   objtool: Handle unreachable entry of rodata
>   objtool/LoongArch: Add support for switch table
>   objtool/LoongArch: Add support for goto table
>   LoongArch: Enable jump table for objtool
>   LoongArch: Convert unreachable() to BUG()
>   drm/amd/display: Mark dc_fixpt_from_fraction() noinline

Are you OK with the first 8 patches?
What's the merge plan for this series?

Thanks,
Tiezhu

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Josh Poimboeuf]

On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> Hi Josh and Peter,
> 
> On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > and arch-specific functions to make the generic code more readable, tested
> > with the latest upstream mainline Binutils, GCC and Clang.
> 
> ...
> 
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > 
> > Tiezhu Yang (9):
> >   objtool: Handle various symbol types of rodata
> >   objtool: Handle different entry size of rodata
> >   objtool: Handle PC relative relocation type
> >   objtool: Handle unreachable entry of rodata
> >   objtool/LoongArch: Add support for switch table
> >   objtool/LoongArch: Add support for goto table
> >   LoongArch: Enable jump table for objtool
> >   LoongArch: Convert unreachable() to BUG()
> >   drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> 
> Are you OK with the first 8 patches?
> What's the merge plan for this series?

Sorry, my inbox is still reeling from the holidays.  I will review this
soon.

-- 
Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Tiezhu Yang]

On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
>> Hi Josh and Peter,
>>
>> On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
>>> This version is based on tip/tip.git objtool/core branch [1], add some weak
>>> and arch-specific functions to make the generic code more readable, tested
>>> with the latest upstream mainline Binutils, GCC and Clang.
>>
>> ...
>>
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
>>>
>>> Tiezhu Yang (9):
>>>   objtool: Handle various symbol types of rodata
>>>   objtool: Handle different entry size of rodata
>>>   objtool: Handle PC relative relocation type
>>>   objtool: Handle unreachable entry of rodata
>>>   objtool/LoongArch: Add support for switch table
>>>   objtool/LoongArch: Add support for goto table
>>>   LoongArch: Enable jump table for objtool
>>>   LoongArch: Convert unreachable() to BUG()
>>>   drm/amd/display: Mark dc_fixpt_from_fraction() noinline
>>
>> Are you OK with the first 8 patches?
>> What's the merge plan for this series?
>
> Sorry, my inbox is still reeling from the holidays.  I will review this
> soon.

What are the status of the first 8 patches?
What is the next step? Is there anything else to be done here?
I would really appreciate any advice on how to get this merged.

Thanks,
Tiezhu

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Josh Poimboeuf]

On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > Hi Josh and Peter,
> > > 
> > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > and arch-specific functions to make the generic code more readable, tested
> > > > with the latest upstream mainline Binutils, GCC and Clang.
> > > 
> > > ...
> > > 
> > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > > 
> > > > Tiezhu Yang (9):
> > > >   objtool: Handle various symbol types of rodata
> > > >   objtool: Handle different entry size of rodata
> > > >   objtool: Handle PC relative relocation type
> > > >   objtool: Handle unreachable entry of rodata
> > > >   objtool/LoongArch: Add support for switch table
> > > >   objtool/LoongArch: Add support for goto table
> > > >   LoongArch: Enable jump table for objtool
> > > >   LoongArch: Convert unreachable() to BUG()
> > > >   drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > > 
> > > Are you OK with the first 8 patches?
> > > What's the merge plan for this series?
> > 
> > Sorry, my inbox is still reeling from the holidays.  I will review this
> > soon.
> 
> What are the status of the first 8 patches?
> What is the next step? Is there anything else to be done here?
> I would really appreciate any advice on how to get this merged.

Please post a new revision rebased on tip/master, with patch 4 dropped,
and I'll queue them up for -tip.  Thanks for your patience.

-- 
Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Huacai Chen]

Hi, Josh,

On Tue, Feb 11, 2025 at 5:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> > On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > > Hi Josh and Peter,
> > > >
> > > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > > and arch-specific functions to make the generic code more readable, tested
> > > > > with the latest upstream mainline Binutils, GCC and Clang.
> > > >
> > > > ...
> > > >
> > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > > >
> > > > > Tiezhu Yang (9):
> > > > >   objtool: Handle various symbol types of rodata
> > > > >   objtool: Handle different entry size of rodata
> > > > >   objtool: Handle PC relative relocation type
> > > > >   objtool: Handle unreachable entry of rodata
> > > > >   objtool/LoongArch: Add support for switch table
> > > > >   objtool/LoongArch: Add support for goto table
> > > > >   LoongArch: Enable jump table for objtool
> > > > >   LoongArch: Convert unreachable() to BUG()
> > > > >   drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > > >
> > > > Are you OK with the first 8 patches?
> > > > What's the merge plan for this series?
> > >
> > > Sorry, my inbox is still reeling from the holidays.  I will review this
> > > soon.
> >
> > What are the status of the first 8 patches?
> > What is the next step? Is there anything else to be done here?
> > I would really appreciate any advice on how to get this merged.
>
> Please post a new revision rebased on tip/master, with patch 4 dropped,
> and I'll queue them up for -tip.  Thanks for your patience.
You will queue the whole series, or the first 5 patches, or the first 3 patches?

Huacai

>
> --
> Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Josh Poimboeuf]

On Tue, Feb 11, 2025 at 07:59:57PM +0800, Huacai Chen wrote:
> Hi, Josh,
> 
> On Tue, Feb 11, 2025 at 5:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> > > On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > > > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > > > Hi Josh and Peter,
> > > > >
> > > > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > > > and arch-specific functions to make the generic code more readable, tested
> > > > > > with the latest upstream mainline Binutils, GCC and Clang.
> > > > >
> > > > > ...
> > > > >
> > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > > > >
> > > > > > Tiezhu Yang (9):
> > > > > >   objtool: Handle various symbol types of rodata
> > > > > >   objtool: Handle different entry size of rodata
> > > > > >   objtool: Handle PC relative relocation type
> > > > > >   objtool: Handle unreachable entry of rodata
> > > > > >   objtool/LoongArch: Add support for switch table
> > > > > >   objtool/LoongArch: Add support for goto table
> > > > > >   LoongArch: Enable jump table for objtool
> > > > > >   LoongArch: Convert unreachable() to BUG()
> > > > > >   drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > > > >
> > > > > Are you OK with the first 8 patches?
> > > > > What's the merge plan for this series?
> > > >
> > > > Sorry, my inbox is still reeling from the holidays.  I will review this
> > > > soon.
> > >
> > > What are the status of the first 8 patches?
> > > What is the next step? Is there anything else to be done here?
> > > I would really appreciate any advice on how to get this merged.
> >
> > Please post a new revision rebased on tip/master, with patch 4 dropped,
> > and I'll queue them up for -tip.  Thanks for your patience.
> You will queue the whole series, or the first 5 patches, or the first 3 patches?

The new series now has 7 patches:

Tiezhu Yang (7):
  objtool: Handle various symbol types of rodata
  objtool: Handle different entry size of rodata
  objtool: Handle PC relative relocation type
  objtool/LoongArch: Add support for switch table
  objtool/LoongArch: Add support for goto table
  LoongArch: Enable jump table for objtool
  LoongArch: Convert unreachable() to BUG()

I was planning on queueing all 7.

In particular, patch 6 should stay with the objtool patches since
they're directly related.

But I was also just going to grab 7 as well.

Please let me know if you disagree.

-- 
Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Huacai Chen]

On Wed, Feb 12, 2025 at 7:30 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Tue, Feb 11, 2025 at 07:59:57PM +0800, Huacai Chen wrote:
> > Hi, Josh,
> >
> > On Tue, Feb 11, 2025 at 5:26 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Mon, Feb 10, 2025 at 02:07:43PM +0800, Tiezhu Yang wrote:
> > > > On 01/15/2025 09:34 AM, Josh Poimboeuf wrote:
> > > > > On Sat, Jan 11, 2025 at 02:57:42PM +0800, Tiezhu Yang wrote:
> > > > > > Hi Josh and Peter,
> > > > > >
> > > > > > On 12/17/2024 09:08 AM, Tiezhu Yang wrote:
> > > > > > > This version is based on tip/tip.git objtool/core branch [1], add some weak
> > > > > > > and arch-specific functions to make the generic code more readable, tested
> > > > > > > with the latest upstream mainline Binutils, GCC and Clang.
> > > > > >
> > > > > > ...
> > > > > >
> > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/log/?h=objtool/core
> > > > > > >
> > > > > > > Tiezhu Yang (9):
> > > > > > >   objtool: Handle various symbol types of rodata
> > > > > > >   objtool: Handle different entry size of rodata
> > > > > > >   objtool: Handle PC relative relocation type
> > > > > > >   objtool: Handle unreachable entry of rodata
> > > > > > >   objtool/LoongArch: Add support for switch table
> > > > > > >   objtool/LoongArch: Add support for goto table
> > > > > > >   LoongArch: Enable jump table for objtool
> > > > > > >   LoongArch: Convert unreachable() to BUG()
> > > > > > >   drm/amd/display: Mark dc_fixpt_from_fraction() noinline
> > > > > >
> > > > > > Are you OK with the first 8 patches?
> > > > > > What's the merge plan for this series?
> > > > >
> > > > > Sorry, my inbox is still reeling from the holidays.  I will review this
> > > > > soon.
> > > >
> > > > What are the status of the first 8 patches?
> > > > What is the next step? Is there anything else to be done here?
> > > > I would really appreciate any advice on how to get this merged.
> > >
> > > Please post a new revision rebased on tip/master, with patch 4 dropped,
> > > and I'll queue them up for -tip.  Thanks for your patience.
> > You will queue the whole series, or the first 5 patches, or the first 3 patches?
>
> The new series now has 7 patches:
>
> Tiezhu Yang (7):
>   objtool: Handle various symbol types of rodata
>   objtool: Handle different entry size of rodata
>   objtool: Handle PC relative relocation type
>   objtool/LoongArch: Add support for switch table
>   objtool/LoongArch: Add support for goto table
>   LoongArch: Enable jump table for objtool
>   LoongArch: Convert unreachable() to BUG()
>
> I was planning on queueing all 7.
>
> In particular, patch 6 should stay with the objtool patches since
> they're directly related.
>
> But I was also just going to grab 7 as well.
>
> Please let me know if you disagree.
What about you merge the first 5 patches, and then I merge the last 2
to the loongarch tree? (I prefer to merge the whole series to the
loongarch tree with your acked-by, but that may be inconvenient to
you).


Huacai

>
> --
> Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Josh Poimboeuf]

On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > The new series now has 7 patches:
> >
> > Tiezhu Yang (7):
> >   objtool: Handle various symbol types of rodata
> >   objtool: Handle different entry size of rodata
> >   objtool: Handle PC relative relocation type
> >   objtool/LoongArch: Add support for switch table
> >   objtool/LoongArch: Add support for goto table
> >   LoongArch: Enable jump table for objtool
> >   LoongArch: Convert unreachable() to BUG()
> >
> > I was planning on queueing all 7.
> >
> > In particular, patch 6 should stay with the objtool patches since
> > they're directly related.
> >
> > But I was also just going to grab 7 as well.
> >
> > Please let me know if you disagree.
> What about you merge the first 5 patches, and then I merge the last 2
> to the loongarch tree? (I prefer to merge the whole series to the
> loongarch tree with your acked-by, but that may be inconvenient to
> you).

I want the first 5 patches to go through the -tip tree because we'll
have other patches depending on them.

I'll go ahead and take the first 5.

If you take in patches 6 & 7 separately, that might introduce a lot of
warnings.  But it's up to you.

For patches 6 & 7:

Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>

-- 
Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Huacai Chen]

On Thu, Feb 13, 2025 at 10:51 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > > The new series now has 7 patches:
> > >
> > > Tiezhu Yang (7):
> > >   objtool: Handle various symbol types of rodata
> > >   objtool: Handle different entry size of rodata
> > >   objtool: Handle PC relative relocation type
> > >   objtool/LoongArch: Add support for switch table
> > >   objtool/LoongArch: Add support for goto table
> > >   LoongArch: Enable jump table for objtool
> > >   LoongArch: Convert unreachable() to BUG()
> > >
> > > I was planning on queueing all 7.
> > >
> > > In particular, patch 6 should stay with the objtool patches since
> > > they're directly related.
> > >
> > > But I was also just going to grab 7 as well.
> > >
> > > Please let me know if you disagree.
> > What about you merge the first 5 patches, and then I merge the last 2
> > to the loongarch tree? (I prefer to merge the whole series to the
> > loongarch tree with your acked-by, but that may be inconvenient to
> > you).
>
> I want the first 5 patches to go through the -tip tree because we'll
> have other patches depending on them.
>
> I'll go ahead and take the first 5.
>
> If you take in patches 6 & 7 separately, that might introduce a lot of
> warnings.  But it's up to you.
>
> For patches 6 & 7:
>
> Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
OK, please take the first 5 patches, I will merge your objtool/core to
the loongarch tree and then apply the last 2 to avoid build warnings.

Huacai

>
> --
> Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Josh Poimboeuf]

On Mon, Feb 17, 2025 at 11:13:43AM +0800, Huacai Chen wrote:
> On Thu, Feb 13, 2025 at 10:51 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> >
> > On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > > > The new series now has 7 patches:
> > > >
> > > > Tiezhu Yang (7):
> > > >   objtool: Handle various symbol types of rodata
> > > >   objtool: Handle different entry size of rodata
> > > >   objtool: Handle PC relative relocation type
> > > >   objtool/LoongArch: Add support for switch table
> > > >   objtool/LoongArch: Add support for goto table
> > > >   LoongArch: Enable jump table for objtool
> > > >   LoongArch: Convert unreachable() to BUG()
> > > >
> > > > I was planning on queueing all 7.
> > > >
> > > > In particular, patch 6 should stay with the objtool patches since
> > > > they're directly related.
> > > >
> > > > But I was also just going to grab 7 as well.
> > > >
> > > > Please let me know if you disagree.
> > > What about you merge the first 5 patches, and then I merge the last 2
> > > to the loongarch tree? (I prefer to merge the whole series to the
> > > loongarch tree with your acked-by, but that may be inconvenient to
> > > you).
> >
> > I want the first 5 patches to go through the -tip tree because we'll
> > have other patches depending on them.
> >
> > I'll go ahead and take the first 5.
> >
> > If you take in patches 6 & 7 separately, that might introduce a lot of
> > warnings.  But it's up to you.
> >
> > For patches 6 & 7:
> >
> > Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
> OK, please take the first 5 patches, I will merge your objtool/core to
> the loongarch tree and then apply the last 2 to avoid build warnings.

Looks like that's not going to work.  Without patch 7 I'm getting a
warning (upgraded to a build error with a pending change to upgrade
objtool warnings to errors):

arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot() falls through to next function crash_shutdown_secondary()
arch/loongarch/kernel/machine_kexec.o: error: objtool:   kexec_reboot+0x1c: (branch)
arch/loongarch/kernel/machine_kexec.o: error: objtool:   kexec_reboot+0x0: <=== (sym)
arch/loongarch/kernel/machine_kexec.o: error: objtool: 1 warning(s) upgraded to errors

And that would break bisection anyway, so that really needs to come
before the others.

-- 
Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Huacai Chen]

On Wed, Feb 19, 2025 at 1:46 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Mon, Feb 17, 2025 at 11:13:43AM +0800, Huacai Chen wrote:
> > On Thu, Feb 13, 2025 at 10:51 AM Josh Poimboeuf <jpoimboe@kernel.org> wrote:
> > >
> > > On Wed, Feb 12, 2025 at 03:22:45PM +0800, Huacai Chen wrote:
> > > > > The new series now has 7 patches:
> > > > >
> > > > > Tiezhu Yang (7):
> > > > >   objtool: Handle various symbol types of rodata
> > > > >   objtool: Handle different entry size of rodata
> > > > >   objtool: Handle PC relative relocation type
> > > > >   objtool/LoongArch: Add support for switch table
> > > > >   objtool/LoongArch: Add support for goto table
> > > > >   LoongArch: Enable jump table for objtool
> > > > >   LoongArch: Convert unreachable() to BUG()
> > > > >
> > > > > I was planning on queueing all 7.
> > > > >
> > > > > In particular, patch 6 should stay with the objtool patches since
> > > > > they're directly related.
> > > > >
> > > > > But I was also just going to grab 7 as well.
> > > > >
> > > > > Please let me know if you disagree.
> > > > What about you merge the first 5 patches, and then I merge the last 2
> > > > to the loongarch tree? (I prefer to merge the whole series to the
> > > > loongarch tree with your acked-by, but that may be inconvenient to
> > > > you).
> > >
> > > I want the first 5 patches to go through the -tip tree because we'll
> > > have other patches depending on them.
> > >
> > > I'll go ahead and take the first 5.
> > >
> > > If you take in patches 6 & 7 separately, that might introduce a lot of
> > > warnings.  But it's up to you.
> > >
> > > For patches 6 & 7:
> > >
> > > Acked-by: Josh Poimboeuf <jpoimboe@kernel.org>
> > OK, please take the first 5 patches, I will merge your objtool/core to
> > the loongarch tree and then apply the last 2 to avoid build warnings.
>
> Looks like that's not going to work.  Without patch 7 I'm getting a
> warning (upgraded to a build error with a pending change to upgrade
> objtool warnings to errors):
>
> arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot() falls through to next function crash_shutdown_secondary()
> arch/loongarch/kernel/machine_kexec.o: error: objtool:   kexec_reboot+0x1c: (branch)
> arch/loongarch/kernel/machine_kexec.o: error: objtool:   kexec_reboot+0x0: <=== (sym)
> arch/loongarch/kernel/machine_kexec.o: error: objtool: 1 warning(s) upgraded to errors
>
> And that would break bisection anyway, so that really needs to come
> before the others.
OK, then please take the whole series of V7, but please exchange the
order of Patch-6 and Patch-7 (I think the enablement should be the
last for bisection).
For V7: Acked-by: Huacai Chen <chenhuacai@loongson.cn>


Huacai

>
> --
> Josh

Re: [PATCH v6 0/9] Add jump table support for objtool on LoongArch

[Josh Poimboeuf]

On Wed, Feb 19, 2025 at 05:49:01PM +0800, Huacai Chen wrote:
> > Looks like that's not going to work.  Without patch 7 I'm getting a
> > warning (upgraded to a build error with a pending change to upgrade
> > objtool warnings to errors):
> >
> > arch/loongarch/kernel/machine_kexec.o: error: objtool: kexec_reboot() falls through to next function crash_shutdown_secondary()
> > arch/loongarch/kernel/machine_kexec.o: error: objtool:   kexec_reboot+0x1c: (branch)
> > arch/loongarch/kernel/machine_kexec.o: error: objtool:   kexec_reboot+0x0: <=== (sym)
> > arch/loongarch/kernel/machine_kexec.o: error: objtool: 1 warning(s) upgraded to errors
> >
> > And that would break bisection anyway, so that really needs to come
> > before the others.
> OK, then please take the whole series of V7, but please exchange the
> order of Patch-6 and Patch-7 (I think the enablement should be the
> last for bisection).
> For V7: Acked-by: Huacai Chen <chenhuacai@loongson.cn>

Ok, but to avoid the warning, patch 7 needs to come *before* the objtool
patches, so it will be the following order:

LoongArch: Convert unreachable() to BUG()
objtool: Handle various symbol types of rodata
objtool: Handle different entry size of rodata
objtool: Handle PC relative relocation type
objtool/LoongArch: Add support for switch table
objtool/LoongArch: Add support for goto table
LoongArch: Enable jump table for objtool

-- 
Josh