* [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
2020-12-15 6:33 [PATCH v5 0/3] xen: add support for automatic debug key actions in case of crash Juergen Gross
@ 2020-12-15 6:33 ` Juergen Gross
2020-12-15 9:02 ` Jan Beulich
2020-12-15 6:33 ` [PATCH v5 2/3] xen: enable keyhandlers to work without register set specified Juergen Gross
2020-12-15 6:33 ` [PATCH v5 3/3] xen: add support for automatic debug key actions in case of crash Juergen Gross
2 siblings, 1 reply; 10+ messages in thread
From: Juergen Gross @ 2020-12-15 6:33 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Stefano Stabellini, Julien Grall,
Volodymyr Babchuk, Andrew Cooper, George Dunlap, Ian Jackson,
Jan Beulich, Wei Liu
Add support to run a function in an exception handler for Arm. Do it
the same way as on x86 via a bug_frame.
Use the same BUGFRAME_* #defines as on x86 in order to make a future
common header file more easily achievable.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4:
- new patch
V5:
- adjust BUG_FRAME() macro (Jan Beulich)
- adjust arm linker script (Jan Beulich)
- drop #ifdef x86 in common/virtual_region.c
I have verified the created bugframes are correct by inspecting the
created binary.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
xen/arch/arm/traps.c | 10 ++++++++-
xen/arch/arm/xen.lds.S | 2 ++
xen/common/virtual_region.c | 2 --
xen/include/asm-arm/bug.h | 45 +++++++++++++++++--------------------
4 files changed, 32 insertions(+), 27 deletions(-)
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 22bd1bd4c6..912b9a3d77 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1236,8 +1236,16 @@ int do_bug_frame(const struct cpu_user_regs *regs, vaddr_t pc)
if ( !bug )
return -ENOENT;
+ if ( id == BUGFRAME_run_fn )
+ {
+ void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug);
+
+ fn(regs);
+ return 0;
+ }
+
/* WARN, BUG or ASSERT: decode the filename pointer and line number. */
- filename = bug_file(bug);
+ filename = bug_ptr(bug);
if ( !is_kernel(filename) )
return -EINVAL;
fixup = strlen(filename);
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 6342ac4ead..004b182acb 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -49,6 +49,8 @@ SECTIONS
__stop_bug_frames_1 = .;
*(.bug_frames.2)
__stop_bug_frames_2 = .;
+ *(.bug_frames.3)
+ __stop_bug_frames_3 = .;
*(.rodata)
*(.rodata.*)
*(.data.rel.ro)
diff --git a/xen/common/virtual_region.c b/xen/common/virtual_region.c
index 4fbc02e35a..30b0b4ab9c 100644
--- a/xen/common/virtual_region.c
+++ b/xen/common/virtual_region.c
@@ -123,9 +123,7 @@ void __init setup_virtual_regions(const struct exception_table_entry *start,
__stop_bug_frames_0,
__stop_bug_frames_1,
__stop_bug_frames_2,
-#ifdef CONFIG_X86
__stop_bug_frames_3,
-#endif
NULL
};
diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
index 36c803357c..4610835ac3 100644
--- a/xen/include/asm-arm/bug.h
+++ b/xen/include/asm-arm/bug.h
@@ -15,65 +15,62 @@
struct bug_frame {
signed int loc_disp; /* Relative address to the bug address */
- signed int file_disp; /* Relative address to the filename */
+ signed int ptr_disp; /* Relative address to the filename or function */
signed int msg_disp; /* Relative address to the predicate (for ASSERT) */
uint16_t line; /* Line number */
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)->ptr_disp);
#define bug_line(b) ((b)->line)
#define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
-#define BUGFRAME_warn 0
-#define BUGFRAME_bug 1
-#define BUGFRAME_assert 2
+#define BUGFRAME_run_fn 0
+#define BUGFRAME_warn 1
+#define BUGFRAME_bug 2
+#define BUGFRAME_assert 3
-#define BUGFRAME_NR 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
* Xen image.
*/
-#define BUG_FRAME(type, line, file, has_msg, msg) do { \
+#define BUG_FRAME(type, line, ptr, msg) do { \
BUILD_BUG_ON((line) >> 16); \
BUILD_BUG_ON((type) >= BUGFRAME_NR); \
asm ("1:"BUG_INSTR"\n" \
- ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
- "2:\t.asciz " __stringify(file) "\n" \
- "3:\n" \
- ".if " #has_msg "\n" \
- "\t.asciz " #msg "\n" \
- ".endif\n" \
- ".popsection\n" \
- ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
- "4:\n" \
+ ".pushsection .bug_frames." __stringify(type) ", \"a\", %%progbits\n"\
+ "2:\n" \
".p2align 2\n" \
- ".long (1b - 4b)\n" \
- ".long (2b - 4b)\n" \
- ".long (3b - 4b)\n" \
+ ".long (1b - 2b)\n" \
+ ".long (%0 - 2b)\n" \
+ ".long (%1 - 2b)\n" \
".hword " __stringify(line) ", 0\n" \
- ".popsection"); \
+ ".popsection" :: "i" (ptr), "i" (msg)); \
} while (0)
-#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "")
+#define run_in_exception_handler(fn) BUG_FRAME(BUGFRAME_run_fn, 0, fn, "")
+
+#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, "")
#define BUG() do { \
- BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, 0, ""); \
+ BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
unreachable(); \
} while (0)
#define assert_failed(msg) do { \
- BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg); \
+ BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 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_2[],
+ __stop_bug_frames_3[];
#endif /* __ARM_BUG_H__ */
/*
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
2020-12-15 6:33 ` [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler() Juergen Gross
@ 2020-12-15 9:02 ` Jan Beulich
2020-12-15 9:23 ` Jürgen Groß
2020-12-15 13:39 ` Julien Grall
0 siblings, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2020-12-15 9:02 UTC (permalink / raw)
To: Juergen Gross
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Andrew Cooper, George Dunlap, Ian Jackson, Wei Liu, xen-devel
On 15.12.2020 07:33, Juergen Gross wrote:
> --- a/xen/include/asm-arm/bug.h
> +++ b/xen/include/asm-arm/bug.h
> @@ -15,65 +15,62 @@
>
> struct bug_frame {
> signed int loc_disp; /* Relative address to the bug address */
> - signed int file_disp; /* Relative address to the filename */
> + signed int ptr_disp; /* Relative address to the filename or function */
> signed int msg_disp; /* Relative address to the predicate (for ASSERT) */
> uint16_t line; /* Line number */
> 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)->ptr_disp);
> #define bug_line(b) ((b)->line)
> #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
>
> -#define BUGFRAME_warn 0
> -#define BUGFRAME_bug 1
> -#define BUGFRAME_assert 2
> +#define BUGFRAME_run_fn 0
> +#define BUGFRAME_warn 1
> +#define BUGFRAME_bug 2
> +#define BUGFRAME_assert 3
>
> -#define BUGFRAME_NR 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
> * Xen image.
> */
> -#define BUG_FRAME(type, line, file, has_msg, msg) do { \
> +#define BUG_FRAME(type, line, ptr, msg) do { \
> BUILD_BUG_ON((line) >> 16); \
> BUILD_BUG_ON((type) >= BUGFRAME_NR); \
> asm ("1:"BUG_INSTR"\n" \
> - ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
> - "2:\t.asciz " __stringify(file) "\n" \
> - "3:\n" \
> - ".if " #has_msg "\n" \
> - "\t.asciz " #msg "\n" \
> - ".endif\n" \
> - ".popsection\n" \
> - ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
> - "4:\n" \
> + ".pushsection .bug_frames." __stringify(type) ", \"a\", %%progbits\n"\
> + "2:\n" \
> ".p2align 2\n" \
> - ".long (1b - 4b)\n" \
> - ".long (2b - 4b)\n" \
> - ".long (3b - 4b)\n" \
> + ".long (1b - 2b)\n" \
> + ".long (%0 - 2b)\n" \
> + ".long (%1 - 2b)\n" \
> ".hword " __stringify(line) ", 0\n" \
> - ".popsection"); \
> + ".popsection" :: "i" (ptr), "i" (msg)); \
> } while (0)
The comment ahead of the construct now looks to be at best stale, if
not entirely pointless. The reference to %c looks quite strange here
to me anyway - I can only guess it appeared here because on x86 one
has to use %c to output constants as operands for .long and alike,
and this was then tried to use on Arm as well without there really
being a need.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
2020-12-15 9:02 ` Jan Beulich
@ 2020-12-15 9:23 ` Jürgen Groß
2020-12-15 13:39 ` Julien Grall
1 sibling, 0 replies; 10+ messages in thread
From: Jürgen Groß @ 2020-12-15 9:23 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Julien Grall, Volodymyr Babchuk,
Andrew Cooper, George Dunlap, Ian Jackson, Wei Liu, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 4166 bytes --]
On 15.12.20 10:02, Jan Beulich wrote:
> On 15.12.2020 07:33, Juergen Gross wrote:
>> --- a/xen/include/asm-arm/bug.h
>> +++ b/xen/include/asm-arm/bug.h
>> @@ -15,65 +15,62 @@
>>
>> struct bug_frame {
>> signed int loc_disp; /* Relative address to the bug address */
>> - signed int file_disp; /* Relative address to the filename */
>> + signed int ptr_disp; /* Relative address to the filename or function */
>> signed int msg_disp; /* Relative address to the predicate (for ASSERT) */
>> uint16_t line; /* Line number */
>> 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)->ptr_disp);
>> #define bug_line(b) ((b)->line)
>> #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
>>
>> -#define BUGFRAME_warn 0
>> -#define BUGFRAME_bug 1
>> -#define BUGFRAME_assert 2
>> +#define BUGFRAME_run_fn 0
>> +#define BUGFRAME_warn 1
>> +#define BUGFRAME_bug 2
>> +#define BUGFRAME_assert 3
>>
>> -#define BUGFRAME_NR 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
>> * Xen image.
>> */
>> -#define BUG_FRAME(type, line, file, has_msg, msg) do { \
>> +#define BUG_FRAME(type, line, ptr, msg) do { \
>> BUILD_BUG_ON((line) >> 16); \
>> BUILD_BUG_ON((type) >= BUGFRAME_NR); \
>> asm ("1:"BUG_INSTR"\n" \
>> - ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
>> - "2:\t.asciz " __stringify(file) "\n" \
>> - "3:\n" \
>> - ".if " #has_msg "\n" \
>> - "\t.asciz " #msg "\n" \
>> - ".endif\n" \
>> - ".popsection\n" \
>> - ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
>> - "4:\n" \
>> + ".pushsection .bug_frames." __stringify(type) ", \"a\", %%progbits\n"\
>> + "2:\n" \
>> ".p2align 2\n" \
>> - ".long (1b - 4b)\n" \
>> - ".long (2b - 4b)\n" \
>> - ".long (3b - 4b)\n" \
>> + ".long (1b - 2b)\n" \
>> + ".long (%0 - 2b)\n" \
>> + ".long (%1 - 2b)\n" \
>> ".hword " __stringify(line) ", 0\n" \
>> - ".popsection"); \
>> + ".popsection" :: "i" (ptr), "i" (msg)); \
>> } while (0)
>
> The comment ahead of the construct now looks to be at best stale, if
> not entirely pointless. The reference to %c looks quite strange here
> to me anyway - I can only guess it appeared here because on x86 one
> has to use %c to output constants as operands for .long and alike,
> and this was then tried to use on Arm as well without there really
> being a need.
Probably so.
I can remove the comment, but would like an Arm maintainer to confirm.
Juergen
[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
2020-12-15 9:02 ` Jan Beulich
2020-12-15 9:23 ` Jürgen Groß
@ 2020-12-15 13:39 ` Julien Grall
2020-12-15 13:59 ` Jan Beulich
1 sibling, 1 reply; 10+ messages in thread
From: Julien Grall @ 2020-12-15 13:39 UTC (permalink / raw)
To: Jan Beulich, Juergen Gross
Cc: Stefano Stabellini, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Ian Jackson, Wei Liu, xen-devel
Hi Jan,
On 15/12/2020 09:02, Jan Beulich wrote:
> On 15.12.2020 07:33, Juergen Gross wrote:
>> --- a/xen/include/asm-arm/bug.h
>> +++ b/xen/include/asm-arm/bug.h
>> @@ -15,65 +15,62 @@
>>
>> struct bug_frame {
>> signed int loc_disp; /* Relative address to the bug address */
>> - signed int file_disp; /* Relative address to the filename */
>> + signed int ptr_disp; /* Relative address to the filename or function */
>> signed int msg_disp; /* Relative address to the predicate (for ASSERT) */
>> uint16_t line; /* Line number */
>> 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)->ptr_disp);
>> #define bug_line(b) ((b)->line)
>> #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
>>
>> -#define BUGFRAME_warn 0
>> -#define BUGFRAME_bug 1
>> -#define BUGFRAME_assert 2
>> +#define BUGFRAME_run_fn 0
>> +#define BUGFRAME_warn 1
>> +#define BUGFRAME_bug 2
>> +#define BUGFRAME_assert 3
>>
>> -#define BUGFRAME_NR 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
>> * Xen image.
>> */
>> -#define BUG_FRAME(type, line, file, has_msg, msg) do { \
>> +#define BUG_FRAME(type, line, ptr, msg) do { \
>> BUILD_BUG_ON((line) >> 16); \
>> BUILD_BUG_ON((type) >= BUGFRAME_NR); \
>> asm ("1:"BUG_INSTR"\n" \
>> - ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
>> - "2:\t.asciz " __stringify(file) "\n" \
>> - "3:\n" \
>> - ".if " #has_msg "\n" \
>> - "\t.asciz " #msg "\n" \
>> - ".endif\n" \
>> - ".popsection\n" \
>> - ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
>> - "4:\n" \
>> + ".pushsection .bug_frames." __stringify(type) ", \"a\", %%progbits\n"\
>> + "2:\n" \
>> ".p2align 2\n" \
>> - ".long (1b - 4b)\n" \
>> - ".long (2b - 4b)\n" \
>> - ".long (3b - 4b)\n" \
>> + ".long (1b - 2b)\n" \
>> + ".long (%0 - 2b)\n" \
>> + ".long (%1 - 2b)\n" \
>> ".hword " __stringify(line) ", 0\n" \
>> - ".popsection"); \
>> + ".popsection" :: "i" (ptr), "i" (msg)); \
>> } while (0)
>
> The comment ahead of the construct now looks to be at best stale, if
> not entirely pointless. The reference to %c looks quite strange here
> to me anyway - I can only guess it appeared here because on x86 one
> has to use %c to output constants as operands for .long and alike,
> and this was then tried to use on Arm as well without there really
> being a need.
Well, %c is one the reason why we can't have a common BUG_FRAME
implementation. So I would like to retain this information before
someone wants to consolidate the code and missed this issue.
Regarding the mergeable string section, I agree that the comment in now
stale. However, could someone confirm that "i" will still retain the
same behavior as using mergeable string sections?
Cheers,
--
Julien Grall
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
2020-12-15 13:39 ` Julien Grall
@ 2020-12-15 13:59 ` Jan Beulich
2020-12-21 16:50 ` Julien Grall
0 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2020-12-15 13:59 UTC (permalink / raw)
To: Julien Grall
Cc: Stefano Stabellini, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Ian Jackson, Wei Liu, xen-devel, Juergen Gross
On 15.12.2020 14:39, Julien Grall wrote:
> On 15/12/2020 09:02, Jan Beulich wrote:
>> On 15.12.2020 07:33, Juergen Gross wrote:
>>> --- a/xen/include/asm-arm/bug.h
>>> +++ b/xen/include/asm-arm/bug.h
>>> @@ -15,65 +15,62 @@
>>>
>>> struct bug_frame {
>>> signed int loc_disp; /* Relative address to the bug address */
>>> - signed int file_disp; /* Relative address to the filename */
>>> + signed int ptr_disp; /* Relative address to the filename or function */
>>> signed int msg_disp; /* Relative address to the predicate (for ASSERT) */
>>> uint16_t line; /* Line number */
>>> 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)->ptr_disp);
>>> #define bug_line(b) ((b)->line)
>>> #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
>>>
>>> -#define BUGFRAME_warn 0
>>> -#define BUGFRAME_bug 1
>>> -#define BUGFRAME_assert 2
>>> +#define BUGFRAME_run_fn 0
>>> +#define BUGFRAME_warn 1
>>> +#define BUGFRAME_bug 2
>>> +#define BUGFRAME_assert 3
>>>
>>> -#define BUGFRAME_NR 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
>>> * Xen image.
>>> */
>>> -#define BUG_FRAME(type, line, file, has_msg, msg) do { \
>>> +#define BUG_FRAME(type, line, ptr, msg) do { \
>>> BUILD_BUG_ON((line) >> 16); \
>>> BUILD_BUG_ON((type) >= BUGFRAME_NR); \
>>> asm ("1:"BUG_INSTR"\n" \
>>> - ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
>>> - "2:\t.asciz " __stringify(file) "\n" \
>>> - "3:\n" \
>>> - ".if " #has_msg "\n" \
>>> - "\t.asciz " #msg "\n" \
>>> - ".endif\n" \
>>> - ".popsection\n" \
>>> - ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
>>> - "4:\n" \
>>> + ".pushsection .bug_frames." __stringify(type) ", \"a\", %%progbits\n"\
>>> + "2:\n" \
>>> ".p2align 2\n" \
>>> - ".long (1b - 4b)\n" \
>>> - ".long (2b - 4b)\n" \
>>> - ".long (3b - 4b)\n" \
>>> + ".long (1b - 2b)\n" \
>>> + ".long (%0 - 2b)\n" \
>>> + ".long (%1 - 2b)\n" \
>>> ".hword " __stringify(line) ", 0\n" \
>>> - ".popsection"); \
>>> + ".popsection" :: "i" (ptr), "i" (msg)); \
>>> } while (0)
>>
>> The comment ahead of the construct now looks to be at best stale, if
>> not entirely pointless. The reference to %c looks quite strange here
>> to me anyway - I can only guess it appeared here because on x86 one
>> has to use %c to output constants as operands for .long and alike,
>> and this was then tried to use on Arm as well without there really
>> being a need.
>
> Well, %c is one the reason why we can't have a common BUG_FRAME
> implementation. So I would like to retain this information before
> someone wants to consolidate the code and missed this issue.
Fair enough, albeit I guess this then could do with re-wording.
> Regarding the mergeable string section, I agree that the comment in now
> stale. However, could someone confirm that "i" will still retain the
> same behavior as using mergeable string sections?
That's depend on compiler settings / behavior.
Jan
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
2020-12-15 13:59 ` Jan Beulich
@ 2020-12-21 16:50 ` Julien Grall
2020-12-22 5:48 ` Jürgen Groß
0 siblings, 1 reply; 10+ messages in thread
From: Julien Grall @ 2020-12-21 16:50 UTC (permalink / raw)
To: Jan Beulich
Cc: Stefano Stabellini, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Ian Jackson, Wei Liu, xen-devel, Juergen Gross
Hi Jan,
On 15/12/2020 13:59, Jan Beulich wrote:
> On 15.12.2020 14:39, Julien Grall wrote:
>> On 15/12/2020 09:02, Jan Beulich wrote:
>>> On 15.12.2020 07:33, Juergen Gross wrote:
>>>> --- a/xen/include/asm-arm/bug.h
>>>> +++ b/xen/include/asm-arm/bug.h
>>>> @@ -15,65 +15,62 @@
>>>>
>>>> struct bug_frame {
>>>> signed int loc_disp; /* Relative address to the bug address */
>>>> - signed int file_disp; /* Relative address to the filename */
>>>> + signed int ptr_disp; /* Relative address to the filename or function */
>>>> signed int msg_disp; /* Relative address to the predicate (for ASSERT) */
>>>> uint16_t line; /* Line number */
>>>> 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)->ptr_disp);
>>>> #define bug_line(b) ((b)->line)
>>>> #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
>>>>
>>>> -#define BUGFRAME_warn 0
>>>> -#define BUGFRAME_bug 1
>>>> -#define BUGFRAME_assert 2
>>>> +#define BUGFRAME_run_fn 0
>>>> +#define BUGFRAME_warn 1
>>>> +#define BUGFRAME_bug 2
>>>> +#define BUGFRAME_assert 3
>>>>
>>>> -#define BUGFRAME_NR 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
>>>> * Xen image.
>>>> */
>>>> -#define BUG_FRAME(type, line, file, has_msg, msg) do { \
>>>> +#define BUG_FRAME(type, line, ptr, msg) do { \
>>>> BUILD_BUG_ON((line) >> 16); \
>>>> BUILD_BUG_ON((type) >= BUGFRAME_NR); \
>>>> asm ("1:"BUG_INSTR"\n" \
>>>> - ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \
>>>> - "2:\t.asciz " __stringify(file) "\n" \
>>>> - "3:\n" \
>>>> - ".if " #has_msg "\n" \
>>>> - "\t.asciz " #msg "\n" \
>>>> - ".endif\n" \
>>>> - ".popsection\n" \
>>>> - ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
>>>> - "4:\n" \
>>>> + ".pushsection .bug_frames." __stringify(type) ", \"a\", %%progbits\n"\
>>>> + "2:\n" \
>>>> ".p2align 2\n" \
>>>> - ".long (1b - 4b)\n" \
>>>> - ".long (2b - 4b)\n" \
>>>> - ".long (3b - 4b)\n" \
>>>> + ".long (1b - 2b)\n" \
>>>> + ".long (%0 - 2b)\n" \
>>>> + ".long (%1 - 2b)\n" \
>>>> ".hword " __stringify(line) ", 0\n" \
>>>> - ".popsection"); \
>>>> + ".popsection" :: "i" (ptr), "i" (msg)); \
>>>> } while (0)
>>>
>>> The comment ahead of the construct now looks to be at best stale, if
>>> not entirely pointless. The reference to %c looks quite strange here
>>> to me anyway - I can only guess it appeared here because on x86 one
>>> has to use %c to output constants as operands for .long and alike,
>>> and this was then tried to use on Arm as well without there really
>>> being a need.
>>
>> Well, %c is one the reason why we can't have a common BUG_FRAME
>> implementation. So I would like to retain this information before
>> someone wants to consolidate the code and missed this issue.
>
> Fair enough, albeit I guess this then could do with re-wording.
I agree.
>
>> Regarding the mergeable string section, I agree that the comment in now
>> stale. However, could someone confirm that "i" will still retain the
>> same behavior as using mergeable string sections?
>
> That's depend on compiler settings / behavior.
Ok. I wanted to see the difference between before and after but it looks
like I can't compile Xen after applying the patch:
In file included from
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:23:0,
from
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/bitmap.h:6,
from bitmap.c:10:
bitmap.c: In function ‘bitmap_allocate_region’:
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
error: asm operand 0 probably doesn’t match constraints [-Werror]
asm ("1:"BUG_INSTR"\n"
\
^
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
note: in expansion of macro ‘BUG_FRAME’
BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
^~~~~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
note: in expansion of macro ‘BUG’
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
^~~
bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
BUG_ON(pages > BITS_PER_LONG);
^~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
error: asm operand 1 probably doesn’t match constraints [-Werror]
asm ("1:"BUG_INSTR"\n"
\
^
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
note: in expansion of macro ‘BUG_FRAME’
BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
^~~~~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
note: in expansion of macro ‘BUG’
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
^~~
bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
BUG_ON(pages > BITS_PER_LONG);
^~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
error: impossible constraint in ‘asm’
asm ("1:"BUG_INSTR"\n"
\
^
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
note: in expansion of macro ‘BUG_FRAME’
BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
^~~~~~~~~
/home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
note: in expansion of macro ‘BUG’
#define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
^~~
bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
BUG_ON(pages > BITS_PER_LONG);
^~~~~~
cc1: all warnings being treated as errors
I am using GCC 7.5.0 built by Linaro (cross-compiler). Native
compilation with GCC 10.2.1 leads to the same error.
@Juergen, which compiler did you use?
--
Julien Grall
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()
2020-12-21 16:50 ` Julien Grall
@ 2020-12-22 5:48 ` Jürgen Groß
0 siblings, 0 replies; 10+ messages in thread
From: Jürgen Groß @ 2020-12-22 5:48 UTC (permalink / raw)
To: Julien Grall, Jan Beulich
Cc: Stefano Stabellini, Volodymyr Babchuk, Andrew Cooper,
George Dunlap, Ian Jackson, Wei Liu, xen-devel
[-- Attachment #1.1.1: Type: text/plain, Size: 9690 bytes --]
On 21.12.20 17:50, Julien Grall wrote:
> Hi Jan,
>
> On 15/12/2020 13:59, Jan Beulich wrote:
>> On 15.12.2020 14:39, Julien Grall wrote:
>>> On 15/12/2020 09:02, Jan Beulich wrote:
>>>> On 15.12.2020 07:33, Juergen Gross wrote:
>>>>> --- a/xen/include/asm-arm/bug.h
>>>>> +++ b/xen/include/asm-arm/bug.h
>>>>> @@ -15,65 +15,62 @@
>>>>> struct bug_frame {
>>>>> signed int loc_disp; /* Relative address to the bug
>>>>> address */
>>>>> - signed int file_disp; /* Relative address to the filename */
>>>>> + signed int ptr_disp; /* Relative address to the filename or
>>>>> function */
>>>>> signed int msg_disp; /* Relative address to the predicate
>>>>> (for ASSERT) */
>>>>> uint16_t line; /* Line number */
>>>>> 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)->ptr_disp);
>>>>> #define bug_line(b) ((b)->line)
>>>>> #define bug_msg(b) ((const char *)(b) + (b)->msg_disp)
>>>>> -#define BUGFRAME_warn 0
>>>>> -#define BUGFRAME_bug 1
>>>>> -#define BUGFRAME_assert 2
>>>>> +#define BUGFRAME_run_fn 0
>>>>> +#define BUGFRAME_warn 1
>>>>> +#define BUGFRAME_bug 2
>>>>> +#define BUGFRAME_assert 3
>>>>> -#define BUGFRAME_NR 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
>>>>> * Xen image.
>>>>> */
>>>>> -#define BUG_FRAME(type, line, file, has_msg, msg) do
>>>>> { \
>>>>> +#define BUG_FRAME(type, line, ptr, msg) do
>>>>> { \
>>>>> BUILD_BUG_ON((line) >>
>>>>> 16); \
>>>>> BUILD_BUG_ON((type) >=
>>>>> BUGFRAME_NR); \
>>>>> asm
>>>>> ("1:"BUG_INSTR"\n" \
>>>>> - ".pushsection .rodata.str, \"aMS\", %progbits,
>>>>> 1\n" \
>>>>> - "2:\t.asciz " __stringify(file)
>>>>> "\n" \
>>>>> -
>>>>> "3:\n" \
>>>>> - ".if " #has_msg
>>>>> "\n" \
>>>>> - "\t.asciz " #msg
>>>>> "\n" \
>>>>> -
>>>>> ".endif\n" \
>>>>> -
>>>>> ".popsection\n" \
>>>>> - ".pushsection .bug_frames." __stringify(type) ", \"a\",
>>>>> %progbits\n"\
>>>>> -
>>>>> "4:\n" \
>>>>> + ".pushsection .bug_frames." __stringify(type) ", \"a\",
>>>>> %%progbits\n"\
>>>>> +
>>>>> "2:\n" \
>>>>> ".p2align
>>>>> 2\n" \
>>>>> - ".long (1b -
>>>>> 4b)\n" \
>>>>> - ".long (2b -
>>>>> 4b)\n" \
>>>>> - ".long (3b -
>>>>> 4b)\n" \
>>>>> + ".long (1b -
>>>>> 2b)\n" \
>>>>> + ".long (%0 -
>>>>> 2b)\n" \
>>>>> + ".long (%1 -
>>>>> 2b)\n" \
>>>>> ".hword " __stringify(line) ",
>>>>> 0\n" \
>>>>> -
>>>>> ".popsection"); \
>>>>> + ".popsection" :: "i" (ptr), "i"
>>>>> (msg)); \
>>>>> } while (0)
>>>>
>>>> The comment ahead of the construct now looks to be at best stale, if
>>>> not entirely pointless. The reference to %c looks quite strange here
>>>> to me anyway - I can only guess it appeared here because on x86 one
>>>> has to use %c to output constants as operands for .long and alike,
>>>> and this was then tried to use on Arm as well without there really
>>>> being a need.
>>>
>>> Well, %c is one the reason why we can't have a common BUG_FRAME
>>> implementation. So I would like to retain this information before
>>> someone wants to consolidate the code and missed this issue.
>>
>> Fair enough, albeit I guess this then could do with re-wording.
>
> I agree.
>
>>
>>> Regarding the mergeable string section, I agree that the comment in now
>>> stale. However, could someone confirm that "i" will still retain the
>>> same behavior as using mergeable string sections?
>>
>> That's depend on compiler settings / behavior.
>
> Ok. I wanted to see the difference between before and after but it looks
> like I can't compile Xen after applying the patch:
>
>
> In file included from
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:23:0,
> from
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/bitmap.h:6,
> from bitmap.c:10:
> bitmap.c: In function ‘bitmap_allocate_region’:
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
> error: asm operand 0 probably doesn’t match constraints [-Werror]
> asm ("1:"BUG_INSTR"\n" \
> ^
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
> note: in expansion of macro ‘BUG_FRAME’
> BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
> ^~~~~~~~~
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
> note: in expansion of macro ‘BUG’
> #define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
> ^~~
> bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
> BUG_ON(pages > BITS_PER_LONG);
> ^~~~~~
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
> error: asm operand 1 probably doesn’t match constraints [-Werror]
> asm ("1:"BUG_INSTR"\n" \
> ^
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
> note: in expansion of macro ‘BUG_FRAME’
> BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
> ^~~~~~~~~
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
> note: in expansion of macro ‘BUG’
> #define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
> ^~~
> bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
> BUG_ON(pages > BITS_PER_LONG);
> ^~~~~~
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:44:5:
> error: impossible constraint in ‘asm’
> asm ("1:"BUG_INSTR"\n" \
> ^
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/asm/bug.h:60:5:
> note: in expansion of macro ‘BUG_FRAME’
> BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, ""); \
> ^~~~~~~~~
> /home/ANT.AMAZON.COM/jgrall/works/oss/xen/xen/include/xen/lib.h:25:42:
> note: in expansion of macro ‘BUG’
> #define BUG_ON(p) do { if (unlikely(p)) BUG(); } while (0)
> ^~~
> bitmap.c:330:2: note: in expansion of macro ‘BUG_ON’
> BUG_ON(pages > BITS_PER_LONG);
> ^~~~~~
> cc1: all warnings being treated as errors
>
> I am using GCC 7.5.0 built by Linaro (cross-compiler). Native
> compilation with GCC 10.2.1 leads to the same error.
>
> @Juergen, which compiler did you use?
>
gcc 7.4.0 aarch64 cross-compiler (SUSE)
Juergen
[-- Attachment #1.1.2: OpenPGP_0xB0DE9DD628BF132F.asc --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 2/3] xen: enable keyhandlers to work without register set specified
2020-12-15 6:33 [PATCH v5 0/3] xen: add support for automatic debug key actions in case of crash Juergen Gross
2020-12-15 6:33 ` [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler() Juergen Gross
@ 2020-12-15 6:33 ` Juergen Gross
2020-12-15 6:33 ` [PATCH v5 3/3] xen: add support for automatic debug key actions in case of crash Juergen Gross
2 siblings, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2020-12-15 6:33 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Andrew Cooper, George Dunlap, Ian Jackson,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
There are only two keyhandlers which make use of the cpu_user_regs
struct passed to them. In order to be able to call any keyhandler in
non-interrupt contexts, too, modify those two handlers to copy with a
NULL regs pointer by using run_in_exception_handler() in that case.
Suggested-by: Julien Grall <julien@xen.org>
Signed-off-by: Juergen Gross <jgross@suse.com>
---
V4:
- new patch
---
xen/common/keyhandler.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 68364e987d..38020a1360 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -181,7 +181,10 @@ static void dump_registers(unsigned char key, struct cpu_user_regs *regs)
cpumask_copy(&dump_execstate_mask, &cpu_online_map);
/* Get local execution state out immediately, in case we get stuck. */
- dump_execstate(regs);
+ if ( regs )
+ dump_execstate(regs);
+ else
+ run_in_exception_handler(dump_execstate);
/* Alt. handling: remaining CPUs are dumped asynchronously one-by-one. */
if ( alt_key_handling )
@@ -481,15 +484,23 @@ static void run_all_keyhandlers(unsigned char key, struct cpu_user_regs *regs)
tasklet_schedule(&run_all_keyhandlers_tasklet);
}
-static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
+static void do_debugger_trap_fatal(struct cpu_user_regs *regs)
{
- printk("'%c' pressed -> trapping into debugger\n", key);
(void)debugger_trap_fatal(0xf001, regs);
/* Prevent tail call optimisation, which confuses xendbg. */
barrier();
}
+static void do_debug_key(unsigned char key, struct cpu_user_regs *regs)
+{
+ printk("'%c' pressed -> trapping into debugger\n", key);
+ if ( regs )
+ do_debugger_trap_fatal(regs);
+ else
+ run_in_exception_handler(do_debugger_trap_fatal);
+}
+
static void do_toggle_alt_key(unsigned char key, struct cpu_user_regs *regs)
{
alt_key_handling = !alt_key_handling;
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v5 3/3] xen: add support for automatic debug key actions in case of crash
2020-12-15 6:33 [PATCH v5 0/3] xen: add support for automatic debug key actions in case of crash Juergen Gross
2020-12-15 6:33 ` [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler() Juergen Gross
2020-12-15 6:33 ` [PATCH v5 2/3] xen: enable keyhandlers to work without register set specified Juergen Gross
@ 2020-12-15 6:33 ` Juergen Gross
2 siblings, 0 replies; 10+ messages in thread
From: Juergen Gross @ 2020-12-15 6:33 UTC (permalink / raw)
To: xen-devel
Cc: Juergen Gross, Andrew Cooper, George Dunlap, Ian Jackson,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
When the host crashes it would sometimes be nice to have additional
debug data available which could be produced via debug keys, but
halting the server for manual intervention might be impossible due to
the need to reboot/kexec rather sooner than later.
Add support for automatic debug key actions in case of crashes which
can be activated via boot- or runtime-parameter.
Depending on the type of crash the desired data might be different, so
support different settings for the possible types of crashes.
The parameter is "crash-debug" with the following syntax:
crash-debug-<type>=<string>
with <type> being one of:
panic, hwdom, watchdog, kexeccmd, debugkey
and <string> a sequence of debug key characters with '+' having the
special semantics of a 10 millisecond pause.
So "crash-debug-watchdog=0+0qr" would result in special output in case
of watchdog triggered crash (dom0 state, 10 ms pause, dom0 state,
domain info, run queues).
Don't call key handlers in early boot, as some (e.g. for 'd') require
some initializations to be finished, like scheduler or idle domain.
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
V2:
- switched special character '.' to '+' (Jan Beulich)
- 10 ms instead of 1 s pause (Jan Beulich)
- added more text to the boot parameter description (Jan Beulich)
V3:
- added const (Jan Beulich)
- thorough test of crash reason parameter (Jan Beulich)
- kexeccmd case should depend on CONFIG_KEXEC (Jan Beulich)
- added dummy get_irq_regs() helper on Arm
V4:
- call keyhandlers with NULL for regs
- use ARRAY_SIZE() (Jan Beulich)
- don't activate handlers in early boot (Jan Beulich)
- avoid recursion
- extend documentation a bit
V5:
- move recursion check down a little bit (Jan Beulich)
---
docs/misc/xen-command-line.pandoc | 41 +++++++++++++++++++++++
xen/common/kexec.c | 8 +++--
xen/common/keyhandler.c | 55 +++++++++++++++++++++++++++++++
xen/common/shutdown.c | 4 +--
xen/drivers/char/console.c | 2 +-
xen/include/xen/kexec.h | 10 ++++--
xen/include/xen/keyhandler.h | 10 ++++++
7 files changed, 122 insertions(+), 8 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index b4a0d60c11..e4c0a144fc 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -574,6 +574,43 @@ reduction of features at Xen's disposal to manage guests.
### cpuinfo (x86)
> `= <boolean>`
+### crash-debug-debugkey
+### crash-debug-hwdom
+### crash-debug-kexeccmd
+### crash-debug-panic
+### crash-debug-watchdog
+> `= <string>`
+
+> Can be modified at runtime
+
+Specify debug-key actions in cases of crashes. Each of the parameters applies
+to a different crash reason. The `<string>` is a sequence of debug key
+characters, with `+` having the special meaning of a 10 millisecond pause.
+
+`crash-debug-debugkey` will be used for crashes induced by the `C` debug
+key (i.e. manually induced crash).
+
+`crash-debug-hwdom` denotes a crash of dom0.
+
+`crash-debug-kexeccmd` is an explicit request of dom0 to continue with the
+kdump kernel via kexec. Only available on hypervisors built with CONFIG_KEXEC.
+
+`crash-debug-panic` is a crash of the hypervisor.
+
+`crash-debug-watchdog` is a crash due to the watchdog timer expiring.
+
+It should be noted that dumping diagnosis data to the console can fail in
+multiple ways (missing data, hanging system, ...) depending on the reason
+of the crash, which might have left the hypervisor in a bad state. In case
+a debug-key action leads to another crash, recursion will be avoided, so no
+additional debug-key actions will be performed in this case. A crash in the
+early boot phase will not result in any debug-key action, as the system
+might not yet be in a state where the handlers can work.
+
+So e.g. `crash-debug-watchdog=0+0r` would dump dom0 state twice with 10
+milliseconds between the two state dumps, followed by the run queues of the
+hypervisor, if the system crashes due to a watchdog timeout.
+
### crashinfo_maxaddr
> `= <size>`
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 52cdc4ebc3..ebeee6405a 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -373,10 +373,12 @@ static int kexec_common_shutdown(void)
return 0;
}
-void kexec_crash(void)
+void kexec_crash(enum crash_reason reason)
{
int pos;
+ keyhandler_crash_action(reason);
+
pos = (test_bit(KEXEC_FLAG_CRASH_POS, &kexec_flags) != 0);
if ( !test_bit(KEXEC_IMAGE_CRASH_BASE + pos, &kexec_flags) )
return;
@@ -409,7 +411,7 @@ static long kexec_reboot(void *_image)
static void do_crashdump_trigger(unsigned char key)
{
printk("'%c' pressed -> triggering crashdump\n", key);
- kexec_crash();
+ kexec_crash(CRASHREASON_DEBUGKEY);
printk(" * no crash kernel loaded!\n");
}
@@ -840,7 +842,7 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
ret = continue_hypercall_on_cpu(0, kexec_reboot, image);
break;
case KEXEC_TYPE_CRASH:
- kexec_crash(); /* Does not return */
+ kexec_crash(CRASHREASON_KEXECCMD); /* Does not return */
break;
}
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 38020a1360..8b9f378371 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -3,7 +3,9 @@
*/
#include <asm/regs.h>
+#include <xen/delay.h>
#include <xen/keyhandler.h>
+#include <xen/param.h>
#include <xen/shutdown.h>
#include <xen/event.h>
#include <xen/console.h>
@@ -518,6 +520,59 @@ void __init initialize_keytable(void)
}
}
+#define CRASHACTION_SIZE 32
+static char crash_debug_panic[CRASHACTION_SIZE];
+string_runtime_param("crash-debug-panic", crash_debug_panic);
+static char crash_debug_hwdom[CRASHACTION_SIZE];
+string_runtime_param("crash-debug-hwdom", crash_debug_hwdom);
+static char crash_debug_watchdog[CRASHACTION_SIZE];
+string_runtime_param("crash-debug-watchdog", crash_debug_watchdog);
+#ifdef CONFIG_KEXEC
+static char crash_debug_kexeccmd[CRASHACTION_SIZE];
+string_runtime_param("crash-debug-kexeccmd", crash_debug_kexeccmd);
+#else
+#define crash_debug_kexeccmd NULL
+#endif
+static char crash_debug_debugkey[CRASHACTION_SIZE];
+string_runtime_param("crash-debug-debugkey", crash_debug_debugkey);
+
+void keyhandler_crash_action(enum crash_reason reason)
+{
+ static const char *const crash_action[] = {
+ [CRASHREASON_PANIC] = crash_debug_panic,
+ [CRASHREASON_HWDOM] = crash_debug_hwdom,
+ [CRASHREASON_WATCHDOG] = crash_debug_watchdog,
+ [CRASHREASON_KEXECCMD] = crash_debug_kexeccmd,
+ [CRASHREASON_DEBUGKEY] = crash_debug_debugkey,
+ };
+ static bool ignore;
+ const char *action;
+
+ /* Some handlers are not functional too early. */
+ if ( system_state < SYS_STATE_smp_boot )
+ return;
+
+ if ( (unsigned int)reason >= ARRAY_SIZE(crash_action) )
+ return;
+ action = crash_action[reason];
+ if ( !action )
+ return;
+
+ /* Avoid recursion. */
+ if ( ignore )
+ return;
+ ignore = true;
+
+ while ( *action )
+ {
+ if ( *action == '+' )
+ mdelay(10);
+ else
+ handle_keypress(*action, NULL);
+ action++;
+ }
+}
+
/*
* Local variables:
* mode: C
diff --git a/xen/common/shutdown.c b/xen/common/shutdown.c
index 912593915b..abde48aa4c 100644
--- a/xen/common/shutdown.c
+++ b/xen/common/shutdown.c
@@ -43,7 +43,7 @@ void hwdom_shutdown(u8 reason)
case SHUTDOWN_crash:
debugger_trap_immediate();
printk("Hardware Dom%u crashed: ", hardware_domain->domain_id);
- kexec_crash();
+ kexec_crash(CRASHREASON_HWDOM);
maybe_reboot();
break; /* not reached */
@@ -56,7 +56,7 @@ void hwdom_shutdown(u8 reason)
case SHUTDOWN_watchdog:
printk("Hardware Dom%u shutdown: watchdog rebooting machine\n",
hardware_domain->domain_id);
- kexec_crash();
+ kexec_crash(CRASHREASON_WATCHDOG);
machine_restart(0);
break; /* not reached */
diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
index 861ad53a8f..acec277f5e 100644
--- a/xen/drivers/char/console.c
+++ b/xen/drivers/char/console.c
@@ -1271,7 +1271,7 @@ void panic(const char *fmt, ...)
debugger_trap_immediate();
- kexec_crash();
+ kexec_crash(CRASHREASON_PANIC);
if ( opt_noreboot )
machine_halt();
diff --git a/xen/include/xen/kexec.h b/xen/include/xen/kexec.h
index e85ba16405..9f7a912e97 100644
--- a/xen/include/xen/kexec.h
+++ b/xen/include/xen/kexec.h
@@ -1,6 +1,8 @@
#ifndef __XEN_KEXEC_H__
#define __XEN_KEXEC_H__
+#include <xen/keyhandler.h>
+
#ifdef CONFIG_KEXEC
#include <public/kexec.h>
@@ -48,7 +50,7 @@ void machine_kexec_unload(struct kexec_image *image);
void machine_kexec_reserved(xen_kexec_reserve_t *reservation);
void machine_reboot_kexec(struct kexec_image *image);
void machine_kexec(struct kexec_image *image);
-void kexec_crash(void);
+void kexec_crash(enum crash_reason reason);
void kexec_crash_save_cpu(void);
struct crash_xen_info *kexec_crash_save_info(void);
void machine_crash_shutdown(void);
@@ -82,7 +84,11 @@ void vmcoreinfo_append_str(const char *fmt, ...)
#define kexecing 0
static inline void kexec_early_calculations(void) {}
-static inline void kexec_crash(void) {}
+static inline void kexec_crash(enum crash_reason reason)
+{
+ keyhandler_crash_action(reason);
+}
+
static inline void kexec_crash_save_cpu(void) {}
static inline void set_kexec_crash_area_size(u64 system_ram) {}
diff --git a/xen/include/xen/keyhandler.h b/xen/include/xen/keyhandler.h
index 5131e86cbc..9c5830a037 100644
--- a/xen/include/xen/keyhandler.h
+++ b/xen/include/xen/keyhandler.h
@@ -48,4 +48,14 @@ void register_irq_keyhandler(unsigned char key,
/* Inject a keypress into the key-handling subsystem. */
extern void handle_keypress(unsigned char key, struct cpu_user_regs *regs);
+enum crash_reason {
+ CRASHREASON_PANIC,
+ CRASHREASON_HWDOM,
+ CRASHREASON_WATCHDOG,
+ CRASHREASON_KEXECCMD,
+ CRASHREASON_DEBUGKEY,
+};
+
+void keyhandler_crash_action(enum crash_reason reason);
+
#endif /* __XEN_KEYHANDLER_H__ */
--
2.26.2
^ permalink raw reply related [flat|nested] 10+ messages in thread