Re: [edk2] [Patch v3 05/23] OvmfPkg: Duplicate PlatformBdsLib to PlatformBootManagerLib

[Laszlo Ersek]

On 04/21/16 08:57, Ruiyu Ni wrote:
> It will be changed to build with MdeModulePkg/BDS.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1575 
> 
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  292 
>  .../PlatformBootManagerLib/PlatformBdsLib.inf  |   73 +
>  .../Library/PlatformBootManagerLib/PlatformData.c  |   51 +
>  .../Library/PlatformBootManagerLib/QemuKernel.c|  170 +++
>  5 files changed, 2161 insertions(+)
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformBdsLib.inf
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c

According to

  git show --find-copies-harder 7d4750d37fad464f6452190c971f9a7dc6b1ce3e

this creates an identical copy of "OvmfPkg/Library/PlatformBdsLib". I
recommend / request the following:

(1) In this patch,
- create a new FILE_GUID for the INF file immediately,
- update the Intel copyright notice as appropriate.

These changes are already present in patch #7 ("OvmfPkg/PlatformBds:
Follow PlatformBootManagerLib interfaces"), but in my opinion, the new
FILE_GUID and the copyright update belong here, not to patch #7.

(2) For the next two patches:

please squash patch #6 ("OvmfPkg/PlatformBds: Rename INF file") and #7
together.

(
Git handles patches well where a file is renamed and then immediately
modified -- just make sure rename detection is enabled in your git config:

[diff]
renames = copies
)

For the unified patch, the subject line should be

OvmfPkg/PlatformBootManagerLib: Follow PlatformBootManagerLib interfaces

(72 characters).

(3) For the rest of the patches: I believe their subject lines should
refer to "OvmfPkg/PlatformBootManagerLib", not "OvmfPkg/PlatformBds".

Thanks!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch v3 05/23] OvmfPkg: Duplicate PlatformBdsLib to PlatformBootManagerLib

[Ruiyu Ni]

It will be changed to build with MdeModulePkg/BDS.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jordan Justen 
Cc: Laszlo Ersek 
---
 .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1575 
 .../Library/PlatformBootManagerLib/BdsPlatform.h   |  292 
 .../PlatformBootManagerLib/PlatformBdsLib.inf  |   73 +
 .../Library/PlatformBootManagerLib/PlatformData.c  |   51 +
 .../Library/PlatformBootManagerLib/QemuKernel.c|  170 +++
 5 files changed, 2161 insertions(+)
 create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
 create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
 create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformBdsLib.inf
 create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
 create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
new file mode 100644
index 000..0bc02ba
--- /dev/null
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -0,0 +1,1575 @@
+/** @file
+  Platform BDS customizations.
+
+  Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "BdsPlatform.h"
+#include 
+#include 
+
+
+//
+// Global data
+//
+
+VOID  *mEfiDevPathNotifyReg;
+EFI_EVENT mEfiDevPathEvent;
+VOID  *mEmuVariableEventReg;
+EFI_EVENT mEmuVariableEvent;
+BOOLEAN   mDetectVgaOnly;
+UINT16mHostBridgeDevId;
+
+//
+// Table of host IRQs matching PCI IRQs A-D
+// (for configuring PCI Interrupt Line register)
+//
+CONST UINT8 PciHostIrqs[] = {
+  0x0a, 0x0a, 0x0b, 0x0b
+};
+
+//
+// Array Size macro
+//
+#define ARRAY_SIZE(array) (sizeof (array) / sizeof (array[0]))
+
+//
+// Type definitions
+//
+
+typedef
+EFI_STATUS
+(EFIAPI *PROTOCOL_INSTANCE_CALLBACK)(
+  IN EFI_HANDLE   Handle,
+  IN VOID *Instance,
+  IN VOID *Context
+  );
+
+/**
+  @param[in]  Handle - Handle of PCI device instance
+  @param[in]  PciIo - PCI IO protocol instance
+  @param[in]  Pci - PCI Header register block
+**/
+typedef
+EFI_STATUS
+(EFIAPI *VISIT_PCI_INSTANCE_CALLBACK)(
+  IN EFI_HANDLE   Handle,
+  IN EFI_PCI_IO_PROTOCOL  *PciIo,
+  IN PCI_TYPE00   *Pci
+  );
+
+
+//
+// Function prototypes
+//
+
+EFI_STATUS
+VisitAllInstancesOfProtocol (
+  IN EFI_GUID*Id,
+  IN PROTOCOL_INSTANCE_CALLBACK  CallBackFunction,
+  IN VOID*Context
+  );
+
+EFI_STATUS
+VisitAllPciInstancesOfProtocol (
+  IN VISIT_PCI_INSTANCE_CALLBACK CallBackFunction
+  );
+
+VOID
+InstallDevicePathCallback (
+  VOID
+  );
+
+//
+// BDS Platform Functions
+//
+VOID
+EFIAPI
+PlatformBdsInit (
+  VOID
+  )
+/*++
+
+Routine Description:
+
+  Platform Bds init. Incude the platform firmware vendor, revision
+  and so crc check.
+
+Arguments:
+
+Returns:
+
+  None.
+
+--*/
+{
+  DEBUG ((EFI_D_INFO, "PlatformBdsInit\n"));
+  InstallDevicePathCallback ();
+}
+
+
+EFI_STATUS
+EFIAPI
+ConnectRootBridge (
+  IN EFI_HANDLE  RootBridgeHandle,
+  IN VOID*Instance,
+  IN VOID*Context
+  )
+{
+  EFI_STATUS Status;
+
+  //
+  // Make the PCI bus driver connect the root bridge, non-recursively. This
+  // will produce a number of child handles with PciIo on them.
+  //
+  Status = gBS->ConnectController (
+  RootBridgeHandle, // ControllerHandle
+  NULL, // DriverImageHandle
+  NULL, // RemainingDevicePath -- produce all
+//   children
+  FALSE // Recursive
+  );
+  return Status;
+}
+
+
+EFI_STATUS
+PrepareLpcBridgeDevicePath (
+  IN EFI_HANDLEDeviceHandle
+  )
+/*++
+
+Routine Description:
+
+  Add IsaKeyboard to ConIn,
+  add IsaSerial to ConOut, ConIn, ErrOut.
+  LPC Bridge: 06 01 00
+
+Arguments:
+
+  DeviceHandle- Handle of PCIIO protocol.
+
+Returns:
+
+  EFI_SUCCESS - LPC bridge is added to ConOut, ConIn, and ErrOut.
+  EFI_STATUS  - No LPC bridge is added.
+
+--*/
+{
+  EFI_STATUSStatus;
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+  EFI_DEVICE_PATH_PROTOCOL  *TempDevicePath;
+  CHAR16*DevPathStr;
+
+  DevicePath = NULL;
+  Status = gBS->HandleProtocol (
+  DeviceHandle,
+