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 82307740032 for ; Thu, 15 Feb 2024 22:39:49 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=sTyAvYDBaORxjoxSLD1N0A5AALEYQFGvXL1C3rsqQQo=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1708036788; v=1; b=gVnb/R2fEHl3Giz5OzjOrfRidQ1M3FufYs9PzaOjyEnPzOlMU7tRT/1cZl3aH7Iio0nDfbRX 0CS7VqqnXQFKP5qOr48SgjNN35Aox0VkAT+hEbuoA8orala8h8tbKY2gMYbmJzcoWo7qlpNcUoQ 8xNOEHDnH93MCI6uJBjlJNVE= X-Received: by 127.0.0.2 with SMTP id t8NXYY7687511xBev4vVgr27; Thu, 15 Feb 2024 14:39:48 -0800 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web11.5450.1708036787446932962 for ; Thu, 15 Feb 2024 14:39:47 -0800 X-Received: from [10.137.194.171] (unknown [131.107.160.171]) by linux.microsoft.com (Postfix) with ESMTPSA id F072620B2000; Thu, 15 Feb 2024 14:39:46 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com F072620B2000 Message-ID: <5442dfa6-3c13-4800-898c-212300d39c49@linux.microsoft.com> Date: Thu, 15 Feb 2024 14:39:46 -0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel][PATCH v1 1/1] MdeModulePkg: DxeCore: Don't Guard Large Runtime Granularity Allocations To: devel@edk2.groups.io, ardb@kernel.org Cc: Leif Lindholm , Sami Mujawar , Liming Gao References: <20240215003412.30983-1-osde@linux.microsoft.com> From: "Oliver Smith-Denny" In-Reply-To: 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,osde@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: T29Qf2DkuLMtXwaUdNMNrF8xx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b="gVnb/R2f"; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io On 2/15/2024 2:17 PM, Ard Biesheuvel wrote: > 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? >> >=20 > 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. >=20 > 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. >=20 > 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. >=20 > 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. >=20 >=20 >> 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? >> >=20 > 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) >=20 > 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. >=20 >=20 I see, thanks this was very helpful. I was focused on the alignment to meet access (which as you say isn't really needed) and for image sections, but the cacheable/uncacheable piece makes sense, too. I'll send out the v2 for this patch. Thanks, Oliver -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115535): https://edk2.groups.io/g/devel/message/115535 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] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-