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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 134F6C433EF for ; Tue, 12 Oct 2021 09:49:51 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E38F661056 for ; Tue, 12 Oct 2021 09:49:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235750AbhJLJvu (ORCPT ); Tue, 12 Oct 2021 05:51:50 -0400 Received: from foss.arm.com ([217.140.110.172]:59854 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232891AbhJLJvr (ORCPT ); Tue, 12 Oct 2021 05:51:47 -0400 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 D5B6B1063; Tue, 12 Oct 2021 02:49:45 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.22.33]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 029083F694; Tue, 12 Oct 2021 02:49:43 -0700 (PDT) Date: Tue, 12 Oct 2021 10:49:36 +0100 From: Mark Rutland To: Anshuman Khandual Cc: linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , James Morse , Marc Zyngier , linux-kernel@vger.kernel.org Subject: Re: [PATCH V2] arm64/mm: Fix idmap on [16K|36VA|48PA] Message-ID: <20211012094936.GA14289@C02TD0UTHF1T.local> References: <1632807225-20189-1-git-send-email-anshuman.khandual@arm.com> <20211004104947.GA4430@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Oct 12, 2021 at 10:19:49AM +0530, Anshuman Khandual wrote: > On 10/4/21 4:19 PM, Mark Rutland wrote: > > On Tue, Sep 28, 2021 at 11:03:45AM +0530, Anshuman Khandual wrote: > > Ignoring backports, I'd prefer if we could refactor things such that we > > decouple the `idmap_pg_dir` creation from the `init_pg_dir` creation, > > Decoupling both page table creation makes sense. > > > and create the idmap in terms of the architectural levels rather than > > pgd/p4d/pud/pmd/pte, so that we can consistently create the idmap with > > at least 48 bits of VA. > > The rationale for creating the idmap table in terms of architectural > levels, rather than kernel pgd/p4d/pud/pmd/pte is to avoid handling > page table folding stuff and also to make it simpler ? Yup; decoupling it from the pgd/p4d/pud/pmd/pte levels means that we can avoid the conditional extension logic (which always has to be precisely the inverse of the regular table creation code), and means we can always map the idmap at page granularity regardless of whether we're using section mappings for the init_pg_dir. > > Pseudo-code wise, I'd like something that looks like: > > > > create_idmap(...) > > { > > idmap_va_bits = 48; > > idmap_t0size = TCR_T0SZ(48); > > > > if (need_52_bit_va(__idmap_text_start)) { > > s/__idmap_text_start/__idmap_text_end/ instead ? I don't think that's necessary. The idmap is always 4K aligned, and we force it to be no more than 4K in size, so as long as we can map the start we can map the rest. If the idmap were exactly 4K in size, and happend to be placed at PA 0x0000ffff_fffff000, we can map that with a 48-bit VA, but checking __idmap_text_end would force a 52-bit VA. That shouldn't really matter either way since the idmap is in the middle of the .text section, but I'd prefer we just check the start address so that we're using hte same address we'll use to create the mapping from. > > > if (!supports_52bit_va()) { > > some_early_spin_loop(); > > With a new CPU_STUCK_REASON_ code ? I'm happy with reusing CPU_STUCK_REASON_52_BIT_VA, or with adding some new CPU_STUCK_REASON_IDMAP_${whatever}. > > } > > idmap_va_bits = 52; > > idmap_t0size = TCR_T0SZ(52); > > } > > > > if (need_table_level(idmap_va_bits, -1)) > > create_table_level(-1, ...); > > > > if (need_table_level(idmap_va_bits, 0)) > > create_table_level(0, ...); > > > > if (need_table_level(idmap_va_bits, 1)) > > create_table_level(1, ...); > > > > if (need_table_level(idmap_va_bits, 2)) > > create_table_level(2, ...); > > > > create_table_level(3, ...); > > } > > > > ... which I think would be much easier to reason about consistently. > > > > How does that sound to you? > > This approach will be simpler and as you mentioned, easier to reason about. > > > > > I've pushed some preparatory rework out to my arm64/pgtable/idmap > > branch, splitting out a __create_idmap_tables() function (and ensuring > > that idmap_t0sz doesn't get silently overridden elsewhere): > > > > https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/pgtable/idmap > > git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/pgtable/idmap > > > > ... but I haven't had the chance to do the actual rework of the idmap > > creation code. > > > > I can send that as a series if that's helpful. > > I could also just pick those changes from the above branch and complete > the rework. I'm happy with that if it works for you. Thanks, Mark. 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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D0816C433F5 for ; Tue, 12 Oct 2021 09:51: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 95CA0604DC for ; Tue, 12 Oct 2021 09:51:54 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 95CA0604DC Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=bKBY0Q7/e5T02YA+VGOG32h897jD2+w8IKtjmCgU3FU=; b=3jUDHCH5YEt2L3 t+W5hnosbbA9NGTBYcGAh36CuVXhtc3ZhU0zLlQAC/Homa9Qwqtgjv62pi7KUdKLZ0Y2mb/fHrvZ4 Uf0A02Eg04UrlotBRuegAHP6ZK8q5a/LiqzW5Qei2m8WA/rhJitSqf6InbcEssUfEkqCfmLQnGmqT /OQ8I0lvRwYrMVqDRqTrpiC2Asmu7DNzaOXBCvxngDzzu2OsIRB0Yd07zljX6o0Ohql144Iml4lDf 2qZZgw9euFgZuIQnM98ktKIxcMwUYw/AB0jU9WM+BJf7R1ag9WKcGEdf4kiqydr2heiC/SIc7K2pB XJIbaEp5PpIltcQcybXw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1maEQ0-00CJJY-N6; Tue, 12 Oct 2021 09:49:56 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1maEPw-00CJIr-Ej for linux-arm-kernel@lists.infradead.org; Tue, 12 Oct 2021 09:49:54 +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 D5B6B1063; Tue, 12 Oct 2021 02:49:45 -0700 (PDT) Received: from C02TD0UTHF1T.local (unknown [10.57.22.33]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 029083F694; Tue, 12 Oct 2021 02:49:43 -0700 (PDT) Date: Tue, 12 Oct 2021 10:49:36 +0100 From: Mark Rutland To: Anshuman Khandual Cc: linux-arm-kernel@lists.infradead.org, Catalin Marinas , Will Deacon , James Morse , Marc Zyngier , linux-kernel@vger.kernel.org Subject: Re: [PATCH V2] arm64/mm: Fix idmap on [16K|36VA|48PA] Message-ID: <20211012094936.GA14289@C02TD0UTHF1T.local> References: <1632807225-20189-1-git-send-email-anshuman.khandual@arm.com> <20211004104947.GA4430@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211012_024952_635884_30EC444A X-CRM114-Status: GOOD ( 28.71 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , 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 Tue, Oct 12, 2021 at 10:19:49AM +0530, Anshuman Khandual wrote: > On 10/4/21 4:19 PM, Mark Rutland wrote: > > On Tue, Sep 28, 2021 at 11:03:45AM +0530, Anshuman Khandual wrote: > > Ignoring backports, I'd prefer if we could refactor things such that we > > decouple the `idmap_pg_dir` creation from the `init_pg_dir` creation, > > Decoupling both page table creation makes sense. > > > and create the idmap in terms of the architectural levels rather than > > pgd/p4d/pud/pmd/pte, so that we can consistently create the idmap with > > at least 48 bits of VA. > > The rationale for creating the idmap table in terms of architectural > levels, rather than kernel pgd/p4d/pud/pmd/pte is to avoid handling > page table folding stuff and also to make it simpler ? Yup; decoupling it from the pgd/p4d/pud/pmd/pte levels means that we can avoid the conditional extension logic (which always has to be precisely the inverse of the regular table creation code), and means we can always map the idmap at page granularity regardless of whether we're using section mappings for the init_pg_dir. > > Pseudo-code wise, I'd like something that looks like: > > > > create_idmap(...) > > { > > idmap_va_bits = 48; > > idmap_t0size = TCR_T0SZ(48); > > > > if (need_52_bit_va(__idmap_text_start)) { > > s/__idmap_text_start/__idmap_text_end/ instead ? I don't think that's necessary. The idmap is always 4K aligned, and we force it to be no more than 4K in size, so as long as we can map the start we can map the rest. If the idmap were exactly 4K in size, and happend to be placed at PA 0x0000ffff_fffff000, we can map that with a 48-bit VA, but checking __idmap_text_end would force a 52-bit VA. That shouldn't really matter either way since the idmap is in the middle of the .text section, but I'd prefer we just check the start address so that we're using hte same address we'll use to create the mapping from. > > > if (!supports_52bit_va()) { > > some_early_spin_loop(); > > With a new CPU_STUCK_REASON_ code ? I'm happy with reusing CPU_STUCK_REASON_52_BIT_VA, or with adding some new CPU_STUCK_REASON_IDMAP_${whatever}. > > } > > idmap_va_bits = 52; > > idmap_t0size = TCR_T0SZ(52); > > } > > > > if (need_table_level(idmap_va_bits, -1)) > > create_table_level(-1, ...); > > > > if (need_table_level(idmap_va_bits, 0)) > > create_table_level(0, ...); > > > > if (need_table_level(idmap_va_bits, 1)) > > create_table_level(1, ...); > > > > if (need_table_level(idmap_va_bits, 2)) > > create_table_level(2, ...); > > > > create_table_level(3, ...); > > } > > > > ... which I think would be much easier to reason about consistently. > > > > How does that sound to you? > > This approach will be simpler and as you mentioned, easier to reason about. > > > > > I've pushed some preparatory rework out to my arm64/pgtable/idmap > > branch, splitting out a __create_idmap_tables() function (and ensuring > > that idmap_t0sz doesn't get silently overridden elsewhere): > > > > https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/pgtable/idmap > > git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arm64/pgtable/idmap > > > > ... but I haven't had the chance to do the actual rework of the idmap > > creation code. > > > > I can send that as a series if that's helpful. > > I could also just pick those changes from the above branch and complete > the rework. I'm happy with that if it works for you. Thanks, Mark. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel