From patchwork Mon Oct 16 09:58:45 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Catalin Marinas X-Patchwork-Id: 10007977 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 9E42D601D5 for ; Mon, 16 Oct 2017 09:59:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 71D5C28421 for ; Mon, 16 Oct 2017 09:59:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 66BDF28429; Mon, 16 Oct 2017 09:59:42 +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,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id DD90328421 for ; Mon, 16 Oct 2017 09:59:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=31vyeZmYDHeBPAIfc/0WCh2nMVoNBPjntTdQpaBnNK0=; b=kcGFj+m69rEOEO wlQfPeZXJvZ6JlUQEgwaSsZGpf7pAWJHvDMvtLmcBC4K0YNugFttbs/WyFwB4crIoC540H/RZSMzb iyU3buyuFlqlkQhHPAYTjm68Bl58x7MO9CvAi0MqUdhzrhwO+PbP3g3bfqOjK9sDAMlEniORAZnFp 7ryUR0Z4+jtBcZ0T1tH8HDkQOgHhsilqvHwiUpzpRNxBotpHlazCm5cjI6zzTWc8gk7SVTCPvueoG dU6GsQdd80ZXGAQi9aLl0+duPvpgHN8x6eKBY+drQwpf6vsER6Ez+kzHNDXADM/UpuhuMQh3N17r2 kmuEeCkJAZoQWaO2RQpA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1e42B2-0000Nl-0Y; Mon, 16 Oct 2017 09:59:16 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1e42Ay-0000LH-LM for linux-arm-kernel@lists.infradead.org; Mon, 16 Oct 2017 09:59:14 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 52CAF1435; Mon, 16 Oct 2017 02:58:50 -0700 (PDT) Received: from armageddon.cambridge.arm.com (armageddon.cambridge.arm.com [10.1.206.84]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 5CC973F483; Mon, 16 Oct 2017 02:58:48 -0700 (PDT) Date: Mon, 16 Oct 2017 10:58:45 +0100 From: Catalin Marinas To: James Morse Subject: Re: [PATCH v3 04/13] arm64: alternatives: use tpidr_el2 on VHE hosts Message-ID: <20171016095845.htg2g4jkyw3nvzub@armageddon.cambridge.arm.com> References: <20170922182614.27885-1-james.morse@arm.com> <20170922182614.27885-5-james.morse@arm.com> <20171013153148.dnejsvhxeui6opfw@armageddon.cambridge.arm.com> <59E0EEE5.2020208@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <59E0EEE5.2020208@arm.com> User-Agent: NeoMutt/20170113 (1.7.2) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171016_025912_715938_64FDBEA8 X-CRM114-Status: GOOD ( 24.87 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, Lorenzo Pieralisi , Marc Zyngier , Will Deacon , Rob Herring , Loc Ho , kvmarm@lists.cs.columbia.edu, Christoffer Dall , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, Oct 13, 2017 at 05:50:45PM +0100, James Morse wrote: > Hi Catalin, > > On 13/10/17 16:31, Catalin Marinas wrote: > > On Fri, Sep 22, 2017 at 07:26:05PM +0100, James Morse wrote: > >> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > >> index cd52d365d1f0..8e4c7da2b126 100644 > >> --- a/arch/arm64/kernel/cpufeature.c > >> +++ b/arch/arm64/kernel/cpufeature.c > > >> @@ -1308,3 +1309,25 @@ static int __init enable_mrs_emulation(void) > >> } > >> > >> late_initcall(enable_mrs_emulation); > >> + > >> +int cpu_copy_el2regs(void *__unused) > >> +{ > >> + int do_copyregs = 0; > >> + > >> + /* > >> + * Copy register values that aren't redirected by hardware. > >> + * > >> + * Before code patching, we only set tpidr_el1, all CPUs need to copy > >> + * this value to tpidr_el2 before we patch the code. Once we've done > >> + * that, freshly-onlined CPUs will set tpidr_el2, so we don't need to > >> + * do anything here. > >> + */ > >> + asm volatile(ALTERNATIVE("mov %0, #1", "mov %0, #0", > >> + ARM64_HAS_VIRT_HOST_EXTN) > >> + : "=r" (do_copyregs) : : ); > > > > Can you just do: > > > > if (cpu_have_const_cap(ARM64_HAS_VIRT_HOST_EXTN)) > > write_sysreg(read_sysreg(tpidr_el1), tpidr_el2); > > > > At this point the capability bits should be set and the jump labels > > enabled. > > These are enabled too early, we haven't done patching yet. > > We need to copy tpidr_el1 -> tpidr_el2 on all CPUs that are online before code > patching. > > After code patching new CPUs set tpidr_el2 when they come online, so we don't > need to do the copy... but this enable method is still called. There is nothing > for us to do, and tpidr_el1 now contains junk, so the copy Ah, I get it now (should've read the comment but I usually expect the code to be obvious; it wasn't, at least to me, in this case ;)). You could have added the sysreg copying directly in the asm volatile. Anyway, I think it's better if we keep it entirely in C with this hunk (untested): --------------8<------------------------------ --------------8<------------------------------ and in the cpu_copy_el2regs(): if (!READ_ONCE(alternatives_applied)) write_sysreg(read_sysreg(tpidr_el1), tpidr_el2); diff --git a/arch/arm64/include/asm/alternative.h b/arch/arm64/include/asm/alternative.h index 6e1cb8c5af4d..f9e2f69f296e 100644 --- a/arch/arm64/include/asm/alternative.h +++ b/arch/arm64/include/asm/alternative.h @@ -11,6 +11,8 @@ #include #include +extern int alternatives_applied; + struct alt_instr { s32 orig_offset; /* offset to original instruction */ s32 alt_offset; /* offset to replacement instruction */ diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c index 6dd0a3a3e5c9..414288a558c8 100644 --- a/arch/arm64/kernel/alternative.c +++ b/arch/arm64/kernel/alternative.c @@ -32,6 +32,8 @@ #define ALT_ORIG_PTR(a) __ALT_PTR(a, orig_offset) #define ALT_REPL_PTR(a) __ALT_PTR(a, alt_offset) +int alternatives_applied; + struct alt_region { struct alt_instr *begin; struct alt_instr *end; @@ -143,7 +145,6 @@ static void __apply_alternatives(void *alt_region, bool use_linear_alias) */ static int __apply_alternatives_multi_stop(void *unused) { - static int patched = 0; struct alt_region region = { .begin = (struct alt_instr *)__alt_instructions, .end = (struct alt_instr *)__alt_instructions_end, @@ -151,14 +152,14 @@ static int __apply_alternatives_multi_stop(void *unused) /* We always have a CPU 0 at this point (__init) */ if (smp_processor_id()) { - while (!READ_ONCE(patched)) + while (!READ_ONCE(alternatives_applied)) cpu_relax(); isb(); } else { - BUG_ON(patched); + BUG_ON(alternatives_applied); __apply_alternatives(®ion, true); /* Barriers provided by the cache flushing */ - WRITE_ONCE(patched, 1); + WRITE_ONCE(alternatives_applied, 1); } return 0;