Ray,

On 04/18/16 08:43, Ruiyu Ni wrote:
> The patch serials creates a flag USE_OLD_BDS and by default the value
> of the flag is FALSE so that the new MdeModulePkg/BDS is used.
> User can define USE_OLD_BDS as TRUE to force to use IntelFrameworkModulePkg
> /BDS.

Thanks for looking into this again.

How much time do you have for working on this?

Namely, the series contains barely any documentation, while patch #1
adds 2410 lines, and patch #2 adds 3072 lines. That this is practically
unreviewable.

It is also not explained how this version differs from v1
<http://thread.gmane.org/gmane.comp.bios.edk2.devel/759>, and/or how my
v1 comments have been addressed.

For example, while reviewing that version, I asked (as point (10)) for
the BmGetLoadOptionBuffer() function to be made available to platform
BDS code. I don't see it happening here.

On one hand, we (Jordan and I) shouldn't have to reconstruct your
thinking from the code in these huge patches; your thinking and work
process should be laid out in small patches and detailed commit messages.

On the other hand, you are doing us a favor by writing the conversion
for us.

So, I'm not sure what to say. I don't want to review this series as-is
(let alone accept it unreviewed). However, I don't know how much time
you have to iterate on it. (I expect quite a few iterations, for a
series this size.)

Perhaps, we could do the following: I could start reading through your
work slowly, and factor out as many small patches as possible. Then we'd
end up with a series where you and I share authorship on a bunch of
patches, and Jordan could review it.

What do you think?

Thanks
Laszlo

> 
> Ruiyu Ni (3):
>   OvmfPkg: Create QemuNewBootOrderLib to work with UefiBootManagerLib
>   OvmfPkg: Create PlatformBootManagerLib to work with UefiBootManagerLib
>   OvmfPkg: Use MdeModulePkg/BDS
> 
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1417 ++++++++++++++
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  246 +++
>  .../Library/PlatformBootManagerLib/MemoryTest.c    |  450 +++++
>  .../PlatformBootManagerLib.inf                     |   81 +
>  .../Library/PlatformBootManagerLib/PlatformData.c  |   35 +
>  .../Library/PlatformBootManagerLib/QemuKernel.c    |  170 ++
>  OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c |  673 +++++++
>  OvmfPkg/Library/PlatformBootManagerLib/Strings.uni |  Bin 0 -> 3658 bytes
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.c  |  313 +++
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.h  |   40 +
>  .../Library/QemuNewBootOrderLib/QemuBootOrderLib.c | 1989 
> ++++++++++++++++++++
>  .../QemuNewBootOrderLib/QemuBootOrderLib.inf       |   68 +
>  OvmfPkg/OvmfPkgIa32.dsc                            |   40 +-
>  OvmfPkg/OvmfPkgIa32.fdf                            |    5 +
>  OvmfPkg/OvmfPkgIa32X64.dsc                         |   42 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf                         |    5 +
>  OvmfPkg/OvmfPkgX64.dsc                             |   40 +-
>  OvmfPkg/OvmfPkgX64.fdf                             |    5 +
>  18 files changed, 5603 insertions(+), 16 deletions(-)
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/MemoryTest.c
>  create mode 100644 
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/Strings.uni
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.c
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.h
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.inf



_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to