From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id CADAED8042C for ; Thu, 15 Feb 2024 22:17:44 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=ELEdukwpGPFtenIf0ZXFHe7SuIrQcnWT3pvEmCvp17U=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1708035463; v=1; b=nhTPeDspQJ/zxvpRZ7zmfBM+Ii2LNkjttH0skvujUuIH1chTvQlzIvw0KKMWGd9n+z1pGgO5 2dGrricKpL/7JhE3nHbnpjTA+H/mXfC28yYt9xj4Tf5g59fiTDaLFtq3uXzYn/hp6OTFZjkXr84 Vb55GdTWb42XIGv8zKrbn6WI= X-Received: by 127.0.0.2 with SMTP id qMg2YY7687511xyZCJAs2UOB; Thu, 15 Feb 2024 14:17:43 -0800 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web10.4930.1708035462120222148 for ; Thu, 15 Feb 2024 14:17:42 -0800 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 2F5A5CE2763 for ; Thu, 15 Feb 2024 22:17:39 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 634F8C433B1 for ; Thu, 15 Feb 2024 22:17:38 +0000 (UTC) X-Received: by mail-lf1-f53.google.com with SMTP id 2adb3069b0e04-511898b6c9eso1737644e87.3 for ; Thu, 15 Feb 2024 14:17:38 -0800 (PST) X-Gm-Message-State: suU3apbBzRqWNP8ZV9FdtFk2x7686176AA= X-Google-Smtp-Source: AGHT+IElHL2rckUXHU27zmXagKl2A1mT2CRl1k/VQGbo3Oy1VT2Z+2UGqudRQxytOA1BJ+sKSLwsA5WYRc+esH4OdnM= X-Received: by 2002:ac2:4c17:0:b0:511:879f:b12a with SMTP id t23-20020ac24c17000000b00511879fb12amr2082915lfq.43.1708035456510; Thu, 15 Feb 2024 14:17:36 -0800 (PST) MIME-Version: 1.0 References: <20240215003412.30983-1-osde@linux.microsoft.com> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 15 Feb 2024 23:17:24 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel][PATCH v1 1/1] MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations To: Oliver Smith-Denny Cc: devel@edk2.groups.io, Leif Lindholm , Sami Mujawar , Liming Gao Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=nhTPeDsp; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none) On Thu, 15 Feb 2024 at 20:55, Oliver Smith-Denny wrote: > > On 2/14/2024 11:50 PM, Ard Biesheuvel wrote: > > I looked at the EFI spec again, and EfiACPIReclaimMemory is not > > actually listed as a memory type that has this 64k alignment > > requirement. This makes sense, given that this memory type has no > > significance to the firmware itself, only to the OS. OTOH, reserved > > memory does appear there. > > > > So I suggest we fix that first, and then drop any mention of > > EfiACPIReclaimMemory from this patch. At least we'll have heap guard > > coverage for ACPI table allocations on arm64 going forward. > > > > The logic in question was added in 2007 in commit 28a00297189c, so > > this was probably the rule on Itanium, but that support is long gone. > > > > I wanted to understand this a little bit further. I do see that the spec > does not call out EfiACPIReclaimMemory as needing 64k alignment. > However, your statement that the memory type does not have have > significance to the FW, only to the OS, so we don't need the 64k > alignment is confusing to me. Isn't the 64k alignment needed only for > regions that the OS does care about? In order to read this information > at their 64k page granularity, doesn't this need to be aligned to 64k? > It is not about accessing the information in the pages. Most consumers don't care about this at all, and even code that does will rarely be materially different between 4k and 64k OSes. When running under the firmware, we run with 4k pages and programming the memory attributes is entirely under the control of the firmware itself. So in this phase, the alignment truly does not matter, especially because all of RAM is 1:1 mapped anyway. When running under the OS, all memory that is part of the OS's memory pool is entirely under its control, and this includes ACPI reclaim memory, given that the firmware itself should not be accessing this: the purpose of this memory type is specifically to expose information to the OS in memory that it can use as it sees fit after it is done processing the information. The alignment issues we need to avoid are: - runtime DXEs where we want to map code RO and data NX, which implies that the boundary between the two must be 64k aligned; - other EFI_MEMORY_RUNTIME regions that may be described with EFI_MEMORY_WC rather than EFI_MEMORY_WB; this could be related to implementations of the varstore or other runtime services, and the OS has no idea what the region is for and how the runtime code is using it; - reserved regions that are mapped, e.g., by AML code, as a SystemMemory OpRegion; there are examples in RAS code where the memory needs to be mapped uncached so that stores are immediately visible to the BMC or other agents that sit on the interconnect. The existence of a cacheable alias of the same mapping will interfere with this, i.e., in many cases it will simply make both mappings cacheable. > Or are you saying the OS can just read the 64k page(s) containing these > 4k aligned EfiACPIReclaimMemory pages and then start reading at the > calculated offset from within the larger page, since these pages don't > have any pointers to outside memory, unlike image sections? > In all these examples, the alignment *itself* is not the goal. The 64k alignment is way to ensure that the OS is never in a situation where a 64k page frame is shared between a cacheable and an uncacheable mapping, or between a ROX and a W+NX mapping. Note that, in the runtime DXE case, the only way to map the 64k page that has both code and data is to map it RWX, given that the contents need to appear contiguous in virtual memory (we had great fun with that when we designed the MAT) For the other cases, there may be mappings that are completely disjoint in the virtual address space. But if two mappings exist of the same physical page, one cacheable via one alias, and one non-cacheable via another, the result is unpredictable and things like RAS or BMC comms from runtime firmware may be completely broken. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115534): https://edk2.groups.io/g/devel/message/115534 Mute This Topic: https://groups.io/mt/104364784/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-