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

Sat Jul 16 03:24:28 UTC 2022

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

Thanks
Liming
> -----邮件原件-----
> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Howell, Stacy
> 发送时间: 2022年4月25日 22:11
> 收件人: Kinney, Michael D <michael.d.kinney at intel.com>;
> devel at edk2.groups.io; spbrogan at outlook.com; Gao, Liming
> <gaoliming at byosoft.com.cn>
> 抄送: Bi, Dandan <dandan.bi at intel.com>; Howell, Stacy
> <stacy.howell at 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 at intel.com>; devel at edk2.groups.io;
> spbrogan at outlook.com; Gao, Liming <gaoliming at byosoft.com.cn>
> Cc: Bi, Dandan <dandan.bi at intel.com>; Howell, Stacy
> <stacy.howell at 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 at intel.com>
> Sent: Tuesday, January 11, 2022 10:57 AM
> To: devel at edk2.groups.io; spbrogan at outlook.com; Gao, Liming
> <gaoliming at byosoft.com.cn>; Howell, Stacy <stacy.howell at intel.com>;
> Kinney, Michael D <michael.d.kinney at intel.com>
> Cc: Bi, Dandan <dandan.bi at 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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Sean
> > Sent: Monday, January 10, 2022 6:47 PM
> > To: devel at edk2.groups.io; Gao, Liming <gaoliming at byosoft.com.cn>;
> > Howell, Stacy <stacy.howell at intel.com>
> > Cc: Bi, Dandan <dandan.bi at 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 at byosoft.com.cn>
> > >
> > > Thanks
> > > Liming
> > >> -----邮件原件-----
> > >> 发件人: devel at edk2.groups.io <devel at edk2.groups.io> 代表 Stacy
> Howell
> > >> 发送时间: 2022年1月8日 3:36
> > >> 收件人: devel at edk2.groups.io
> > >> 抄送: Stacy Howell <stacy.howell at intel.com>; Dandan Bi
> > >> <dandan.bi at intel.com>; Liming Gao <gaoliming at 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 at intel.com>
> > >> CC: Liming Gao <gaoliming at 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 at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-