From patchwork Wed Jun 14 08:20:39 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Alexandre Ghiti X-Patchwork-Id: 1794845 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:3::133; helo=bombadil.infradead.org; envelope-from=opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Authentication-Results: legolas.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=DhqvNHnk; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=rivosinc-com.20221208.gappssmtp.com header.i=@rivosinc-com.20221208.gappssmtp.com header.a=rsa-sha256 header.s=20221208 header.b=pLq3xrkP; dkim-atps=neutral Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:3::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4Qgz0937nBz20QH for ; Wed, 14 Jun 2023 18:21:11 +1000 (AEST) 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:MIME-Version:Message-Id:Date:Subject:Cc :To:From:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References: List-Owner; bh=zbS6eemw17ez5D8TrBGujZgUyl/arWH7bCLLReUpkEo=; b=DhqvNHnkFo1TqZ TV4UiHGEz7qzuxD13Xau5o8Tz3qLGauFGuc9Fm80fpB0Fe1rhGrU4VyAapEbYG28Gsia/E8VUuEaI 2xXqtNdZOacbBJVx77DyOx30Eiei2QPLo3O1QvDH8YqumNOD2cIm66+3wZynG+u8KRyqH347OaJbL ooel2AmRfNtOTm+hGj00hgjgrMmQKULfNsEkS8ItUivIU4sltUlrmNJ8tn7zstGwWyV4m7V/Xcbc2 63+XjouE62bCvA0TGVSIKNdA2Yk8XlmeIOgzRtLxjT+9cJzxO2wLaja0bIerOrBScHwUDNj92iX8j SpEI884c1oFTK/YhiZZg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1q9LkG-00Aru9-0M; Wed, 14 Jun 2023 08:20:48 +0000 Received: from mail-wm1-x330.google.com ([2a00:1450:4864:20::330]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1q9LkC-00Arsf-2X for opensbi@lists.infradead.org; Wed, 14 Jun 2023 08:20:47 +0000 Received: by mail-wm1-x330.google.com with SMTP id 5b1f17b1804b1-3f736e0c9b1so4342085e9.3 for ; Wed, 14 Jun 2023 01:20:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20221208.gappssmtp.com; s=20221208; t=1686730842; x=1689322842; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=wx4bo1TT4ZBPiEfEehQE8UMq2ilRIgbUW27N92TF3is=; b=pLq3xrkPt9/qdIGn6UUwBAo7mj6Nm23nYnT5iLGgZyYCzCU8sFeJ4pJgSDyMEg8qoD BN6dRlycdE5BP3/kGz1N+nLLz2N5YN1M9yJI/Hu29ts9fUUKNWHXFhVC3kX9WMtSVrPN zsk5RwiJHoqkCY17yZFzyUB+e2MloJGxSDG/ASuz7yPwHL/ecN1jq6jImIkxEJPC34kF fsPoOwvV0li95aMewJ1N7r5hOSe7+NY0urNzCrWMSESe4l/AhJzpvsSQhfhpbpMwnPZu zh81Gls/goF/aiRwLPBzi3BMFNyFzvseaHcm7JxkgUGKTU5bQengs0xCbsFB/rCm8a8O hRmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686730842; x=1689322842; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=wx4bo1TT4ZBPiEfEehQE8UMq2ilRIgbUW27N92TF3is=; b=cM3Q4eVkWCw4rVnyzVeXpDQ46ggnBUIVZH0c/q1UIK4VlWn0PO5GCVTRdBL1aFXMTk 3v1tBdY10DXsnQRLXthz3OD+j8SVqFZ/Ja1Mnr9ziC9kUrJ5oQaUO3MNqVj2v+ils02E ejEL688EiaOiW0tsFBNU3TDr7xnGbU796uVWdrDnkc5OLXJmxLR5x+gJbRbG+ki5nf8F 09krQbiN7kAyz1OzfdL+HTcgMka80tvvoWzDyLAxsg8a1Gr+DNm/rAETNPnqnMGKg/2t xhYl1/5j2IOnMd4y/rEd2WVC5KTwEc/SP+GaanUs8UxWvGCxxlTV84yE9d/5zyH7ZdmW UJ+g== X-Gm-Message-State: AC+VfDzr5Ixv9/vs7T0jLS4ocqZgpjt6b7dpgrihreF/ltu1pRifLzwB MyJL9NbK1XBbQzBgny48HAWpZD/q9TL0cmhmLdo= X-Google-Smtp-Source: ACHHUZ55EirtoipjkOL4VXJwhlEV8Deao6SXXm1ytyVleT/A/kWHz3m8lIvXve/ituOhfT/aWgaYAA== X-Received: by 2002:a7b:cd11:0:b0:3f7:4961:52ad with SMTP id f17-20020a7bcd11000000b003f7496152admr11097188wmj.3.1686730841578; Wed, 14 Jun 2023 01:20:41 -0700 (PDT) Received: from alex-rivos.ba.rivosinc.com (amontpellier-656-1-456-62.w92-145.abo.wanadoo.fr. [92.145.124.62]) by smtp.gmail.com with ESMTPSA id z21-20020a1c4c15000000b003f80622bb65sm15641426wmf.12.2023.06.14.01.20.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 14 Jun 2023 01:20:40 -0700 (PDT) From: Alexandre Ghiti To: opensbi@lists.infradead.org Cc: Alexandre Ghiti Subject: [PATCH -fixes] platform/lib: Set no-map attribute on all PMP regions Date: Wed, 14 Jun 2023 10:20:39 +0200 Message-Id: <20230614082039.484666-1-alexghiti@rivosinc.com> X-Mailer: git-send-email 2.39.2 MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230614_012045_045351_28AEEAA5 X-CRM114-Status: GOOD ( 22.96 ) X-Spam-Score: 0.0 (/) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: This reverts commit 6966ad0abe70 ("platform/lib: Allow the OS to map the regions that are protected by PMP"). It was thought at the time of this commit that allowing the kernel to map PMP protected regions was safe but it is actually not: for example, the hibernation process will try to access any linear mapp [...] Content analysis details: (0.0 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2a00:1450:4864:20:0:0:0:330 listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid X-BeenThere: opensbi@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "opensbi" Errors-To: opensbi-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org This reverts commit 6966ad0abe70 ("platform/lib: Allow the OS to map the regions that are protected by PMP"). It was thought at the time of this commit that allowing the kernel to map PMP protected regions was safe but it is actually not: for example, the hibernation process will try to access any linear mapping page and then will fault on such mapped PMP regions [1]. Another issue is that the device tree specification [2] states that a !no-map region must be declared as EfiBootServicesData/Code in the EFI memory map which would make the PMP protected regions reclaimable by the kernel. And to circumvent this, RISC-V edk2 diverges from the DT specification to declare those regions as EfiReserved. The no-map attribute was removed to allow the kernel to use hugepages larger than 2MB to map the linear mapping to improve the performance but actually a recent talk from Mike Rapoport [3] stated that the performance benefit was marginal. For all those reasons, let's mark all the PMP protected regions as "no-map". [1] https://lore.kernel.org/linux-riscv/CAAYs2=gQvkhTeioMmqRDVGjdtNF_vhB+vm_1dHJxPNi75YDQ_Q@mail.gmail.com/ [2] "3.5.4 /reserved-memory and UEFI" https://github.com/devicetree-org/devicetree-specification/releases/download/v0.4-rc1/devicetree-specification-v0.4-rc1.pdf [3] https://lwn.net/Articles/931406/ Signed-off-by: Alexandre Ghiti Reviewed-by: Atish Patra Reviewed-by: Xiang W --- include/sbi_utils/fdt/fdt_fixup.h | 14 --------- lib/utils/fdt/fdt_fixup.c | 49 +++++++------------------------ platform/generic/sifive/fu540.c | 13 -------- 3 files changed, 10 insertions(+), 66 deletions(-) diff --git a/include/sbi_utils/fdt/fdt_fixup.h b/include/sbi_utils/fdt/fdt_fixup.h index cab3f0f..ecd55a7 100644 --- a/include/sbi_utils/fdt/fdt_fixup.h +++ b/include/sbi_utils/fdt/fdt_fixup.h @@ -93,20 +93,6 @@ void fdt_plic_fixup(void *fdt); */ int fdt_reserved_memory_fixup(void *fdt); -/** - * Fix up the reserved memory subnodes in the device tree - * - * This routine adds the no-map property to the reserved memory subnodes so - * that the OS does not map those PMP protected memory regions. - * - * Platform codes must call this helper in their final_init() after fdt_fixups() - * if the OS should not map the PMP protected reserved regions. - * - * @param fdt: device tree blob - * @return zero on success and -ve on failure - */ -int fdt_reserved_memory_nomap_fixup(void *fdt); - /** * General device tree fix-up * diff --git a/lib/utils/fdt/fdt_fixup.c b/lib/utils/fdt/fdt_fixup.c index ae6be00..e213ded 100644 --- a/lib/utils/fdt/fdt_fixup.c +++ b/lib/utils/fdt/fdt_fixup.c @@ -210,7 +210,7 @@ void fdt_plic_fixup(void *fdt) static int fdt_resv_memory_update_node(void *fdt, unsigned long addr, unsigned long size, int index, - int parent, bool no_map) + int parent) { int na = fdt_address_cells(fdt, 0); int ns = fdt_size_cells(fdt, 0); @@ -239,16 +239,14 @@ static int fdt_resv_memory_update_node(void *fdt, unsigned long addr, if (subnode < 0) return subnode; - if (no_map) { - /* - * Tell operating system not to create a virtual - * mapping of the region as part of its standard - * mapping of system memory. - */ - err = fdt_setprop_empty(fdt, subnode, "no-map"); - if (err < 0) - return err; - } + /* + * Tell operating system not to create a virtual + * mapping of the region as part of its standard + * mapping of system memory. + */ + err = fdt_setprop_empty(fdt, subnode, "no-map"); + if (err < 0) + return err; /* encode the property value */ val = reg; @@ -286,7 +284,6 @@ int fdt_reserved_memory_fixup(void *fdt) { struct sbi_domain_memregion *reg; struct sbi_domain *dom = sbi_domain_thishart_ptr(); - struct sbi_scratch *scratch = sbi_scratch_thishart_ptr(); unsigned long filtered_base[PMP_COUNT] = { 0 }; unsigned char filtered_order[PMP_COUNT] = { 0 }; unsigned long addr, size; @@ -382,33 +379,7 @@ int fdt_reserved_memory_fixup(void *fdt) for (j = 0; j < i; j++) { addr = filtered_base[j]; size = 1UL << filtered_order[j]; - fdt_resv_memory_update_node(fdt, addr, size, j, parent, - (sbi_hart_pmp_count(scratch)) - ? false : true); - } - - return 0; -} - -int fdt_reserved_memory_nomap_fixup(void *fdt) -{ - int parent, subnode; - int err; - - /* Locate the reserved memory node */ - parent = fdt_path_offset(fdt, "/reserved-memory"); - if (parent < 0) - return parent; - - fdt_for_each_subnode(subnode, fdt, parent) { - /* - * Tell operating system not to create a virtual - * mapping of the region as part of its standard - * mapping of system memory. - */ - err = fdt_setprop_empty(fdt, subnode, "no-map"); - if (err < 0) - return err; + fdt_resv_memory_update_node(fdt, addr, size, j, parent); } return 0; diff --git a/platform/generic/sifive/fu540.c b/platform/generic/sifive/fu540.c index 08f7bfc..b980f44 100644 --- a/platform/generic/sifive/fu540.c +++ b/platform/generic/sifive/fu540.c @@ -20,18 +20,6 @@ static u64 sifive_fu540_tlbr_flush_limit(const struct fdt_match *match) return 0; } -static int sifive_fu540_fdt_fixup(void *fdt, const struct fdt_match *match) -{ - /* - * SiFive Freedom U540 has an erratum that prevents S-mode software - * to access a PMP protected region using 1GB page table mapping, so - * always add the no-map attribute on this platform. - */ - fdt_reserved_memory_nomap_fixup(fdt); - - return 0; -} - static const struct fdt_match sifive_fu540_match[] = { { .compatible = "sifive,fu540" }, { .compatible = "sifive,fu540g" }, @@ -43,5 +31,4 @@ static const struct fdt_match sifive_fu540_match[] = { const struct platform_override sifive_fu540 = { .match_table = sifive_fu540_match, .tlbr_flush_limit = sifive_fu540_tlbr_flush_limit, - .fdt_fixup = sifive_fu540_fdt_fixup, };