public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* managing memory attributes in PEI
@ 2023-05-22 11:31 Ard Biesheuvel
  2023-05-22 12:06 ` Gerd Hoffmann
  2023-05-23  5:31 ` Laszlo Ersek
  0 siblings, 2 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2023-05-22 11:31 UTC (permalink / raw)
  To: edk2-devel-groups-io, Ray Ni, Jiewen Yao, Gerd Hoffmann,
	Laszlo Ersek, Taylor Beebe, Oliver Smith-Denny

Hello all,

(OVMF specific questions below - please keep reading)

As a follow-up to the discussion we had last week regarding DXE core,
I'd like to raise the issue of managing memory permissions in PEI,
including the mapping attributes of the code and data regions of DXE
core itself.

This is about good hygiene in general, but on arm64 in particular,
limiting execution permissions to memory regions that are mapped
read-only allows the MMU to be enabled in WXN mode, where all writable
regions are non-executable by default.

I have implemented a proof-of-concept of this for ArmVirtQemu and
Raspberry Pi 4 (the former using PEI and the latter PEI-less), and
this seems quite feasible in practice, but there are a few issues that
I have identified:

- PEI shadowing is currently disabled entirely - this is actually an
advantage for the [virtual] platform in question, given that shadowing
is more work for no benefit, but it is something that needs to be
addressed in the general case;
- no generic method exists to manage page table permissions.

So what I would like to propose (and what I intend to prototype) is a
PPI that abstracts this capability, and which can be used by the PEI
image loader as well as the DxeIpl to manage read-only and non-exec
permissions. Most PEIMs only have a code region anyway, so hopefully
there is some room for optimization where not all PEIMs need 4k
alignment.

That leaves one big issue, and this is related to OVMF's use of IA32
PEI with X64 DXE. This complicates the DxeIpl substantially already,
but would make this effort rather tricky as well.

So my questions are:
- do we need to retain mixed IA32 / X64 support, and if so, why? (I
think it is related to SMM emulation but I need someone to confirm
this)
- if we need to retain it, could we run PEI in long mode but with
32-bit compatibility enabled, so that we don't need two or three
incompatible sets of page tables?

In the latter case, the PPI in question would carry the same logic for
IA32 and X64 builds, and create 4-level page tables with the code
still being 32-bit.

Once we clear this up, I'm happy to look into extending my prototype
to x86 as well.

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-22 11:31 managing memory attributes in PEI Ard Biesheuvel
@ 2023-05-22 12:06 ` Gerd Hoffmann
  2023-05-22 23:20   ` Ni, Ray
  2023-05-23  5:44   ` Laszlo Ersek
  2023-05-23  5:31 ` Laszlo Ersek
  1 sibling, 2 replies; 20+ messages in thread
From: Gerd Hoffmann @ 2023-05-22 12:06 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel-groups-io, Ray Ni, Jiewen Yao, Laszlo Ersek,
	Taylor Beebe, Oliver Smith-Denny

> So my questions are:
> - do we need to retain mixed IA32 / X64 support, and if so, why? (I
> think it is related to SMM emulation but I need someone to confirm
> this)

Yes, it's SMM related.  S3 suspend in SMM-enabled builds requires
32bit PEI.

Intel seems to be working on removing the IA32 dependency, by adding
full X64 support to various places in the code base.  There have been
numerous patch sets on the list over the last months, some of them
are merged meanwhile.  As far I know the patch series addressing the
suspend problem is not yet merged (Ray, Jiewen, please correct me if
I'm wrong).

So, right now we still need that, but I expect that to change in near
future.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-22 12:06 ` Gerd Hoffmann
@ 2023-05-22 23:20   ` Ni, Ray
  2023-05-23  4:49     ` Gerd Hoffmann
  2023-05-23  5:44   ` Laszlo Ersek
  1 sibling, 1 reply; 20+ messages in thread
From: Ni, Ray @ 2023-05-22 23:20 UTC (permalink / raw)
  To: Gerd Hoffmann, Ard Biesheuvel
  Cc: edk2-devel-groups-io, Yao, Jiewen, Laszlo Ersek, Taylor Beebe,
	Oliver Smith-Denny

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

Gerd,
The S3 path has been 64bit ready.
Can you check if OVMF pei64 image can do s3?

At least internally in some real platform we tested s3 flow with 64bit PEI.

Thanks,
Ray

thanks,
ray
________________________________
From: Gerd Hoffmann <kraxel@redhat.com>
Sent: Monday, May 22, 2023 8:06:46 PM
To: Ard Biesheuvel <ardb@kernel.org>
Cc: edk2-devel-groups-io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Laszlo Ersek <lersek@redhat.com>; Taylor Beebe <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
Subject: Re: managing memory attributes in PEI

> So my questions are:
> - do we need to retain mixed IA32 / X64 support, and if so, why? (I
> think it is related to SMM emulation but I need someone to confirm
> this)

Yes, it's SMM related.  S3 suspend in SMM-enabled builds requires
32bit PEI.

Intel seems to be working on removing the IA32 dependency, by adding
full X64 support to various places in the code base.  There have been
numerous patch sets on the list over the last months, some of them
are merged meanwhile.  As far I know the patch series addressing the
suspend problem is not yet merged (Ray, Jiewen, please correct me if
I'm wrong).

So, right now we still need that, but I expect that to change in near
future.

take care,
  Gerd


[-- Attachment #2: Type: text/html, Size: 2336 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-22 23:20   ` Ni, Ray
@ 2023-05-23  4:49     ` Gerd Hoffmann
  2023-05-23  5:46       ` Laszlo Ersek
  0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2023-05-23  4:49 UTC (permalink / raw)
  To: Ni, Ray
  Cc: Ard Biesheuvel, edk2-devel-groups-io, Yao, Jiewen, Laszlo Ersek,
	Taylor Beebe, Oliver Smith-Denny

On Mon, May 22, 2023 at 11:20:16PM +0000, Ni, Ray wrote:
> Gerd,
> The S3 path has been 64bit ready.
> Can you check if OVMF pei64 image can do s3?
> 
> At least internally in some real platform we tested s3 flow with 64bit PEI.

Tested on OVMF, passed too.

Which are the commits implementing this?  I'd like the reference them in
the commit message for S3Verification() removal.

thanks,
  Gerd


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-22 11:31 managing memory attributes in PEI Ard Biesheuvel
  2023-05-22 12:06 ` Gerd Hoffmann
@ 2023-05-23  5:31 ` Laszlo Ersek
  2023-05-23  5:39   ` Ni, Ray
  1 sibling, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2023-05-23  5:31 UTC (permalink / raw)
  To: Ard Biesheuvel, edk2-devel-groups-io, Ray Ni, Jiewen Yao,
	Gerd Hoffmann, Taylor Beebe, Oliver Smith-Denny

On 5/22/23 13:31, Ard Biesheuvel wrote:
> Hello all,
> 
> (OVMF specific questions below - please keep reading)
> 
> As a follow-up to the discussion we had last week regarding DXE core,
> I'd like to raise the issue of managing memory permissions in PEI,
> including the mapping attributes of the code and data regions of DXE
> core itself.
> 
> This is about good hygiene in general, but on arm64 in particular,
> limiting execution permissions to memory regions that are mapped
> read-only allows the MMU to be enabled in WXN mode, where all writable
> regions are non-executable by default.
> 
> I have implemented a proof-of-concept of this for ArmVirtQemu and
> Raspberry Pi 4 (the former using PEI and the latter PEI-less), and
> this seems quite feasible in practice, but there are a few issues that
> I have identified:
> 
> - PEI shadowing is currently disabled entirely - this is actually an
> advantage for the [virtual] platform in question, given that shadowing
> is more work for no benefit, but it is something that needs to be
> addressed in the general case;
> - no generic method exists to manage page table permissions.
> 
> So what I would like to propose (and what I intend to prototype) is a
> PPI that abstracts this capability, and which can be used by the PEI
> image loader as well as the DxeIpl to manage read-only and non-exec
> permissions. Most PEIMs only have a code region anyway, so hopefully
> there is some room for optimization where not all PEIMs need 4k
> alignment.
> 
> That leaves one big issue, and this is related to OVMF's use of IA32
> PEI with X64 DXE. This complicates the DxeIpl substantially already,
> but would make this effort rather tricky as well.
> 
> So my questions are:
> - do we need to retain mixed IA32 / X64 support, and if so, why? (I
> think it is related to SMM emulation but I need someone to confirm
> this)

For a long time, IA32X64 had been required if you wanted (a) X64 DXE,
(b) SMM, and (c) ACPI S3 resume. The reason was that
UefiCpuPkg/Universal/Acpi/S3Resume2Pei didn't support SMM on X64, only
on IA32.

See commit 5133d1f1d297 ("OvmfPkg: replace README fine print about X64
SMM S3 with PlatformPei check", 2015-11-30).

This S3Resume2Pei limitation got lifted last year, in commit
6acf72901a2e ("UefiCpuPkg: Supporting S3 in 64bit PEI", 2022-12-19), for
<https://bugzilla.tianocore.org/show_bug.cgi?id=4195>.

Gerd tested the according removal of S3Verification() in OVMF
<https://bugzilla.tianocore.org/show_bug.cgi?id=4195#c4>, but that code
is not upstream (or downstream at that, IIUC), yet.

Once S3Verification() is removed, OVMF IA32X64 will remain useful for
exercising a particular IA32X64 combination of modules that physical
platforms use, but I reckon IA32X64 will no longer be required for
virtualization purposes per se.

Before we enabled SMM for OVMF, we had never really used IA32X64 OVMF --
SMM-less ACPI S3 resume had just worked fine with X64-only OVMF. IA32X64
only proved a great platform option to fall back to, when we realized
that on X64 OVMF, ACPI S3 resume wouldn't just seamlessly extend to SMM.

Thanks,
Laszlo

> - if we need to retain it, could we run PEI in long mode but with
> 32-bit compatibility enabled, so that we don't need two or three
> incompatible sets of page tables?
> 
> In the latter case, the PPI in question would carry the same logic for
> IA32 and X64 builds, and create 4-level page tables with the code
> still being 32-bit.
> 
> Once we clear this up, I'm happy to look into extending my prototype
> to x86 as well.
> 
> Thanks,
> Ard.
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-23  5:31 ` Laszlo Ersek
@ 2023-05-23  5:39   ` Ni, Ray
  2023-05-23  7:34     ` Laszlo Ersek
  0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ray @ 2023-05-23  5:39 UTC (permalink / raw)
  To: Laszlo Ersek, Ard Biesheuvel, edk2-devel-groups-io, Yao, Jiewen,
	Gerd Hoffmann, Taylor Beebe, Oliver Smith-Denny



> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, May 23, 2023 1:31 PM
> To: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io
> <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
> Subject: Re: managing memory attributes in PEI
> 
> On 5/22/23 13:31, Ard Biesheuvel wrote:
> > Hello all,
> >
> > (OVMF specific questions below - please keep reading)
> >
> > As a follow-up to the discussion we had last week regarding DXE core,
> > I'd like to raise the issue of managing memory permissions in PEI,
> > including the mapping attributes of the code and data regions of DXE
> > core itself.
> >
> > This is about good hygiene in general, but on arm64 in particular,
> > limiting execution permissions to memory regions that are mapped
> > read-only allows the MMU to be enabled in WXN mode, where all writable
> > regions are non-executable by default.
> >
> > I have implemented a proof-of-concept of this for ArmVirtQemu and
> > Raspberry Pi 4 (the former using PEI and the latter PEI-less), and
> > this seems quite feasible in practice, but there are a few issues that
> > I have identified:
> >
> > - PEI shadowing is currently disabled entirely - this is actually an
> > advantage for the [virtual] platform in question, given that shadowing
> > is more work for no benefit, but it is something that needs to be
> > addressed in the general case;
> > - no generic method exists to manage page table permissions.
> >
> > So what I would like to propose (and what I intend to prototype) is a
> > PPI that abstracts this capability, and which can be used by the PEI
> > image loader as well as the DxeIpl to manage read-only and non-exec
> > permissions. Most PEIMs only have a code region anyway, so hopefully
> > there is some room for optimization where not all PEIMs need 4k
> > alignment.
> >
> > That leaves one big issue, and this is related to OVMF's use of IA32
> > PEI with X64 DXE. This complicates the DxeIpl substantially already,
> > but would make this effort rather tricky as well.
> >
> > So my questions are:
> > - do we need to retain mixed IA32 / X64 support, and if so, why? (I
> > think it is related to SMM emulation but I need someone to confirm
> > this)
> 
> For a long time, IA32X64 had been required if you wanted (a) X64 DXE,
> (b) SMM, and (c) ACPI S3 resume. The reason was that
> UefiCpuPkg/Universal/Acpi/S3Resume2Pei didn't support SMM on X64, only
> on IA32.
> 
> See commit 5133d1f1d297 ("OvmfPkg: replace README fine print about X64
> SMM S3 with PlatformPei check", 2015-11-30).
> 
> This S3Resume2Pei limitation got lifted last year, in commit
> 6acf72901a2e ("UefiCpuPkg: Supporting S3 in 64bit PEI", 2022-12-19), for
> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195>.
> 
> Gerd tested the according removal of S3Verification() in OVMF
> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195#c4>, but that code
> is not upstream (or downstream at that, IIUC), yet.
> 
> Once S3Verification() is removed, OVMF IA32X64 will remain useful for
> exercising a particular IA32X64 combination of modules that physical
> platforms use, but I reckon IA32X64 will no longer be required for
> virtualization purposes per se.

Wow. I didn't realize OVMF had S3Verification() to explicitly educate users
X64 PEI + SMM doesn't support S3.:)
That will be great to remove the code today.

> 
> Before we enabled SMM for OVMF, we had never really used IA32X64 OVMF --
> SMM-less ACPI S3 resume had just worked fine with X64-only OVMF. IA32X64
> only proved a great platform option to fall back to, when we realized
> that on X64 OVMF, ACPI S3 resume wouldn't just seamlessly extend to SMM.

I don't quite understand. So, what's the conclusion of IA32X64 OVMF? Keep it? Remove it?


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-22 12:06 ` Gerd Hoffmann
  2023-05-22 23:20   ` Ni, Ray
@ 2023-05-23  5:44   ` Laszlo Ersek
  1 sibling, 0 replies; 20+ messages in thread
From: Laszlo Ersek @ 2023-05-23  5:44 UTC (permalink / raw)
  To: Gerd Hoffmann, Ard Biesheuvel
  Cc: edk2-devel-groups-io, Ray Ni, Jiewen Yao, Taylor Beebe,
	Oliver Smith-Denny

On 5/22/23 14:06, Gerd Hoffmann wrote:
>> So my questions are:
>> - do we need to retain mixed IA32 / X64 support, and if so, why? (I
>> think it is related to SMM emulation but I need someone to confirm
>> this)
> 
> Yes, it's SMM related.  S3 suspend in SMM-enabled builds requires
> 32bit PEI.
> 
> Intel seems to be working on removing the IA32 dependency, by adding
> full X64 support to various places in the code base.  There have been
> numerous patch sets on the list over the last months, some of them
> are merged meanwhile.  As far I know the patch series addressing the
> suspend problem is not yet merged (Ray, Jiewen, please correct me if
> I'm wrong).

You've forgotten about our discussion in <https://bugzilla.tianocore.org/show_bug.cgi?id=4195> :)

> So, right now we still need that, but I expect that to change in near
> future.

BTW just last evening I found out about X86-S:

- https://www.phoronix.com/news/Intel-X86-S-64-bit-Only
- https://www.intel.com/content/www/us/en/developer/articles/technical/envisioning-future-simplified-architecture.html
- https://cdrdv2.intel.com/v1/dl/getContent/776648

I'm slightly curious if those IA32->X64 "replacement" patches you mention are related to X86-S ;)

Laszlo


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-23  4:49     ` Gerd Hoffmann
@ 2023-05-23  5:46       ` Laszlo Ersek
  2023-05-23  5:50         ` [edk2-devel] " Ni, Ray
  0 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2023-05-23  5:46 UTC (permalink / raw)
  To: Gerd Hoffmann, Ni, Ray
  Cc: Ard Biesheuvel, edk2-devel-groups-io, Yao, Jiewen, Taylor Beebe,
	Oliver Smith-Denny

On 5/23/23 06:49, Gerd Hoffmann wrote:
> On Mon, May 22, 2023 at 11:20:16PM +0000, Ni, Ray wrote:
>> Gerd,
>> The S3 path has been 64bit ready.
>> Can you check if OVMF pei64 image can do s3?
>>
>> At least internally in some real platform we tested s3 flow with 64bit PEI.
> 
> Tested on OVMF, passed too.
> 
> Which are the commits implementing this?  I'd like the reference them in
> the commit message for S3Verification() removal.

commit 6acf72901a2e / bug 4195, AFAIK; there could be other related
commits since I last looked, of course


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [edk2-devel] managing memory attributes in PEI
  2023-05-23  5:46       ` Laszlo Ersek
@ 2023-05-23  5:50         ` Ni, Ray
  0 siblings, 0 replies; 20+ messages in thread
From: Ni, Ray @ 2023-05-23  5:50 UTC (permalink / raw)
  To: devel@edk2.groups.io, lersek@redhat.com, Gerd Hoffmann
  Cc: Ard Biesheuvel, Yao, Jiewen, Taylor Beebe, Oliver Smith-Denny,
	Kuo, Ted

Revision: 8bd2028f9ac391144c67eaf6eb08c3f727c32498
Author: Kuo, Ted <ted.kuo@intel.com>
Date: 12/16/2022 8:46:27 PM
Message:
MdeModulePkg: Supporting S3 in 64bit PEI

https://bugzilla.tianocore.org/show_bug.cgi?id=4195
Transfer from DXE to OS waking vector by calling SwitchStack() when
both are in the same execution mode.

Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com>
Signed-off-by: Ted Kuo <ted.kuo@intel.com>
----
Modified: MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/ScriptExecute.c
Modified: MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/X64/SetIdtEntry.c
Modified: MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c

Revision: 6acf72901a2e811a2838cafd496239a34779066a
Author: Kuo, Ted <ted.kuo@intel.com>
Date: 12/16/2022 8:46:26 PM
Message:
UefiCpuPkg: Supporting S3 in 64bit PEI

https://bugzilla.tianocore.org/show_bug.cgi?id=4195
1.Updated the GDT table in VTF0 to align with the one in S3Resume2Pei.
  By doing so can simplify the changes to enable S3 in 64bit PEI.
2.Use SwitchStack() between PEI and SMM in S3 resume path when both
  are in the same execution mode.
3.Transfer from PEI to OS waking vector by calling SwitchStack() when
  both are in the same execution mode.
4.Removed the debug assertion in S3Resume.c to support 64bit PEI.

Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Ashraf Ali S <ashraf.ali.s@intel.com>
Cc: Chinni B Duggapu <chinni.b.duggapu@intel.com>
Signed-off-by: Ted Kuo <ted.kuo@intel.com>
----
Modified: UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
Modified: UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
Modified: UefiCpuPkg/ResetVector/Vtf0/Ia16/Real16ToFlat32.asm
Modified: UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
> Sent: Tuesday, May 23, 2023 1:46 PM
> To: Gerd Hoffmann <kraxel@redhat.com>; Ni, Ray <ray.ni@intel.com>
> Cc: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io
> <devel@edk2.groups.io>; Yao, Jiewen <jiewen.yao@intel.com>; Taylor Beebe
> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
> Subject: Re: [edk2-devel] managing memory attributes in PEI
> 
> On 5/23/23 06:49, Gerd Hoffmann wrote:
> > On Mon, May 22, 2023 at 11:20:16PM +0000, Ni, Ray wrote:
> >> Gerd,
> >> The S3 path has been 64bit ready.
> >> Can you check if OVMF pei64 image can do s3?
> >>
> >> At least internally in some real platform we tested s3 flow with 64bit PEI.
> >
> > Tested on OVMF, passed too.
> >
> > Which are the commits implementing this?  I'd like the reference them in
> > the commit message for S3Verification() removal.
> 
> commit 6acf72901a2e / bug 4195, AFAIK; there could be other related
> commits since I last looked, of course
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-23  5:39   ` Ni, Ray
@ 2023-05-23  7:34     ` Laszlo Ersek
  2023-05-23  7:52       ` Ni, Ray
                         ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: Laszlo Ersek @ 2023-05-23  7:34 UTC (permalink / raw)
  To: Ni, Ray, Ard Biesheuvel, edk2-devel-groups-io, Yao, Jiewen,
	Gerd Hoffmann, Taylor Beebe, Oliver Smith-Denny

On 5/23/23 07:39, Ni, Ray wrote:
> 
> 
>> -----Original Message-----
>> From: Laszlo Ersek <lersek@redhat.com>
>> Sent: Tuesday, May 23, 2023 1:31 PM
>> To: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io
>> <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
>> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
>> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
>> Subject: Re: managing memory attributes in PEI
>>
>> On 5/22/23 13:31, Ard Biesheuvel wrote:
>>> Hello all,
>>>
>>> (OVMF specific questions below - please keep reading)
>>>
>>> As a follow-up to the discussion we had last week regarding DXE core,
>>> I'd like to raise the issue of managing memory permissions in PEI,
>>> including the mapping attributes of the code and data regions of DXE
>>> core itself.
>>>
>>> This is about good hygiene in general, but on arm64 in particular,
>>> limiting execution permissions to memory regions that are mapped
>>> read-only allows the MMU to be enabled in WXN mode, where all writable
>>> regions are non-executable by default.
>>>
>>> I have implemented a proof-of-concept of this for ArmVirtQemu and
>>> Raspberry Pi 4 (the former using PEI and the latter PEI-less), and
>>> this seems quite feasible in practice, but there are a few issues that
>>> I have identified:
>>>
>>> - PEI shadowing is currently disabled entirely - this is actually an
>>> advantage for the [virtual] platform in question, given that shadowing
>>> is more work for no benefit, but it is something that needs to be
>>> addressed in the general case;
>>> - no generic method exists to manage page table permissions.
>>>
>>> So what I would like to propose (and what I intend to prototype) is a
>>> PPI that abstracts this capability, and which can be used by the PEI
>>> image loader as well as the DxeIpl to manage read-only and non-exec
>>> permissions. Most PEIMs only have a code region anyway, so hopefully
>>> there is some room for optimization where not all PEIMs need 4k
>>> alignment.
>>>
>>> That leaves one big issue, and this is related to OVMF's use of IA32
>>> PEI with X64 DXE. This complicates the DxeIpl substantially already,
>>> but would make this effort rather tricky as well.
>>>
>>> So my questions are:
>>> - do we need to retain mixed IA32 / X64 support, and if so, why? (I
>>> think it is related to SMM emulation but I need someone to confirm
>>> this)
>>
>> For a long time, IA32X64 had been required if you wanted (a) X64 DXE,
>> (b) SMM, and (c) ACPI S3 resume. The reason was that
>> UefiCpuPkg/Universal/Acpi/S3Resume2Pei didn't support SMM on X64, only
>> on IA32.
>>
>> See commit 5133d1f1d297 ("OvmfPkg: replace README fine print about X64
>> SMM S3 with PlatformPei check", 2015-11-30).
>>
>> This S3Resume2Pei limitation got lifted last year, in commit
>> 6acf72901a2e ("UefiCpuPkg: Supporting S3 in 64bit PEI", 2022-12-19), for
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195>.
>>
>> Gerd tested the according removal of S3Verification() in OVMF
>> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195#c4>, but that code
>> is not upstream (or downstream at that, IIUC), yet.
>>
>> Once S3Verification() is removed, OVMF IA32X64 will remain useful for
>> exercising a particular IA32X64 combination of modules that physical
>> platforms use, but I reckon IA32X64 will no longer be required for
>> virtualization purposes per se.
> 
> Wow. I didn't realize OVMF had S3Verification() to explicitly educate users
> X64 PEI + SMM doesn't support S3.:)
> That will be great to remove the code today.
> 
>>
>> Before we enabled SMM for OVMF, we had never really used IA32X64 OVMF --
>> SMM-less ACPI S3 resume had just worked fine with X64-only OVMF. IA32X64
>> only proved a great platform option to fall back to, when we realized
>> that on X64 OVMF, ACPI S3 resume wouldn't just seamlessly extend to SMM.
> 
> I don't quite understand. So, what's the conclusion of IA32X64 OVMF? Keep it? Remove it?
> 

As long as edk2 (core modules) will continue supporting IA32X64 firmware
platforms, I think keeping OVMF IA32X64 is useful, minimally as a test
bed for those core modules / PCDs / boot paths. If it becomes difficult
/ costly to maintain OVMF IA32X64, then removing it might make sense at
some point, but I don't think it's time for that already.

So right now I'd just consider "shifting emphasis" from OVMF IA32X64 to
OVMF X64.

And of course this is just my opinion.

Laszlo


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-23  7:34     ` Laszlo Ersek
@ 2023-05-23  7:52       ` Ni, Ray
  2023-05-23  7:54       ` Ard Biesheuvel
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Ni, Ray @ 2023-05-23  7:52 UTC (permalink / raw)
  To: Laszlo Ersek, Ard Biesheuvel, edk2-devel-groups-io, Yao, Jiewen,
	Gerd Hoffmann, Taylor Beebe, Oliver Smith-Denny

> As long as edk2 (core modules) will continue supporting IA32X64 firmware
> platforms, I think keeping OVMF IA32X64 is useful, minimally as a test
> bed for those core modules / PCDs / boot paths. If it becomes difficult
> / costly to maintain OVMF IA32X64, then removing it might make sense at
> some point, but I don't think it's time for that already.

Agree.

> 
> So right now I'd just consider "shifting emphasis" from OVMF IA32X64 to
> OVMF X64.
> 
> And of course this is just my opinion.
> 
> Laszlo


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-23  7:34     ` Laszlo Ersek
  2023-05-23  7:52       ` Ni, Ray
@ 2023-05-23  7:54       ` Ard Biesheuvel
  2023-05-23  8:05       ` Gerd Hoffmann
  2023-05-23 14:49       ` [edk2-devel] " Michael D Kinney
  3 siblings, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2023-05-23  7:54 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Ni, Ray, edk2-devel-groups-io, Yao, Jiewen, Gerd Hoffmann,
	Taylor Beebe, Oliver Smith-Denny

On Tue, 23 May 2023 at 09:34, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 5/23/23 07:39, Ni, Ray wrote:
> >
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Tuesday, May 23, 2023 1:31 PM
> >> To: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io
> >> <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> >> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> >> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
> >> Subject: Re: managing memory attributes in PEI
> >>
> >> On 5/22/23 13:31, Ard Biesheuvel wrote:
> >>> Hello all,
> >>>
> >>> (OVMF specific questions below - please keep reading)
> >>>
> >>> As a follow-up to the discussion we had last week regarding DXE core,
> >>> I'd like to raise the issue of managing memory permissions in PEI,
> >>> including the mapping attributes of the code and data regions of DXE
> >>> core itself.
> >>>
> >>> This is about good hygiene in general, but on arm64 in particular,
> >>> limiting execution permissions to memory regions that are mapped
> >>> read-only allows the MMU to be enabled in WXN mode, where all writable
> >>> regions are non-executable by default.
> >>>
> >>> I have implemented a proof-of-concept of this for ArmVirtQemu and
> >>> Raspberry Pi 4 (the former using PEI and the latter PEI-less), and
> >>> this seems quite feasible in practice, but there are a few issues that
> >>> I have identified:
> >>>
> >>> - PEI shadowing is currently disabled entirely - this is actually an
> >>> advantage for the [virtual] platform in question, given that shadowing
> >>> is more work for no benefit, but it is something that needs to be
> >>> addressed in the general case;
> >>> - no generic method exists to manage page table permissions.
> >>>
> >>> So what I would like to propose (and what I intend to prototype) is a
> >>> PPI that abstracts this capability, and which can be used by the PEI
> >>> image loader as well as the DxeIpl to manage read-only and non-exec
> >>> permissions. Most PEIMs only have a code region anyway, so hopefully
> >>> there is some room for optimization where not all PEIMs need 4k
> >>> alignment.
> >>>
> >>> That leaves one big issue, and this is related to OVMF's use of IA32
> >>> PEI with X64 DXE. This complicates the DxeIpl substantially already,
> >>> but would make this effort rather tricky as well.
> >>>
> >>> So my questions are:
> >>> - do we need to retain mixed IA32 / X64 support, and if so, why? (I
> >>> think it is related to SMM emulation but I need someone to confirm
> >>> this)
> >>
> >> For a long time, IA32X64 had been required if you wanted (a) X64 DXE,
> >> (b) SMM, and (c) ACPI S3 resume. The reason was that
> >> UefiCpuPkg/Universal/Acpi/S3Resume2Pei didn't support SMM on X64, only
> >> on IA32.
> >>
> >> See commit 5133d1f1d297 ("OvmfPkg: replace README fine print about X64
> >> SMM S3 with PlatformPei check", 2015-11-30).
> >>
> >> This S3Resume2Pei limitation got lifted last year, in commit
> >> 6acf72901a2e ("UefiCpuPkg: Supporting S3 in 64bit PEI", 2022-12-19), for
> >> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195>.
> >>
> >> Gerd tested the according removal of S3Verification() in OVMF
> >> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195#c4>, but that code
> >> is not upstream (or downstream at that, IIUC), yet.
> >>
> >> Once S3Verification() is removed, OVMF IA32X64 will remain useful for
> >> exercising a particular IA32X64 combination of modules that physical
> >> platforms use, but I reckon IA32X64 will no longer be required for
> >> virtualization purposes per se.
> >
> > Wow. I didn't realize OVMF had S3Verification() to explicitly educate users
> > X64 PEI + SMM doesn't support S3.:)
> > That will be great to remove the code today.
> >
> >>
> >> Before we enabled SMM for OVMF, we had never really used IA32X64 OVMF --
> >> SMM-less ACPI S3 resume had just worked fine with X64-only OVMF. IA32X64
> >> only proved a great platform option to fall back to, when we realized
> >> that on X64 OVMF, ACPI S3 resume wouldn't just seamlessly extend to SMM.
> >
> > I don't quite understand. So, what's the conclusion of IA32X64 OVMF? Keep it? Remove it?
> >
>
> As long as edk2 (core modules) will continue supporting IA32X64 firmware
> platforms, I think keeping OVMF IA32X64 is useful, minimally as a test
> bed for those core modules / PCDs / boot paths. If it becomes difficult
> / costly to maintain OVMF IA32X64, then removing it might make sense at
> some point, but I don't think it's time for that already.
>
> So right now I'd just consider "shifting emphasis" from OVMF IA32X64 to
> OVMF X64.
>
> And of course this is just my opinion.
>

Thanks Laszlo. I tend to agree.

It will just mean that IA32X64 will be left behind in terms of future
enhancements. IOW, I am not going to bother catering for IA32X64 when
proposing a memory attributes PPI that manages page table permissions
for shadowed PEIMs and the DXE core. I don't think anyone would mind
that.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-23  7:34     ` Laszlo Ersek
  2023-05-23  7:52       ` Ni, Ray
  2023-05-23  7:54       ` Ard Biesheuvel
@ 2023-05-23  8:05       ` Gerd Hoffmann
  2023-05-23  8:15         ` Ard Biesheuvel
  2023-05-23 14:49       ` [edk2-devel] " Michael D Kinney
  3 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2023-05-23  8:05 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Ni, Ray, Ard Biesheuvel, edk2-devel-groups-io, Yao, Jiewen,
	Taylor Beebe, Oliver Smith-Denny

  Hi,

> >> Before we enabled SMM for OVMF, we had never really used IA32X64 OVMF --
> >> SMM-less ACPI S3 resume had just worked fine with X64-only OVMF. IA32X64
> >> only proved a great platform option to fall back to, when we realized
> >> that on X64 OVMF, ACPI S3 resume wouldn't just seamlessly extend to SMM.
> > 
> > I don't quite understand. So, what's the conclusion of IA32X64 OVMF? Keep it? Remove it?
> 
> As long as edk2 (core modules) will continue supporting IA32X64 firmware
> platforms, I think keeping OVMF IA32X64 is useful, minimally as a test
> bed for those core modules / PCDs / boot paths.

Agree.

I'll go switch downstream SMM-enabled builds from OvmfPkgIa32X64.dsc to
OvmfPkgX64.dsc ASAP, so for virtualization use cases OvmfPkgIa32X64.dsc
will not be needed any more.

But it indeed makes sense to keep OvmfPkgIa32X64.dsc for testing and CI
purposes.  So the question what the future of OvmfPkgIa32X64.dsc (and
OvmfPkgIa32.dsc) should be is is more a question of what the overall
edk2 plans for IA32 are.

take care,
  Gerd


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: managing memory attributes in PEI
  2023-05-23  8:05       ` Gerd Hoffmann
@ 2023-05-23  8:15         ` Ard Biesheuvel
  0 siblings, 0 replies; 20+ messages in thread
From: Ard Biesheuvel @ 2023-05-23  8:15 UTC (permalink / raw)
  To: Gerd Hoffmann
  Cc: Laszlo Ersek, Ni, Ray, edk2-devel-groups-io, Yao, Jiewen,
	Taylor Beebe, Oliver Smith-Denny

On Tue, 23 May 2023 at 10:05, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > >> Before we enabled SMM for OVMF, we had never really used IA32X64 OVMF --
> > >> SMM-less ACPI S3 resume had just worked fine with X64-only OVMF. IA32X64
> > >> only proved a great platform option to fall back to, when we realized
> > >> that on X64 OVMF, ACPI S3 resume wouldn't just seamlessly extend to SMM.
> > >
> > > I don't quite understand. So, what's the conclusion of IA32X64 OVMF? Keep it? Remove it?
> >
> > As long as edk2 (core modules) will continue supporting IA32X64 firmware
> > platforms, I think keeping OVMF IA32X64 is useful, minimally as a test
> > bed for those core modules / PCDs / boot paths.
>
> Agree.
>
> I'll go switch downstream SMM-enabled builds from OvmfPkgIa32X64.dsc to
> OvmfPkgX64.dsc ASAP, so for virtualization use cases OvmfPkgIa32X64.dsc
> will not be needed any more.
>
> But it indeed makes sense to keep OvmfPkgIa32X64.dsc for testing and CI
> purposes.  So the question what the future of OvmfPkgIa32X64.dsc (and
> OvmfPkgIa32.dsc) should be is is more a question of what the overall
> edk2 plans for IA32 are.
>

Indeed. Let's make it very clear that it only remains as a validation
target, so keeping it alive only makes sense if other supported
IA32X64 based platforms still exist as well.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [edk2-devel] managing memory attributes in PEI
  2023-05-23  7:34     ` Laszlo Ersek
                         ` (2 preceding siblings ...)
  2023-05-23  8:05       ` Gerd Hoffmann
@ 2023-05-23 14:49       ` Michael D Kinney
  2023-05-23 14:58         ` Ard Biesheuvel
  3 siblings, 1 reply; 20+ messages in thread
From: Michael D Kinney @ 2023-05-23 14:49 UTC (permalink / raw)
  To: devel@edk2.groups.io, lersek@redhat.com, Ni, Ray, Ard Biesheuvel,
	Yao, Jiewen, Gerd Hoffmann, Taylor Beebe, Oliver Smith-Denny
  Cc: Kinney, Michael D

Ard,

I would prefer to keep the IA32 PEI support for OVMF.  

Ray had proposed an idea to introduce a library class to help
with the DXEIPL complexity.  Perhaps that can be combines with
this effort.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
> Ersek
> Sent: Tuesday, May 23, 2023 12:35 AM
> To: Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel <ardb@kernel.org>; edk2-
> devel-groups-io <devel@edk2.groups.io>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor
> Beebe <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
> Subject: Re: [edk2-devel] managing memory attributes in PEI
> 
> On 5/23/23 07:39, Ni, Ray wrote:
> >
> >
> >> -----Original Message-----
> >> From: Laszlo Ersek <lersek@redhat.com>
> >> Sent: Tuesday, May 23, 2023 1:31 PM
> >> To: Ard Biesheuvel <ardb@kernel.org>; edk2-devel-groups-io
> >> <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> >> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor
> Beebe
> >> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
> >> Subject: Re: managing memory attributes in PEI
> >>
> >> On 5/22/23 13:31, Ard Biesheuvel wrote:
> >>> Hello all,
> >>>
> >>> (OVMF specific questions below - please keep reading)
> >>>
> >>> As a follow-up to the discussion we had last week regarding DXE core,
> >>> I'd like to raise the issue of managing memory permissions in PEI,
> >>> including the mapping attributes of the code and data regions of DXE
> >>> core itself.
> >>>
> >>> This is about good hygiene in general, but on arm64 in particular,
> >>> limiting execution permissions to memory regions that are mapped
> >>> read-only allows the MMU to be enabled in WXN mode, where all
> writable
> >>> regions are non-executable by default.
> >>>
> >>> I have implemented a proof-of-concept of this for ArmVirtQemu and
> >>> Raspberry Pi 4 (the former using PEI and the latter PEI-less), and
> >>> this seems quite feasible in practice, but there are a few issues that
> >>> I have identified:
> >>>
> >>> - PEI shadowing is currently disabled entirely - this is actually an
> >>> advantage for the [virtual] platform in question, given that shadowing
> >>> is more work for no benefit, but it is something that needs to be
> >>> addressed in the general case;
> >>> - no generic method exists to manage page table permissions.
> >>>
> >>> So what I would like to propose (and what I intend to prototype) is a
> >>> PPI that abstracts this capability, and which can be used by the PEI
> >>> image loader as well as the DxeIpl to manage read-only and non-exec
> >>> permissions. Most PEIMs only have a code region anyway, so hopefully
> >>> there is some room for optimization where not all PEIMs need 4k
> >>> alignment.
> >>>
> >>> That leaves one big issue, and this is related to OVMF's use of IA32
> >>> PEI with X64 DXE. This complicates the DxeIpl substantially already,
> >>> but would make this effort rather tricky as well.
> >>>
> >>> So my questions are:
> >>> - do we need to retain mixed IA32 / X64 support, and if so, why? (I
> >>> think it is related to SMM emulation but I need someone to confirm
> >>> this)
> >>
> >> For a long time, IA32X64 had been required if you wanted (a) X64 DXE,
> >> (b) SMM, and (c) ACPI S3 resume. The reason was that
> >> UefiCpuPkg/Universal/Acpi/S3Resume2Pei didn't support SMM on X64,
> only
> >> on IA32.
> >>
> >> See commit 5133d1f1d297 ("OvmfPkg: replace README fine print about
> X64
> >> SMM S3 with PlatformPei check", 2015-11-30).
> >>
> >> This S3Resume2Pei limitation got lifted last year, in commit
> >> 6acf72901a2e ("UefiCpuPkg: Supporting S3 in 64bit PEI", 2022-12-19), for
> >> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195>.
> >>
> >> Gerd tested the according removal of S3Verification() in OVMF
> >> <https://bugzilla.tianocore.org/show_bug.cgi?id=4195#c4>, but that
> code
> >> is not upstream (or downstream at that, IIUC), yet.
> >>
> >> Once S3Verification() is removed, OVMF IA32X64 will remain useful for
> >> exercising a particular IA32X64 combination of modules that physical
> >> platforms use, but I reckon IA32X64 will no longer be required for
> >> virtualization purposes per se.
> >
> > Wow. I didn't realize OVMF had S3Verification() to explicitly educate users
> > X64 PEI + SMM doesn't support S3.:)
> > That will be great to remove the code today.
> >
> >>
> >> Before we enabled SMM for OVMF, we had never really used IA32X64
> OVMF --
> >> SMM-less ACPI S3 resume had just worked fine with X64-only OVMF.
> IA32X64
> >> only proved a great platform option to fall back to, when we realized
> >> that on X64 OVMF, ACPI S3 resume wouldn't just seamlessly extend to
> SMM.
> >
> > I don't quite understand. So, what's the conclusion of IA32X64 OVMF?
> Keep it? Remove it?
> >
> 
> As long as edk2 (core modules) will continue supporting IA32X64 firmware
> platforms, I think keeping OVMF IA32X64 is useful, minimally as a test
> bed for those core modules / PCDs / boot paths. If it becomes difficult
> / costly to maintain OVMF IA32X64, then removing it might make sense at
> some point, but I don't think it's time for that already.
> 
> So right now I'd just consider "shifting emphasis" from OVMF IA32X64 to
> OVMF X64.
> 
> And of course this is just my opinion.
> 
> Laszlo
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [edk2-devel] managing memory attributes in PEI
  2023-05-23 14:49       ` [edk2-devel] " Michael D Kinney
@ 2023-05-23 14:58         ` Ard Biesheuvel
  2023-05-23 15:14           ` Michael D Kinney
  0 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2023-05-23 14:58 UTC (permalink / raw)
  To: Kinney, Michael D
  Cc: devel@edk2.groups.io, lersek@redhat.com, Ni, Ray, Yao, Jiewen,
	Gerd Hoffmann, Taylor Beebe, Oliver Smith-Denny

On Tue, 23 May 2023 at 16:49, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
>
> Ard,
>
> I would prefer to keep the IA32 PEI support for OVMF.
>

Sure. But does that imply that all enhancements regarding memory
protections should be introduced there as well?

If so, could you help figure out whether or not running IA32 PEI in
32-bit compatible long mode with 4 level page tables would be
feasible? That would greatly reduce the complexity, given that PEI and
DXE will be able to share page tables, and will only need one version
of the page table logic.

> Ray had proposed an idea to introduce a library class to help
> with the DXEIPL complexity.  Perhaps that can be combines with
> this effort.
>

Indeed. But the problem remains that creating a set of page tables
that are incompatible with the present execution mode is highly
specific to IA32 PEI + X64 DXE, and this impacts the code for all
other architectures.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [edk2-devel] managing memory attributes in PEI
  2023-05-23 14:58         ` Ard Biesheuvel
@ 2023-05-23 15:14           ` Michael D Kinney
  2023-05-23 15:51             ` Ard Biesheuvel
  0 siblings, 1 reply; 20+ messages in thread
From: Michael D Kinney @ 2023-05-23 15:14 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: devel@edk2.groups.io, lersek@redhat.com, Ni, Ray, Yao, Jiewen,
	Gerd Hoffmann, Taylor Beebe, Oliver Smith-Denny,
	Kinney, Michael D



> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Tuesday, May 23, 2023 7:59 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: devel@edk2.groups.io; lersek@redhat.com; Ni, Ray <ray.ni@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann
> <kraxel@redhat.com>; Taylor Beebe <t@taylorbeebe.com>; Oliver Smith-
> Denny <osd@smith-denny.com>
> Subject: Re: [edk2-devel] managing memory attributes in PEI
> 
> On Tue, 23 May 2023 at 16:49, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> >
> > Ard,
> >
> > I would prefer to keep the IA32 PEI support for OVMF.
> >
> 
> Sure. But does that imply that all enhancements regarding memory
> protections should be introduced there as well?

I would prefer to not support these protections in IA32 PEI.  Same
for IA32 DXE.  Can the proposed PPI do nothing for IA32?

> 
> If so, could you help figure out whether or not running IA32 PEI in
> 32-bit compatible long mode with 4 level page tables would be
> feasible? That would greatly reduce the complexity, given that PEI and
> DXE will be able to share page tables, and will only need one version
> of the page table logic.
> 
> > Ray had proposed an idea to introduce a library class to help
> > with the DXEIPL complexity.  Perhaps that can be combines with
> > this effort.
> >
> 
> Indeed. But the problem remains that creating a set of page tables
> that are incompatible with the present execution mode is highly
> specific to IA32 PEI + X64 DXE, and this impacts the code for all
> other architectures.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [edk2-devel] managing memory attributes in PEI
  2023-05-23 15:14           ` Michael D Kinney
@ 2023-05-23 15:51             ` Ard Biesheuvel
  2023-05-23 16:47               ` Michael D Kinney
  0 siblings, 1 reply; 20+ messages in thread
From: Ard Biesheuvel @ 2023-05-23 15:51 UTC (permalink / raw)
  To: devel, michael.d.kinney
  Cc: lersek@redhat.com, Ni, Ray, Yao, Jiewen, Gerd Hoffmann,
	Taylor Beebe, Oliver Smith-Denny

On Tue, 23 May 2023 at 17:15, Michael D Kinney
<michael.d.kinney@intel.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ardb@kernel.org>
> > Sent: Tuesday, May 23, 2023 7:59 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: devel@edk2.groups.io; lersek@redhat.com; Ni, Ray <ray.ni@intel.com>;
> > Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann
> > <kraxel@redhat.com>; Taylor Beebe <t@taylorbeebe.com>; Oliver Smith-
> > Denny <osd@smith-denny.com>
> > Subject: Re: [edk2-devel] managing memory attributes in PEI
> >
> > On Tue, 23 May 2023 at 16:49, Kinney, Michael D
> > <michael.d.kinney@intel.com> wrote:
> > >
> > > Ard,
> > >
> > > I would prefer to keep the IA32 PEI support for OVMF.
> > >
> >
> > Sure. But does that imply that all enhancements regarding memory
> > protections should be introduced there as well?
>
> I would prefer to not support these protections in IA32 PEI.  Same
> for IA32 DXE.  Can the proposed PPI do nothing for IA32?
>

Absolutely. I was just trying to narrow down whether your 'keeping
IA32' meant just keeping it in working order, or have it keep up with
future enhancements.

My intent is to implement an optional PPI that will be used by the PEI
image loader to map PE code and data sections with the appropriate
permissions if they are suitably aligned. Only the DXE core would
generally fit this description, but there is no reason to disallow
this for shadowed PEIMs that happen to be built as PE32 binaries with
4k section alignment (although I'm not convinced of the value add
there)

If the PPI is not exposed (for any reason) things should just keep
working as they do today.

Given that OVMF no longer functionally depends on IA32 PEI, we simply
won't bother to implement the PPI at all for IA32.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [edk2-devel] managing memory attributes in PEI
  2023-05-23 15:51             ` Ard Biesheuvel
@ 2023-05-23 16:47               ` Michael D Kinney
  2023-05-24  2:54                 ` Ni, Ray
  0 siblings, 1 reply; 20+ messages in thread
From: Michael D Kinney @ 2023-05-23 16:47 UTC (permalink / raw)
  To: devel@edk2.groups.io, ardb@kernel.org
  Cc: lersek@redhat.com, Ni, Ray, Yao, Jiewen, Gerd Hoffmann,
	Taylor Beebe, Oliver Smith-Denny, Kinney, Michael D

Hi Ard,

Thanks.  I agree with your plan.

In the future, if we think there is value in enabling paging in 32-bit PEI, we could add the PPI at that time.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Tuesday, May 23, 2023 8:51 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: lersek@redhat.com; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor
> Beebe <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
> Subject: Re: [edk2-devel] managing memory attributes in PEI
> 
> On Tue, 23 May 2023 at 17:15, Michael D Kinney
> <michael.d.kinney@intel.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Ard Biesheuvel <ardb@kernel.org>
> > > Sent: Tuesday, May 23, 2023 7:59 AM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: devel@edk2.groups.io; lersek@redhat.com; Ni, Ray
> <ray.ni@intel.com>;
> > > Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann
> > > <kraxel@redhat.com>; Taylor Beebe <t@taylorbeebe.com>; Oliver
> Smith-
> > > Denny <osd@smith-denny.com>
> > > Subject: Re: [edk2-devel] managing memory attributes in PEI
> > >
> > > On Tue, 23 May 2023 at 16:49, Kinney, Michael D
> > > <michael.d.kinney@intel.com> wrote:
> > > >
> > > > Ard,
> > > >
> > > > I would prefer to keep the IA32 PEI support for OVMF.
> > > >
> > >
> > > Sure. But does that imply that all enhancements regarding memory
> > > protections should be introduced there as well?
> >
> > I would prefer to not support these protections in IA32 PEI.  Same
> > for IA32 DXE.  Can the proposed PPI do nothing for IA32?
> >
> 
> Absolutely. I was just trying to narrow down whether your 'keeping
> IA32' meant just keeping it in working order, or have it keep up with
> future enhancements.
> 
> My intent is to implement an optional PPI that will be used by the PEI
> image loader to map PE code and data sections with the appropriate
> permissions if they are suitably aligned. Only the DXE core would
> generally fit this description, but there is no reason to disallow
> this for shadowed PEIMs that happen to be built as PE32 binaries with
> 4k section alignment (although I'm not convinced of the value add
> there)
> 
> If the PPI is not exposed (for any reason) things should just keep
> working as they do today.
> 
> Given that OVMF no longer functionally depends on IA32 PEI, we simply
> won't bother to implement the PPI at all for IA32.
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [edk2-devel] managing memory attributes in PEI
  2023-05-23 16:47               ` Michael D Kinney
@ 2023-05-24  2:54                 ` Ni, Ray
  0 siblings, 0 replies; 20+ messages in thread
From: Ni, Ray @ 2023-05-24  2:54 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, ardb@kernel.org
  Cc: lersek@redhat.com, Yao, Jiewen, Gerd Hoffmann, Taylor Beebe,
	Oliver Smith-Denny

Today X86 CpuMp PEIM enables the paging in 32bit and 64bit mode for protection of:
1. Stack overflow
2. Avoid accessing SPI flash after NEM tear down

We could either producing a 32bit PPI for above needs (DxeIpl should not call this PPI for DxeCore protection in mixed 32PEI+64DXE env) or separating above protection logic into separate 32bit/64bit functions. For the latter case, 32bit function could use existing logic, 64bit function could use the new PPI.

Thanks,
Ray

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, May 24, 2023 12:48 AM
> To: devel@edk2.groups.io; ardb@kernel.org
> Cc: lersek@redhat.com; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor Beebe
> <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>
> Subject: RE: [edk2-devel] managing memory attributes in PEI
> 
> Hi Ard,
> 
> Thanks.  I agree with your plan.
> 
> In the future, if we think there is value in enabling paging in 32-bit PEI, we
> could add the PPI at that time.
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > Biesheuvel
> > Sent: Tuesday, May 23, 2023 8:51 AM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: lersek@redhat.com; Ni, Ray <ray.ni@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Taylor
> > Beebe <t@taylorbeebe.com>; Oliver Smith-Denny <osd@smith-denny.com>
> > Subject: Re: [edk2-devel] managing memory attributes in PEI
> >
> > On Tue, 23 May 2023 at 17:15, Michael D Kinney
> > <michael.d.kinney@intel.com> wrote:
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel <ardb@kernel.org>
> > > > Sent: Tuesday, May 23, 2023 7:59 AM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Cc: devel@edk2.groups.io; lersek@redhat.com; Ni, Ray
> > <ray.ni@intel.com>;
> > > > Yao, Jiewen <jiewen.yao@intel.com>; Gerd Hoffmann
> > > > <kraxel@redhat.com>; Taylor Beebe <t@taylorbeebe.com>; Oliver
> > Smith-
> > > > Denny <osd@smith-denny.com>
> > > > Subject: Re: [edk2-devel] managing memory attributes in PEI
> > > >
> > > > On Tue, 23 May 2023 at 16:49, Kinney, Michael D
> > > > <michael.d.kinney@intel.com> wrote:
> > > > >
> > > > > Ard,
> > > > >
> > > > > I would prefer to keep the IA32 PEI support for OVMF.
> > > > >
> > > >
> > > > Sure. But does that imply that all enhancements regarding memory
> > > > protections should be introduced there as well?
> > >
> > > I would prefer to not support these protections in IA32 PEI.  Same
> > > for IA32 DXE.  Can the proposed PPI do nothing for IA32?
> > >
> >
> > Absolutely. I was just trying to narrow down whether your 'keeping
> > IA32' meant just keeping it in working order, or have it keep up with
> > future enhancements.
> >
> > My intent is to implement an optional PPI that will be used by the PEI
> > image loader to map PE code and data sections with the appropriate
> > permissions if they are suitably aligned. Only the DXE core would
> > generally fit this description, but there is no reason to disallow
> > this for shadowed PEIMs that happen to be built as PE32 binaries with
> > 4k section alignment (although I'm not convinced of the value add
> > there)
> >
> > If the PPI is not exposed (for any reason) things should just keep
> > working as they do today.
> >
> > Given that OVMF no longer functionally depends on IA32 PEI, we simply
> > won't bother to implement the PPI at all for IA32.
> >
> >
> > 
> >


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-05-24  2:55 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-22 11:31 managing memory attributes in PEI Ard Biesheuvel
2023-05-22 12:06 ` Gerd Hoffmann
2023-05-22 23:20   ` Ni, Ray
2023-05-23  4:49     ` Gerd Hoffmann
2023-05-23  5:46       ` Laszlo Ersek
2023-05-23  5:50         ` [edk2-devel] " Ni, Ray
2023-05-23  5:44   ` Laszlo Ersek
2023-05-23  5:31 ` Laszlo Ersek
2023-05-23  5:39   ` Ni, Ray
2023-05-23  7:34     ` Laszlo Ersek
2023-05-23  7:52       ` Ni, Ray
2023-05-23  7:54       ` Ard Biesheuvel
2023-05-23  8:05       ` Gerd Hoffmann
2023-05-23  8:15         ` Ard Biesheuvel
2023-05-23 14:49       ` [edk2-devel] " Michael D Kinney
2023-05-23 14:58         ` Ard Biesheuvel
2023-05-23 15:14           ` Michael D Kinney
2023-05-23 15:51             ` Ard Biesheuvel
2023-05-23 16:47               ` Michael D Kinney
2023-05-24  2:54                 ` Ni, Ray

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox