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=-5.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 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 0073FC38A2A for ; Thu, 7 May 2020 16:23:54 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 534542053B for ; Thu, 7 May 2020 16:23:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="mHRRdOyy" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 534542053B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-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=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:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=MF8zUQQBtPb0ftGqvpQ2eIHh34Wqx388mIINJ2sMumI=; b=mHRRdOyy6Re3j1 ioi9PNEH/x7X8Gq8e1uX1PEAFPJZgEdQ8V0eioNX0P/QO36quLByuyZGV1EOXTFOcQ4NBkv38g1KD riWbIWOV3SyNGw9RxLSMrLtk6Hp+hfdvVdn3JI55DxSjrHqnTROFS2HA9mHyjX9UL5qxWEEjwx0Rj GlsqchPwHmMtqko5wc7Xs6dHD6vzmtUeMOp6i/ypcM04wNpNGBW4edIMftiTU0QFmk1RFYFw0d9xM O8wuqCxLmV/jvzpKCR+HBM5ksepOZOvbDe2UFcJxzpbilT/KeqBkRpBjFK8zoe5mpouBpBONPCBc7 jp3HIVe51rseOnG82Odg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jWjJN-0000br-6e; Thu, 07 May 2020 16:23:49 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jWjII-0007yM-Vf; Thu, 07 May 2020 16:22:46 +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 6EF1B101E; Thu, 7 May 2020 09:22:40 -0700 (PDT) Received: from [192.168.0.14] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B47893F71F; Thu, 7 May 2020 09:22:37 -0700 (PDT) Subject: Re: [PATCH v9 16/18] arm64: kexec: configure trans_pgd page table for kexec To: Pavel Tatashin References: <20200326032420.27220-1-pasha.tatashin@soleen.com> <20200326032420.27220-17-pasha.tatashin@soleen.com> From: James Morse Message-ID: <5ed92d16-123c-8b79-0fc1-4cefdee65d5d@arm.com> Date: Thu, 7 May 2020 17:22:36 +0100 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <20200326032420.27220-17-pasha.tatashin@soleen.com> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200507_092243_183259_A69DACF6 X-CRM114-Status: GOOD ( 23.18 ) 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: sashal@kernel.org, mark.rutland@arm.com, vladimir.murzin@arm.com, corbet@lwn.net, catalin.marinas@arm.com, bhsharma@redhat.com, steve.capper@arm.com, kexec@lists.infradead.org, linux-kernel@vger.kernel.org, jmorris@namei.org, linux-mm@kvack.org, rfontana@redhat.com, ebiederm@xmission.com, maz@kernel.org, matthias.bgg@gmail.com, tglx@linutronix.de, will@kernel.org, selindag@gmail.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Pavel, On 26/03/2020 03:24, Pavel Tatashin wrote: > Configure a page table located in kexec-safe memory that has > the following mappings: > > 1. identity mapping for text of relocation function with executable > permission. > 2. linear mappings for all source ranges > 3. linear mappings for all destination ranges. Its taken this long to work out your definition of linear here doesn't match the way the rest of the arch code uses the term. You are using the MMU to re-assemble the scattered kexec image in VA space, so that the relocation code doesn't have to walk the list. While its a cool trick, I don't think this is a good idea, it makes it much harder to debug as we have a new definition for VA->PA, instead of re-using the kernels. We should do the least surprising thing. The person debugging a problem's first assumptions should be correct. Doing this means any debug information printed before kexec() is suddenly useless for debugging a problem that occurs during relocation. ... Let me hack together what I've been describing and we can discuss whether its simpler. (most of next week is gone already though...) (some Nits below) > diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h > index 0f758fd51518..8f4332ac607a 100644 > --- a/arch/arm64/include/asm/kexec.h > +++ b/arch/arm64/include/asm/kexec.h > @@ -108,6 +108,12 @@ extern const unsigned long kexec_el2_vectors_offset; > * el2_vector If present means that relocation routine will go to EL1 > * from EL2 to do the copy, and then back to EL2 to do the jump > * to new world. > + * trans_ttbr0 idmap for relocation function and its argument > + * trans_ttbr1 linear map for source/destination addresses. > + * trans_t0sz t0sz for idmap page in trans_ttbr0 You should be able to load the TTBR0_EL1 (and corresponding TCR_EL1.T0SZ) before kicking off the relocation code. There should be no need to pass it in to assembly. For example, hibernate sets TTBR0_EL1 in create_safe_exec_page(). > + * src_addr linear map for source pages. > + * dst_addr linear map for destination pages. > + * copy_len Number of bytes that need to be copied > */ > struct kern_reloc_arg { > phys_addr_t head; > @@ -70,10 +71,90 @@ static void *kexec_page_alloc(void *arg) > return page_address(page); > } > > +/* > + * Map source segments starting from src_va, and map destination > + * segments starting from dst_va, and return size of copy in > + * *copy_len argument. > + * Relocation function essentially needs to do: > + * memcpy(dst_va, src_va, copy_len); > + */ > +static int map_segments(struct kimage *kimage, pgd_t *pgdp, > + struct trans_pgd_info *info, > + unsigned long src_va, > + unsigned long dst_va, > + unsigned long *copy_len) > +{ > + unsigned long *ptr = 0; > + unsigned long dest = 0; > + unsigned long len = 0; > + unsigned long entry, addr; > + int rc; > + > + for (entry = kimage->head; !(entry & IND_DONE); entry = *ptr++) { > + addr = entry & PAGE_MASK; > + > + switch (entry & IND_FLAGS) { > + case IND_DESTINATION: > + dest = addr; > + break; So we hope to always find a destination first? > + case IND_INDIRECTION: > + ptr = __va(addr); > + if (rc) > + return rc; Where does rc come from? > + break; > + case IND_SOURCE: > + rc = trans_pgd_map_page(info, pgdp, __va(addr), > + src_va, PAGE_KERNEL); > + if (rc) > + return rc; > + rc = trans_pgd_map_page(info, pgdp, __va(dest), > + dst_va, PAGE_KERNEL); > + if (rc) > + return rc; > + dest += PAGE_SIZE; > + src_va += PAGE_SIZE; > + dst_va += PAGE_SIZE; > + len += PAGE_SIZE; > + } > + } > + *copy_len = len; > + > + return 0; > +} > + > @@ -89,9 +170,18 @@ int machine_kexec_post_load(struct kimage *kimage) > kern_reloc_arg->el2_vector = __pa(reloc_code) > + kexec_el2_vectors_offset; > } > + > + /* > + * If relocation is not needed, we do not need to enable MMU in Strictly you aren't enabling it, but disabling it _after_ the relocation. > + * relocation routine, therefore do not create page tables for > + * scenarios such as crash kernel > + */ > + if (!(kimage->head & IND_DONE)) > + rc = mmu_relocate_setup(kimage, reloc_code, kern_reloc_arg); > + > kexec_image_info(kimage); > > - return 0; > + return rc; > } Thanks, James _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel