From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754653AbbJAToY (ORCPT ); Thu, 1 Oct 2015 15:44:24 -0400 Received: from mail-bn1bon0095.outbound.protection.outlook.com ([157.56.111.95]:19392 "EHLO na01-bn1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752494AbbJAToW (ORCPT ); Thu, 1 Oct 2015 15:44:22 -0400 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Yuri.Norov@caviumnetworks.com; Date: Thu, 1 Oct 2015 22:44:05 +0300 From: Yury Norov To: Nathan Lynch CC: , , , , , , , , , , Subject: Re: [PATCH v5 17/23] arm64:ilp32: add vdso-ilp32 and use for signal return Message-ID: <20151001194405.GB31021@yury-N73SV> References: <1443564860-31208-1-git-send-email-ynorov@caviumnetworks.com> <1443564860-31208-18-git-send-email-ynorov@caviumnetworks.com> <560B5FB5.6080605@mentor.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <560B5FB5.6080605@mentor.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Originating-IP: [91.78.158.8] X-ClientProxiedBy: DB4PR07CA026.eurprd07.prod.outlook.com (10.242.229.36) To DM2PR07MB622.namprd07.prod.outlook.com (10.141.177.146) X-Microsoft-Exchange-Diagnostics: 1;DM2PR07MB622;2:Uu3h1otO+3fMZkUePrnrRt9LeJzab+ImFQL35d3u5HYVFnMrMjH3PCtl3g7c/PlDicM3zjjuqw+0NbMCtCeOHHoAEgIpgCDAuFOAVziLDFaz09jG2pB6R8iT/BrTFiU5KZciKr9L1Ja2aid5Qhhv8ID4Iubp/x1GhwqLGnK3gRQ=;3:PEhj7rvhstbRm7DzA6/4Fdt6L8dk/d37UUoEFE6pOMDgJJTRhsDXo+sg1D7tTkHh88EEbnffWPwVB48fgXQRkP35ZfyqVzhs6GOtf/5QZkuTPwDejzIceGuvRxPRZSO7NzNSkOgQTyYbRrxApuJo4A==;25:0ZXJ3L2GRUTvitQ07oKBZhcHnopwWWaeKqexhM2pesmJ/DdxXfZTeItahYBRq29dIguSnVVXc8e3A1VYsCGJyYLZ79sjUX9jScVRybWSsY+f0kp/Yp93x7vuQzvUJTY1imGuSjuNKsb0LtdY5UTXQEkrpNCQccvwu5oCNGxDYZpcrkQymN5y5kpMSAtgCyFlvbuAEADt2X99jhyXwgUrluaxcpXwx6PAN6hgY9T7Ayg1vEaQ0DrNSUqE/FhiYJaPxCbI0oCWrysYweb91lLQWQ== X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:DM2PR07MB622; X-Microsoft-Exchange-Diagnostics: 1;DM2PR07MB622;20:1egym93PT8JSK2+plw/QZ8gtI34Q0z69JnUPLcAuqqfn8ahOYrk9JEaoSdM5M8UyuTTxaH59mXxxHfqYPQu8mCzgKNn48puYhD+SsZywOeRKo5i/9jRai15ShATVscMwcIqG2nlBdV+pGWsNPT0Lk1iY8MHU7J2WCo+m5T5ZsedsEsPnRVMm6QXXJPMX71pDGmUdY8EU6Hx3oT8s9pUSCSWSW6Pc2O7UY4b2QqxtsppTkNZugNKIBld0Y1YQrKWMd7ADymB5GSn6V14QQSrXovrUgvWunprhgcGVXmN2I/T6g1dJkuD1jvUu3Tk0p1ki4J4KaxrApD7G0xvpeofjfFLNVZLIIGRd8kuY0xu3hXcrBIR27B1Cua/OUjxXoaZKi1DjgHgPT67RMCYkzNqpKvrG8MlrdF2x8KbX05bjp/35i1Ix24FMC21YqQuBsIQUy9wBR0ojUqHHIU6KGVfQ7viQVvhm4+EB9ymItZooUeFRmmSBn3Mze04ELseqc2DDJFuI1f1OYsrYkZNe+ScvRzlrYB8qbzhS+guiFKlXr49LAMzgHuz+sVoxLWLnB82T95VA130BzjAOxUYA2XSXfQm/mAZLv/vrx9mQza9IIcQ= X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:; X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(601004)(2401047)(520078)(5005006)(8121501046)(3002001);SRVR:DM2PR07MB622;BCL:0;PCL:0;RULEID:;SRVR:DM2PR07MB622; X-Microsoft-Exchange-Diagnostics: 1;DM2PR07MB622;4:044rtYBfyKqkuFowCbmkp3KdETbln148E93HnrGBM6G5SgSYWkewjoZ+4UJ3/xXDZGEcJW40f930XRxIaO3IQreYvNIxVvarLPCs4vH2VrO5/X2J/CKxR7xEWBCQPMokNrXDK17+gc71FapaDkHfAB2ygA93Ncg28yySHX9I9IbRwU3KPGyVX8NJptiBQlnGS5m6pURdExdi+huY1nm+xDhHoRGqKdOH7X4snd5c/F4BoCWH3kONxa2aWLpRUcMjcJkl7iK2E3TKxLCjnareHMMjRQ4E3tmNZBGgGz9cicLEDDnCvWo8FdFGGYHDkFMgXtI4OLm4v+NEk/rwec18OYJ3O1zsZo2xrJsa+mYMqEE= X-Forefront-PRVS: 0716E70AB6 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(6069001)(6009001)(377454003)(199003)(479174004)(24454002)(189002)(50466002)(50986999)(5004730100002)(5008740100001)(64706001)(2950100001)(47776003)(76176999)(5007970100001)(46102003)(54356999)(15975445007)(77096005)(76506005)(87976001)(68736005)(42186005)(189998001)(66066001)(122386002)(5001860100001)(97736004)(5001960100002)(19580405001)(33716001)(110136002)(19580395003)(101416001)(97756001)(4001540100001)(33656002)(106356001)(92566002)(81156007)(23726002)(40100003)(62966003)(46406003)(83506001)(77156002)(4001350100001)(105586002)(5001830100001);DIR:OUT;SFP:1101;SCL:1;SRVR:DM2PR07MB622;H:localhost;FPR:;SPF:None;PTR:InfoNoRecords;A:1;MX:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1;DM2PR07MB622;23:1xRaabo4R348psn1mZYeqHkYDKbxwU00EWG2dqTHCv?= =?us-ascii?Q?X6I19J2ri5vHeQlic+g8+wa/6mEgcBq1+V9o1Z/FJnad2AS0fEdf0xTC3GQo?= =?us-ascii?Q?qMPRXnU7q+cx+wGQJFCkU6gva/ShaWcpSWKlylJYwVPpKMN7GvafjnKkF70P?= =?us-ascii?Q?dabxArTrC891pbDOht48327txDqeAgY4id5xzsqFiaAk3t7XmODdLwd20A4d?= =?us-ascii?Q?27wzAodwAcuSyhe8gL1cUoLOMjXWYDC4XH8M6sV+5tEMRG2l3a+0cHFh3i6p?= =?us-ascii?Q?D6k7XFZGBUeaG/r+++9af6Diz347Wb8h/fsv8swxwJkd1Xo3qA+K7xDa7C1m?= =?us-ascii?Q?kwowZzdMR5GBDVsIbg+xFvZg3lB9Ull/23Gtl/cDEAOrWNVI4nQCRLU2yaYx?= =?us-ascii?Q?dJT+Tl2Q1NmeBDhrIywPnTu1zCX+Pz7KTYXz6fHGi3vfbFrbCZeANWXcuIYU?= =?us-ascii?Q?H9Bi2wbRo+U2lkWQ0g2ntvuBzN9geFE2ryAb+9NdM698VOYrz/x1s9ZrSKmP?= =?us-ascii?Q?/8KZWToIyQAp4tRdXUZf2tMZlQ4GjFA3XWgXIIawsCxUEC/GNq6VhIUhqAn6?= =?us-ascii?Q?PNLtx97Fot+39os8Z9FkYGXbGcHsfKi+5hsvhlidr5BoLcVEwoZioWeRTiqY?= =?us-ascii?Q?JJ5b95WNbXxnZ8Q4eDGf3/q3pKwB0YVKkfJwexp17pA6e1VNwBQR0WLuu4mx?= =?us-ascii?Q?aQz9lUis6gVkXv8dyfGHkBwVvZ06ZfJ+K9UqZhA7C2x36yvYqfQqx3oc2069?= =?us-ascii?Q?R8on4lBZenm0vIA45sfJG/2OXTfyQDIUmaUw/N7nFeuZwWQ//THfle1WJE2A?= =?us-ascii?Q?RqSv8ookhFzASD4s58e9A/gq4nnQnRwkRgdwXxvG/0kZDHrSQjFjYofeOeYD?= =?us-ascii?Q?y+LAyyNHuiQnIpXF8uUgbWZUnTvwLiCqSCtPkUP/UMPRHE/4pmgq8cUvWTDJ?= =?us-ascii?Q?y87eIuR4Dau+YK13h+6XM/1AOePuus/d5+WAJ4DCI6ZrkE/NnaUhC2F5zJka?= =?us-ascii?Q?NWH4HsM7HExnPlxR/yqNcJs60G/o8op4nRkRhbpA3GxiAthyn5yUyKVndUMV?= =?us-ascii?Q?BaM3x/sX3eQ/y0Q8blSCyvcFg6uCba45sUpzUz0qKaABVCQhdyCoth8W5I05?= =?us-ascii?Q?0Mr35sxNiaJ6OTV8+Z+93xSFubkDZpo05aZCNOlMrKsIU5d4xoZ7l96Hp5z2?= =?us-ascii?Q?zMDxVBmURoObcV7JYg61hE8IlWu+IkCgaYwZPXcASf2HB7wYqHlM0PMRBVB8?= =?us-ascii?Q?k3VxeiNknX3ivulrA9Ip0TvKTRtuwC3Vq2dz1nbpHjEi+jmwT+Gy2J9h+Nwz?= =?us-ascii?Q?arvI1zPwUkK8AtLq0TlqtTtaQwMU9azNYi9t+OAEHwNBuMaaKxXFvqgiUNKT?= =?us-ascii?Q?0tRQ=3D=3D?= X-Microsoft-Exchange-Diagnostics: 1;DM2PR07MB622;5:5LH88NUdYYwcxJCLEM9UVLT82r/kg8Jdi+DgAg3eS2Qyt18dEr4YQJeybfeVHuJRTlzMcbW2hETeFwDKeYNQAuTI4mhGc2n4B0SPFCPFGSSS9+A86ua0sOjiiTl7OR7nEWLpx0MtdCZAIiAVpUCUzQ==;24:1pzDcW6tPi9nwjSDeikfY3XmxyJCY0xIbQjMKsm4v5ECPW3X+uW7j849THQ/tLUe3/BFoqT0whgzJ2Xx80hNs8metzPzQhShfvEwlH5AD3E=;20:Woyiha0v1FQpJDONDMkVH+nRlrxZSsoGJLmZ9UuLC95uefxRyTZv6jYZtOc3VB9GTn20/HGkOq2/3rz51+0ETw== SpamDiagnosticOutput: 1:23 SpamDiagnosticMetadata: NSPM X-OriginatorOrg: caviumnetworks.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Oct 2015 19:44:18.7163 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM2PR07MB622 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 29, 2015 at 11:06:13PM -0500, Nathan Lynch wrote: > On 09/29/2015 05:14 PM, Yury Norov wrote: > > From: Philipp Tomsich > > > > Adjusted to move the move data page before code pages in sync with > > commit 601255ae3c98fdeeee3a8bb4696425e4f868b4f1 > > This commit message needs more information about how the ilp32 VDSO uses > the existing arm64 code. I had to really hunt through the Makefile to > figure out what's going on. > > The commit message should also identify the APIs that are supported. > The subject line mentions signal return, but gettimeofday, clock_gettime > and clock_getres are being added here too, and it is not obvious. > > > > Signed-off-by: Philipp Tomsich > > Signed-off-by: Christoph Muellner > > Signed-off-by: Yury Norov > > > > create mode 100644 arch/arm64/kernel/vdso-ilp32/.gitignore > > create mode 100644 arch/arm64/kernel/vdso-ilp32/Makefile > > copy arch/arm64/{include/asm/vdso.h => kernel/vdso-ilp32/vdso-ilp32.S} (56%) > > create mode 100644 arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S > > How are you invoking git-format-patch? The copy detection in this case > is not conducive to review. > > It looks like the existing arm64 vdso Makefile has been copied to > vdso-ilp32/ and adjusted for paths and naming. While the gettimeofday > assembly implementation is reused, the build logic is duplicated. x86 > produces VDSOs for multiple ABIs with a single Makefile; is a similar > approach not appropriate for arm64? > > > > diff --git a/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S > > new file mode 100644 > > index 0000000..ac8029b > > --- /dev/null > > +++ b/arch/arm64/kernel/vdso-ilp32/vdso-ilp32.lds.S > > @@ -0,0 +1,98 @@ > > [...] > > > +#include > > +#include > > +#include > > + > > +/*OUTPUT_FORMAT("elf32-littleaarch64", "elf32-bigaarch64", "elf32-littleaarch64") > > +OUTPUT_ARCH(aarch64) > > +*/ > > If these lines aren't needed then omit them. > > [...] > > > > +/* > > + * This controls what symbols we export from the DSO. > > + */ > > +VERSION > > +{ > > + LINUX_2.6.39 { > > + global: > > + __kernel_rt_sigreturn; > > + __kernel_gettimeofday; > > + __kernel_clock_gettime; > > + __kernel_clock_getres; > > + local: *; > > + }; > > +} > > Something that came up during review of arch/arm's VDSO code: consider > using version and names that match x86, i.e. LINUX_2.6, __vdso_gettimeofday. > > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-June/267940.html > > Using LINUX_2.6.39 for this code is nonsensical. > > > > diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c > > index b239b9b..bed6cf1 100644 > > --- a/arch/arm64/kernel/vdso.c > > +++ b/arch/arm64/kernel/vdso.c > > @@ -40,6 +40,12 @@ extern char vdso_start, vdso_end; > > static unsigned long vdso_pages; > > static struct page **vdso_pagelist; > > > > +#ifdef CONFIG_ARM64_ILP32 > > +extern char vdso_ilp32_start, vdso_ilp32_end; > > +static unsigned long vdso_ilp32_pages; > > +static struct page **vdso_ilp32_pagelist; > > +#endif > > + > > /* > > * The vDSO data page. > > */ > > @@ -117,24 +123,29 @@ int aarch32_setup_vectors_page(struct linux_binprm *bprm, int uses_interp) > > } > > #endif /* CONFIG_AARCH32_EL0 */ > > > > -static struct vm_special_mapping vdso_spec[2]; > > - > > -static int __init vdso_init(void) > > +static inline int __init vdso_init_common(char *vdso_start, char *vdso_end, > > No inline please. > > > > + unsigned long *vdso_pagesp, > > + struct page ***vdso_pagelistp, > > + struct vm_special_mapping* vdso_spec) > > { > > [...] > > > int arch_setup_additional_pages(struct linux_binprm *bprm, > > int uses_interp) > > { > > struct mm_struct *mm = current->mm; > > unsigned long vdso_base, vdso_text_len, vdso_mapping_len; > > - void *ret; > > + void* ret; > > Gratuitous (and incorrect) style change. > > > > + unsigned long pages = vdso_pages; > > + struct vm_special_mapping* spec = vdso_spec; > > Incorrect style: *spec Hi Nathan, If Philipp Philipp Tomsich will not answer soon, I'll fix all this. BR, Yury.