Re: [Xen-devel] [PATCH] arm32: fix build after 063188f4b3

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Jan,

On 10/10/2014 15:51, Jan Beulich wrote:
> > > > On 10.10.14 at 16:12, <julien.grall@xxxxxxxxxx> wrote:
> > On 10/10/2014 14:58, Jan Beulich wrote:
> > > "xen: arm: Add support for the Exynos secure firmware" introduced code
> > > assuming that exynos_smc() would get called with arguments in certain
> > > registers. While the "noinline" attribute guarantees the function to
> > > not get inlined, it does not guarantee that all arguments arrive in the
> > > assumed registers: gcc's interprocedural analysis can result in clone
> > > functions to be created where some of the incoming arguments (commonly
> > > when they have constant values) get replaced by putting in place the
> > > respective values inside the clone.
> > I'm not sure to understand here. If the function is marked as noinlined,
> > that would mean the arguments will be passed with the ARM in the
> > register with the ARM calling convention (i.e r0 for argument 0...). Why
> > GCC would try to create a clone of this function?
> The compiler is free to do so as long as the language specification isn't
> being violated.
Thanks for the explanation.

> > > The alternative of adding __attribute__((optimize("-fno-ipa-cp")))
> > > to the function definition would likely not work with all supported
> > > compiler versions.
> > The function was first introduced in arch/arm/psci.c (it's a copy of the
> > Linux one in arch/arm/kernel/psci.c).
> >
> > Does it mean that Linux code is buggy too?
> Likely.
>
> > This function is duplicate in 3 different places in Xen:
> >         - arch/arm/psci.c
> >         - arch/platforms/exynos5.c
> >         - arch/platforms/seattle.c
> >
> > So all those functions should be fixed. I think it's time to introduce a
> > global SMC function...
> Okay, I got the build failure only in this one place. But if and when
> the compiler choses to do such transformations is entirely up to it,
> so yes, if there are multiple instances likely they all would need
> fixing.
BTW,  named register is a GNU extension and not supported by clang. Can 
you avoid to use them? Maybe by writing the function in assembly. So we 
are safe against any compiler optimization.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel