linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* your mail
       [not found] <1250258343-14203-1-git-send-email-thierry.reding@avionic-design.de>
@ 2009-10-02 14:53 ` Thierry Reding
  2010-01-02 12:06   ` Russell King - ARM Linux
  0 siblings, 1 reply; 27+ messages in thread
From: Thierry Reding @ 2009-10-02 14:53 UTC (permalink / raw)
  To: linux-arm-kernel

* Thierry Reding wrote:
> So it's been quite a while since I last posted this series and there have been
> a number of changes.
> 
> Support for T-Pid analog variants is dropped and will be handled in user-space
> instead. There hardware does not differ from the standard T-Pid. This gets rid 
> of some ugly platform-specific #ifdefs.
> 
> Front-panel controls are now handled in a separate driver, adx-keypad. That
> way backlight control can be moved to userspace.
> 
> This series also adds a watchdog driver that can be used on all Avionic Design
> Xanthos-based boards.

Russell,

with the watchdog and backlight drivers out of the way, would you like me to
get any of the other drivers (fb, keypad) in via seperate trees or can they go
in through the ARM tree?

Do the remaining patches need additional work or can I go ahead and submit
them to the patch system? Or do you prefer some other way to get them into
your tree?

Thierry

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2009-10-02 14:53 ` Thierry Reding
@ 2010-01-02 12:06   ` Russell King - ARM Linux
  0 siblings, 0 replies; 27+ messages in thread
From: Russell King - ARM Linux @ 2010-01-02 12:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Oct 02, 2009 at 04:53:15PM +0200, Thierry Reding wrote:
> * Thierry Reding wrote:
> > So it's been quite a while since I last posted this series and there have been
> > a number of changes.
> > 
> > Support for T-Pid analog variants is dropped and will be handled in user-space
> > instead. There hardware does not differ from the standard T-Pid. This gets rid 
> > of some ugly platform-specific #ifdefs.
> > 
> > Front-panel controls are now handled in a separate driver, adx-keypad. That
> > way backlight control can be moved to userspace.
> > 
> > This series also adds a watchdog driver that can be used on all Avionic Design
> > Xanthos-based boards.
> 
> Russell,
> 
> with the watchdog and backlight drivers out of the way, would you like me to
> get any of the other drivers (fb, keypad) in via seperate trees or can they go
> in through the ARM tree?

They should go in via the relevant driver trees.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2010-05-18 10:38 No subject Marek Szyprowski
@ 2010-05-19  2:24 ` Ben Dooks
  0 siblings, 0 replies; 27+ messages in thread
From: Ben Dooks @ 2010-05-19  2:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, May 18, 2010 at 12:38:38PM +0200, Marek Szyprowski wrote:
> Hello,

Hi, messages are supposed to have subjects... please supply one next
timne.
 
> This patch series perform a general cleanup in Samsung S5PC100 SoC support.
> This chip is moved from custom s5pc1xx platform framework to new plat-s5p
> framework, so more common code can be easily reused in upcomming extensions
> for S5PV210/S5PC110 SoCs.
> 
> This patch series is prepared against next-samsung tree, with assumption
> that the "[PATCH v3] ARM: S5PC100: Pre-requisite clock patch for
> plat-s5pc1xx to plat-s5p" has been applied as well as the '[PATCH v6]
> ARM: S5PV210: Add Ext interrupt support' (with additional bug fixes).
>
> I've tried to split my changes as much as possible to clearly show how the
> transition from plat-s5pc1xx to plat-s5p is being done.

Ok, this seems to be a pretty good progression from one to the other.

I will chase up the EINT support, it got dropped from the first pull due
to your bug reports.

Will start sorting out a second round of branches with a view to trying to
get this into the current merge window. I hope to have something to test
by the end of today.
 
> Changes since v2:
> - fixed some whitespace/tabs errors
> - removed external interrupt code, a common code from plat-s5p will be used
> - moved SMDKC100 fixes to separate patch series
> 
> Changes since v1:
> - bases on completely new clock code provided by Kukjin Kim
> - added some plat-s5p fixes required for transition
> - removed custom functions from gpiolib implementation (now uses common
>   code from plat-samsung)
> - restructured the changes to avoid breaking the functionality beteen the
>   patches
> - some other minor cleanups (mainly c1xx to c100 renames)
> 
> This patch series includes:
> 
> [PATCH 01/11] drivers: serial: S5PC100 serial driver cleanup
> [PATCH 02/11] ARM: S5PC100: Use common functions for gpiolib implementation
> [PATCH 03/11] ARM: S5PC100: Move gpio support from plat-s5pc1xx to mach-s5pc100
> [PATCH 04/11] ARM: S5PC100: gpio.h cleanup
> [PATCH 05/11] ARM: S5PC100: Move frame buffer helpers from plat-s5pc1xx to mach-s5pc100
> [PATCH 06/11] ARM: S5PC100: Move i2c helpers from plat-s5pc1xx to mach-s5pc100
> [PATCH 07/11] ARM: S5PC100: Move sdhci helpers from plat-s5pc1xx to mach-s5pc100
> [PATCH 08/11] ARM: Samsung: move S5PC100 support from plat-s5pc1xx to plat-s5p framework
> [PATCH 09/11] ARM: S5PC100: Add support for gpio interrupt
> [PATCH 10/11] ARM: S5PC100: use common plat-s5p external interrupt code
> [PATCH 11/11] ARM: remove obsolete plat-s5pc1xx directory
> 
> Best regards
> --
> Marek Szyprowski
> Samsung Poland R&D Center
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* No subject
@ 2010-06-07 17:58 Dave Hylands
  2010-06-07 22:10 ` your mail Jamie Lokier
  0 siblings, 1 reply; 27+ messages in thread
From: Dave Hylands @ 2010-06-07 17:58 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

I'm trying to understand what I need to be concerned about with SMP
processors and sharing global data (in particular a dual Cortex-A9)

I'm familiar with spinlocks, but in this case I'm trying to work with
some lockless data structures.

What I'm not sure is whether the following would work. Suppose I have
a couple of 8-bit get/put indicies which are in adjacent memory
locations (within the same 32-bit word).

If I have an ISR and a thread running on an SMP core, and the ISR is
running on one core and the thread is running on a second core, if the
ISR were to only write to the put pointer and the thread were only to
write to the get pointer, does the cache maintain consistency? Or do
the get and put pointers need to be in separate cache lines?

Another way of asking this: If both cores are writing to the same
32-bit word (but different bytes) do the writes collide?

--
Dave Hylands
Shuswap, BC, Canada
http://www.DaveHylands.com/

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2010-06-07 17:58 No subject Dave Hylands
@ 2010-06-07 22:10 ` Jamie Lokier
  2010-06-07 22:44   ` Dave Hylands
  0 siblings, 1 reply; 27+ messages in thread
From: Jamie Lokier @ 2010-06-07 22:10 UTC (permalink / raw)
  To: linux-arm-kernel

Dave Hylands wrote:
> I'm trying to understand what I need to be concerned about with SMP
> processors and sharing global data (in particular a dual Cortex-A9)
> 
> I'm familiar with spinlocks, but in this case I'm trying to work with
> some lockless data structures.
> 
> What I'm not sure is whether the following would work. Suppose I have
> a couple of 8-bit get/put indicies which are in adjacent memory
> locations (within the same 32-bit word).
> 
> If I have an ISR and a thread running on an SMP core, and the ISR is
> running on one core and the thread is running on a second core, if the
> ISR were to only write to the put pointer and the thread were only to
> write to the get pointer, does the cache maintain consistency? Or do
> the get and put pointers need to be in separate cache lines?
> 
> Another way of asking this: If both cores are writing to the same
> 32-bit word (but different bytes) do the writes collide?

I'm pretty sure any system compatible with pthreads has to be fine
with the variables being independent, because the bytes could be
variables protected by separate mutexes.

However, other questions for your lockless structures are whether
writes by one processor are seen in a reasonable or even bounded time
by reads on another processor (write buffering), and which barrier
instructions to use between the index accesses and accessing some
array they might indexing (only a problem for userspace, because the
kernel already provides barriers).

-- Jamie

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2010-06-07 22:10 ` your mail Jamie Lokier
@ 2010-06-07 22:44   ` Dave Hylands
  0 siblings, 0 replies; 27+ messages in thread
From: Dave Hylands @ 2010-06-07 22:44 UTC (permalink / raw)
  To: linux-arm-kernel

HI Jamie,

On Mon, Jun 7, 2010 at 3:10 PM, Jamie Lokier wrote:
> Dave Hylands wrote:
[...]
>> Another way of asking this: If both cores are writing to the same
>> 32-bit word (but different bytes) do the writes collide?
>
> I'm pretty sure any system compatible with pthreads has to be fine
> with the variables being independent, because the bytes could be
> variables protected by separate mutexes.
>
> However, other questions for your lockless structures are whether
> writes by one processor are seen in a reasonable or even bounded time
> by reads on another processor (write buffering), and which barrier
> instructions to use between the index accesses and accessing some
> array they might indexing (only a problem for userspace, because the
> kernel already provides barriers).

After lots of reading - I think that the Snoop Control Unit takes care
of moving the cache lines from one core to the other when both cores
are trying to access data from the same cache line.

So it's not very efficient to have both cores accessing data from the
same cache line - but it seems that the integrity of the data should
be maintained (or at least this is my current understanding).

-- 
Dave Hylands
Shuswap, BC, Canada
http://www.DaveHylands.com/

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2011-05-13 19:35 No subject Vadim Bendebury
@ 2011-05-14  3:32 ` Jean-Christophe PLAGNIOL-VILLARD
  0 siblings, 0 replies; 27+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2011-05-14  3:32 UTC (permalink / raw)
  To: linux-arm-kernel

On 12:35 Fri 13 May     , Vadim Bendebury wrote:
> >From 8ed0ea5fcf5552ca3307611b685b4518794eca41 Mon Sep 17 00:00:00 2001
> From: Vadim Bendebury <vbendeb@chromium.org>
> Date: Wed, 11 May 2011 17:31:40 -0700
> Subject: [PATCH 1/1] ARM: thumb: Have the machine name indicate operation in thumb mode.
> 
> This is a cosmetic change, adding a '_thumb' prefix to the
> 'Hardware' line in /proc/cpuinfo. Tested as follows:
I do think is the right place or right way to do so

I think we need to create a sysfs entry to allow machine to more information

Best Regards,
J.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2011-12-02 16:01 No subject Will Deacon
@ 2011-12-02 16:11 ` Will Deacon
  0 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2011-12-02 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

Hmmm,

On Fri, Dec 02, 2011 at 04:01:13PM +0000, Will Deacon wrote:
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Not that there's a lot to ignore, but please ignore this empty mail. For
some reason, my box decided to tell the world nothing.

Sorry for the spam,

Will (thinks it's time for a reboot...)

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2012-06-21 18:26 No subject Paul Walmsley
@ 2012-06-22  8:56 ` Tony Lindgren
  0 siblings, 0 replies; 27+ messages in thread
From: Tony Lindgren @ 2012-06-22  8:56 UTC (permalink / raw)
  To: linux-arm-kernel

* Paul Walmsley <paul@pwsan.com> [120621 11:31]:
> Hi Tony
> 
> The following changes since commit 485802a6c524e62b5924849dd727ddbb1497cc71:
> 
>   Linux 3.5-rc3 (2012-06-16 17:25:17 -0700)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/pjw/omap-pending.git tags/omap-cleanup-a-for-3.6
> 
> for you to fetch changes up to 07b3a13957aa250ff5b5409b8ed756b113544112:
> 
>   Merge branches 'clock_cleanup_misc_3.6', 'control_clean_dspbridge_writes_cleanup_3.6', 'hwmod_soc_conditional_cleanup_3.6', 'mcbsp_clock_aliases_cleanup_3.6' and 'remove_clkdm_requirement_from_hwmod_3.6' into omap_cleanup_a_3.6 (2012-06-20 20:11:36 -0600)
> 
> ----------------------------------------------------------------
> 
> Some OMAP hwmod, clock, and System Control Module cleanup patches for 3.6.
> 
> ----------------------------------------------------------------

Merging into cleanup-hwmod and merging into l-o master for testing.

Tony

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2014-10-28 14:13 No subject Mark Rutland
@ 2014-10-28 14:19 ` Mark Rutland
  0 siblings, 0 replies; 27+ messages in thread
From: Mark Rutland @ 2014-10-28 14:19 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Oct 28, 2014 at 02:13:25PM +0000, Mark Rutland wrote:
> Bcc:
> Subject: Re: [PATCHv4 7/7] arm64: add better page protections to arm64
> Reply-To:
> In-Reply-To: <1414440752-9411-8-git-send-email-lauraa@codeaurora.org>

Apologies for this, I appear to have accidentally inserted a newline and
confused my mail client. I'll resend this with that fixed.

Mark.

> 
> Hi Laura,
> 
> On Mon, Oct 27, 2014 at 08:12:32PM +0000, Laura Abbott wrote:
> > Add page protections for arm64 similar to those in arm.
> > This is for security reasons to prevent certain classes
> > of exploits. The current method:
> >
> > - Map all memory as either RWX or RW. We round to the nearest
> >   section to avoid creating page tables before everything is mapped
> > - Once everything is mapped, if either end of the RWX section should
> >   not be X, we split the PMD and remap as necessary
> > - When initmem is to be freed, we change the permissions back to
> >   RW (using stop machine if necessary to flush the TLB)
> > - If CONFIG_DEBUG_RODATA is set, the read only sections are set
> >   read only.
> >
> > Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
> > ---
> > v4: Combined the Kconfig options
> > ---
> >  arch/arm64/Kconfig.debug            |  23 +++
> >  arch/arm64/include/asm/cacheflush.h |   4 +
> >  arch/arm64/kernel/vmlinux.lds.S     |  17 ++
> >  arch/arm64/mm/init.c                |   1 +
> >  arch/arm64/mm/mm.h                  |   2 +
> >  arch/arm64/mm/mmu.c                 | 303 +++++++++++++++++++++++++++++++-----
> >  6 files changed, 314 insertions(+), 36 deletions(-)
> 
> With this patch applied to v3.18-rc2, my board to blows up at boot when
> using UEFI (without DEBUG_RODATA selected):
> 
> ---->8----
> EFI stub: Booting Linux Kernel...
> Initializing cgroup subsys cpu
> Linux version 3.18.0-rc2+ (mark at leverpostej) (gcc version 4.9.2 20140904 (prerelease) (crosstool-NG linaro-1.13.1-4.9-2014.09 - Linaro GCC 4.9-2014.09) ) #112 SMP PREEMPT Tue Oct 28 13:58:41 GMT 2014
> CPU: AArch64 Processor [410fd030] revision 0
> Detected VIPT I-cache on CPU0
> bootconsole [uart0] enabled
> efi: Getting EFI parameters from FDT:
> EFI v2.40 by ARM Juno EFI Oct  7 2014 15:05:42
> efi:  ACPI=0xfebdc000  ACPI 2.0=0xfebdc014
> cma: Reserved 16 MiB at fd800000
> BUG: failure at arch/arm64/mm/mmu.c:234/alloc_init_pmd()!
> Kernel panic - not syncing: BUG!
> CPU: 0 PID: 0 Comm: swapper Not tainted 3.18.0-rc2+ #112
> Call trace:
> [<ffffffc000087ec8>] dump_backtrace+0x0/0x124
> [<ffffffc000087ffc>] show_stack+0x10/0x1c
> [<ffffffc0004ebd58>] dump_stack+0x74/0xb8
> [<ffffffc0004eb018>] panic+0xe0/0x220
> [<ffffffc0004e8e08>] alloc_init_pmd+0x1cc/0x1dc
> [<ffffffc0004e8e3c>] alloc_init_pud+0x24/0x6c
> [<ffffffc0004e8f54>] __create_mapping+0xd0/0xf0
> [<ffffffc00069a0a0>] create_id_mapping+0x5c/0x68
> [<ffffffc00069964c>] efi_idmap_init+0x54/0xd8
> [<ffffffc0006978a8>] setup_arch+0x408/0x5c0
> [<ffffffc00069566c>] start_kernel+0x94/0x3a0
> ---[ end Kernel panic - not syncing: BUG!
> ---->8----
> 
> I've not yet figured out precisely why. I haven't tried a !EFI boot
> because of the way my board is set up at the moment.
> 
> Mark.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 10:18 No subject Ard Biesheuvel
@ 2015-04-21 10:46 ` Dave P Martin
  2015-04-21 10:50   ` Ard Biesheuvel
  0 siblings, 1 reply; 27+ messages in thread
From: Dave P Martin @ 2015-04-21 10:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 11:18:08AM +0100, Ard Biesheuvel wrote:
> On 21 April 2015 at 12:13, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Apr 21, 2015 at 12:08:51PM +0200, Ard Biesheuvel wrote:
> >> This updates the PROCINFO offset-to-setup-function fields of the
> >> Thumb2 capable CPU definitions to include the Thumb bit when building
> >> a Thumb2 kernel. This ensures that these function are always called
> >> in the correct mode.
> >
> > I don't think this is necessary, in fact, I think this is positively
> > regression causing.
> >
> > The symbol 'initfunc' is known to the assembler to be a thumb symbol.
> > As we have seen already from the kernel dumps from the V7M kernel, when
> > it calculates initfunc - name in a T2 kernel, the resulting value is an
> > _odd_ number.
> >
> 
> OK, so BSYM() uses '+ 1' rather than ' | 1'? I wasn't expecting that, sorry.

'| 1' is more logical, but can't be resolved at link time because
there's no relocation for this operation.  Hence '+ 1'.  This matters
for local cross-section references that can't be resolved at assembly
time.

> But looking at proc-v7.S again, the problem may just be the missing
> ENDPROC() declarations for a couple of the setup() functions, which
> explains why they are lacking the Thumb annotations.

Yes, if any are missing ENDPROC() then it should be added there.

Cheers
---Dave

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 10:46 ` your mail Dave P Martin
@ 2015-04-21 10:50   ` Ard Biesheuvel
  2015-04-21 11:10     ` Dave P Martin
  0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2015-04-21 10:50 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 April 2015 at 12:46, Dave P Martin <Dave.Martin@arm.com> wrote:
> On Tue, Apr 21, 2015 at 11:18:08AM +0100, Ard Biesheuvel wrote:
>> On 21 April 2015 at 12:13, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Tue, Apr 21, 2015 at 12:08:51PM +0200, Ard Biesheuvel wrote:
>> >> This updates the PROCINFO offset-to-setup-function fields of the
>> >> Thumb2 capable CPU definitions to include the Thumb bit when building
>> >> a Thumb2 kernel. This ensures that these function are always called
>> >> in the correct mode.
>> >
>> > I don't think this is necessary, in fact, I think this is positively
>> > regression causing.
>> >
>> > The symbol 'initfunc' is known to the assembler to be a thumb symbol.
>> > As we have seen already from the kernel dumps from the V7M kernel, when
>> > it calculates initfunc - name in a T2 kernel, the resulting value is an
>> > _odd_ number.
>> >
>>
>> OK, so BSYM() uses '+ 1' rather than ' | 1'? I wasn't expecting that, sorry.
>
> '| 1' is more logical, but can't be resolved at link time because
> there's no relocation for this operation.  Hence '+ 1'.  This matters
> for local cross-section references that can't be resolved at assembly
> time.
>

OK, that makes sense. But it does appear that the local cross-section
references are working just fine, i.e., references from other sections
in the same .o have the thumb bit set correctly even without BSYM()

>> But looking at proc-v7.S again, the problem may just be the missing
>> ENDPROC() declarations for a couple of the setup() functions, which
>> explains why they are lacking the Thumb annotations.
>
> Yes, if any are missing ENDPROC() then it should be added there.
>

I am putting together a v2 with this instead of the BSYM() on the initfn


Cheers,
Ard.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 10:50   ` Ard Biesheuvel
@ 2015-04-21 11:10     ` Dave P Martin
  2015-04-21 11:15       ` Ard Biesheuvel
  2015-04-21 11:24       ` Russell King - ARM Linux
  0 siblings, 2 replies; 27+ messages in thread
From: Dave P Martin @ 2015-04-21 11:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 11:50:02AM +0100, Ard Biesheuvel wrote:
> On 21 April 2015 at 12:46, Dave P Martin <Dave.Martin@arm.com> wrote:
> > On Tue, Apr 21, 2015 at 11:18:08AM +0100, Ard Biesheuvel wrote:
> >> On 21 April 2015 at 12:13, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > On Tue, Apr 21, 2015 at 12:08:51PM +0200, Ard Biesheuvel wrote:
> >> >> This updates the PROCINFO offset-to-setup-function fields of the
> >> >> Thumb2 capable CPU definitions to include the Thumb bit when building
> >> >> a Thumb2 kernel. This ensures that these function are always called
> >> >> in the correct mode.
> >> >
> >> > I don't think this is necessary, in fact, I think this is positively
> >> > regression causing.
> >> >
> >> > The symbol 'initfunc' is known to the assembler to be a thumb symbol.
> >> > As we have seen already from the kernel dumps from the V7M kernel, when
> >> > it calculates initfunc - name in a T2 kernel, the resulting value is an
> >> > _odd_ number.
> >> >
> >>
> >> OK, so BSYM() uses '+ 1' rather than ' | 1'? I wasn't expecting that, sorry.
> >
> > '| 1' is more logical, but can't be resolved at link time because
> > there's no relocation for this operation.  Hence '+ 1'.  This matters
> > for local cross-section references that can't be resolved at assembly
> > time.
> >
> 
> OK, that makes sense. But it does appear that the local cross-section
> references are working just fine, i.e., references from other sections
> in the same .o have the thumb bit set correctly even without BSYM()

For branch targets, the set of situations where BSYM must be used and
the set of situations where it must not be used are mutually exclusive:

 * If the assembler resolves the address and the address will be used as a
   branch target, BSYM() must be used.  This applies to non-cross-section
   references within in single object only.

 * If the linker resolves the address, BSYM() must not be used and the
   target label must be annotated with ENDPROC().  This applies to
   all cross-section or cross-object references.

For any address that won't be used as a branch target, BSYM() must not
be used.

Cross-section non-cross-object references where the target is missing
ENDPROC() and BSYM _is_ used will also work, but this should be avoided
-- it's an abuse really.


The reason for these rules is that the assembler doesn't do any Thumb
bit handling when taking the address of a local symbol, irrespective
of the symbol's type.  This can result in a screwup unless the assembler
can't resolve the address and must leave it as a relocation for the
linker to process, or if the correct annotation for the linker to do
the right fixup is missing.


I don't know the history of why this inconsistency exists between the
assembler and linker behaviour, but changing it would likely break
more stuff than it fixes now.

Maybe it would be a good idea to add a comment in asm/unified.h
explaining this, assuming (hopefully) that my explanation was right...

[...]

Cheers
---Dave

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 11:10     ` Dave P Martin
@ 2015-04-21 11:15       ` Ard Biesheuvel
  2015-04-21 11:24       ` Russell King - ARM Linux
  1 sibling, 0 replies; 27+ messages in thread
From: Ard Biesheuvel @ 2015-04-21 11:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 April 2015 at 13:10, Dave P Martin <Dave.Martin@arm.com> wrote:
> On Tue, Apr 21, 2015 at 11:50:02AM +0100, Ard Biesheuvel wrote:
>> On 21 April 2015 at 12:46, Dave P Martin <Dave.Martin@arm.com> wrote:
>> > On Tue, Apr 21, 2015 at 11:18:08AM +0100, Ard Biesheuvel wrote:
>> >> On 21 April 2015 at 12:13, Russell King - ARM Linux
>> >> <linux@arm.linux.org.uk> wrote:
>> >> > On Tue, Apr 21, 2015 at 12:08:51PM +0200, Ard Biesheuvel wrote:
>> >> >> This updates the PROCINFO offset-to-setup-function fields of the
>> >> >> Thumb2 capable CPU definitions to include the Thumb bit when building
>> >> >> a Thumb2 kernel. This ensures that these function are always called
>> >> >> in the correct mode.
>> >> >
>> >> > I don't think this is necessary, in fact, I think this is positively
>> >> > regression causing.
>> >> >
>> >> > The symbol 'initfunc' is known to the assembler to be a thumb symbol.
>> >> > As we have seen already from the kernel dumps from the V7M kernel, when
>> >> > it calculates initfunc - name in a T2 kernel, the resulting value is an
>> >> > _odd_ number.
>> >> >
>> >>
>> >> OK, so BSYM() uses '+ 1' rather than ' | 1'? I wasn't expecting that, sorry.
>> >
>> > '| 1' is more logical, but can't be resolved at link time because
>> > there's no relocation for this operation.  Hence '+ 1'.  This matters
>> > for local cross-section references that can't be resolved at assembly
>> > time.
>> >
>>
>> OK, that makes sense. But it does appear that the local cross-section
>> references are working just fine, i.e., references from other sections
>> in the same .o have the thumb bit set correctly even without BSYM()
>
> For branch targets, the set of situations where BSYM must be used and
> the set of situations where it must not be used are mutually exclusive:
>
>  * If the assembler resolves the address and the address will be used as a
>    branch target, BSYM() must be used.  This applies to non-cross-section
>    references within in single object only.
>

Here, 'sym | 1' should work ...

>  * If the linker resolves the address, BSYM() must not be used and the
>    target label must be annotated with ENDPROC().  This applies to
>    all cross-section or cross-object references.
>
> For any address that won't be used as a branch target, BSYM() must not
> be used.
>
> Cross-section non-cross-object references where the target is missing
> ENDPROC() and BSYM _is_ used will also work, but this should be avoided
> -- it's an abuse really.
>

... and for the other cases, we rely on the assembler to emit
relocations for the linker to process.

So in summary, defining BSYM() as 'sym + 1' rather than 'sym | 1' only
enables cases where BSYM() shouldn't be used in the first place. Or am
I missing something?

>
> The reason for these rules is that the assembler doesn't do any Thumb
> bit handling when taking the address of a local symbol, irrespective
> of the symbol's type.  This can result in a screwup unless the assembler
> can't resolve the address and must leave it as a relocation for the
> linker to process, or if the correct annotation for the linker to do
> the right fixup is missing.
>
>
> I don't know the history of why this inconsistency exists between the
> assembler and linker behaviour, but changing it would likely break
> more stuff than it fixes now.
>
> Maybe it would be a good idea to add a comment in asm/unified.h
> explaining this, assuming (hopefully) that my explanation was right...
>

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 11:10     ` Dave P Martin
  2015-04-21 11:15       ` Ard Biesheuvel
@ 2015-04-21 11:24       ` Russell King - ARM Linux
  2015-04-21 12:50         ` Russell King - ARM Linux
  1 sibling, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2015-04-21 11:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 12:10:43PM +0100, Dave P Martin wrote:
> On Tue, Apr 21, 2015 at 11:50:02AM +0100, Ard Biesheuvel wrote:
> > OK, that makes sense. But it does appear that the local cross-section
> > references are working just fine, i.e., references from other sections
> > in the same .o have the thumb bit set correctly even without BSYM()
> 
> For branch targets, the set of situations where BSYM must be used and
> the set of situations where it must not be used are mutually exclusive:
> 
>  * If the assembler resolves the address and the address will be used as a
>    branch target, BSYM() must be used.  This applies to non-cross-section
>    references within in single object only.
> 
>  * If the linker resolves the address, BSYM() must not be used and the
>    target label must be annotated with ENDPROC().  This applies to
>    all cross-section or cross-object references.

Yes, that agrees with the situation we have for the initfunc stuff.

> For any address that won't be used as a branch target, BSYM() must not
> be used.
> 
> Cross-section non-cross-object references where the target is missing
> ENDPROC() and BSYM _is_ used will also work, but this should be avoided
> -- it's an abuse really.

We should probably create a badr macro to complement the adr pseudo-op
which incorporates the BSYM thing so make this clearer - and a comment
before it.  This is really the case where BSYM should be used.

We have one case in the kernel source which is probably buggy:

arch/arm/kvm/interrupts.S:      ldr     r2, =BSYM(panic)

and killing BSYM in favour of a badr macro would prevent this.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 11:24       ` Russell King - ARM Linux
@ 2015-04-21 12:50         ` Russell King - ARM Linux
  2015-04-21 13:10           ` Ard Biesheuvel
  2015-04-21 13:18           ` Dave P Martin
  0 siblings, 2 replies; 27+ messages in thread
From: Russell King - ARM Linux @ 2015-04-21 12:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
> We should probably create a badr macro to complement the adr pseudo-op
> which incorporates the BSYM thing so make this clearer - and a comment
> before it.  This is really the case where BSYM should be used.

Something like this.  Note that I've also removed the BSYM() usage in
the KVM code.

 arch/arm/boot/compressed/head.S          |  4 ++--
 arch/arm/common/mcpm_head.S              |  2 +-
 arch/arm/include/asm/assembler.h         | 17 ++++++++++++++++-
 arch/arm/include/asm/entry-macro-multi.S |  4 ++--
 arch/arm/include/asm/unified.h           |  2 --
 arch/arm/kernel/entry-armv.S             | 12 ++++++------
 arch/arm/kernel/entry-common.S           |  6 +++---
 arch/arm/kernel/entry-ftrace.S           |  2 +-
 arch/arm/kernel/head-nommu.S             |  6 +++---
 arch/arm/kernel/head.S                   |  8 ++++----
 arch/arm/kernel/sleep.S                  |  2 +-
 arch/arm/kvm/interrupts.S                |  2 +-
 arch/arm/lib/call_with_stack.S           |  2 +-
 arch/arm/mm/proc-v7m.S                   |  2 +-
 14 files changed, 42 insertions(+), 29 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 2c45b5709fa4..06e983f59980 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -130,7 +130,7 @@ start:
 		.endr
    ARM(		mov	r0, r0		)
    ARM(		b	1f		)
- THUMB(		adr	r12, BSYM(1f)	)
+ THUMB(		badr	r12, 1f		)
  THUMB(		bx	r12		)
 
 		.word	_magic_sig	@ Magic numbers to help the loader
@@ -447,7 +447,7 @@ dtb_check_done:
 
 		bl	cache_clean_flush
 
-		adr	r0, BSYM(restart)
+		badr	r0, restart
 		add	r0, r0, r6
 		mov	pc, r0
 
diff --git a/arch/arm/common/mcpm_head.S b/arch/arm/common/mcpm_head.S
index e02db4b81a66..08b3bb9bc6a2 100644
--- a/arch/arm/common/mcpm_head.S
+++ b/arch/arm/common/mcpm_head.S
@@ -49,7 +49,7 @@
 ENTRY(mcpm_entry_point)
 
  ARM_BE8(setend        be)
- THUMB(	adr	r12, BSYM(1f)	)
+ THUMB(	badr	r12, 1f		)
  THUMB(	bx	r12		)
  THUMB(	.thumb			)
 1:
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 186270b3e194..4abe57279c66 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -178,6 +178,21 @@
 	.endm
 
 /*
+ * Assembly version of "adr rd, BSYM(sym)".  This should only be used to
+ * reference local symbols in the same assembly file which are to be
+ * resolved by the assembler.  Other usage is undefined.
+ */
+	.irp	c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo
+	.macro	badr\c, rd, sym
+#ifdef CONFIG_THUMB2_KERNEL
+	adr\c	\rd, \sym + 1
+#else
+	adr\c	\rd, \sym
+#endif
+	.endm
+	.endr
+
+/*
  * Get current thread_info.
  */
 	.macro	get_thread_info, rd
@@ -326,7 +341,7 @@
 THUMB(	orr	\reg , \reg , #PSR_T_BIT	)
 	bne	1f
 	orr	\reg, \reg, #PSR_A_BIT
-	adr	lr, BSYM(2f)
+	badr	lr, 2f
 	msr	spsr_cxsf, \reg
 	__MSR_ELR_HYP(14)
 	__ERET
diff --git a/arch/arm/include/asm/entry-macro-multi.S b/arch/arm/include/asm/entry-macro-multi.S
index 469a2b30fa27..609184f522ee 100644
--- a/arch/arm/include/asm/entry-macro-multi.S
+++ b/arch/arm/include/asm/entry-macro-multi.S
@@ -10,7 +10,7 @@
 	@
 	@ routine called with r0 = irq number, r1 = struct pt_regs *
 	@
-	adrne	lr, BSYM(1b)
+	badrne	lr, 1b
 	bne	asm_do_IRQ
 
 #ifdef CONFIG_SMP
@@ -23,7 +23,7 @@
 	ALT_SMP(test_for_ipi r0, r2, r6, lr)
 	ALT_UP_B(9997f)
 	movne	r1, sp
-	adrne	lr, BSYM(1b)
+	badrne	lr, 1b
 	bne	do_IPI
 #endif
 9997:
diff --git a/arch/arm/include/asm/unified.h b/arch/arm/include/asm/unified.h
index 200f9a7cd623..a91ae499614c 100644
--- a/arch/arm/include/asm/unified.h
+++ b/arch/arm/include/asm/unified.h
@@ -45,7 +45,6 @@
 #define THUMB(x...)	x
 #ifdef __ASSEMBLY__
 #define W(instr)	instr.w
-#define BSYM(sym)	sym + 1
 #else
 #define WASM(instr)	#instr ".w"
 #endif
@@ -59,7 +58,6 @@
 #define THUMB(x...)
 #ifdef __ASSEMBLY__
 #define W(instr)	instr
-#define BSYM(sym)	sym
 #else
 #define WASM(instr)	#instr
 #endif
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 570306c49406..f8f7398c74c2 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -40,7 +40,7 @@
 #ifdef CONFIG_MULTI_IRQ_HANDLER
 	ldr	r1, =handle_arch_irq
 	mov	r0, sp
-	adr	lr, BSYM(9997f)
+	badr	lr, 9997f
 	ldr	pc, [r1]
 #else
 	arch_irq_handler_default
@@ -273,7 +273,7 @@ __und_svc:
 	str	r4, [sp, #S_PC]
 	orr	r0, r9, r0, lsl #16
 #endif
-	adr	r9, BSYM(__und_svc_finish)
+	badr	r9, __und_svc_finish
 	mov	r2, r4
 	bl	call_fpe
 
@@ -469,7 +469,7 @@ __und_usr:
 	@ instruction, or the more conventional lr if we are to treat
 	@ this as a real undefined instruction
 	@
-	adr	r9, BSYM(ret_from_exception)
+	badr	r9, ret_from_exception
 
 	@ IRQs must be enabled before attempting to read the instruction from
 	@ user space since that could cause a page/translation fault if the
@@ -486,7 +486,7 @@ __und_usr:
 	@ r2 = PC value for the following instruction (:= regs->ARM_pc)
 	@ r4 = PC value for the faulting instruction
 	@ lr = 32-bit undefined instruction function
-	adr	lr, BSYM(__und_usr_fault_32)
+	badr	lr, __und_usr_fault_32
 	b	call_fpe
 
 __und_usr_thumb:
@@ -522,7 +522,7 @@ ARM_BE8(rev16	r0, r0)				@ little endian instruction
 	add	r2, r2, #2			@ r2 is PC + 2, make it PC + 4
 	str	r2, [sp, #S_PC]			@ it's a 2x16bit instr, update
 	orr	r0, r0, r5, lsl #16
-	adr	lr, BSYM(__und_usr_fault_32)
+	badr	lr, __und_usr_fault_32
 	@ r0 = the two 16-bit Thumb instructions which caused the exception
 	@ r2 = PC value for the following Thumb instruction (:= regs->ARM_pc)
 	@ r4 = PC value for the first 16-bit Thumb instruction
@@ -716,7 +716,7 @@ __und_usr_fault_32:
 __und_usr_fault_16:
 	mov	r1, #2
 1:	mov	r0, sp
-	adr	lr, BSYM(ret_from_exception)
+	badr	lr, ret_from_exception
 	b	__und_fault
 ENDPROC(__und_usr_fault_32)
 ENDPROC(__und_usr_fault_16)
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index f8ccc21fa032..6ab159384667 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -88,7 +88,7 @@ ENTRY(ret_from_fork)
 	bl	schedule_tail
 	cmp	r5, #0
 	movne	r0, r4
-	adrne	lr, BSYM(1f)
+	badrne	lr, 1f
 	retne	r5
 1:	get_thread_info tsk
 	b	ret_slow_syscall
@@ -196,7 +196,7 @@ local_restart:
 	bne	__sys_trace
 
 	cmp	scno, #NR_syscalls		@ check upper syscall limit
-	adr	lr, BSYM(ret_fast_syscall)	@ return address
+	badr	lr, ret_fast_syscall		@ return address
 	ldrcc	pc, [tbl, scno, lsl #2]		@ call sys_* routine
 
 	add	r1, sp, #S_OFF
@@ -231,7 +231,7 @@ __sys_trace:
 	add	r0, sp, #S_OFF
 	bl	syscall_trace_enter
 
-	adr	lr, BSYM(__sys_trace_return)	@ return address
+	badr	lr, __sys_trace_return		@ return address
 	mov	scno, r0			@ syscall number (possibly new)
 	add	r1, sp, #S_R0 + S_OFF		@ pointer to regs
 	cmp	scno, #NR_syscalls		@ check upper syscall limit
diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index fe57c73e70a4..c73c4030ca5d 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -87,7 +87,7 @@
 
 1: 	mcount_get_lr	r1			@ lr of instrumented func
 	mcount_adjust_addr	r0, lr		@ instrumented function
-	adr	lr, BSYM(2f)
+	badr	lr, 2f
 	mov	pc, r2
 2:	mcount_exit
 .endm
diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index 84da14b7cd04..c9660167ef1a 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -46,7 +46,7 @@ ENTRY(stext)
 	.arm
 ENTRY(stext)
 
- THUMB(	adr	r9, BSYM(1f)	)	@ Kernel is always entered in ARM.
+ THUMB(	badr	r9, 1f		)	@ Kernel is always entered in ARM.
  THUMB(	bx	r9		)	@ If this is a Thumb-2 kernel,
  THUMB(	.thumb			)	@ switch to Thumb now.
  THUMB(1:			)
@@ -79,7 +79,7 @@ ENTRY(stext)
 #endif
 	ldr	r13, =__mmap_switched		@ address to jump to after
 						@ initialising sctlr
-	adr	lr, BSYM(1f)			@ return (PIC) address
+	badr	lr, 1f				@ return (PIC) address
 	ldr	r12, [r10, #PROCINFO_INITFUNC]
 	add	r12, r12, r10
 	ret	r12
@@ -115,7 +115,7 @@ ENTRY(secondary_startup)
 	bl      __setup_mpu			@ Initialize the MPU
 #endif
 
-	adr	lr, BSYM(__after_proc_init)	@ return address
+	badr	lr, __after_proc_init		@ return address
 	mov	r13, r12			@ __secondary_switched address
 	ldr	r12, [r10, #PROCINFO_INITFUNC]
 	add	r12, r12, r10
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 7304b4c44b52..1eecd57453be 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -80,7 +80,7 @@
 ENTRY(stext)
  ARM_BE8(setend	be )			@ ensure we are in BE8 mode
 
- THUMB(	adr	r9, BSYM(1f)	)	@ Kernel is always entered in ARM.
+ THUMB(	badr	r9, 1f		)	@ Kernel is always entered in ARM.
  THUMB(	bx	r9		)	@ If this is a Thumb-2 kernel,
  THUMB(	.thumb			)	@ switch to Thumb now.
  THUMB(1:			)
@@ -148,7 +148,7 @@ ENTRY(stext)
 	 */
 	ldr	r13, =__mmap_switched		@ address to jump to after
 						@ mmu has been enabled
-	adr	lr, BSYM(1f)			@ return (PIC) address
+	badr	lr, 1f				@ return (PIC) address
 #ifdef CONFIG_ARM_LPAE
 	mov	r5, #0				@ high TTBR0
 	mov	r8, r4, lsr #12			@ TTBR1 is swapper_pg_dir pfn
@@ -364,7 +364,7 @@ __turn_mmu_on_loc:
 	.text
 ENTRY(secondary_startup_arm)
 	.arm
- THUMB(	adr	r9, BSYM(1f)	)	@ Kernel is entered in ARM.
+ THUMB(	badr	r9, 1f		)	@ Kernel is entered in ARM.
  THUMB(	bx	r9		)	@ If this is a Thumb-2 kernel,
  THUMB(	.thumb			)	@ switch to Thumb now.
  THUMB(1:			)
@@ -400,7 +400,7 @@ ENTRY(secondary_startup)
 	add	r3, r7, lr
 	ldrd	r4, [r3, #0]			@ get secondary_data.pgdir
 	ldr	r8, [r3, #8]			@ get secondary_data.swapper_pg_dir
-	adr	lr, BSYM(__enable_mmu)		@ return address
+	badr	lr, __enable_mmu		@ return address
 	mov	r13, r12			@ __secondary_switched address
 	ldr	r12, [r10, #PROCINFO_INITFUNC]
 	add	r12, r12, r10			@ initialise processor
diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S
index 7d37bfc50830..76bb3128e135 100644
--- a/arch/arm/kernel/sleep.S
+++ b/arch/arm/kernel/sleep.S
@@ -81,7 +81,7 @@ ENTRY(__cpu_suspend)
 	mov	r1, r4			@ size of save block
 	add	r0, sp, #8		@ pointer to save block
 	bl	__cpu_suspend_save
-	adr	lr, BSYM(cpu_suspend_abort)
+	badr	lr, cpu_suspend_abort
 	ldmfd	sp!, {r0, pc}		@ call suspend fn
 ENDPROC(__cpu_suspend)
 	.ltorg
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
diff --git a/arch/arm/lib/call_with_stack.S b/arch/arm/lib/call_with_stack.S
index ed1a421813cb..bf3a40889205 100644
--- a/arch/arm/lib/call_with_stack.S
+++ b/arch/arm/lib/call_with_stack.S
@@ -35,7 +35,7 @@ ENTRY(call_with_stack)
 	mov	r2, r0
 	mov	r0, r1
 
-	adr	lr, BSYM(1f)
+	badr	lr, 1f
 	ret	r2
 
 1:	ldr	lr, [sp]
diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index e08e1f2bab76..67d9209077c6 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -98,7 +98,7 @@ __v7m_setup:
 	str	r5, [r0, V7M_SCB_SHPR3]	@ set PendSV priority
 
 	@ SVC to run the kernel in this mode
-	adr	r1, BSYM(1f)
+	badr	r1, 1f
 	ldr	r5, [r12, #11 * 4]	@ read the SVC vector entry
 	str	r1, [r12, #11 * 4]	@ write the temporary SVC vector entry
 	mov	r6, lr			@ save LR

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

^ permalink raw reply related	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 12:50         ` Russell King - ARM Linux
@ 2015-04-21 13:10           ` Ard Biesheuvel
  2015-04-21 13:21             ` Dave P Martin
  2015-04-21 13:18           ` Dave P Martin
  1 sibling, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2015-04-21 13:10 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 April 2015 at 14:50, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
>> We should probably create a badr macro to complement the adr pseudo-op
>> which incorporates the BSYM thing so make this clearer - and a comment
>> before it.  This is really the case where BSYM should be used.
>
> Something like this.  Note that I've also removed the BSYM() usage in
> the KVM code.
>

Yes, that is much better. It is a pity that we still can't use '| 1'
but the fact that you are forced to use 'adr' now probably mostly
eliminates the risk regarding that.

I did notice that are are 4 or 5 instances (commented inline) of an
ARM to thumb mode switch which can just as easily be implemented as
'blx 1f' instead of using this badr macro (whose use we want to
discourage, I assume, since the address arithmetic is still slightly
dodgy). Do you think we should do something about that as well?

>  arch/arm/boot/compressed/head.S          |  4 ++--
>  arch/arm/common/mcpm_head.S              |  2 +-
>  arch/arm/include/asm/assembler.h         | 17 ++++++++++++++++-
>  arch/arm/include/asm/entry-macro-multi.S |  4 ++--
>  arch/arm/include/asm/unified.h           |  2 --
>  arch/arm/kernel/entry-armv.S             | 12 ++++++------
>  arch/arm/kernel/entry-common.S           |  6 +++---
>  arch/arm/kernel/entry-ftrace.S           |  2 +-
>  arch/arm/kernel/head-nommu.S             |  6 +++---
>  arch/arm/kernel/head.S                   |  8 ++++----
>  arch/arm/kernel/sleep.S                  |  2 +-
>  arch/arm/kvm/interrupts.S                |  2 +-
>  arch/arm/lib/call_with_stack.S           |  2 +-
>  arch/arm/mm/proc-v7m.S                   |  2 +-
>  14 files changed, 42 insertions(+), 29 deletions(-)
>
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 2c45b5709fa4..06e983f59980 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -130,7 +130,7 @@ start:
>                 .endr
>     ARM(                mov     r0, r0          )
>     ARM(                b       1f              )
> - THUMB(                adr     r12, BSYM(1f)   )
> + THUMB(                badr    r12, 1f         )
>   THUMB(                bx      r12             )
>

blx 1f

>                 .word   _magic_sig      @ Magic numbers to help the loader
> @@ -447,7 +447,7 @@ dtb_check_done:
>
>                 bl      cache_clean_flush
>
> -               adr     r0, BSYM(restart)
> +               badr    r0, restart
>                 add     r0, r0, r6
>                 mov     pc, r0
>
> diff --git a/arch/arm/common/mcpm_head.S b/arch/arm/common/mcpm_head.S
> index e02db4b81a66..08b3bb9bc6a2 100644
> --- a/arch/arm/common/mcpm_head.S
> +++ b/arch/arm/common/mcpm_head.S
> @@ -49,7 +49,7 @@
>  ENTRY(mcpm_entry_point)
>
>   ARM_BE8(setend        be)
> - THUMB(        adr     r12, BSYM(1f)   )
> + THUMB(        badr    r12, 1f         )
>   THUMB(        bx      r12             )
>   THUMB(        .thumb                  )
>  1:

likewise

> [...]
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index 84da14b7cd04..c9660167ef1a 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -46,7 +46,7 @@ ENTRY(stext)
>         .arm
>  ENTRY(stext)
>
> - THUMB(        adr     r9, BSYM(1f)    )       @ Kernel is always entered in ARM.
> + THUMB(        badr    r9, 1f          )       @ Kernel is always entered in ARM.
>   THUMB(        bx      r9              )       @ If this is a Thumb-2 kernel,
>   THUMB(        .thumb                  )       @ switch to Thumb now.
>   THUMB(1:                      )

likewise

> @@ -79,7 +79,7 @@ ENTRY(stext)
>  #endif
>         ldr     r13, =__mmap_switched           @ address to jump to after
>                                                 @ initialising sctlr
> -       adr     lr, BSYM(1f)                    @ return (PIC) address
> +       badr    lr, 1f                          @ return (PIC) address
>         ldr     r12, [r10, #PROCINFO_INITFUNC]
>         add     r12, r12, r10
>         ret     r12
> @@ -115,7 +115,7 @@ ENTRY(secondary_startup)
>         bl      __setup_mpu                     @ Initialize the MPU
>  #endif
>
> -       adr     lr, BSYM(__after_proc_init)     @ return address
> +       badr    lr, __after_proc_init           @ return address
>         mov     r13, r12                        @ __secondary_switched address
>         ldr     r12, [r10, #PROCINFO_INITFUNC]
>         add     r12, r12, r10
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 7304b4c44b52..1eecd57453be 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -80,7 +80,7 @@
>  ENTRY(stext)
>   ARM_BE8(setend        be )                    @ ensure we are in BE8 mode
>
> - THUMB(        adr     r9, BSYM(1f)    )       @ Kernel is always entered in ARM.
> + THUMB(        badr    r9, 1f          )       @ Kernel is always entered in ARM.
>   THUMB(        bx      r9              )       @ If this is a Thumb-2 kernel,
>   THUMB(        .thumb                  )       @ switch to Thumb now.
>   THUMB(1:                      )

likewise

> @@ -148,7 +148,7 @@ ENTRY(stext)
>          */
>         ldr     r13, =__mmap_switched           @ address to jump to after
>                                                 @ mmu has been enabled
> -       adr     lr, BSYM(1f)                    @ return (PIC) address
> +       badr    lr, 1f                          @ return (PIC) address
>  #ifdef CONFIG_ARM_LPAE
>         mov     r5, #0                          @ high TTBR0
>         mov     r8, r4, lsr #12                 @ TTBR1 is swapper_pg_dir pfn
> @@ -364,7 +364,7 @@ __turn_mmu_on_loc:
>         .text
>  ENTRY(secondary_startup_arm)
>         .arm
> - THUMB(        adr     r9, BSYM(1f)    )       @ Kernel is entered in ARM.
> + THUMB(        badr    r9, 1f          )       @ Kernel is entered in ARM.
>   THUMB(        bx      r9              )       @ If this is a Thumb-2 kernel,
>   THUMB(        .thumb                  )       @ switch to Thumb now.
>   THUMB(1:                      )

likewise

-- 
Ard.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 12:50         ` Russell King - ARM Linux
  2015-04-21 13:10           ` Ard Biesheuvel
@ 2015-04-21 13:18           ` Dave P Martin
  2015-04-21 13:55             ` Russell King - ARM Linux
  1 sibling, 1 reply; 27+ messages in thread
From: Dave P Martin @ 2015-04-21 13:18 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 01:50:50PM +0100, Russell King - ARM Linux wrote:
> On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
> > We should probably create a badr macro to complement the adr pseudo-op
> > which incorporates the BSYM thing so make this clearer - and a comment
> > before it.  This is really the case where BSYM should be used.
> 
> Something like this.  Note that I've also removed the BSYM() usage in
> the KVM code.

Nice.  Wrapping this around adr will make the assembler check that it's
not a cross-section reference too.

>  arch/arm/boot/compressed/head.S          |  4 ++--
>  arch/arm/common/mcpm_head.S              |  2 +-
>  arch/arm/include/asm/assembler.h         | 17 ++++++++++++++++-
>  arch/arm/include/asm/entry-macro-multi.S |  4 ++--
>  arch/arm/include/asm/unified.h           |  2 --
>  arch/arm/kernel/entry-armv.S             | 12 ++++++------
>  arch/arm/kernel/entry-common.S           |  6 +++---
>  arch/arm/kernel/entry-ftrace.S           |  2 +-
>  arch/arm/kernel/head-nommu.S             |  6 +++---
>  arch/arm/kernel/head.S                   |  8 ++++----
>  arch/arm/kernel/sleep.S                  |  2 +-
>  arch/arm/kvm/interrupts.S                |  2 +-
>  arch/arm/lib/call_with_stack.S           |  2 +-
>  arch/arm/mm/proc-v7m.S                   |  2 +-
>  14 files changed, 42 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 2c45b5709fa4..06e983f59980 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -130,7 +130,7 @@ start:
>                 .endr
>     ARM(                mov     r0, r0          )
>     ARM(                b       1f              )
> - THUMB(                adr     r12, BSYM(1f)   )
> + THUMB(                badr    r12, 1f         )
>   THUMB(                bx      r12             )
> 
>                 .word   _magic_sig      @ Magic numbers to help the loader
> @@ -447,7 +447,7 @@ dtb_check_done:
> 
>                 bl      cache_clean_flush
> 
> -               adr     r0, BSYM(restart)
> +               badr    r0, restart
>                 add     r0, r0, r6
>                 mov     pc, r0
> 
> diff --git a/arch/arm/common/mcpm_head.S b/arch/arm/common/mcpm_head.S
> index e02db4b81a66..08b3bb9bc6a2 100644
> --- a/arch/arm/common/mcpm_head.S
> +++ b/arch/arm/common/mcpm_head.S
> @@ -49,7 +49,7 @@
>  ENTRY(mcpm_entry_point)
> 
>   ARM_BE8(setend        be)
> - THUMB(        adr     r12, BSYM(1f)   )
> + THUMB(        badr    r12, 1f         )
>   THUMB(        bx      r12             )
>   THUMB(        .thumb                  )
>  1:
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index 186270b3e194..4abe57279c66 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -178,6 +178,21 @@
>         .endm
> 
>  /*
> + * Assembly version of "adr rd, BSYM(sym)".  This should only be used to
> + * reference local symbols in the same assembly file which are to be
> + * resolved by the assembler.  Other usage is undefined.

BSYM() is gone, so this comment shouldn't refer to it...

> + */
> +       .irp    c,,eq,ne,cs,cc,mi,pl,vs,vc,hi,ls,ge,lt,gt,le,hs,lo

This wrap-macro-with-cc idiom could be factored, but it may not be worth
it just yet.

[...]

Cheers
---Dave

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 13:10           ` Ard Biesheuvel
@ 2015-04-21 13:21             ` Dave P Martin
  2015-04-21 13:28               ` Ard Biesheuvel
  2015-04-21 14:05               ` Russell King - ARM Linux
  0 siblings, 2 replies; 27+ messages in thread
From: Dave P Martin @ 2015-04-21 13:21 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 02:10:42PM +0100, Ard Biesheuvel wrote:
> On 21 April 2015 at 14:50, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
> >> We should probably create a badr macro to complement the adr pseudo-op
> >> which incorporates the BSYM thing so make this clearer - and a comment
> >> before it.  This is really the case where BSYM should be used.
> >
> > Something like this.  Note that I've also removed the BSYM() usage in
> > the KVM code.
> >
> 
> Yes, that is much better. It is a pity that we still can't use '| 1'
> but the fact that you are forced to use 'adr' now probably mostly
> eliminates the risk regarding that.
> 
> I did notice that are are 4 or 5 instances (commented inline) of an
> ARM to thumb mode switch which can just as easily be implemented as
> 'blx 1f' instead of using this badr macro (whose use we want to
> discourage, I assume, since the address arithmetic is still slightly
> dodgy). Do you think we should do something about that as well?

Err, probably.  That just looks like an oversight -- I think I'm
responsible for at least some of those.

There's no good reason not to replace adr+BSYM+bx.

For switches from ARM, this could be replaced with bx <label> which
should work just fine.  There shouldn't be any instances of this from
Thumb, because switching instruction set is the whole point here.

(Thumb doesn't have bx <label>, but blx <label> is available at the cost
of clobbering lr).

Cheers
---Dave

[...]

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 13:21             ` Dave P Martin
@ 2015-04-21 13:28               ` Ard Biesheuvel
  2015-04-21 15:51                 ` Dave Martin
  2015-04-21 14:05               ` Russell King - ARM Linux
  1 sibling, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2015-04-21 13:28 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 April 2015 at 15:21, Dave P Martin <Dave.Martin@arm.com> wrote:
> On Tue, Apr 21, 2015 at 02:10:42PM +0100, Ard Biesheuvel wrote:
>> On 21 April 2015 at 14:50, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
>> >> We should probably create a badr macro to complement the adr pseudo-op
>> >> which incorporates the BSYM thing so make this clearer - and a comment
>> >> before it.  This is really the case where BSYM should be used.
>> >
>> > Something like this.  Note that I've also removed the BSYM() usage in
>> > the KVM code.
>> >
>>
>> Yes, that is much better. It is a pity that we still can't use '| 1'
>> but the fact that you are forced to use 'adr' now probably mostly
>> eliminates the risk regarding that.
>>
>> I did notice that are are 4 or 5 instances (commented inline) of an
>> ARM to thumb mode switch which can just as easily be implemented as
>> 'blx 1f' instead of using this badr macro (whose use we want to
>> discourage, I assume, since the address arithmetic is still slightly
>> dodgy). Do you think we should do something about that as well?
>
> Err, probably.  That just looks like an oversight -- I think I'm
> responsible for at least some of those.
>
> There's no good reason not to replace adr+BSYM+bx.
>
> For switches from ARM, this could be replaced with bx <label> which
> should work just fine.  There shouldn't be any instances of this from
> Thumb, because switching instruction set is the whole point here.
>
> (Thumb doesn't have bx <label>, but blx <label> is available at the cost
> of clobbering lr).
>

It appears neither have 'bx <label>', but 'add pc, pc, #1; nop' does
the job nicely as well.
And as you say, there are no instances of Thumb->ARM mode switches.

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 13:18           ` Dave P Martin
@ 2015-04-21 13:55             ` Russell King - ARM Linux
  2015-04-21 14:06               ` Ard Biesheuvel
  0 siblings, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2015-04-21 13:55 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 02:18:03PM +0100, Dave P Martin wrote:
> On Tue, Apr 21, 2015 at 01:50:50PM +0100, Russell King - ARM Linux wrote:
> > On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
> > > We should probably create a badr macro to complement the adr pseudo-op
> > > which incorporates the BSYM thing so make this clearer - and a comment
> > > before it.  This is really the case where BSYM should be used.
> > 
> > Something like this.  Note that I've also removed the BSYM() usage in
> > the KVM code.
> 
> Nice.  Wrapping this around adr will make the assembler check that it's
> not a cross-section reference too.

While looking at this, I've become very upset with our toolchain's
abilities.  This is with stock binutils 2.22-2.25, and Ard's suggestion
for using blx:

00000000 <secondary_startup_arm>:
   0:   fafffffe        blx     4 <secondary_startup>

00000004 <secondary_startup>:
   4:   f7ff fffe       bl      0 <__hyp_stub_install_secondary>
   8:   f3ef 8900       mrs     r9, CPSR
   c:   f089 091a       eor.w   r9, r9, #26
  10:   f019 0f1f       tst.w   r9, #31

That's fine, but now if we look at the .head.text section (I also added
an ENTRY(start) to try and solve this):

00000000 <stext>:
   0:   ffff faff                       ; <UNDEFINED> instruction: 0xfffffaff

00000004 <start>:
   4:   fffef7ff        .word   0xfffef7ff
   8:   f3ef 8900       mrs     r9, CPSR
   c:   091af089        .word   0x091af089
  10:   f019 0f1f       tst.w   r9, #31
  14:   091ff029        .word   0x091ff029
  18:   09d3f049        .word   0x09d3f049
  1c:   f049 0920       orr.w   r9, r9, #32
  20:   f449d109        .word   0xf449d109
  24:   f20f7980        .word   0xf20f7980
  28:   0e13            lsrs    r3, r2, #24
  2a:   f399            .short  0xf399
  2c:   8f00            ldrh    r0, [r0, #56]   ; 0x38
  2e:   f38e            .short  0xf38e
  30:   f3de8e30        .word   0xf3de8e30
  34:   8f00            .short  0x8f00
  36:   f389 8100       msr     CPSR_c, r9

readelf for this shows for section 5:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 5] .head.text        PROGBITS        00000000 000290 000254 00  AX  0   0  4
...
   Num:    Value  Size Type    Bind   Vis      Ndx Name
     4: 00000000     0 SECTION LOCAL  DEFAULT    5
     5: 00000000     0 NOTYPE  LOCAL  DEFAULT    5 $a
     6: 00000004     0 NOTYPE  LOCAL  DEFAULT    5 $t
     7: 0000002e     0 NOTYPE  LOCAL  DEFAULT    5 $d
     8: 00000036     0 NOTYPE  LOCAL  DEFAULT    5 $t
...
    65: 00000000     4 FUNC    GLOBAL DEFAULT    5 stext
    66: 00000005   122 FUNC    GLOBAL DEFAULT    5 start

One has to wonder about the toolchain when the stupid $[adt] hack seems
to be going soooo wrong.

I think I'm going to stop working on this until we have a toolchain
which behaves sensibly... when you can't get the damned thing to
disassemble for confirmation purposes, its best to leave the damned
code alone.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 13:21             ` Dave P Martin
  2015-04-21 13:28               ` Ard Biesheuvel
@ 2015-04-21 14:05               ` Russell King - ARM Linux
  1 sibling, 0 replies; 27+ messages in thread
From: Russell King - ARM Linux @ 2015-04-21 14:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 02:21:46PM +0100, Dave P Martin wrote:
> On Tue, Apr 21, 2015 at 02:10:42PM +0100, Ard Biesheuvel wrote:
> > Yes, that is much better. It is a pity that we still can't use '| 1'
> > but the fact that you are forced to use 'adr' now probably mostly
> > eliminates the risk regarding that.
> > 
> > I did notice that are are 4 or 5 instances (commented inline) of an
> > ARM to thumb mode switch which can just as easily be implemented as
> > 'blx 1f' instead of using this badr macro (whose use we want to
> > discourage, I assume, since the address arithmetic is still slightly
> > dodgy). Do you think we should do something about that as well?
> 
> Err, probably.  That just looks like an oversight -- I think I'm
> responsible for at least some of those.
> 
> There's no good reason not to replace adr+BSYM+bx.
> 
> For switches from ARM, this could be replaced with bx <label> which
> should work just fine.  There shouldn't be any instances of this from
> Thumb, because switching instruction set is the whole point here.

Where do you get this bx <label> from?  It doesn't seem to be in
DDI0406C.

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

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 13:55             ` Russell King - ARM Linux
@ 2015-04-21 14:06               ` Ard Biesheuvel
  2015-04-21 17:03                 ` Dave Martin
  0 siblings, 1 reply; 27+ messages in thread
From: Ard Biesheuvel @ 2015-04-21 14:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 21 April 2015 at 15:55, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Apr 21, 2015 at 02:18:03PM +0100, Dave P Martin wrote:
>> On Tue, Apr 21, 2015 at 01:50:50PM +0100, Russell King - ARM Linux wrote:
>> > On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
>> > > We should probably create a badr macro to complement the adr pseudo-op
>> > > which incorporates the BSYM thing so make this clearer - and a comment
>> > > before it.  This is really the case where BSYM should be used.
>> >
>> > Something like this.  Note that I've also removed the BSYM() usage in
>> > the KVM code.
>>
>> Nice.  Wrapping this around adr will make the assembler check that it's
>> not a cross-section reference too.
>
> While looking at this, I've become very upset with our toolchain's
> abilities.  This is with stock binutils 2.22-2.25, and Ard's suggestion
> for using blx:
>
> 00000000 <secondary_startup_arm>:
>    0:   fafffffe        blx     4 <secondary_startup>
>
> 00000004 <secondary_startup>:
>    4:   f7ff fffe       bl      0 <__hyp_stub_install_secondary>
>    8:   f3ef 8900       mrs     r9, CPSR
>    c:   f089 091a       eor.w   r9, r9, #26
>   10:   f019 0f1f       tst.w   r9, #31
>
> That's fine, but now if we look at the .head.text section (I also added
> an ENTRY(start) to try and solve this):
>
> 00000000 <stext>:
>    0:   ffff faff                       ; <UNDEFINED> instruction: 0xfffffaff
>
> 00000004 <start>:
>    4:   fffef7ff        .word   0xfffef7ff
>    8:   f3ef 8900       mrs     r9, CPSR
>    c:   091af089        .word   0x091af089
>   10:   f019 0f1f       tst.w   r9, #31
>   14:   091ff029        .word   0x091ff029
>   18:   09d3f049        .word   0x09d3f049
>   1c:   f049 0920       orr.w   r9, r9, #32
>   20:   f449d109        .word   0xf449d109
>   24:   f20f7980        .word   0xf20f7980
>   28:   0e13            lsrs    r3, r2, #24
>   2a:   f399            .short  0xf399
>   2c:   8f00            ldrh    r0, [r0, #56]   ; 0x38
>   2e:   f38e            .short  0xf38e
>   30:   f3de8e30        .word   0xf3de8e30
>   34:   8f00            .short  0x8f00
>   36:   f389 8100       msr     CPSR_c, r9
>
> readelf for this shows for section 5:
>
> Section Headers:
>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>   [ 5] .head.text        PROGBITS        00000000 000290 000254 00  AX  0   0  4
> ...
>    Num:    Value  Size Type    Bind   Vis      Ndx Name
>      4: 00000000     0 SECTION LOCAL  DEFAULT    5
>      5: 00000000     0 NOTYPE  LOCAL  DEFAULT    5 $a
>      6: 00000004     0 NOTYPE  LOCAL  DEFAULT    5 $t
>      7: 0000002e     0 NOTYPE  LOCAL  DEFAULT    5 $d
>      8: 00000036     0 NOTYPE  LOCAL  DEFAULT    5 $t
> ...
>     65: 00000000     4 FUNC    GLOBAL DEFAULT    5 stext
>     66: 00000005   122 FUNC    GLOBAL DEFAULT    5 start
>
> One has to wonder about the toolchain when the stupid $[adt] hack seems
> to be going soooo wrong.
>
> I think I'm going to stop working on this until we have a toolchain
> which behaves sensibly... when you can't get the damned thing to
> disassemble for confirmation purposes, its best to leave the damned
> code alone.
>

It indeed seems to be objdump that is failing, but the code itself
looks fine to me.  For the record, binutils v2.23.52 works fine

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 13:28               ` Ard Biesheuvel
@ 2015-04-21 15:51                 ` Dave Martin
  0 siblings, 0 replies; 27+ messages in thread
From: Dave Martin @ 2015-04-21 15:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 03:28:14PM +0200, Ard Biesheuvel wrote:
> On 21 April 2015 at 15:21, Dave P Martin <Dave.Martin@arm.com> wrote:
> > On Tue, Apr 21, 2015 at 02:10:42PM +0100, Ard Biesheuvel wrote:
> >> On 21 April 2015 at 14:50, Russell King - ARM Linux
> >> <linux@arm.linux.org.uk> wrote:
> >> > On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
> >> >> We should probably create a badr macro to complement the adr pseudo-op
> >> >> which incorporates the BSYM thing so make this clearer - and a comment
> >> >> before it.  This is really the case where BSYM should be used.
> >> >
> >> > Something like this.  Note that I've also removed the BSYM() usage in
> >> > the KVM code.
> >> >
> >>
> >> Yes, that is much better. It is a pity that we still can't use '| 1'
> >> but the fact that you are forced to use 'adr' now probably mostly
> >> eliminates the risk regarding that.
> >>
> >> I did notice that are are 4 or 5 instances (commented inline) of an
> >> ARM to thumb mode switch which can just as easily be implemented as
> >> 'blx 1f' instead of using this badr macro (whose use we want to
> >> discourage, I assume, since the address arithmetic is still slightly
> >> dodgy). Do you think we should do something about that as well?
> >
> > Err, probably.  That just looks like an oversight -- I think I'm
> > responsible for at least some of those.
> >
> > There's no good reason not to replace adr+BSYM+bx.
> >
> > For switches from ARM, this could be replaced with bx <label> which
> > should work just fine.  There shouldn't be any instances of this from
> > Thumb, because switching instruction set is the whole point here.
> >
> > (Thumb doesn't have bx <label>, but blx <label> is available at the cost
> > of clobbering lr).
> >
> 
> It appears neither have 'bx <label>', but 'add pc, pc, #1; nop' does

Duh, I'm getting myself confused.  Yes, both have blx <label> but not
bx <label>.

Note that blx may result in suboptimal branch prediction because it
looks like a function call.  But these aren't hot paths, so I doubt
it matters.

> the job nicely as well.

Bit icky though...

Cheers
---Dave

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2015-04-21 14:06               ` Ard Biesheuvel
@ 2015-04-21 17:03                 ` Dave Martin
  0 siblings, 0 replies; 27+ messages in thread
From: Dave Martin @ 2015-04-21 17:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Apr 21, 2015 at 04:06:02PM +0200, Ard Biesheuvel wrote:
> On 21 April 2015 at 15:55, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Apr 21, 2015 at 02:18:03PM +0100, Dave P Martin wrote:
> >> On Tue, Apr 21, 2015 at 01:50:50PM +0100, Russell King - ARM Linux wrote:
> >> > On Tue, Apr 21, 2015 at 12:24:20PM +0100, Russell King - ARM Linux wrote:
> >> > > We should probably create a badr macro to complement the adr pseudo-op
> >> > > which incorporates the BSYM thing so make this clearer - and a comment
> >> > > before it.  This is really the case where BSYM should be used.
> >> >
> >> > Something like this.  Note that I've also removed the BSYM() usage in
> >> > the KVM code.
> >>
> >> Nice.  Wrapping this around adr will make the assembler check that it's
> >> not a cross-section reference too.
> >
> > While looking at this, I've become very upset with our toolchain's
> > abilities.  This is with stock binutils 2.22-2.25, and Ard's suggestion
> > for using blx:
> >
> > 00000000 <secondary_startup_arm>:
> >    0:   fafffffe        blx     4 <secondary_startup>
> >
> > 00000004 <secondary_startup>:
> >    4:   f7ff fffe       bl      0 <__hyp_stub_install_secondary>
> >    8:   f3ef 8900       mrs     r9, CPSR
> >    c:   f089 091a       eor.w   r9, r9, #26
> >   10:   f019 0f1f       tst.w   r9, #31
> >
> > That's fine, but now if we look at the .head.text section (I also added
> > an ENTRY(start) to try and solve this):
> >
> > 00000000 <stext>:
> >    0:   ffff faff                       ; <UNDEFINED> instruction: 0xfffffaff
> >
> > 00000004 <start>:
> >    4:   fffef7ff        .word   0xfffef7ff
> >    8:   f3ef 8900       mrs     r9, CPSR
> >    c:   091af089        .word   0x091af089
> >   10:   f019 0f1f       tst.w   r9, #31
> >   14:   091ff029        .word   0x091ff029
> >   18:   09d3f049        .word   0x09d3f049
> >   1c:   f049 0920       orr.w   r9, r9, #32
> >   20:   f449d109        .word   0xf449d109
> >   24:   f20f7980        .word   0xf20f7980
> >   28:   0e13            lsrs    r3, r2, #24
> >   2a:   f399            .short  0xf399
> >   2c:   8f00            ldrh    r0, [r0, #56]   ; 0x38
> >   2e:   f38e            .short  0xf38e
> >   30:   f3de8e30        .word   0xf3de8e30
> >   34:   8f00            .short  0x8f00
> >   36:   f389 8100       msr     CPSR_c, r9
> >
> > readelf for this shows for section 5:
> >
> > Section Headers:
> >   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> >   [ 5] .head.text        PROGBITS        00000000 000290 000254 00  AX  0   0  4
> > ...
> >    Num:    Value  Size Type    Bind   Vis      Ndx Name
> >      4: 00000000     0 SECTION LOCAL  DEFAULT    5
> >      5: 00000000     0 NOTYPE  LOCAL  DEFAULT    5 $a
> >      6: 00000004     0 NOTYPE  LOCAL  DEFAULT    5 $t
> >      7: 0000002e     0 NOTYPE  LOCAL  DEFAULT    5 $d
> >      8: 00000036     0 NOTYPE  LOCAL  DEFAULT    5 $t
> > ...
> >     65: 00000000     4 FUNC    GLOBAL DEFAULT    5 stext
> >     66: 00000005   122 FUNC    GLOBAL DEFAULT    5 start
> >
> > One has to wonder about the toolchain when the stupid $[adt] hack seems
> > to be going soooo wrong.
> >
> > I think I'm going to stop working on this until we have a toolchain
> > which behaves sensibly... when you can't get the damned thing to
> > disassemble for confirmation purposes, its best to leave the damned
> > code alone.
> >
> 
> It indeed seems to be objdump that is failing, but the code itself
> looks fine to me.  For the record, binutils v2.23.52 works fine

I've come across weird disassembly issues like this in the past.
The objdump code is something of a fragmented mess (shock, horror),
but I think there were fixes in the pipeline for some issues of this
sort.

If it is possible to manually check most or all of the cases of
badr, that might be sufficient -- this only gets used in a few,
specific places.

Objdump doesn't inspire much confidence though :(

Cheers
---Dave

^ permalink raw reply	[flat|nested] 27+ messages in thread

* your mail
  2017-06-04 11:59 No subject Yury Norov
@ 2017-06-14 20:16 ` Yury Norov
  0 siblings, 0 replies; 27+ messages in thread
From: Yury Norov @ 2017-06-14 20:16 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Catalin, all.

Thank you for your time on reviewing the series. I really appreciate it.

This is the updated version where I tried to address all comments:
https://github.com/norov/linux/commits/ilp32-20170613.4

(3 last patches here is the Andrew Pinski's rework of vdso rebased on
ilp32 series)

If nothing will come here on review, I'll send v8 at the beginning of
the next week. Is this plan OK?

And this is the backport on the v4.11 kernel:
https://github.com/norov/linux/commits/ilp32-4.11.4

Yury

On Sun, Jun 04, 2017 at 02:59:49PM +0300, Yury Norov wrote:
> Subject: [PATCH v7 resend 2 00/20] ILP32 for ARM64
> 
> Hi Catalin,
>  
> Here is a rebase of latest kernel patchset against next-20170602. There's almost
> no changes, but there are some conflicts that are not trivial, and I'd like to
> refresh the submission therefore.
> 
> How are your experiments with testing and benchmarking of ILP32 are going? In
> my current tests I see 0 failures on LTP. Benchmarking on SPEC CPU2006 and
> LMBench shows no difference for LP64 and expected performance boost for ILP32
> (compared to LP64 results).
> 
> Steve Ellcey is handling upstream submission of Glibc patches. The patches are
> ready and have been reviewed and reworked per community?s comments. There are
> no outstanding userspace ABI issues from Glibc. Glibc submission is now waiting
> on ILP32 kernel submission.
> 
> Catalin, regarding rootfs, is OpenSuSe?s build sufficient for your experiments?
> I?ve also seen Wookey merging patches for ILP32 triplet to binutils and pushing
> them to Debian.
> 
> One last thing I wanted to check with you about is ILP32 PCS - does, in your
> view, ARM Ltd. needs to publish any additional docs for ABI to become official?
> 
> Below is the regular description.
> 
> Thanks.
> Yury
> 
> --------
> 
> This series enables aarch64 with ilp32 mode.
> 
> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> option that is enabled for existing 32-bit architectures but disabled
> for new arches (so 64-bit off_t is is used by new userspace). Also it
> deprecates getrlimit and setrlimit syscalls prior to prlimit64.
> 
> This version is based on linux-next from 2017-03-01. It works with
> glibc-2.25, and tested with LTP, glibc testsuite, trinity, lmbench,
> CPUSpec.
> 
> Patches 1, 2, 3 and 8 are general, and may be applied separately.
> 
> This is the rebase of v7 - still no major changes has been made.
> 
> Kernel and GLIBC trees:
> https://github.com/norov/linux/tree/ilp32-20170602
> https://github.com/norov/glibc/tree/dev9
> 
> (GLIBC patches are managed by Steve Ellcey, so my tree is only for
> reference.)
> 
> Changes:
> v3: https://lkml.org/lkml/2014/9/3/704
> v4: https://lkml.org/lkml/2015/4/13/691
> v5: https://lkml.org/lkml/2015/9/29/911
> v6: https://lkml.org/lkml/2016/5/23/661
> v7: RFC nowrap:  https://lkml.org/lkml/2016/6/17/990
> v7: RFC2 nowrap: https://lkml.org/lkml/2016/8/17/245
> v7: RFC3 nowrap: https://lkml.org/lkml/2016/10/21/883
> v7: https://lkml.org/lkml/2017/1/9/213
> v7: Resend: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-March/490801.html
> v7: Resend 2:
>     - vdso-ilp32 Makefile synced with lp64 Makefile (patch 19);
>     - rebased on next-20170602.
> 
> Andrew Pinski (6):
>   arm64: rename COMPAT to AARCH32_EL0 in Kconfig
>   arm64: ensure the kernel is compiled for LP64
>   arm64:uapi: set __BITS_PER_LONG correctly for ILP32 and LP64
>   arm64: ilp32: add sys_ilp32.c and a separate table (in entry.S) to use
>     it
>   arm64: ilp32: introduce ilp32-specific handlers for sigframe and
>     ucontext
>   arm64:ilp32: add ARM64_ILP32 to Kconfig
> 
> Philipp Tomsich (1):
>   arm64:ilp32: add vdso-ilp32 and use for signal return
> 
> Yury Norov (13):
>   compat ABI: use non-compat openat and open_by_handle_at variants
>   32-bit ABI: introduce ARCH_32BIT_OFF_T config option
>   asm-generic: Drop getrlimit and setrlimit syscalls from default list
>   arm64: ilp32: add documentation on the ILP32 ABI for ARM64
>   thread: move thread bits accessors to separated file
>   arm64: introduce is_a32_task and is_a32_thread (for AArch32 compat)
>   arm64: ilp32: add is_ilp32_compat_{task,thread} and TIF_32BIT_AARCH64
>   arm64: introduce binfmt_elf32.c
>   arm64: ilp32: introduce binfmt_ilp32.c
>   arm64: ilp32: share aarch32 syscall handlers
>   arm64: signal: share lp64 signal routines to ilp32
>   arm64: signal32: move ilp32 and aarch32 common code to separated file
>   arm64: ptrace: handle ptrace_request differently for aarch32 and ilp32
> 
>  Documentation/arm64/ilp32.txt                 |  45 +++++++
>  arch/Kconfig                                  |   4 +
>  arch/arc/Kconfig                              |   1 +
>  arch/arc/include/uapi/asm/unistd.h            |   1 +
>  arch/arm/Kconfig                              |   1 +
>  arch/arm64/Kconfig                            |  19 ++-
>  arch/arm64/Makefile                           |   8 ++
>  arch/arm64/include/asm/compat.h               |  19 +--
>  arch/arm64/include/asm/elf.h                  |  37 ++----
>  arch/arm64/include/asm/fpsimd.h               |   2 +-
>  arch/arm64/include/asm/ftrace.h               |   2 +-
>  arch/arm64/include/asm/hwcap.h                |   6 +-
>  arch/arm64/include/asm/is_compat.h            |  90 ++++++++++++++
>  arch/arm64/include/asm/memory.h               |   5 +-
>  arch/arm64/include/asm/processor.h            |  11 +-
>  arch/arm64/include/asm/ptrace.h               |   2 +-
>  arch/arm64/include/asm/seccomp.h              |   2 +-
>  arch/arm64/include/asm/signal32.h             |   9 +-
>  arch/arm64/include/asm/signal32_common.h      |  27 ++++
>  arch/arm64/include/asm/signal_common.h        |  33 +++++
>  arch/arm64/include/asm/signal_ilp32.h         |  38 ++++++
>  arch/arm64/include/asm/syscall.h              |   2 +-
>  arch/arm64/include/asm/thread_info.h          |   4 +-
>  arch/arm64/include/asm/unistd.h               |   6 +-
>  arch/arm64/include/asm/vdso.h                 |   6 +
>  arch/arm64/include/uapi/asm/bitsperlong.h     |   9 +-
>  arch/arm64/include/uapi/asm/unistd.h          |  13 ++
>  arch/arm64/kernel/Makefile                    |   8 +-
>  arch/arm64/kernel/asm-offsets.c               |   9 +-
>  arch/arm64/kernel/binfmt_elf32.c              |  38 ++++++
>  arch/arm64/kernel/binfmt_ilp32.c              |  85 +++++++++++++
>  arch/arm64/kernel/cpufeature.c                |   8 +-
>  arch/arm64/kernel/cpuinfo.c                   |  20 +--
>  arch/arm64/kernel/entry.S                     |  34 +++++-
>  arch/arm64/kernel/entry32.S                   |  80 ------------
>  arch/arm64/kernel/entry32_common.S            | 107 ++++++++++++++++
>  arch/arm64/kernel/entry_ilp32.S               |  22 ++++
>  arch/arm64/kernel/head.S                      |   2 +-
>  arch/arm64/kernel/hw_breakpoint.c             |   8 +-
>  arch/arm64/kernel/perf_regs.c                 |   2 +-
>  arch/arm64/kernel/process.c                   |   7 +-
>  arch/arm64/kernel/ptrace.c                    |  80 ++++++++++--
>  arch/arm64/kernel/signal.c                    | 102 ++++++++++------
>  arch/arm64/kernel/signal32.c                  | 107 ----------------
>  arch/arm64/kernel/signal32_common.c           | 135 ++++++++++++++++++++
>  arch/arm64/kernel/signal_ilp32.c              | 170 ++++++++++++++++++++++++++
>  arch/arm64/kernel/sys_ilp32.c                 | 100 +++++++++++++++
>  arch/arm64/kernel/traps.c                     |   5 +-
>  arch/arm64/kernel/vdso-ilp32/.gitignore       |   2 +
>  arch/arm64/kernel/vdso-ilp32/Makefile         |  80 ++++++++++++
>  arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S     |  33 +++++
>  arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S |  95 ++++++++++++++
>  arch/arm64/kernel/vdso.c                      |  69 +++++++++--
>  arch/arm64/kernel/vdso/gettimeofday.S         |  20 ++-
>  arch/arm64/kernel/vdso/vdso.S                 |   6 +-
>  arch/blackfin/Kconfig                         |   1 +
>  arch/c6x/include/uapi/asm/unistd.h            |   1 +
>  arch/cris/Kconfig                             |   1 +
>  arch/frv/Kconfig                              |   1 +
>  arch/h8300/Kconfig                            |   1 +
>  arch/h8300/include/uapi/asm/unistd.h          |   1 +
>  arch/hexagon/Kconfig                          |   1 +
>  arch/hexagon/include/uapi/asm/unistd.h        |   1 +
>  arch/m32r/Kconfig                             |   1 +
>  arch/m68k/Kconfig                             |   1 +
>  arch/metag/Kconfig                            |   1 +
>  arch/metag/include/uapi/asm/unistd.h          |   1 +
>  arch/microblaze/Kconfig                       |   1 +
>  arch/mips/Kconfig                             |   1 +
>  arch/mn10300/Kconfig                          |   1 +
>  arch/nios2/Kconfig                            |   1 +
>  arch/nios2/include/uapi/asm/unistd.h          |   1 +
>  arch/openrisc/Kconfig                         |   1 +
>  arch/openrisc/include/uapi/asm/unistd.h       |   1 +
>  arch/parisc/Kconfig                           |   1 +
>  arch/powerpc/Kconfig                          |   1 +
>  arch/score/Kconfig                            |   1 +
>  arch/score/include/uapi/asm/unistd.h          |   1 +
>  arch/sh/Kconfig                               |   1 +
>  arch/sparc/Kconfig                            |   1 +
>  arch/tile/Kconfig                             |   1 +
>  arch/tile/include/uapi/asm/unistd.h           |   1 +
>  arch/tile/kernel/compat.c                     |   3 +
>  arch/unicore32/Kconfig                        |   1 +
>  arch/unicore32/include/uapi/asm/unistd.h      |   1 +
>  arch/x86/Kconfig                              |   1 +
>  arch/x86/um/Kconfig                           |   1 +
>  arch/xtensa/Kconfig                           |   1 +
>  drivers/clocksource/arm_arch_timer.c          |   2 +-
>  include/linux/fcntl.h                         |   2 +-
>  include/linux/thread_bits.h                   |  63 ++++++++++
>  include/linux/thread_info.h                   |  66 ++--------
>  include/uapi/asm-generic/unistd.h             |  10 +-
>  93 files changed, 1601 insertions(+), 413 deletions(-)
>  create mode 100644 Documentation/arm64/ilp32.txt
>  create mode 100644 arch/arm64/include/asm/is_compat.h
>  create mode 100644 arch/arm64/include/asm/signal32_common.h
>  create mode 100644 arch/arm64/include/asm/signal_common.h
>  create mode 100644 arch/arm64/include/asm/signal_ilp32.h
>  create mode 100644 arch/arm64/kernel/binfmt_elf32.c
>  create mode 100644 arch/arm64/kernel/binfmt_ilp32.c
>  create mode 100644 arch/arm64/kernel/entry32_common.S
>  create mode 100644 arch/arm64/kernel/entry_ilp32.S
>  create mode 100644 arch/arm64/kernel/signal32_common.c
>  create mode 100644 arch/arm64/kernel/signal_ilp32.c
>  create mode 100644 arch/arm64/kernel/sys_ilp32.c
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.S
>  create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S
>  create mode 100644 include/linux/thread_bits.h
> 
> -- 
> 2.11.0

^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: your mail
       [not found] <20191026192359.27687-1-frank-w@public-files.de>
@ 2019-10-26 19:30 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 27+ messages in thread
From: Greg Kroah-Hartman @ 2019-10-26 19:30 UTC (permalink / raw)
  To: Frank Wunderlich
  Cc: Matthias Brugger, linux-mediatek, linux-arm-kernel, linux-serial,
	linux-kernel

On Sat, Oct 26, 2019 at 09:23:59PM +0200, Frank Wunderlich wrote:
> Date: Sat, 26 Oct 2019 20:53:28 +0200
> Subject: [PATCH] serial: 8250-mtk: Ask for IRQ-count before request one

Odd email with no subject line :(

Plaese fix up and resend.

thanks,

greg k-h-

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2019-10-26 19:30 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-07 17:58 No subject Dave Hylands
2010-06-07 22:10 ` your mail Jamie Lokier
2010-06-07 22:44   ` Dave Hylands
     [not found] <20191026192359.27687-1-frank-w@public-files.de>
2019-10-26 19:30 ` Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2017-06-04 11:59 No subject Yury Norov
2017-06-14 20:16 ` your mail Yury Norov
2015-04-21 10:18 No subject Ard Biesheuvel
2015-04-21 10:46 ` your mail Dave P Martin
2015-04-21 10:50   ` Ard Biesheuvel
2015-04-21 11:10     ` Dave P Martin
2015-04-21 11:15       ` Ard Biesheuvel
2015-04-21 11:24       ` Russell King - ARM Linux
2015-04-21 12:50         ` Russell King - ARM Linux
2015-04-21 13:10           ` Ard Biesheuvel
2015-04-21 13:21             ` Dave P Martin
2015-04-21 13:28               ` Ard Biesheuvel
2015-04-21 15:51                 ` Dave Martin
2015-04-21 14:05               ` Russell King - ARM Linux
2015-04-21 13:18           ` Dave P Martin
2015-04-21 13:55             ` Russell King - ARM Linux
2015-04-21 14:06               ` Ard Biesheuvel
2015-04-21 17:03                 ` Dave Martin
2014-10-28 14:13 No subject Mark Rutland
2014-10-28 14:19 ` your mail Mark Rutland
2012-06-21 18:26 No subject Paul Walmsley
2012-06-22  8:56 ` your mail Tony Lindgren
2011-12-02 16:01 No subject Will Deacon
2011-12-02 16:11 ` your mail Will Deacon
2011-05-13 19:35 No subject Vadim Bendebury
2011-05-14  3:32 ` your mail Jean-Christophe PLAGNIOL-VILLARD
2010-05-18 10:38 No subject Marek Szyprowski
2010-05-19  2:24 ` your mail Ben Dooks
     [not found] <1250258343-14203-1-git-send-email-thierry.reding@avionic-design.de>
2009-10-02 14:53 ` Thierry Reding
2010-01-02 12:06   ` Russell King - ARM Linux

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).