Re: [edk2] [PATCH v3 6/8] OvmfPkg: introduce PublishPeiMemory

Subject: Re: [edk2] [PATCH v3 6/8] OvmfPkg: introduce PublishPeiMemory

From: Wei Liu <wei.liu2@citrix.com>

To: Jordan Justen <jljusten@gmail.com>

Date: 2013-11-29 21:16:04

On Thu, Nov 28, 2013 at 08:31:11PM -0800, Jordan Justen wrote:
[...]
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                  Status;
> > +  EFI_PHYSICAL_ADDRESS        MemoryBase;
> > +  UINT64                      MemorySize;
> > +  UINT64                      LowerMemorySize;
> > +
> > +  LowerMemorySize = GetSystemMemorySizeBelow4gb ();
> 
> I'm going to say
> Reviewed-by: Jordan Justen 
> for patches 5 & 6, but I am not too happy with them.
> 
> For 5, I think maybe it would be nice to not require the details of
> 'XenLeaf' to leak out of Xen.c. I think XenDetect should return
> BOOLEAN, and store XenLeaf in a static global in Xen.c.
> 
> For 6, I think it is inconsistent that Xen continues to use CMOS here,
> but moves to the E820 tables otherwise. (GetSystemMemorySizeBelow4gb
> could have a different path for Xen, or maybe PublishPeiMemory could
> have an input parameter of MemoryLimit32Bit and get called by
> MemDetect too.)
> 
> But, these don't seem critical, so to reduce thrash for you I'll let
> you move forward with these patches as-is.
> 

Thanks for reviewing.

Wei.

------------------------------------------------------------------------------
Rapidly troubleshoot problems before they affect your business. Most IT 
organizations don't have a clear picture of how application performance 
affects their revenue. With AppDynamics, you get 100% visibility into your 
Java,.NET, & PHP application. Start your 15-day FREE TRIAL of AppDynamics Pro!
http://pubads.g.doubleclick.net/gampad/clk?id=84349351&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel