public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gerd Hoffmann" <kraxel@redhat.com>
To: Taylor Beebe <t@taylorbeebe.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: [edk2-devel] [PATCH 00/14] Implement Dynamic Memory Protections
Date: Mon, 17 Jul 2023 13:14:54 +0200	[thread overview]
Message-ID: <ejpwmtswftoqd3zf6bxl4oyk6agf77qhyczk2cz3hr7h6pu2qu@xvrgtwodrecx> (raw)
In-Reply-To: <7cc0ebd1-c847-d98e-68fc-5cb46d71969e@taylorbeebe.com>

  Hi,

> > 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?

See below, I'll come back to that in a moment.

> > Speaking of profile management: What is the longer-term vision here?

> 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.

I think it makes sense to have the concept of named profiles in edk2.
Right now there are a bunch of #defines with profiles, I think it would
be nice to have names and descriptions attached to them.  Could be as
simple as an array in the library:

struct {
   CHAR16 *Name;
   CHAR16 *Description;
   DXE_MEMORY_PROTECTION_SETTINGS Settings;
} MemoryProtectionProfiles[] = {
   {
      .Name        = "debug",
      .Description = "development profile",
      .Settings    = DXE_MEMORY_PROTECTION_SETTINGS_DEBUG,
   },{
      /* ... */
   }
}

Platforms could just loop over the list and add the profiles to the
platform configuration menu in uefi firmware settings.  Maybe it makes
sense to have a MemoryProtectionConfigDxe instead so platforms can
easily share the Hii code for that.

One of the profiles could be created from PCDs:

    {
	.Name               = "pcd",
	.Description        = "legacy PCD based settings",
	.Settings.PoolGuard = PcdGetU32(PcdHeapGuardPoolType),
        /* ... */
    }

> > 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 think being able to select a named profile is good enough, for example
this way:

qemu -fw_cfg name=opt/org.tianocore/MemoryProtectionProfile,string=debug

> > > 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.

Page guard isn't much of a problem.

Heap guard has significant overhead in both performance and memory
usage.

> Isn't development the primary use case for these virtual platforms?

Development is surely one important use case   Linux-based VM hosting
often runs OVMF in production though.

> Is there
> another profile you think would work better?

"pcd" (or "production") would IMHO be a better default.

thanks & take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106963): https://edk2.groups.io/g/devel/message/106963
Mute This Topic: https://groups.io/mt/100090629/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  reply	other threads:[~2023-07-17 11:15 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
2023-07-17 11:14     ` Gerd Hoffmann [this message]
2023-07-17 16:15 ` [edk2-devel] " 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=ejpwmtswftoqd3zf6bxl4oyk6agf77qhyczk2cz3hr7h6pu2qu@xvrgtwodrecx \
    --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