From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.7 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0B763C433DB for ; Mon, 1 Feb 2021 16:34:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C91A764E95 for ; Mon, 1 Feb 2021 16:34:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231529AbhBAQeM (ORCPT ); Mon, 1 Feb 2021 11:34:12 -0500 Received: from foss.arm.com ([217.140.110.172]:34312 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230085AbhBAQdc (ORCPT ); Mon, 1 Feb 2021 11:33:32 -0500 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9A1B01042; Mon, 1 Feb 2021 08:32:44 -0800 (PST) Received: from C02TD0UTHF1T.local (unknown [10.57.41.104]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 17B6B3F718; Mon, 1 Feb 2021 08:32:42 -0800 (PST) Date: Mon, 1 Feb 2021 16:32:40 +0000 From: Mark Rutland To: Russell King - ARM Linux admin Cc: Giancarlo Ferrari , linux-kernel@vger.kernel.org, penberg@kernel.org, geert@linux-m68k.org, linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, rppt@kernel.org, giancarlo.ferrari@nokia.com Subject: Re: [PATCH] ARM: kexec: Fix panic after TLB are invalidated Message-ID: <20210201163240.GE66060@C02TD0UTHF1T.local> References: <1612140296-12546-1-git-send-email-giancarlo.ferrari89@gmail.com> <20210201124720.GA66060@C02TD0UTHF1T.local> <20210201130344.GF1463@shell.armlinux.org.uk> <20210201135714.GB66060@C02TD0UTHF1T.local> <20210201160838.GH1463@shell.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210201160838.GH1463@shell.armlinux.org.uk> Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Feb 01, 2021 at 04:08:38PM +0000, Russell King - ARM Linux admin wrote: > On Mon, Feb 01, 2021 at 01:57:14PM +0000, Mark Rutland wrote: > > We could simplify this slightly if we moved the kexec_& variables into a > > struct (using asm-offset KEXEC_VAR_* offsets and a KEXEC_VAR_SIZE region > > reserved in the asm), then here we could do something like: > > > > static struct kexec_vars *kexec_buffer_vars(void *buffer) > > { > > unsigned long code = ((unisigned long)relocate_new_kernel) & ~1; > > unsigned long vars - (unsigned long)relocate_vars; > > unsigned long offset = vars - code; > > > > return buffer + offset; > > } > > > > ... and in machine_kexec() do: > > > > struct kexec_vars *kv = kexec_buffer_vars(reboot_code_buffer); > > > > kv->start_address = image->start; > > kv->indirection_page = page_list; > > kv->mach_type = machine-arch_type; > > kv->boot_atags = arch.kernel_r2; > > > > ... if that looks any better to you? > > Something like this? Nice! That looks about right to me, modulo a bit of cache maintenance noted below. > diff --git a/arch/arm/include/asm/kexec-internal.h b/arch/arm/include/asm/kexec-internal.h > new file mode 100644 > index 000000000000..ecc2322db7aa > --- /dev/null > +++ b/arch/arm/include/asm/kexec-internal.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ARM_KEXEC_INTERNAL_H > +#define _ARM_KEXEC_INTERNAL_H > + > +struct kexec_relocate_data { > + unsigned long kexec_start_address; > + unsigned long kexec_indirection_page; > + unsigned long kexec_mach_type; > + unsigned long kexec_r2; > +}; > + > +#endif > diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c > index a1570c8bab25..be8050b0c3df 100644 > --- a/arch/arm/kernel/asm-offsets.c > +++ b/arch/arm/kernel/asm-offsets.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -170,5 +171,9 @@ int main(void) > DEFINE(MPU_RGN_PRBAR, offsetof(struct mpu_rgn, prbar)); > DEFINE(MPU_RGN_PRLAR, offsetof(struct mpu_rgn, prlar)); > #endif > + DEFINE(KEXEC_START_ADDR, offsetof(struct kexec_relocate_data, kexec_start_address)); > + DEFINE(KEXEC_INDIR_PAGE, offsetof(struct kexec_relocate_data, kexec_indirection_page)); > + DEFINE(KEXEC_MACH_TYPE, offsetof(struct kexec_relocate_data, kexec_mach_type)); > + DEFINE(KEXEC_R2, offsetof(struct kexec_relocate_data, kexec_r2)); > return 0; > } > diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c > index 5d84ad333f05..2b09dad7935e 100644 > --- a/arch/arm/kernel/machine_kexec.c > +++ b/arch/arm/kernel/machine_kexec.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -22,11 +23,6 @@ > extern void relocate_new_kernel(void); > extern const unsigned int relocate_new_kernel_size; > > -extern unsigned long kexec_start_address; > -extern unsigned long kexec_indirection_page; > -extern unsigned long kexec_mach_type; > -extern unsigned long kexec_boot_atags; > - > static atomic_t waiting_for_crash_ipi; > > /* > @@ -159,6 +155,7 @@ void (*kexec_reinit)(void); > void machine_kexec(struct kimage *image) > { > unsigned long page_list, reboot_entry_phys; > + struct kexec_relocate_data *data; > void (*reboot_entry)(void); > void *reboot_code_buffer; > > @@ -174,18 +171,17 @@ void machine_kexec(struct kimage *image) > > reboot_code_buffer = page_address(image->control_code_page); > > - /* Prepare parameters for reboot_code_buffer*/ > - set_kernel_text_rw(); > - kexec_start_address = image->start; > - kexec_indirection_page = page_list; > - kexec_mach_type = machine_arch_type; > - kexec_boot_atags = image->arch.kernel_r2; > - > /* copy our kernel relocation code to the control code page */ > reboot_entry = fncpy(reboot_code_buffer, > &relocate_new_kernel, > relocate_new_kernel_size); > > + data = reboot_code_buffer + relocate_new_kernel_size; > + data->kexec_start_address = image->start; > + data->kexec_indirection_page = page_list; > + data->kexec_mach_type = machine_arch_type; > + data->kexec_r2 = image->arch.kernel_r2; I reckon here we need: __cpuc_flush_dcache_area(reboot_code_buffer, relocate_new_kernel_size + sizeof(*data)); ... to make sure both the instructions and data are visible with the MMU off (since fncpy() only cleans to the PoU, not the PoC). Otherwise this all looks good to me. Mark. > + > /* get the identity mapping physical address for the reboot code */ > reboot_entry_phys = virt_to_idmap(reboot_entry); > > diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S > index 72a08786e16e..218d524360fc 100644 > --- a/arch/arm/kernel/relocate_kernel.S > +++ b/arch/arm/kernel/relocate_kernel.S > @@ -5,14 +5,16 @@ > > #include > #include > +#include > #include > > .align 3 /* not needed for this code, but keeps fncpy() happy */ > > ENTRY(relocate_new_kernel) > > - ldr r0,kexec_indirection_page > - ldr r1,kexec_start_address > + adr r7, relocate_new_kernel_end > + ldr r0, [r7, #KEXEC_INDIR_PAGE] > + ldr r1, [r7, #KEXEC_START_ADDR] > > /* > * If there is no indirection page (we are doing crashdumps) > @@ -57,34 +59,16 @@ ENTRY(relocate_new_kernel) > > 2: > /* Jump to relocated kernel */ > - mov lr,r1 > - mov r0,#0 > - ldr r1,kexec_mach_type > - ldr r2,kexec_boot_atags > - ARM( ret lr ) > - THUMB( bx lr ) > - > - .align > - > - .globl kexec_start_address > -kexec_start_address: > - .long 0x0 > - > - .globl kexec_indirection_page > -kexec_indirection_page: > - .long 0x0 > - > - .globl kexec_mach_type > -kexec_mach_type: > - .long 0x0 > - > - /* phy addr of the atags for the new kernel */ > - .globl kexec_boot_atags > -kexec_boot_atags: > - .long 0x0 > + mov lr, r1 > + mov r0, #0 > + ldr r1, [r7, #KEXEC_MACH_TYPE] > + ldr r2, [r7, #KEXEC_R2] > + ARM( ret lr ) > + THUMB( bx lr ) > > ENDPROC(relocate_new_kernel) > > + .align 3 > relocate_new_kernel_end: > > .globl relocate_new_kernel_size > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83987C433E0 for ; Mon, 1 Feb 2021 16:34:19 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 1F91964E95 for ; Mon, 1 Feb 2021 16:34:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1F91964E95 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.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=UR5Fu/PaesPR4N99TaK/GEWQmyZSiGn6sPZcYWUP/Dc=; b=tOgoWuwumX3A6UIgT330pIaZn 8WYTclKKgi7Am4Y55JJshGEeLtDVZbBdeGTMH4dUOqE6BUnWe8/M2ihb0RDnCskOtvHmxXzLnFXEY hQcyAVZbqnvZ/NOfVwOoi4yTUHsnKJDoNZk2EhDWiDxHLatkwT1+GiMqivoSy/ACPMS4RSLEPMtDE ML5Q8JTm05tESad6GAjbiX8QWrIigxRLrOCtCu7DiPEJBszEpcRnfv7g7oj0pBMn91G2EzMouiOZK i+RHKc8qCmUQVM67vlZzddndQ9DbtDboYHJlY+7yfRibtfCy/aHK9p1aHqiinHPSxJHrmOURAI6H4 2HPTUPBfw==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l6c89-0001hv-6q; Mon, 01 Feb 2021 16:32:49 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1l6c86-0001hP-GN for linux-arm-kernel@lists.infradead.org; Mon, 01 Feb 2021 16:32:47 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 9A1B01042; Mon, 1 Feb 2021 08:32:44 -0800 (PST) Received: from C02TD0UTHF1T.local (unknown [10.57.41.104]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 17B6B3F718; Mon, 1 Feb 2021 08:32:42 -0800 (PST) Date: Mon, 1 Feb 2021 16:32:40 +0000 From: Mark Rutland To: Russell King - ARM Linux admin Subject: Re: [PATCH] ARM: kexec: Fix panic after TLB are invalidated Message-ID: <20210201163240.GE66060@C02TD0UTHF1T.local> References: <1612140296-12546-1-git-send-email-giancarlo.ferrari89@gmail.com> <20210201124720.GA66060@C02TD0UTHF1T.local> <20210201130344.GF1463@shell.armlinux.org.uk> <20210201135714.GB66060@C02TD0UTHF1T.local> <20210201160838.GH1463@shell.armlinux.org.uk> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210201160838.GH1463@shell.armlinux.org.uk> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210201_113246_650126_93D555B2 X-CRM114-Status: GOOD ( 29.55 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-kernel@vger.kernel.org, penberg@kernel.org, geert@linux-m68k.org, rppt@kernel.org, Giancarlo Ferrari , akpm@linux-foundation.org, linux-arm-kernel@lists.infradead.org, giancarlo.ferrari@nokia.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Feb 01, 2021 at 04:08:38PM +0000, Russell King - ARM Linux admin wrote: > On Mon, Feb 01, 2021 at 01:57:14PM +0000, Mark Rutland wrote: > > We could simplify this slightly if we moved the kexec_& variables into a > > struct (using asm-offset KEXEC_VAR_* offsets and a KEXEC_VAR_SIZE region > > reserved in the asm), then here we could do something like: > > > > static struct kexec_vars *kexec_buffer_vars(void *buffer) > > { > > unsigned long code = ((unisigned long)relocate_new_kernel) & ~1; > > unsigned long vars - (unsigned long)relocate_vars; > > unsigned long offset = vars - code; > > > > return buffer + offset; > > } > > > > ... and in machine_kexec() do: > > > > struct kexec_vars *kv = kexec_buffer_vars(reboot_code_buffer); > > > > kv->start_address = image->start; > > kv->indirection_page = page_list; > > kv->mach_type = machine-arch_type; > > kv->boot_atags = arch.kernel_r2; > > > > ... if that looks any better to you? > > Something like this? Nice! That looks about right to me, modulo a bit of cache maintenance noted below. > diff --git a/arch/arm/include/asm/kexec-internal.h b/arch/arm/include/asm/kexec-internal.h > new file mode 100644 > index 000000000000..ecc2322db7aa > --- /dev/null > +++ b/arch/arm/include/asm/kexec-internal.h > @@ -0,0 +1,12 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ARM_KEXEC_INTERNAL_H > +#define _ARM_KEXEC_INTERNAL_H > + > +struct kexec_relocate_data { > + unsigned long kexec_start_address; > + unsigned long kexec_indirection_page; > + unsigned long kexec_mach_type; > + unsigned long kexec_r2; > +}; > + > +#endif > diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c > index a1570c8bab25..be8050b0c3df 100644 > --- a/arch/arm/kernel/asm-offsets.c > +++ b/arch/arm/kernel/asm-offsets.c > @@ -12,6 +12,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -170,5 +171,9 @@ int main(void) > DEFINE(MPU_RGN_PRBAR, offsetof(struct mpu_rgn, prbar)); > DEFINE(MPU_RGN_PRLAR, offsetof(struct mpu_rgn, prlar)); > #endif > + DEFINE(KEXEC_START_ADDR, offsetof(struct kexec_relocate_data, kexec_start_address)); > + DEFINE(KEXEC_INDIR_PAGE, offsetof(struct kexec_relocate_data, kexec_indirection_page)); > + DEFINE(KEXEC_MACH_TYPE, offsetof(struct kexec_relocate_data, kexec_mach_type)); > + DEFINE(KEXEC_R2, offsetof(struct kexec_relocate_data, kexec_r2)); > return 0; > } > diff --git a/arch/arm/kernel/machine_kexec.c b/arch/arm/kernel/machine_kexec.c > index 5d84ad333f05..2b09dad7935e 100644 > --- a/arch/arm/kernel/machine_kexec.c > +++ b/arch/arm/kernel/machine_kexec.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -22,11 +23,6 @@ > extern void relocate_new_kernel(void); > extern const unsigned int relocate_new_kernel_size; > > -extern unsigned long kexec_start_address; > -extern unsigned long kexec_indirection_page; > -extern unsigned long kexec_mach_type; > -extern unsigned long kexec_boot_atags; > - > static atomic_t waiting_for_crash_ipi; > > /* > @@ -159,6 +155,7 @@ void (*kexec_reinit)(void); > void machine_kexec(struct kimage *image) > { > unsigned long page_list, reboot_entry_phys; > + struct kexec_relocate_data *data; > void (*reboot_entry)(void); > void *reboot_code_buffer; > > @@ -174,18 +171,17 @@ void machine_kexec(struct kimage *image) > > reboot_code_buffer = page_address(image->control_code_page); > > - /* Prepare parameters for reboot_code_buffer*/ > - set_kernel_text_rw(); > - kexec_start_address = image->start; > - kexec_indirection_page = page_list; > - kexec_mach_type = machine_arch_type; > - kexec_boot_atags = image->arch.kernel_r2; > - > /* copy our kernel relocation code to the control code page */ > reboot_entry = fncpy(reboot_code_buffer, > &relocate_new_kernel, > relocate_new_kernel_size); > > + data = reboot_code_buffer + relocate_new_kernel_size; > + data->kexec_start_address = image->start; > + data->kexec_indirection_page = page_list; > + data->kexec_mach_type = machine_arch_type; > + data->kexec_r2 = image->arch.kernel_r2; I reckon here we need: __cpuc_flush_dcache_area(reboot_code_buffer, relocate_new_kernel_size + sizeof(*data)); ... to make sure both the instructions and data are visible with the MMU off (since fncpy() only cleans to the PoU, not the PoC). Otherwise this all looks good to me. Mark. > + > /* get the identity mapping physical address for the reboot code */ > reboot_entry_phys = virt_to_idmap(reboot_entry); > > diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S > index 72a08786e16e..218d524360fc 100644 > --- a/arch/arm/kernel/relocate_kernel.S > +++ b/arch/arm/kernel/relocate_kernel.S > @@ -5,14 +5,16 @@ > > #include > #include > +#include > #include > > .align 3 /* not needed for this code, but keeps fncpy() happy */ > > ENTRY(relocate_new_kernel) > > - ldr r0,kexec_indirection_page > - ldr r1,kexec_start_address > + adr r7, relocate_new_kernel_end > + ldr r0, [r7, #KEXEC_INDIR_PAGE] > + ldr r1, [r7, #KEXEC_START_ADDR] > > /* > * If there is no indirection page (we are doing crashdumps) > @@ -57,34 +59,16 @@ ENTRY(relocate_new_kernel) > > 2: > /* Jump to relocated kernel */ > - mov lr,r1 > - mov r0,#0 > - ldr r1,kexec_mach_type > - ldr r2,kexec_boot_atags > - ARM( ret lr ) > - THUMB( bx lr ) > - > - .align > - > - .globl kexec_start_address > -kexec_start_address: > - .long 0x0 > - > - .globl kexec_indirection_page > -kexec_indirection_page: > - .long 0x0 > - > - .globl kexec_mach_type > -kexec_mach_type: > - .long 0x0 > - > - /* phy addr of the atags for the new kernel */ > - .globl kexec_boot_atags > -kexec_boot_atags: > - .long 0x0 > + mov lr, r1 > + mov r0, #0 > + ldr r1, [r7, #KEXEC_MACH_TYPE] > + ldr r2, [r7, #KEXEC_R2] > + ARM( ret lr ) > + THUMB( bx lr ) > > ENDPROC(relocate_new_kernel) > > + .align 3 > relocate_new_kernel_end: > > .globl relocate_new_kernel_size > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel