Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Avoid printing hard coded timeout value

2023-07-17 Thread Sunil V L
On Mon, Jul 17, 2023 at 08:47:55PM +, Andrei Warkentin wrote:
> Looks good to me.
> 
> Minor nit: is that really a %d (signed) and not a %u?
> 
Thanks!, Andrei. Yes, let me fix it while merging.

Thanks,
Sunil


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106978): https://edk2.groups.io/g/devel/message/106978
Mute This Topic: https://groups.io/mt/100198952/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

2023-07-17 Thread Sunil V L
On Mon, Jul 17, 2023 at 07:03:28PM +0100, Pedro Falcato wrote:
> On Mon, Jul 17, 2023 at 5:59 PM Sunil V L  wrote:
> >
> > OpenSBI now marks PMP regions with "no-map" attribute.
> > So, remove the workaround and add the ReservedMemory only
> > when no-map is set so that it follows DT spec.
> 
> Isn't there a concern for supporting older OpenSBI versions? Is there
> no guarantee that it will work?
> 
> This change looks like it should break older OpenSBI versions.
> 
Hi Pedro,

You are right, but it is OK because this workaround was mainly required
for RISC-V ACPI. Currently, ACPI is not fully enabled for RISC-V. So,
there are no practical users who get affected.

Thanks,
Sunl


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106977): https://edk2.groups.io/g/devel/message/106977
Mute This Topic: https://groups.io/mt/100198972/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 00/14] Implement Dynamic Memory Protections

2023-07-17 Thread Taylor Beebe




On 7/17/23 9:49 AM, Pedro Falcato wrote:

On Mon, Jul 17, 2023 at 5:26 PM Ard Biesheuvel  wrote:


On Mon, 17 Jul 2023 at 18:15, Pedro Falcato  wrote:


On Wed, Jul 12, 2023 at 12:53 AM 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.


How do you mitigate the possibility of an attack overwriting the
dynamic configuration data (the HOBs)?
It seems most dangerous to me to publish this sort of
security-sensitive configuration knobs dynamically such that an
attacker can change them.



That is a very good point. One of the things I have on my TODO list
for the memory attributes PEI work is to remap HOB memory read-only
before entering DXE. They are conceptually read-only anyway when PEI
completes, so they should never be modified afterwards.


I agree, but it also seems that this patch set needs some sort of
__ro_after_init capabilities. For example, in
https://github.com/tianocore/edk2/pull/4566/commits/e485459b6efb1e49591c6f3011d9da14746c52bc#diff-02c0ef19d024b43162043efdd9ed95e0eef1653bcb5bef1e2f2b77587aee2622R101
(DxeMemoryProtectionHobLibConstructor), a copy of this same HOB is
made onto .data, while it should be RO-protected as well.
With both the HOB list and this sort of __ro_after_init protected, the
only remaining exploits would be to DMA over those pages (addressed by
IOMMU, not in this scope), to remap those pages (requires ring 0
access, therefore irrelevant) or to toggle some sort of WP-like bit
(CR0.WP, other archs may have equivalents), which already bypasses
most of the memory protections and therefore isn't all that concerning
to me.



Thank you both for you time and feedback.

Ard, do you think it's sufficient to use the Memory Attribute PPI to
mark HOB list memory as RO before handoff, or should the HOB list
memory be marked RO upon memory discovery and the PEI core
HOB logic be updated to manipulate protection attributes
with the PPI as it manipulates the HOB list?

-Taylor


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106976): https://edk2.groups.io/g/devel/message/106976
Mute This Topic: https://groups.io/mt/100090629/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, July 18, 2023 #cal-reminder

2023-07-17 Thread Group Notification
*Reminder: TianoCore Bug Triage - APAC / NAMO*

*When:*
Tuesday, July 18, 2023
6:30pm to 7:30pm
(UTC-07:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d

*Organizer:* Liming Gao gaolim...@byosoft.com.cn ( 
gaolim...@byosoft.com.cn?subject=Re:%20Event:%20TianoCore%20Bug%20Triage%20-%20APAC%20%2F%20NAMO
 )

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1949447 )

*Description:*

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao



Microsoft Teams meeting

*Join on your computer or mobile app*

Click here to join the meeting ( 
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTk1YzJhN2UtOGQwNi00NjY4LWEwMTktY2JiODRlYTY1NmY0%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%226e4ce4c4-1242-431b-9a51-92cd01a5df3c%22%7d
 )

*Join with a video conferencing device*

te...@conf.intel.com

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions ( 
https://conf.intel.com/teams/?conf=1160620940=teams=conf.intel.com=test_call
 )

*Or call in (audio only)*

+1 916-245-6934,,77463821# ( tel:+19162456934,,77463821# ) United States, 
Sacramento

Phone Conference ID: 774 638 21#

Find a local number ( 
https://dialin.teams.microsoft.com/d195d438-2daa-420e-b9ea-da26f9d1d6d5?id=77463821
 ) | Reset PIN ( https://mysettings.lync.com/pstnconferencing )

Learn More ( https://aka.ms/JoinTeamsMeeting ) | Meeting options ( 
https://teams.microsoft.com/meetingOptions/?organizerId=b286b53a-1218-4db3-bfc9-3d4c5aa7669e=46c98d88-e344-4ed4-8496-4ed7712e255d=19_meeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh@thread.v2=0=en-US
 )


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106975): https://edk2.groups.io/g/devel/message/106975
Mute This Topic: https://groups.io/mt/100207985/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] Now: Tools, CI, Code base construction meeting series - Monday, July 17, 2023 #cal-notice

2023-07-17 Thread Group Notification
*Tools, CI, Code base construction meeting series*

*When:*
Monday, July 17, 2023
4:30pm to 5:30pm
(UTC-07:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_ZDI2ZDg4NmMtMjI1My00MzI5LWFmYjAtMGQyNjUzNTBjZGYw%40thread.v2/0?context=%7b%22Tid%22%3a%2272f988bf-86f1-41af-91ab-2d7cd011db47%22%2c%22Oid%22%3a%2223af6561-6e1c-450d-b917-d9d674eb3cb6%22%7d

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1949451 )

*Description:*

TianoCore community,

Microsoft and Intel will be hosting a series of open meetings to discuss build, 
CI, tools, and other related topics. If you are interested, have ideas/opinions 
please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft 
Teams.

MS Teams Link in following discussion: * 
https://github.com/tianocore/edk2/discussions/2614

Anyone is welcome to join.

* tianocore/edk2: EDK II (github.com)
* tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP module 
(github.com) https://github.com/tianocore/edk2-basetools
* tianocore/edk2-pytool-extensions: Extensions to the edk2 build system 
allowing for a more robust and plugin based build system and tool execution 
environment (github.com) https://github.com/tianocore/edk2-pytool-extensions
* tianocore/edk2-pytool-library: Python library package that supports UEFI 
development (github.com) https://github.com/tianocore/edk2-pytool-library

MS Teams Browser Clients * 
https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106974): https://edk2.groups.io/g/devel/message/106974
Mute This Topic: https://groups.io/mt/100206189/21656
Mute #cal-notice:https://edk2.groups.io/g/devel/mutehashtag/cal-notice
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] Event: Tools, CI, Code base construction meeting series - Monday, July 17, 2023 #cal-reminder

2023-07-17 Thread Group Notification
*Reminder: Tools, CI, Code base construction meeting series*

*When:*
Monday, July 17, 2023
4:30pm to 5:30pm
(UTC-07:00) America/Los Angeles

*Where:*
https://teams.microsoft.com/l/meetup-join/19%3ameeting_ZDI2ZDg4NmMtMjI1My00MzI5LWFmYjAtMGQyNjUzNTBjZGYw%40thread.v2/0?context=%7b%22Tid%22%3a%2272f988bf-86f1-41af-91ab-2d7cd011db47%22%2c%22Oid%22%3a%2223af6561-6e1c-450d-b917-d9d674eb3cb6%22%7d

View Event ( https://edk2.groups.io/g/devel/viewevent?eventid=1949451 )

*Description:*

TianoCore community,

Microsoft and Intel will be hosting a series of open meetings to discuss build, 
CI, tools, and other related topics. If you are interested, have ideas/opinions 
please join us. These meetings will be Monday 4:30pm Pacific Time on Microsoft 
Teams.

MS Teams Link in following discussion: * 
https://github.com/tianocore/edk2/discussions/2614

Anyone is welcome to join.

* tianocore/edk2: EDK II (github.com)
* tianocore/edk2-basetools: EDK II BaseTools Python tools as a PIP module 
(github.com) https://github.com/tianocore/edk2-basetools
* tianocore/edk2-pytool-extensions: Extensions to the edk2 build system 
allowing for a more robust and plugin based build system and tool execution 
environment (github.com) https://github.com/tianocore/edk2-pytool-extensions
* tianocore/edk2-pytool-library: Python library package that supports UEFI 
development (github.com) https://github.com/tianocore/edk2-pytool-library

MS Teams Browser Clients * 
https://docs.microsoft.com/en-us/microsoftteams/get-clients?tabs=Windows#browser-client


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106973): https://edk2.groups.io/g/devel/message/106973
Mute This Topic: https://groups.io/mt/100185549/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Avoid printing hard coded timeout value

2023-07-17 Thread Andrei Warkentin
Looks good to me.

Minor nit: is that really a %d (signed) and not a %u?

Reviewed-by: Andrei Warkentin 

> -Original Message-
> From: Sunil V L 
> Sent: Monday, July 17, 2023 11:59 AM
> To: devel@edk2.groups.io
> Cc: Sunil V L ; Ard Biesheuvel
> ; Yao, Jiewen ; Justen,
> Jordan L ; Gerd Hoffmann ;
> Warkentin, Andrei 
> Subject: [PATCH 1/1] OvmfPkg/RiscVVirt: Avoid printing hard coded timeout 
> value
> 
> Print the timeout value set in the PCD variable instead of hard coded 10 
> seconds.
> 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Gerd Hoffmann 
> Cc: Andrei Warkentin 
> 
> Signed-off-by: Sunil V L 
> ---
>  OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c
> b/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c
> index be4316f320f9..964c35ee5328 100644
> --- a/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -936,7 +936,7 @@ PlatformBootManagerAfterConsole (
>);
>}
> 
> -  Print (L"Press ESCAPE within 10 seconds for boot options ");
> +  Print (L"Press ESCAPE within %d seconds for boot options ", PcdGet16
> + (PcdPlatformBootTimeOut));
>//
>// Process QEMU's -kernel command line option. The kernel booted this way
>// will receive ACPI tables: in PlatformBootManagerBeforeConsole(), we
> --
> 2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106972): https://edk2.groups.io/g/devel/message/106972
Mute This Topic: https://groups.io/mt/100198952/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH V1] OvmfPkg/PeilessStartupLib: Updated with PcdSecureBootSupported

2023-07-17 Thread Erdem Aktas via groups.io
Reviewed-by: Erdem Aktas 


On Sun, Jul 16, 2023 at 6:55 PM Yao, Jiewen  wrote:
>
> Reviewed-by: Jiewen Yao 
>
> > -Original Message-
> > From: devel@edk2.groups.io  On Behalf Of Gerd
> > Hoffmann
> > Sent: Monday, July 10, 2023 6:36 PM
> > To: devel@edk2.groups.io; Sun, CepingX 
> > Cc: Aktas, Erdem ; James Bottomley
> > ; Yao, Jiewen ; Xu, Min M
> > ; Tom Lendacky ; Michael
> > Roth 
> > Subject: Re: [edk2-devel] [PATCH V1] OvmfPkg/PeilessStartupLib: Updated with
> > PcdSecureBootSupported
> >
> > On Mon, Jul 10, 2023 at 06:05:39PM +0800, sunceping wrote:
> > > SECURE_BOOT_FEATURE_ENABLED was dropped by the commit(92da8a154f),
> > but the
> > > PeilessStartupLib was not updated with PcdSecureBootSupported, that made
> > > SecureBoot no longer work in IntelTdxX64.
> > >
> > > Fix this by replacing SECURE_BOOT_FEATURE_ENABLED with
> > > PcdSecureBootSupported in PeilessStartupLib.
> > >
> > > Cc: Erdem Aktas 
> > > Cc: James Bottomley 
> > > Cc: Jiewen Yao 
> > > Cc: Gerd Hoffmann 
> > > Cc: Min Xu 
> > > Cc: Tom Lendacky 
> > > Cc: Michael Roth 
> > > Signed-off-by: Ceping Sun 
> >
> > Acked-by: Gerd Hoffmann 
> >
> >
> >
> > 
> >
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106971): https://edk2.groups.io/g/devel/message/106971
Mute This Topic: https://groups.io/mt/100054785/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

2023-07-17 Thread Pedro Falcato
On Mon, Jul 17, 2023 at 5:59 PM Sunil V L  wrote:
>
> OpenSBI now marks PMP regions with "no-map" attribute.
> So, remove the workaround and add the ReservedMemory only
> when no-map is set so that it follows DT spec.

Isn't there a concern for supporting older OpenSBI versions? Is there
no guarantee that it will work?

This change looks like it should break older OpenSBI versions.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106970): https://edk2.groups.io/g/devel/message/106970
Mute This Topic: https://groups.io/mt/100198972/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Check "no-map" and mark EfiReservedMemoryType

2023-07-17 Thread Sunil V L
OpenSBI now marks PMP regions with "no-map" attribute.
So, remove the workaround and add the ReservedMemory only
when no-map is set so that it follows DT spec.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Andrei Warkentin 

Signed-off-by: Sunil V L 
---
 OvmfPkg/RiscVVirt/Sec/Memory.c | 34 +-
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/Sec/Memory.c b/OvmfPkg/RiscVVirt/Sec/Memory.c
index aad71ee5dcbb..164b33cd258f 100644
--- a/OvmfPkg/RiscVVirt/Sec/Memory.c
+++ b/OvmfPkg/RiscVVirt/Sec/Memory.c
@@ -141,22 +141,6 @@ GetNumCells (
 
 /** Mark reserved memory ranges in the EFI memory map
 
-  The M-mode firmware ranges should not be used by the
-  EDK2/OS. These ranges are passed via device tree using reserved
-  memory nodes. Parse the DT and mark those ranges as of
-  type EfiReservedMemoryType.
-
-  NOTE: Device Tree spec section 3.5.4 says reserved memory regions
-  without no-map property should be installed as EfiBootServicesData.
-  As per UEFI spec, memory of type EfiBootServicesData can be used
-  by the OS after ExitBootServices().
-  This is not an issue for DT since OS can parse the DT also along
-  with EFI memory map and avoid using these ranges. But with ACPI,
-  there is no such mechanisms possible.
-  Since EDK2 needs to support both DT and ACPI, we are deviating
-  from the DT spec and marking all reserved memory ranges as
-  EfiReservedMemoryType itself irrespective of no-map.
-
   @param FdtPointer Pointer to FDT
 
 **/
@@ -228,11 +212,19 @@ AddReservedMemoryMap (
   Size
   ));
 
-BuildMemoryAllocationHob (
-  Addr,
-  Size,
-  EfiReservedMemoryType
-  );
+if (fdt_getprop (FdtPointer, SubNode, "no-map", )) {
+  BuildMemoryAllocationHob (
+Addr,
+Size,
+EfiReservedMemoryType
+);
+} else {
+  BuildMemoryAllocationHob (
+Addr,
+Size,
+EfiBootServicesData
+);
+}
   }
 }
   }
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106969): https://edk2.groups.io/g/devel/message/106969
Mute This Topic: https://groups.io/mt/100198972/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 1/1] OvmfPkg/RiscVVirt: Avoid printing hard coded timeout value

2023-07-17 Thread Sunil V L
Print the timeout value set in the PCD variable instead of
hard coded 10 seconds.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Andrei Warkentin 

Signed-off-by: Sunil V L 
---
 OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c 
b/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c
index be4316f320f9..964c35ee5328 100644
--- a/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/OvmfPkg/RiscVVirt/Library/PlatformBootManagerLib/PlatformBm.c
@@ -936,7 +936,7 @@ PlatformBootManagerAfterConsole (
   );
   }
 
-  Print (L"Press ESCAPE within 10 seconds for boot options ");
+  Print (L"Press ESCAPE within %d seconds for boot options ", PcdGet16 
(PcdPlatformBootTimeOut));
   //
   // Process QEMU's -kernel command line option. The kernel booted this way
   // will receive ACPI tables: in PlatformBootManagerBeforeConsole(), we
-- 
2.39.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106968): https://edk2.groups.io/g/devel/message/106968
Mute This Topic: https://groups.io/mt/100198952/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 00/14] Implement Dynamic Memory Protections

2023-07-17 Thread Pedro Falcato
On Mon, Jul 17, 2023 at 5:26 PM Ard Biesheuvel  wrote:
>
> On Mon, 17 Jul 2023 at 18:15, Pedro Falcato  wrote:
> >
> > On Wed, Jul 12, 2023 at 12:53 AM 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.
> >
> > How do you mitigate the possibility of an attack overwriting the
> > dynamic configuration data (the HOBs)?
> > It seems most dangerous to me to publish this sort of
> > security-sensitive configuration knobs dynamically such that an
> > attacker can change them.
> >
>
> That is a very good point. One of the things I have on my TODO list
> for the memory attributes PEI work is to remap HOB memory read-only
> before entering DXE. They are conceptually read-only anyway when PEI
> completes, so they should never be modified afterwards.

I agree, but it also seems that this patch set needs some sort of
__ro_after_init capabilities. For example, in
https://github.com/tianocore/edk2/pull/4566/commits/e485459b6efb1e49591c6f3011d9da14746c52bc#diff-02c0ef19d024b43162043efdd9ed95e0eef1653bcb5bef1e2f2b77587aee2622R101
(DxeMemoryProtectionHobLibConstructor), a copy of this same HOB is
made onto .data, while it should be RO-protected as well.
With both the HOB list and this sort of __ro_after_init protected, the
only remaining exploits would be to DMA over those pages (addressed by
IOMMU, not in this scope), to remap those pages (requires ring 0
access, therefore irrelevant) or to toggle some sort of WP-like bit
(CR0.WP, other archs may have equivalents), which already bypasses
most of the memory protections and therefore isn't all that concerning
to me.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106967): https://edk2.groups.io/g/devel/message/106967
Mute This Topic: https://groups.io/mt/100090629/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 00/14] Implement Dynamic Memory Protections

2023-07-17 Thread Ard Biesheuvel
On Mon, 17 Jul 2023 at 18:15, Pedro Falcato  wrote:
>
> On Wed, Jul 12, 2023 at 12:53 AM 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.
>
> How do you mitigate the possibility of an attack overwriting the
> dynamic configuration data (the HOBs)?
> It seems most dangerous to me to publish this sort of
> security-sensitive configuration knobs dynamically such that an
> attacker can change them.
>

That is a very good point. One of the things I have on my TODO list
for the memory attributes PEI work is to remap HOB memory read-only
before entering DXE. They are conceptually read-only anyway when PEI
completes, so they should never be modified afterwards.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106966): https://edk2.groups.io/g/devel/message/106966
Mute This Topic: https://groups.io/mt/100090629/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 00/14] Implement Dynamic Memory Protections

2023-07-17 Thread Pedro Falcato
On Wed, Jul 12, 2023 at 12:53 AM 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.

How do you mitigate the possibility of an attack overwriting the
dynamic configuration data (the HOBs)?
It seems most dangerous to me to publish this sort of
security-sensitive configuration knobs dynamically such that an
attacker can change them.

-- 
Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106965): https://edk2.groups.io/g/devel/message/106965
Mute This Topic: https://groups.io/mt/100090629/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] OvmfPkg:Fix Hii form name mismatch with EFI variable

2023-07-17 Thread Gerd Hoffmann
On Fri, Jul 14, 2023 at 04:59:48PM +0800, Wang, Yin wrote:
> Onemore need change beside fixs:16acacf24c ("OvmfPkg: fix PlatformConfig")
> Find by sct case:ExtractConfigConformance fail.
> 
> Signed-off-by: Yin Wang 
> ---
>  OvmfPkg/PlatformDxe/Platform.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/PlatformDxe/Platform.c b/OvmfPkg/PlatformDxe/Platform.c
> index cd3bfd554c98..c32439683d83 100644
> --- a/OvmfPkg/PlatformDxe/Platform.c
> +++ b/OvmfPkg/PlatformDxe/Platform.c
> @@ -279,7 +279,7 @@ ExtractConfig (
>  //
>  ConfigRequestHdr = HiiConstructConfigHdr (
>   ,
> - mVariableName,
> + mHiiFormName,
>   mImageHandle
>   );
>  if (ConfigRequestHdr == NULL) {

Acked-by: Gerd Hoffmann 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106964): https://edk2.groups.io/g/devel/message/106964
Mute This Topic: https://groups.io/mt/100142143/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 00/14] Implement Dynamic Memory Protections

2023-07-17 Thread Gerd Hoffmann
  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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v1 1/1] CryptoPkg/BaseCryptoLib: Remove unnecessary key generation.

2023-07-17 Thread levi.yun
When EcGenerateKey() is called with PublicKeySize set to zero or
less than the required size,
it returns the size of the required buffer with failure.
However, EcGenerateKey() generates a key and then checks
if the buffer size is insufficient.
This can be optimised by moving the public key size check
before generating the key.
Therefore, optimise to avoid unnecessary key generation.

Signed-off-by: levi.yun 
---
This changes can be seen at 
https://github.com/LeviYeoReum/edk2/tree/levi/2716_not_generate_key_on_fail_size_v1

 CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
index 
d8cc9ba0e8f968f6cbd9ac4c56018f9a4392cd0b..af67f512a22b23af3844b9bbc87dd57bcf952f04
 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptEc.c
@@ -497,16 +497,16 @@ EcGenerateKey (
   Group= EC_KEY_get0_group (EcKey);
   HalfSize = (EC_GROUP_get_degree (Group) + 7) / 8;

+  if (*PublicKeySize < HalfSize * 2) {
+*PublicKeySize = HalfSize * 2;
+return FALSE;
+  }
+
   // Assume RAND_seed was called
   if (EC_KEY_generate_key (EcKey) != 1) {
 return FALSE;
   }

-  if (*PublicKeySize < HalfSize * 2) {
-*PublicKeySize = HalfSize * 2;
-return FALSE;
-  }
-
   *PublicKeySize = HalfSize * 2;

   EcPoint = EC_KEY_get0_public_key (EcKey);
--
Guid("CE165669-3EF3-493F-B85D-6190EE5B9759")
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106962): https://edk2.groups.io/g/devel/message/106962
Mute This Topic: https://groups.io/mt/100191693/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] 回复: [PATCH v2 0/4] OvmfPkg/RiscVVirt: Add CLANGDWARF toolchain support

2023-07-17 Thread Ard Biesheuvel
On Mon, 17 Jul 2023 at 03:51, gaoliming via groups.io
 wrote:
>
> Sunil:
>   Do you use which CLANG version is used to verify this change?
>

For the series,

Tested-by: Ard Biesheuvel  # Debian clang version 14.0.6


> > -邮件原件-
> > 发件人: Sunil V L 
> > 发送时间: 2023年7月11日 23:44
> > 收件人: devel@edk2.groups.io
> > 抄送: Sunil V L ; Rebecca Cran
> > ; Liming Gao ; Bob Feng
> > ; Yuwei Chen ; Ard
> > Biesheuvel ; Jiewen Yao
> > ; Jordan Justen ; Andrei
> > Warkentin ; Heinrich Schuchardt
> > 
> > 主题: [PATCH v2 0/4] OvmfPkg/RiscVVirt: Add CLANGDWARF toolchain
> > support
> >
> > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4478
> >
> > This series adds support for building RiscVVirtQemu EDK2 using
> > CLANGDWARF toolchain. Adding this support helps people to use
> > the same toolchain to build EDK2 for different architectures.
> >
> > Cc: Rebecca Cran 
> > Cc: Liming Gao 
> > Cc: Bob Feng 
> > Cc: Yuwei Chen 
> > Cc: Ard Biesheuvel 
> > Cc: Jiewen Yao 
> > Cc: Jordan Justen 
> > Cc  Gerd Hoffmann 
> > Cc: Andrei Warkentin 
> > Cc: Heinrich Schuchardt 
> >
> > Changes since v1:
> >   1) Addressed Liming's comments in PATCH 3.
> >
> > Sunil V L (4):
> >   OvmfPkg/RiscVVirt: use 'auto' alignment and FIXED for XIP modules
> >   OvmfPkg/RiscVVirt: SecEntry: Remove unnecessary assembly directives
> >   BaseTools/tools_def: Add CLANGDWARF support for RISC-V
> >   OvmfPkg/RiscVVirt: Update README for CLANGDWARF support
> >
> >  OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 34 ++
> >  BaseTools/Conf/tools_def.template   | 53
> > +
> >  OvmfPkg/RiscVVirt/README.md | 28 +--
> >  OvmfPkg/RiscVVirt/Sec/SecEntry.S|  3 --
> >  4 files changed, 88 insertions(+), 30 deletions(-)
> >
> > --
> > 2.34.1
>
>
>
>
>
> 
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106961): https://edk2.groups.io/g/devel/message/106961
Mute This Topic: https://groups.io/mt/100187566/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA breaks GRUB on Ubuntu 22.04

2023-07-17 Thread Gerd Hoffmann
  Hi,

> > The idea is: Improve page fault handler to (a) print a big'n'fat
> > warning, and (b) loosening up memory permissions for the faulting
> > page address.
> >
> > No patch for that emerged (yet?).
> 
> Ack. I can work on that.

FYI: There was a patch series on the list last week to move various
paging / security related options from compile time (PCD) to runtime
(config struct in HOB).  All NX settings are in there, also page guard
and heap guard.  Also some (very basic) support for config profiles.

With that in place it would be possible to make this configurable in
uefi firmware settings (or via fw_cfg, or both).

> Also, what's the situation on this for x86? I assume it's a lot worse there?

Currently x86 is less problematic in practice, but only because many of
the security features are not (yet) enabled.

Note it's not only grub+shim, the linux kernel stub is affected too.

The new, uefi-stub-only archs (armv7, armv8,riscv) are fixed meanwhile,
and they all use the common zboot code.  x86 is wip still, ard has a
patch series in flight, it's more tricky there due to hybrid bios/uefi
kernels and other legacy cruft ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106960): https://edk2.groups.io/g/devel/message/106960
Mute This Topic: https://groups.io/mt/100057351/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: Uses gMmst in MmSaveStateLib

2023-07-17 Thread Ni, Ray
Reviewed-by: Ray Ni 

> -Original Message-
> From: Abdul Lateef Attar 
> Sent: Wednesday, July 12, 2023 2:24 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar ; Dong, Eric
> ; Ni, Ray ; Kumar, Rahul R
> ; Gerd Hoffmann ; Abner Chang
> 
> Subject: [PATCH v1 1/1] UefiCpuPkg: Uses gMmst in MmSaveStateLib
> 
> From: Abdul Lateef Attar 
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4182
> 
> Use gMmst instead of gSmst.
> Replace SmmServicesTableLib with MmServicesTableLib.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Abner Chang 
> Signed-off-by: Abdul Lateef Attar 
> ---
>  UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf   | 2 +-
>  UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf | 2 +-
>  UefiCpuPkg/Library/MmSaveStateLib/MmSaveState.h   | 2 +-
>  UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c| 4 ++--
>  UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveState.c  | 2 +-
>  UefiCpuPkg/Library/MmSaveStateLib/MmSaveStateCommon.c | 8 
>  6 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
> b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
> index 5c0685f283d3..dcee6c401d30 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveStateLib.inf
> @@ -31,4 +31,4 @@ [LibraryClasses]
>BaseLib
>BaseMemoryLib
>DebugLib
> -  SmmServicesTableLib
> +  MmServicesTableLib
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf
> b/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf
> index b92dfa643203..b7fd4078f58a 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveStateLib.inf
> @@ -31,4 +31,4 @@ [LibraryClasses]
>BaseLib
>BaseMemoryLib
>DebugLib
> -  SmmServicesTableLib
> +  MmServicesTableLib
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/MmSaveState.h
> b/UefiCpuPkg/Library/MmSaveStateLib/MmSaveState.h
> index c3499cbb3b17..6c7e8abd5f62 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/MmSaveState.h
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/MmSaveState.h
> @@ -14,7 +14,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
> 
>  // Macro used to simplify the lookup table entries of type
> CPU_MM_SAVE_STATE_REGISTER_RANGE
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> index 9fed52896f5c..3315a6cc44ff 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> @@ -108,7 +108,7 @@ MmSaveStateReadRegister (
>UINT8  DataWidth;
> 
>// Read CPU State
> -  CpuSaveState = (AMD_SMRAM_SAVE_STATE_MAP *)gSmst-
> >CpuSaveState[CpuIndex];
> +  CpuSaveState = (AMD_SMRAM_SAVE_STATE_MAP *)gMmst-
> >CpuSaveState[CpuIndex];
> 
>// Check for special EFI_MM_SAVE_STATE_REGISTER_LMA
>if (Register == EFI_MM_SAVE_STATE_REGISTER_LMA) {
> @@ -226,7 +226,7 @@ MmSaveStateWriteRegister (
>  return EFI_NOT_FOUND;
>}
> 
> -  CpuSaveState = gSmst->CpuSaveState[CpuIndex];
> +  CpuSaveState = gMmst->CpuSaveState[CpuIndex];
> 
>//
>// Do not write non-writable SaveState, because it will cause exception.
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveState.c
> b/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveState.c
> index fd321bb571d5..c2ccd65b1dc4 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveState.c
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/IntelMmSaveState.c
> @@ -296,7 +296,7 @@ MmSaveStateWriteRegister (
>  return EFI_NOT_FOUND;
>}
> 
> -  CpuSaveState = gSmst->CpuSaveState[CpuIndex];
> +  CpuSaveState = gMmst->CpuSaveState[CpuIndex];
> 
>//
>// Do not write non-writable SaveState, because it will cause exception.
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/MmSaveStateCommon.c
> b/UefiCpuPkg/Library/MmSaveStateLib/MmSaveStateCommon.c
> index 09c6c3f96fed..f66245b82c40 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/MmSaveStateCommon.c
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/MmSaveStateCommon.c
> @@ -99,8 +99,8 @@ MmSaveStateReadRegisterByIndex (
>  //
>  // Write return buffer
>  //
> -ASSERT (gSmst->CpuSaveState[CpuIndex] != NULL);
> -CopyMem (Buffer, (UINT8 *)gSmst->CpuSaveState[CpuIndex] +
> mCpuWidthOffset[RegisterIndex].Offset32, Width);
> +ASSERT (gMmst->CpuSaveState[CpuIndex] != NULL);
> +CopyMem (Buffer, (UINT8 *)gMmst->CpuSaveState[CpuIndex] +
> mCpuWidthOffset[RegisterIndex].Offset32, Width);
>} else {
>  //
>  // If 64-bit mode width is zero, then the specified register can not be 
> accessed
> @@ -119,12 +119,12 @@ MmSaveStateReadRegisterByIndex (
>  //
>  // Write lower 32-bits of return buffer
>  //
> -CopyMem (Buffer, (UINT8 

[edk2-devel] 回复: [PATCH v4 0/8] SecurityPkg/MdePkg: Update RngLib GUID identification

2023-07-17 Thread gaoliming via groups.io
Pierre:
  Now, BaseRngLibTimerLib in MdePkg is used in many platforms. I think we
need to reserve enough time for the platform owner to update their DSC
files. 

  So, I suggest to keep current BaseRngLibTimerLib in MdePkg for
compatibility, and add new BaseRngLibTimerLib in MdeModulePkg for this
support. After some time, such as two stable tags, we can propose to remove
the one in MdePkg. 

Thanks
Liming
> -邮件原件-
> 发件人: pierre.gond...@arm.com 
> 发送时间: 2023年7月12日 21:30
> 收件人: devel@edk2.groups.io
> 抄送: Michael D Kinney ; Liming Gao
> ; Zhiguang Liu ; Jiewen
> Yao ; Jian J Wang ; Ard
> Biesheuvel ; Sami Mujawar
> ; Jose Marinho ; Kun
> Qin ; pierre.gond...@arm.com
> 主题: [PATCH v4 0/8] SecurityPkg/MdePkg: Update RngLib GUID identification
> 
> From: Pierre Gondois 
> 
> v4:
> - New patches:
>   - [1/8] MdePkg: Move BaseRngLibTimerLib to MdeModulePkg
>   - [5/8] MdeModulePkg/Rng: Add GUID to describe unsafe Rng algorithms
> - This patch-set now requires to be accepted along an edk-platforms patch
>   moving the BaseRngLibTimerLib to MdeModulePkg
> 
> v3:
> - As the unsafe algorithm GUID will not be added to the UEFI
>   specification, rename:
>   - gEfiRngAlgorithmUnSafe to gEdkiiRngAlgorithmUnSafe
>   - EFI_RNG_ALGORITHM_UNSAFE to EDKII_RNG_ALGORITHM_UNSAFE
> 
> v2:
> [1/8] MdePkg/ArmTrngLib: Remove ASSERTs in Null implementation
> - Dropped
> [2/8] MdePkg/MdePkg.dec: Move PcdCpuRngSupportedAlgorithm to MdePkg
> - Change gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm
>   token number
> - Rename to SecurityPkg/SecurityPkg.dec: Move
>   PcdCpuRngSupportedAlgorithm to MdePkg
> [5/8] MdePkg/Rng: Add GetRngGuid() to RngLib
> - Remove gEfiRngAlgorithmUnSafe from inf file
> - Split Guids definitions in arch specific sections
> [6/8] SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
> - Remove RngFindDefaultAlgo() and change logic accordingly.
> [7/8] SecurityPkg/RngDxe: Select safe default Rng algorithm
> - Dropped due to changes in [6/8]
> 
> This patch also requires the following patch on top of the serie:
> - https://edk2.groups.io/g/devel/message/106546
> 
> This patchset follows the 'code first' approach and relates to [1].
> This patchset follows the thread at [3] that aims to solve [2].
> [1] and [2] are bound and this patchset aims to solve both.
> 
> In this patchset:
> a-
> The RngDxe can rely on the RngLib. However the RngLib has no
> interface allowing to describe which Rng algorithm is implemented.
> The RngDxe must advertise the algorithm that are available through
> the RngGetInfo() callback.
> Add a GetRngGuid() for interface to the RngLib.
> 
> b-
> The Arm Architecture states the RNDR that the DRBG algorithm should
> be compliant with NIST SP800-90A, while not mandating a particular
> algorithm, so as to be inclusive of different geographies.
> The RngLib can rely on this Arm RNDR instruction. In order to
> accurately describe the implementation using the RNDR instruction,
> add a EFI_RNG_ALGORITHM_ARM_RNDR GUID [1].
> 
> c-
> For the same reason as a/b, add a GUID describing unsafe RNG
> algorithms, allowing to accurately describe the BaseRngLibTimerLib.
> 
> d-
> Use a/b/c mechanisms/GUIDs to select a safe Rng algorithm in the
> Arm implementation of the RngDxe.
> 
> [1] BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4441
> [2] BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=4151
> [3] https://edk2.groups.io/g/devel/message/100806
> 
> Pierre Gondois (8):
>   MdePkg: Move BaseRngLibTimerLib to MdeModulePkg
>   SecurityPkg/SecurityPkg.dec: Move PcdCpuRngSupportedAlgorithm to
> MdePkg
>   MdePkg/DxeRngLib: Request raw algorithm instead of default
>   MdePkg/Rng: Add GUID to describe Arm Rndr Rng algorithms
>   MdeModulePkg/Rng: Add GUID to describe unsafe Rng algorithms
>   MdePkg/Rng: Add GetRngGuid() to RngLib
>   SecurityPkg/RngDxe: Use GetRngGuid() when probing RngLib
>   SecurityPkg/RngDxe: Simplify Rng algorithm selection for Arm
> 
>  ArmVirtPkg/ArmVirt.dsc.inc|  2 +-
>  EmulatorPkg/EmulatorPkg.dsc   |  2 +-
>  MdeModulePkg/Include/Guid/RngAlgorithm.h  | 23 
>  .../BaseRngLibTimerLib/BaseRngLibTimerLib.inf |  4 ++
>  .../BaseRngLibTimerLib/BaseRngLibTimerLib.uni |  0
>  .../Library/BaseRngLibTimerLib/RngLibTimer.c  | 28 ++
>  MdeModulePkg/MdeModulePkg.dec |  3 +
>  MdeModulePkg/MdeModulePkg.dsc |  1 +
>  MdePkg/Include/Library/RngLib.h   | 17 ++
>  MdePkg/Include/Protocol/Rng.h | 10 
>  MdePkg/Library/BaseRngLib/AArch64/Rndr.c  | 42 ++
>  MdePkg/Library/BaseRngLib/BaseRngLib.inf  | 10 
>  MdePkg/Library/BaseRngLib/Rand/RdRand.c   | 26 +
>  .../Library/BaseRngLibNull/BaseRngLibNull.c   | 22 
>  MdePkg/Library/DxeRngLib/DxeRngLib.c  | 36 +++-
>  MdePkg/MdePkg.dec |  6 ++
>  MdePkg/MdePkg.dsc |  1 -
>  

[edk2-devel] [PATCH v2 1/1] ShellPkg: Acpiview/GTDT: Print timer flags information.

2023-07-17 Thread levi.yun
Currently, GTDT only prints the value of timer flags in hex.
This change prints the detail information about Timer flags in GTDT.

before:
Shell> acpiview -s GTDT
...
Non-Secure EL1 timer FLAGS : 0x2
Virtual timer GSIV : 0x1B
Virtual timer FLAGS: 0x2
...

after:
Shell> acpiview -s GTDT
...
Non-Secure EL1 timer FLAGS : 0x2
Timer Interrupt Mode : Level Trigger(0)
Timer Interrupt Polarity : Active Low(1)
Always-on Capability : 0
Reserved : 0

Virtual timer GSIV : 0x1B
Virtual timer FLAGS: 0x2

Signed-off-by: levi.yun 
Tested-by: Pierre Gondois 
---
The changes can be seen at 
https://github.com/LeviYeoReum/edk2/tree/2711_gtdt_flags_v2

Notes:
v2:
   - Fix typos.
   - Remove unnecessary type casting.

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 111 
+---
 1 file changed, 98 insertions(+), 13 deletions(-)

diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index 
e62927098a010a0e1dad8361dcfc6559d32dcebf..c95b96a673bf7e6afea7902f5730dcb61a3ce508
 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -84,33 +84,118 @@ ValidateGtFrameNumber (
   }
 }

+/**
+  This function prints trigger mode information in timer flags.
+
+  @param [in] Format Print format.
+  @param [in] PtrPointer to the start of the field data.
+**/
+STATIC
+VOID
+EFIAPI
+PrintTimerInterruptMode (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  UINT8  TriggerMode;
+
+  TriggerMode = *Ptr;
+
+  Print (
+L"%s(%d)",
+(TriggerMode ? L"Edge Trigger" : L"Level Trigger"),
+TriggerMode
+);
+}
+
+/**
+  This function prints polarity information in timer flags.
+
+  @param [in] Format Print format.
+  @param [in] PtrPointer to the start of the field data.
+**/
+STATIC
+VOID
+EFIAPI
+PrintTimerInterruptPolarity (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  UINT8  Polarity;
+
+  Polarity = *Ptr;
+
+  Print (
+L"%s(%d)",
+(Polarity ? L"Active Low" : L"Active High"),
+Polarity
+);
+}
+
+/**
+  An ACPI_PARSER array describing the Timer Flags Field in GTDT Table.
+**/
+STATIC CONST ACPI_PARSER  TimerFlagsParser[] = {
+  { L"Timer Interrupt Mode", 1,  0, NULL,  PrintTimerInterruptMode, 
NULL, NULL, NULL },
+  { L"Timer Interrupt Polarity", 1,  1, NULL,  PrintTimerInterruptPolarity, 
NULL, NULL, NULL },
+  { L"Always-on Capability", 1,  2, L"%d", NULL,
NULL, NULL, NULL },
+  { L"Reserved", 29, 3, L"%d", NULL,
NULL, NULL, NULL },
+};
+
+/**
+  This function parses the Timer Flags.
+
+  @param [in]   Format  Print format.
+  @param [in]   Ptr Pointer to the start of the Timer flags.
+ **/
+STATIC
+VOID
+EFIAPI
+DumpTimerFlags (
+  IN CONST CHAR16  *Format OPTIONAL,
+  IN UINT8 *Ptr
+  )
+{
+  DumpUint32 (L"0x%x\n", Ptr);
+  ParseAcpiBitFields (
+TRUE,
+2,
+NULL,
+Ptr,
+4,
+PARSER_PARAMS (TimerFlagsParser)
+);
+}
+
 /**
   An ACPI_PARSER array describing the ACPI GTDT Table.
 **/
 STATIC CONST ACPI_PARSER  GtdtParser[] = {
   PARSE_ACPI_HEADER (),
-  { L"CntControlBase Physical Address",8,  36,  L"0x%lx", NULL, NULL,
+  { L"CntControlBase Physical Address",8,  36,  L"0x%lx", NULL,   
NULL,
 NULL, NULL },
-  { L"Reserved",  4,  44,  L"0x%x",  NULL, NULL,NULL,  
NULL },
-  { L"Secure EL1 timer GSIV", 4,  48,  L"0x%x",  NULL, NULL,NULL,  
NULL },
-  { L"Secure EL1 timer FLAGS",4,  52,  L"0x%x",  NULL, NULL,NULL,  
NULL },
+  { L"Reserved",  4,  44,  L"0x%x",  NULL,   
NULL,NULL,  NULL },
+  { L"Secure EL1 timer GSIV", 4,  48,  L"0x%x",  NULL,   
NULL,NULL,  NULL },
+  { L"Secure EL1 timer FLAGS",4,  52,  NULL, DumpTimerFlags, 
NULL,NULL,  NULL },

-  { L"Non-Secure EL1 timer GSIV", 4,  56,  L"0x%x",  NULL, NULL,NULL,  
NULL },
-  { L"Non-Secure EL1 timer FLAGS",4,  60,  L"0x%x",  NULL, NULL,NULL,  
NULL },
+  { L"Non-Secure EL1 timer GSIV", 4,  56,  L"0x%x",  NULL,   
NULL,NULL,  NULL },
+  { L"Non-Secure EL1 timer FLAGS",4,  60,  NULL, DumpTimerFlags, 
NULL,NULL,  NULL },

-  { L"Virtual timer GSIV",4,  64,  L"0x%x",  NULL, NULL,NULL,  
NULL },
-  { L"Virtual timer FLAGS",   4,  68,  L"0x%x",  NULL, NULL,NULL,  
NULL },
+  { L"Virtual timer GSIV",4,  64,  L"0x%x",  NULL,   
NULL,NULL,  NULL },
+  { L"Virtual timer FLAGS",