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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2895 bytes --]


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@gmail.com/

~ 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@citrix.com>
> ----
> CC: Stefano Stabellini<sstabellini@kernel.org>
> CC: Julien Grall<julien@xen.org>
> CC: Volodymyr Babchuk<Volodymyr_Babchuk@epam.com>
> CC: Bertrand Marquis<bertrand.marquis@arm.com>
> CC: Michal Orzel<michal.orzel@amd.com>
> CC: Oleksii Kurochko<oleksii.kurochko@gmail.com>
> ---
>   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, "")

[-- Attachment #2: Type: text/html, Size: 4050 bytes --]