public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Taylor Beebe" <t@taylorbeebe.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: devel@edk2.groups.io, Jian J Wang <jian.j.wang@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	Dandan Bi <dandan.bi@intel.com>,
	Ard Biesheuvel <ardb+tianocore@kernel.org>,
	Jiewen Yao <jiewen.yao@intel.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Leif Lindholm <quic_llindhol@quicinc.com>,
	Sami Mujawar <sami.mujawar@arm.com>,
	Andrew Fish <afish@apple.com>, Ray Ni <ray.ni@intel.com>,
	Eric Dong <eric.dong@intel.com>,
	Rahul Kumar <rahul1.kumar@intel.com>,
	Guo Dong <guo.dong@intel.com>,
	Sean Rhodes <sean@starlabs.systems>,
	James Lu <james.lu@intel.com>, Gua Guo <gua.guo@intel.com>
Subject: Re: [PATCH 00/14] Implement Dynamic Memory Protections
Date: Wed, 12 Jul 2023 14:54:01 -0700	[thread overview]
Message-ID: <7cc0ebd1-c847-d98e-68fc-5cb46d71969e@taylorbeebe.com> (raw)
In-Reply-To: <i5cudvjuculyff7dprayxkt2ed6hjwjmpcrheishbp5mu2oqgx@yjywmmvhng2p>



On 7/12/2023 3:05 AM, Gerd Hoffmann wrote:
> On Tue, Jul 11, 2023 at 04:52:37PM -0700, Taylor Beebe wrote:
>> In the past, memory protection settings were configured via FixedAtBuild PCDs,
>> which resulted in a build-time configuration of memory mitigations. This
>> approach limited the flexibility of applying mitigations to the
>> system and made it difficult to update or adjust the settings post-build.
> 
> Can we have both?
> 
> Being able to adjust settings at runtime is great.  But being able to
> set them at compile time on the command line (via build --pcd), without
> patching code, is very useful too.
> 
> I'd suggest to keep the PCDs, create a profile from PCD settings and use
> that profile by default.  At least for the transition phase and while we
> don't have good support (yet) to actually manage profiles.
> 

Hey, Gerd.

The idea to keep PCDs around as another method of configuring 
protections is good, but I don't think there would be a way to tell if a 
zero-ed PCD value was an explicit setting or just the default without 
adding another PCD to indicate which value should be used. I think if 
the HOB is produced by the platform those settings should be used by 
default. Is that what you're envisioning as well?

The flow in this case would be: DxeIpl before handoff will search for 
the memory protection HOB entry. If it exists, do nothing. If the HOB 
was not produced, create a HOB entry using the PCD values.

I suppose we could also do some sort of hybrid where the logic always 
uses the PCD values if they are non-zero, but that may be confusing for 
platform developers.

> Speaking of profile management: What is the longer-term vision here?
> 
> Have a configuration form in the uefi firmware setup (aka UiApp)?
> 
> Try adapt settings automatically, for example pick a less strict profile
> in case an old/broken bootloader triggers a page fault due to using the
> wrong memory type for allocations?
> 

Overall, the idea is to empower platform developers with more 
configuration and compatibility options to encourage the use of memory 
protections in debug and production scenarios.

On Surface products, we've made certain memory protections a necessary 
feature of the trusted boot path with the ability to fall back to an 
unverified boot path if a page fault occurs. We also added a 
configuration feature to only allow the trusted boot path which can be 
managed via enterprise policy or through the UEFI menu.

There are lots of ways OEMs might want to configure their platform 
security and I think it's an open question what sorts of profile 
management tools would be useful to add to EDK2.

> For virtual machine firmware it a good idea to allow picking up the
> profile configuration from the host.  For qemu that can use fw_cfg,
> similar to the PcdSetNxForStack option we have today.

I don't have much experience using the fw_cfg so I'd need to look into 
the details, but would it make sense to expand the options which can be 
passed via fw_cfg to be the gamut of memory protection configuration 
settings? I can add it to the v2 if you think so.

>> This patch series also increases the memory protection level for OvmfPkg and
>> ArmVirtPkg.
> 
> Not a good idea, especially not using the 'debug' profile (which turns
> on all guard bits) because that slows down firmware boot alot.
> 

I haven't done performance testing, but I don't notice much slowdown. 
Isn't development the primary use case for these virtual platforms? Is 
there another profile you think would work better?

Thanks for your feedback :)

> take care,
>    Gerd
> 

  reply	other threads:[~2023-07-12 21:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-11 23:52 [PATCH 00/14] Implement Dynamic Memory Protections Taylor Beebe
2023-07-11 23:52 ` [PATCH 01/14] MdeModulePkg: Add DXE and MM Memory Protection Settings Definitions Taylor Beebe
2023-07-11 23:52 ` [PATCH 02/14] MdeModulePkg: Add MemoryProtectionHobLib Definitions and NULL Libs Taylor Beebe
2023-07-11 23:52 ` [PATCH 03/14] MdeModulePkg: Add Phase-Specific MemoryProtectionHobLib Implementations Taylor Beebe
2023-07-11 23:52 ` [PATCH 04/14] OvmfPkg: Create the memory protection settings HOB Taylor Beebe
2023-07-11 23:52 ` [PATCH 05/14] ArmVirtPkg: Create " Taylor Beebe
2023-07-11 23:52 ` [PATCH 06/14] ArmPkg: Update to use memory protection HOB Taylor Beebe
2023-07-11 23:52 ` [PATCH 07/14] EmulatorPkg: " Taylor Beebe
2023-07-11 23:52 ` [PATCH 08/14] MdeModulePkg: " Taylor Beebe
2023-07-11 23:52 ` [PATCH 09/14] OvmfPkg: " Taylor Beebe
2023-07-11 23:52 ` [PATCH 10/14] UefiCpuPkg: " Taylor Beebe
2023-07-11 23:52 ` [PATCH 11/14] UefiPayloadPkg: " Taylor Beebe
2023-07-11 23:52 ` [PATCH 12/14] OvmfPkg: Delete Memory Protection PCDs Taylor Beebe
2023-07-11 23:52 ` [PATCH 14/14] MdeModulePkg: " Taylor Beebe
2023-07-11 23:52 ` Taylor Beebe
     [not found] ` <1770F551E2E594C0.16575@groups.io>
2023-07-12  0:01   ` [edk2-devel] " Taylor Beebe
2023-07-12 10:05 ` [PATCH 00/14] Implement Dynamic Memory Protections Gerd Hoffmann
2023-07-12 21:54   ` Taylor Beebe [this message]
2023-07-17 11:14     ` [edk2-devel] " Gerd Hoffmann
2023-07-17 16:15 ` Pedro Falcato
2023-07-17 16:25   ` Ard Biesheuvel
2023-07-17 16:49     ` Pedro Falcato
2023-07-18  2:45       ` Taylor Beebe
2023-07-18  6:11     ` Ni, Ray

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7cc0ebd1-c847-d98e-68fc-5cb46d71969e@taylorbeebe.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox