Re: [PATCH v5 1/3] xen/arm: add support for run_in_exception_handler()

[Jan Beulich <jbeulich@xxxxxxxx>]

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