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

[Julien Grall <julien@xxxxxxx>]

Hi Juergen,

On 16/01/2021 10:33, Juergen Gross wrote:
> Add support to run a function in an exception handler for Arm. Do it
> as on x86 via a bug_frame, but pass the function pointer via a
> register (this needs to be done that way, because inline asm support
> for 32-bit Arm lacks the needed functionality to reference an
> arbitrary function via the bugframe).
I was going to commit the series, but then realized the commit message 
and comment needs some tweaking because technically GCC is supporting 
'i' (I managed to get it working with -fno-pie).
So how about:

"This is needed to be done that way because GCC complains about invalid 
constraint when using a function pointer with "i" and PIE is enabled 
(default on most of GCC shipped with distro). Clang happily accepts it, 
so it may be a bug in GCC."

> +/*
> + * Unfortunately gcc for arm 32-bit doesn't support the "i" constraint, so
> + * the easiest way to implement run_in_exception_handler() is to pass the
> + * to be called function in a fixed register.
> + */
This comment should also be updated.

I can update both while committing if you are happy with the change.

> +#define  run_in_exception_handler(fn) do {                                  \
> +    asm ("mov " __stringify(BUG_FN_REG) ", %0\n"                            \
> +         "1:"BUG_INSTR"\n"                                                  \
> +         ".pushsection .bug_frames." __stringify(BUGFRAME_run_fn) ","       \
> +         "             \"a\", %%progbits\n"                                 \
> +         "2:\n"                                                             \
> +         ".p2align 2\n"                                                     \
> +         ".long (1b - 2b)\n"                                                \
> +         ".long 0, 0, 0\n"                                                  \
> +         ".popsection" :: "r" (fn) : __stringify(BUG_FN_REG) );             \
> +} while (0)
> +
>   #define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "")
>   
>   #define BUG() do {                                              \
> @@ -73,7 +91,8 @@ struct 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_2[],
> +                              __stop_bug_frames_3[];
>   
>   #endif /* __ARM_BUG_H__ */
>   /*
Cheers,

--
Julien Grall