Re: [PATCH v3 3/4] xen/arm: switch ARM to use generic implementation of bug.h

[Oleksii <oleksii.kurochko@xxxxxxxxx>]

On Wed, 2023-03-01 at 09:31 +0000, Julien Grall wrote:
> Hi,
> 
> On 01/03/2023 08:58, Oleksii wrote:
> > On Tue, 2023-02-28 at 17:48 +0000, Julien Grall wrote:
> > > Hi Oleksii,
> > > 
> > > On 28/02/2023 15:09, Oleksii wrote:
> > > > On Sat, 2023-02-25 at 16:49 +0000, Julien Grall wrote:
> > > > > Hi Oleksii,
> > > > > 
> > > > > On 24/02/2023 11:31, Oleksii Kurochko wrote:
> > > > > > The following changes were made:
> > > > > > * make GENERIC_BUG_FRAME mandatory for ARM
> > > > > 
> > > > > I have asked in patch #1 but will ask it again because I
> > > > > think
> > > > > this
> > > > > should be recorded in the commit message. Can you outline why
> > > > > it
> > > > > is
> > > > > not
> > > > > possible to completely switch to the generic version?
> > > > I haven't tried to switch ARM too because of comment regarding
> > > > 'i'
> > > > in
> > > > <asm/bug.h>:
> > > > /*
> > > >    * GCC will not allow to use "i"  when PIE is enabled (Xen
> > > > doesn't
> > > > set
> > > > the
> > > >    * flag but instead rely on the default value from the
> > > > compiler).
> > > > So
> > > > the
> > > >    * easiest way to implement run_in_exception_handler() is to
> > > > pass
> > > > the
> > > > to
> > > >    * be called function in a fixed register.
> > > >    */
> > > 
> > > I would expect this comment to be valid for any arch. So if there
> > > is
> > > a
> > > need to deal with PIE, then we would not be able to use "i" in
> > > the
> > > BUG
> > > frame.
> > > 
> > > Note that we are now explicitly compiling Xen without PIE (see
> > > Config.mk).
> > Then it looks like some architectures isn't expected to be compiled
> > with PIE. I mean that x86's bug.h is used 'i' and there is no any
> > alternative version in case of PIE.
> > 
> > If Xen should be compilable with PIE then we have to use ARM
> > implementation of bug.h everywhere. ( based on comment about 'i'
> > with
> > PIE ).
> 
> The comment was added because until commit ecd6b9759919 ("Config.mk: 
> correct PIE-related option(s) in EMBEDDED_EXTRA_CFLAGS") we would let
> the compiler to decide whether PIE should be enabled.
> 
> Now we are forcing -fno-pie for Xen on any architecture (the flag is 
> added at the top-level in Config.mk).
> 
> > 
> > Now I am totally confused...
> 
> My point was not about using the Arm implementation everywhere. My
> point 
> was that we disable even for Arm and therefore it is fine to use the 
> common version.
> 
> If in the future we need to support PIE in Xen (I am not aware of any
> effort yet), then we could decide to update the common BUG framework.
> But for now, I don't think this is something you need to care in your
> series.
> 
Thanks for clarification.

> Cheers,
>