From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH 1/2] ARM: kvm: fix a bad BSYM() usage Date: Mon, 11 May 2015 11:05:37 +0200 Message-ID: <20150511090537.GA2009@cbox> References: <20150509200717.GA30634@cbox> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Russell King , KVM devel mailing list , Nicolas Pitre , Marc Zyngier , "kvmarm@lists.cs.columbia.edu" , Gleb Natapov , Paolo Bonzini , Dave P Martin , "linux-arm-kernel@lists.infradead.org" To: Ard Biesheuvel Return-path: Received: from mail-lb0-f180.google.com ([209.85.217.180]:35169 "EHLO mail-lb0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753120AbbEKJFb (ORCPT ); Mon, 11 May 2015 05:05:31 -0400 Received: by lbbuc2 with SMTP id uc2so89125325lbb.2 for ; Mon, 11 May 2015 02:05:30 -0700 (PDT) Content-Disposition: inline In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On Sat, May 09, 2015 at 10:10:56PM +0200, Ard Biesheuvel wrote: > On 9 May 2015 at 22:07, 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 > >> --- > >> 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