public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"spbrogan@outlook.com" <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
Date: Tue, 11 Jan 2022 15:57:22 +0000	[thread overview]
Message-ID: <CO1PR11MB49292C9A4F2277702C38B253D2519@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <SA1PR19MB491137A198F64B571163838BC8519@SA1PR19MB4911.namprd19.prod.outlook.com>

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
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> >
> >
> >
> >
> >
> 
> 
> 
> 


  parent reply	other threads:[~2022-01-11 15:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-07 19:36 [PATCH] MdeModulePkg\CoreDxe: Allow DXE Drivers to use untested memory Stacy Howell
2022-01-11  1:59 ` 回复: [edk2-devel] " gaoliming
2022-01-11  2:47   ` Sean
2022-01-11  3:08     ` 回复: " gaoliming
2022-01-11 15:57     ` Michael D Kinney [this message]
2022-01-14 21:12       ` Howell, Stacy
2022-04-25 14:10         ` Howell, Stacy
2022-07-16  3:24           ` 回复: " gaoliming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO1PR11MB49292C9A4F2277702C38B253D2519@CO1PR11MB4929.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox