[PATCH v3 0/5] rust: Add bug/warn abstractions

[PATCH v3 0/5] rust: Add bug/warn abstractions

[FUJITA Tomonori]

This patchset adds warn_on and warn_on_once macros with the bug/warn
abstraction.

Wrapping C's BUG/WARN macros does not work well for x86, RISC-V,
ARM64, and LoongArch. Rust code needs to directly execute the same
assembly code used on the C side. To avoid duplicating the assembly
code, this approach mirrors what the static branch code does: it
generates the assembly code for Rust using the C preprocessor at
compile time.

The 1st to 4th patches export the BUG/WARN assembly code for Rust on
each architecture, with no functional changes on the C side. The
changes for x86 and RISC-V are straightforward. However, the ARM64 and
LoongArch assembly code are designed differently; they are used by
both C inline assembly and assembly code. As a result, sharing this
code with Rust is complicated.

The last patch adds the bug abstraction with warn_on and warn_on_once
implementations.

This has been tested on x86, ARM64, and RISC-V (QEMU), with only a
compile test performed for LoongArch.

The assembly code can be used for both BUG and WARN, but currently
only supports warn_on and warn_on_once. I will work on the remaining
functionality after this abstraction is merged.

v3:
- rebased on rust-next
- use ANNOTATE_REACHABLE macro (replaced ASM_REACHABLE)
- added Acked-by tag to the x86 change
v2: https://lore.kernel.org/linux-arm-kernel/20241218062009.2402650-1-fujita.tomonori@gmail.com/
- remove target_arch cfg by using asm comment
- clean up the changes to loongarch asm
v1: https://lore.kernel.org/linux-arm-kernel/20241210001802.228725-1-fujita.tomonori@gmail.com/

FUJITA Tomonori (5):
  x86/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with
    Rust
  riscv/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with
    Rust
  arm64/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with
    Rust
  loongarch/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing
    with Rust
  rust: Add warn_on and warn_on_once

 arch/arm64/include/asm/asm-bug.h              |  33 +++++-
 arch/loongarch/include/asm/bug.h              |  35 +++++-
 arch/riscv/include/asm/bug.h                  |  37 ++++---
 arch/x86/include/asm/bug.h                    |  56 +++++-----
 rust/Makefile                                 |   8 ++
 rust/kernel/.gitignore                        |   2 +
 rust/kernel/bug.rs                            | 100 ++++++++++++++++++
 rust/kernel/generated_arch_reachable_asm.rs.S |   7 ++
 rust/kernel/generated_arch_warn_asm.rs.S      |   7 ++
 rust/kernel/lib.rs                            |   1 +
 10 files changed, 235 insertions(+), 51 deletions(-)
 create mode 100644 rust/kernel/bug.rs
 create mode 100644 rust/kernel/generated_arch_reachable_asm.rs.S
 create mode 100644 rust/kernel/generated_arch_warn_asm.rs.S


base-commit: beeb78d46249cab8b2b8359a2ce8fa5376b5ad2d
-- 
2.43.0

[PATCH v3 1/5] x86/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with Rust

[FUJITA Tomonori]

Add new ARCH_WARN_ASM macro for BUG/WARN assembly code sharing with
Rust to avoid the duplication.

No functional changes.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 arch/x86/include/asm/bug.h | 56 +++++++++++++++++++-------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index e85ac0c7c039..61570ec9464c 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -28,45 +28,42 @@
 #ifdef CONFIG_GENERIC_BUG
 
 #ifdef CONFIG_X86_32
-# define __BUG_REL(val)	".long " __stringify(val)
+# define __BUG_REL(val)	".long " val
 #else
-# define __BUG_REL(val)	".long " __stringify(val) " - ."
+# define __BUG_REL(val)	".long " val " - ."
 #endif
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
+#define __BUG_ENTRY(file, line, flags)					\
+	"2:\t" __BUG_REL("1b") "\t# bug_entry::bug_addr\n"		\
+	"\t" __BUG_REL(file)   "\t# bug_entry::file\n"			\
+	"\t.word " line        "\t# bug_entry::line\n"			\
+	"\t.word " flags       "\t# bug_entry::flags\n"
+#else
+#define __BUG_ENTRY(file, ine, flags)					\
+	"2:\t" __BUG_REL("1b") "\t# bug_entry::bug_addr\n"		\
+	"\t.word " flags       "\t# bug_entry::flags\n"
+#endif
+
+#define _BUG_FLAGS_ASM(ins, file, line, flags, size, extra)		\
+	"1:\t" ins "\n"							\
+	".pushsection __bug_table,\"aw\"\n"				\
+	__BUG_ENTRY(file, line, flags)					\
+	"\t.org 2b + " size "\n"					\
+	".popsection\n"							\
+	extra
 
 #define _BUG_FLAGS(ins, flags, extra)					\
 do {									\
-	asm_inline volatile("1:\t" ins "\n"				\
-		     ".pushsection __bug_table,\"aw\"\n"		\
-		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
-		     "\t"  __BUG_REL(%c0) "\t# bug_entry::file\n"	\
-		     "\t.word %c1"        "\t# bug_entry::line\n"	\
-		     "\t.word %c2"        "\t# bug_entry::flags\n"	\
-		     "\t.org 2b+%c3\n"					\
-		     ".popsection\n"					\
-		     extra						\
+	asm_inline volatile(_BUG_FLAGS_ASM(ins, "%c0",			\
+					   "%c1", "%c2", "%c3", extra)	\
 		     : : "i" (__FILE__), "i" (__LINE__),		\
 			 "i" (flags),					\
 			 "i" (sizeof(struct bug_entry)));		\
 } while (0)
 
-#else /* !CONFIG_DEBUG_BUGVERBOSE */
-
-#define _BUG_FLAGS(ins, flags, extra)					\
-do {									\
-	asm_inline volatile("1:\t" ins "\n"				\
-		     ".pushsection __bug_table,\"aw\"\n"		\
-		     "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n"	\
-		     "\t.word %c0"        "\t# bug_entry::flags\n"	\
-		     "\t.org 2b+%c1\n"					\
-		     ".popsection\n"					\
-		     extra						\
-		     : : "i" (flags),					\
-			 "i" (sizeof(struct bug_entry)));		\
-} while (0)
-
-#endif /* CONFIG_DEBUG_BUGVERBOSE */
+#define ARCH_WARN_ASM(file, line, flags, size)				\
+	_BUG_FLAGS_ASM(ASM_UD2, file, line, flags, size, "")
 
 #else
 
@@ -88,11 +85,14 @@ do {								\
  * were to trigger, we'd rather wreck the machine in an attempt to get the
  * message out than not know about it.
  */
+
+#define ARCH_WARN_REACHABLE	ANNOTATE_REACHABLE(1b)
+
 #define __WARN_FLAGS(flags)					\
 do {								\
 	__auto_type __flags = BUGFLAG_WARNING|(flags);		\
 	instrumentation_begin();				\
-	_BUG_FLAGS(ASM_UD2, __flags, ANNOTATE_REACHABLE(1b));	\
+	_BUG_FLAGS(ASM_UD2, __flags, ARCH_WARN_REACHABLE);	\
 	instrumentation_end();					\
 } while (0)
 
-- 
2.43.0

[PATCH v3 2/5] riscv/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with Rust

[FUJITA Tomonori]

Add new ARCH_WARN_ASM macro for BUG/WARN assembly code sharing with
Rust to avoid the duplication.

No functional changes.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 arch/riscv/include/asm/bug.h | 37 +++++++++++++++++++++---------------
 1 file changed, 22 insertions(+), 15 deletions(-)

diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
index 1aaea81fb141..6ab13b56feb0 100644
--- a/arch/riscv/include/asm/bug.h
+++ b/arch/riscv/include/asm/bug.h
@@ -31,40 +31,45 @@ typedef u32 bug_insn_t;
 
 #ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
 #define __BUG_ENTRY_ADDR	RISCV_INT " 1b - ."
-#define __BUG_ENTRY_FILE	RISCV_INT " %0 - ."
+#define __BUG_ENTRY_FILE(file)	RISCV_INT " " file " - ."
 #else
 #define __BUG_ENTRY_ADDR	RISCV_PTR " 1b"
-#define __BUG_ENTRY_FILE	RISCV_PTR " %0"
+#define __BUG_ENTRY_FILE(file)	RISCV_PTR " " file
 #endif
 
 #ifdef CONFIG_DEBUG_BUGVERBOSE
-#define __BUG_ENTRY			\
+#define __BUG_ENTRY(file, line, flags)	\
 	__BUG_ENTRY_ADDR "\n\t"		\
-	__BUG_ENTRY_FILE "\n\t"		\
-	RISCV_SHORT " %1\n\t"		\
-	RISCV_SHORT " %2"
+	__BUG_ENTRY_FILE(file) "\n\t"	\
+	RISCV_SHORT " " line "\n\t"	\
+	RISCV_SHORT " " flags
 #else
-#define __BUG_ENTRY			\
-	__BUG_ENTRY_ADDR "\n\t"		\
-	RISCV_SHORT " %2"
+#define __BUG_ENTRY(file, line, flags)		\
+	__BUG_ENTRY_ADDR "\n\t"			\
+	RISCV_SHORT " " flags
 #endif
 
 #ifdef CONFIG_GENERIC_BUG
-#define __BUG_FLAGS(flags)					\
-do {								\
-	__asm__ __volatile__ (					\
+
+#define ARCH_WARN_ASM(file, line, flags, size)			\
 		"1:\n\t"					\
 			"ebreak\n"				\
 			".pushsection __bug_table,\"aw\"\n\t"	\
 		"2:\n\t"					\
-			__BUG_ENTRY "\n\t"			\
-			".org 2b + %3\n\t"                      \
-			".popsection"				\
+		__BUG_ENTRY(file, line, flags) "\n\t"		\
+			".org 2b + " size "\n\t"                \
+			".popsection\n"				\
+
+#define __BUG_FLAGS(flags)					\
+do {								\
+	__asm__ __volatile__ (					\
+		ARCH_WARN_ASM("%0", "%1", "%2", "%3")		\
 		:						\
 		: "i" (__FILE__), "i" (__LINE__),		\
 		  "i" (flags),					\
 		  "i" (sizeof(struct bug_entry)));              \
 } while (0)
+
 #else /* CONFIG_GENERIC_BUG */
 #define __BUG_FLAGS(flags) do {					\
 	__asm__ __volatile__ ("ebreak\n");			\
@@ -78,6 +83,8 @@ do {								\
 
 #define __WARN_FLAGS(flags) __BUG_FLAGS(BUGFLAG_WARNING|(flags))
 
+#define ARCH_WARN_REACHABLE
+
 #define HAVE_ARCH_BUG
 
 #include <asm-generic/bug.h>
-- 
2.43.0

[PATCH v3 3/5] arm64/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with Rust

[FUJITA Tomonori]

Add new ARCH_WARN_ASM macro for BUG/WARN assembly code sharing with
Rust to avoid the duplication.

No functional changes.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 arch/arm64/include/asm/asm-bug.h | 33 ++++++++++++++++++++++++++++----
 1 file changed, 29 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/asm-bug.h b/arch/arm64/include/asm/asm-bug.h
index 6e73809f6492..a5f13801b784 100644
--- a/arch/arm64/include/asm/asm-bug.h
+++ b/arch/arm64/include/asm/asm-bug.h
@@ -21,16 +21,21 @@
 #endif
 
 #ifdef CONFIG_GENERIC_BUG
-
-#define __BUG_ENTRY(flags) 				\
+#define __BUG_ENTRY_START				\
 		.pushsection __bug_table,"aw";		\
 		.align 2;				\
 	14470:	.long 14471f - .;			\
-_BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
-		.short flags; 				\
+
+#define __BUG_ENTRY_END					\
 		.align 2;				\
 		.popsection;				\
 	14471:
+
+#define __BUG_ENTRY(flags)				\
+		__BUG_ENTRY_START			\
+_BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
+		.short flags;				\
+		__BUG_ENTRY_END
 #else
 #define __BUG_ENTRY(flags)
 #endif
@@ -41,4 +46,24 @@ _BUGVERBOSE_LOCATION(__FILE__, __LINE__)		\
 
 #define ASM_BUG()	ASM_BUG_FLAGS(0)
 
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+#define __BUG_LOCATION_STRING(file, line)		\
+		".long " file "- .;"			\
+		".short " line ";"
+#else
+#define __BUG_LOCATION_STRING(file, line)
+#endif
+
+#define __BUG_ENTRY_STRING(file, line, flags)		\
+		__stringify(__BUG_ENTRY_START)		\
+		__BUG_LOCATION_STRING(file, line)	\
+		".short " flags ";"			\
+		__stringify(__BUG_ENTRY_END)
+
+#define ARCH_WARN_ASM(file, line, flags, size)		\
+	__BUG_ENTRY_STRING(file, line, flags)		\
+	__stringify(brk BUG_BRK_IMM)
+
+#define ARCH_WARN_REACHABLE
+
 #endif /* __ASM_ASM_BUG_H */
-- 
2.43.0

[PATCH v3 4/5] loongarch/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with Rust

[FUJITA Tomonori]

Add new ARCH_WARN_ASM macro for BUG/WARN assembly code sharing with
Rust to avoid the duplication.

No functional changes.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 arch/loongarch/include/asm/bug.h | 35 ++++++++++++++++++++++++++++----
 1 file changed, 31 insertions(+), 4 deletions(-)

diff --git a/arch/loongarch/include/asm/bug.h b/arch/loongarch/include/asm/bug.h
index f6f254f2c5db..06903aef1101 100644
--- a/arch/loongarch/include/asm/bug.h
+++ b/arch/loongarch/include/asm/bug.h
@@ -22,14 +22,21 @@
 #ifndef CONFIG_GENERIC_BUG
 #define __BUG_ENTRY(flags)
 #else
-#define __BUG_ENTRY(flags) 					\
+
+#define __BUG_ENTRY_START					\
 		.pushsection __bug_table, "aw";			\
 		.align 2;					\
 	10000:	.long 10001f - .;				\
-		_BUGVERBOSE_LOCATION(__FILE__, __LINE__)	\
-		.short flags; 					\
+
+#define __BUG_ENTRY_END						\
 		.popsection;					\
 	10001:
+
+#define __BUG_ENTRY(flags)					\
+		__BUG_ENTRY_START			\
+		_BUGVERBOSE_LOCATION(__FILE__, __LINE__)	\
+		.short flags;					\
+		__BUG_ENTRY_END
 #endif
 
 #define ASM_BUG_FLAGS(flags)					\
@@ -42,10 +49,12 @@
 	asm_inline volatile (__stringify(ASM_BUG_FLAGS(flags))		\
 			     extra);
 
+#define ARCH_WARN_REACHABLE	ANNOTATE_REACHABLE(10001b)
+
 #define __WARN_FLAGS(flags)					\
 do {								\
 	instrumentation_begin();				\
-	__BUG_FLAGS(BUGFLAG_WARNING|(flags), ANNOTATE_REACHABLE(10001b));\
+	__BUG_FLAGS(BUGFLAG_WARNING|(flags), ARCH_WARN_REACHABLE);\
 	instrumentation_end();					\
 } while (0)
 
@@ -56,6 +65,24 @@ do {								\
 	unreachable();						\
 } while (0)
 
+#ifdef CONFIG_DEBUG_BUGVERBOSE
+#define __BUG_LOCATION_STRING(file, line)			\
+		".long " file "- .;"				\
+		".short " line ";"
+#else
+#define __BUG_LOCATION_STRING(file, line)
+#endif
+
+#define __BUG_ENTRY_STRING(file, line, flags)			\
+		__stringify(__BUG_ENTRY_START)			\
+		__BUG_LOCATION_STRING(file, line)		\
+		".short " flags ";"				\
+		__stringify(__BUG_ENTRY_END)
+
+#define ARCH_WARN_ASM(file, line, flags, size)			\
+	__BUG_ENTRY_STRING(file, line, flags)			\
+	__stringify(break BRK_BUG) ";"
+
 #define HAVE_ARCH_BUG
 
 #include <asm-generic/bug.h>
-- 
2.43.0

[PATCH v3 5/5] rust: Add warn_on and warn_on_once

[FUJITA Tomonori]

Add warn_on and warn_on_once macros. Wrapping the C's WARN_* and BUG_*
macros doesn't work so this uses the assembly code exported by the C
side via ARCH_WARN_ASM macro. Like the static branch code, this
generates the assembly code for rust at compile time by using the C
preprocessor.

file()! macro doesn't work for the Rust inline assembly in the same
way as __FILE__ for the C inline assembly. So the code to handle a
file name is different from the C assembly code (similar to the
arm64/loongarch assembly).

Similarly, ARCH_WARN_REACHABLE is also used at compile time to
generate the assembly code; objtool's reachable anotation code. Only
architectures that use objtool (x86 and loongarch) need it.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/Makefile                                 |   8 ++
 rust/kernel/.gitignore                        |   2 +
 rust/kernel/bug.rs                            | 100 ++++++++++++++++++
 rust/kernel/generated_arch_reachable_asm.rs.S |   7 ++
 rust/kernel/generated_arch_warn_asm.rs.S      |   7 ++
 rust/kernel/lib.rs                            |   1 +
 6 files changed, 125 insertions(+)
 create mode 100644 rust/kernel/bug.rs
 create mode 100644 rust/kernel/generated_arch_reachable_asm.rs.S
 create mode 100644 rust/kernel/generated_arch_warn_asm.rs.S

diff --git a/rust/Makefile b/rust/Makefile
index 8fcfd60447bc..a295b65c43f3 100644
--- a/rust/Makefile
+++ b/rust/Makefile
@@ -34,6 +34,9 @@ obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o
 obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
 
 always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) += kernel/generated_arch_static_branch_asm.rs
+ifndef CONFIG_UML
+always-$(subst y,$(CONFIG_RUST),$(CONFIG_BUG)) += kernel/generated_arch_warn_asm.rs kernel/generated_arch_reachable_asm.rs
+endif
 
 # Avoids running `$(RUSTC)` when it may not be available.
 ifdef CONFIG_RUST
@@ -481,5 +484,10 @@ $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/build_error.o \
 ifdef CONFIG_JUMP_LABEL
 $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs
 endif
+ifndef CONFIG_UML
+ifdef CONFIG_BUG
+$(obj)/kernel.o: $(obj)/kernel/generated_arch_warn_asm.rs $(obj)/kernel/generated_arch_reachable_asm.rs
+endif
+endif
 
 endif # CONFIG_RUST
diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
index 6ba39a178f30..f1d7f4225332 100644
--- a/rust/kernel/.gitignore
+++ b/rust/kernel/.gitignore
@@ -1,3 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 
 /generated_arch_static_branch_asm.rs
+/generated_arch_warn_asm.rs
+/generated_arch_reachable_asm.rs
\ No newline at end of file
diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs
new file mode 100644
index 000000000000..7ffd9cb1ad75
--- /dev/null
+++ b/rust/kernel/bug.rs
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+
+// Copyright (C) 2024 FUJITA Tomonori
+
+//! Support for BUG_* and WARN_* functionality.
+//!
+//! C header: [`include/asm-generic/bug.h`](srctree/include/asm-generic/bug.h)
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
+macro_rules! warn_flags {
+    ($flags:expr) => {
+        const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
+        // SAFETY: Just an FFI call.
+        #[cfg(CONFIG_DEBUG_BUGVERBOSE)]
+        unsafe {
+            $crate::asm!(concat!(
+                "/* {size} */",
+                ".pushsection .rodata.str1.1, \"aMS\",@progbits, 1\n",
+                "111:\t .string ", "\"", file!(), "\"\n",
+                ".popsection\n",
+                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
+                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
+            line = const line!(),
+            flags = const FLAGS,
+            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
+            );
+        }
+        // SAFETY: Just an FFI call.
+        #[cfg(not(CONFIG_DEBUG_BUGVERBOSE))]
+        unsafe {
+            $crate::asm!(
+            concat!(
+                "/* {size} */",
+                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
+                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
+            flags = const FLAGS,
+            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
+            );
+        }
+    }
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(all(CONFIG_BUG, CONFIG_UML))]
+macro_rules! warn_flags {
+    ($flags:expr) => {
+        // SAFETY: Just an FFI call.
+        unsafe {
+            $crate::bindings::warn_slowpath_fmt(
+                $crate::c_str!(::core::file!()).as_ptr() as *const ::core::ffi::c_char,
+                line!() as i32,
+                $flags as u32,
+                ::core::ptr::null() as *const ::core::ffi::c_char,
+            );
+        }
+    };
+}
+
+#[macro_export]
+#[doc(hidden)]
+#[cfg(not(CONFIG_BUG))]
+macro_rules! warn_flags {
+    ($flags:expr) => {};
+}
+
+#[doc(hidden)]
+#[macro_export]
+macro_rules! bugflag_taint {
+    ($taint:expr) => {
+        $taint << 8
+    };
+}
+
+/// Report a warning only once.
+#[macro_export]
+macro_rules! warn_on_once {
+    ($cond:expr) => {
+        if $cond {
+            $crate::warn_flags!(
+                $crate::bindings::BUGFLAG_ONCE
+                    | $crate::bugflag_taint!($crate::bindings::TAINT_WARN)
+            );
+        }
+        $cond
+    };
+}
+
+/// Report a warning.
+#[macro_export]
+macro_rules! warn_on {
+    ($cond:expr) => {
+        if $cond {
+            $crate::warn_flags!($crate::bugflag_taint!($crate::bindings::TAINT_WARN));
+        }
+        $cond
+    };
+}
diff --git a/rust/kernel/generated_arch_reachable_asm.rs.S b/rust/kernel/generated_arch_reachable_asm.rs.S
new file mode 100644
index 000000000000..3886a9ad3a99
--- /dev/null
+++ b/rust/kernel/generated_arch_reachable_asm.rs.S
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/bug.h>
+
+// Cut here.
+
+::kernel::concat_literals!(ARCH_WARN_REACHABLE)
diff --git a/rust/kernel/generated_arch_warn_asm.rs.S b/rust/kernel/generated_arch_warn_asm.rs.S
new file mode 100644
index 000000000000..8f239b431aa4
--- /dev/null
+++ b/rust/kernel/generated_arch_warn_asm.rs.S
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <linux/bug.h>
+
+// Cut here.
+
+::kernel::concat_literals!(ARCH_WARN_ASM("111b", "{line}",  "{flags}", "{size}"))
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 496ed32b0911..c0720846b249 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -38,6 +38,7 @@
 pub mod alloc;
 #[cfg(CONFIG_BLOCK)]
 pub mod block;
+pub mod bug;
 #[doc(hidden)]
 pub mod build_assert;
 pub mod cred;
-- 
2.43.0

Re: [PATCH v3 3/5] arm64/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with Rust

[Catalin Marinas]

On Thu, Feb 13, 2025 at 10:57:57PM +0900, FUJITA Tomonori wrote:
> Add new ARCH_WARN_ASM macro for BUG/WARN assembly code sharing with
> Rust to avoid the duplication.
> 
> No functional changes.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v3 0/5] rust: Add bug/warn abstractions

[Andreas Hindborg]

Hi Fujita,

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> This patchset adds warn_on and warn_on_once macros with the bug/warn
> abstraction.
>
> Wrapping C's BUG/WARN macros does not work well for x86, RISC-V,
> ARM64, and LoongArch. Rust code needs to directly execute the same
> assembly code used on the C side. To avoid duplicating the assembly
> code, this approach mirrors what the static branch code does: it
> generates the assembly code for Rust using the C preprocessor at
> compile time.
>
> The 1st to 4th patches export the BUG/WARN assembly code for Rust on
> each architecture, with no functional changes on the C side. The
> changes for x86 and RISC-V are straightforward. However, the ARM64 and
> LoongArch assembly code are designed differently; they are used by
> both C inline assembly and assembly code. As a result, sharing this
> code with Rust is complicated.
>
> The last patch adds the bug abstraction with warn_on and warn_on_once
> implementations.
>
> This has been tested on x86, ARM64, and RISC-V (QEMU), with only a
> compile test performed for LoongArch.
>
> The assembly code can be used for both BUG and WARN, but currently
> only supports warn_on and warn_on_once. I will work on the remaining
> functionality after this abstraction is merged.
>

How does this series compare/overlap with [1] ?


Best regards,
Andreas Hindborg



[1] https://lore.kernel.org/all/20241126-pr_once_macros-v4-0-410b8ca9643e@tuta.io/

Re: [PATCH v3 0/5] rust: Add bug/warn abstractions

[FUJITA Tomonori]

On Wed, 26 Feb 2025 20:39:45 +0100
Andreas Hindborg <a.hindborg@kernel.org> wrote:

> How does this series compare/overlap with [1] ?
> 
> [1] https://lore.kernel.org/all/20241126-pr_once_macros-v4-0-410b8ca9643e@tuta.io/

No overlap. Each solves a different problem. Both are necessary.

This patchset enables Rust code to call C's BUG/WARN properly.

Currently, Rust's BUG() is a simple wrapper for C's BUG()
(rust/helpers/bug.c). I added BUG() to rnull's init() and got the
following:

# insmod /root/rnull_mod.ko
rnull_mod: Rust null_blk loaded
------------[ cut here ]------------
kernel BUG at rust/helpers/bug.c:7!
Oops: invalid opcode: 0000 [#1] SMP
CPU: 0 UID: 0 PID: 31 Comm: insmod Not tainted 6.14.0-rc1+ #103
RIP: 0010:rust_helper_BUG+0x8/0x10
(snip)

This is NOT debug information that we expect. The problem is that
BUG/WARN feature (lib/bug.c) can only be used from assembly.

This patchset includes only warn() but with bug() implementation on
top of this patchset, I got:

# insmod /root/rnull_mod.ko
rnull_mod: Rust null_blk loaded
------------[ cut here ]------------
WARNING: CPU: 0 PID: 31 at /home/fujita/git/linux-rust/drivers/block/rnull.rs:46 _RNvXCsafUg3oOYix8_5rnullNtB2_13NullBlkModu]
Modules linked in: rnull_mod(+)
CPU: 0 UID: 0 PID: 31 Comm: insmod Not tainted 6.14.0-rc1+ #104
RIP: 0010:_RNvXCsafUg3oOYix8_5rnullNtB2_13NullBlkModuleNtCsaYBeKL739Xz_6kernel13InPlaceModule4init+0x71/0x4f0 [rnull_mod]


The [1] patchset adds an abstraciton for include/linux/once_lite.h,
'call a function once' feature, with pr_*_once() implementation.

pr_*_once() just calls printk() once. Unlike BUG/WARN, no debug
information (call place, registers, stack trace, etc).


The only connection between two patchset is that WARN_ONCE() can be
built on top of both like the C side.

Re: [PATCH v3 3/5] arm64/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with Rust

[FUJITA Tomonori]

On Wed, 26 Feb 2025 19:27:55 +0000
Catalin Marinas <catalin.marinas@arm.com> wrote:

> On Thu, Feb 13, 2025 at 10:57:57PM +0900, FUJITA Tomonori wrote:
>> Add new ARCH_WARN_ASM macro for BUG/WARN assembly code sharing with
>> Rust to avoid the duplication.
>> 
>> No functional changes.
>> 
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>

Thanks a lot!

RISC-V and LoongArch maintainers,

Can I get an ack? Or you prefer a different approach?

Re: [PATCH v3 0/5] rust: Add bug/warn abstractions

[Andreas Hindborg]

"FUJITA Tomonori" <fujita.tomonori@gmail.com> writes:

> On Wed, 26 Feb 2025 20:39:45 +0100
> Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
>> How does this series compare/overlap with [1] ?
>>
>> [1] https://lore.kernel.org/all/20241126-pr_once_macros-v4-0-410b8ca9643e@tuta.io/
>
> No overlap. Each solves a different problem. Both are necessary.
>
> This patchset enables Rust code to call C's BUG/WARN properly.
>
> Currently, Rust's BUG() is a simple wrapper for C's BUG()
> (rust/helpers/bug.c). I added BUG() to rnull's init() and got the
> following:
>
> # insmod /root/rnull_mod.ko
> rnull_mod: Rust null_blk loaded
> ------------[ cut here ]------------
> kernel BUG at rust/helpers/bug.c:7!
> Oops: invalid opcode: 0000 [#1] SMP
> CPU: 0 UID: 0 PID: 31 Comm: insmod Not tainted 6.14.0-rc1+ #103
> RIP: 0010:rust_helper_BUG+0x8/0x10
> (snip)
>
> This is NOT debug information that we expect. The problem is that
> BUG/WARN feature (lib/bug.c) can only be used from assembly.
>
> This patchset includes only warn() but with bug() implementation on
> top of this patchset, I got:
>
> # insmod /root/rnull_mod.ko
> rnull_mod: Rust null_blk loaded
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 31 at /home/fujita/git/linux-rust/drivers/block/rnull.rs:46 _RNvXCsafUg3oOYix8_5rnullNtB2_13NullBlkModu]
> Modules linked in: rnull_mod(+)
> CPU: 0 UID: 0 PID: 31 Comm: insmod Not tainted 6.14.0-rc1+ #104
> RIP: 0010:_RNvXCsafUg3oOYix8_5rnullNtB2_13NullBlkModuleNtCsaYBeKL739Xz_6kernel13InPlaceModule4init+0x71/0x4f0 [rnull_mod]
>
>
> The [1] patchset adds an abstraciton for include/linux/once_lite.h,
> 'call a function once' feature, with pr_*_once() implementation.
>
> pr_*_once() just calls printk() once. Unlike BUG/WARN, no debug
> information (call place, registers, stack trace, etc).
>
>
> The only connection between two patchset is that WARN_ONCE() can be
> built on top of both like the C side.

Awesome, thanks for explaining 👍


Best regards,
Andreas Hindborg

Re: [PATCH v3 2/5] riscv/bug: Add ARCH_WARN_ASM macro for BUG/WARN asm code sharing with Rust

[Alexandre Ghiti]

Hi FUJITA,

On 13/02/2025 14:57, FUJITA Tomonori wrote:
> Add new ARCH_WARN_ASM macro for BUG/WARN assembly code sharing with
> Rust to avoid the duplication.
>
> No functional changes.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>   arch/riscv/include/asm/bug.h | 37 +++++++++++++++++++++---------------
>   1 file changed, 22 insertions(+), 15 deletions(-)
>
> diff --git a/arch/riscv/include/asm/bug.h b/arch/riscv/include/asm/bug.h
> index 1aaea81fb141..6ab13b56feb0 100644
> --- a/arch/riscv/include/asm/bug.h
> +++ b/arch/riscv/include/asm/bug.h
> @@ -31,40 +31,45 @@ typedef u32 bug_insn_t;
>   
>   #ifdef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
>   #define __BUG_ENTRY_ADDR	RISCV_INT " 1b - ."
> -#define __BUG_ENTRY_FILE	RISCV_INT " %0 - ."
> +#define __BUG_ENTRY_FILE(file)	RISCV_INT " " file " - ."
>   #else
>   #define __BUG_ENTRY_ADDR	RISCV_PTR " 1b"
> -#define __BUG_ENTRY_FILE	RISCV_PTR " %0"
> +#define __BUG_ENTRY_FILE(file)	RISCV_PTR " " file
>   #endif
>   
>   #ifdef CONFIG_DEBUG_BUGVERBOSE
> -#define __BUG_ENTRY			\
> +#define __BUG_ENTRY(file, line, flags)	\
>   	__BUG_ENTRY_ADDR "\n\t"		\
> -	__BUG_ENTRY_FILE "\n\t"		\
> -	RISCV_SHORT " %1\n\t"		\
> -	RISCV_SHORT " %2"
> +	__BUG_ENTRY_FILE(file) "\n\t"	\
> +	RISCV_SHORT " " line "\n\t"	\
> +	RISCV_SHORT " " flags
>   #else
> -#define __BUG_ENTRY			\
> -	__BUG_ENTRY_ADDR "\n\t"		\
> -	RISCV_SHORT " %2"
> +#define __BUG_ENTRY(file, line, flags)		\
> +	__BUG_ENTRY_ADDR "\n\t"			\
> +	RISCV_SHORT " " flags
>   #endif
>   
>   #ifdef CONFIG_GENERIC_BUG
> -#define __BUG_FLAGS(flags)					\
> -do {								\
> -	__asm__ __volatile__ (					\
> +
> +#define ARCH_WARN_ASM(file, line, flags, size)			\
>   		"1:\n\t"					\
>   			"ebreak\n"				\
>   			".pushsection __bug_table,\"aw\"\n\t"	\
>   		"2:\n\t"					\
> -			__BUG_ENTRY "\n\t"			\
> -			".org 2b + %3\n\t"                      \
> -			".popsection"				\
> +		__BUG_ENTRY(file, line, flags) "\n\t"		\
> +			".org 2b + " size "\n\t"                \
> +			".popsection\n"				\
> +
> +#define __BUG_FLAGS(flags)					\
> +do {								\
> +	__asm__ __volatile__ (					\
> +		ARCH_WARN_ASM("%0", "%1", "%2", "%3")		\
>   		:						\
>   		: "i" (__FILE__), "i" (__LINE__),		\
>   		  "i" (flags),					\
>   		  "i" (sizeof(struct bug_entry)));              \
>   } while (0)
> +
>   #else /* CONFIG_GENERIC_BUG */
>   #define __BUG_FLAGS(flags) do {					\
>   	__asm__ __volatile__ ("ebreak\n");			\
> @@ -78,6 +83,8 @@ do {								\
>   
>   #define __WARN_FLAGS(flags) __BUG_FLAGS(BUGFLAG_WARNING|(flags))
>   
> +#define ARCH_WARN_REACHABLE
> +
>   #define HAVE_ARCH_BUG
>   
>   #include <asm-generic/bug.h>


This looks good to me so:

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

Thanks,

Alex

Re: [PATCH v3 5/5] rust: Add warn_on and warn_on_once

[Alice Ryhl]

On Thu, Feb 13, 2025 at 3:01 PM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Add warn_on and warn_on_once macros. Wrapping the C's WARN_* and BUG_*
> macros doesn't work so this uses the assembly code exported by the C
> side via ARCH_WARN_ASM macro. Like the static branch code, this
> generates the assembly code for rust at compile time by using the C
> preprocessor.
>
> file()! macro doesn't work for the Rust inline assembly in the same
> way as __FILE__ for the C inline assembly. So the code to handle a
> file name is different from the C assembly code (similar to the
> arm64/loongarch assembly).
>
> Similarly, ARCH_WARN_REACHABLE is also used at compile time to
> generate the assembly code; objtool's reachable anotation code. Only
> architectures that use objtool (x86 and loongarch) need it.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/Makefile                                 |   8 ++
>  rust/kernel/.gitignore                        |   2 +
>  rust/kernel/bug.rs                            | 100 ++++++++++++++++++
>  rust/kernel/generated_arch_reachable_asm.rs.S |   7 ++
>  rust/kernel/generated_arch_warn_asm.rs.S      |   7 ++
>  rust/kernel/lib.rs                            |   1 +
>  6 files changed, 125 insertions(+)
>  create mode 100644 rust/kernel/bug.rs
>  create mode 100644 rust/kernel/generated_arch_reachable_asm.rs.S
>  create mode 100644 rust/kernel/generated_arch_warn_asm.rs.S
>
> diff --git a/rust/Makefile b/rust/Makefile
> index 8fcfd60447bc..a295b65c43f3 100644
> --- a/rust/Makefile
> +++ b/rust/Makefile
> @@ -34,6 +34,9 @@ obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated.o
>  obj-$(CONFIG_RUST_KERNEL_DOCTESTS) += doctests_kernel_generated_kunit.o
>
>  always-$(subst y,$(CONFIG_RUST),$(CONFIG_JUMP_LABEL)) += kernel/generated_arch_static_branch_asm.rs
> +ifndef CONFIG_UML
> +always-$(subst y,$(CONFIG_RUST),$(CONFIG_BUG)) += kernel/generated_arch_warn_asm.rs kernel/generated_arch_reachable_asm.rs
> +endif
>
>  # Avoids running `$(RUSTC)` when it may not be available.
>  ifdef CONFIG_RUST
> @@ -481,5 +484,10 @@ $(obj)/kernel.o: $(src)/kernel/lib.rs $(obj)/build_error.o \
>  ifdef CONFIG_JUMP_LABEL
>  $(obj)/kernel.o: $(obj)/kernel/generated_arch_static_branch_asm.rs
>  endif
> +ifndef CONFIG_UML
> +ifdef CONFIG_BUG
> +$(obj)/kernel.o: $(obj)/kernel/generated_arch_warn_asm.rs $(obj)/kernel/generated_arch_reachable_asm.rs
> +endif
> +endif
>
>  endif # CONFIG_RUST
> diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
> index 6ba39a178f30..f1d7f4225332 100644
> --- a/rust/kernel/.gitignore
> +++ b/rust/kernel/.gitignore
> @@ -1,3 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>
>  /generated_arch_static_branch_asm.rs
> +/generated_arch_warn_asm.rs
> +/generated_arch_reachable_asm.rs
> \ No newline at end of file
> diff --git a/rust/kernel/bug.rs b/rust/kernel/bug.rs
> new file mode 100644
> index 000000000000..7ffd9cb1ad75
> --- /dev/null
> +++ b/rust/kernel/bug.rs
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 FUJITA Tomonori
> +
> +//! Support for BUG_* and WARN_* functionality.
> +//!
> +//! C header: [`include/asm-generic/bug.h`](srctree/include/asm-generic/bug.h)
> +
> +#[macro_export]
> +#[doc(hidden)]
> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
> +macro_rules! warn_flags {
> +    ($flags:expr) => {
> +        const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
> +        // SAFETY: Just an FFI call.
> +        #[cfg(CONFIG_DEBUG_BUGVERBOSE)]
> +        unsafe {
> +            $crate::asm!(concat!(
> +                "/* {size} */",
> +                ".pushsection .rodata.str1.1, \"aMS\",@progbits, 1\n",
> +                "111:\t .string ", "\"", file!(), "\"\n",
> +                ".popsection\n",

It looks like you're doing this so that you can reference the filename
with "111b", but could you do this instead:

const _FILE: &[u8] = file!().as_bytes();
// Plus one for nul-terminator.
static FILE: [u8; 1 + _FILE.len()] = {
    let mut bytes = [0; 1 + _FILE.len()];
    let mut i = 0;
    while i < _FILE.len() {
        bytes[i] = _FILE[i];
        i += 1;
    }
    bytes
};

and then use

asm!(
    concat!(
        "/* {size} */",
        include!(concat!(env!("OBJTREE"),
"/rust/kernel/generated_arch_warn_asm.rs")),
        include!(concat!(env!("OBJTREE"),
"/rust/kernel/generated_arch_reachable_asm.rs")));
    file = sym FILE,
    line = const line!(),
    ...
);

with
::kernel::concat_literals!(ARCH_WARN_ASM("{file}", "{line}",
"{flags}", "{size}")),

That would be a lot simpler to understand than what you are doing.


Alice

Re: [PATCH v3 5/5] rust: Add warn_on and warn_on_once

[FUJITA Tomonori]

On Mon, 3 Mar 2025 14:33:39 +0100
Alice Ryhl <aliceryhl@google.com> wrote:

>> +#[macro_export]
>> +#[doc(hidden)]
>> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
>> +macro_rules! warn_flags {
>> +    ($flags:expr) => {
>> +        const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
>> +        // SAFETY: Just an FFI call.
>> +        #[cfg(CONFIG_DEBUG_BUGVERBOSE)]
>> +        unsafe {
>> +            $crate::asm!(concat!(
>> +                "/* {size} */",
>> +                ".pushsection .rodata.str1.1, \"aMS\",@progbits, 1\n",
>> +                "111:\t .string ", "\"", file!(), "\"\n",
>> +                ".popsection\n",
> 
> It looks like you're doing this so that you can reference the filename
> with "111b", but could you do this instead:
> 
> const _FILE: &[u8] = file!().as_bytes();
> // Plus one for nul-terminator.
> static FILE: [u8; 1 + _FILE.len()] = {
>     let mut bytes = [0; 1 + _FILE.len()];
>     let mut i = 0;
>     while i < _FILE.len() {
>         bytes[i] = _FILE[i];
>         i += 1;
>     }
>     bytes
> };

Neat! I will use this in the next version.

> and then use
> 
> asm!(
>     concat!(
>         "/* {size} */",
>         include!(concat!(env!("OBJTREE"),
> "/rust/kernel/generated_arch_warn_asm.rs")),
>         include!(concat!(env!("OBJTREE"),
> "/rust/kernel/generated_arch_reachable_asm.rs")));
>     file = sym FILE,
>     line = const line!(),
>     ...
> );
> 
> with
> ::kernel::concat_literals!(ARCH_WARN_ASM("{file}", "{line}",
> "{flags}", "{size}")),
> 
> That would be a lot simpler to understand than what you are doing.

Indeed.

Thanks a lot!

Re: [PATCH v3 5/5] rust: Add warn_on and warn_on_once

[Alice Ryhl]

On Thu, Feb 13, 2025 at 10:57:59PM +0900, FUJITA Tomonori wrote:
> Add warn_on and warn_on_once macros. Wrapping the C's WARN_* and BUG_*
> macros doesn't work so this uses the assembly code exported by the C
> side via ARCH_WARN_ASM macro. Like the static branch code, this
> generates the assembly code for rust at compile time by using the C
> preprocessor.
> 
> file()! macro doesn't work for the Rust inline assembly in the same
> way as __FILE__ for the C inline assembly. So the code to handle a
> file name is different from the C assembly code (similar to the
> arm64/loongarch assembly).

Nit: Should be file!() not file()!.

> diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
> index 6ba39a178f30..f1d7f4225332 100644
> --- a/rust/kernel/.gitignore
> +++ b/rust/kernel/.gitignore
> @@ -1,3 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  
>  /generated_arch_static_branch_asm.rs
> +/generated_arch_warn_asm.rs
> +/generated_arch_reachable_asm.rs
> \ No newline at end of file

There should be a newline.

> +++ b/rust/kernel/bug.rs
> @@ -0,0 +1,100 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2024 FUJITA Tomonori

2025?

> +#[macro_export]
> +#[doc(hidden)]
> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
> +macro_rules! warn_flags {
> +    ($flags:expr) => {
> +        const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
> +        // SAFETY: Just an FFI call.
> +        #[cfg(CONFIG_DEBUG_BUGVERBOSE)]
> +        unsafe {
> +            $crate::asm!(concat!(
> +                "/* {size} */",
> +                ".pushsection .rodata.str1.1, \"aMS\",@progbits, 1\n",
> +                "111:\t .string ", "\"", file!(), "\"\n",
> +                ".popsection\n",
> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
> +            line = const line!(),
> +            flags = const FLAGS,
> +            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
> +            );
> +        }
> +        // SAFETY: Just an FFI call.
> +        #[cfg(not(CONFIG_DEBUG_BUGVERBOSE))]
> +        unsafe {
> +            $crate::asm!(
> +            concat!(
> +                "/* {size} */",
> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
> +            flags = const FLAGS,
> +            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
> +            );
> +        }

I generally prefer to have the cfgs on the macro rather in its
expansion. That avoids emitting a lot of code that is not actually used.

> +#[doc(hidden)]
> +#[macro_export]
> +macro_rules! bugflag_taint {
> +    ($taint:expr) => {
> +        $taint << 8
> +    };
> +}

This could just be a const fn.

> +/// Report a warning only once.
> +#[macro_export]
> +macro_rules! warn_on_once {
> +    ($cond:expr) => {
> +        if $cond {
> +            $crate::warn_flags!(
> +                $crate::bindings::BUGFLAG_ONCE
> +                    | $crate::bugflag_taint!($crate::bindings::TAINT_WARN)

Or maybe a constant?

const WARN_ON_ONCE_FLAGS: u32 = bindings::BUGFLAG_ONCE | (bindings::TAINT_WARN << 8);

$crate::warn_flags!($crate::bug::WARN_ON_ONCE_FLAGS);

Alice

Re: [PATCH v3 5/5] rust: Add warn_on and warn_on_once

[FUJITA Tomonori]

On Wed, 5 Mar 2025 08:42:57 +0000
Alice Ryhl <aliceryhl@google.com> wrote:

> On Thu, Feb 13, 2025 at 10:57:59PM +0900, FUJITA Tomonori wrote:
>> Add warn_on and warn_on_once macros. Wrapping the C's WARN_* and BUG_*
>> macros doesn't work so this uses the assembly code exported by the C
>> side via ARCH_WARN_ASM macro. Like the static branch code, this
>> generates the assembly code for rust at compile time by using the C
>> preprocessor.
>> 
>> file()! macro doesn't work for the Rust inline assembly in the same
>> way as __FILE__ for the C inline assembly. So the code to handle a
>> file name is different from the C assembly code (similar to the
>> arm64/loongarch assembly).
> 
> Nit: Should be file!() not file()!.

Ops, thanks.

Actually, the above comment is obsolete. With your solution in the
previous mail, I can remove the asm code for the file name. I'll
remove the comment.


>> diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
>> index 6ba39a178f30..f1d7f4225332 100644
>> --- a/rust/kernel/.gitignore
>> +++ b/rust/kernel/.gitignore
>> @@ -1,3 +1,5 @@
>>  # SPDX-License-Identifier: GPL-2.0
>>  
>>  /generated_arch_static_branch_asm.rs
>> +/generated_arch_warn_asm.rs
>> +/generated_arch_reachable_asm.rs
>> \ No newline at end of file
> 
> There should be a newline.

Ah, I'll fix.

>> +++ b/rust/kernel/bug.rs
>> @@ -0,0 +1,100 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +// Copyright (C) 2024 FUJITA Tomonori
> 
> 2025?

I'll add.

>> +#[macro_export]
>> +#[doc(hidden)]
>> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
>> +macro_rules! warn_flags {
>> +    ($flags:expr) => {
>> +        const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
>> +        // SAFETY: Just an FFI call.
>> +        #[cfg(CONFIG_DEBUG_BUGVERBOSE)]
>> +        unsafe {
>> +            $crate::asm!(concat!(
>> +                "/* {size} */",
>> +                ".pushsection .rodata.str1.1, \"aMS\",@progbits, 1\n",
>> +                "111:\t .string ", "\"", file!(), "\"\n",
>> +                ".popsection\n",
>> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
>> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
>> +            line = const line!(),
>> +            flags = const FLAGS,
>> +            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
>> +            );
>> +        }
>> +        // SAFETY: Just an FFI call.
>> +        #[cfg(not(CONFIG_DEBUG_BUGVERBOSE))]
>> +        unsafe {
>> +            $crate::asm!(
>> +            concat!(
>> +                "/* {size} */",
>> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
>> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
>> +            flags = const FLAGS,
>> +            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
>> +            );
>> +        }
> 
> I generally prefer to have the cfgs on the macro rather in its
> expansion. That avoids emitting a lot of code that is not actually used.

You prefer the following?

#[cfg(all(CONFIG_BUG, CONFIG_DEBUG_BUGVERBOSE, not(CONFIG_UML)))]
macro_rules! warn_flags {
...
}

#[cfg(all(CONFIG_BUG, not(CONFIG_DEBUG_BUGVERBOSE), not(CONFIG_UML)))]
macro_rules! warn_flags {
...
}

>> +#[doc(hidden)]
>> +#[macro_export]
>> +macro_rules! bugflag_taint {
>> +    ($taint:expr) => {
>> +        $taint << 8
>> +    };
>> +}
> 
> This could just be a const fn.

Yeah, would a const fn be preferable?

>> +/// Report a warning only once.
>> +#[macro_export]
>> +macro_rules! warn_on_once {
>> +    ($cond:expr) => {
>> +        if $cond {
>> +            $crate::warn_flags!(
>> +                $crate::bindings::BUGFLAG_ONCE
>> +                    | $crate::bugflag_taint!($crate::bindings::TAINT_WARN)
> 
> Or maybe a constant?
> 
> const WARN_ON_ONCE_FLAGS: u32 = bindings::BUGFLAG_ONCE | (bindings::TAINT_WARN << 8);

Ok, but you prefer "<< 8" than using const fn bugflag_taint()?

> $crate::warn_flags!($crate::bug::WARN_ON_ONCE_FLAGS);

Re: [PATCH v3 5/5] rust: Add warn_on and warn_on_once

[Alice Ryhl]

On Wed, Mar 05, 2025 at 07:24:03PM +0900, FUJITA Tomonori wrote:
> On Wed, 5 Mar 2025 08:42:57 +0000
> Alice Ryhl <aliceryhl@google.com> wrote:
> 
> > On Thu, Feb 13, 2025 at 10:57:59PM +0900, FUJITA Tomonori wrote:
> >> Add warn_on and warn_on_once macros. Wrapping the C's WARN_* and BUG_*
> >> macros doesn't work so this uses the assembly code exported by the C
> >> side via ARCH_WARN_ASM macro. Like the static branch code, this
> >> generates the assembly code for rust at compile time by using the C
> >> preprocessor.
> >> 
> >> file()! macro doesn't work for the Rust inline assembly in the same
> >> way as __FILE__ for the C inline assembly. So the code to handle a
> >> file name is different from the C assembly code (similar to the
> >> arm64/loongarch assembly).
> > 
> > Nit: Should be file!() not file()!.
> 
> Ops, thanks.
> 
> Actually, the above comment is obsolete. With your solution in the
> previous mail, I can remove the asm code for the file name. I'll
> remove the comment.
> 
> 
> >> diff --git a/rust/kernel/.gitignore b/rust/kernel/.gitignore
> >> index 6ba39a178f30..f1d7f4225332 100644
> >> --- a/rust/kernel/.gitignore
> >> +++ b/rust/kernel/.gitignore
> >> @@ -1,3 +1,5 @@
> >>  # SPDX-License-Identifier: GPL-2.0
> >>  
> >>  /generated_arch_static_branch_asm.rs
> >> +/generated_arch_warn_asm.rs
> >> +/generated_arch_reachable_asm.rs
> >> \ No newline at end of file
> > 
> > There should be a newline.
> 
> Ah, I'll fix.
> 
> >> +++ b/rust/kernel/bug.rs
> >> @@ -0,0 +1,100 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +
> >> +// Copyright (C) 2024 FUJITA Tomonori
> > 
> > 2025?
> 
> I'll add.
> 
> >> +#[macro_export]
> >> +#[doc(hidden)]
> >> +#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
> >> +macro_rules! warn_flags {
> >> +    ($flags:expr) => {
> >> +        const FLAGS: u32 = $crate::bindings::BUGFLAG_WARNING | $flags;
> >> +        // SAFETY: Just an FFI call.
> >> +        #[cfg(CONFIG_DEBUG_BUGVERBOSE)]
> >> +        unsafe {
> >> +            $crate::asm!(concat!(
> >> +                "/* {size} */",
> >> +                ".pushsection .rodata.str1.1, \"aMS\",@progbits, 1\n",
> >> +                "111:\t .string ", "\"", file!(), "\"\n",
> >> +                ".popsection\n",
> >> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
> >> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
> >> +            line = const line!(),
> >> +            flags = const FLAGS,
> >> +            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
> >> +            );
> >> +        }
> >> +        // SAFETY: Just an FFI call.
> >> +        #[cfg(not(CONFIG_DEBUG_BUGVERBOSE))]
> >> +        unsafe {
> >> +            $crate::asm!(
> >> +            concat!(
> >> +                "/* {size} */",
> >> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_warn_asm.rs")),
> >> +                include!(concat!(env!("OBJTREE"), "/rust/kernel/generated_arch_reachable_asm.rs")));
> >> +            flags = const FLAGS,
> >> +            size = const ::core::mem::size_of::<$crate::bindings::bug_entry>(),
> >> +            );
> >> +        }
> > 
> > I generally prefer to have the cfgs on the macro rather in its
> > expansion. That avoids emitting a lot of code that is not actually used.
> 
> You prefer the following?
> 
> #[cfg(all(CONFIG_BUG, CONFIG_DEBUG_BUGVERBOSE, not(CONFIG_UML)))]
> macro_rules! warn_flags {
> ...
> }
> 
> #[cfg(all(CONFIG_BUG, not(CONFIG_DEBUG_BUGVERBOSE), not(CONFIG_UML)))]
> macro_rules! warn_flags {
> ...
> }

In this case it probably reads better as

#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
#[cfg(CONFIG_DEBUG_BUGVERBOSE)]
macro_rules! warn_flags {
...
}

#[cfg(all(CONFIG_BUG, not(CONFIG_UML)))]
#[cfg(not(CONFIG_DEBUG_BUGVERBOSE))]
macro_rules! warn_flags {
...
}

but yes.

> >> +#[doc(hidden)]
> >> +#[macro_export]
> >> +macro_rules! bugflag_taint {
> >> +    ($taint:expr) => {
> >> +        $taint << 8
> >> +    };
> >> +}
> > 
> > This could just be a const fn.
> 
> Yeah, would a const fn be preferable?

Yes, I think a constant or const fn is preferable over a macro whenever
possible.

> >> +/// Report a warning only once.
> >> +#[macro_export]
> >> +macro_rules! warn_on_once {
> >> +    ($cond:expr) => {
> >> +        if $cond {
> >> +            $crate::warn_flags!(
> >> +                $crate::bindings::BUGFLAG_ONCE
> >> +                    | $crate::bugflag_taint!($crate::bindings::TAINT_WARN)
> > 
> > Or maybe a constant?
> > 
> > const WARN_ON_ONCE_FLAGS: u32 = bindings::BUGFLAG_ONCE | (bindings::TAINT_WARN << 8);
> 
> Ok, but you prefer "<< 8" than using const fn bugflag_taint()?

I'm also happy with
const WARN_ON_ONCE_FLAGS: u32 = bindings::BUGFLAG_ONCE | bugflag_taint(bindings::TAINT_WARN);

Up to you.

Alice