MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

[edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by Stacy Howell 2 years, 3 months ago

REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
CC: Dandan Bi <dandan.bi@intel.com>
CC: Liming Gao <gaoliming@byosoft.com.cn>

Updated CoreInternalAllocatePages() to call PromoteMemoryResource() and
re-attempt the allocation if unable to convert the specified memory range

Signed-off-by: Stacy Howell <stacy.howell@intel.com>
---
 MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c b/MdeModulePkg/Core/Dxe/Mem/Page.c
index 47d4c5d92e..cc0b90ac0d 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Page.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
@@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
     Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
   }
 
+  if (EFI_ERROR (Status)) {
+    //
+    // If requested memory region is unavailable it may be untested memory
+    // Attempt to promote memory resources, then re-attempt the allocation
+    //
+    if (PromoteMemoryResource ()) {
+      if (NeedGuard) {
+        Status = CoreConvertPagesWithGuard (Start, NumberOfPages, MemoryType);
+      } else {
+        Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
+      }
+    }
+  }
+
 Done:
   CoreReleaseMemoryLock ();
 
-- 
2.32.0.windows.2



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

回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by gaoliming 2 years, 3 months ago

Stacy:
  This fix covers the case with AllocateAddress allocation type. I agree
this fix. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Stacy Howell
> 发送时间: 2022年1月8日 3:36
> 收件人: devel@edk2.groups.io
> 抄送: Stacy Howell <stacy.howell@intel.com>; Dandan Bi
> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> 主题: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to
> use untested memory
> 
> REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
> CC: Dandan Bi <dandan.bi@intel.com>
> CC: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Updated CoreInternalAllocatePages() to call PromoteMemoryResource() and
> re-attempt the allocation if unable to convert the specified memory range
> 
> Signed-off-by: Stacy Howell <stacy.howell@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> index 47d4c5d92e..cc0b90ac0d 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> @@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
>      Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
>    }
> 
> +  if (EFI_ERROR (Status)) {
> +    //
> +    // If requested memory region is unavailable it may be untested
> memory
> +    // Attempt to promote memory resources, then re-attempt the
> allocation
> +    //
> +    if (PromoteMemoryResource ()) {
> +      if (NeedGuard) {
> +        Status = CoreConvertPagesWithGuard (Start, NumberOfPages,
> MemoryType);
> +      } else {
> +        Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> +      }
> +    }
> +  }
> +
>  Done:
>    CoreReleaseMemoryLock ();
> 
> --
> 2.32.0.windows.2
> 
> 
> 
> 
> 





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

Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by Sean 2 years, 3 months ago

if this is auto promotion is happening in the core then what is the 
value of memory testing and tracking that state.   Is memory testing 
state a necessary feature of the Dxe Core?


I think it makes more sense that if you platform wants to use a given 
range your platform should either test it and/or mark it as tested.

OR

The dxe core should do away with the memory testing tracking.


On most platforms i have seen in the past few years all memory is marked 
as tested without doing any testing.  The only value in the flag is keep 
the initial memory allocations in a given low range (below 4gb).




On 1/10/2022 5:59 PM, gaoliming wrote:
> Stacy:
>    This fix covers the case with AllocateAddress allocation type. I agree
> this fix. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> 
> Thanks
> Liming
>> -----邮件原件-----
>> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Stacy Howell
>> 发送时间: 2022年1月8日 3:36
>> 收件人: devel@edk2.groups.io
>> 抄送: Stacy Howell <stacy.howell@intel.com>; Dandan Bi
>> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
>> 主题: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to
>> use untested memory
>>
>> REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
>> CC: Dandan Bi <dandan.bi@intel.com>
>> CC: Liming Gao <gaoliming@byosoft.com.cn>
>>
>> Updated CoreInternalAllocatePages() to call PromoteMemoryResource() and
>> re-attempt the allocation if unable to convert the specified memory range
>>
>> Signed-off-by: Stacy Howell <stacy.howell@intel.com>
>> ---
>>   MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
>>   1 file changed, 14 insertions(+)
>>
>> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
>> b/MdeModulePkg/Core/Dxe/Mem/Page.c
>> index 47d4c5d92e..cc0b90ac0d 100644
>> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
>> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
>> @@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
>>       Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
>>     }
>>
>> +  if (EFI_ERROR (Status)) {
>> +    //
>> +    // If requested memory region is unavailable it may be untested
>> memory
>> +    // Attempt to promote memory resources, then re-attempt the
>> allocation
>> +    //
>> +    if (PromoteMemoryResource ()) {
>> +      if (NeedGuard) {
>> +        Status = CoreConvertPagesWithGuard (Start, NumberOfPages,
>> MemoryType);
>> +      } else {
>> +        Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
>> +      }
>> +    }
>> +  }
>> +
>>   Done:
>>     CoreReleaseMemoryLock ();
>>
>> --
>> 2.32.0.windows.2
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 
> 
> 


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

回复: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by gaoliming 2 years, 3 months ago

Sean:
 Yes. Platform can report the tested memory for the specific range. 

Stacy:
 Can you evaluate Sean's solution for your problem?

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Sean
> 发送时间: 2022年1月11日 10:47
> 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn;
> stacy.howell@intel.com
> 抄送: 'Dandan Bi' <dandan.bi@intel.com>
> 主题: Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE
> Drivers to use untested memory
> 
> if this is auto promotion is happening in the core then what is the
> value of memory testing and tracking that state.   Is memory testing
> state a necessary feature of the Dxe Core?
> 
> 
> I think it makes more sense that if you platform wants to use a given
> range your platform should either test it and/or mark it as tested.
> 
> OR
> 
> The dxe core should do away with the memory testing tracking.
> 
> 
> On most platforms i have seen in the past few years all memory is marked
> as tested without doing any testing.  The only value in the flag is keep
> the initial memory allocations in a given low range (below 4gb).
> 
> 
> 
> 
> On 1/10/2022 5:59 PM, gaoliming wrote:
> > Stacy:
> >    This fix covers the case with AllocateAddress allocation type. I agree
> > this fix. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Stacy
> Howell
> >> 发送时间: 2022年1月8日 3:36
> >> 收件人: devel@edk2.groups.io
> >> 抄送: Stacy Howell <stacy.howell@intel.com>; Dandan Bi
> >> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> >> 主题: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers
> to
> >> use untested memory
> >>
> >> REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
> >> CC: Dandan Bi <dandan.bi@intel.com>
> >> CC: Liming Gao <gaoliming@byosoft.com.cn>
> >>
> >> Updated CoreInternalAllocatePages() to call PromoteMemoryResource()
> and
> >> re-attempt the allocation if unable to convert the specified memory range
> >>
> >> Signed-off-by: Stacy Howell <stacy.howell@intel.com>
> >> ---
> >>   MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> index 47d4c5d92e..cc0b90ac0d 100644
> >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> @@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
> >>       Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> >>     }
> >>
> >> +  if (EFI_ERROR (Status)) {
> >> +    //
> >> +    // If requested memory region is unavailable it may be untested
> >> memory
> >> +    // Attempt to promote memory resources, then re-attempt the
> >> allocation
> >> +    //
> >> +    if (PromoteMemoryResource ()) {
> >> +      if (NeedGuard) {
> >> +        Status = CoreConvertPagesWithGuard (Start, NumberOfPages,
> >> MemoryType);
> >> +      } else {
> >> +        Status = CoreConvertPages (Start, NumberOfPages,
> MemoryType);
> >> +      }
> >> +    }
> >> +  }
> >> +
> >>   Done:
> >>     CoreReleaseMemoryLock ();
> >>
> >> --
> >> 2.32.0.windows.2
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 





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

Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by Michael D Kinney 2 years, 3 months ago

Hi Sean,

The auto promotion of memory was only intended as a dev/debug feature to maximize
platform boot without having to tune what memory is tested in PEI phase.

In my opinion, a production platform should never trigger any auto promotions of
untested to tested memory, and part of production validation should make sure this
event never occurs in any production boot scenarios.

The specific bug being fix here is that auto promotion was not symmetric across
all memory allocation types.  It simply aligns this dev/debug feature.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> Sent: Monday, January 10, 2022 6:47 PM
> To: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; Howell, Stacy <stacy.howell@intel.com>
> Cc: Bi, Dandan <dandan.bi@intel.com>
> Subject: Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory
> 
> if this is auto promotion is happening in the core then what is the
> value of memory testing and tracking that state.   Is memory testing
> state a necessary feature of the Dxe Core?
> 
> 
> I think it makes more sense that if you platform wants to use a given
> range your platform should either test it and/or mark it as tested.
> 
> OR
> 
> The dxe core should do away with the memory testing tracking.
> 
> 
> On most platforms i have seen in the past few years all memory is marked
> as tested without doing any testing.  The only value in the flag is keep
> the initial memory allocations in a given low range (below 4gb).
> 
> 
> 
> 
> On 1/10/2022 5:59 PM, gaoliming wrote:
> > Stacy:
> >    This fix covers the case with AllocateAddress allocation type. I agree
> > this fix. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Stacy Howell
> >> 发送时间: 2022年1月8日 3:36
> >> 收件人: devel@edk2.groups.io
> >> 抄送: Stacy Howell <stacy.howell@intel.com>; Dandan Bi
> >> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> >> 主题: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to
> >> use untested memory
> >>
> >> REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
> >> CC: Dandan Bi <dandan.bi@intel.com>
> >> CC: Liming Gao <gaoliming@byosoft.com.cn>
> >>
> >> Updated CoreInternalAllocatePages() to call PromoteMemoryResource() and
> >> re-attempt the allocation if unable to convert the specified memory range
> >>
> >> Signed-off-by: Stacy Howell <stacy.howell@intel.com>
> >> ---
> >>   MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> index 47d4c5d92e..cc0b90ac0d 100644
> >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> @@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
> >>       Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> >>     }
> >>
> >> +  if (EFI_ERROR (Status)) {
> >> +    //
> >> +    // If requested memory region is unavailable it may be untested
> >> memory
> >> +    // Attempt to promote memory resources, then re-attempt the
> >> allocation
> >> +    //
> >> +    if (PromoteMemoryResource ()) {
> >> +      if (NeedGuard) {
> >> +        Status = CoreConvertPagesWithGuard (Start, NumberOfPages,
> >> MemoryType);
> >> +      } else {
> >> +        Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> >> +      }
> >> +    }
> >> +  }
> >> +
> >>   Done:
> >>     CoreReleaseMemoryLock ();
> >>
> >> --
> >> 2.32.0.windows.2
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 



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

Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by Howell, Stacy 2 years, 3 months ago

Hi Sean,
Setting all memory as tested in PEI is a workaround for the issue that this patch addresses.  However, promoting all memory in PEI is not a workable solution for BIOSes that incorporate full memory testing functionality, as this relies on the tested flag to determine which memory regions to test.

This patch addresses a discrepancy in EDK2 core regarding how untested memory is treated for allocation by DXE drivers.  In the case where a DXE driver does not request a specific memory region DXE Core will promote untested memory if necessary to provide memory to the driver.  In the case where a DXE driver requests a specific memory range of untested memory, DXE Core will currently return an error instead of promoting untested memory to make the region available for the driver.

Thanks,
Stacy

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Tuesday, January 11, 2022 10:57 AM
To: devel@edk2.groups.io; spbrogan@outlook.com; Gao, Liming <gaoliming@byosoft.com.cn>; Howell, Stacy <stacy.howell@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Bi, Dandan <dandan.bi@intel.com>
Subject: RE: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Hi Sean,

The auto promotion of memory was only intended as a dev/debug feature to maximize platform boot without having to tune what memory is tested in PEI phase.

In my opinion, a production platform should never trigger any auto promotions of untested to tested memory, and part of production validation should make sure this event never occurs in any production boot scenarios.

The specific bug being fix here is that auto promotion was not symmetric across all memory allocation types.  It simply aligns this dev/debug feature.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> Sent: Monday, January 10, 2022 6:47 PM
> To: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; 
> Howell, Stacy <stacy.howell@intel.com>
> Cc: Bi, Dandan <dandan.bi@intel.com>
> Subject: Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE 
> Drivers to use untested memory
> 
> if this is auto promotion is happening in the core then what is the
> value of memory testing and tracking that state.   Is memory testing
> state a necessary feature of the Dxe Core?
> 
> 
> I think it makes more sense that if you platform wants to use a given 
> range your platform should either test it and/or mark it as tested.
> 
> OR
> 
> The dxe core should do away with the memory testing tracking.
> 
> 
> On most platforms i have seen in the past few years all memory is 
> marked as tested without doing any testing.  The only value in the 
> flag is keep the initial memory allocations in a given low range (below 4gb).
> 
> 
> 
> 
> On 1/10/2022 5:59 PM, gaoliming wrote:
> > Stacy:
> >    This fix covers the case with AllocateAddress allocation type. I 
> > agree this fix. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Stacy Howell
> >> 发送时间: 2022年1月8日 3:36
> >> 收件人: devel@edk2.groups.io
> >> 抄送: Stacy Howell <stacy.howell@intel.com>; Dandan Bi 
> >> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> >> 主题: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to 
> >> use untested memory
> >>
> >> REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
> >> CC: Dandan Bi <dandan.bi@intel.com>
> >> CC: Liming Gao <gaoliming@byosoft.com.cn>
> >>
> >> Updated CoreInternalAllocatePages() to call PromoteMemoryResource() 
> >> and re-attempt the allocation if unable to convert the specified 
> >> memory range
> >>
> >> Signed-off-by: Stacy Howell <stacy.howell@intel.com>
> >> ---
> >>   MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> index 47d4c5d92e..cc0b90ac0d 100644
> >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> @@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
> >>       Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> >>     }
> >>
> >> +  if (EFI_ERROR (Status)) {
> >> +    //
> >> +    // If requested memory region is unavailable it may be 
> >> + untested
> >> memory
> >> +    // Attempt to promote memory resources, then re-attempt the
> >> allocation
> >> +    //
> >> +    if (PromoteMemoryResource ()) {
> >> +      if (NeedGuard) {
> >> +        Status = CoreConvertPagesWithGuard (Start, NumberOfPages,
> >> MemoryType);
> >> +      } else {
> >> +        Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> >> +      }
> >> +    }
> >> +  }
> >> +
> >>   Done:
> >>     CoreReleaseMemoryLock ();
> >>
> >> --
> >> 2.32.0.windows.2
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 



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

Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by Howell, Stacy 1 year, 12 months ago

Hi all,
Are there any other comments or concerns regarding this patch?

Thanks,
Stacy

-----Original Message-----
From: Howell, Stacy 
Sent: Friday, January 14, 2022 4:13 PM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io; spbrogan@outlook.com; Gao, Liming <gaoliming@byosoft.com.cn>
Cc: Bi, Dandan <dandan.bi@intel.com>; Howell, Stacy <stacy.howell@intel.com>
Subject: RE: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Hi Sean,
Setting all memory as tested in PEI is a workaround for the issue that this patch addresses.  However, promoting all memory in PEI is not a workable solution for BIOSes that incorporate full memory testing functionality, as this relies on the tested flag to determine which memory regions to test.

This patch addresses a discrepancy in EDK2 core regarding how untested memory is treated for allocation by DXE drivers.  In the case where a DXE driver does not request a specific memory region DXE Core will promote untested memory if necessary to provide memory to the driver.  In the case where a DXE driver requests a specific memory range of untested memory, DXE Core will currently return an error instead of promoting untested memory to make the region available for the driver.

Thanks,
Stacy

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Tuesday, January 11, 2022 10:57 AM
To: devel@edk2.groups.io; spbrogan@outlook.com; Gao, Liming <gaoliming@byosoft.com.cn>; Howell, Stacy <stacy.howell@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Bi, Dandan <dandan.bi@intel.com>
Subject: RE: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Hi Sean,

The auto promotion of memory was only intended as a dev/debug feature to maximize platform boot without having to tune what memory is tested in PEI phase.

In my opinion, a production platform should never trigger any auto promotions of untested to tested memory, and part of production validation should make sure this event never occurs in any production boot scenarios.

The specific bug being fix here is that auto promotion was not symmetric across all memory allocation types.  It simply aligns this dev/debug feature.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> Sent: Monday, January 10, 2022 6:47 PM
> To: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>; 
> Howell, Stacy <stacy.howell@intel.com>
> Cc: Bi, Dandan <dandan.bi@intel.com>
> Subject: Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE 
> Drivers to use untested memory
> 
> if this is auto promotion is happening in the core then what is the
> value of memory testing and tracking that state.   Is memory testing
> state a necessary feature of the Dxe Core?
> 
> 
> I think it makes more sense that if you platform wants to use a given 
> range your platform should either test it and/or mark it as tested.
> 
> OR
> 
> The dxe core should do away with the memory testing tracking.
> 
> 
> On most platforms i have seen in the past few years all memory is 
> marked as tested without doing any testing.  The only value in the 
> flag is keep the initial memory allocations in a given low range (below 4gb).
> 
> 
> 
> 
> On 1/10/2022 5:59 PM, gaoliming wrote:
> > Stacy:
> >    This fix covers the case with AllocateAddress allocation type. I 
> > agree this fix. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> >
> > Thanks
> > Liming
> >> -----邮件原件-----
> >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Stacy Howell
> >> 发送时间: 2022年1月8日 3:36
> >> 收件人: devel@edk2.groups.io
> >> 抄送: Stacy Howell <stacy.howell@intel.com>; Dandan Bi 
> >> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> >> 主题: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to 
> >> use untested memory
> >>
> >> REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
> >> CC: Dandan Bi <dandan.bi@intel.com>
> >> CC: Liming Gao <gaoliming@byosoft.com.cn>
> >>
> >> Updated CoreInternalAllocatePages() to call PromoteMemoryResource() 
> >> and re-attempt the allocation if unable to convert the specified 
> >> memory range
> >>
> >> Signed-off-by: Stacy Howell <stacy.howell@intel.com>
> >> ---
> >>   MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
> >>   1 file changed, 14 insertions(+)
> >>
> >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> index 47d4c5d92e..cc0b90ac0d 100644
> >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> >> @@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
> >>       Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> >>     }
> >>
> >> +  if (EFI_ERROR (Status)) {
> >> +    //
> >> +    // If requested memory region is unavailable it may be 
> >> + untested
> >> memory
> >> +    // Attempt to promote memory resources, then re-attempt the
> >> allocation
> >> +    //
> >> +    if (PromoteMemoryResource ()) {
> >> +      if (NeedGuard) {
> >> +        Status = CoreConvertPagesWithGuard (Start, NumberOfPages,
> >> MemoryType);
> >> +      } else {
> >> +        Status = CoreConvertPages (Start, NumberOfPages, MemoryType);
> >> +      }
> >> +    }
> >> +  }
> >> +
> >>   Done:
> >>     CoreReleaseMemoryLock ();
> >>
> >> --
> >> 2.32.0.windows.2
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 



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

回复: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory

Posted by gaoliming via groups.io 1 year, 9 months ago

If no other objection, I will merge this patch next week. 

Thanks
Liming
> -----邮件原件-----
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Howell, Stacy
> 发送时间: 2022年4月25日 22:11
> 收件人: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; spbrogan@outlook.com; Gao, Liming
> <gaoliming@byosoft.com.cn>
> 抄送: Bi, Dandan <dandan.bi@intel.com>; Howell, Stacy
> <stacy.howell@intel.com>
> 主题: Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE
> Drivers to use untested memory
> 
> Hi all,
> Are there any other comments or concerns regarding this patch?
> 
> Thanks,
> Stacy
> 
> -----Original Message-----
> From: Howell, Stacy
> Sent: Friday, January 14, 2022 4:13 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io;
> spbrogan@outlook.com; Gao, Liming <gaoliming@byosoft.com.cn>
> Cc: Bi, Dandan <dandan.bi@intel.com>; Howell, Stacy
> <stacy.howell@intel.com>
> Subject: RE: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE
> Drivers to use untested memory
> 
> Hi Sean,
> Setting all memory as tested in PEI is a workaround for the issue that this
> patch addresses.  However, promoting all memory in PEI is not a workable
> solution for BIOSes that incorporate full memory testing functionality, as this
> relies on the tested flag to determine which memory regions to test.
> 
> This patch addresses a discrepancy in EDK2 core regarding how untested
> memory is treated for allocation by DXE drivers.  In the case where a DXE
> driver does not request a specific memory region DXE Core will promote
> untested memory if necessary to provide memory to the driver.  In the case
> where a DXE driver requests a specific memory range of untested memory,
> DXE Core will currently return an error instead of promoting untested memory
> to make the region available for the driver.
> 
> Thanks,
> Stacy
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Tuesday, January 11, 2022 10:57 AM
> To: devel@edk2.groups.io; spbrogan@outlook.com; Gao, Liming
> <gaoliming@byosoft.com.cn>; Howell, Stacy <stacy.howell@intel.com>;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Bi, Dandan <dandan.bi@intel.com>
> Subject: RE: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE
> Drivers to use untested memory
> 
> Hi Sean,
> 
> The auto promotion of memory was only intended as a dev/debug feature to
> maximize platform boot without having to tune what memory is tested in PEI
> phase.
> 
> In my opinion, a production platform should never trigger any auto
> promotions of untested to tested memory, and part of production validation
> should make sure this event never occurs in any production boot scenarios.
> 
> The specific bug being fix here is that auto promotion was not symmetric
> across all memory allocation types.  It simply aligns this dev/debug feature.
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean
> > Sent: Monday, January 10, 2022 6:47 PM
> > To: devel@edk2.groups.io; Gao, Liming <gaoliming@byosoft.com.cn>;
> > Howell, Stacy <stacy.howell@intel.com>
> > Cc: Bi, Dandan <dandan.bi@intel.com>
> > Subject: Re: 回复: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow
> DXE
> > Drivers to use untested memory
> >
> > if this is auto promotion is happening in the core then what is the
> > value of memory testing and tracking that state.   Is memory testing
> > state a necessary feature of the Dxe Core?
> >
> >
> > I think it makes more sense that if you platform wants to use a given
> > range your platform should either test it and/or mark it as tested.
> >
> > OR
> >
> > The dxe core should do away with the memory testing tracking.
> >
> >
> > On most platforms i have seen in the past few years all memory is
> > marked as tested without doing any testing.  The only value in the
> > flag is keep the initial memory allocations in a given low range (below 4gb).
> >
> >
> >
> >
> > On 1/10/2022 5:59 PM, gaoliming wrote:
> > > Stacy:
> > >    This fix covers the case with AllocateAddress allocation type. I
> > > agree this fix. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
> > >
> > > Thanks
> > > Liming
> > >> -----邮件原件-----
> > >> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Stacy
> Howell
> > >> 发送时间: 2022年1月8日 3:36
> > >> 收件人: devel@edk2.groups.io
> > >> 抄送: Stacy Howell <stacy.howell@intel.com>; Dandan Bi
> > >> <dandan.bi@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
> > >> 主题: [edk2-devel] [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers
> to
> > >> use untested memory
> > >>
> > >> REF: https://https://bugzilla.tianocore.org/show_bug.cgi?id=3795
> > >> CC: Dandan Bi <dandan.bi@intel.com>
> > >> CC: Liming Gao <gaoliming@byosoft.com.cn>
> > >>
> > >> Updated CoreInternalAllocatePages() to call PromoteMemoryResource()
> > >> and re-attempt the allocation if unable to convert the specified
> > >> memory range
> > >>
> > >> Signed-off-by: Stacy Howell <stacy.howell@intel.com>
> > >> ---
> > >>   MdeModulePkg/Core/Dxe/Mem/Page.c | 14 ++++++++++++++
> > >>   1 file changed, 14 insertions(+)
> > >>
> > >> diff --git a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> index 47d4c5d92e..cc0b90ac0d 100644
> > >> --- a/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> +++ b/MdeModulePkg/Core/Dxe/Mem/Page.c
> > >> @@ -1417,6 +1417,20 @@ CoreInternalAllocatePages (
> > >>       Status = CoreConvertPages (Start, NumberOfPages,
> MemoryType);
> > >>     }
> > >>
> > >> +  if (EFI_ERROR (Status)) {
> > >> +    //
> > >> +    // If requested memory region is unavailable it may be
> > >> + untested
> > >> memory
> > >> +    // Attempt to promote memory resources, then re-attempt the
> > >> allocation
> > >> +    //
> > >> +    if (PromoteMemoryResource ()) {
> > >> +      if (NeedGuard) {
> > >> +        Status = CoreConvertPagesWithGuard (Start,
> NumberOfPages,
> > >> MemoryType);
> > >> +      } else {
> > >> +        Status = CoreConvertPages (Start, NumberOfPages,
> MemoryType);
> > >> +      }
> > >> +    }
> > >> +  }
> > >> +
> > >>   Done:
> > >>     CoreReleaseMemoryLock ();
> > >>
> > >> --
> > >> 2.32.0.windows.2
> > >>
> > >>
> > >>
> > >>
> > >>
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> 
> 
> 
> 
> 





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