[edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Cheng-Chieh Huang via groups.io posted 6 patches 4 years, 6 months ago
There is a newer version of this series
[edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data
Posted by Cheng-Chieh Huang via groups.io 4 years, 6 months ago
Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (
 
   // The UEFI payload FV
   BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 (PcdPayloadFdMemSize), EfiBootServicesData);
+  // The UEFI payload config FV
+  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, SIZE_64KB, EfiRuntimeServicesData);
 
   //
   // Build CPU memory space and IO space hob
-- 
2.32.0.402.g57bb445576-goog



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78028): https://edk2.groups.io/g/devel/message/78028
Mute This Topic: https://groups.io/mt/84357536/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data
Posted by Guo Dong 4 years, 6 months ago
why build runtime memory allocation hob here? 
The PayloadEntry should already got the information and build a new HOB list to DXE core, will anyone access these region late?
If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and update commit message on this.

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Cheng-Chieh Huang via groups.io
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io
Cc: Cheng-Chieh Huang <chengchieh@google.com>
Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
---
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (
 
   // The UEFI payload FV
   BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 (PcdPayloadFdMemSize), EfiBootServicesData);
+  // The UEFI payload config FV
+  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, SIZE_64KB, EfiRuntimeServicesData);
 
   //
   // Build CPU memory space and IO space hob
-- 
2.32.0.402.g57bb445576-goog








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78627): https://edk2.groups.io/g/devel/message/78627
Mute This Topic: https://groups.io/mt/84357536/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data
Posted by Cheng-Chieh Huang via groups.io 4 years, 6 months ago
Hi Guo,

Thanks for the review. This is a quick fix for today's
PlatformHookLib design. PlatformHookLib (probably via
PlatformBootManagerLib) will be linked by BDS and potentially called again
in Windows bootloader. If we did not reserve this range of memory, system
will crash the system when it was called. This can be reproduced if we boot
Windows 2018. I also did not think reserving this memory til runtime is a
good idea, but I am not sure why we want to link PlatformHookLib to
PlatformBootManagerLib. I tried to remove PlatformHookLib in
PlatformBootManagerLib and it seems to work as well. maybe we should take
this approach. what do you think?

--
Cheng-Chieh

On Wed, Aug 4, 2021 at 10:44 AM Dong, Guo <guo.dong@intel.com> wrote:

>
> why build runtime memory allocation hob here?
> The PayloadEntry should already got the information and build a new HOB
> list to DXE core, will anyone access these region late?
> If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and
> update commit message on this.
>
> Thanks,
> Guo
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Cheng-Chieh Huang via groups.io
> Sent: Wednesday, July 21, 2021 6:23 AM
> To: devel@edk2.groups.io
> Cc: Cheng-Chieh Huang <chengchieh@google.com>
> Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload
> config in runtime services data
>
> Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com>
> ---
>  UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> index ae16f25c7c0e..70afbf83ed4a 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> @@ -517,6 +517,8 @@ BuildGenericHob (
>
>    // The UEFI payload FV
>    BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32
> (PcdPayloadFdMemSize), EfiBootServicesData);
> +  // The UEFI payload config FV
> +  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB,
> SIZE_64KB, EfiRuntimeServicesData);
>
>    //
>    // Build CPU memory space and IO space hob
> --
> 2.32.0.402.g57bb445576-goog
>
>
>
> 
>
>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78648): https://edk2.groups.io/g/devel/message/78648
Mute This Topic: https://groups.io/mt/84357536/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data
Posted by Guo Dong 4 years, 6 months ago
Hi Cheng-Chieh,

I see. In order to use same PlatformHookLib to make DebugLib works in both PayloadEntry and DXE phase, the PlatformHookLib gets serial port info from bootloader.
In the DXE phase, UniversalPayloadPlatformHookLib will use HOB instead of getting info from bootloader. In the PayloadEntry module, we should build same serial port HOB so that DXE module could use UniversalPayloadPlatformHookLib Instead of PlatformHookLib.
For this patch, you could add LINUXBOOT_PAYLOAD flag with commit message update, I will revert this patch once above change is completed.

Thanks,
Guo

From: Cheng-Chieh Huang <chengchieh@google.com>
Sent: Tuesday, August 3, 2021 11:24 PM
To: Dong, Guo <guo.dong@intel.com>
Cc: devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Hi Guo,

Thanks for the review. This is a quick fix for today's PlatformHookLib design. PlatformHookLib (probably via PlatformBootManagerLib) will be linked by BDS and potentially called again in Windows bootloader. If we did not reserve this range of memory, system will crash the system when it was called. This can be reproduced if we boot Windows 2018. I also did not think reserving this memory til runtime is a good idea, but I am not sure why we want to link PlatformHookLib to PlatformBootManagerLib. I tried to remove PlatformHookLib in PlatformBootManagerLib and it seems to work as well. maybe we should take this approach. what do you think?

--
Cheng-Chieh

On Wed, Aug 4, 2021 at 10:44 AM Dong, Guo <guo.dong@intel.com<mailto:guo.dong@intel.com>> wrote:

why build runtime memory allocation hob here?
The PayloadEntry should already got the information and build a new HOB list to DXE core, will anyone access these region late?
If yes, maybe you need add LINUXBOOT_PAYLOAD flag for this code, and update commit message on this.

Thanks,
Guo

-----Original Message-----
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Cheng-Chieh Huang via groups.io<http://groups.io>
Sent: Wednesday, July 21, 2021 6:23 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Cc: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
Subject: [edk2-devel] [PATCH v1 4/6] UefiPayloadPkg: Reserve Payload config in runtime services data

Signed-off-by: Cheng-Chieh Huang <chengchieh@google.com<mailto:chengchieh@google.com>>
---
 UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
index ae16f25c7c0e..70afbf83ed4a 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
@@ -517,6 +517,8 @@ BuildGenericHob (

   // The UEFI payload FV
   BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase), PcdGet32 (PcdPayloadFdMemSize), EfiBootServicesData);
+  // The UEFI payload config FV
+  BuildMemoryAllocationHob (PcdGet32 (PcdPayloadFdMemBase) - SIZE_64KB, SIZE_64KB, EfiRuntimeServicesData);

   //
   // Build CPU memory space and IO space hob
--
2.32.0.402.g57bb445576-goog







-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78647): https://edk2.groups.io/g/devel/message/78647
Mute This Topic: https://groups.io/mt/84357536/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-