From patchwork Wed Aug 17 02:45:58 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konrad Rzeszutek Wilk X-Patchwork-Id: 9285097 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 138F3600CB for ; Wed, 17 Aug 2016 02:49:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E939728B97 for ; Wed, 17 Aug 2016 02:49:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DDD8428BA0; Wed, 17 Aug 2016 02:49:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E7D3928B97 for ; Wed, 17 Aug 2016 02:49:13 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bZqs3-0004Uh-BY; Wed, 17 Aug 2016 02:46:23 +0000 Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bZqs2-0004Ub-F0 for xen-devel@lists.xenproject.org; Wed, 17 Aug 2016 02:46:22 +0000 Received: from [85.158.143.35] by server-7.bemta-6.messagelabs.com id 78/D7-15404-DFFC3B75; Wed, 17 Aug 2016 02:46:21 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrEIsWRWlGSWpSXmKPExsUyZ7p8oO6f85v DDfafNLH4vmUykwOjx+EPV1gCGKNYM/OS8isSWDN+ftnNUrDZumLdnylsDYyfDboYuTiEBDqY JPbN/8sK4XxhlJh1/CiQwwnkbGSUmLgiCCLRwyjRuvsFI0iCRUBVYt+pVqAiDg42AROJN6scQ cIiAroSzxY8YwOpZxbYxyix7s4DsHphAU+Jmbu6mUFsXgEriZWXt0ItuMos8aE1ESIuKHFy5h MWEJtZQEvixr+XTCDzmQWkJZb/4wAJcwrYSfz+0QBWIiqgLNEw4wHYSAkBY4m+WX0sExgFZyG ZNAvJpFkIkxYwMq9i1ChOLSpLLdI1NtRLKspMzyjJTczM0TU0MNPLTS0uTkxPzUlMKtZLzs/d xAgMWwYg2MHYtCjwEKMkB5OSKO/MiRvDhfiS8lMqMxKLM+KLSnNSiw8xynBwKEnw3j+3OVxIs Cg1PbUiLTMHGEEwaQkOHiUR3iUgad7igsTc4sx0iNQpRkUpcd4ikIQASCKjNA+uDRa1lxhlpY R5GYEOEeIpSC3KzSxBlX/FKM7BqCTMOxNkCk9mXgnc9FdAi5mAFutLbwBZXJKIkJJqYFS/mCd SM0Hr94xb9ZeMfx9wipMR3CV2aK3h00jO0Ev8qjfa/y6X2f/W+7dGD+dxm536nSWK+/i7zy+4 nn2kNrFrm+WalcobFY8vlvDusykt3b00q3Hj8tCJFt/1ts2qfjvNNGZyeGnv/62r75f9vC9jk XFxs8mJXdtPtm/MPhsw51XJxq/XDvEosRRnJBpqMRcVJwIAvzobxdUCAAA= X-Env-Sender: konrad.wilk@oracle.com X-Msg-Ref: server-15.tower-21.messagelabs.com!1471401979!28742437!1 X-Originating-IP: [156.151.31.81] X-SpamReason: No, hits=0.0 required=7.0 tests=sa_preprocessor: VHJ1c3RlZCBJUDogMTU2LjE1MS4zMS44MSA9PiAyODgzMzk=\n X-StarScan-Received: X-StarScan-Version: 8.84; banners=-,-,- X-VirusChecked: Checked Received: (qmail 16708 invoked from network); 17 Aug 2016 02:46:20 -0000 Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by server-15.tower-21.messagelabs.com with DHE-RSA-AES256-GCM-SHA384 encrypted SMTP; 17 Aug 2016 02:46:20 -0000 Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u7H2k7sd008951 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 17 Aug 2016 02:46:08 GMT Received: from userv0121.oracle.com (userv0121.oracle.com [156.151.31.72]) by aserv0021.oracle.com (8.13.8/8.13.8) with ESMTP id u7H2k7Bo002121 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 17 Aug 2016 02:46:07 GMT Received: from abhmp0004.oracle.com (abhmp0004.oracle.com [141.146.116.10]) by userv0121.oracle.com (8.13.8/8.13.8) with ESMTP id u7H2k5K0023876; Wed, 17 Aug 2016 02:46:06 GMT Received: from localhost.localdomain (/209.6.196.81) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 16 Aug 2016 19:46:05 -0700 Date: Tue, 16 Aug 2016 22:45:58 -0400 From: Konrad Rzeszutek Wilk To: Andrew Cooper Message-ID: <20160817024552.GA29431@localhost.localdomain> References: <1471216074-3007-1-git-send-email-konrad.wilk@oracle.com> <1471216074-3007-7-git-send-email-konrad.wilk@oracle.com> <57B197BC0200007800105CCA@prv-mh.provo.novell.com> <20160815140934.GE26970@char.us.oracle.com> <57B1EC6D0200007800106011@prv-mh.provo.novell.com> <97cb0b0f-2b8f-bed0-9b2e-788dc97a043a@arm.com> <20160815151743.GN26970@char.us.oracle.com> <814840f8-cf04-b80f-f310-53bf53522f74@arm.com> <8e1149ed-88be-27d6-0d72-a07750ae3615@citrix.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <8e1149ed-88be-27d6-0d72-a07750ae3615@citrix.com> User-Agent: Mutt/1.6.1 (2016-04-27) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Cc: sstabellini@kernel.org, ross.lagerwall@citrix.com, Julien Grall , Jan Beulich , xen-devel@lists.xenproject.org Subject: Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support. X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Mon, Aug 15, 2016 at 04:27:12PM +0100, Andrew Cooper wrote: > On 15/08/16 16:25, Julien Grall wrote: > > > > > > On 15/08/2016 17:17, Konrad Rzeszutek Wilk wrote: > >> On Mon, Aug 15, 2016 at 04:57:26PM +0200, Julien Grall wrote: > >>> Hi Jan and Konrad, > >>> > >>> On 15/08/2016 16:23, Jan Beulich wrote: > >>>>>>> On 15.08.16 at 16:09, wrote: > >>>>> On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote: > >>>>>>>>> On 15.08.16 at 01:07, wrote: > >>>>>>> @@ -711,9 +711,15 @@ static int prepare_payload(struct payload > >>>>>>> *payload, > >>>>>>> return -EINVAL; > >>>>>>> } > >>>>>>> } > >>>>>>> +#ifndef CONFIG_ARM > >>>>>>> apply_alternatives_nocheck(start, end); > >>>>>>> +#else > >>>>>>> + apply_alternatives(start, sec->sec->sh_size); > >>>>>>> +#endif > >>>>>> > >>>>>> Conditionals like this are ugly - can't this be properly abstracted? > >>>>> > >>>>> Yes, I can introduce an apply_alternatives_nocheck on ARM that will > >>>>> hava the same set of arguments on x86. > >>>>> > >>>>> Or I can make a new function name? > >>>> > >>>> Either way is fine with me, with a slight preference to the former > >>>> one. > >>> > >>> I am fine with the prototype of the function > >>> apply_alternatives_nocheck but > >>> I don't think the name is relevant for ARM. > >>> > >>> Is there any reason we don't want to call directly > >>> apply_alternatives in > >>> x86? > >> > >> It assumes (and has an ASSERT) that it is called with interrupts > >> disabled. > >> And we don't need to do that (as during livepatch loading we can > >> modify the > >> livepatch payload without worrying about interrupts). > > > > Oh, it makes more sense now. > > > >> > >> P.S. > >> loading != applying. > >> > >> I could do a patch where we rename 'apply_alternatives' -> > >> 'apply_alternatives_boot' > >> and 'apply_alternatives_nocheck' to 'apply_alternatives'. > > The only reason apply_alternatives() is named thusly is to match Linux. > I am not fussed if it changes. Would this be OK with folks? There is a bit of disreprancy - ARM has 'const struct alt_instr *' where the 'const' gets dropped later on. That can't be done x86 as 'apply_alternatives' at the gecko modifies the structure. Thoughts? Make it the same on ARM and x86? From 2c26d4d214926cd23b73f98c1fdaecd98b010da6 Mon Sep 17 00:00:00 2001 From: Konrad Rzeszutek Wilk Date: Tue, 16 Aug 2016 22:20:54 -0400 Subject: [PATCH] alternatives: x86 rename and change parameters on ARM On x86 we rename 'apply_alternatives' -> 'apply_alternatives_boot' and 'apply_alternatives_nocheck' to 'apply_alternatives'. On ARM we change the parameters for 'apply_alternatives' to be of 'const struct alt_instr *' instead of void pointer and size length. Signed-off-by: Konrad Rzeszutek Wilk --- Cc: Stefano Stabellini Cc: Julien Grall Cc: Andrew Cooper Cc: Jan Beulich v3.1: First submission. --- xen/arch/arm/alternative.c | 4 ++-- xen/arch/x86/alternative.c | 8 ++++---- xen/common/livepatch.c | 2 +- xen/include/asm-arm/alternative.h | 2 +- xen/include/asm-x86/alternative.h | 5 ++--- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c index bf4101c..aba06db 100644 --- a/xen/arch/arm/alternative.c +++ b/xen/arch/arm/alternative.c @@ -200,11 +200,11 @@ void __init apply_alternatives_all(void) BUG_ON(ret); } -int apply_alternatives(void *start, size_t length) +int apply_alternatives(const struct alt_instr *start, const struct alt_instr *end) { const struct alt_region region = { .begin = start, - .end = start + length, + .end = end, }; return __apply_alternatives(®ion); diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index fd8528e..7addc2c 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -144,7 +144,7 @@ static void *init_or_livepatch text_poke(void *addr, const void *opcode, size_t * APs have less capabilities than the boot processor are not handled. * Tough. Make sure you disable such features by hand. */ -void init_or_livepatch apply_alternatives_nocheck(struct alt_instr *start, struct alt_instr *end) +void init_or_livepatch apply_alternatives(struct alt_instr *start, struct alt_instr *end) { struct alt_instr *a; u8 *instr, *replacement; @@ -187,7 +187,7 @@ void init_or_livepatch apply_alternatives_nocheck(struct alt_instr *start, struc * This routine is called with local interrupt disabled and used during * bootup. */ -void __init apply_alternatives(struct alt_instr *start, struct alt_instr *end) +void __init apply_alternatives_boot(struct alt_instr *start, struct alt_instr *end) { unsigned long cr0 = read_cr0(); @@ -196,7 +196,7 @@ void __init apply_alternatives(struct alt_instr *start, struct alt_instr *end) /* Disable WP to allow application of alternatives to read-only pages. */ write_cr0(cr0 & ~X86_CR0_WP); - apply_alternatives_nocheck(start, end); + apply_alternatives(start, end); /* Reinstate WP. */ write_cr0(cr0); @@ -225,7 +225,7 @@ void __init alternative_instructions(void) * expect a machine check to cause undue problems during to code * patching. */ - apply_alternatives(__alt_instructions, __alt_instructions_end); + apply_alternatives_boot(__alt_instructions, __alt_instructions_end); set_nmi_callback(saved_nmi_callback); } diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index 238492a..4458751 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -698,7 +698,7 @@ static int prepare_payload(struct payload *payload, return -EINVAL; } } - apply_alternatives_nocheck(start, end); + apply_alternatives(start, end); } sec = livepatch_elf_sec_by_name(elf, ".ex_table"); diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h index 58d5fa7..b6fa827 100644 --- a/xen/include/asm-arm/alternative.h +++ b/xen/include/asm-arm/alternative.h @@ -26,7 +26,7 @@ struct alt_instr { #define ALT_REPL_PTR(a) __ALT_PTR(a, alt_offset) void __init apply_alternatives_all(void); -int apply_alternatives(void *start, size_t length); +int apply_alternatives(const struct alt_instr *start, const struct alt_instr *end); #define ALTINSTR_ENTRY(feature) \ " .word 661b - .\n" /* label */ \ diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h index b72c401..47fa1be 100644 --- a/xen/include/asm-x86/alternative.h +++ b/xen/include/asm-x86/alternative.h @@ -28,10 +28,9 @@ struct alt_instr { #define ALT_REPL_PTR(a) __ALT_PTR(a, repl_offset) extern void add_nops(void *insns, unsigned int len); -/* Similar to apply_alternatives except it can be run with IRQs enabled. */ -extern void apply_alternatives_nocheck(struct alt_instr *start, - struct alt_instr *end); +/* Similar to apply_alternatives_boot except it can be run with IRQs enabled. */ extern void apply_alternatives(struct alt_instr *start, struct alt_instr *end); +extern void apply_alternatives_boot(struct alt_instr *start, struct alt_instr *end); extern void alternative_instructions(void); #define OLDINSTR(oldinstr) "661:\n\t" oldinstr "\n662:\n"