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).
>
> 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@xxxxxxxx>
> ---
> V4:
> - new patch
>
> V5:
> - adjust BUG_FRAME() macro (Jan Beulich)
> - adjust arm linker script (Jan Beulich)
> - drop #ifdef x86 in common/virtual_region.c
>
> V6:
> - use register for function address (Arm32 build failure noticed by
>    Julien Grall)
Thank you for trying to sort out the issue on Arm32 :).

> +/*
> + * 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.
There are a few uses of "i" in Linux Arm32 (such as jump label), 
therefore I am pretty confident "i" works at least in some situation.
I did some more digging this afternoon. Our use of "i" is very similar 
to Linux, so I decided to look at the GCC flags used.
It turns out that Linux will always build with -fno-pie on Arm (either 
32 or 64) whereas Xen will let the compiler to decide (PIE is enabled by 
on my compiler).
I wrote a small example to test the issue (based on Linux static key)

static void test(void)
{
}

int main(void)
{
    asm volatile (".pushsection .bug_frames, \"aw\"\n"
              ".quad %0\n"
              ".popsection\n"
              :: "i"(test));

    return 0;
}

If I add -fno-pie on the command, the constraint error disappears.

On the previous version, you rewrote that didn't see any error. Is it 
possible that your compiler is disabling PIE by default?
I think we need to code to be position independent for at least UEFI. I 
also have plan to look at selecting the Xen virtual address at boot time 
(this is necessary to solve some memory issue on Arm).
From a quick test, if I use -fno-pie -fpic, then the snipped above will 
build fine. But it is not entirely clear whether the code would still be 
position independent.
I need to have a look how Linux is dealing with KASLR given that 
-fno-pie is used...
> + */
> +#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)
My concern with this approach is we are going to clobber multiple 
registers. On Arm64, this code will be replaced by:
    13cc:       90000001        adrp    x1, 0 <show_execution_state>
    13d0:       91000021        add     x1, x1, #0x0
    13d4:       aa0103e0        mov     x0, x1
    13d8:       d4200020        brk     #0x1

I guess we can optimize it to just clobber one register. Do we expect 
the function executed to rely/replace the content of the registers?
Cheers,

--
Julien Grall