Re: [PATCH 2/4] ARM: Fix register constraints in run_in_exception_handler()

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 2/8/25 1:02 AM, Andrew Cooper wrote:

> Right now, run_in_exception_handler() takes an input in an arbitrary register,
> and clobbers BUG_FN_REG.  This causes the compiler to calculate fn in the
> wrong regsiter.
>     
Probably, we should give a chance for the patch which suggests to use GENERIC_BUG_FRAME:
  https://lore.kernel.org/xen-devel/8fdb98350ae4fc6029738d0aabe13a57e1945a50.1680086655.git.oleksii.kurochko@xxxxxxxxx/

~ Oleksii
> Instead, use `register asm()` which is the normal way of tying register
> constraints to exact registers.
>
> Bloat-o-meter reports:
>
>   ARM64:
>     Function                                     old     new   delta
>     dump_registers                               356     348      -8
>
>   ARM32:
>     ns16550_poll                                  52      48      -4
>     dump_registers                               432     428      -4
>
> The other instruction dropped in ARM64's dump_registers() is an alignment nop.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ----
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Michal Orzel <michal.orzel@xxxxxxx>
> CC: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> ---
>  xen/arch/arm/include/asm/bug.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/arm/include/asm/bug.h b/xen/arch/arm/include/asm/bug.h
> index cacaf014ab09..8bf71587bea1 100644
> --- a/xen/arch/arm/include/asm/bug.h
> +++ b/xen/arch/arm/include/asm/bug.h
> @@ -59,15 +59,15 @@ struct bug_frame {
>   * be called function in a fixed register.
>   */
>  #define  run_in_exception_handler(fn) do {                                  \
> -    asm ("mov " __stringify(BUG_FN_REG) ", %0\n"                            \
> -         "1:"BUG_INSTR"\n"                                                  \
> +    register unsigned long _fn asm (STR(BUG_FN_REG)) = (unsigned long)(fn); \
> +    asm ("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) );             \
> +         ".popsection" :: "r" (_fn) );                                      \
>  } while (0)
>  
>  #define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "")
>