From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail04.groups.io (mail04.groups.io [45.79.224.9]) by spool.mail.gandi.net (Postfix) with ESMTPS id 3DEE1D80281 for ; Wed, 17 Apr 2024 14:05:31 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=784iWER5+ou3KkG878SZVi1WW92qLJM42ygD1v5826Y=; c=relaxed/simple; d=groups.io; h=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:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20240206; t=1713362729; v=1; b=yfmcVMYr1Uasi0BZ5V/pD4H4eLztEfP0b+veEM28FqeneE8K0DFTSiJAjqsts3sMFFZNWn3C PvwL8LpX6wnoMtcWhekLuLQcrUidzAyFsq9S/PiLIVhzq9mxfR+RoCAdlqADF1w3WuwnvNvHHVT naM55krHlxgztlR5OutvozKpag+xVmA9O4Q2EDsvmaFHmxLzT8PvkJt5wpzINpXygCczgcW+BGp lbIW24+QCxs6hR3uq5SFO/FUsxWHe1MRhAjyoqmEAjU3c1sHER5Y2QI/XNBiPtoSZbc8tT6BeAz BV4t1aJFXKT1WhqaqsPHWDgD11F0Y5JrxBNJcK80QeQtQ== X-Received: by 127.0.0.2 with SMTP id QMhyYY7687511xJdv7ciy3wK; Wed, 17 Apr 2024 07:05:29 -0700 X-Received: from mail-pf1-f181.google.com (mail-pf1-f181.google.com [209.85.210.181]) by mx.groups.io with SMTP id smtpd.web10.13789.1713362729230918062 for ; Wed, 17 Apr 2024 07:05:29 -0700 X-Received: by mail-pf1-f181.google.com with SMTP id d2e1a72fcca58-6ecf05fd12fso5266716b3a.2 for ; Wed, 17 Apr 2024 07:05:29 -0700 (PDT) X-Forwarded-Encrypted: i=1; AJvYcCVwJ45+YWS+tXDNxcubwMJ3whrlyh0xZspBxYsfGkVabF01DSxKc6bwT4j1rdnw2fNoBEnCgs/9mEUlSM8Qf7g+Z/jJEQ== X-Gm-Message-State: F2l8CAvGCoTMdHNHiZiq3bmtx7686176AA= X-Google-Smtp-Source: AGHT+IG0zCPdPoUYEpdoxKMEE8+wDyyFoGXfjmTY75h7Nt2efttitEh/DkRYy9LMgCQE7ADQBXtfJg== X-Received: by 2002:a05:6a20:8428:b0:1a9:8152:5102 with SMTP id c40-20020a056a20842800b001a981525102mr19767578pzd.24.1713362727850; Wed, 17 Apr 2024 07:05:27 -0700 (PDT) X-Received: from [10.0.4.35] ([4.155.48.113]) by smtp.gmail.com with ESMTPSA id ka13-20020a056a00938d00b006e57247f4e5sm10601760pfb.8.2024.04.17.07.05.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 17 Apr 2024 07:05:27 -0700 (PDT) Message-ID: <2644bcd1-29c7-4cc0-9600-ae2a2eca9927@gmail.com> Date: Wed, 17 Apr 2024 07:05:27 -0700 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v1] MdeModulePkg: Fixup MAT Attributes After Splitting EFI Memory Map To: Oliver Smith-Denny , devel@edk2.groups.io, ardb@kernel.org Cc: Liming Gao References: <20240417022836.1593-1-taylor.d.beebe@gmail.com> From: "Taylor Beebe" 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 Resent-Date: Wed, 17 Apr 2024 07:05:29 -0700 Resent-From: taylor.d.beebe@gmail.com Reply-To: devel@edk2.groups.io,taylor.d.beebe@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Language: en-CA 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=20240206 header.b=yfmcVMYr; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.9 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none) On 4/17/2024 6:40 AM, Oliver Smith-Denny wrote: > Hi Ard, > > On 4/16/2024 11:38 PM, Ard Biesheuvel wrote: >> >> For entries where we lack such additional metadata, I don't think we >> can make assumptions based on the type beyond mapping data and MMIO >> regions XP. We have no idea how those EfiRuntimeServicesCode regions >> may be used, and currently, the spec permits RWX semantics for those >> if no restrictions are specified. >> The logic before the ImagePropertiesRecordLib addition was applying XP to these EfiRuntimeServicesCode regions which are not part of loaded images. I agree with you that we cannot make assumptions on how these regions are used, but it seems the logic was this way long enough for expectations to be built around the incorrect behaviour. Something to consider. >> The spec suggests that omitting an entry from the MAT is the same as >> listing it with RO|XP cleared. How RO|XP from the original entry >> should be interpreted wrt to the MAT is unspecified. So I think the >> only thing we can do at this point is preserve the entry if it has >> RO|XP set, or just drop it otherwise. > > I do agree that it is probably safer to exclude the sub-region from > the MAT entirely. However, from my reading of the spec, it is > unclear to me whether it envisions that. From section 4.6.4 of > UEFI spec 2.10: > > "The Memory Attributes Table may define multiple entries to describe > sub-regions that comprise a single entry returned by GetMemoryMap() > however the sub-regions must total to completely describe the larger > region and may not cross boundaries between entries reported by > GetMemoryMap() . If a run-time region returned in GetMemoryMap() entry > is not described within the Memory Attributes Table, this region is > assumed to not be compatible with any memory protections." > > The unclear part to me here is "the sub-regions must total to completely > describe the larger region." To me that says that in Taylor's case, > where we have a memory map descriptor with attributes set for part of > the region but not the whole region, that the spec envisions the MAT > having either the whole region split into sub-regions or not > including the MAT entry for this region. In this reading the final > sentence would say that if an entire memory map entry is not > described in the MAT, then it is assumed to be incompatible. > > A different reading would say what you are saying, that a sub-region > can be dropped from the MAT (although it is hard to justify that with > the language that says the sub-regions must total to completely > describe the larger region). What are your thoughts here? > > Aside from this, I wonder if we can be more aspirational here. These > EfiRuntimeServicesCode regions without attributes set are, if I am > understanding correctly, from loaded images.=20 These EfiRuntimeServicesCode regions without attributes set are not part of loaded image memory. I think that's what you meant but wanted to clarify. > The spec does call out > that EfiRuntimeServicesCode is explicitly for code sections of loaded > images. Can we just say outright that any memory of this type should > be RO? I understand that existing drivers may attempt to break this, > but from the core, I think this is reasonable to say, but would love > to hear thoughts. >> >> Note that the spec also mentions that the MAT must only contain >> EfiRuntimeServicesCode or EfiRuntimeServicesData entries, and it looks >> like this is not being enforced either. > > Agreed, this should be amended. The InstallMemoryAttributesTable() logic will only add the=20 EfiRuntimeServicesCode and EfiRuntimeServicesData regions to the final MAT. I'm not sure why EnforceMemoryMapAttribute() is setting XP on EfiMemoryMappedIO and EfiMemoryMappedIOPortSpace here when those entries are not included in the MAT anyways. > > 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 (#117922): https://edk2.groups.io/g/devel/message/117922 Mute This Topic: https://groups.io/mt/105570114/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-