From patchwork Fri Mar 21 17:35:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Riley X-Patchwork-Id: 3875801 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 4BADABF540 for ; Fri, 21 Mar 2014 17:36:00 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1307D201EF for ; Fri, 21 Mar 2014 17:35:59 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 90329201ED for ; Fri, 21 Mar 2014 17:35:57 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WR3MG-0006Je-Qs; Fri, 21 Mar 2014 17:35:53 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WR3ME-0005Lp-GG; Fri, 21 Mar 2014 17:35:50 +0000 Received: from mail-qa0-x236.google.com ([2607:f8b0:400d:c00::236]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WR3MA-0005J1-2r for linux-arm-kernel@lists.infradead.org; Fri, 21 Mar 2014 17:35:47 +0000 Received: by mail-qa0-f54.google.com with SMTP id w8so2778902qac.27 for ; Fri, 21 Mar 2014 10:35:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=J6lpnf/xENfssGkWIZmZVz7wHi7Rrka9gjVwh+fQv20=; b=jhcSIn9xiE5T8DpKM1YAj30s/OCL5hFDvFUrSHBX0H7zoXqLc147NicPHulHQdxSdv wW7ymX/PbepXFAMTrCQDt5Hg8MPRZ5Gcf558NTJVlmFsoOOiB5uMLAuu4G15AZK9FdWe 0yrhcKB+YKiV4gDTMRtRvFALIItYiyACv0IEt5Uxh41U+FbguSLCYcpVB20wBEmg/PN5 lVu7obQjiKhkbtDkAKno0hG9kEdX+sfMff+YhG/sPNvm3nAxTnHElyUKb5FTN87jrGi/ Yg6Baw77+9C3xRNlgIuAyAoykKVkP56VRqnZMNg9PnzHsZUSDb8gz+ooNHVglQGxTxtq 8FZA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=J6lpnf/xENfssGkWIZmZVz7wHi7Rrka9gjVwh+fQv20=; b=UGLZOU5qyAfXNdnpQiR3CBWg8Qiphrl3L2dTvflrMwfZI2d+zjOCSRmVo8txpqYMSX K2fHLfrqXcKhHmi8kRz4AZ8vOp0gzFoPaMB2PRyexL4jT4O7zzhAaD4BIHCpc7uOWstI LzUmsORaXu+Pf92YFfig2irbOiclJQPRv9nwTkogt8EvxE4lOH/Pn399gxE7u7BEU+ea mvyVL5njqJKLrJjCuhjgMM5DduTwZGH7OSwkuxPjC+ktYHNFqYHuHhxNMRacrM3sWx9N 4sFS90Wlmq4GgxEBRy4coXcceDWacPSOfJzYz9EvnVkVGvPb3Lbe/RG2iaBPWuMLk5Tb b7oA== X-Gm-Message-State: ALoCoQlVxAjDGucvk1Kc7eu8YXYGHPqiL4zS0Atb03nsZCN/TejA2DpejUz3WHpvonPNua6sU/uEkqi3eI2oH5Gke7X4W3lD76OklUUAITK2UXfP4RJtg3Gcyb5oV5QVPDzs3dren11J/CdBE/rgWnB9nM7nRCsbqyayLDmATXl16gth/R0D/XXGBe/6/53lqFaVqDmXwyzizPDDq7wyijW2kpADbxj0Bg== MIME-Version: 1.0 X-Received: by 10.140.83.232 with SMTP id j95mr4296514qgd.42.1395423321536; Fri, 21 Mar 2014 10:35:21 -0700 (PDT) Received: by 10.140.102.246 with HTTP; Fri, 21 Mar 2014 10:35:21 -0700 (PDT) In-Reply-To: References: <1394734769-32760-1-git-send-email-nathan_lynch@mentor.com> <532C65CB.2080501@mentor.com> Date: Fri, 21 Mar 2014 10:35:21 -0700 Message-ID: Subject: Re: [PATCH v4] ARM: vDSO gettimeofday using generic timer architecture From: David Riley To: Nathan Lynch X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140321_133546_260220_CBE75996 X-CRM114-Status: GOOD ( 30.33 ) X-Spam-Score: -2.0 (--) Cc: "linux@arm.linux.org.uk" , Steve Capper , "linux-arm-kernel@lists.infradead.org" , Will Deacon X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,RCVD_IN_DNSWL_MED,T_DKIM_INVALID,T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Err, my nm was actually with the discard of .GCC.command.line and .comment applied. Without that line in the script, it's: Dave On Fri, Mar 21, 2014 at 10:05 AM, David Riley wrote: > I was thinking the same thing about having the data section before the > code section would make things more robust. I'll try out the > incremental patch. > > Here are the contents of my vdso.so.dbg: > 000003c0 t $a > 000004b4 t $a > 00000700 t $a > 00000780 t $a > 0000081c t $a > 0000082c t $a > 0000083c t $a > 00000870 t $a > 00000b48 t $a > 00000b58 t $a > 00000b6c t $a > 000004ac t $d > 000002ec r $d > 00000364 r $d > 000006f8 t $d > 0000077c t $d > 00000818 t $d > 00000828 t $d > 00000838 t $d > 00000010 N $d > 000001f8 N $d > 0000004c N $d > 0000006c N $d > 0000086c t $d > 0000002c N $d > 00000250 r $d > 00000b54 t $d > 00000b64 t $d > 00000b74 t $d > 00000880 t $t > 00000b30 t $t > 00000b68 t $t > 00000886 t .divsi3_skip_div0_test > 00000000 A LINUX_3.15 > 00000264 a _DYNAMIC > 00000b78 a _GLOBAL_OFFSET_TABLE_ > 00000b69 t ____aeabi_idiv0_from_thumb > 00000b58 t ____aeabi_idiv_veneer > 00000b48 t ____aeabi_llsr_veneer > 00000881 t __aeabi_idiv > 0000081c t __aeabi_idiv0 > 00000b15 t __aeabi_idivmod > 0000082c t __aeabi_ldiv0 > 00000b31 t __aeabi_llsr > 0000083c t __aeabi_unwind_cpp_pr0 > 0000084c t __aeabi_unwind_cpp_pr1 > 0000085c t __aeabi_unwind_cpp_pr2 > 0000080c t __div0 > 00000881 t __divsi3 > 00000870 t __get_datapage > 00000700 T __kernel_clock_getres > 000004b4 T __kernel_clock_gettime > 00000780 T __kernel_gettimeofday > 00000b31 t __lshrdi3 > 00001000 a _vdso_data > 000003c0 t do_realtime > 00000000 a shift > > On Fri, Mar 21, 2014 at 9:16 AM, Nathan Lynch wrote: >> On 03/21/2014 09:58 AM, Steve Capper wrote: >>> On 18 March 2014 00:49, David Riley wrote: >>>> Hi Nathan, >>>> >>>> I gave this a try against a 3.10 system and ran into issues when >>>> vdso.so grew to two pages because of of a large .GCC.command.line >>>> section. __get_datapage was returning an address that was at the >>>> beginning of the second code page instead of the data page since it >>>> didn't account for the additional section. I got it working by adding >>>> .GCC.command.line to the DISCARD group in the linker script, but in >>>> general it feels as if this code is a bit fragile since it depends on >>>> knowing exactly which sections exist in the .so for the _vdso_data >>>> symbol to be correct. >>>> >>>> Cheers, >>>> David >>>> >>> >>> Hi David, >>> Could you please send us the following output: >>> $ nm -C ./arch/arm/kernel/vdso/vdso.so.dbg >>> >>> For the vdso without the extra DISCARD? >>> (As I'm interested in the alignment of __vdso_data) >> >> Something like this, I imagine: (I just added -frecord-gcc-switches to >> ccflags-y): >> >> 00000770 t __aeabi_idiv0 >> 00000774 t __aeabi_ldiv0 >> 00000778 t __aeabi_unwind_cpp_pr0 >> 0000077c t __aeabi_unwind_cpp_pr1 >> 00000780 t __aeabi_unwind_cpp_pr2 >> 0000076c t __div0 >> 000002e8 t do_realtime >> 00000240 a _DYNAMIC >> 00000788 t __get_datapage >> 00000798 a _GLOBAL_OFFSET_TABLE_ >> 0000066c T __kernel_clock_getres >> 000003f8 T __kernel_clock_gettime >> 000006e4 T __kernel_gettimeofday >> 00000000 A LINUX_3.15 >> 00001000 d _vdso_data >> >> And yes, vdso.so.dbg and vdso.so come out larger than 4K. So >> _vdso_data is wrong (we'd want it to be 0x2000). >> >> The issue, I think, is the linker is free to deposit "orphan" sections >> -- those which aren't explicitly treated in the linker script -- such >> as .GCC.command.line wherever it sees fit, and the counter in the >> linker script doesn't account for those. So it seems to me that >> mapping the data page after the text is a losing game, and it's more >> robust to map it at the beginning of the VMA. >> >> Incremental patch against v4 below, tested okay on OMAP5: >> >> arch/arm/include/asm/elf.h | 11 +++++++---- >> arch/arm/include/asm/vdso_datapage.h | 7 +++++++ >> arch/arm/kernel/asm-offsets.c | 5 +++++ >> arch/arm/kernel/vdso.c | 14 ++++++-------- >> arch/arm/kernel/vdso/datapage.S | 3 ++- >> arch/arm/kernel/vdso/vdso.lds.S | 6 +++--- >> 6 files changed, 30 insertions(+), 16 deletions(-) >> >> diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h >> index fc96b78a8102..45d2ddff662a 100644 >> --- a/arch/arm/include/asm/elf.h >> +++ b/arch/arm/include/asm/elf.h >> @@ -3,6 +3,7 @@ >> >> #include >> #include >> +#include >> >> /* >> * ELF register definitions.. >> @@ -131,10 +132,12 @@ extern unsigned long arch_randomize_brk(struct mm_struct *mm); >> >> #ifdef CONFIG_MMU >> #ifdef CONFIG_VDSO >> -#define ARCH_DLINFO \ >> -do { \ >> - NEW_AUX_ENT(AT_SYSINFO_EHDR, \ >> - (elf_addr_t)current->mm->context.vdso); \ >> +#define ARCH_DLINFO \ >> +do { \ >> + /* Account for the data page at the beginning of the [vdso] VMA. */ \ >> + NEW_AUX_ENT(AT_SYSINFO_EHDR, \ >> + (elf_addr_t)current->mm->context.vdso + \ >> + sizeof(union vdso_data_store)); \ >> } while (0) >> #endif >> #define ARCH_HAS_SETUP_ADDITIONAL_PAGES 1 >> diff --git a/arch/arm/include/asm/vdso_datapage.h b/arch/arm/include/asm/vdso_datapage.h >> index 9011ec75a24b..f08bdb73d3f4 100644 >> --- a/arch/arm/include/asm/vdso_datapage.h >> +++ b/arch/arm/include/asm/vdso_datapage.h >> @@ -22,6 +22,8 @@ >> >> #ifndef __ASSEMBLY__ >> >> +#include >> + >> /* Try to be cache-friendly on systems that don't implement the >> * generic timer: fit the unconditionally updated fields in the first >> * 32 bytes. >> @@ -46,6 +48,11 @@ struct vdso_data { >> u32 tz_dsttime; >> }; >> >> +union vdso_data_store { >> + struct vdso_data data; >> + u8 page[PAGE_SIZE]; >> +}; >> + >> #endif /* !__ASSEMBLY__ */ >> >> #endif /* __KERNEL__ */ >> diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c >> index ded041711beb..dda3363ef0bf 100644 >> --- a/arch/arm/kernel/asm-offsets.c >> +++ b/arch/arm/kernel/asm-offsets.c >> @@ -24,6 +24,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> @@ -199,5 +200,9 @@ int main(void) >> #endif >> DEFINE(KVM_VTTBR, offsetof(struct kvm, arch.vttbr)); >> #endif >> + BLANK(); >> +#ifdef CONFIG_VDSO >> + DEFINE(VDSO_DATA_SIZE, sizeof(union vdso_data_store)); >> +#endif >> return 0; >> } >> diff --git a/arch/arm/kernel/vdso.c b/arch/arm/kernel/vdso.c >> index 91cbc5f14fb5..1a2bc699c1ac 100644 >> --- a/arch/arm/kernel/vdso.c >> +++ b/arch/arm/kernel/vdso.c >> @@ -33,11 +33,8 @@ static unsigned long vdso_pages; >> static unsigned long vdso_mapping_len; >> static struct page **vdso_pagelist; >> >> -static union { >> - struct vdso_data data; >> - u8 page[PAGE_SIZE]; >> -} vdso_data_store __page_aligned_data; >> -struct vdso_data *vdso_data = &vdso_data_store.data; >> +static union vdso_data_store vdso_data_store __page_aligned_data; >> +static struct vdso_data *vdso_data = &vdso_data_store.data; >> >> /* >> * The vDSO data page. >> @@ -62,12 +59,13 @@ static int __init vdso_init(void) >> if (vdso_pagelist == NULL) >> return -ENOMEM; >> >> + /* Grab the vDSO data page. */ >> + vdso_pagelist[0] = virt_to_page(vdso_data); >> + >> /* Grab the vDSO code pages. */ >> for (i = 0; i < vdso_pages; i++) >> - vdso_pagelist[i] = virt_to_page(&vdso_start + i * PAGE_SIZE); >> + vdso_pagelist[i + 1] = virt_to_page(&vdso_start + i * PAGE_SIZE); >> >> - /* Grab the vDSO data page. */ >> - vdso_pagelist[i] = virt_to_page(vdso_data); >> >> /* Precompute the mapping size */ >> vdso_mapping_len = (vdso_pages + 1) << PAGE_SHIFT; >> diff --git a/arch/arm/kernel/vdso/datapage.S b/arch/arm/kernel/vdso/datapage.S >> index 91b19b815888..fbf36d75da06 100644 >> --- a/arch/arm/kernel/vdso/datapage.S >> +++ b/arch/arm/kernel/vdso/datapage.S >> @@ -1,8 +1,9 @@ >> #include >> +#include >> >> .align 2 >> .L_vdso_data_ptr: >> - .long _vdso_data - . >> + .long _start - . - VDSO_DATA_SIZE >> >> ENTRY(__get_datapage) >> .cfi_startproc >> diff --git a/arch/arm/kernel/vdso/vdso.lds.S b/arch/arm/kernel/vdso/vdso.lds.S >> index 24dd7b84e366..799d259f86e0 100644 >> --- a/arch/arm/kernel/vdso/vdso.lds.S >> +++ b/arch/arm/kernel/vdso/vdso.lds.S >> @@ -30,6 +30,8 @@ OUTPUT_ARCH(arm) >> >> SECTIONS >> { >> + PROVIDE(_start = .); >> + >> . = VDSO_LBASE + SIZEOF_HEADERS; >> >> .hash : { *(.hash) } :text >> @@ -55,14 +57,12 @@ SECTIONS >> .got : { *(.got) } >> .rel.plt : { *(.rel.plt) } >> >> - . = ALIGN(PAGE_SIZE); >> - PROVIDE(_vdso_data = .); >> - >> /DISCARD/ : { >> *(.note.GNU-stack) >> *(.data .data.* .gnu.linkonce.d.* .sdata*) >> *(.bss .sbss .dynbss .dynsbss) >> } >> + >> } >> >> /* >> --- /tmp/nm-discard 2014-03-21 10:29:59.519497758 -0700 +++ /tmp/nm-no-discard 2014-03-21 10:23:59.622483162 -0700 @@ -9,6 +9,7 @@ 00000b48 t $a 00000b58 t $a 00000b6c t $a +0000000e n $d 000004ac t $d 000002ec r $d 00000364 r $d Your new patch passes a quick sanity test run for me. One additional note is that I needed the following change to get this patch to compile: diff --git a/arch/arm/kernel/vdso/Makefile b/arch/arm/kernel/vdso/Makefile index 313c0e2..817426f 100644 --- a/arch/arm/kernel/vdso/Makefile +++ b/arch/arm/kernel/vdso/Makefile @@ -4,7 +4,7 @@ obj-vdso := vgettimeofday.o datapage.o targets := $(obj-vdso) vdso.so vdso.so.dbg vdso.lds obj-vdso := $(addprefix $(obj)/, $(obj-vdso)) -ccflags-y := -shared -fPIC -fno-common -fno-builtin +ccflags-y := -shared -fPIC -fno-common -fno-builtin -fno-stack-protector ccflags-y += -nostdlib -Wl,-soname=linux-vdso.so.1 \ $(call cc-ldoption, -Wl$(comma)--hash-style=sysv)