Re: [PATCH 1/2] ARM: kvm: fix a bad BSYM() usage

[Dave P Martin <Dave.Martin@arm.com>]

On Mon, May 11, 2015 at 11:17:39AM +0100, Russell King - ARM Linux wrote:
> On Mon, May 11, 2015 at 10:56:57AM +0100, Dave P Martin wrote:
> > ldr= will do the right thing *if* the target symbol's type is correctly
> > annotated.  This means that ldr =some_local_code_symbol does the right
> > thing for branch target addresses if and only if some_local_code_symbol
> > is marked with .type %function (or ENDPROC).
> > 
> > The fact that a symbol is in a code section is *not* enough.
> > 
> > For ARM code this never mattered, so local symbols in .S files are
> > probably under-annotated in general.  BSYM() might have been used to
> > work around this in some cases.
> > 
> > We should check that all the BSYMs removed by this series from ldr=
> > and .long/.word etc. point to a correctly annotated symbol, and add
> > the annotations if not.
> 
> I guess the problem here is that people forget what a patch series is
> actually doing and then start making comments based on hypothetical
> stuff rather than what the series is actually doing.
> 
> To recap, this series:
> 
> 1. Removes the wrong usage of BSYM() in the KVM code.  This is used with
>    the symbol "panic" which is a function declared by generic C code.
>    Therefore, panic will have the appropriate annotations attached to
>    this symbol, unless GCC itself is buggy.
> 
>    This is the _only_ case of "ldr rd, =BSYM(sym)".

That's a sound conclusion -- you did mention before that that was the
only BSYM with ldr=, but it slipped my mind again, sorry.

> 2. It replaces the remaining "adr rd, BSYM(sym)" with a new "badr" macro,
>    and removes the BSYM() preprocessor macro.  The new "badr" macro which
>    is identical in behaviour to the former, but ensures that BSYM()
>    doesn't get mis-used with "ldr".

And this part wasn't in question anyway, so all is fine.

Cheers
---Dave