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

[Jürgen Groß <jgross@xxxxxxxx>]

On 14.12.20 12:21, Jürgen Groß wrote:
> On 14.12.20 12:14, Julien Grall wrote:
> > Hi Juergen,
> >
> > On 14/12/2020 10:51, Jürgen Groß wrote:
> > > On 14.12.20 11:17, Julien Grall wrote:
> > > > Hi Juergen,
> > > >
> > > > On 14/12/2020 07:56, Juergen Gross wrote:
> > > > > Add support to run a function in an exception handler for Arm. Do it
> > > > > the same way as on x86 via a bug_frame.
> > > > >
> > > > > Unfortunately inline assembly on Arm seems to be less capable than on
> > > > > x86, leading to functions called via run_in_exception_handler() having
> > > > > to be globally visible.
> > > > Jan already commented on this, so I am not going to comment again.
> > > Maybe I can ask some Arm specific question related to this:
> > >
> > > In my experiments the only working solution was using the "i" constraint
> > > for the function pointer. Do you know whether this is supported for all
> > > gcc versions we care about?
> > I don't know for sure. However, Linux has been using "i" since 2012. 
> > So I would assume it ought to be fine for all the version we care.
> > > Or is there another way to achieve the desired functionality? I'm using
> > > now the following macros:
> > >
> > > #define BUG_FRAME_run_fn(fn) do {                                      \
> > >      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 - 
> > > 2b)\n"                                           \
> > >           ".long 
> > > 0\n"                                                   \
> > >           ".hword 0, 
> > > 0\n"                                               \
> > >           ".popsection" :: "i" 
> > > (fn));                                   \
> > > } while (0)
> > May I ask why we need a new macro?
> Using a common one might be possible, but not with the current way how
> BUG_FRAME() is defined: gcc complained about the input parameter in case
> of ASSERT() and WARN().
>
> I might be missing something, but this was the fastest way to at least
> confirm the scheme is working for Arm.
Okay, I think I have found a way to use a common macro, which seems to
be even more simple than the original one:

#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 .bug_frames." __stringify(type)              \
                       ", \"a\", %%progbits\n"                      \
         "2:\n"                                                     \
         ".p2align 2\n"                                             \
         ".long (1b - 2b)\n"                                        \
         ".long (%0 - 2b)\n"                                        \
         ".long (%1 - 2b)\n"                                        \
         ".hword " __stringify(line) ", 0\n"                        \
         ".popsection" :: "i" (ptr), "i" (msg)); i                  \
} while (0)


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys

Attachment: OpenPGP_signature
Description: OpenPGP digital signature