* [PATCH v2 0/4] introduce generic implementation of macros from bug.h
@ 2023-02-20 16:40 Oleksii Kurochko
2023-02-20 16:40 ` [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Oleksii Kurochko @ 2023-02-20 16:40 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
Oleksii Kurochko, George Dunlap, Julien Grall, Wei Liu,
Bertrand Marquis, Volodymyr Babchuk, Roger Pau Monné
A large part of the content of the bug.h is repeated among all
architectures (especially x86 and RISCV have the same implementation), so it
was created a new config CONFIG_GENERIC_BUG_FRAME which is used to
introduce generic implementation of do_bug_frame() and move x86's <asm/bug.h>
to <xen/common/...> with the following changes:
* Add inclusion of arch-specific header <asm/bug.h>
* Rename the guard and remove x86 specific changes
* Wrap macros BUG_FRAME/run_in_exception_handler/WARN/BUG/assert_failed/etc
into #ifndef "BUG_FRAME/run_in_exception_handler/WARN/BUG/assert_failed/etc"
thereby each architecture can override the generic implementation of macros.
* Add #if{n}def __ASSEMBLY__ ... #endif
* Introduce BUG_FRAME_STRUCTURE define to be able to change the structure of bug
frame
* Introduce BUG_INSTR and MODIFIER to make _ASM_BUGFRAME_TEXT reusable between x86 and
RISC-V.
* Make macros related to bug frame structure more generic.
For ARM only common parts were removed from <asm/bug.h> and re-use generic
do_bug_frame() for ARM.
RISC-V will be switched to use <xen/bug.h> and in the future, it will use common
the version of do_bug_frame() when xen/common will work for RISC-V.
---
Changes in V2:
* Update cover letter.
* Switch to x86 implementation as generic as it is more compact ( at least from the point of view of bug frame structure).
* Put [PATCH v1 4/4] xen: change <asm/bug.h> to <xen/bug.h> as second patch,
update the patch to change all <asm/bug.h> to <xen/bug.h> among the whole project
to not break compilation.
* Rename CONFIG_GENERIC_DO_BUG_FRAME to CONFIG_GENERIC_BUG_FRAME.
* Change the macro bug_loc(b) to avoid the need for a cast:
#define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp)
* Rename BUG_FRAME_STUFF to BUG_FRAME_STRUCT
* Make macros related to bug frame structure more generic.
* Rename bug_file() in ARM implementation to bug_ptr() as generic do_bug_frame() uses
bug_ptr().
* Introduce BUG_INSTR and MODIFIER to make _ASM_BUGFRAME_TEXT reusable between x86 and
RISC-V.
* Rework do_invalid_op() in x86 ( re-use handle_bug_frame() and find_bug_frame() )
---
Oleksii Kurochko (4):
xen: introduce CONFIG_GENERIC_BUG_FRAME
xen: change <asm/bug.h> to <xen/bug.h>
xen/arm: switch ARM to use generic implementation of bug.h
xen/x86: switch x86 to use generic implemetation of bug.h
xen/arch/arm/Kconfig | 1 +
xen/arch/arm/include/asm/bug.h | 32 +-----
xen/arch/arm/include/asm/div64.h | 2 +-
xen/arch/arm/include/asm/traps.h | 2 -
xen/arch/arm/traps.c | 79 -------------
xen/arch/arm/vgic/vgic-v2.c | 2 +-
xen/arch/arm/vgic/vgic.c | 2 +-
xen/arch/x86/Kconfig | 1 +
xen/arch/x86/acpi/cpufreq/cpufreq.c | 2 +-
xen/arch/x86/include/asm/asm_defns.h | 2 +-
xen/arch/x86/include/asm/bug.h | 88 +--------------
xen/arch/x86/traps.c | 81 +++-----------
xen/common/Kconfig | 3 +
xen/common/Makefile | 1 +
xen/common/bug.c | 113 +++++++++++++++++++
xen/drivers/cpufreq/cpufreq.c | 2 +-
xen/include/xen/bug.h | 161 +++++++++++++++++++++++++++
xen/include/xen/lib.h | 2 +-
18 files changed, 306 insertions(+), 270 deletions(-)
create mode 100644 xen/common/bug.c
create mode 100644 xen/include/xen/bug.h
--
2.39.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-20 16:40 [PATCH v2 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
@ 2023-02-20 16:40 ` Oleksii Kurochko
2023-02-22 12:46 ` Jan Beulich
2023-02-23 13:32 ` Jan Beulich
2023-02-20 16:40 ` [PATCH v2 2/4] xen: change <asm/bug.h> to <xen/bug.h> Oleksii Kurochko
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Oleksii Kurochko @ 2023-02-20 16:40 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
Oleksii Kurochko, George Dunlap, Julien Grall, Wei Liu
A large part of the content of the bug.h is repeated among all
architectures, so it was decided to create a new config
CONFIG_GENERIC_BUG_FRAME.
The version of <bug.h> from x86 was taken as the base version.
The patch introduces the following stuff:
* common bug.h header
* generic implementation of do_bug_frame()
* new config CONFIG_GENERIC_BUG_FRAME
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
- Switch to x86 implementation as generic as it is more compact
( at least from the point of view of bug frame structure ).
- Rename CONFIG_GENERIC_DO_BUG_FRAME to CONFIG_GENERIC_BUG_FRAME.
- Change the macro bug_loc(b) to avoid the need for a cast:
#define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp)
- Rename BUG_FRAME_STUFF to BUG_FRAME_STRUCT
- Make macros related to bug frame structure more generic.
- Introduce BUG_INSTR and MODIFIER to make _ASM_BUGFRAME_TEXT reusable
between x86 and RISC-V.
- Rework do_bug_frame() and introduce find_bug_frame() and handle_bug_frame()
functions to make it reusable by x86.
- code style fixes
---
xen/common/Kconfig | 3 +
xen/common/Makefile | 1 +
xen/common/bug.c | 113 +++++++++++++++++++++++++++++
xen/include/xen/bug.h | 161 ++++++++++++++++++++++++++++++++++++++++++
4 files changed, 278 insertions(+)
create mode 100644 xen/common/bug.c
create mode 100644 xen/include/xen/bug.h
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index f1ea3199c8..b226323537 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -28,6 +28,9 @@ config ALTERNATIVE_CALL
config ARCH_MAP_DOMAIN_PAGE
bool
+config GENERIC_BUG_FRAME
+ bool
+
config HAS_ALTERNATIVE
bool
diff --git a/xen/common/Makefile b/xen/common/Makefile
index bbd75b4be6..46049eac35 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,5 +1,6 @@
obj-$(CONFIG_ARGO) += argo.o
obj-y += bitmap.o
+obj-$(CONFIG_GENERIC_BUG_FRAME) += bug.o
obj-$(CONFIG_HYPFS_CONFIG) += config_data.o
obj-$(CONFIG_CORE_PARKING) += core_parking.o
obj-y += cpu.o
diff --git a/xen/common/bug.c b/xen/common/bug.c
new file mode 100644
index 0000000000..5eab3ebb53
--- /dev/null
+++ b/xen/common/bug.c
@@ -0,0 +1,113 @@
+#include <xen/bug.h>
+#include <xen/errno.h>
+#include <xen/kernel.h>
+#include <xen/livepatch.h>
+#include <xen/string.h>
+#include <xen/types.h>
+#include <xen/virtual_region.h>
+
+#include <asm/processor.h>
+
+const struct bug_frame* find_bug_frame(unsigned long pc, unsigned int *id)
+{
+ const struct bug_frame *bug = NULL;
+ const struct virtual_region *region;
+
+ region = find_text_region(pc);
+ if ( region )
+ {
+ for ( *id = 0; *id < BUGFRAME_NR; (*id)++ )
+ {
+ const struct bug_frame *b;
+ unsigned int i;
+
+ for ( i = 0, b = region->frame[*id].bugs;
+ i < region->frame[*id].n_bugs; b++, i++ )
+ {
+ if ( bug_loc(b) == pc )
+ {
+ bug = b;
+ goto found;
+ }
+ }
+ }
+ }
+
+ found:
+ return bug;
+}
+
+int handle_bug_frame(const struct cpu_user_regs *regs,
+ const struct bug_frame *bug,
+ unsigned int id)
+{
+ const char *prefix = "", *filename, *predicate;
+ unsigned long fixup;
+ unsigned int lineno;
+
+ if ( id == BUGFRAME_run_fn )
+ {
+#ifdef ARM
+ void (*fn)(const struct cpu_user_regs *) = (void *)regs->BUG_FN_REG;
+#else
+ void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug);
+#endif
+
+ fn(regs);
+ return 0;
+ }
+
+ /* WARN, BUG or ASSERT: decode the filename pointer and line number. */
+ filename = bug_ptr(bug);
+ if ( !is_kernel(filename) && !is_patch(filename) )
+ return -EINVAL;
+ fixup = strlen(filename);
+ if ( fixup > 50 )
+ {
+ filename += fixup - 47;
+ prefix = "...";
+ }
+ lineno = bug_line(bug);
+
+ switch ( id )
+ {
+ case BUGFRAME_warn:
+ printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno);
+ show_execution_state(regs);
+ return 0;
+
+ case BUGFRAME_bug:
+ printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
+
+ show_execution_state(regs);
+ panic("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
+
+ case BUGFRAME_assert:
+ /* ASSERT: decode the predicate string pointer. */
+ predicate = bug_msg(bug);
+ if ( !is_kernel(predicate) )
+ predicate = "<unknown>";
+
+ printk("Assertion '%s' failed at %s%s:%d\n",
+ predicate, prefix, filename, lineno);
+
+ show_execution_state(regs);
+ panic("Assertion '%s' failed at %s%s:%d\n",
+ predicate, prefix, filename, lineno);
+ }
+
+ return -EINVAL;
+}
+
+int do_bug_frame(const struct cpu_user_regs *regs, unsigned long pc)
+{
+ const struct bug_frame *bug = NULL;
+ unsigned int id;
+
+ bug = find_bug_frame(pc, &id);
+ if (!bug)
+ return -ENOENT;
+
+ return handle_bug_frame(regs, bug, id);
+}
+
diff --git a/xen/include/xen/bug.h b/xen/include/xen/bug.h
new file mode 100644
index 0000000000..4aedebeb18
--- /dev/null
+++ b/xen/include/xen/bug.h
@@ -0,0 +1,161 @@
+#ifndef __XEN_BUG_H__
+#define __XEN_BUG_H__
+
+#define BUG_DISP_WIDTH 24
+#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
+#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
+
+#define BUGFRAME_run_fn 0
+#define BUGFRAME_warn 1
+#define BUGFRAME_bug 2
+#define BUGFRAME_assert 3
+
+#define BUGFRAME_NR 4
+
+#ifndef __ASSEMBLY__
+
+#include <xen/errno.h>
+#include <xen/lib.h>
+#include <xen/stringify.h>
+#include <xen/types.h>
+
+#include <asm/bug.h>
+
+#ifndef BUG_FRAME_STRUCT
+
+struct bug_frame {
+ signed int loc_disp:BUG_DISP_WIDTH;
+ unsigned int line_hi:BUG_LINE_HI_WIDTH;
+ signed int ptr_disp:BUG_DISP_WIDTH;
+ unsigned int line_lo:BUG_LINE_LO_WIDTH;
+ signed int msg_disp[];
+};
+
+#endif /* BUG_FRAME_STRUCT */
+
+#ifndef bug_loc
+#define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp)
+#endif /* bug_loc */
+
+#ifndef bug_ptr
+#define bug_ptr(b) ((const void *)(b) + (b)->ptr_disp)
+#endif /* bug_ptr */
+
+#ifndef bug_line
+#define bug_line(b) (((((b)->line_hi + ((b)->loc_disp < 0)) & \
+ ((1 << BUG_LINE_HI_WIDTH) - 1)) << \
+ BUG_LINE_LO_WIDTH) + \
+ (((b)->line_lo + ((b)->ptr_disp < 0)) & \
+ ((1 << BUG_LINE_LO_WIDTH) - 1)))
+#endif /* bug_line */
+
+#ifndef bug_msg
+#define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
+#endif /* bug_msg */
+
+#ifndef _ASM_BUGFRAME_TEXT
+
+#define _ASM_BUGFRAME_TEXT(second_frame) \
+ ".Lbug%=:"BUG_INSTR"\n" \
+ ".pushsection .bug_frames.%"MODIFIER"[bf_type], \"a\", @progbits\n" \
+ ".p2align 2\n" \
+ ".Lfrm%=:\n" \
+ ".long (.Lbug%= - .Lfrm%=) + %"MODIFIER"[bf_line_hi]\n" \
+ ".long (%"MODIFIER"[bf_ptr] - .Lfrm%=) + %"MODIFIER"[bf_line_lo]\n" \
+ ".if " #second_frame "\n" \
+ ".long 0, %"MODIFIER"[bf_msg] - .Lfrm%=\n" \
+ ".endif\n" \
+ ".popsection\n"
+
+#endif /* _ASM_BUGFRAME_TEXT */
+
+#ifndef _ASM_BUGFRAME_INFO
+
+#define _ASM_BUGFRAME_INFO(type, line, ptr, msg) \
+ [bf_type] "i" (type), \
+ [bf_ptr] "i" (ptr), \
+ [bf_msg] "i" (msg), \
+ [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1)) \
+ << BUG_DISP_WIDTH), \
+ [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
+
+#endif /* _ASM_BUGFRAME_INFO */
+
+#ifndef BUG_FRAME
+
+#define BUG_FRAME(type, line, ptr, second_frame, msg) do { \
+ BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH)); \
+ BUILD_BUG_ON((type) >= BUGFRAME_NR); \
+ asm volatile ( _ASM_BUGFRAME_TEXT(second_frame) \
+ :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) ); \
+} while (0)
+
+#endif
+
+#ifndef run_in_exception_handler
+
+/*
+ * TODO: untangle header dependences, break BUILD_BUG_ON() out of xen/lib.h,
+ * and use a real static inline here to get proper type checking of fn().
+ */
+#define run_in_exception_handler(fn) \
+ do { \
+ (void)((fn) == (void (*)(struct cpu_user_regs *))NULL); \
+ BUG_FRAME(BUGFRAME_run_fn, 0, fn, 0, NULL); \
+ } while ( 0 )
+
+#endif /* run_in_exception_handler */
+
+#ifndef WARN
+#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, NULL)
+#endif /* WARN */
+
+#ifndef BUG
+#define BUG() do { \
+ BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, 0, NULL); \
+ unreachable(); \
+} while (0)
+#endif
+
+#ifndef assert_failed
+#define assert_failed(msg) do { \
+ BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg); \
+ unreachable(); \
+} while (0)
+#endif
+
+#ifdef CONFIG_GENERIC_BUG_FRAME
+
+struct cpu_user_regs;
+
+const struct bug_frame* find_bug_frame(unsigned long pc, unsigned int *id);
+
+int handle_bug_frame(const struct cpu_user_regs *regs,
+ const struct bug_frame *bug,
+ unsigned int id);
+
+int do_bug_frame(const struct cpu_user_regs *regs, unsigned long pc);
+
+#endif /* CONFIG_GENERIC_BUG_FRAME */
+
+extern const struct bug_frame __start_bug_frames[],
+ __stop_bug_frames_0[],
+ __stop_bug_frames_1[],
+ __stop_bug_frames_2[],
+ __stop_bug_frames_3[];
+
+#else /* !__ASSEMBLY__ */
+
+#include <asm/bug.h>
+
+#endif /* __ASSEMBLY__ */
+
+#endif /* __XEN_BUG_H__ */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
--
2.39.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 2/4] xen: change <asm/bug.h> to <xen/bug.h>
2023-02-20 16:40 [PATCH v2 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
2023-02-20 16:40 ` [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
@ 2023-02-20 16:40 ` Oleksii Kurochko
2023-02-23 13:34 ` Jan Beulich
2023-02-20 16:40 ` [PATCH v2 3/4] xen/arm: switch ARM to use generic implementation of bug.h Oleksii Kurochko
2023-02-20 16:41 ` [PATCH v2 4/4] xen/x86: switch x86 to use generic implemetation " Oleksii Kurochko
3 siblings, 1 reply; 15+ messages in thread
From: Oleksii Kurochko @ 2023-02-20 16:40 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
Oleksii Kurochko, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk, George Dunlap, Wei Liu, Roger Pau Monné
Since the generic version of bug.h stuff was introduced use <xen/bug.h>
instead of unnecessary <asm/bug.h>
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
* Put [PATCH v1 4/4] xen: change <asm/bug.h> to <xen/bug.h> as second patch,
update the patch to change all <asm/bug.h> to <xen/bug.h> among the whole project
to not break build.
* Update the commit message.
---
xen/arch/arm/include/asm/div64.h | 2 +-
xen/arch/arm/vgic/vgic-v2.c | 2 +-
xen/arch/arm/vgic/vgic.c | 2 +-
xen/arch/x86/acpi/cpufreq/cpufreq.c | 2 +-
xen/arch/x86/include/asm/asm_defns.h | 2 +-
xen/drivers/cpufreq/cpufreq.c | 2 +-
xen/include/xen/lib.h | 2 +-
7 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/xen/arch/arm/include/asm/div64.h b/xen/arch/arm/include/asm/div64.h
index 1cd58bc51a..fc667a80f9 100644
--- a/xen/arch/arm/include/asm/div64.h
+++ b/xen/arch/arm/include/asm/div64.h
@@ -74,7 +74,7 @@
#elif __GNUC__ >= 4
-#include <asm/bug.h>
+#include <xen/bug.h>
/*
* If the divisor happens to be constant, we determine the appropriate
diff --git a/xen/arch/arm/vgic/vgic-v2.c b/xen/arch/arm/vgic/vgic-v2.c
index 1a99d3a8b4..c90e88fddb 100644
--- a/xen/arch/arm/vgic/vgic-v2.c
+++ b/xen/arch/arm/vgic/vgic-v2.c
@@ -16,8 +16,8 @@
*/
#include <asm/new_vgic.h>
-#include <asm/bug.h>
#include <asm/gic.h>
+#include <xen/bug.h>
#include <xen/sched.h>
#include <xen/sizes.h>
diff --git a/xen/arch/arm/vgic/vgic.c b/xen/arch/arm/vgic/vgic.c
index f0f2ea5021..b9463a5f27 100644
--- a/xen/arch/arm/vgic/vgic.c
+++ b/xen/arch/arm/vgic/vgic.c
@@ -15,9 +15,9 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
+#include <xen/bug.h>
#include <xen/list_sort.h>
#include <xen/sched.h>
-#include <asm/bug.h>
#include <asm/event.h>
#include <asm/new_vgic.h>
diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c
index c27cbb2304..18ff2a443b 100644
--- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
+++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
@@ -27,6 +27,7 @@
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/
+#include <xen/bug.h>
#include <xen/types.h>
#include <xen/errno.h>
#include <xen/delay.h>
@@ -35,7 +36,6 @@
#include <xen/sched.h>
#include <xen/timer.h>
#include <xen/xmalloc.h>
-#include <asm/bug.h>
#include <asm/msr.h>
#include <asm/io.h>
#include <asm/processor.h>
diff --git a/xen/arch/x86/include/asm/asm_defns.h b/xen/arch/x86/include/asm/asm_defns.h
index d9431180cf..a8526cf36c 100644
--- a/xen/arch/x86/include/asm/asm_defns.h
+++ b/xen/arch/x86/include/asm/asm_defns.h
@@ -6,7 +6,7 @@
/* NB. Auto-generated from arch/.../asm-offsets.c */
#include <asm/asm-offsets.h>
#endif
-#include <asm/bug.h>
+#include <xen/bug.h>
#include <asm/x86-defns.h>
#include <xen/stringify.h>
#include <asm/cpufeature.h>
diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index a94520ee57..354f78580b 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -26,6 +26,7 @@
* ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
*/
+#include <xen/bug.h>
#include <xen/types.h>
#include <xen/errno.h>
#include <xen/delay.h>
@@ -39,7 +40,6 @@
#include <xen/guest_access.h>
#include <xen/domain.h>
#include <xen/cpu.h>
-#include <asm/bug.h>
#include <asm/io.h>
#include <asm/processor.h>
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 05ee1e18af..e914ccade0 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -24,12 +24,12 @@
#ifndef __ASSEMBLY__
+#include <xen/bug.h>
#include <xen/inttypes.h>
#include <xen/stdarg.h>
#include <xen/types.h>
#include <xen/xmalloc.h>
#include <xen/string.h>
-#include <asm/bug.h>
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
#define WARN_ON(p) ({ \
--
2.39.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 3/4] xen/arm: switch ARM to use generic implementation of bug.h
2023-02-20 16:40 [PATCH v2 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
2023-02-20 16:40 ` [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
2023-02-20 16:40 ` [PATCH v2 2/4] xen: change <asm/bug.h> to <xen/bug.h> Oleksii Kurochko
@ 2023-02-20 16:40 ` Oleksii Kurochko
2023-02-20 16:41 ` [PATCH v2 4/4] xen/x86: switch x86 to use generic implemetation " Oleksii Kurochko
3 siblings, 0 replies; 15+ messages in thread
From: Oleksii Kurochko @ 2023-02-20 16:40 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
Oleksii Kurochko, Julien Grall, Bertrand Marquis,
Volodymyr Babchuk
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
* Rename bug_file() in ARM implementation to bug_ptr() as
generic do_bug_frame() uses bug_ptr().
* Remove generic parts from bug.h
* Remove declaration of 'int do_bug_frame(...)'
from <asm/traps.h> as it was introduced in <xen/bug.h>
---
xen/arch/arm/Kconfig | 1 +
xen/arch/arm/include/asm/bug.h | 32 +------------
xen/arch/arm/include/asm/traps.h | 2 -
xen/arch/arm/traps.c | 79 --------------------------------
4 files changed, 3 insertions(+), 111 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 239d3aed3c..aad6644a7b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -12,6 +12,7 @@ config ARM_64
config ARM
def_bool y
+ select GENERIC_BUG_FRAME
select HAS_ALTERNATIVE
select HAS_DEVICE_TREE
select HAS_PASSTHROUGH
diff --git a/xen/arch/arm/include/asm/bug.h b/xen/arch/arm/include/asm/bug.h
index f4088d0913..174cee477d 100644
--- a/xen/arch/arm/include/asm/bug.h
+++ b/xen/arch/arm/include/asm/bug.h
@@ -9,9 +9,7 @@
# error "unknown ARM variant"
#endif
-#define BUG_DISP_WIDTH 24
-#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
-#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
+#define BUG_FRAME_STRUCT
struct bug_frame {
signed int loc_disp; /* Relative address to the bug address */
@@ -21,18 +19,10 @@ struct bug_frame {
uint32_t pad0:16; /* Padding for 8-bytes align */
};
-#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
-#define bug_file(b) ((const void *)(b) + (b)->file_disp);
+#define bug_ptr(b) ((const void *)(b) + (b)->file_disp)
#define bug_line(b) ((b)->line)
#define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
-#define BUGFRAME_run_fn 0
-#define BUGFRAME_warn 1
-#define BUGFRAME_bug 2
-#define BUGFRAME_assert 3
-
-#define BUGFRAME_NR 4
-
/* Many versions of GCC doesn't support the asm %c parameter which would
* be preferable to this unpleasantness. We use mergeable string
* sections to avoid multiple copies of the string appearing in the
@@ -77,24 +67,6 @@ struct bug_frame {
".popsection" :: "r" (fn) : __stringify(BUG_FN_REG) ); \
} while (0)
-#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "")
-
-#define BUG() do { \
- BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, 0, ""); \
- unreachable(); \
-} while (0)
-
-#define assert_failed(msg) do { \
- BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg); \
- unreachable(); \
-} while (0)
-
-extern const struct bug_frame __start_bug_frames[],
- __stop_bug_frames_0[],
- __stop_bug_frames_1[],
- __stop_bug_frames_2[],
- __stop_bug_frames_3[];
-
#endif /* __ARM_BUG_H__ */
/*
* Local variables:
diff --git a/xen/arch/arm/include/asm/traps.h b/xen/arch/arm/include/asm/traps.h
index 883dae368e..c6518008ec 100644
--- a/xen/arch/arm/include/asm/traps.h
+++ b/xen/arch/arm/include/asm/traps.h
@@ -69,8 +69,6 @@ void do_cp(struct cpu_user_regs *regs, const union hsr hsr);
void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr);
void do_trap_hvc_smccc(struct cpu_user_regs *regs);
-int do_bug_frame(const struct cpu_user_regs *regs, vaddr_t pc);
-
void noreturn do_unexpected_trap(const char *msg,
const struct cpu_user_regs *regs);
void do_trap_hyp_sync(struct cpu_user_regs *regs);
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 061c92acbd..751c3277aa 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1197,85 +1197,6 @@ void do_unexpected_trap(const char *msg, const struct cpu_user_regs *regs)
panic("CPU%d: Unexpected Trap: %s\n", smp_processor_id(), msg);
}
-int do_bug_frame(const struct cpu_user_regs *regs, vaddr_t pc)
-{
- const struct bug_frame *bug = NULL;
- const char *prefix = "", *filename, *predicate;
- unsigned long fixup;
- int id = -1, lineno;
- const struct virtual_region *region;
-
- region = find_text_region(pc);
- if ( region )
- {
- for ( id = 0; id < BUGFRAME_NR; id++ )
- {
- const struct bug_frame *b;
- unsigned int i;
-
- for ( i = 0, b = region->frame[id].bugs;
- i < region->frame[id].n_bugs; b++, i++ )
- {
- if ( ((vaddr_t)bug_loc(b)) == pc )
- {
- bug = b;
- goto found;
- }
- }
- }
- }
- found:
- if ( !bug )
- return -ENOENT;
-
- if ( id == BUGFRAME_run_fn )
- {
- void (*fn)(const struct cpu_user_regs *) = (void *)regs->BUG_FN_REG;
-
- fn(regs);
- return 0;
- }
-
- /* WARN, BUG or ASSERT: decode the filename pointer and line number. */
- filename = bug_file(bug);
- if ( !is_kernel(filename) )
- return -EINVAL;
- fixup = strlen(filename);
- if ( fixup > 50 )
- {
- filename += fixup - 47;
- prefix = "...";
- }
- lineno = bug_line(bug);
-
- switch ( id )
- {
- case BUGFRAME_warn:
- printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno);
- show_execution_state(regs);
- return 0;
-
- case BUGFRAME_bug:
- printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
- show_execution_state(regs);
- panic("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
-
- case BUGFRAME_assert:
- /* ASSERT: decode the predicate string pointer. */
- predicate = bug_msg(bug);
- if ( !is_kernel(predicate) )
- predicate = "<unknown>";
-
- printk("Assertion '%s' failed at %s%s:%d\n",
- predicate, prefix, filename, lineno);
- show_execution_state(regs);
- panic("Assertion '%s' failed at %s%s:%d\n",
- predicate, prefix, filename, lineno);
- }
-
- return -EINVAL;
-}
-
#ifdef CONFIG_ARM_64
static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
{
--
2.39.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v2 4/4] xen/x86: switch x86 to use generic implemetation of bug.h
2023-02-20 16:40 [PATCH v2 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
` (2 preceding siblings ...)
2023-02-20 16:40 ` [PATCH v2 3/4] xen/arm: switch ARM to use generic implementation of bug.h Oleksii Kurochko
@ 2023-02-20 16:41 ` Oleksii Kurochko
3 siblings, 0 replies; 15+ messages in thread
From: Oleksii Kurochko @ 2023-02-20 16:41 UTC (permalink / raw
To: xen-devel
Cc: Jan Beulich, Andrew Cooper, Stefano Stabellini, Gianluca Guida,
Oleksii Kurochko, Roger Pau Monné, Wei Liu
Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
---
Changes in V2:
* Remove all unnecessary things from <asm/bug.h> as they were introduced in
<xen/bug.h>.
* Define BUG_INSTR = 'ud2' and MODIFIER = 'c' ( it is needed to skip '$'
when use an imidiate in x86 assembly )
* Update do_invalid_op() to re-use handle_bug_frame() and find_bug_frame()
from generic implemetation of CONFIG_GENERIC_BUG_FRAME
* Code style fixes.
---
xen/arch/x86/Kconfig | 1 +
xen/arch/x86/include/asm/bug.h | 88 ++--------------------------------
xen/arch/x86/traps.c | 81 +++++--------------------------
3 files changed, 18 insertions(+), 152 deletions(-)
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 6a7825f4ba..b0ff1f3ee6 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -11,6 +11,7 @@ config X86
select ARCH_MAP_DOMAIN_PAGE
select ARCH_SUPPORTS_INT128
select CORE_PARKING
+ select GENERIC_BUG_FRAME
select HAS_ALTERNATIVE
select HAS_COMPAT
select HAS_CPUFREQ
diff --git a/xen/arch/x86/include/asm/bug.h b/xen/arch/x86/include/asm/bug.h
index b7265bdfbe..31fc687669 100644
--- a/xen/arch/x86/include/asm/bug.h
+++ b/xen/arch/x86/include/asm/bug.h
@@ -1,92 +1,12 @@
#ifndef __X86_BUG_H__
#define __X86_BUG_H__
-#define BUG_DISP_WIDTH 24
-#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
-#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
-
-#define BUGFRAME_run_fn 0
-#define BUGFRAME_warn 1
-#define BUGFRAME_bug 2
-#define BUGFRAME_assert 3
-
-#define BUGFRAME_NR 4
-
#ifndef __ASSEMBLY__
-struct bug_frame {
- signed int loc_disp:BUG_DISP_WIDTH;
- unsigned int line_hi:BUG_LINE_HI_WIDTH;
- signed int ptr_disp:BUG_DISP_WIDTH;
- unsigned int line_lo:BUG_LINE_LO_WIDTH;
- signed int msg_disp[];
-};
-
-#define bug_loc(b) ((const void *)(b) + (b)->loc_disp)
-#define bug_ptr(b) ((const void *)(b) + (b)->ptr_disp)
-#define bug_line(b) (((((b)->line_hi + ((b)->loc_disp < 0)) & \
- ((1 << BUG_LINE_HI_WIDTH) - 1)) << \
- BUG_LINE_LO_WIDTH) + \
- (((b)->line_lo + ((b)->ptr_disp < 0)) & \
- ((1 << BUG_LINE_LO_WIDTH) - 1)))
-#define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
-
-#define _ASM_BUGFRAME_TEXT(second_frame) \
- ".Lbug%=: ud2\n" \
- ".pushsection .bug_frames.%c[bf_type], \"a\", @progbits\n" \
- ".p2align 2\n" \
- ".Lfrm%=:\n" \
- ".long (.Lbug%= - .Lfrm%=) + %c[bf_line_hi]\n" \
- ".long (%c[bf_ptr] - .Lfrm%=) + %c[bf_line_lo]\n" \
- ".if " #second_frame "\n" \
- ".long 0, %c[bf_msg] - .Lfrm%=\n" \
- ".endif\n" \
- ".popsection\n" \
-
-#define _ASM_BUGFRAME_INFO(type, line, ptr, msg) \
- [bf_type] "i" (type), \
- [bf_ptr] "i" (ptr), \
- [bf_msg] "i" (msg), \
- [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1)) \
- << BUG_DISP_WIDTH), \
- [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
-
-#define BUG_FRAME(type, line, ptr, second_frame, msg) do { \
- BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH)); \
- BUILD_BUG_ON((type) >= BUGFRAME_NR); \
- asm volatile ( _ASM_BUGFRAME_TEXT(second_frame) \
- :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) ); \
-} while (0)
-
-
-#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, NULL)
-#define BUG() do { \
- BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, 0, NULL); \
- unreachable(); \
-} while (0)
-
-/*
- * TODO: untangle header dependences, break BUILD_BUG_ON() out of xen/lib.h,
- * and use a real static inline here to get proper type checking of fn().
- */
-#define run_in_exception_handler(fn) \
- do { \
- (void)((fn) == (void (*)(struct cpu_user_regs *))NULL); \
- BUG_FRAME(BUGFRAME_run_fn, 0, fn, 0, NULL); \
- } while ( 0 )
-
-#define assert_failed(msg) do { \
- BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg); \
- unreachable(); \
-} while (0)
-
-extern const struct bug_frame __start_bug_frames[],
- __stop_bug_frames_0[],
- __stop_bug_frames_1[],
- __stop_bug_frames_2[],
- __stop_bug_frames_3[];
+#define BUG_INSTR "ud2"
+#define MODIFIER "c"
-#else /* !__ASSEMBLY__ */
+#else
/*
* Construct a bugframe, suitable for using in assembly code. Should always
@@ -128,6 +48,6 @@ extern const struct bug_frame __start_bug_frames[],
#define ASSERT_FAILED(msg) \
BUG_FRAME BUGFRAME_assert, __LINE__, __FILE__, 1, msg
-#endif /* !__ASSEMBLY__ */
+#endif /* __ASSEMBLY__ */
#endif /* __X86_BUG_H__ */
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index cade9e12f8..39f153a53b 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -24,6 +24,7 @@
* Gareth Hughes <gareth@valinux.com>, May 2000
*/
+#include <xen/bug.h>
#include <xen/init.h>
#include <xen/sched.h>
#include <xen/lib.h>
@@ -1168,10 +1169,8 @@ void do_invalid_op(struct cpu_user_regs *regs)
{
const struct bug_frame *bug = NULL;
u8 bug_insn[2];
- const char *prefix = "", *filename, *predicate, *eip = (char *)regs->rip;
- unsigned long fixup;
- int id = -1, lineno;
- const struct virtual_region *region;
+ const char *eip = (char *)regs->rip;
+ unsigned int id;
if ( likely(guest_mode(regs)) )
{
@@ -1185,83 +1184,29 @@ void do_invalid_op(struct cpu_user_regs *regs)
memcmp(bug_insn, "\xf\xb", sizeof(bug_insn)) )
goto die;
- region = find_text_region(regs->rip);
- if ( region )
- {
- for ( id = 0; id < BUGFRAME_NR; id++ )
- {
- const struct bug_frame *b;
- unsigned int i;
-
- for ( i = 0, b = region->frame[id].bugs;
- i < region->frame[id].n_bugs; b++, i++ )
- {
- if ( bug_loc(b) == eip )
- {
- bug = b;
- goto found;
- }
- }
- }
- }
-
- found:
+ bug = find_bug_frame((unsigned long)eip, &id);
if ( !bug )
goto die;
+
eip += sizeof(bug_insn);
- if ( id == BUGFRAME_run_fn )
- {
- void (*fn)(struct cpu_user_regs *) = bug_ptr(bug);
- fn(regs);
- fixup_exception_return(regs, (unsigned long)eip);
- return;
+ switch ( id )
+ {
+ case BUGFRAME_bug:
+ case BUGFRAME_assert:
+ if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
+ return;
}
- /* WARN, BUG or ASSERT: decode the filename pointer and line number. */
- filename = bug_ptr(bug);
- if ( !is_kernel(filename) && !is_patch(filename) )
+ if ( handle_bug_frame(regs, bug, id) )
goto die;
- fixup = strlen(filename);
- if ( fixup > 50 )
- {
- filename += fixup - 47;
- prefix = "...";
- }
- lineno = bug_line(bug);
switch ( id )
{
+ case BUGFRAME_run_fn:
case BUGFRAME_warn:
- printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno);
- show_execution_state(regs);
fixup_exception_return(regs, (unsigned long)eip);
return;
-
- case BUGFRAME_bug:
- printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
-
- if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
- return;
-
- show_execution_state(regs);
- panic("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
-
- case BUGFRAME_assert:
- /* ASSERT: decode the predicate string pointer. */
- predicate = bug_msg(bug);
- if ( !is_kernel(predicate) && !is_patch(predicate) )
- predicate = "<unknown>";
-
- printk("Assertion '%s' failed at %s%s:%d\n",
- predicate, prefix, filename, lineno);
-
- if ( debugger_trap_fatal(TRAP_invalid_op, regs) )
- return;
-
- show_execution_state(regs);
- panic("Assertion '%s' failed at %s%s:%d\n",
- predicate, prefix, filename, lineno);
}
die:
--
2.39.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-20 16:40 ` [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
@ 2023-02-22 12:46 ` Jan Beulich
2023-02-22 16:16 ` Oleksii
2023-02-23 13:16 ` Oleksii
2023-02-23 13:32 ` Jan Beulich
1 sibling, 2 replies; 15+ messages in thread
From: Jan Beulich @ 2023-02-22 12:46 UTC (permalink / raw
To: Oleksii Kurochko, Andrew Cooper
Cc: Stefano Stabellini, Gianluca Guida, George Dunlap, Julien Grall,
Wei Liu, xen-devel
On 20.02.2023 17:40, Oleksii Kurochko wrote:
> A large part of the content of the bug.h is repeated among all
> architectures, so it was decided to create a new config
> CONFIG_GENERIC_BUG_FRAME.
>
> The version of <bug.h> from x86 was taken as the base version.
>
> The patch introduces the following stuff:
> * common bug.h header
> * generic implementation of do_bug_frame()
> * new config CONFIG_GENERIC_BUG_FRAME
>
> Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
>
> ---
> Changes in V2:
> - Switch to x86 implementation as generic as it is more compact
> ( at least from the point of view of bug frame structure ).
> - Rename CONFIG_GENERIC_DO_BUG_FRAME to CONFIG_GENERIC_BUG_FRAME.
> - Change the macro bug_loc(b) to avoid the need for a cast:
> #define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp)
> - Rename BUG_FRAME_STUFF to BUG_FRAME_STRUCT
> - Make macros related to bug frame structure more generic.
> - Introduce BUG_INSTR and MODIFIER to make _ASM_BUGFRAME_TEXT reusable
> between x86 and RISC-V.
Hmm, below I see it's really just "MODIFIER". I see two issues with this:
For one the name is too generic for something which cannot be #undef-ed
after use inside the header. And then it also doesn't really say what is
being modified. While ending up longer, how about BUG_ASM_CONST or alike?
I also think this should default to something if not overridden by an
arch. Perhaps simply to an expansion to nothing (at which point you
won't need to define it for RISC-V, aiui).
> --- /dev/null
> +++ b/xen/common/bug.c
> @@ -0,0 +1,113 @@
> +#include <xen/bug.h>
> +#include <xen/errno.h>
> +#include <xen/kernel.h>
> +#include <xen/livepatch.h>
> +#include <xen/string.h>
> +#include <xen/types.h>
> +#include <xen/virtual_region.h>
> +
> +#include <asm/processor.h>
Is this really needed here?
> +const struct bug_frame* find_bug_frame(unsigned long pc, unsigned int *id)
Is this function going to be needed outside of this CU? IOW why is it not
static?
Also, nit: Left most star wants changing places with the adjacent blank.
> +{
> + const struct bug_frame *bug = NULL;
> + const struct virtual_region *region;
> +
> + region = find_text_region(pc);
> + if ( region )
> + {
> + for ( *id = 0; *id < BUGFRAME_NR; (*id)++ )
> + {
> + const struct bug_frame *b;
> + unsigned int i;
> +
> + for ( i = 0, b = region->frame[*id].bugs;
> + i < region->frame[*id].n_bugs; b++, i++ )
> + {
> + if ( bug_loc(b) == pc )
> + {
> + bug = b;
> + goto found;
While in the original code the goto is kind of warranted, it isn't really
here imo. You can simply "return b" here and ...
> + }
> + }
> + }
> + }
> +
> + found:
> + return bug;
... "return NULL" here. That'll allow the function scope "bug" to go away,
at which point the inner scope "b" can become "bug".
> +}
> +
> +int handle_bug_frame(const struct cpu_user_regs *regs,
> + const struct bug_frame *bug,
> + unsigned int id)
Nit: Indentation is off by one here. Also same question regarding the lack
of static here.
> +{
> + const char *prefix = "", *filename, *predicate;
> + unsigned long fixup;
> + unsigned int lineno;
> +
> + if ( id == BUGFRAME_run_fn )
> + {
> +#ifdef ARM
Who or what defines ARM? Anyway, seeing ...
> + void (*fn)(const struct cpu_user_regs *) = (void *)regs->BUG_FN_REG;
... this, wouldn't it be better (and independent of the specific arch) if
you checked for BUG_FN_REG being defined?
Another (#ifdef-free) variant would be to have bug_ptr() take a 2nd argument
and then uniformly use ...
> +#else
> + void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug);
... this, slightly altered to
void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug, regs);
> +#endif
> +
> + fn(regs);
> + return 0;
> + }
> +
> + /* WARN, BUG or ASSERT: decode the filename pointer and line number. */
> + filename = bug_ptr(bug);
> + if ( !is_kernel(filename) && !is_patch(filename) )
> + return -EINVAL;
> + fixup = strlen(filename);
> + if ( fixup > 50 )
> + {
> + filename += fixup - 47;
> + prefix = "...";
> + }
> + lineno = bug_line(bug);
> +
> + switch ( id )
> + {
> + case BUGFRAME_warn:
> + printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno);
> + show_execution_state(regs);
> + return 0;
> +
> + case BUGFRAME_bug:
> + printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
> +
> + show_execution_state(regs);
> + panic("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
> +
> + case BUGFRAME_assert:
> + /* ASSERT: decode the predicate string pointer. */
> + predicate = bug_msg(bug);
> + if ( !is_kernel(predicate) )
> + predicate = "<unknown>";
> +
> + printk("Assertion '%s' failed at %s%s:%d\n",
> + predicate, prefix, filename, lineno);
> +
> + show_execution_state(regs);
> + panic("Assertion '%s' failed at %s%s:%d\n",
> + predicate, prefix, filename, lineno);
> + }
> +
> + return -EINVAL;
> +}
> +
> +int do_bug_frame(const struct cpu_user_regs *regs, unsigned long pc)
> +{
> + const struct bug_frame *bug = NULL;
Nit: pointless initializer. You could of course have the assignment below
become the initializer here.
> + unsigned int id;
> +
> + bug = find_bug_frame(pc, &id);
> + if (!bug)
Nit: Style (missing blanks).
> --- /dev/null
> +++ b/xen/include/xen/bug.h
> @@ -0,0 +1,161 @@
> +#ifndef __XEN_BUG_H__
> +#define __XEN_BUG_H__
> +
> +#define BUG_DISP_WIDTH 24
> +#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
> +#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
> +
> +#define BUGFRAME_run_fn 0
> +#define BUGFRAME_warn 1
> +#define BUGFRAME_bug 2
> +#define BUGFRAME_assert 3
> +
> +#define BUGFRAME_NR 4
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <xen/errno.h>
> +#include <xen/lib.h>
> +#include <xen/stringify.h>
> +#include <xen/types.h>
> +
> +#include <asm/bug.h>
Any reason this cannot live ahead of the #ifndef, eliminating the need for
an #else further down?
> +#ifndef BUG_FRAME_STRUCT
> +
> +struct bug_frame {
> + signed int loc_disp:BUG_DISP_WIDTH;
> + unsigned int line_hi:BUG_LINE_HI_WIDTH;
> + signed int ptr_disp:BUG_DISP_WIDTH;
> + unsigned int line_lo:BUG_LINE_LO_WIDTH;
> + signed int msg_disp[];
> +};
> +
> +#endif /* BUG_FRAME_STRUCT */
> +
> +#ifndef bug_loc
> +#define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp)
> +#endif /* bug_loc */
For short #if / #ifdef I don't think such comments are necessary on the
#else or #endif; personally I consider such to hamper readability.
> +#ifndef bug_ptr
> +#define bug_ptr(b) ((const void *)(b) + (b)->ptr_disp)
> +#endif /* bug_ptr */
> +
> +#ifndef bug_line
> +#define bug_line(b) (((((b)->line_hi + ((b)->loc_disp < 0)) & \
> + ((1 << BUG_LINE_HI_WIDTH) - 1)) << \
> + BUG_LINE_LO_WIDTH) + \
> + (((b)->line_lo + ((b)->ptr_disp < 0)) & \
> + ((1 << BUG_LINE_LO_WIDTH) - 1)))
> +#endif /* bug_line */
> +
> +#ifndef bug_msg
> +#define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
> +#endif /* bug_msg */
> +
> +#ifndef _ASM_BUGFRAME_TEXT
> +
> +#define _ASM_BUGFRAME_TEXT(second_frame) \
> + ".Lbug%=:"BUG_INSTR"\n" \
> + ".pushsection .bug_frames.%"MODIFIER"[bf_type], \"a\", @progbits\n" \
You may want to use %progbits here right away, as being the more portable
form.
> + ".p2align 2\n" \
> + ".Lfrm%=:\n" \
> + ".long (.Lbug%= - .Lfrm%=) + %"MODIFIER"[bf_line_hi]\n" \
> + ".long (%"MODIFIER"[bf_ptr] - .Lfrm%=) + %"MODIFIER"[bf_line_lo]\n" \
> + ".if " #second_frame "\n" \
> + ".long 0, %"MODIFIER"[bf_msg] - .Lfrm%=\n" \
> + ".endif\n" \
> + ".popsection\n"
I think I said so in reply to an earlier version already: The moment
assembly code moves to a common header, it should be adjusted to be as
portable as possible. In particular directives should never start at the
beginning of a line, while labels always should. (Whether .long is
actually portable is another question, which I'll be happy to leave
aside for now.)
Also nit (style): The line continuation characters want to all line up.
> +#endif /* _ASM_BUGFRAME_TEXT */
> +
> +#ifndef _ASM_BUGFRAME_INFO
I don't think these two make sense for an arch to define independently.
INFO absolutely has to match TEXT, so I think an arch should always
define (or not) both together.
> +#define _ASM_BUGFRAME_INFO(type, line, ptr, msg) \
> + [bf_type] "i" (type), \
> + [bf_ptr] "i" (ptr), \
> + [bf_msg] "i" (msg), \
> + [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) - 1)) \
> + << BUG_DISP_WIDTH), \
> + [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
> +
> +#endif /* _ASM_BUGFRAME_INFO */
> +
> +#ifndef BUG_FRAME
> +
> +#define BUG_FRAME(type, line, ptr, second_frame, msg) do { \
> + BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH + BUG_LINE_HI_WIDTH)); \
> + BUILD_BUG_ON((type) >= BUGFRAME_NR); \
> + asm volatile ( _ASM_BUGFRAME_TEXT(second_frame) \
> + :: _ASM_BUGFRAME_INFO(type, line, ptr, msg) ); \
> +} while (0)
> +
> +#endif
> +
> +#ifndef run_in_exception_handler
> +
> +/*
> + * TODO: untangle header dependences, break BUILD_BUG_ON() out of xen/lib.h,
> + * and use a real static inline here to get proper type checking of fn().
> + */
I realize you only copy this comment, but I'm having a hard time seeing
the connection to BUILD_BUG_ON() here. Would be nice if the comment was
"generalized" in a form that actually can be understood. Andrew?
> +#define run_in_exception_handler(fn) \
> + do { \
> + (void)((fn) == (void (*)(struct cpu_user_regs *))NULL); \
> + BUG_FRAME(BUGFRAME_run_fn, 0, fn, 0, NULL); \
> + } while ( 0 )
> +
> +#endif /* run_in_exception_handler */
> +
> +#ifndef WARN
> +#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, NULL)
> +#endif /* WARN */
No real need for the comment here; you also have none below for e.g.
BUG().
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-22 12:46 ` Jan Beulich
@ 2023-02-22 16:16 ` Oleksii
2023-02-23 10:11 ` Jan Beulich
2023-02-23 13:16 ` Oleksii
1 sibling, 1 reply; 15+ messages in thread
From: Oleksii @ 2023-02-22 16:16 UTC (permalink / raw
To: Jan Beulich, Andrew Cooper
Cc: Stefano Stabellini, Gianluca Guida, George Dunlap, Julien Grall,
Wei Liu, xen-devel
Hello Jan,
On Wed, 2023-02-22 at 13:46 +0100, Jan Beulich wrote:
> On 20.02.2023 17:40, Oleksii Kurochko wrote:
> > A large part of the content of the bug.h is repeated among all
> > architectures, so it was decided to create a new config
> > CONFIG_GENERIC_BUG_FRAME.
> >
> > The version of <bug.h> from x86 was taken as the base version.
> >
> > The patch introduces the following stuff:
> > * common bug.h header
> > * generic implementation of do_bug_frame()
> > * new config CONFIG_GENERIC_BUG_FRAME
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
> >
> > ---
> > Changes in V2:
> > - Switch to x86 implementation as generic as it is more compact
> > ( at least from the point of view of bug frame structure ).
> > - Rename CONFIG_GENERIC_DO_BUG_FRAME to CONFIG_GENERIC_BUG_FRAME.
> > - Change the macro bug_loc(b) to avoid the need for a cast:
> > #define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp)
> > - Rename BUG_FRAME_STUFF to BUG_FRAME_STRUCT
> > - Make macros related to bug frame structure more generic.
> > - Introduce BUG_INSTR and MODIFIER to make _ASM_BUGFRAME_TEXT
> > reusable
> > between x86 and RISC-V.
>
> Hmm, below I see it's really just "MODIFIER". I see two issues with
> this:
> For one the name is too generic for something which cannot be #undef-
> ed
> after use inside the header. And then it also doesn't really say what
> is
> being modified. While ending up longer, how about BUG_ASM_CONST or
> alike?
BUG_ASM_CONST will be much better.
>
> I also think this should default to something if not overridden by an
> arch. Perhaps simply to an expansion to nothing (at which point you
> won't need to define it for RISC-V, aiui).
>
Agree.
Initially, I thought about that too but couldn't estimate how well the
modifier '%c' is supported, deciding that each architecture has to
define it.
But we can make it equal to nothing (at least for 2 architectures ) it
doesn't have the same support as in x86.
> > --- /dev/null
> > +++ b/xen/common/bug.c
> > @@ -0,0 +1,113 @@
> > +#include <xen/bug.h>
> > +#include <xen/errno.h>
> > +#include <xen/kernel.h>
> > +#include <xen/livepatch.h>
> > +#include <xen/string.h>
> > +#include <xen/types.h>
> > +#include <xen/virtual_region.h>
> > +
> > +#include <asm/processor.h>
>
> Is this really needed here?
Yes, it is.
Function show_execution_state() is declared in this header for all
architectures and is used by handle_bug_frame().
>
> > +const struct bug_frame* find_bug_frame(unsigned long pc, unsigned
> > int *id)
>
> Is this function going to be needed outside of this CU? IOW why is it
> not
> static?
>
It's not static because there is not possible to use do_bug_frame() as
is for x86 as x86 has some additional checks for some cases which
aren't in generic implementation:
[1]
https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1217
[2]
https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1238
[3]
https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1259
Probably to make generic do_bug_frame() more re-usable for x86 we can
implement as stubs fixup_exception_return() and debugger_trap_fatal()
under #ifndef X86 ... #endif inside common/bug.c.
Could you please share your thoughts about that?
> Also, nit: Left most star wants changing places with the adjacent
> blank.
Thanks. I'll update in the next version of the patch series.
>
> > +{
> > + const struct bug_frame *bug = NULL;
> > + const struct virtual_region *region;
> > +
> > + region = find_text_region(pc);
> > + if ( region )
> > + {
> > + for ( *id = 0; *id < BUGFRAME_NR; (*id)++ )
> > + {
> > + const struct bug_frame *b;
> > + unsigned int i;
> > +
> > + for ( i = 0, b = region->frame[*id].bugs;
> > + i < region->frame[*id].n_bugs; b++, i++ )
> > + {
> > + if ( bug_loc(b) == pc )
> > + {
> > + bug = b;
> > + goto found;
>
> While in the original code the goto is kind of warranted, it isn't
> really
> here imo. You can simply "return b" here and ...
>
> > + }
> > + }
> > + }
> > + }
> > +
> > + found:
> > + return bug;
>
> ... "return NULL" here. That'll allow the function scope "bug" to go
> away,
> at which point the inner scope "b" can become "bug".
Agree, missed that when decided to move that part of the code to
separate function.
>
> > +}
> > +
> > +int handle_bug_frame(const struct cpu_user_regs *regs,
> > + const struct bug_frame *bug,
> > + unsigned int id)
>
> Nit: Indentation is off by one here. Also same question regarding the
> lack
> of static here.
Regarding static an answer is the same as with previous one static
question.
>
> > +{
> > + const char *prefix = "", *filename, *predicate;
> > + unsigned long fixup;
> > + unsigned int lineno;
> > +
> > + if ( id == BUGFRAME_run_fn )
> > + {
> > +#ifdef ARM
>
> Who or what defines ARM? Anyway, seeing ...
it is defined by default in Kconfig:
https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/arm/Kconfig#L13
>
> > + void (*fn)(const struct cpu_user_regs *) = (void *)regs-
> > >BUG_FN_REG;
>
> ... this, wouldn't it be better (and independent of the specific
> arch) if
> you checked for BUG_FN_REG being defined?
If I understand Kconfig correctly than there is no significant
difference what check. But probably BUG_FN_REG would be a little bit
better if someone will decide to change a way how pointer to function
will be passed in case of ARM than we will get compilation error and so
won't miss to fix the line in do_bug_frame().
>
> Another (#ifdef-free) variant would be to have bug_ptr() take a 2nd
> argument
> and then uniformly use ...
>
> > +#else
> > + void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug);
>
> ... this, slightly altered to
>
> void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug,
> regs);
Probably this one option will be the most universal and we have to
stick to it.
>
> > +#endif
> > +
> > + fn(regs);
> > + return 0;
> > + }
> > +
> > + /* WARN, BUG or ASSERT: decode the filename pointer and line
> > number. */
> > + filename = bug_ptr(bug);
> > + if ( !is_kernel(filename) && !is_patch(filename) )
> > + return -EINVAL;
> > + fixup = strlen(filename);
> > + if ( fixup > 50 )
> > + {
> > + filename += fixup - 47;
> > + prefix = "...";
> > + }
> > + lineno = bug_line(bug);
> > +
> > + switch ( id )
> > + {
> > + case BUGFRAME_warn:
> > + printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno);
> > + show_execution_state(regs);
> > + return 0;
> > +
> > + case BUGFRAME_bug:
> > + printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
> > +
> > + show_execution_state(regs);
> > + panic("Xen BUG at %s%s:%d\n", prefix, filename, lineno);
> > +
> > + case BUGFRAME_assert:
> > + /* ASSERT: decode the predicate string pointer. */
> > + predicate = bug_msg(bug);
> > + if ( !is_kernel(predicate) )
> > + predicate = "<unknown>";
> > +
> > + printk("Assertion '%s' failed at %s%s:%d\n",
> > + predicate, prefix, filename, lineno);
> > +
> > + show_execution_state(regs);
> > + panic("Assertion '%s' failed at %s%s:%d\n",
> > + predicate, prefix, filename, lineno);
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +
> > +int do_bug_frame(const struct cpu_user_regs *regs, unsigned long
> > pc)
> > +{
> > + const struct bug_frame *bug = NULL;
>
> Nit: pointless initializer. You could of course have the assignment
> below
> become the initializer here.
Thanks. I'll update it the next version of the patch.
>
> > + unsigned int id;
> > +
> > + bug = find_bug_frame(pc, &id);
> > + if (!bug)
>
> Nit: Style (missing blanks).
Thanks. I'll update it the next version of the patch.
>
> > --- /dev/null
> > +++ b/xen/include/xen/bug.h
> > @@ -0,0 +1,161 @@
> > +#ifndef __XEN_BUG_H__
> > +#define __XEN_BUG_H__
> > +
> > +#define BUG_DISP_WIDTH 24
> > +#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
> > +#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
> > +
> > +#define BUGFRAME_run_fn 0
> > +#define BUGFRAME_warn 1
> > +#define BUGFRAME_bug 2
> > +#define BUGFRAME_assert 3
> > +
> > +#define BUGFRAME_NR 4
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <xen/errno.h>
> > +#include <xen/lib.h>
> > +#include <xen/stringify.h>
> > +#include <xen/types.h>
> > +
> > +#include <asm/bug.h>
>
> Any reason this cannot live ahead of the #ifndef, eliminating the
> need for
> an #else further down?
It should be fine.
Probably I had some issues during the initial stage of making bug.h
more generic...
>
> > +#ifndef BUG_FRAME_STRUCT
> > +
> > +struct bug_frame {
> > + signed int loc_disp:BUG_DISP_WIDTH;
> > + unsigned int line_hi:BUG_LINE_HI_WIDTH;
> > + signed int ptr_disp:BUG_DISP_WIDTH;
> > + unsigned int line_lo:BUG_LINE_LO_WIDTH;
> > + signed int msg_disp[];
> > +};
> > +
> > +#endif /* BUG_FRAME_STRUCT */
> > +
> > +#ifndef bug_loc
> > +#define bug_loc(b) ((unsigned long)(b) + (b)->loc_disp)
> > +#endif /* bug_loc */
>
> For short #if / #ifdef I don't think such comments are necessary on
> the
> #else or #endif; personally I consider such to hamper readability.
Thanks. I'll take it into account.
>
> > +#ifndef bug_ptr
> > +#define bug_ptr(b) ((const void *)(b) + (b)->ptr_disp)
> > +#endif /* bug_ptr */
> > +
> > +#ifndef bug_line
> > +#define bug_line(b) (((((b)->line_hi + ((b)->loc_disp < 0))
> > & \
> > + ((1 << BUG_LINE_HI_WIDTH) - 1))
> > << \
> > + BUG_LINE_LO_WIDTH)
> > + \
> > + (((b)->line_lo + ((b)->ptr_disp < 0))
> > & \
> > + ((1 << BUG_LINE_LO_WIDTH) - 1)))
> > +#endif /* bug_line */
> > +
> > +#ifndef bug_msg
> > +#define bug_msg(b) ((const char *)(b) + (b)->msg_disp[1])
> > +#endif /* bug_msg */
> > +
> > +#ifndef _ASM_BUGFRAME_TEXT
> > +
> > +#define
> > _ASM_BUGFRAME_TEXT(second_frame)
> > \
> > +
> > ".Lbug%=:"BUG_INSTR"\n"
> > \
> > + ".pushsection .bug_frames.%"MODIFIER"[bf_type], \"a\",
> > @progbits\n" \
>
> You may want to use %progbits here right away, as being the more
> portable
> form.
Sure. Thanks. I'll take that into account.
>
> > + ".p2align
> > 2\n" \
> > +
> > ".Lfrm%=:\n"
> > \
> > + ".long (.Lbug%= - .Lfrm%=) +
> > %"MODIFIER"[bf_line_hi]\n" \
> > + ".long (%"MODIFIER"[bf_ptr] - .Lfrm%=) +
> > %"MODIFIER"[bf_line_lo]\n" \
> > + ".if " #second_frame
> > "\n" \
> > + ".long 0, %"MODIFIER"[bf_msg] -
> > .Lfrm%=\n" \
> > +
> > ".endif\n"
> > \
> > + ".popsection\n"
>
> I think I said so in reply to an earlier version already: The moment
> assembly code moves to a common header, it should be adjusted to be
> as
> portable as possible. In particular directives should never start at
> the
> beginning of a line, while labels always should. (Whether .long is
> actually portable is another question, which I'll be happy to leave
> aside for now.)
I am not sure that I understand about which one directive we are
talking about... Are we talking about .{push/pop}section and .p2align?
Probably you can show me an example in Xen or other project?
>
> Also nit (style): The line continuation characters want to all line
> up.
>
> > +#endif /* _ASM_BUGFRAME_TEXT */
> > +
> > +#ifndef _ASM_BUGFRAME_INFO
>
> I don't think these two make sense for an arch to define
> independently.
> INFO absolutely has to match TEXT, so I think an arch should always
> define (or not) both together.
You are right. I'll take into account that.
>
> > +#define _ASM_BUGFRAME_INFO(type, line, ptr,
> > msg) \
> > + [bf_type] "i"
> > (type), \
> > + [bf_ptr] "i"
> > (ptr), \
> > + [bf_msg] "i"
> > (msg), \
> > + [bf_line_lo] "i" ((line & ((1 << BUG_LINE_LO_WIDTH) -
> > 1)) \
> > + <<
> > BUG_DISP_WIDTH), \
> > + [bf_line_hi] "i" (((line) >> BUG_LINE_LO_WIDTH) <<
> > BUG_DISP_WIDTH)
> > +
> > +#endif /* _ASM_BUGFRAME_INFO */
> > +
> > +#ifndef BUG_FRAME
> > +
> > +#define BUG_FRAME(type, line, ptr, second_frame, msg) do
> > { \
> > + BUILD_BUG_ON((line) >> (BUG_LINE_LO_WIDTH +
> > BUG_LINE_HI_WIDTH)); \
> > + BUILD_BUG_ON((type) >=
> > BUGFRAME_NR); \
> > + asm volatile (
> > _ASM_BUGFRAME_TEXT(second_frame) \
> > + :: _ASM_BUGFRAME_INFO(type, line, ptr, msg)
> > ); \
> > +} while (0)
> > +
> > +#endif
> > +
> > +#ifndef run_in_exception_handler
> > +
> > +/*
> > + * TODO: untangle header dependences, break BUILD_BUG_ON() out of
> > xen/lib.h,
> > + * and use a real static inline here to get proper type checking
> > of fn().
> > + */
>
> I realize you only copy this comment, but I'm having a hard time
> seeing
> the connection to BUILD_BUG_ON() here. Would be nice if the comment
> was
> "generalized" in a form that actually can be understood. Andrew?
>
> > +#define run_in_exception_handler(fn) \
> > + do { \
> > + (void)((fn) == (void (*)(struct cpu_user_regs *))NULL); \
> > + BUG_FRAME(BUGFRAME_run_fn, 0, fn, 0, NULL); \
> > + } while ( 0 )
> > +
> > +#endif /* run_in_exception_handler */
> > +
> > +#ifndef WARN
> > +#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0,
> > NULL)
> > +#endif /* WARN */
>
> No real need for the comment here; you also have none below for e.g.
> BUG().
Thanks.
>
> Jan
~ Oleksii
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-22 16:16 ` Oleksii
@ 2023-02-23 10:11 ` Jan Beulich
2023-02-23 12:14 ` Oleksii
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2023-02-23 10:11 UTC (permalink / raw
To: Oleksii
Cc: Stefano Stabellini, Gianluca Guida, George Dunlap, Julien Grall,
Wei Liu, xen-devel, Andrew Cooper
On 22.02.2023 17:16, Oleksii wrote:
> On Wed, 2023-02-22 at 13:46 +0100, Jan Beulich wrote:
>> On 20.02.2023 17:40, Oleksii Kurochko wrote:
>>> --- /dev/null
>>> +++ b/xen/common/bug.c
>>> @@ -0,0 +1,113 @@
>>> +#include <xen/bug.h>
>>> +#include <xen/errno.h>
>>> +#include <xen/kernel.h>
>>> +#include <xen/livepatch.h>
>>> +#include <xen/string.h>
>>> +#include <xen/types.h>
>>> +#include <xen/virtual_region.h>
>>> +
>>> +#include <asm/processor.h>
>>
>> Is this really needed here?
> Yes, it is.
>
> Function show_execution_state() is declared in this header for all
> architectures and is used by handle_bug_frame().
Ugly, but yes, you're right.
>>> +const struct bug_frame* find_bug_frame(unsigned long pc, unsigned
>>> int *id)
>>
>> Is this function going to be needed outside of this CU? IOW why is it
>> not
>> static?
>>
> It's not static because there is not possible to use do_bug_frame() as
> is for x86 as x86 has some additional checks for some cases which
> aren't in generic implementation:
> [1]
> https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1217
> [2]
> https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1238
> [3]
> https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1259
>
> Probably to make generic do_bug_frame() more re-usable for x86 we can
> implement as stubs fixup_exception_return() and debugger_trap_fatal()
> under #ifndef X86 ... #endif inside common/bug.c.
>
> Could you please share your thoughts about that?
Isn't all that's needed a suitable return value from the function,
for the caller to take appropriate further action? (Maybe even that
isn't really necessary.)
That said, debugger_trap_fatal() imo may sensibly be generalized,
and hence could be left in common code. Arm may simply stub this to
nothing then for the time being.
>>> +{
>>> + const char *prefix = "", *filename, *predicate;
>>> + unsigned long fixup;
>>> + unsigned int lineno;
>>> +
>>> + if ( id == BUGFRAME_run_fn )
>>> + {
>>> +#ifdef ARM
>>
>> Who or what defines ARM? Anyway, seeing ...
> it is defined by default in Kconfig:
> https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/arm/Kconfig#L13
That'll be CONFIG_ARM then in uses in C code.
>>> + void (*fn)(const struct cpu_user_regs *) = (void *)regs-
>>>> BUG_FN_REG;
>>
>> ... this, wouldn't it be better (and independent of the specific
>> arch) if
>> you checked for BUG_FN_REG being defined?
> If I understand Kconfig correctly than there is no significant
> difference what check. But probably BUG_FN_REG would be a little bit
> better if someone will decide to change a way how pointer to function
> will be passed in case of ARM than we will get compilation error and so
> won't miss to fix the line in do_bug_frame().
Indeed - #ifdef like this one generally want to check for the precise
aspect in question, without making assumptions about something implying
something else. (Pretty certainly there are exceptions to this rule,
but it holds here.)
>>> + ".p2align
>>> 2\n" \
>>> +
>>> ".Lfrm%=:\n"
>>> \
>>> + ".long (.Lbug%= - .Lfrm%=) +
>>> %"MODIFIER"[bf_line_hi]\n" \
>>> + ".long (%"MODIFIER"[bf_ptr] - .Lfrm%=) +
>>> %"MODIFIER"[bf_line_lo]\n" \
>>> + ".if " #second_frame
>>> "\n" \
>>> + ".long 0, %"MODIFIER"[bf_msg] -
>>> .Lfrm%=\n" \
>>> +
>>> ".endif\n"
>>> \
>>> + ".popsection\n"
>>
>> I think I said so in reply to an earlier version already: The moment
>> assembly code moves to a common header, it should be adjusted to be
>> as
>> portable as possible. In particular directives should never start at
>> the
>> beginning of a line, while labels always should. (Whether .long is
>> actually portable is another question, which I'll be happy to leave
>> aside for now.)
> I am not sure that I understand about which one directive we are
> talking about... Are we talking about .{push/pop}section and .p2align?
> Probably you can show me an example in Xen or other project?
I'm talking about all directives here. Fundamentally assembly language
source lines are like this (assuming colons follow labels):
[<label>:|<blank>][<directive>|<insn>]
Both parts can optionally be empty, but if the right one isn't, then
the left one also shouldn't be (and hence minimally a blank is needed;
commonly it would be a tab). Directives, unlike insns, start with a
dot in most dialects. Labels can also start with a dot, but are
disambiguated by the colon after them (yet more strict placement of
items is therefore required when labels are not followed by colons -
there are such dialects -, which is why it is generally a good idea
to follow that simple formatting rule). Also, ftaod,
- <insn> covers both actual insns as well as assembler macro
invocations,
- there can of course be multiple labels on a single line (iirc this
requires that colons follow labels).
As to examples: I'm afraid I'm unaware of arch-independent assembly
code anywhere in Xen so far.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-23 10:11 ` Jan Beulich
@ 2023-02-23 12:14 ` Oleksii
0 siblings, 0 replies; 15+ messages in thread
From: Oleksii @ 2023-02-23 12:14 UTC (permalink / raw
To: Jan Beulich
Cc: Stefano Stabellini, Gianluca Guida, George Dunlap, Julien Grall,
Wei Liu, xen-devel, Andrew Cooper
On Thu, 2023-02-23 at 11:11 +0100, Jan Beulich wrote:
> On 22.02.2023 17:16, Oleksii wrote:
> > On Wed, 2023-02-22 at 13:46 +0100, Jan Beulich wrote:
> > > On 20.02.2023 17:40, Oleksii Kurochko wrote:
> > > > --- /dev/null
> > > > +++ b/xen/common/bug.c
> > > > @@ -0,0 +1,113 @@
> > > > +#include <xen/bug.h>
> > > > +#include <xen/errno.h>
> > > > +#include <xen/kernel.h>
> > > > +#include <xen/livepatch.h>
> > > > +#include <xen/string.h>
> > > > +#include <xen/types.h>
> > > > +#include <xen/virtual_region.h>
> > > > +
> > > > +#include <asm/processor.h>
> > >
> > > Is this really needed here?
> > Yes, it is.
> >
> > Function show_execution_state() is declared in this header for all
> > architectures and is used by handle_bug_frame().
>
> Ugly, but yes, you're right.
>
> > > > +const struct bug_frame* find_bug_frame(unsigned long pc,
> > > > unsigned
> > > > int *id)
> > >
> > > Is this function going to be needed outside of this CU? IOW why
> > > is it
> > > not
> > > static?
> > >
> > It's not static because there is not possible to use do_bug_frame()
> > as
> > is for x86 as x86 has some additional checks for some cases which
> > aren't in generic implementation:
> > [1]
> > https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1217
> > [2]
> > https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1238
> > [3]
> > https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/x86/traps.c#L1259
> >
> > Probably to make generic do_bug_frame() more re-usable for x86 we
> > can
> > implement as stubs fixup_exception_return() and
> > debugger_trap_fatal()
> > under #ifndef X86 ... #endif inside common/bug.c.
> >
> > Could you please share your thoughts about that?
>
> Isn't all that's needed a suitable return value from the function,
> for the caller to take appropriate further action? (Maybe even that
> isn't really necessary.)
>
> That said, debugger_trap_fatal() imo may sensibly be generalized,
> and hence could be left in common code. Arm may simply stub this to
> nothing then for the time being.
I checked the source code I found that debugger_trap_fatal() is
generalized:
https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/include/xen/debugger.h
So we can use in generic version of do_bug_frame().
About fixup_exception_return() you are right as we can it handle(call)
by the caller so it's needed only to return a suitable return value for
do_bug_frame().
>
> > > > +{
> > > > + const char *prefix = "", *filename, *predicate;
> > > > + unsigned long fixup;
> > > > + unsigned int lineno;
> > > > +
> > > > + if ( id == BUGFRAME_run_fn )
> > > > + {
> > > > +#ifdef ARM
> > >
> > > Who or what defines ARM? Anyway, seeing ...
> > it is defined by default in Kconfig:
> > https://gitlab.com/xen-project/people/olkur/xen/-/blob/staging/xen/arch/arm/Kconfig#L13
>
> That'll be CONFIG_ARM then in uses in C code.
Ahh, yeah. I am always missing CONFIG_...
>
> > > > + void (*fn)(const struct cpu_user_regs *) = (void
> > > > *)regs-
> > > > > BUG_FN_REG;
> > >
> > > ... this, wouldn't it be better (and independent of the specific
> > > arch) if
> > > you checked for BUG_FN_REG being defined?
> > If I understand Kconfig correctly than there is no significant
> > difference what check. But probably BUG_FN_REG would be a little
> > bit
> > better if someone will decide to change a way how pointer to
> > function
> > will be passed in case of ARM than we will get compilation error
> > and so
> > won't miss to fix the line in do_bug_frame().
>
> Indeed - #ifdef like this one generally want to check for the precise
> aspect in question, without making assumptions about something
> implying
> something else. (Pretty certainly there are exceptions to this rule,
> but it holds here.)
Thanks for the explanation.
>
> > > > + ".p2align
> > > > 2\n" \
> > > > +
> > > > ".Lfrm%=:\n"
> > > >
> > > > \
> > > > + ".long (.Lbug%= - .Lfrm%=) +
> > > > %"MODIFIER"[bf_line_hi]\n" \
> > > > + ".long (%"MODIFIER"[bf_ptr] - .Lfrm%=) +
> > > > %"MODIFIER"[bf_line_lo]\n" \
> > > > + ".if " #second_frame
> > > > "\n" \
> > > > + ".long 0, %"MODIFIER"[bf_msg] -
> > > > .Lfrm%=\n" \
> > > > +
> > > > ".endif\n"
> > > >
> > > > \
> > > > + ".popsection\n"
> > >
> > > I think I said so in reply to an earlier version already: The
> > > moment
> > > assembly code moves to a common header, it should be adjusted to
> > > be
> > > as
> > > portable as possible. In particular directives should never start
> > > at
> > > the
> > > beginning of a line, while labels always should. (Whether .long
> > > is
> > > actually portable is another question, which I'll be happy to
> > > leave
> > > aside for now.)
> > I am not sure that I understand about which one directive we are
> > talking about... Are we talking about .{push/pop}section and
> > .p2align?
> > Probably you can show me an example in Xen or other project?
>
> I'm talking about all directives here. Fundamentally assembly
> language
> source lines are like this (assuming colons follow labels):
>
> [<label>:|<blank>][<directive>|<insn>]
>
> Both parts can optionally be empty, but if the right one isn't, then
> the left one also shouldn't be (and hence minimally a blank is
> needed;
> commonly it would be a tab). Directives, unlike insns, start with a
> dot in most dialects. Labels can also start with a dot, but are
> disambiguated by the colon after them (yet more strict placement of
> items is therefore required when labels are not followed by colons -
> there are such dialects -, which is why it is generally a good idea
> to follow that simple formatting rule). Also, ftaod,
> - <insn> covers both actual insns as well as assembler macro
> invocations,
> - there can of course be multiple labels on a single line (iirc this
> requires that colons follow labels).
>
> As to examples: I'm afraid I'm unaware of arch-independent assembly
> code anywhere in Xen so far.
Thank you very much for the explanation.
>
> Jan
~ Oleksii
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-22 12:46 ` Jan Beulich
2023-02-22 16:16 ` Oleksii
@ 2023-02-23 13:16 ` Oleksii
2023-02-23 13:25 ` Jan Beulich
1 sibling, 1 reply; 15+ messages in thread
From: Oleksii @ 2023-02-23 13:16 UTC (permalink / raw
To: Jan Beulich, Andrew Cooper
Cc: Stefano Stabellini, Gianluca Guida, George Dunlap, Julien Grall,
Wei Liu, xen-devel
>
> > + void (*fn)(const struct cpu_user_regs *) = (void *)regs-
> > >BUG_FN_REG;
>
> ... this, wouldn't it be better (and independent of the specific
> arch) if
> you checked for BUG_FN_REG being defined?
>
> Another (#ifdef-free) variant would be to have bug_ptr() take a 2nd
> argument
> and then uniformly use ...
>
> > +#else
> > + void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug);
>
> ... this, slightly altered to
>
> void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug,
> regs);
I think that I will go with BUG_FN_REG instead of changing bug_ptr()'s
arguments as bug_ptr() is used below to get file name so it won't be
clear what bug_ptr() should return either an address of file name or
regs->BUG_FN_REG.
>
> > +#endif
> > +
> > + fn(regs);
> > + return 0;
> > + }
> > +
> > + /* WARN, BUG or ASSERT: decode the filename pointer and line
> > number. */
> > + filename = bug_ptr(bug);
> > + if ( !is_kernel(filename) && !is_patch(filename) )
>
~ Oleksii
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-23 13:16 ` Oleksii
@ 2023-02-23 13:25 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2023-02-23 13:25 UTC (permalink / raw
To: Oleksii
Cc: Stefano Stabellini, Gianluca Guida, George Dunlap, Julien Grall,
Wei Liu, xen-devel, Andrew Cooper
On 23.02.2023 14:16, Oleksii wrote:
>>
>>> + void (*fn)(const struct cpu_user_regs *) = (void *)regs-
>>>> BUG_FN_REG;
>>
>> ... this, wouldn't it be better (and independent of the specific
>> arch) if
>> you checked for BUG_FN_REG being defined?
>>
>> Another (#ifdef-free) variant would be to have bug_ptr() take a 2nd
>> argument
>> and then uniformly use ...
>>
>>> +#else
>>> + void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug);
>>
>> ... this, slightly altered to
>>
>> void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug,
>> regs);
> I think that I will go with BUG_FN_REG instead of changing bug_ptr()'s
> arguments as bug_ptr() is used below to get file name so it won't be
> clear what bug_ptr() should return either an address of file name or
> regs->BUG_FN_REG.
Oh, indeed - I'm sorry that I didn't pay attention to ...
>>> +#endif
>>> +
>>> + fn(regs);
>>> + return 0;
>>> + }
>>> +
>>> + /* WARN, BUG or ASSERT: decode the filename pointer and line
>>> number. */
>>> + filename = bug_ptr(bug);
>>> + if ( !is_kernel(filename) && !is_patch(filename) )
... this 2nd use.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-20 16:40 ` [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
2023-02-22 12:46 ` Jan Beulich
@ 2023-02-23 13:32 ` Jan Beulich
2023-02-23 15:09 ` Oleksii
1 sibling, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2023-02-23 13:32 UTC (permalink / raw
To: Oleksii Kurochko
Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida, George Dunlap,
Julien Grall, Wei Liu, xen-devel
On 20.02.2023 17:40, Oleksii Kurochko wrote:
> --- /dev/null
> +++ b/xen/include/xen/bug.h
> @@ -0,0 +1,161 @@
> +#ifndef __XEN_BUG_H__
> +#define __XEN_BUG_H__
> +
> +#define BUG_DISP_WIDTH 24
> +#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
> +#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
> +
> +#define BUGFRAME_run_fn 0
> +#define BUGFRAME_warn 1
> +#define BUGFRAME_bug 2
> +#define BUGFRAME_assert 3
> +
> +#define BUGFRAME_NR 4
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <xen/errno.h>
> +#include <xen/lib.h>
> +#include <xen/stringify.h>
> +#include <xen/types.h>
> +
> +#include <asm/bug.h>
Looking at patch 2 onwards I came to wonder: How does that work without
you first removing stuff from the respective asm/bug.h (or adding
suitable #define-s to limit what gets defined below)? From all I can
tell the compiler should object to ...
> +#ifndef BUG_FRAME_STRUCT
> +
> +struct bug_frame {
> + signed int loc_disp:BUG_DISP_WIDTH;
> + unsigned int line_hi:BUG_LINE_HI_WIDTH;
> + signed int ptr_disp:BUG_DISP_WIDTH;
> + unsigned int line_lo:BUG_LINE_LO_WIDTH;
> + signed int msg_disp[];
> +};
... this, as asm/bug.h will have declared such a struct already. The
#define-s further down may not be a problem if what they expand to
matches in both places, but that's then still a latent trap to fall
into.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] xen: change <asm/bug.h> to <xen/bug.h>
2023-02-20 16:40 ` [PATCH v2 2/4] xen: change <asm/bug.h> to <xen/bug.h> Oleksii Kurochko
@ 2023-02-23 13:34 ` Jan Beulich
2023-02-23 15:14 ` Oleksii
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2023-02-23 13:34 UTC (permalink / raw
To: Oleksii Kurochko
Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida, Julien Grall,
Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Wei Liu,
Roger Pau Monné, xen-devel
On 20.02.2023 17:40, Oleksii Kurochko wrote:
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -24,12 +24,12 @@
>
> #ifndef __ASSEMBLY__
>
> +#include <xen/bug.h>
> #include <xen/inttypes.h>
> #include <xen/stdarg.h>
> #include <xen/types.h>
> #include <xen/xmalloc.h>
> #include <xen/string.h>
> -#include <asm/bug.h>
>
> #define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
> #define WARN_ON(p) ({ \
As just said in reply to patch 1 - I can't see how this would build
at this point. There looks to be an ordering problem; you first need
to remove from asm/bug.h what's now also available from xen/bug.h.
Possibly parts of patches 3 and 4 need to move here.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
2023-02-23 13:32 ` Jan Beulich
@ 2023-02-23 15:09 ` Oleksii
0 siblings, 0 replies; 15+ messages in thread
From: Oleksii @ 2023-02-23 15:09 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida, George Dunlap,
Julien Grall, Wei Liu, xen-devel
On Thu, 2023-02-23 at 14:32 +0100, Jan Beulich wrote:
> On 20.02.2023 17:40, Oleksii Kurochko wrote:
> > --- /dev/null
> > +++ b/xen/include/xen/bug.h
> > @@ -0,0 +1,161 @@
> > +#ifndef __XEN_BUG_H__
> > +#define __XEN_BUG_H__
> > +
> > +#define BUG_DISP_WIDTH 24
> > +#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH)
> > +#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH)
> > +
> > +#define BUGFRAME_run_fn 0
> > +#define BUGFRAME_warn 1
> > +#define BUGFRAME_bug 2
> > +#define BUGFRAME_assert 3
> > +
> > +#define BUGFRAME_NR 4
> > +
> > +#ifndef __ASSEMBLY__
> > +
> > +#include <xen/errno.h>
> > +#include <xen/lib.h>
> > +#include <xen/stringify.h>
> > +#include <xen/types.h>
> > +
> > +#include <asm/bug.h>
>
> Looking at patch 2 onwards I came to wonder: How does that work
> without
> you first removing stuff from the respective asm/bug.h (or adding
> suitable #define-s to limit what gets defined below)? From all I can
> tell the compiler should object to ...
>
> > +#ifndef BUG_FRAME_STRUCT
> > +
> > +struct bug_frame {
> > + signed int loc_disp:BUG_DISP_WIDTH;
> > + unsigned int line_hi:BUG_LINE_HI_WIDTH;
> > + signed int ptr_disp:BUG_DISP_WIDTH;
> > + unsigned int line_lo:BUG_LINE_LO_WIDTH;
> > + signed int msg_disp[];
> > +};
>
> ... this, as asm/bug.h will have declared such a struct already. The
> #define-s further down may not be a problem if what they expand to
> matches in both places, but that's then still a latent trap to fall
> into.
My fault. It doesn't work. I checked only RISC-V arch before and didn't
start all the test for patch 2...
So yeah, in patch 2 should be updated asm/bug.h headers with define
BUG_FRAME_STRUCT and remove all common parts [ BUG_DISP_WIDTH,
BUG_LINE_LO_WIDTH, BUG_LINE_HI_WIDTH, BUGFRAME_run_fn, BUGFRAME_warn,
BUGFRAME_bug, BUGFRAME_assert, BUGFRAME_NR,
{__start|__stop}_bug_frames{ | _{0-3} }[], ].
Thanks for noticing that.
>
> Jan
~ Oleksii
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 2/4] xen: change <asm/bug.h> to <xen/bug.h>
2023-02-23 13:34 ` Jan Beulich
@ 2023-02-23 15:14 ` Oleksii
0 siblings, 0 replies; 15+ messages in thread
From: Oleksii @ 2023-02-23 15:14 UTC (permalink / raw
To: Jan Beulich
Cc: Andrew Cooper, Stefano Stabellini, Gianluca Guida, Julien Grall,
Bertrand Marquis, Volodymyr Babchuk, George Dunlap, Wei Liu,
Roger Pau Monné, xen-devel
On Thu, 2023-02-23 at 14:34 +0100, Jan Beulich wrote:
> On 20.02.2023 17:40, Oleksii Kurochko wrote:
> > --- a/xen/include/xen/lib.h
> > +++ b/xen/include/xen/lib.h
> > @@ -24,12 +24,12 @@
> >
> > #ifndef __ASSEMBLY__
> >
> > +#include <xen/bug.h>
> > #include <xen/inttypes.h>
> > #include <xen/stdarg.h>
> > #include <xen/types.h>
> > #include <xen/xmalloc.h>
> > #include <xen/string.h>
> > -#include <asm/bug.h>
> >
> > #define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
> > #define WARN_ON(p) ({ \
>
> As just said in reply to patch 1 - I can't see how this would build
> at this point. There looks to be an ordering problem; you first need
> to remove from asm/bug.h what's now also available from xen/bug.h.
> Possibly parts of patches 3 and 4 need to move here.
Yeah, you are right as I answared to your reply to patch 1. I missed
that because I tested only RISC-V and didn't run tests for all
architectures.
I'll remove generic parts from patches 3 and 4 to patch 2 and will add
define BUG_FRAME_STRUCT to asm/bug.h specific headers.
>
> Jan
~ Oleksii
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-02-23 15:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-20 16:40 [PATCH v2 0/4] introduce generic implementation of macros from bug.h Oleksii Kurochko
2023-02-20 16:40 ` [PATCH v2 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME Oleksii Kurochko
2023-02-22 12:46 ` Jan Beulich
2023-02-22 16:16 ` Oleksii
2023-02-23 10:11 ` Jan Beulich
2023-02-23 12:14 ` Oleksii
2023-02-23 13:16 ` Oleksii
2023-02-23 13:25 ` Jan Beulich
2023-02-23 13:32 ` Jan Beulich
2023-02-23 15:09 ` Oleksii
2023-02-20 16:40 ` [PATCH v2 2/4] xen: change <asm/bug.h> to <xen/bug.h> Oleksii Kurochko
2023-02-23 13:34 ` Jan Beulich
2023-02-23 15:14 ` Oleksii
2023-02-20 16:40 ` [PATCH v2 3/4] xen/arm: switch ARM to use generic implementation of bug.h Oleksii Kurochko
2023-02-20 16:41 ` [PATCH v2 4/4] xen/x86: switch x86 to use generic implemetation " Oleksii Kurochko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).