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

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

[Russell King]

BSYM() should only be used when refering to local symbols in the same
assembly file which are resolved by the assembler, and not for
linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
the linker is involved in fixing up this relocation, and it knows
whether panic() is ARM or Thumb.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/kvm/interrupts.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
index 79caf79b304a..87847d2c5f99 100644
--- a/arch/arm/kvm/interrupts.S
+++ b/arch/arm/kvm/interrupts.S
@@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
 THUMB(	orr	r2, r2, #PSR_T_BIT	)
 	msr	spsr_cxsf, r2
 	mrs	r1, ELR_hyp
-	ldr	r2, =BSYM(panic)
+	ldr	r2, =panic
 	msr	ELR_hyp, r2
 	ldr	r0, =\panic_str
 	clrex				@ Clear exclusive monitor
-- 
1.8.3.1

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

[Nicolas Pitre]

On Fri, 8 May 2015, Russell King wrote:

> BSYM() should only be used when refering to local symbols in the same
> assembly file which are resolved by the assembler, and not for
> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> the linker is involved in fixing up this relocation, and it knows
> whether panic() is ARM or Thumb.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Nicolas Pitre <nico@linaro.org>

> ---
>  arch/arm/kvm/interrupts.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> index 79caf79b304a..87847d2c5f99 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
>  THUMB(	orr	r2, r2, #PSR_T_BIT	)
>  	msr	spsr_cxsf, r2
>  	mrs	r1, ELR_hyp
> -	ldr	r2, =BSYM(panic)
> +	ldr	r2, =panic
>  	msr	ELR_hyp, r2
>  	ldr	r0, =\panic_str
>  	clrex				@ Clear exclusive monitor
> -- 
> 1.8.3.1
> 
> 

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

[Dave P Martin]

On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
> BSYM() should only be used when refering to local symbols in the same
> assembly file which are resolved by the assembler, and not for
> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> the linker is involved in fixing up this relocation, and it knows
> whether panic() is ARM or Thumb.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Acked-by: Dave Martin <Dave.Martin@arm.com>

> ---
>  arch/arm/kvm/interrupts.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> index 79caf79b304a..87847d2c5f99 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
>  THUMB(	orr	r2, r2, #PSR_T_BIT	)
>  	msr	spsr_cxsf, r2
>  	mrs	r1, ELR_hyp
> -	ldr	r2, =BSYM(panic)
> +	ldr	r2, =panic
>  	msr	ELR_hyp, r2
>  	ldr	r0, =\panic_str
>  	clrex				@ Clear exclusive monitor
> -- 
> 1.8.3.1
> 

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

[Christoffer Dall]

On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
> BSYM() should only be used when refering to local symbols in the same
> assembly file which are resolved by the assembler, and not for
> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> the linker is involved in fixing up this relocation, and it knows
> whether panic() is ARM or Thumb.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  arch/arm/kvm/interrupts.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> index 79caf79b304a..87847d2c5f99 100644
> --- a/arch/arm/kvm/interrupts.S
> +++ b/arch/arm/kvm/interrupts.S
> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
>  THUMB(	orr	r2, r2, #PSR_T_BIT	)
>  	msr	spsr_cxsf, r2
>  	mrs	r1, ELR_hyp
> -	ldr	r2, =BSYM(panic)
> +	ldr	r2, =panic
>  	msr	ELR_hyp, r2
>  	ldr	r0, =\panic_str
>  	clrex				@ Clear exclusive monitor
> -- 
> 1.8.3.1
> 
Indeed, the linker figures it out as it should.  It does seem like the
right result is produced with the BSYM() macro as well so not sure what
the harm is.

Anyway, I've queued this to merge via the KVM tree.

Thanks,
-Christoffer

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

[Ard Biesheuvel]

On 9 May 2015 at 22:07, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
>> BSYM() should only be used when refering to local symbols in the same
>> assembly file which are resolved by the assembler, and not for
>> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
>> the linker is involved in fixing up this relocation, and it knows
>> whether panic() is ARM or Thumb.
>>
>> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>> ---
>>  arch/arm/kvm/interrupts.S | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
>> index 79caf79b304a..87847d2c5f99 100644
>> --- a/arch/arm/kvm/interrupts.S
>> +++ b/arch/arm/kvm/interrupts.S
>> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
>>  THUMB(       orr     r2, r2, #PSR_T_BIT      )
>>       msr     spsr_cxsf, r2
>>       mrs     r1, ELR_hyp
>> -     ldr     r2, =BSYM(panic)
>> +     ldr     r2, =panic
>>       msr     ELR_hyp, r2
>>       ldr     r0, =\panic_str
>>       clrex                           @ Clear exclusive monitor
>> --
>> 1.8.3.1
>>
> Indeed, the linker figures it out as it should.  It does seem like the
> right result is produced with the BSYM() macro as well so not sure what
> the harm is.
>

BSYM() is defined as 'sym + 1' not 'sym | 1', so if the symbol has the
thumb bit set already, the result is incorrect.

> Anyway, I've queued this to merge via the KVM tree.
>
> Thanks,
> -Christoffer
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Russell King - ARM Linux]

On Sat, May 09, 2015 at 10:07:17PM +0200, Christoffer Dall wrote:
> On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
> > BSYM() should only be used when refering to local symbols in the same
> > assembly file which are resolved by the assembler, and not for
> > linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> > the linker is involved in fixing up this relocation, and it knows
> > whether panic() is ARM or Thumb.
> > 
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > ---
> >  arch/arm/kvm/interrupts.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> > index 79caf79b304a..87847d2c5f99 100644
> > --- a/arch/arm/kvm/interrupts.S
> > +++ b/arch/arm/kvm/interrupts.S
> > @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
> >  THUMB(	orr	r2, r2, #PSR_T_BIT	)
> >  	msr	spsr_cxsf, r2
> >  	mrs	r1, ELR_hyp
> > -	ldr	r2, =BSYM(panic)
> > +	ldr	r2, =panic
> >  	msr	ELR_hyp, r2
> >  	ldr	r0, =\panic_str
> >  	clrex				@ Clear exclusive monitor
> > -- 
> > 1.8.3.1
> > 
> Indeed, the linker figures it out as it should.  It does seem like the
> right result is produced with the BSYM() macro as well so not sure what
> the harm is.
> 
> Anyway, I've queued this to merge via the KVM tree.

I already have it in my tree (and linux-next) as the second patch (which
removes the BSYM macro entirely) depends on this.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

[Christoffer Dall]

On Sat, May 09, 2015 at 09:10:57PM +0100, Russell King - ARM Linux wrote:
> On Sat, May 09, 2015 at 10:07:17PM +0200, Christoffer Dall wrote:
> > On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
> > > BSYM() should only be used when refering to local symbols in the same
> > > assembly file which are resolved by the assembler, and not for
> > > linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> > > the linker is involved in fixing up this relocation, and it knows
> > > whether panic() is ARM or Thumb.
> > > 
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > > ---
> > >  arch/arm/kvm/interrupts.S | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> > > index 79caf79b304a..87847d2c5f99 100644
> > > --- a/arch/arm/kvm/interrupts.S
> > > +++ b/arch/arm/kvm/interrupts.S
> > > @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
> > >  THUMB(	orr	r2, r2, #PSR_T_BIT	)
> > >  	msr	spsr_cxsf, r2
> > >  	mrs	r1, ELR_hyp
> > > -	ldr	r2, =BSYM(panic)
> > > +	ldr	r2, =panic
> > >  	msr	ELR_hyp, r2
> > >  	ldr	r0, =\panic_str
> > >  	clrex				@ Clear exclusive monitor
> > > -- 
> > > 1.8.3.1
> > > 
> > Indeed, the linker figures it out as it should.  It does seem like the
> > right result is produced with the BSYM() macro as well so not sure what
> > the harm is.
> > 
> > Anyway, I've queued this to merge via the KVM tree.
> 
> I already have it in my tree (and linux-next) as the second patch (which
> removes the BSYM macro entirely) depends on this.
> 
ok, fine, you can add my ack then if you like:

Acked-by: Christoffer Dall <christoffer.dall@linaro.org>

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

[Christoffer Dall]

On Sat, May 09, 2015 at 10:10:56PM +0200, Ard Biesheuvel wrote:
> On 9 May 2015 at 22:07, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
> >> BSYM() should only be used when refering to local symbols in the same
> >> assembly file which are resolved by the assembler, and not for
> >> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> >> the linker is involved in fixing up this relocation, and it knows
> >> whether panic() is ARM or Thumb.
> >>
> >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> >> ---
> >>  arch/arm/kvm/interrupts.S | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> >> index 79caf79b304a..87847d2c5f99 100644
> >> --- a/arch/arm/kvm/interrupts.S
> >> +++ b/arch/arm/kvm/interrupts.S
> >> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
> >>  THUMB(       orr     r2, r2, #PSR_T_BIT      )
> >>       msr     spsr_cxsf, r2
> >>       mrs     r1, ELR_hyp
> >> -     ldr     r2, =BSYM(panic)
> >> +     ldr     r2, =panic
> >>       msr     ELR_hyp, r2
> >>       ldr     r0, =\panic_str
> >>       clrex                           @ Clear exclusive monitor
> >> --
> >> 1.8.3.1
> >>
> > Indeed, the linker figures it out as it should.  It does seem like the
> > right result is produced with the BSYM() macro as well so not sure what
> > the harm is.
> >
> 
> BSYM() is defined as 'sym + 1' not 'sym | 1', so if the symbol has the
> thumb bit set already, the result is incorrect.
> 
yeah, but the linker will look at the result of 'sym + 1', so on my
system it ends up with 'sym + 1' after the linker has done its thing
(verified by looking at the disassembly of vmlinux); I assume the
linker logic is that it's branching to a thumb function but the target
is already the +1 so no action necessary, as opposed to just blindly
adding 1.

-Christoffer

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

[Ard Biesheuvel]

On 11 May 2015 at 11:05, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> On Sat, May 09, 2015 at 10:10:56PM +0200, Ard Biesheuvel wrote:
>> On 9 May 2015 at 22:07, Christoffer Dall <christoffer.dall@linaro.org> wrote:
>> > On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
>> >> BSYM() should only be used when refering to local symbols in the same
>> >> assembly file which are resolved by the assembler, and not for
>> >> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
>> >> the linker is involved in fixing up this relocation, and it knows
>> >> whether panic() is ARM or Thumb.
>> >>
>> >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
>> >> ---
>> >>  arch/arm/kvm/interrupts.S | 2 +-
>> >>  1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
>> >> index 79caf79b304a..87847d2c5f99 100644
>> >> --- a/arch/arm/kvm/interrupts.S
>> >> +++ b/arch/arm/kvm/interrupts.S
>> >> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
>> >>  THUMB(       orr     r2, r2, #PSR_T_BIT      )
>> >>       msr     spsr_cxsf, r2
>> >>       mrs     r1, ELR_hyp
>> >> -     ldr     r2, =BSYM(panic)
>> >> +     ldr     r2, =panic
>> >>       msr     ELR_hyp, r2
>> >>       ldr     r0, =\panic_str
>> >>       clrex                           @ Clear exclusive monitor
>> >> --
>> >> 1.8.3.1
>> >>
>> > Indeed, the linker figures it out as it should.  It does seem like the
>> > right result is produced with the BSYM() macro as well so not sure what
>> > the harm is.
>> >
>>
>> BSYM() is defined as 'sym + 1' not 'sym | 1', so if the symbol has the
>> thumb bit set already, the result is incorrect.
>>
> yeah, but the linker will look at the result of 'sym + 1', so on my
> system it ends up with 'sym + 1' after the linker has done its thing
> (verified by looking at the disassembly of vmlinux);

Hmm, I though had done the same when this was under discussion a
couple of weeks ago, and had arrived at the opposite conclusion, but
now I cannot reproduce anymore so apparently not.
Sorry for the noise.

> I assume the
> linker logic is that it's branching to a thumb function but the target
> is already the +1 so no action necessary, as opposed to just blindly
> adding 1.
>
> -Christoffer

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

[Dave P Martin]

On Mon, May 11, 2015 at 10:05:37AM +0100, Christoffer Dall wrote:
> On Sat, May 09, 2015 at 10:10:56PM +0200, Ard Biesheuvel wrote:
> > On 9 May 2015 at 22:07, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > > On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
> > >> BSYM() should only be used when refering to local symbols in the same
> > >> assembly file which are resolved by the assembler, and not for
> > >> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> > >> the linker is involved in fixing up this relocation, and it knows
> > >> whether panic() is ARM or Thumb.
> > >>
> > >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > >> ---
> > >>  arch/arm/kvm/interrupts.S | 2 +-
> > >>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>
> > >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> > >> index 79caf79b304a..87847d2c5f99 100644
> > >> --- a/arch/arm/kvm/interrupts.S
> > >> +++ b/arch/arm/kvm/interrupts.S
> > >> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
> > >>  THUMB(       orr     r2, r2, #PSR_T_BIT      )
> > >>       msr     spsr_cxsf, r2
> > >>       mrs     r1, ELR_hyp
> > >> -     ldr     r2, =BSYM(panic)
> > >> +     ldr     r2, =panic
> > >>       msr     ELR_hyp, r2
> > >>       ldr     r0, =\panic_str
> > >>       clrex                           @ Clear exclusive monitor
> > >> --
> > >> 1.8.3.1
> > >>
> > > Indeed, the linker figures it out as it should.  It does seem like the
> > > right result is produced with the BSYM() macro as well so not sure what
> > > the harm is.
> > >
> > 
> > BSYM() is defined as 'sym + 1' not 'sym | 1', so if the symbol has the
> > thumb bit set already, the result is incorrect.
> > 
> yeah, but the linker will look at the result of 'sym + 1', so on my
> system it ends up with 'sym + 1' after the linker has done its thing
> (verified by looking at the disassembly of vmlinux); I assume the
> linker logic is that it's branching to a thumb function but the target
> is already the +1 so no action necessary, as opposed to just blindly
> adding 1.

There are a few overlapping confusions.

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.

Cheers
---Dave

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

[Dave P Martin]

On Mon, May 11, 2015 at 10:44:49AM +0100, Ard Biesheuvel wrote:
> On 11 May 2015 at 11:05, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> > On Sat, May 09, 2015 at 10:10:56PM +0200, Ard Biesheuvel wrote:
> >> On 9 May 2015 at 22:07, Christoffer Dall <christoffer.dall@linaro.org> wrote:
> >> > On Fri, May 08, 2015 at 05:08:42PM +0100, Russell King wrote:
> >> >> BSYM() should only be used when refering to local symbols in the same
> >> >> assembly file which are resolved by the assembler, and not for
> >> >> linker-fixed up symbols.  The use of BSYM() with panic is incorrect as
> >> >> the linker is involved in fixing up this relocation, and it knows
> >> >> whether panic() is ARM or Thumb.
> >> >>
> >> >> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> >> >> ---
> >> >>  arch/arm/kvm/interrupts.S | 2 +-
> >> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/arch/arm/kvm/interrupts.S b/arch/arm/kvm/interrupts.S
> >> >> index 79caf79b304a..87847d2c5f99 100644
> >> >> --- a/arch/arm/kvm/interrupts.S
> >> >> +++ b/arch/arm/kvm/interrupts.S
> >> >> @@ -309,7 +309,7 @@ ENTRY(kvm_call_hyp)
> >> >>  THUMB(       orr     r2, r2, #PSR_T_BIT      )
> >> >>       msr     spsr_cxsf, r2
> >> >>       mrs     r1, ELR_hyp
> >> >> -     ldr     r2, =BSYM(panic)
> >> >> +     ldr     r2, =panic
> >> >>       msr     ELR_hyp, r2
> >> >>       ldr     r0, =\panic_str
> >> >>       clrex                           @ Clear exclusive monitor
> >> >> --
> >> >> 1.8.3.1
> >> >>
> >> > Indeed, the linker figures it out as it should.  It does seem like the
> >> > right result is produced with the BSYM() macro as well so not sure what
> >> > the harm is.
> >> >
> >>
> >> BSYM() is defined as 'sym + 1' not 'sym | 1', so if the symbol has the
> >> thumb bit set already, the result is incorrect.
> >>
> > yeah, but the linker will look at the result of 'sym + 1', so on my
> > system it ends up with 'sym + 1' after the linker has done its thing
> > (verified by looking at the disassembly of vmlinux);
> 
> Hmm, I though had done the same when this was under discussion a
> couple of weeks ago, and had arrived at the opposite conclusion, but
> now I cannot reproduce anymore so apparently not.
> Sorry for the noise.

I think that was me confusing myself.  In fact, the linker behaves
differently depending on the target symbol type.

If the relevant symbols are all correctly annoted with .type %function
or ENDPROC, the linker should do the right thing without needing any
BSYMs.

Cheers
---Dave

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

[Russell King - ARM Linux]

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)".

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".

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

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

[Dave P Martin]

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