public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
@ 2019-03-25  5:28 Hao Wu
  2019-03-25  5:28 ` [PATCH v2 1/3] OvmfPkg: Drop the ISA Floppy device support Hao Wu
                   ` (4 more replies)
  0 siblings, 5 replies; 28+ messages in thread
From: Hao Wu @ 2019-03-25  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Ray Ni

The series is also available at:
https://github.com/hwu25/edk2/tree/ovmf_siobus_v2

V2 changes:
* Introduce a static build flag 'USE_LEGACY_ISA_STACK' in OVMF DSC files
  for users to select between the ISA driver stacks.
* V1 patch 2/2 is split into 2 patches in V2. The first one will add the
  new OVMF SioBusDxe driver and list it in the DSC files. Then second one
  will add the whole new ISA stack in DSC/FDF files.


V1 history:

This series will update the OVMF to stop using the ISA drivers within
IntelFrameworkModulePkg.

As the replacement, a new OVMF Super I/O bus driver has been add which
will install the Super I/O protocol for ISA serial and PS2 keyboard
devices. By doing so, these devices can be managed by:

  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf

respectively.


Tests done:
A. GCC5 & VS2015x86 tool chains build pass
B. Launch QEMU (2.4.50, Windows) with command:
   > qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -serial file:1.txt -serial file:2.txt

   Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under Shell
   using command 'devtree';

   Both the serials and PS2 keyboard are working fine;

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>


Hao Wu (3):
  OvmfPkg: Drop the ISA Floppy device support
  OvmfPkg: Add an Super IO bus driver
  OvmfPkg: Add a build flag to select ISA driver stack

 OvmfPkg/OvmfPkgIa32.dsc           |  10 +-
 OvmfPkg/OvmfPkgIa32X64.dsc        |  10 +-
 OvmfPkg/OvmfPkgX64.dsc            |  10 +-
 OvmfPkg/OvmfPkgIa32.fdf           |  21 +-
 OvmfPkg/OvmfPkgIa32X64.fdf        |  21 +-
 OvmfPkg/OvmfPkgX64.fdf            |  21 +-
 OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
 OvmfPkg/SioBusDxe/SioBusDxe.h     | 332 +++++++++++
 OvmfPkg/SioBusDxe/SioService.h    | 221 +++++++
 OvmfPkg/SioBusDxe/ComponentName.c | 167 ++++++
 OvmfPkg/SioBusDxe/SioBusDxe.c     | 622 ++++++++++++++++++++
 OvmfPkg/SioBusDxe/SioService.c    | 405 +++++++++++++
 OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
 13 files changed, 1885 insertions(+), 30 deletions(-)
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.inf
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.h
 create mode 100644 OvmfPkg/SioBusDxe/SioService.h
 create mode 100644 OvmfPkg/SioBusDxe/ComponentName.c
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.c
 create mode 100644 OvmfPkg/SioBusDxe/SioService.c
 create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.uni

-- 
2.12.0.windows.1



^ permalink raw reply	[flat|nested] 28+ messages in thread

* [PATCH v2 1/3] OvmfPkg: Drop the ISA Floppy device support
  2019-03-25  5:28 [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Hao Wu
@ 2019-03-25  5:28 ` Hao Wu
  2019-03-25 10:42   ` Laszlo Ersek
  2019-03-25  5:28 ` [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver Hao Wu
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 28+ messages in thread
From: Hao Wu @ 2019-03-25  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Ray Ni

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495

There is a plan to remove the IntelFrameworkModulePkg:
https://bugzilla.tianocore.org/show_bug.cgi?id=1605

And for driver:
IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe

This patch proposes to drop the ISA Floppy device support in OVMF.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 3 +--
 OvmfPkg/OvmfPkgIa32X64.dsc | 3 +--
 OvmfPkg/OvmfPkgX64.dsc     | 3 +--
 OvmfPkg/OvmfPkgIa32.fdf    | 3 +--
 OvmfPkg/OvmfPkgIa32X64.fdf | 3 +--
 OvmfPkg/OvmfPkgX64.fdf     | 3 +--
 6 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 5b885590b2..1710ab5a88 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -756,7 +756,6 @@
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index bbf0853ee6..5bceef3116 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -765,7 +765,6 @@
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d81460f520..3f5d948dbb 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
 #
-#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -763,7 +763,6 @@
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 4999403ad7..54d7f06a70 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -273,7 +273,6 @@ INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
 !endif
 
 INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index d0cc107928..7519b53a9b 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -274,7 +274,6 @@ INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
 !endif
 
 INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index d0cc107928..7519b53a9b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -1,7 +1,7 @@
 ## @file
 #  Open Virtual Machine Firmware: FDF
 #
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #
 #  This program and the accompanying materials
@@ -274,7 +274,6 @@ INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
 !endif
 
 INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
-- 
2.12.0.windows.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25  5:28 [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Hao Wu
  2019-03-25  5:28 ` [PATCH v2 1/3] OvmfPkg: Drop the ISA Floppy device support Hao Wu
@ 2019-03-25  5:28 ` Hao Wu
  2019-03-25 11:22   ` Laszlo Ersek
  2019-03-25 12:00   ` Laszlo Ersek
  2019-03-25  5:28 ` [PATCH v2 3/3] OvmfPkg: Add a build flag to select ISA driver stack Hao Wu
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 28+ messages in thread
From: Hao Wu @ 2019-03-25  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Ray Ni

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495

There is a plan to remove the IntelFrameworkModulePkg:
https://bugzilla.tianocore.org/show_bug.cgi?id=1605

This patch will a new OVMF Super I/O bus driver which will create the
below child devices:

* COM 1 UART
* COM 2 UART
* PS/2 Keyboard

and installs the Super I/O Protocol on them.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 OvmfPkg/OvmfPkgIa32.dsc           |   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc        |   1 +
 OvmfPkg/OvmfPkgX64.dsc            |   1 +
 OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
 OvmfPkg/SioBusDxe/SioBusDxe.h     | 332 +++++++++++
 OvmfPkg/SioBusDxe/SioService.h    | 221 +++++++
 OvmfPkg/SioBusDxe/ComponentName.c | 167 ++++++
 OvmfPkg/SioBusDxe/SioBusDxe.c     | 622 ++++++++++++++++++++
 OvmfPkg/SioBusDxe/SioService.c    | 405 +++++++++++++
 OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
 10 files changed, 1825 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 1710ab5a88..3be0314146 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -752,6 +752,7 @@
   #
   # ISA Support
   #
+  OvmfPkg/SioBusDxe/SioBusDxe.inf
   PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 5bceef3116..3b85c2e6af 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -761,6 +761,7 @@
   #
   # ISA Support
   #
+  OvmfPkg/SioBusDxe/SioBusDxe.inf
   PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3f5d948dbb..104b2e79a5 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -759,6 +759,7 @@
   #
   # ISA Support
   #
+  OvmfPkg/SioBusDxe/SioBusDxe.inf
   PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
diff --git a/OvmfPkg/SioBusDxe/SioBusDxe.inf b/OvmfPkg/SioBusDxe/SioBusDxe.inf
new file mode 100644
index 0000000000..5c462f1a8c
--- /dev/null
+++ b/OvmfPkg/SioBusDxe/SioBusDxe.inf
@@ -0,0 +1,54 @@
+## @file
+#  The SioBusDxe driver is used to create child devices on the ISA bus and
+#  installs the Super I/O protocols on them.
+#
+#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+#
+#  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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = SioBusDxe
+  MODULE_UNI_FILE                = SioBusDxe.uni
+  FILE_GUID                      = 864E1CA8-85EB-4D63-9DCC-6E0FC90FFD55
+  MODULE_TYPE                    = UEFI_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = SioBusDxeDriverEntryPoint
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 EBC
+#
+
+[Sources]
+  ComponentName.c
+  SioService.c
+  SioService.h
+  SioBusDxe.c
+  SioBusDxe.h
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  UefiDriverEntryPoint
+  UefiLib
+  UefiBootServicesTableLib
+  DebugLib
+  ReportStatusCodeLib
+  MemoryAllocationLib
+  BaseMemoryLib
+  DevicePathLib
+
+[Protocols]
+  gEfiPciIoProtocolGuid    ## TO_START
+  gEfiSioProtocolGuid      ## BY_START
diff --git a/OvmfPkg/SioBusDxe/SioBusDxe.h b/OvmfPkg/SioBusDxe/SioBusDxe.h
new file mode 100644
index 0000000000..1455c48f63
--- /dev/null
+++ b/OvmfPkg/SioBusDxe/SioBusDxe.h
@@ -0,0 +1,332 @@
+/** @file
+  The SioBusDxe driver is used to create child devices on the ISA bus and
+  installs the Super I/O protocols on them.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+  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.
+
+**/
+
+#ifndef __SIO_BUS_DXE_H__
+#define __SIO_BUS_DXE_H__
+
+#include <Uefi.h>
+
+#include <IndustryStandard/Pci.h>
+
+#include <Protocol/PciIo.h>
+#include <Protocol/SuperIo.h>
+
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/DebugLib.h>
+#include <Library/ReportStatusCodeLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DevicePathLib.h>
+
+#include "SioService.h"
+
+//
+// SIO Bus driver private data structure
+//
+typedef struct {
+  EFI_PCI_IO_PROTOCOL    *PciIo;
+  UINT64                 OriginalAttributes;
+} SIO_BUS_DRIVER_PRIVATE_DATA;
+
+
+//
+// Global Variables
+//
+extern EFI_COMPONENT_NAME_PROTOCOL  gSioBusComponentName;
+extern EFI_COMPONENT_NAME2_PROTOCOL gSioBusComponentName2;
+
+
+//
+// EFI Component Name Functions
+//
+
+/**
+  Retrieves a Unicode string that is the user readable name of the driver.
+
+  This function retrieves the user readable name of a driver in the form of a
+  Unicode string. If the driver specified by This has a user readable name in
+  the language specified by Language, then a pointer to the driver name is
+  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
+  by This does not support the language specified by Language, then
+  EFI_UNSUPPORTED is returned.
+
+  @param[in]  This              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL
+                                or EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param[in]  Language          A pointer to a Null-terminated ASCII string
+                                array indicating the language. This is the
+                                language of the driver name that the caller is
+                                requesting, and it must match one of the
+                                languages specified in SupportedLanguages. The
+                                number of languages supported by a driver is up
+                                to the driver writer. Language is specified
+                                in RFC 4646 or ISO 639-2 language code format.
+  @param[out] DriverName        A pointer to the Unicode string to return. This
+                                Unicode string is the name of the driver
+                                specified by This in the language specified by
+                                Language.
+
+  @retval EFI_SUCCESS           The Unicode string for the Driver specified by
+                                This and the language specified by Language was
+                                returned in DriverName.
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+  @retval EFI_UNSUPPORTED       The driver specified by This does not support
+                                the language specified by Language.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusComponentNameGetDriverName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL    *This,
+  IN  CHAR8                          *Language,
+  OUT CHAR16                         **DriverName
+  );
+
+/**
+  Retrieves a Unicode string that is the user readable name of the controller
+  that is being managed by a driver.
+
+  This function retrieves the user readable name of the controller specified by
+  ControllerHandle and ChildHandle in the form of a Unicode string. If the
+  driver specified by This has a user readable name in the language specified
+  by Language, then a pointer to the controller name is returned in
+  ControllerName, and EFI_SUCCESS is returned.  If the driver specified by This
+  is not currently managing the controller specified by ControllerHandle and
+  ChildHandle, then EFI_UNSUPPORTED is returned.  If the driver specified by
+  This does not support the language specified by Language, then
+  EFI_UNSUPPORTED is returned.
+
+  @param[in]  This              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL
+                                or EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param[in]  ControllerHandle  The handle of a controller that the driver
+                                specified by This is managing.  This handle
+                                specifies the controller whose name is to be
+                                returned.
+  @param[in]  ChildHandle       The handle of the child controller to retrieve
+                                the name of.  This is an optional parameter
+                                that may be NULL.  It will be NULL for device
+                                drivers.  It will also be NULL for a bus
+                                drivers that wish to retrieve the name of the
+                                bus controller.  It will not be NULL for a bus
+                                driver that wishes to retrieve the name of a
+                                child controller.
+  @param[in]  Language          A pointer to a Null-terminated ASCII string
+                                array indicating the language.  This is the
+                                language of the driver name that the caller is
+                                requesting, and it must match one of the
+                                languages specified in SupportedLanguages. The
+                                number of languages supported by a driver is up
+                                to the driver writer. Language is specified in
+                                RFC 4646 or ISO 639-2 language code format.
+  @param[out] ControllerName    A pointer to the Unicode string to return.
+                                This Unicode string is the name of the
+                                controller specified by ControllerHandle and
+                                ChildHandle in the language specified by
+                                Language from the point of view of the driver
+                                specified by This.
+
+  @retval EFI_SUCCESS           The Unicode string for the user readable name
+                                in the language specified by Language for the
+                                driver specified by This was returned in
+                                DriverName.
+  @retval EFI_INVALID_PARAMETER ControllerHandle is NULL.
+  @retval EFI_INVALID_PARAMETER ChildHandle is not NULL and it is not a valid
+                                EFI_HANDLE.
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+  @retval EFI_INVALID_PARAMETER ControllerName is NULL.
+  @retval EFI_UNSUPPORTED       The driver specified by This is not currently
+                                managing the controller specified by
+                                ControllerHandle and ChildHandle.
+  @retval EFI_UNSUPPORTED       The driver specified by This does not support
+                                the language specified by Language.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusComponentNameGetControllerName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL    *This,
+  IN  EFI_HANDLE                     ControllerHandle,
+  IN  EFI_HANDLE                     ChildHandle        OPTIONAL,
+  IN  CHAR8                          *Language,
+  OUT CHAR16                         **ControllerName
+  );
+
+
+//
+// Driver Binding Protocol interfaces
+//
+
+/**
+  Tests to see if this driver supports a given controller. If a child device is
+  provided, it further tests to see if this driver supports creating a handle
+  for the specified child device.
+
+  This function checks to see if the driver specified by This supports the
+  device specified by ControllerHandle. Drivers will typically use the device
+  path attached to ControllerHandle and/or the services from the bus I/O
+  abstraction attached to ControllerHandle to determine if the driver supports
+  ControllerHandle. This function may be called many times during platform
+  initialization. In order to reduce boot times, the tests performed by this
+  function must be very small, and take as little time as possible to execute.
+  This function must not change the state of any hardware devices, and this
+  function must be aware that the device specified by ControllerHandle may
+  already be managed by the same driver or a different driver. This function
+  must match its calls to AllocatePages() with FreePages(), AllocatePool() with
+  FreePool(), and OpenProtocol() with CloseProtocol(). Since ControllerHandle
+  may have been previously started by the same driver, if a protocol is already
+  in the opened state, then it must not be closed with CloseProtocol(). This is
+  required to guarantee the state of ControllerHandle is not modified by this
+  function.
+
+  @param[in]  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL
+                                   instance.
+  @param[in]  ControllerHandle     The handle of the controller to test. This
+                                   handle must support a protocol interface
+                                   that supplies an I/O abstraction to the
+                                   driver.
+  @param[in]  RemainingDevicePath  A pointer to the remaining portion of a
+                                   device path.  This parameter is ignored by
+                                   device drivers, and is optional for bus
+                                   drivers. For bus drivers, if this parameter
+                                   is not NULL, then the bus driver must
+                                   determine if the bus controller specified by
+                                   ControllerHandle and the child controller
+                                   specified by RemainingDevicePath are both
+                                   supported by this bus driver.
+
+  @retval EFI_SUCCESS              The device specified by ControllerHandle and
+                                   RemainingDevicePath is supported by the
+                                   driver specified by This.
+  @retval EFI_ALREADY_STARTED      The device specified by ControllerHandle and
+                                   RemainingDevicePath is already being managed
+                                   by the driver specified by This.
+  @retval EFI_ACCESS_DENIED        The device specified by ControllerHandle and
+                                   RemainingDevicePath is already being managed
+                                   by a different driver or an application that
+                                   requires exclusive access.
+  @retval EFI_UNSUPPORTED          The device specified by ControllerHandle and
+                                   RemainingDevicePath is not supported by the
+                                   driver specified by This.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusDriverBindingSupported (
+  IN EFI_DRIVER_BINDING_PROTOCOL    *This,
+  IN EFI_HANDLE                     Controller,
+  IN EFI_DEVICE_PATH_PROTOCOL       *RemainingDevicePath
+  );
+
+/**
+  Starts a device controller or a bus controller.
+
+  The Start() function is designed to be invoked from the EFI boot service
+  ConnectController(). As a result, much of the error checking on the
+  parameters to Start() has been moved into this common boot service. It is
+  legal to call Start() from other locations, but the following calling
+  restrictions must be followed or the system behavior will not be
+  deterministic.
+  1. ControllerHandle must be a valid EFI_HANDLE.
+  2. If RemainingDevicePath is not NULL, then it must be a pointer to a
+     naturally aligned EFI_DEVICE_PATH_PROTOCOL.
+  3. Prior to calling Start(), the Supported() function for the driver
+     specified by This must have been called with the same calling parameters,
+     and Supported() must have returned EFI_SUCCESS.
+
+  @param[in]  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL
+                                   instance.
+  @param[in]  ControllerHandle     The handle of the controller to start. This
+                                   handle must support a protocol interface
+                                   that supplies an I/O abstraction to the
+                                   driver.
+  @param[in]  RemainingDevicePath  A pointer to the remaining portion of a
+                                   device path.  This parameter is ignored by
+                                   device drivers, and is optional for bus
+                                   drivers. For a bus driver, if this parameter
+                                   is NULL, then handles for all the children
+                                   of Controller are created by this driver. If
+                                   this parameter is not NULL and the first
+                                   Device Path Node is not the End of Device
+                                   Path Node, then only the handle for the
+                                   child device specified by the first Device
+                                   Path Node of RemainingDevicePath is created
+                                   by this driver. If the first Device Path
+                                   Node of RemainingDevicePath is the End of
+                                   Device Path Node, no child handle is created
+                                   by this driver.
+
+  @retval EFI_SUCCESS              The device was started.
+  @retval EFI_DEVICE_ERROR         The device could not be started due to a
+                                   device error.
+  @retval EFI_OUT_OF_RESOURCES     The request could not be completed due to a
+                                   lack of resources.
+  @retval Others                   The driver failded to start the device.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusDriverBindingStart (
+  IN EFI_DRIVER_BINDING_PROTOCOL    *This,
+  IN EFI_HANDLE                     Controller,
+  IN EFI_DEVICE_PATH_PROTOCOL       *RemainingDevicePath
+  );
+
+/**
+  Stops a device controller or a bus controller.
+
+  The Stop() function is designed to be invoked from the EFI boot service
+  DisconnectController(). As a result, much of the error checking on the
+  parameters to Stop() has been moved into this common boot service. It is
+  legal to call Stop() from other locations, but the following calling
+  restrictions must be followed or the system behavior will not be
+  deterministic.
+  1. ControllerHandle must be a valid EFI_HANDLE that was used on a previous
+     call to this same driver's Start() function.
+  2. The first NumberOfChildren handles of ChildHandleBuffer must all be a
+     valid EFI_HANDLE. In addition, all of these handles must have been created
+     in this driver's Start() function, and the Start() function must have
+     called OpenProtocol() on ControllerHandle with an Attribute of
+     EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER.
+
+  @param[in]  This               A pointer to the EFI_DRIVER_BINDING_PROTOCOL
+                                 instance.
+  @param[in]  ControllerHandle   A handle to the device being stopped. The
+                                 handle must support a bus specific I/O
+                                 protocol for the driver to use to stop the
+                                 device.
+  @param[in]  NumberOfChildren   The number of child device handles in
+                                 ChildHandleBuffer.
+  @param[in]  ChildHandleBuffer  An array of child handles to be freed. May be
+                                 NULL if NumberOfChildren is 0.
+
+  @retval EFI_SUCCESS            The device was stopped.
+  @retval EFI_DEVICE_ERROR       The device could not be stopped due to a
+                                 device error.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusDriverBindingStop (
+  IN  EFI_DRIVER_BINDING_PROTOCOL    *This,
+  IN  EFI_HANDLE                     Controller,
+  IN  UINTN                          NumberOfChildren,
+  IN  EFI_HANDLE                     *ChildHandleBuffer
+  );
+
+#endif  // __SIO_BUS_DXE_H__
diff --git a/OvmfPkg/SioBusDxe/SioService.h b/OvmfPkg/SioBusDxe/SioService.h
new file mode 100644
index 0000000000..758703dd89
--- /dev/null
+++ b/OvmfPkg/SioBusDxe/SioService.h
@@ -0,0 +1,221 @@
+/** @file
+  The SioBusDxe driver is used to create child devices on the ISA bus and
+  installs the Super I/O protocols on them.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+  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.
+
+**/
+
+#ifndef __SIO_SERVICE_H__
+#define __SIO_SERVICE_H__
+
+#pragma pack(1)
+
+typedef struct {
+  EFI_ACPI_FIXED_LOCATION_IO_PORT_DESCRIPTOR  Io;
+  EFI_ACPI_END_TAG_DESCRIPTOR                 End;
+} SIO_RESOURCES_IO;
+
+#pragma pack()
+
+typedef struct {
+  UINT32                      Hid;
+  UINT32                      Uid;
+  ACPI_RESOURCE_HEADER_PTR    Resources;
+} SIO_DEVICE_INFO;
+
+//
+// SIO device private data structure
+//
+typedef struct {
+  UINT32                      Signature;
+  EFI_HANDLE                  Handle;
+  EFI_PCI_IO_PROTOCOL         *PciIo;
+  EFI_DEVICE_PATH_PROTOCOL    *DevicePath;
+
+  EFI_SIO_PROTOCOL            Sio;
+  UINT32                      DeviceIndex;
+} SIO_DEV;
+#define SIO_DEV_SIGNATURE      SIGNATURE_32 ('S', 'I', 'O', 'D')
+#define SIO_DEV_FROM_SIO(a)    CR (a, SIO_DEV, Sio, SIO_DEV_SIGNATURE)
+
+
+//
+// Super I/O Protocol interfaces
+//
+
+/**
+  Provides a low level access to the registers for the Super I/O.
+
+  @param[in]     This           Indicates a pointer to the calling context.
+  @param[in]     Write          Specifies the type of the register operation.
+                                If this parameter is TRUE, Value is interpreted
+                                as an input parameter and the operation is a
+                                register write. If this parameter is FALSE,
+                                Value is interpreted as an output parameter and
+                                the operation is a register read.
+  @param[in]     ExitCfgMode    Exit Configuration Mode Indicator. If this
+                                parameter is set to TRUE, the Super I/O driver
+                                will turn off configuration mode of the Super
+                                I/O prior to returning from this function. If
+                                this parameter is set to FALSE, the Super I/O
+                                driver will leave Super I/O in the
+                                configuration mode. The Super I/O driver must
+                                track the current state of the Super I/O and
+                                enable the configuration mode of Super I/O if
+                                necessary prior to register access.
+  @param[in]     Register       Register number.
+  @param[in,out] Value          If Write is TRUE, Value is a pointer to the
+                                buffer containing the byte of data to be
+                                written to the Super I/O register. If Write is
+                                FALSE, Value is a pointer to the destination
+                                buffer for the byte of data to be read from the
+                                Super I/O register.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER The Value is NULL.
+  @retval EFI_INVALID_PARAMETER Invalid Register number.
+
+**/
+EFI_STATUS
+EFIAPI
+SioRegisterAccess (
+  IN CONST EFI_SIO_PROTOCOL    *This,
+  IN       BOOLEAN             Write,
+  IN       BOOLEAN             ExitCfgMode,
+  IN       UINT8               Register,
+  IN OUT   UINT8               *Value
+  );
+
+/**
+  Provides an interface to get a list of the current resources consumed by the
+  device in the ACPI Resource Descriptor format.
+
+  GetResources() returns a list of resources currently consumed by the device.
+  The ResourceList is a pointer to the buffer containing resource descriptors
+  for the device. The descriptors are in the format of Small or Large ACPI
+  resource descriptor as defined by ACPI specification (2.0 & 3.0). The buffer
+  of resource descriptors is terminated with the 'End tag' resource descriptor.
+
+  @param[in]  This              Indicates a pointer to the calling context.
+  @param[out] ResourceList      A pointer to an ACPI resource descriptor list
+                                that defines the current resources used by the
+                                device.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER ResourceList is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+SioGetResources (
+  IN CONST EFI_SIO_PROTOCOL            *This,
+  OUT      ACPI_RESOURCE_HEADER_PTR    *ResourceList
+  );
+
+/**
+  Sets the resources for the device.
+
+  @param[in] This               Indicates a pointer to the calling context.
+  @param[in] ResourceList       Pointer to the ACPI resource descriptor list.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER ResourceList is invalid.
+  @retval EFI_ACCESS_DENIED     Some of the resources in ResourceList are in
+                                use.
+
+**/
+EFI_STATUS
+EFIAPI
+SioSetResources (
+  IN CONST EFI_SIO_PROTOCOL            *This,
+  IN       ACPI_RESOURCE_HEADER_PTR    ResourceList
+  );
+
+/**
+  Provides a collection of resource descriptor lists. Each resource descriptor
+  list in the collection defines a combination of resources that can
+  potentially be used by the device.
+
+  @param[in]  This                  Indicates a pointer to the calling context.
+  @param[out] ResourceCollection    Collection of the resource descriptor
+                                    lists.
+
+  @retval EFI_SUCCESS               The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER     ResourceCollection is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+SioPossibleResources (
+  IN CONST EFI_SIO_PROTOCOL            *This,
+  OUT      ACPI_RESOURCE_HEADER_PTR    *ResourceCollection
+  );
+
+/**
+  Provides an interface for a table based programming of the Super I/O
+  registers.
+
+  The Modify() function provides an interface for table based programming of
+  the Super I/O registers. This function can be used to perform programming of
+  multiple Super I/O registers with a single function call. For each table
+  entry, the Register is read, its content is bitwise ANDed with AndMask, and
+  then ORed with OrMask before being written back to the Register. The Super
+  I/O driver must track the current state of the Super I/O and enable the
+  configuration mode of Super I/O if necessary prior to table processing. Once
+  the table is processed, the Super I/O device has to be returned to the
+  original state.
+
+  @param[in] This               Indicates a pointer to the calling context.
+  @param[in] Command            A pointer to an array of NumberOfCommands
+                                EFI_SIO_REGISTER_MODIFY structures. Each
+                                structure specifies a single Super I/O register
+                                modify operation.
+  @param[in] NumberOfCommands   Number of elements in the Command array.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER Command is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+SioModify (
+  IN CONST EFI_SIO_PROTOCOL           *This,
+  IN CONST EFI_SIO_REGISTER_MODIFY    *Command,
+  IN       UINTN                      NumberOfCommands
+  );
+
+//
+// Internal functions
+//
+
+/**
+  Create all the ISA child devices on the ISA bus controller (PCI to ISA
+  bridge).
+
+  @param[in] This              The EFI_DRIVER_BINDING_PROTOCOL instance.
+  @param[in] Controller        The handle of ISA bus controller.
+  @param[in] PciIo             The pointer to the PCI protocol.
+  @param[in] ParentDevicePath  Device path of the ISA bus controller.
+
+  @retval The number of child device that is successfully created.
+
+**/
+UINT32
+SioCreateAllChildDevices (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN EFI_HANDLE                   Controller,
+  IN EFI_PCI_IO_PROTOCOL          *PciIo,
+  IN EFI_DEVICE_PATH_PROTOCOL     *ParentDevicePath
+  );
+
+#endif  // __SIO_SERVICE_H__
diff --git a/OvmfPkg/SioBusDxe/ComponentName.c b/OvmfPkg/SioBusDxe/ComponentName.c
new file mode 100644
index 0000000000..668a2c6a7a
--- /dev/null
+++ b/OvmfPkg/SioBusDxe/ComponentName.c
@@ -0,0 +1,167 @@
+/** @file
+  UEFI Component Name(2) protocol implementation for SioBusDxe driver.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+  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 "SioBusDxe.h"
+
+//
+// Driver name table
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE mSioBusDriverNameTable[] = {
+  { "eng;en", L"OVMF Sio Bus Driver" },
+  { NULL , NULL }
+};
+
+//
+// EFI Component Name Protocol
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME_PROTOCOL  gSioBusComponentName = {
+  SioBusComponentNameGetDriverName,
+  SioBusComponentNameGetControllerName,
+  "eng"
+};
+
+//
+// EFI Component Name 2 Protocol
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME2_PROTOCOL gSioBusComponentName2 = {
+  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME) SioBusComponentNameGetDriverName,
+  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME) SioBusComponentNameGetControllerName,
+  "en"
+};
+
+
+/**
+  Retrieves a Unicode string that is the user readable name of the driver.
+
+  This function retrieves the user readable name of a driver in the form of a
+  Unicode string. If the driver specified by This has a user readable name in
+  the language specified by Language, then a pointer to the driver name is
+  returned in DriverName, and EFI_SUCCESS is returned. If the driver specified
+  by This does not support the language specified by Language, then
+  EFI_UNSUPPORTED is returned.
+
+  @param[in]  This              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL
+                                or EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param[in]  Language          A pointer to a Null-terminated ASCII string
+                                array indicating the language. This is the
+                                language of the driver name that the caller is
+                                requesting, and it must match one of the
+                                languages specified in SupportedLanguages. The
+                                number of languages supported by a driver is up
+                                to the driver writer. Language is specified
+                                in RFC 4646 or ISO 639-2 language code format.
+  @param[out] DriverName        A pointer to the Unicode string to return. This
+                                Unicode string is the name of the driver
+                                specified by This in the language specified by
+                                Language.
+
+  @retval EFI_SUCCESS           The Unicode string for the Driver specified by
+                                This and the language specified by Language was
+                                returned in DriverName.
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+  @retval EFI_INVALID_PARAMETER DriverName is NULL.
+  @retval EFI_UNSUPPORTED       The driver specified by This does not support
+                                the language specified by Language.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusComponentNameGetDriverName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL    *This,
+  IN  CHAR8                          *Language,
+  OUT CHAR16                         **DriverName
+  )
+{
+  return LookupUnicodeString2 (
+           Language,
+           This->SupportedLanguages,
+           mSioBusDriverNameTable,
+           DriverName,
+           (BOOLEAN)(This == &gSioBusComponentName)
+           );
+}
+
+/**
+  Retrieves a Unicode string that is the user readable name of the controller
+  that is being managed by a driver.
+
+  This function retrieves the user readable name of the controller specified by
+  ControllerHandle and ChildHandle in the form of a Unicode string. If the
+  driver specified by This has a user readable name in the language specified
+  by Language, then a pointer to the controller name is returned in
+  ControllerName, and EFI_SUCCESS is returned.  If the driver specified by This
+  is not currently managing the controller specified by ControllerHandle and
+  ChildHandle, then EFI_UNSUPPORTED is returned.  If the driver specified by
+  This does not support the language specified by Language, then
+  EFI_UNSUPPORTED is returned.
+
+  @param[in]  This              A pointer to the EFI_COMPONENT_NAME2_PROTOCOL
+                                or EFI_COMPONENT_NAME_PROTOCOL instance.
+  @param[in]  ControllerHandle  The handle of a controller that the driver
+                                specified by This is managing.  This handle
+                                specifies the controller whose name is to be
+                                returned.
+  @param[in]  ChildHandle       The handle of the child controller to retrieve
+                                the name of.  This is an optional parameter
+                                that may be NULL.  It will be NULL for device
+                                drivers.  It will also be NULL for a bus
+                                drivers that wish to retrieve the name of the
+                                bus controller.  It will not be NULL for a bus
+                                driver that wishes to retrieve the name of a
+                                child controller.
+  @param[in]  Language          A pointer to a Null-terminated ASCII string
+                                array indicating the language.  This is the
+                                language of the driver name that the caller is
+                                requesting, and it must match one of the
+                                languages specified in SupportedLanguages. The
+                                number of languages supported by a driver is up
+                                to the driver writer. Language is specified in
+                                RFC 4646 or ISO 639-2 language code format.
+  @param[out] ControllerName    A pointer to the Unicode string to return.
+                                This Unicode string is the name of the
+                                controller specified by ControllerHandle and
+                                ChildHandle in the language specified by
+                                Language from the point of view of the driver
+                                specified by This.
+
+  @retval EFI_SUCCESS           The Unicode string for the user readable name
+                                in the language specified by Language for the
+                                driver specified by This was returned in
+                                DriverName.
+  @retval EFI_INVALID_PARAMETER ControllerHandle is NULL.
+  @retval EFI_INVALID_PARAMETER ChildHandle is not NULL and it is not a valid
+                                EFI_HANDLE.
+  @retval EFI_INVALID_PARAMETER Language is NULL.
+  @retval EFI_INVALID_PARAMETER ControllerName is NULL.
+  @retval EFI_UNSUPPORTED       The driver specified by This is not currently
+                                managing the controller specified by
+                                ControllerHandle and ChildHandle.
+  @retval EFI_UNSUPPORTED       The driver specified by This does not support
+                                the language specified by Language.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusComponentNameGetControllerName (
+  IN  EFI_COMPONENT_NAME_PROTOCOL    *This,
+  IN  EFI_HANDLE                     ControllerHandle,
+  IN  EFI_HANDLE                     ChildHandle        OPTIONAL,
+  IN  CHAR8                          *Language,
+  OUT CHAR16                         **ControllerName
+  )
+{
+  return EFI_UNSUPPORTED;
+}
diff --git a/OvmfPkg/SioBusDxe/SioBusDxe.c b/OvmfPkg/SioBusDxe/SioBusDxe.c
new file mode 100644
index 0000000000..0cf9a2b524
--- /dev/null
+++ b/OvmfPkg/SioBusDxe/SioBusDxe.c
@@ -0,0 +1,622 @@
+/** @file
+  The SioBusDxe driver is used to create child devices on the ISA bus and
+  installs the Super I/O protocols on them.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+  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 "SioBusDxe.h"
+
+//
+// SioBus Driver Binding Protocol
+//
+EFI_DRIVER_BINDING_PROTOCOL gSioBusDriverBinding = {
+  SioBusDriverBindingSupported,
+  SioBusDriverBindingStart,
+  SioBusDriverBindingStop,
+  0x10,
+  NULL,
+  NULL
+};
+
+
+/**
+  Tests to see if this driver supports a given controller. If a child device is
+  provided, it further tests to see if this driver supports creating a handle
+  for the specified child device.
+
+  This function checks to see if the driver specified by This supports the
+  device specified by ControllerHandle. Drivers will typically use the device
+  path attached to ControllerHandle and/or the services from the bus I/O
+  abstraction attached to ControllerHandle to determine if the driver supports
+  ControllerHandle. This function may be called many times during platform
+  initialization. In order to reduce boot times, the tests performed by this
+  function must be very small, and take as little time as possible to execute.
+  This function must not change the state of any hardware devices, and this
+  function must be aware that the device specified by ControllerHandle may
+  already be managed by the same driver or a different driver. This function
+  must match its calls to AllocatePages() with FreePages(), AllocatePool() with
+  FreePool(), and OpenProtocol() with CloseProtocol(). Since ControllerHandle
+  may have been previously started by the same driver, if a protocol is already
+  in the opened state, then it must not be closed with CloseProtocol(). This is
+  required to guarantee the state of ControllerHandle is not modified by this
+  function.
+
+  @param[in]  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL
+                                   instance.
+  @param[in]  ControllerHandle     The handle of the controller to test. This
+                                   handle must support a protocol interface
+                                   that supplies an I/O abstraction to the
+                                   driver.
+  @param[in]  RemainingDevicePath  A pointer to the remaining portion of a
+                                   device path.  This parameter is ignored by
+                                   device drivers, and is optional for bus
+                                   drivers. For bus drivers, if this parameter
+                                   is not NULL, then the bus driver must
+                                   determine if the bus controller specified by
+                                   ControllerHandle and the child controller
+                                   specified by RemainingDevicePath are both
+                                   supported by this bus driver.
+
+  @retval EFI_SUCCESS              The device specified by ControllerHandle and
+                                   RemainingDevicePath is supported by the
+                                   driver specified by This.
+  @retval EFI_ALREADY_STARTED      The device specified by ControllerHandle and
+                                   RemainingDevicePath is already being managed
+                                   by the driver specified by This.
+  @retval EFI_ACCESS_DENIED        The device specified by ControllerHandle and
+                                   RemainingDevicePath is already being managed
+                                   by a different driver or an application that
+                                   requires exclusive access.
+  @retval EFI_UNSUPPORTED          The device specified by ControllerHandle and
+                                   RemainingDevicePath is not supported by the
+                                   driver specified by This.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusDriverBindingSupported (
+  IN EFI_DRIVER_BINDING_PROTOCOL    *This,
+  IN EFI_HANDLE                     Controller,
+  IN EFI_DEVICE_PATH_PROTOCOL       *RemainingDevicePath
+  )
+{
+  EFI_STATUS             Status;
+  EFI_PCI_IO_PROTOCOL    *PciIo;
+  PCI_TYPE00             Pci;
+  UINTN                  SegmentNumber;
+  UINTN                  BusNumber;
+  UINTN                  DeviceNumber;
+  UINTN                  FunctionNumber;
+
+  //
+  // Get PciIo protocol instance
+  //
+  Status = gBS->OpenProtocol (
+                  Controller,
+                  &gEfiPciIoProtocolGuid,
+                  (VOID**)&PciIo,
+                  This->DriverBindingHandle,
+                  Controller,
+                  EFI_OPEN_PROTOCOL_BY_DRIVER
+                  );
+  if (EFI_ERROR(Status)) {
+    return Status;
+  }
+
+  Status = PciIo->Pci.Read (
+                    PciIo,
+                    EfiPciIoWidthUint32,
+                    0,
+                    sizeof(Pci) / sizeof(UINT32),
+                    &Pci);
+
+  if (!EFI_ERROR (Status)) {
+    Status = EFI_UNSUPPORTED;
+    if ((Pci.Hdr.Command & 0x03) == 0x03) {
+      if (Pci.Hdr.ClassCode[2] == PCI_CLASS_BRIDGE) {
+        //
+        // See if this is a standard PCI to ISA Bridge from the Base Code and
+        // Class Code
+        //
+        if (Pci.Hdr.ClassCode[1] == PCI_CLASS_BRIDGE_ISA) {
+          Status = EFI_SUCCESS;
+        }
+
+        //
+        // See if this is an Intel PCI to ISA bridge in Positive Decode Mode
+        //
+        if (Pci.Hdr.ClassCode[1] == PCI_CLASS_BRIDGE_ISA_PDECODE &&
+            Pci.Hdr.VendorId     == 0x8086) {
+          //
+          // See if this is on Function #0 to avoid false positives on
+          // PCI_CLASS_BRIDGE_OTHER that has the same value as
+          // PCI_CLASS_BRIDGE_ISA_PDECODE
+          //
+          Status = PciIo->GetLocation (
+                            PciIo,
+                            &SegmentNumber,
+                            &BusNumber,
+                            &DeviceNumber,
+                            &FunctionNumber
+                            );
+          if (!EFI_ERROR (Status) && FunctionNumber == 0) {
+            Status = EFI_SUCCESS;
+          } else {
+            Status = EFI_UNSUPPORTED;
+          }
+        }
+      }
+    }
+  }
+
+  gBS->CloseProtocol (
+         Controller,
+         &gEfiPciIoProtocolGuid,
+         This->DriverBindingHandle,
+         Controller
+         );
+
+  return Status;
+}
+
+/**
+  Starts a device controller or a bus controller.
+
+  The Start() function is designed to be invoked from the EFI boot service
+  ConnectController(). As a result, much of the error checking on the
+  parameters to Start() has been moved into this common boot service. It is
+  legal to call Start() from other locations, but the following calling
+  restrictions must be followed or the system behavior will not be
+  deterministic.
+  1. ControllerHandle must be a valid EFI_HANDLE.
+  2. If RemainingDevicePath is not NULL, then it must be a pointer to a
+     naturally aligned EFI_DEVICE_PATH_PROTOCOL.
+  3. Prior to calling Start(), the Supported() function for the driver
+     specified by This must have been called with the same calling parameters,
+     and Supported() must have returned EFI_SUCCESS.
+
+  @param[in]  This                 A pointer to the EFI_DRIVER_BINDING_PROTOCOL
+                                   instance.
+  @param[in]  ControllerHandle     The handle of the controller to start. This
+                                   handle must support a protocol interface
+                                   that supplies an I/O abstraction to the
+                                   driver.
+  @param[in]  RemainingDevicePath  A pointer to the remaining portion of a
+                                   device path.  This parameter is ignored by
+                                   device drivers, and is optional for bus
+                                   drivers. For a bus driver, if this parameter
+                                   is NULL, then handles for all the children
+                                   of Controller are created by this driver. If
+                                   this parameter is not NULL and the first
+                                   Device Path Node is not the End of Device
+                                   Path Node, then only the handle for the
+                                   child device specified by the first Device
+                                   Path Node of RemainingDevicePath is created
+                                   by this driver. If the first Device Path
+                                   Node of RemainingDevicePath is the End of
+                                   Device Path Node, no child handle is created
+                                   by this driver.
+
+  @retval EFI_SUCCESS              The device was started.
+  @retval EFI_DEVICE_ERROR         The device could not be started due to a
+                                   device error.
+  @retval EFI_OUT_OF_RESOURCES     The request could not be completed due to a
+                                   lack of resources.
+  @retval Others                   The driver failded to start the device.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusDriverBindingStart (
+  IN EFI_DRIVER_BINDING_PROTOCOL    *This,
+  IN EFI_HANDLE                     Controller,
+  IN EFI_DEVICE_PATH_PROTOCOL       *RemainingDevicePath
+  )
+{
+  EFI_STATUS                     Status;
+  EFI_PCI_IO_PROTOCOL            *PciIo;
+  EFI_DEVICE_PATH_PROTOCOL       *ParentDevicePath;
+  UINT64                         Supports;
+  UINT64                         OriginalAttributes;
+  UINT64                         Attributes;
+  BOOLEAN                        Enabled;
+  SIO_BUS_DRIVER_PRIVATE_DATA    *Private;
+  UINT32                         ChildDeviceNumber;
+
+  Enabled            = FALSE;
+  Supports           = 0;
+  OriginalAttributes = 0;
+  Private            = NULL;
+
+  //
+  // Open the PCI I/O Protocol Interface
+  //
+  PciIo = NULL;
+  Status = gBS->OpenProtocol (
+                  Controller,
+                  &gEfiPciIoProtocolGuid,
+                  (VOID**) &PciIo,
+                  This->DriverBindingHandle,
+                  Controller,
+                  EFI_OPEN_PROTOCOL_BY_DRIVER
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // Open Device Path Protocol
+  //
+  Status = gBS->OpenProtocol (
+                  Controller,
+                  &gEfiDevicePathProtocolGuid,
+                  (VOID **) &ParentDevicePath,
+                  This->DriverBindingHandle,
+                  Controller,
+                  EFI_OPEN_PROTOCOL_BY_DRIVER
+                  );
+  if (EFI_ERROR (Status)) {
+    gBS->CloseProtocol (
+           Controller,
+           &gEfiPciIoProtocolGuid,
+           This->DriverBindingHandle,
+           Controller
+           );
+    return Status;
+  }
+
+  //
+  // Get supported PCI attributes
+  //
+  Status = PciIo->Attributes (
+                    PciIo,
+                    EfiPciIoAttributeOperationSupported,
+                    0,
+                    &Supports
+                    );
+  if (EFI_ERROR (Status)) {
+    goto Done;
+  }
+
+  Supports &= (UINT64) (EFI_PCI_IO_ATTRIBUTE_ISA_IO |
+                        EFI_PCI_IO_ATTRIBUTE_ISA_IO_16);
+  if (Supports == 0 ||
+      Supports == (EFI_PCI_IO_ATTRIBUTE_ISA_IO |
+                   EFI_PCI_IO_ATTRIBUTE_ISA_IO_16)) {
+    Status = EFI_UNSUPPORTED;
+    goto Done;
+  }
+
+  Status = PciIo->Attributes (
+                    PciIo,
+                    EfiPciIoAttributeOperationGet,
+                    0,
+                    &OriginalAttributes
+                    );
+  if (EFI_ERROR (Status)) {
+    goto Done;
+  }
+
+  Attributes = EFI_PCI_DEVICE_ENABLE |
+               Supports |
+               EFI_PCI_IO_ATTRIBUTE_ISA_MOTHERBOARD_IO;
+  Status = PciIo->Attributes (
+                    PciIo,
+                    EfiPciIoAttributeOperationEnable,
+                    Attributes,
+                    NULL
+                    );
+  if (EFI_ERROR (Status)) {
+    goto Done;
+  }
+
+  Enabled = TRUE;
+
+  //
+  // Store the OriginalAttributes for the restore in BindingStop()
+  //
+  Private = AllocateZeroPool (sizeof (SIO_BUS_DRIVER_PRIVATE_DATA));
+  if (Private == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Done;
+  }
+  Private->PciIo              = PciIo;
+  Private->OriginalAttributes = OriginalAttributes;
+
+  Status = gBS->InstallProtocolInterface (
+                  &Controller,
+                  &gEfiCallerIdGuid,
+                  EFI_NATIVE_INTERFACE,
+                  Private
+                  );
+  if (EFI_ERROR (Status)) {
+    goto Done;
+  }
+
+  //
+  // Report status code for the start of general controller initialization
+  //
+  REPORT_STATUS_CODE_WITH_DEVICE_PATH (
+    EFI_PROGRESS_CODE,
+    (EFI_IO_BUS_LPC | EFI_IOB_PC_INIT),
+    ParentDevicePath
+    );
+
+  //
+  // Report status code for the start of enabling devices on the bus
+  //
+  REPORT_STATUS_CODE_WITH_DEVICE_PATH (
+    EFI_PROGRESS_CODE,
+    (EFI_IO_BUS_LPC | EFI_IOB_PC_ENABLE),
+    ParentDevicePath
+    );
+
+  //
+  // Create all the children upon the first entrance
+  //
+  ChildDeviceNumber = SioCreateAllChildDevices (
+                        This,
+                        Controller,
+                        PciIo,
+                        ParentDevicePath
+                        );
+  if (ChildDeviceNumber == 0) {
+    Status = EFI_DEVICE_ERROR;
+  }
+
+Done:
+  if (EFI_ERROR (Status)) {
+    if (PciIo != NULL && Enabled) {
+      PciIo->Attributes (
+               PciIo,
+               EfiPciIoAttributeOperationSet,
+               OriginalAttributes,
+               NULL
+               );
+    }
+
+    gBS->CloseProtocol (
+           Controller,
+           &gEfiDevicePathProtocolGuid,
+           This->DriverBindingHandle,
+           Controller
+           );
+
+    gBS->CloseProtocol (
+           Controller,
+           &gEfiPciIoProtocolGuid,
+           This->DriverBindingHandle,
+           Controller
+           );
+
+    if (Private != NULL) {
+      gBS->UninstallMultipleProtocolInterfaces (
+             Controller,
+             &gEfiCallerIdGuid,
+             Private,
+             NULL
+             );
+      FreePool (Private);
+    }
+
+    return Status;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Stops a device controller or a bus controller.
+
+  The Stop() function is designed to be invoked from the EFI boot service
+  DisconnectController(). As a result, much of the error checking on the
+  parameters to Stop() has been moved into this common boot service. It is
+  legal to call Stop() from other locations, but the following calling
+  restrictions must be followed or the system behavior will not be
+  deterministic.
+  1. ControllerHandle must be a valid EFI_HANDLE that was used on a previous
+     call to this same driver's Start() function.
+  2. The first NumberOfChildren handles of ChildHandleBuffer must all be a
+     valid EFI_HANDLE. In addition, all of these handles must have been created
+     in this driver's Start() function, and the Start() function must have
+     called OpenProtocol() on ControllerHandle with an Attribute of
+     EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER.
+
+  @param[in]  This               A pointer to the EFI_DRIVER_BINDING_PROTOCOL
+                                 instance.
+  @param[in]  ControllerHandle   A handle to the device being stopped. The
+                                 handle must support a bus specific I/O
+                                 protocol for the driver to use to stop the
+                                 device.
+  @param[in]  NumberOfChildren   The number of child device handles in
+                                 ChildHandleBuffer.
+  @param[in]  ChildHandleBuffer  An array of child handles to be freed. May be
+                                 NULL if NumberOfChildren is 0.
+
+  @retval EFI_SUCCESS            The device was stopped.
+  @retval EFI_DEVICE_ERROR       The device could not be stopped due to a
+                                 device error.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusDriverBindingStop (
+  IN  EFI_DRIVER_BINDING_PROTOCOL    *This,
+  IN  EFI_HANDLE                     Controller,
+  IN  UINTN                          NumberOfChildren,
+  IN  EFI_HANDLE                     *ChildHandleBuffer
+  )
+{
+  EFI_STATUS                     Status;
+  SIO_BUS_DRIVER_PRIVATE_DATA    *Private;
+  UINTN                          Index;
+  BOOLEAN                        AllChildrenStopped;
+  EFI_SIO_PROTOCOL               *Sio;
+  SIO_DEV                        *SioDevice;
+  EFI_PCI_IO_PROTOCOL            *PciIo;
+
+  if (NumberOfChildren == 0) {
+    //
+    // Restore PCI attributes
+    //
+    Status = gBS->OpenProtocol (
+                    Controller,
+                    &gEfiCallerIdGuid,
+                    (VOID **) &Private,
+                    This->DriverBindingHandle,
+                    Controller,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                    );
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    Status = Private->PciIo->Attributes (
+                               Private->PciIo,
+                               EfiPciIoAttributeOperationSet,
+                               Private->OriginalAttributes,
+                               NULL
+                               );
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    gBS->UninstallProtocolInterface (
+          Controller,
+          &gEfiCallerIdGuid,
+          Private
+          );
+    FreePool (Private);
+
+    //
+    // Close the bus driver
+    //
+    Status = gBS->CloseProtocol (
+                    Controller,
+                    &gEfiDevicePathProtocolGuid,
+                    This->DriverBindingHandle,
+                    Controller
+                    );
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    Status = gBS->CloseProtocol (
+                    Controller,
+                    &gEfiPciIoProtocolGuid,
+                    This->DriverBindingHandle,
+                    Controller
+                    );
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    return EFI_SUCCESS;
+  }
+
+  //
+  // Stop all the children
+  //
+  AllChildrenStopped = TRUE;
+
+  for (Index = 0; Index < NumberOfChildren; Index++) {
+    Status = gBS->OpenProtocol (
+                    ChildHandleBuffer[Index],
+                    &gEfiSioProtocolGuid,
+                    (VOID **) &Sio,
+                    This->DriverBindingHandle,
+                    Controller,
+                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
+                    );
+    if (!EFI_ERROR (Status)) {
+      SioDevice = SIO_DEV_FROM_SIO (Sio);
+
+      //
+      // Close the child handle
+      //
+      Status = gBS->CloseProtocol (
+                      Controller,
+                      &gEfiPciIoProtocolGuid,
+                      This->DriverBindingHandle,
+                      ChildHandleBuffer[Index]
+                      );
+      Status = gBS->UninstallMultipleProtocolInterfaces (
+                      ChildHandleBuffer[Index],
+                      &gEfiDevicePathProtocolGuid,
+                      SioDevice->DevicePath,
+                      &gEfiSioProtocolGuid,
+                      &SioDevice->Sio,
+                      NULL
+                      );
+
+      if (!EFI_ERROR (Status)) {
+        FreePool (SioDevice->DevicePath);
+        FreePool (SioDevice);
+      } else {
+        //
+        // Re-open PCI IO Protocol on behalf of the child device
+        // because of failure of destroying the child device handle
+        //
+        gBS->OpenProtocol (
+               Controller,
+               &gEfiPciIoProtocolGuid,
+               (VOID **) &PciIo,
+               This->DriverBindingHandle,
+               ChildHandleBuffer[Index],
+               EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
+               );
+      }
+    }
+
+    if (EFI_ERROR (Status)) {
+      AllChildrenStopped = FALSE;
+    }
+  }
+
+  if (!AllChildrenStopped) {
+    return EFI_DEVICE_ERROR;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  The entry point for the SioBusDxe driver.
+
+  @param[in] ImageHandle    The firmware allocated handle for the EFI image.
+  @param[in] SystemTable    A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS       The entry point is executed successfully.
+  @retval other             Some error occurs when executing this entry point.
+
+**/
+EFI_STATUS
+EFIAPI
+SioBusDxeDriverEntryPoint (
+  IN EFI_HANDLE          ImageHandle,
+  IN EFI_SYSTEM_TABLE    *SystemTable
+  )
+{
+  //
+  // Install driver model protocol(s).
+  //
+  return EfiLibInstallDriverBindingComponentName2 (
+           ImageHandle,
+           SystemTable,
+           &gSioBusDriverBinding,
+           ImageHandle,
+           &gSioBusComponentName,
+           &gSioBusComponentName2
+           );
+}
diff --git a/OvmfPkg/SioBusDxe/SioService.c b/OvmfPkg/SioBusDxe/SioService.c
new file mode 100644
index 0000000000..6a97a4a2c0
--- /dev/null
+++ b/OvmfPkg/SioBusDxe/SioService.c
@@ -0,0 +1,405 @@
+/** @file
+  The SioBusDxe driver is used to create child devices on the ISA bus and
+  installs the Super I/O protocols on them.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+  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 "SioBusDxe.h"
+
+//
+// Super I/O Protocol interfaces
+//
+EFI_SIO_PROTOCOL mSioInterface = {
+  SioRegisterAccess,
+  SioGetResources,
+  SioSetResources,
+  SioPossibleResources,
+  SioModify
+};
+
+//
+// COM 1 UART Controller
+//
+GLOBAL_REMOVE_IF_UNREFERENCED
+SIO_RESOURCES_IO mCom1Resources = {
+  { { ACPI_FIXED_LOCATION_IO_PORT_DESCRIPTOR }, 0x3F8, 8 },
+  { ACPI_END_TAG_DESCRIPTOR,                    0        }
+};
+
+//
+// COM 2 UART Controller
+//
+GLOBAL_REMOVE_IF_UNREFERENCED
+SIO_RESOURCES_IO mCom2Resources = {
+  { { ACPI_FIXED_LOCATION_IO_PORT_DESCRIPTOR }, 0x2F8, 8 },
+  { ACPI_END_TAG_DESCRIPTOR,                    0        }
+};
+
+//
+// PS/2 Keyboard Controller
+//
+GLOBAL_REMOVE_IF_UNREFERENCED
+SIO_RESOURCES_IO mPs2KeyboardDeviceResources = {
+  { { ACPI_FIXED_LOCATION_IO_PORT_DESCRIPTOR }, 0x60, 5 },
+  { ACPI_END_TAG_DESCRIPTOR,                    0       }
+};
+
+//
+// Table of SIO Controllers
+//
+GLOBAL_REMOVE_IF_UNREFERENCED
+SIO_DEVICE_INFO mDevicesInfo[] = {
+  {
+    EISA_PNP_ID (0x501),
+    0,
+    { (ACPI_SMALL_RESOURCE_HEADER *) &mCom1Resources }
+  },  // COM 1 UART Controller
+  {
+    EISA_PNP_ID (0x501),
+    1,
+    { (ACPI_SMALL_RESOURCE_HEADER *) &mCom2Resources }
+  },  // COM 2 UART Controller
+  {
+    EISA_PNP_ID(0x303),
+    0,
+    { (ACPI_SMALL_RESOURCE_HEADER *) &mPs2KeyboardDeviceResources }
+  }   // PS/2 Keyboard Controller
+};
+
+//
+// ACPI Device Path Node template
+//
+GLOBAL_REMOVE_IF_UNREFERENCED
+ACPI_HID_DEVICE_PATH mAcpiDeviceNodeTemplate = {
+  {        // Header
+    ACPI_DEVICE_PATH,
+    ACPI_DP,
+    {
+      (UINT8) (sizeof (ACPI_HID_DEVICE_PATH)),
+      (UINT8) ((sizeof (ACPI_HID_DEVICE_PATH)) >> 8)
+    }
+  },
+  0x0,     // HID
+  0x0      // UID
+};
+
+
+/**
+  Provides a low level access to the registers for the Super I/O.
+
+  @param[in]     This           Indicates a pointer to the calling context.
+  @param[in]     Write          Specifies the type of the register operation.
+                                If this parameter is TRUE, Value is interpreted
+                                as an input parameter and the operation is a
+                                register write. If this parameter is FALSE,
+                                Value is interpreted as an output parameter and
+                                the operation is a register read.
+  @param[in]     ExitCfgMode    Exit Configuration Mode Indicator. If this
+                                parameter is set to TRUE, the Super I/O driver
+                                will turn off configuration mode of the Super
+                                I/O prior to returning from this function. If
+                                this parameter is set to FALSE, the Super I/O
+                                driver will leave Super I/O in the
+                                configuration mode. The Super I/O driver must
+                                track the current state of the Super I/O and
+                                enable the configuration mode of Super I/O if
+                                necessary prior to register access.
+  @param[in]     Register       Register number.
+  @param[in,out] Value          If Write is TRUE, Value is a pointer to the
+                                buffer containing the byte of data to be
+                                written to the Super I/O register. If Write is
+                                FALSE, Value is a pointer to the destination
+                                buffer for the byte of data to be read from the
+                                Super I/O register.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER The Value is NULL.
+  @retval EFI_INVALID_PARAMETER Invalid Register number.
+
+**/
+EFI_STATUS
+EFIAPI
+SioRegisterAccess (
+  IN CONST EFI_SIO_PROTOCOL    *This,
+  IN       BOOLEAN             Write,
+  IN       BOOLEAN             ExitCfgMode,
+  IN       UINT8               Register,
+  IN OUT   UINT8               *Value
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/**
+  Provides an interface to get a list of the current resources consumed by the
+  device in the ACPI Resource Descriptor format.
+
+  GetResources() returns a list of resources currently consumed by the device.
+  The ResourceList is a pointer to the buffer containing resource descriptors
+  for the device. The descriptors are in the format of Small or Large ACPI
+  resource descriptor as defined by ACPI specification (2.0 & 3.0). The buffer
+  of resource descriptors is terminated with the 'End tag' resource descriptor.
+
+  @param[in]  This              Indicates a pointer to the calling context.
+  @param[out] ResourceList      A pointer to an ACPI resource descriptor list
+                                that defines the current resources used by the
+                                device.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER ResourceList is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+SioGetResources (
+  IN CONST EFI_SIO_PROTOCOL            *This,
+  OUT      ACPI_RESOURCE_HEADER_PTR    *ResourceList
+  )
+{
+  SIO_DEV    *SioDevice;
+
+  if (ResourceList == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  SioDevice = SIO_DEV_FROM_SIO (This);
+  if (SioDevice->DeviceIndex < ARRAY_SIZE (mDevicesInfo)) {
+    *ResourceList = mDevicesInfo[SioDevice->DeviceIndex].Resources;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Sets the resources for the device.
+
+  @param[in] This               Indicates a pointer to the calling context.
+  @param[in] ResourceList       Pointer to the ACPI resource descriptor list.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER ResourceList is invalid.
+  @retval EFI_ACCESS_DENIED     Some of the resources in ResourceList are in
+                                use.
+
+**/
+EFI_STATUS
+EFIAPI
+SioSetResources (
+  IN CONST EFI_SIO_PROTOCOL            *This,
+  IN       ACPI_RESOURCE_HEADER_PTR    ResourceList
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/**
+  Provides a collection of resource descriptor lists. Each resource descriptor
+  list in the collection defines a combination of resources that can
+  potentially be used by the device.
+
+  @param[in]  This                  Indicates a pointer to the calling context.
+  @param[out] ResourceCollection    Collection of the resource descriptor
+                                    lists.
+
+  @retval EFI_SUCCESS               The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER     ResourceCollection is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+SioPossibleResources (
+  IN CONST EFI_SIO_PROTOCOL            *This,
+  OUT      ACPI_RESOURCE_HEADER_PTR    *ResourceCollection
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/**
+  Provides an interface for a table based programming of the Super I/O
+  registers.
+
+  The Modify() function provides an interface for table based programming of
+  the Super I/O registers. This function can be used to perform programming of
+  multiple Super I/O registers with a single function call. For each table
+  entry, the Register is read, its content is bitwise ANDed with AndMask, and
+  then ORed with OrMask before being written back to the Register. The Super
+  I/O driver must track the current state of the Super I/O and enable the
+  configuration mode of Super I/O if necessary prior to table processing. Once
+  the table is processed, the Super I/O device has to be returned to the
+  original state.
+
+  @param[in] This               Indicates a pointer to the calling context.
+  @param[in] Command            A pointer to an array of NumberOfCommands
+                                EFI_SIO_REGISTER_MODIFY structures. Each
+                                structure specifies a single Super I/O register
+                                modify operation.
+  @param[in] NumberOfCommands   Number of elements in the Command array.
+
+  @retval EFI_SUCCESS           The operation completed successfully.
+  @retval EFI_INVALID_PARAMETER Command is NULL.
+
+**/
+EFI_STATUS
+EFIAPI
+SioModify (
+  IN CONST EFI_SIO_PROTOCOL           *This,
+  IN CONST EFI_SIO_REGISTER_MODIFY    *Command,
+  IN       UINTN                      NumberOfCommands
+  )
+{
+  return EFI_SUCCESS;
+}
+
+/**
+  Create the child device with a given device index.
+
+  @param[in] This              The EFI_DRIVER_BINDING_PROTOCOL instance.
+  @param[in] Controller        The handle of ISA bus controller.
+  @param[in] PciIo             The pointer to the PCI protocol.
+  @param[in] ParentDevicePath  Device path of the ISA bus controller.
+  @param[in] DeviceIndex       Index of the device supported by this driver.
+
+  @retval EFI_SUCCESS          The child device has been created successfully.
+  @retval Others               Error occured during the child device creation.
+
+**/
+EFI_STATUS
+SioCreateChildDevice (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN EFI_HANDLE                   Controller,
+  IN EFI_PCI_IO_PROTOCOL          *PciIo,
+  IN EFI_DEVICE_PATH_PROTOCOL     *ParentDevicePath,
+  IN UINT32                       DeviceIndex
+  )
+{
+  EFI_STATUS    Status;
+  SIO_DEV       *SioDevice;
+
+  //
+  // Initialize the SIO_DEV structure
+  //
+  SioDevice = AllocateZeroPool (sizeof (SIO_DEV));
+  if (SioDevice == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  SioDevice->Signature  = SIO_DEV_SIGNATURE;
+  SioDevice->Handle     = NULL;
+  SioDevice->PciIo      = PciIo;
+
+  //
+  // Construct the child device path
+  //
+  mAcpiDeviceNodeTemplate.HID = mDevicesInfo[DeviceIndex].Hid;
+  mAcpiDeviceNodeTemplate.UID = mDevicesInfo[DeviceIndex].Uid;
+  SioDevice->DevicePath = AppendDevicePathNode (
+                            ParentDevicePath,
+                            (EFI_DEVICE_PATH_PROTOCOL *) &mAcpiDeviceNodeTemplate
+                            );
+  if (SioDevice->DevicePath == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    goto Done;
+  }
+
+  CopyMem (&SioDevice->Sio, &mSioInterface, sizeof (EFI_SIO_PROTOCOL));
+  SioDevice->DeviceIndex = DeviceIndex;
+
+  //
+  // Create a child handle and install Device Path and Super I/O protocols
+  //
+  Status = gBS->InstallMultipleProtocolInterfaces (
+                  &SioDevice->Handle,
+                  &gEfiDevicePathProtocolGuid,
+                  SioDevice->DevicePath,
+                  &gEfiSioProtocolGuid,
+                  &SioDevice->Sio,
+                  NULL
+                  );
+  if (EFI_ERROR (Status)) {
+    goto Done;
+  }
+
+  Status = gBS->OpenProtocol (
+                  Controller,
+                  &gEfiPciIoProtocolGuid,
+                  (VOID **) &PciIo,
+                  This->DriverBindingHandle,
+                  SioDevice->Handle,
+                  EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
+                  );
+  if (EFI_ERROR (Status)) {
+    gBS->UninstallMultipleProtocolInterfaces (
+           SioDevice->Handle,
+           &gEfiDevicePathProtocolGuid,
+           SioDevice->DevicePath,
+           &gEfiSioProtocolGuid,
+           &SioDevice->Sio,
+           NULL
+           );
+  }
+
+Done:
+  if (EFI_ERROR (Status)) {
+    if (SioDevice->DevicePath != NULL) {
+      FreePool (SioDevice->DevicePath);
+    }
+
+    FreePool (SioDevice);
+  }
+
+  return Status;
+}
+
+/**
+  Create all the ISA child devices on the ISA bus controller (PCI to ISA
+  bridge).
+
+  @param[in] This              The EFI_DRIVER_BINDING_PROTOCOL instance.
+  @param[in] Controller        The handle of ISA bus controller.
+  @param[in] PciIo             The pointer to the PCI protocol.
+  @param[in] ParentDevicePath  Device path of the ISA bus controller.
+
+  @retval The number of child device that is successfully created.
+
+**/
+UINT32
+SioCreateAllChildDevices (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN EFI_HANDLE                   Controller,
+  IN EFI_PCI_IO_PROTOCOL          *PciIo,
+  IN EFI_DEVICE_PATH_PROTOCOL     *ParentDevicePath
+  )
+{
+  UINT32        Index;
+  UINT32        ChildDeviceNumber;
+  EFI_STATUS    Status;
+
+  ChildDeviceNumber = 0;
+
+  for (Index = 0; Index < ARRAY_SIZE (mDevicesInfo); Index++) {
+    Status = SioCreateChildDevice (
+               This,
+               Controller,
+               PciIo,
+               ParentDevicePath,
+               Index
+               );
+    if (!EFI_ERROR (Status)) {
+      ChildDeviceNumber++;
+    }
+  }
+
+  return ChildDeviceNumber;
+}
diff --git a/OvmfPkg/SioBusDxe/SioBusDxe.uni b/OvmfPkg/SioBusDxe/SioBusDxe.uni
new file mode 100644
index 0000000000..4793180557
--- /dev/null
+++ b/OvmfPkg/SioBusDxe/SioBusDxe.uni
@@ -0,0 +1,21 @@
+// /** @file
+// The SioBusDxe driver is used to create child devices on the ISA bus and
+// installs the Super I/O protocols on them.
+//
+// Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+//
+// 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.
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT       #language en-US "Manage ISA devices on the ISA bus"
+
+#string STR_MODULE_DESCRIPTION    #language en-US "The SioBusDxe driver is used to create child devices on the ISA bus and installs the Super I/O protocols on them."
-- 
2.12.0.windows.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* [PATCH v2 3/3] OvmfPkg: Add a build flag to select ISA driver stack
  2019-03-25  5:28 [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Hao Wu
  2019-03-25  5:28 ` [PATCH v2 1/3] OvmfPkg: Drop the ISA Floppy device support Hao Wu
  2019-03-25  5:28 ` [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver Hao Wu
@ 2019-03-25  5:28 ` Hao Wu
  2019-03-25 11:20   ` Laszlo Ersek
  2019-03-25  8:28 ` [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Ard Biesheuvel
  2019-03-25 10:58 ` Laszlo Ersek
  4 siblings, 1 reply; 28+ messages in thread
From: Hao Wu @ 2019-03-25  5:28 UTC (permalink / raw)
  To: edk2-devel; +Cc: Hao Wu, Jordan Justen, Laszlo Ersek, Ard Biesheuvel, Ray Ni

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495

This commit will add a static build flag 'USE_LEGACY_ISA_STACK' to select
the ISA driver stack.

If the flag is set to TRUE, the below driver stack will be used:
  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf

If the flag is set to FALSE, the below driver stack will be used:
  OvmfPkg/SioBusDxe/SioBusDxe.inf
  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf

The default value is set to FALSE in OVMF DSC files.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Ray Ni <ray.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 OvmfPkg/OvmfPkgIa32.dsc    |  6 ++++++
 OvmfPkg/OvmfPkgIa32X64.dsc |  6 ++++++
 OvmfPkg/OvmfPkgX64.dsc     |  6 ++++++
 OvmfPkg/OvmfPkgIa32.fdf    | 18 ++++++++++++------
 OvmfPkg/OvmfPkgIa32X64.fdf | 18 ++++++++++++------
 OvmfPkg/OvmfPkgX64.fdf     | 18 ++++++++++++------
 6 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 3be0314146..f55ab5a3d2 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -41,6 +41,7 @@
   DEFINE TLS_ENABLE              = FALSE
   DEFINE TPM2_ENABLE             = FALSE
   DEFINE TPM2_CONFIG_ENABLE      = FALSE
+  DEFINE USE_LEGACY_ISA_STACK    = FALSE
 
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
@@ -752,11 +753,16 @@
   #
   # ISA Support
   #
+!if $(USE_LEGACY_ISA_STACK) == FALSE
   OvmfPkg/SioBusDxe/SioBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
+!else
   PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+!endif
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 3b85c2e6af..5c9bdf034e 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -41,6 +41,7 @@
   DEFINE TLS_ENABLE              = FALSE
   DEFINE TPM2_ENABLE             = FALSE
   DEFINE TPM2_CONFIG_ENABLE      = FALSE
+  DEFINE USE_LEGACY_ISA_STACK    = FALSE
 
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
@@ -761,11 +762,16 @@
   #
   # ISA Support
   #
+!if $(USE_LEGACY_ISA_STACK) == FALSE
   OvmfPkg/SioBusDxe/SioBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
+!else
   PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+!endif
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 104b2e79a5..2943e9e8af 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -41,6 +41,7 @@
   DEFINE TLS_ENABLE              = FALSE
   DEFINE TPM2_ENABLE             = FALSE
   DEFINE TPM2_CONFIG_ENABLE      = FALSE
+  DEFINE USE_LEGACY_ISA_STACK    = FALSE
 
   #
   # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
@@ -759,11 +760,16 @@
   #
   # ISA Support
   #
+!if $(USE_LEGACY_ISA_STACK) == FALSE
   OvmfPkg/SioBusDxe/SioBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
+!else
   PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
+!endif
 
   #
   # SMBIOS Support
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 54d7f06a70..006ea9a415 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -265,14 +265,20 @@ INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
 INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
-INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
-
+!if $(USE_LEGACY_ISA_STACK) == FALSE
+  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
 !ifndef $(SOURCE_DEBUG_ENABLE)
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+  INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+!endif
+  INF  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
+!else
+  INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
+  INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
+!ifndef $(SOURCE_DEBUG_ENABLE)
+  INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+!endif
+  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
 !endif
-
-INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 7519b53a9b..6c40540202 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -266,14 +266,20 @@ INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
 INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
-INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
-
+!if $(USE_LEGACY_ISA_STACK) == FALSE
+  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
 !ifndef $(SOURCE_DEBUG_ENABLE)
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+  INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+!endif
+  INF  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
+!else
+  INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
+  INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
+!ifndef $(SOURCE_DEBUG_ENABLE)
+  INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+!endif
+  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
 !endif
-
-INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 7519b53a9b..6c40540202 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -266,14 +266,20 @@ INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
 INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
 
-INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
-
+!if $(USE_LEGACY_ISA_STACK) == FALSE
+  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
 !ifndef $(SOURCE_DEBUG_ENABLE)
-INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+  INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
+!endif
+  INF  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
+!else
+  INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
+  INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
+!ifndef $(SOURCE_DEBUG_ENABLE)
+  INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
+!endif
+  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
 !endif
-
-INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
 
 INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
-- 
2.12.0.windows.1



^ permalink raw reply related	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-25  5:28 [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Hao Wu
                   ` (2 preceding siblings ...)
  2019-03-25  5:28 ` [PATCH v2 3/3] OvmfPkg: Add a build flag to select ISA driver stack Hao Wu
@ 2019-03-25  8:28 ` Ard Biesheuvel
  2019-03-25 10:58 ` Laszlo Ersek
  4 siblings, 0 replies; 28+ messages in thread
From: Ard Biesheuvel @ 2019-03-25  8:28 UTC (permalink / raw)
  To: Hao Wu; +Cc: edk2-devel@lists.01.org, Jordan Justen, Laszlo Ersek, Ray Ni

On Mon, 25 Mar 2019 at 06:28, Hao Wu <hao.a.wu@intel.com> wrote:
>
> The series is also available at:
> https://github.com/hwu25/edk2/tree/ovmf_siobus_v2
>
> V2 changes:
> * Introduce a static build flag 'USE_LEGACY_ISA_STACK' in OVMF DSC files
>   for users to select between the ISA driver stacks.
> * V1 patch 2/2 is split into 2 patches in V2. The first one will add the
>   new OVMF SioBusDxe driver and list it in the DSC files. Then second one
>   will add the whole new ISA stack in DSC/FDF files.
>
>
> V1 history:
>
> This series will update the OVMF to stop using the ISA drivers within
> IntelFrameworkModulePkg.
>
> As the replacement, a new OVMF Super I/O bus driver has been add which
> will install the Super I/O protocol for ISA serial and PS2 keyboard
> devices. By doing so, these devices can be managed by:
>
>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
>
> respectively.
>
>
> Tests done:
> A. GCC5 & VS2015x86 tool chains build pass
> B. Launch QEMU (2.4.50, Windows) with command:
>    > qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -serial file:1.txt -serial file:2.txt
>
>    Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under Shell
>    using command 'devtree';
>
>    Both the serials and PS2 keyboard are working fine;
>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
>
>
> Hao Wu (3):
>   OvmfPkg: Drop the ISA Floppy device support
>   OvmfPkg: Add an Super IO bus driver
>   OvmfPkg: Add a build flag to select ISA driver stack
>

Thanks Hao.

For the series,

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

but I will leave the final decision to Laszlo.


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 1/3] OvmfPkg: Drop the ISA Floppy device support
  2019-03-25  5:28 ` [PATCH v2 1/3] OvmfPkg: Drop the ISA Floppy device support Hao Wu
@ 2019-03-25 10:42   ` Laszlo Ersek
  0 siblings, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-25 10:42 UTC (permalink / raw)
  To: Hao Wu, edk2-devel; +Cc: Jordan Justen, Ard Biesheuvel, Ray Ni

On 03/25/19 06:28, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495
> 
> There is a plan to remove the IntelFrameworkModulePkg:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1605
> 
> And for driver:
> IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe
> 
> This patch proposes to drop the ISA Floppy device support in OVMF.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 3 +--
>  OvmfPkg/OvmfPkgIa32X64.dsc | 3 +--
>  OvmfPkg/OvmfPkgX64.dsc     | 3 +--
>  OvmfPkg/OvmfPkgIa32.fdf    | 3 +--
>  OvmfPkg/OvmfPkgIa32X64.fdf | 3 +--
>  OvmfPkg/OvmfPkgX64.fdf     | 3 +--
>  6 files changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 5b885590b2..1710ab5a88 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -1,7 +1,7 @@
>  ## @file
>  #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -756,7 +756,6 @@
>    IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> -  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
>  
>    #
>    # SMBIOS Support
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index bbf0853ee6..5bceef3116 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -1,7 +1,7 @@
>  ## @file
>  #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -765,7 +765,6 @@
>    IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> -  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
>  
>    #
>    # SMBIOS Support
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index d81460f520..3f5d948dbb 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -1,7 +1,7 @@
>  ## @file
>  #  EFI/Framework Open Virtual Machine Firmware (OVMF) platform
>  #
> -#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -763,7 +763,6 @@
>    IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> -  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
>  
>    #
>    # SMBIOS Support
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 4999403ad7..54d7f06a70 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Open Virtual Machine Firmware: FDF
>  #
> -#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -273,7 +273,6 @@ INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>  !endif
>  
>  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
>  
>  INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
>  INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index d0cc107928..7519b53a9b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Open Virtual Machine Firmware: FDF
>  #
> -#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -274,7 +274,6 @@ INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>  !endif
>  
>  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
>  
>  INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
>  INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index d0cc107928..7519b53a9b 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Open Virtual Machine Firmware: FDF
>  #
> -#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -274,7 +274,6 @@ INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>  !endif
>  
>  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaFloppyDxe/IsaFloppyDxe.inf
>  
>  INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
>  INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-25  5:28 [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Hao Wu
                   ` (3 preceding siblings ...)
  2019-03-25  8:28 ` [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Ard Biesheuvel
@ 2019-03-25 10:58 ` Laszlo Ersek
  2019-03-25 11:29   ` Laszlo Ersek
  4 siblings, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-25 10:58 UTC (permalink / raw)
  To: Hao Wu, edk2-devel; +Cc: Jordan Justen, Ard Biesheuvel, Ray Ni

On 03/25/19 06:28, Hao Wu wrote:
> The series is also available at:
> https://github.com/hwu25/edk2/tree/ovmf_siobus_v2
> 
> V2 changes:
> * Introduce a static build flag 'USE_LEGACY_ISA_STACK' in OVMF DSC files
>   for users to select between the ISA driver stacks.
> * V1 patch 2/2 is split into 2 patches in V2. The first one will add the
>   new OVMF SioBusDxe driver and list it in the DSC files. Then second one
>   will add the whole new ISA stack in DSC/FDF files.
> 
> 
> V1 history:
> 
> This series will update the OVMF to stop using the ISA drivers within
> IntelFrameworkModulePkg.
> 
> As the replacement, a new OVMF Super I/O bus driver has been add which
> will install the Super I/O protocol for ISA serial and PS2 keyboard
> devices. By doing so, these devices can be managed by:
> 
>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> 
> respectively.
> 
> 
> Tests done:
> A. GCC5 & VS2015x86 tool chains build pass
> B. Launch QEMU (2.4.50, Windows) with command:
>    > qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -serial file:1.txt -serial file:2.txt
> 
>    Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under Shell
>    using command 'devtree';
> 
>    Both the serials and PS2 keyboard are working fine;

Can you please confirm the following:

(1) In the PrepareLpcBridgeDevicePath() function, in file
"OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c", we add
IsaKeyboard to ConIn, and IsaSerial to ConOut, ConIn, ErrOut.

This function takes the "LPC Bridge device" handle from its caller,
namely DetectAndPreparePlatformPciDevicePath(), and appends some
constant device path nodes, from "BdsPlatform.h" / "PlatformData.c".

Can you please confirm that the existing Platform BDS code described
above is compatible with the new driver?

In other words, do DetectAndPreparePlatformPciDevicePath() +
PrepareLpcBridgeDevicePath() still add the proper device paths to
ConIn/ConOut/ErrOut?

(Note, they need not be identical to the previous device paths, but the
*logic* must continue to work -- i.e. *some* device paths have to be
added, and they should be correct.)

(2) Can you please confirm if the new build survives repeated

  reconnect -r

commands in the UEFI shell? Both the ISA keyboard and the serial console
should resume working after "reconnect -r".

Thanks,
Laszlo






> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
> 
> 
> Hao Wu (3):
>   OvmfPkg: Drop the ISA Floppy device support
>   OvmfPkg: Add an Super IO bus driver
>   OvmfPkg: Add a build flag to select ISA driver stack
> 
>  OvmfPkg/OvmfPkgIa32.dsc           |  10 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc        |  10 +-
>  OvmfPkg/OvmfPkgX64.dsc            |  10 +-
>  OvmfPkg/OvmfPkgIa32.fdf           |  21 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf        |  21 +-
>  OvmfPkg/OvmfPkgX64.fdf            |  21 +-
>  OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
>  OvmfPkg/SioBusDxe/SioBusDxe.h     | 332 +++++++++++
>  OvmfPkg/SioBusDxe/SioService.h    | 221 +++++++
>  OvmfPkg/SioBusDxe/ComponentName.c | 167 ++++++
>  OvmfPkg/SioBusDxe/SioBusDxe.c     | 622 ++++++++++++++++++++
>  OvmfPkg/SioBusDxe/SioService.c    | 405 +++++++++++++
>  OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
>  13 files changed, 1885 insertions(+), 30 deletions(-)
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.inf
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.h
>  create mode 100644 OvmfPkg/SioBusDxe/SioService.h
>  create mode 100644 OvmfPkg/SioBusDxe/ComponentName.c
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.c
>  create mode 100644 OvmfPkg/SioBusDxe/SioService.c
>  create mode 100644 OvmfPkg/SioBusDxe/SioBusDxe.uni
> 



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 3/3] OvmfPkg: Add a build flag to select ISA driver stack
  2019-03-25  5:28 ` [PATCH v2 3/3] OvmfPkg: Add a build flag to select ISA driver stack Hao Wu
@ 2019-03-25 11:20   ` Laszlo Ersek
  0 siblings, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-25 11:20 UTC (permalink / raw)
  To: Hao Wu, edk2-devel; +Cc: Jordan Justen, Ard Biesheuvel, Ray Ni

On 03/25/19 06:28, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495
> 
> This commit will add a static build flag 'USE_LEGACY_ISA_STACK' to select
> the ISA driver stack.
> 
> If the flag is set to TRUE, the below driver stack will be used:
>   PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
>   IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
>   IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>   IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> 
> If the flag is set to FALSE, the below driver stack will be used:
>   OvmfPkg/SioBusDxe/SioBusDxe.inf
>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> 
> The default value is set to FALSE in OVMF DSC files.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    |  6 ++++++
>  OvmfPkg/OvmfPkgIa32X64.dsc |  6 ++++++
>  OvmfPkg/OvmfPkgX64.dsc     |  6 ++++++
>  OvmfPkg/OvmfPkgIa32.fdf    | 18 ++++++++++++------
>  OvmfPkg/OvmfPkgIa32X64.fdf | 18 ++++++++++++------
>  OvmfPkg/OvmfPkgX64.fdf     | 18 ++++++++++++------
>  6 files changed, 54 insertions(+), 18 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 3be0314146..f55ab5a3d2 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -41,6 +41,7 @@
>    DEFINE TLS_ENABLE              = FALSE
>    DEFINE TPM2_ENABLE             = FALSE
>    DEFINE TPM2_CONFIG_ENABLE      = FALSE
> +  DEFINE USE_LEGACY_ISA_STACK    = FALSE
>  
>    #
>    # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
> @@ -752,11 +753,16 @@
>    #
>    # ISA Support
>    #
> +!if $(USE_LEGACY_ISA_STACK) == FALSE
>    OvmfPkg/SioBusDxe/SioBusDxe.inf
> +  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> +  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> +!else
>    PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> +!endif
>  
>    #
>    # SMBIOS Support
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 3b85c2e6af..5c9bdf034e 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -41,6 +41,7 @@
>    DEFINE TLS_ENABLE              = FALSE
>    DEFINE TPM2_ENABLE             = FALSE
>    DEFINE TPM2_CONFIG_ENABLE      = FALSE
> +  DEFINE USE_LEGACY_ISA_STACK    = FALSE
>  
>    #
>    # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
> @@ -761,11 +762,16 @@
>    #
>    # ISA Support
>    #
> +!if $(USE_LEGACY_ISA_STACK) == FALSE
>    OvmfPkg/SioBusDxe/SioBusDxe.inf
> +  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> +  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> +!else
>    PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> +!endif
>  
>    #
>    # SMBIOS Support
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 104b2e79a5..2943e9e8af 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -41,6 +41,7 @@
>    DEFINE TLS_ENABLE              = FALSE
>    DEFINE TPM2_ENABLE             = FALSE
>    DEFINE TPM2_CONFIG_ENABLE      = FALSE
> +  DEFINE USE_LEGACY_ISA_STACK    = FALSE
>  
>    #
>    # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
> @@ -759,11 +760,16 @@
>    #
>    # ISA Support
>    #
> +!if $(USE_LEGACY_ISA_STACK) == FALSE
>    OvmfPkg/SioBusDxe/SioBusDxe.inf
> +  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> +  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> +!else
>    PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
>    IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
> +!endif
>  
>    #
>    # SMBIOS Support
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 54d7f06a70..006ea9a415 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -265,14 +265,20 @@ INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>  INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>  INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>  
> -INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
> -
> +!if $(USE_LEGACY_ISA_STACK) == FALSE
> +  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
>  !ifndef $(SOURCE_DEBUG_ENABLE)
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
> +  INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> +!endif
> +  INF  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> +!else
> +  INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
> +  INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
> +!ifndef $(SOURCE_DEBUG_ENABLE)
> +  INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
> +!endif
> +  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
>  !endif
> -
> -INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
>  
>  INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
>  INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 7519b53a9b..6c40540202 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -266,14 +266,20 @@ INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>  INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>  INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>  
> -INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
> -
> +!if $(USE_LEGACY_ISA_STACK) == FALSE
> +  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
>  !ifndef $(SOURCE_DEBUG_ENABLE)
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
> +  INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> +!endif
> +  INF  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> +!else
> +  INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
> +  INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
> +!ifndef $(SOURCE_DEBUG_ENABLE)
> +  INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
> +!endif
> +  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
>  !endif
> -
> -INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
>  
>  INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
>  INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 7519b53a9b..6c40540202 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -266,14 +266,20 @@ INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>  INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>  INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
>  
> -INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
> -
> +!if $(USE_LEGACY_ISA_STACK) == FALSE
> +  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
>  !ifndef $(SOURCE_DEBUG_ENABLE)
> -INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
> +  INF  MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> +!endif
> +  INF  MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> +!else
> +  INF  PcAtChipsetPkg/IsaAcpiDxe/IsaAcpi.inf
> +  INF  IntelFrameworkModulePkg/Bus/Isa/IsaBusDxe/IsaBusDxe.inf
> +!ifndef $(SOURCE_DEBUG_ENABLE)
> +  INF  IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe/IsaSerialDxe.inf
> +!endif
> +  INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
>  !endif
> -
> -INF  IntelFrameworkModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2keyboardDxe.inf
>  
>  INF  MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
>  INF  OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.inf
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25  5:28 ` [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver Hao Wu
@ 2019-03-25 11:22   ` Laszlo Ersek
  2019-03-26  2:52     ` Wu, Hao A
  2019-03-25 12:00   ` Laszlo Ersek
  1 sibling, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-25 11:22 UTC (permalink / raw)
  To: Hao Wu, edk2-devel; +Cc: Jordan Justen, Ard Biesheuvel, Ray Ni

On 03/25/19 06:28, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495
> 
> There is a plan to remove the IntelFrameworkModulePkg:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1605
> 
> This patch will a new OVMF Super I/O bus driver which will create the
> below child devices:
> 
> * COM 1 UART
> * COM 2 UART
> * PS/2 Keyboard
> 
> and installs the Super I/O Protocol on them.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Ray Ni <ray.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc           |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc        |   1 +
>  OvmfPkg/OvmfPkgX64.dsc            |   1 +
>  OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
>  OvmfPkg/SioBusDxe/SioBusDxe.h     | 332 +++++++++++
>  OvmfPkg/SioBusDxe/SioService.h    | 221 +++++++
>  OvmfPkg/SioBusDxe/ComponentName.c | 167 ++++++
>  OvmfPkg/SioBusDxe/SioBusDxe.c     | 622 ++++++++++++++++++++
>  OvmfPkg/SioBusDxe/SioService.c    | 405 +++++++++++++
>  OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
>  10 files changed, 1825 insertions(+)

Acked-by: Laszlo Ersek <lersek@redhat.com>

but please don't push the series before answering my questions under 0/3.

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-25 10:58 ` Laszlo Ersek
@ 2019-03-25 11:29   ` Laszlo Ersek
  2019-03-26  2:49     ` Wu, Hao A
                       ` (2 more replies)
  0 siblings, 3 replies; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-25 11:29 UTC (permalink / raw)
  To: Hao Wu, edk2-devel, Julien Grall, Anthony Perard
  Cc: Jordan Justen, Ard Biesheuvel, Ray Ni

On 03/25/19 11:58, Laszlo Ersek wrote:
> On 03/25/19 06:28, Hao Wu wrote:
>> The series is also available at:
>> https://github.com/hwu25/edk2/tree/ovmf_siobus_v2
>>
>> V2 changes:
>> * Introduce a static build flag 'USE_LEGACY_ISA_STACK' in OVMF DSC files
>>   for users to select between the ISA driver stacks.
>> * V1 patch 2/2 is split into 2 patches in V2. The first one will add the
>>   new OVMF SioBusDxe driver and list it in the DSC files. Then second one
>>   will add the whole new ISA stack in DSC/FDF files.
>>
>>
>> V1 history:
>>
>> This series will update the OVMF to stop using the ISA drivers within
>> IntelFrameworkModulePkg.
>>
>> As the replacement, a new OVMF Super I/O bus driver has been add which
>> will install the Super I/O protocol for ISA serial and PS2 keyboard
>> devices. By doing so, these devices can be managed by:
>>
>>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
>>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
>>
>> respectively.
>>
>>
>> Tests done:
>> A. GCC5 & VS2015x86 tool chains build pass
>> B. Launch QEMU (2.4.50, Windows) with command:
>>    > qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -serial file:1.txt -serial file:2.txt
>>
>>    Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under Shell
>>    using command 'devtree';
>>
>>    Both the serials and PS2 keyboard are working fine;
> 
> Can you please confirm the following:
> 
> (1) In the PrepareLpcBridgeDevicePath() function, in file
> "OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c", we add
> IsaKeyboard to ConIn, and IsaSerial to ConOut, ConIn, ErrOut.
> 
> This function takes the "LPC Bridge device" handle from its caller,
> namely DetectAndPreparePlatformPciDevicePath(), and appends some
> constant device path nodes, from "BdsPlatform.h" / "PlatformData.c".
> 
> Can you please confirm that the existing Platform BDS code described
> above is compatible with the new driver?
> 
> In other words, do DetectAndPreparePlatformPciDevicePath() +
> PrepareLpcBridgeDevicePath() still add the proper device paths to
> ConIn/ConOut/ErrOut?
> 
> (Note, they need not be identical to the previous device paths, but the
> *logic* must continue to work -- i.e. *some* device paths have to be
> added, and they should be correct.)
> 
> (2) Can you please confirm if the new build survives repeated
> 
>   reconnect -r
> 
> commands in the UEFI shell? Both the ISA keyboard and the serial console
> should resume working after "reconnect -r".

(3) Hao, can you please verify the above steps on the "q35" machine type
as well?

(The QEMU command line that you mention selects the "i440fx" machine
type. QEMU provides an ICH9 ISA controller with the q35 board, and a
PIIX4 one with the i440fx board. I think we should regression-test this
work with both.)

(4) Julien, Anthony: can you please fetch this series (github URL at the
top) and see if the PS/2 keyboard, and the serial port, still work on
Xen, to interact e.g. with the UEFI shell?

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25  5:28 ` [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver Hao Wu
  2019-03-25 11:22   ` Laszlo Ersek
@ 2019-03-25 12:00   ` Laszlo Ersek
  2019-03-25 17:30     ` Kinney, Michael D
  1 sibling, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-25 12:00 UTC (permalink / raw)
  To: Hao Wu, edk2-devel; +Cc: Jordan Justen, Ard Biesheuvel, Ray Ni, Michael Kinney

Hello Hao,

(+Mike)

my apologies that I seem unable to collect my thoughts in one go... So
here's another comment:

On 03/25/19 06:28, Hao Wu wrote:

> diff --git a/OvmfPkg/SioBusDxe/SioBusDxe.inf b/OvmfPkg/SioBusDxe/SioBusDxe.inf
> new file mode 100644
> index 0000000000..5c462f1a8c
> --- /dev/null
> +++ b/OvmfPkg/SioBusDxe/SioBusDxe.inf
> @@ -0,0 +1,54 @@
> +## @file
> +#  The SioBusDxe driver is used to create child devices on the ISA bus and
> +#  installs the Super I/O protocols on them.
> +#
> +#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +#
> +#  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.
> +#
> +##

[...]

This patch adds 7 new instances of the 2-clause BSDL (in expanded form)
to the edk2 tree. In that it conflicts with Mike's work for
<https://bugzilla.tianocore.org/show_bug.cgi?id=1373>. Can you please
rework this patch so that the new files say

  SPDX-License-Identifier: BSD-2-Clause-Patent

instead?

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25 12:00   ` Laszlo Ersek
@ 2019-03-25 17:30     ` Kinney, Michael D
  2019-03-25 18:33       ` Laszlo Ersek
  0 siblings, 1 reply; 28+ messages in thread
From: Kinney, Michael D @ 2019-03-25 17:30 UTC (permalink / raw)
  To: Laszlo Ersek, Wu, Hao A, edk2-devel@lists.01.org,
	Kinney, Michael D
  Cc: Justen, Jordan L, Ard Biesheuvel, Ni, Ray

Hi Laszlo,

I do not think content added before April 9, 2019
should use the new license type.  We need to let the
30-day review period complete and make sure all feedback
is resolved.

We will handle files added between the edk2-stable201903
and April 9, 2019 in a final patch series with an easy
way for all maintainers to see what has changed between
those two points.

Thanks,

Mike

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, March 25, 2019 5:00 AM
> To: Wu, Hao A <hao.a.wu@intel.com>; edk2-
> devel@lists.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>; Ni, Ray
> <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus
> driver
> 
> Hello Hao,
> 
> (+Mike)
> 
> my apologies that I seem unable to collect my thoughts
> in one go... So
> here's another comment:
> 
> On 03/25/19 06:28, Hao Wu wrote:
> 
> > diff --git a/OvmfPkg/SioBusDxe/SioBusDxe.inf
> b/OvmfPkg/SioBusDxe/SioBusDxe.inf
> > new file mode 100644
> > index 0000000000..5c462f1a8c
> > --- /dev/null
> > +++ b/OvmfPkg/SioBusDxe/SioBusDxe.inf
> > @@ -0,0 +1,54 @@
> > +## @file
> > +#  The SioBusDxe driver is used to create child
> devices on the ISA bus and
> > +#  installs the Super I/O protocols on them.
> > +#
> > +#  Copyright (c) 2019, Intel Corporation. All rights
> reserved.<BR>
> > +#
> > +#  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.
> > +#
> > +##
> 
> [...]
> 
> This patch adds 7 new instances of the 2-clause BSDL (in
> expanded form)
> to the edk2 tree. In that it conflicts with Mike's work
> for
> <https://bugzilla.tianocore.org/show_bug.cgi?id=1373>.
> Can you please
> rework this patch so that the new files say
> 
>   SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> instead?
> 
> Thanks,
> Laszlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25 17:30     ` Kinney, Michael D
@ 2019-03-25 18:33       ` Laszlo Ersek
  2019-03-25 20:02         ` Kinney, Michael D
  0 siblings, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-25 18:33 UTC (permalink / raw)
  To: Kinney, Michael D, Wu, Hao A, edk2-devel@lists.01.org
  Cc: Justen, Jordan L, Ard Biesheuvel, Ni, Ray

On 03/25/19 18:30, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> I do not think content added before April 9, 2019
> should use the new license type.  We need to let the
> 30-day review period complete and make sure all feedback
> is resolved.

Good point.

> We will handle files added between the edk2-stable201903
> and April 9, 2019 in a final patch series with an easy
> way for all maintainers to see what has changed between
> those two points.

Hm. From the reviewer side, this is not optimal. The patch set (and the
individual patches themselves) are pretty big, and doing incremental
reviews on them is taxing. Regardless of whether the incremental review
needs to target an updated "full" patch set, or just an incremental
patch set (for new files), the reviewer needs to re-evaluate whether
something is now missed, after the introduction of new files.

Instead, I'd prefer a "lock" period for OvmfPkg and ArmVirtPkg, between
(a) my next (hopefully, final) review for the license conversion
patches, and (b) the pushing of those patches. For that, I see two options:

- We could delay Hao's work (and all other patches that add files to
OvmfPkg and ArmVirtPkg files) until after April 9. We can of course
collaborate on feature / bugfix patches meanwhile, it's just that the
final versions of *those* should be reposted with updated license
blocks. Incrementally reviewing *those* changes feels a lot easier to me.

- Alternatively, I could delay my next (hopefully, final) review of the
license conversion patches until reasonably close to April 9, until
which "review point" new files could be added freely, to OvmfPkg and
ArmVirtPkg. (This wouldn't eliminate the "lock period", just make it
shorter for contributors.)

IOW, this is similar to the stabilization period / feature freezes, just
much more intrusive, because everything has to be switched at the same
moment.

I'd like to reach an understanding on our approach before I start
reviewing "[edk2] [PATCH V2] Change EDK II to BSD+Patent License".

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25 18:33       ` Laszlo Ersek
@ 2019-03-25 20:02         ` Kinney, Michael D
  2019-03-26 11:19           ` Laszlo Ersek
  0 siblings, 1 reply; 28+ messages in thread
From: Kinney, Michael D @ 2019-03-25 20:02 UTC (permalink / raw)
  To: Laszlo Ersek, Wu, Hao A, edk2-devel@lists.01.org,
	Kinney, Michael D
  Cc: Justen, Jordan L, Ard Biesheuvel, Ni, Ray

Hi Laszlo,

The review of the content based on the edk2-stable201903
is intended to make sure there are no mistakes on that
content so I can adjust for the final patch series.  A
mistake would be applying new license to files that should
not be updated, or not applying the license to a file that
should have been updated.  You provided valuable feedback
on those two points for OvmfPkg in V1.

I agree it will be simpler if we can guarantee no file
add/remove commits occur in a window leading up to 
April 9, 2019.  So it is not a freeze on all content.
It would be a freeze on commits that add/remove files.

How does a ~1 week of no commits of file add/remove
starting April 1 sound?  I can produce a V3 on April 2
for final review by all package maintainers.

I would of course rebase the patch series on April 9 and
also verify that no files were added/removed.

Thanks,

Mike

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, March 25, 2019 11:33 AM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Wu,
> Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Ard
> Biesheuvel <ard.biesheuvel@linaro.org>; Ni, Ray
> <ray.ni@intel.com>
> Subject: Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus
> driver
> 
> On 03/25/19 18:30, Kinney, Michael D wrote:
> > Hi Laszlo,
> >
> > I do not think content added before April 9, 2019
> > should use the new license type.  We need to let the
> > 30-day review period complete and make sure all
> feedback
> > is resolved.
> 
> Good point.
> 
> > We will handle files added between the edk2-
> stable201903
> > and April 9, 2019 in a final patch series with an easy
> > way for all maintainers to see what has changed
> between
> > those two points.
> 
> Hm. From the reviewer side, this is not optimal. The
> patch set (and the
> individual patches themselves) are pretty big, and doing
> incremental
> reviews on them is taxing. Regardless of whether the
> incremental review
> needs to target an updated "full" patch set, or just an
> incremental
> patch set (for new files), the reviewer needs to re-
> evaluate whether
> something is now missed, after the introduction of new
> files.
> 
> Instead, I'd prefer a "lock" period for OvmfPkg and
> ArmVirtPkg, between
> (a) my next (hopefully, final) review for the license
> conversion
> patches, and (b) the pushing of those patches. For that,
> I see two options:
> 
> - We could delay Hao's work (and all other patches that
> add files to
> OvmfPkg and ArmVirtPkg files) until after April 9. We
> can of course
> collaborate on feature / bugfix patches meanwhile, it's
> just that the
> final versions of *those* should be reposted with
> updated license
> blocks. Incrementally reviewing *those* changes feels a
> lot easier to me.
> 
> - Alternatively, I could delay my next (hopefully,
> final) review of the
> license conversion patches until reasonably close to
> April 9, until
> which "review point" new files could be added freely, to
> OvmfPkg and
> ArmVirtPkg. (This wouldn't eliminate the "lock period",
> just make it
> shorter for contributors.)
> 
> IOW, this is similar to the stabilization period /
> feature freezes, just
> much more intrusive, because everything has to be
> switched at the same
> moment.
> 
> I'd like to reach an understanding on our approach
> before I start
> reviewing "[edk2] [PATCH V2] Change EDK II to BSD+Patent
> License".
> 
> Thanks
> Laszlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-25 11:29   ` Laszlo Ersek
@ 2019-03-26  2:49     ` Wu, Hao A
  2019-03-26 10:14       ` Laszlo Ersek
  2019-03-26 10:09     ` Julien Grall
  2019-03-26 13:03     ` Anthony PERARD
  2 siblings, 1 reply; 28+ messages in thread
From: Wu, Hao A @ 2019-03-26  2:49 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel@lists.01.org, Julien Grall,
	Anthony Perard
  Cc: Justen, Jordan L, Ard Biesheuvel, Ni, Ray

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Monday, March 25, 2019 7:29 PM
> To: Wu, Hao A; edk2-devel@lists.01.org; Julien Grall; Anthony Perard
> Cc: Justen, Jordan L; Ard Biesheuvel; Ni, Ray
> Subject: Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within
> IntelFrameworkModulePkg
> 
> On 03/25/19 11:58, Laszlo Ersek wrote:
> > On 03/25/19 06:28, Hao Wu wrote:
> >> The series is also available at:
> >> https://github.com/hwu25/edk2/tree/ovmf_siobus_v2
> >>
> >> V2 changes:
> >> * Introduce a static build flag 'USE_LEGACY_ISA_STACK' in OVMF DSC files
> >>   for users to select between the ISA driver stacks.
> >> * V1 patch 2/2 is split into 2 patches in V2. The first one will add the
> >>   new OVMF SioBusDxe driver and list it in the DSC files. Then second one
> >>   will add the whole new ISA stack in DSC/FDF files.
> >>
> >>
> >> V1 history:
> >>
> >> This series will update the OVMF to stop using the ISA drivers within
> >> IntelFrameworkModulePkg.
> >>
> >> As the replacement, a new OVMF Super I/O bus driver has been add which
> >> will install the Super I/O protocol for ISA serial and PS2 keyboard
> >> devices. By doing so, these devices can be managed by:
> >>
> >>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
> >>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
> >>
> >> respectively.
> >>
> >>
> >> Tests done:
> >> A. GCC5 & VS2015x86 tool chains build pass
> >> B. Launch QEMU (2.4.50, Windows) with command:
> >>    > qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -serial
> file:1.txt -serial file:2.txt
> >>
> >>    Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under
> Shell
> >>    using command 'devtree';
> >>
> >>    Both the serials and PS2 keyboard are working fine;
> >
> > Can you please confirm the following:
> >
> > (1) In the PrepareLpcBridgeDevicePath() function, in file
> > "OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c", we add
> > IsaKeyboard to ConIn, and IsaSerial to ConOut, ConIn, ErrOut.
> >
> > This function takes the "LPC Bridge device" handle from its caller,
> > namely DetectAndPreparePlatformPciDevicePath(), and appends some
> > constant device path nodes, from "BdsPlatform.h" / "PlatformData.c".
> >
> > Can you please confirm that the existing Platform BDS code described
> > above is compatible with the new driver?
> >
> > In other words, do DetectAndPreparePlatformPciDevicePath() +
> > PrepareLpcBridgeDevicePath() still add the proper device paths to
> > ConIn/ConOut/ErrOut?
> >
> > (Note, they need not be identical to the previous device paths, but the
> > *logic* must continue to work -- i.e. *some* device paths have to be
> > added, and they should be correct.)

Hello Laszlo,

For (1), since I saw in function PrepareLpcBridgeDevicePath() there are
already some codes to print out the device path for COM1/COM2. So I just
add some logic to print the device path for the PS2 keyboard as well.

I ran the qemu with command:
qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402

For the new and origin ISA stack, their logs both show:
Found LPC Bridge device
BdsPlatform.c+585: PS2 Keyboard DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Acpi(PNP0303,0x0)
BdsPlatform.c+615: COM1 DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Serial(0x0)/Uart(115200,8,N,1)/VenMsg(E0C14753-F9BE-11D2-9A0C-0090273FC14D)
BdsPlatform.c+647: COM2 DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Serial(0x1)/Uart(115200,8,N,1)/VenMsg(E0C14753-F9BE-11D2-9A0C-0090273FC14D)

Thus, I think the Platform BDS code behavior remains the same.

> >
> > (2) Can you please confirm if the new build survives repeated
> >
> >   reconnect -r
> >
> > commands in the UEFI shell? Both the ISA keyboard and the serial console
> > should resume working after "reconnect -r".

For (2), I also run the qemu with:
qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402

After running 'reconnect -r' under Shell, the PS2 keyboard works fine.
Then I switch the display from 'VGA' to 'serial0', and the serial works
fine. (I just re-run the 'reconnect -r' under 'serial0', after the command
finishes, the keyboard is responding and there is still output on 'serial0'.)

> 
> (3) Hao, can you please verify the above steps on the "q35" machine type
> as well?
> 
> (The QEMU command line that you mention selects the "i440fx" machine
> type. QEMU provides an ICH9 ISA controller with the q35 board, and a
> PIIX4 one with the i440fx board. I think we should regression-test this
> work with both.)

For (3), I run qemu with:
qemu-system-x86_64.exe -machine q35 -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402

Both tests (1) and (2) passed in this case.


Please help to let me know if there is anything not right in the above
tests. Thanks.

Best Regards,
Hao Wu

> 
> (4) Julien, Anthony: can you please fetch this series (github URL at the
> top) and see if the PS/2 keyboard, and the serial port, still work on
> Xen, to interact e.g. with the UEFI shell?
> 
> Thanks!
> Laszlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25 11:22   ` Laszlo Ersek
@ 2019-03-26  2:52     ` Wu, Hao A
  0 siblings, 0 replies; 28+ messages in thread
From: Wu, Hao A @ 2019-03-26  2:52 UTC (permalink / raw)
  To: Laszlo Ersek, edk2-devel@lists.01.org; +Cc: Justen, Jordan L

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Monday, March 25, 2019 7:22 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Justen, Jordan L
> Subject: Re: [edk2] [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
> 
> On 03/25/19 06:28, Hao Wu wrote:
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1495
> >
> > There is a plan to remove the IntelFrameworkModulePkg:
> > https://bugzilla.tianocore.org/show_bug.cgi?id=1605
> >
> > This patch will a new OVMF Super I/O bus driver which will create the
> > below child devices:
> >
> > * COM 1 UART
> > * COM 2 UART
> > * PS/2 Keyboard
> >
> > and installs the Super I/O Protocol on them.
> >
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc           |   1 +
> >  OvmfPkg/OvmfPkgIa32X64.dsc        |   1 +
> >  OvmfPkg/OvmfPkgX64.dsc            |   1 +
> >  OvmfPkg/SioBusDxe/SioBusDxe.inf   |  54 ++
> >  OvmfPkg/SioBusDxe/SioBusDxe.h     | 332 +++++++++++
> >  OvmfPkg/SioBusDxe/SioService.h    | 221 +++++++
> >  OvmfPkg/SioBusDxe/ComponentName.c | 167 ++++++
> >  OvmfPkg/SioBusDxe/SioBusDxe.c     | 622 ++++++++++++++++++++
> >  OvmfPkg/SioBusDxe/SioService.c    | 405 +++++++++++++
> >  OvmfPkg/SioBusDxe/SioBusDxe.uni   |  21 +
> >  10 files changed, 1825 insertions(+)
> 
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> 
> but please don't push the series before answering my questions under 0/3.

Thanks Laszlo,

I have replied your comments under patch 0/3.

I will wait for your approval, since I saw there is an open with regard
to the file license header for the new files.

Best Regards,
Hao Wu

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


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-25 11:29   ` Laszlo Ersek
  2019-03-26  2:49     ` Wu, Hao A
@ 2019-03-26 10:09     ` Julien Grall
  2019-03-26 11:53       ` Laszlo Ersek
  2019-03-26 13:03     ` Anthony PERARD
  2 siblings, 1 reply; 28+ messages in thread
From: Julien Grall @ 2019-03-26 10:09 UTC (permalink / raw)
  To: Laszlo Ersek, Hao Wu, edk2-devel, Anthony Perard
  Cc: Jordan Justen, Ard Biesheuvel, Ray Ni

Hi Laszlo,

On 3/25/19 11:29 AM, Laszlo Ersek wrote:
> On 03/25/19 11:58, Laszlo Ersek wrote:
>> On 03/25/19 06:28, Hao Wu wrote:
> (4) Julien, Anthony: can you please fetch this series (github URL at the
> top) and see if the PS/2 keyboard, and the serial port, still work on
> Xen, to interact e.g. with the UEFI shell?

Just to confirm, should we test for x86 only? If so, I will leave that 
to Anthony.

Cheers,

-- 
Julien Grall


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-26  2:49     ` Wu, Hao A
@ 2019-03-26 10:14       ` Laszlo Ersek
  2019-03-26 11:21         ` Laszlo Ersek
  0 siblings, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-26 10:14 UTC (permalink / raw)
  To: Wu, Hao A, edk2-devel@lists.01.org, Julien Grall, Anthony Perard
  Cc: Justen, Jordan L, Ard Biesheuvel, Ni, Ray

On 03/26/19 03:49, Wu, Hao A wrote:
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Monday, March 25, 2019 7:29 PM
>> To: Wu, Hao A; edk2-devel@lists.01.org; Julien Grall; Anthony Perard
>> Cc: Justen, Jordan L; Ard Biesheuvel; Ni, Ray
>> Subject: Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within
>> IntelFrameworkModulePkg
>>
>> On 03/25/19 11:58, Laszlo Ersek wrote:
>>> On 03/25/19 06:28, Hao Wu wrote:
>>>> The series is also available at:
>>>> https://github.com/hwu25/edk2/tree/ovmf_siobus_v2
>>>>
>>>> V2 changes:
>>>> * Introduce a static build flag 'USE_LEGACY_ISA_STACK' in OVMF DSC files
>>>>   for users to select between the ISA driver stacks.
>>>> * V1 patch 2/2 is split into 2 patches in V2. The first one will add the
>>>>   new OVMF SioBusDxe driver and list it in the DSC files. Then second one
>>>>   will add the whole new ISA stack in DSC/FDF files.
>>>>
>>>>
>>>> V1 history:
>>>>
>>>> This series will update the OVMF to stop using the ISA drivers within
>>>> IntelFrameworkModulePkg.
>>>>
>>>> As the replacement, a new OVMF Super I/O bus driver has been add which
>>>> will install the Super I/O protocol for ISA serial and PS2 keyboard
>>>> devices. By doing so, these devices can be managed by:
>>>>
>>>>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
>>>>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
>>>>
>>>> respectively.
>>>>
>>>>
>>>> Tests done:
>>>> A. GCC5 & VS2015x86 tool chains build pass
>>>> B. Launch QEMU (2.4.50, Windows) with command:
>>>>    > qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -serial
>> file:1.txt -serial file:2.txt
>>>>
>>>>    Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under
>> Shell
>>>>    using command 'devtree';
>>>>
>>>>    Both the serials and PS2 keyboard are working fine;
>>>
>>> Can you please confirm the following:
>>>
>>> (1) In the PrepareLpcBridgeDevicePath() function, in file
>>> "OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c", we add
>>> IsaKeyboard to ConIn, and IsaSerial to ConOut, ConIn, ErrOut.
>>>
>>> This function takes the "LPC Bridge device" handle from its caller,
>>> namely DetectAndPreparePlatformPciDevicePath(), and appends some
>>> constant device path nodes, from "BdsPlatform.h" / "PlatformData.c".
>>>
>>> Can you please confirm that the existing Platform BDS code described
>>> above is compatible with the new driver?
>>>
>>> In other words, do DetectAndPreparePlatformPciDevicePath() +
>>> PrepareLpcBridgeDevicePath() still add the proper device paths to
>>> ConIn/ConOut/ErrOut?
>>>
>>> (Note, they need not be identical to the previous device paths, but the
>>> *logic* must continue to work -- i.e. *some* device paths have to be
>>> added, and they should be correct.)
> 
> Hello Laszlo,
> 
> For (1), since I saw in function PrepareLpcBridgeDevicePath() there are
> already some codes to print out the device path for COM1/COM2. So I just
> add some logic to print the device path for the PS2 keyboard as well.
> 
> I ran the qemu with command:
> qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402
> 
> For the new and origin ISA stack, their logs both show:
> Found LPC Bridge device
> BdsPlatform.c+585: PS2 Keyboard DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Acpi(PNP0303,0x0)
> BdsPlatform.c+615: COM1 DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Serial(0x0)/Uart(115200,8,N,1)/VenMsg(E0C14753-F9BE-11D2-9A0C-0090273FC14D)
> BdsPlatform.c+647: COM2 DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Serial(0x1)/Uart(115200,8,N,1)/VenMsg(E0C14753-F9BE-11D2-9A0C-0090273FC14D)
> 
> Thus, I think the Platform BDS code behavior remains the same.
> 
>>>
>>> (2) Can you please confirm if the new build survives repeated
>>>
>>>   reconnect -r
>>>
>>> commands in the UEFI shell? Both the ISA keyboard and the serial console
>>> should resume working after "reconnect -r".
> 
> For (2), I also run the qemu with:
> qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402
> 
> After running 'reconnect -r' under Shell, the PS2 keyboard works fine.
> Then I switch the display from 'VGA' to 'serial0', and the serial works
> fine. (I just re-run the 'reconnect -r' under 'serial0', after the command
> finishes, the keyboard is responding and there is still output on 'serial0'.)
> 
>>
>> (3) Hao, can you please verify the above steps on the "q35" machine type
>> as well?
>>
>> (The QEMU command line that you mention selects the "i440fx" machine
>> type. QEMU provides an ICH9 ISA controller with the q35 board, and a
>> PIIX4 one with the i440fx board. I think we should regression-test this
>> work with both.)
> 
> For (3), I run qemu with:
> qemu-system-x86_64.exe -machine q35 -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402
> 
> Both tests (1) and (2) passed in this case.
> 
> 
> Please help to let me know if there is anything not right in the above
> tests. Thanks.

All of the above sounds great, thank you for the thorough testing.

Please let's wait for (4) below, and also let's sort out the license
identifier scheduling.

Thanks!
Laszlo

>>
>> (4) Julien, Anthony: can you please fetch this series (github URL at the
>> top) and see if the PS/2 keyboard, and the serial port, still work on
>> Xen, to interact e.g. with the UEFI shell?
>>
>> Thanks!
>> Laszlo



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver
  2019-03-25 20:02         ` Kinney, Michael D
@ 2019-03-26 11:19           ` Laszlo Ersek
  0 siblings, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-26 11:19 UTC (permalink / raw)
  To: Kinney, Michael D, Wu, Hao A, edk2-devel@lists.01.org
  Cc: Justen, Jordan L, Ard Biesheuvel, Ni, Ray

On 03/25/19 21:02, Kinney, Michael D wrote:
> Hi Laszlo,
> 
> The review of the content based on the edk2-stable201903
> is intended to make sure there are no mistakes on that
> content so I can adjust for the final patch series.  A
> mistake would be applying new license to files that should
> not be updated, or not applying the license to a file that
> should have been updated.  You provided valuable feedback
> on those two points for OvmfPkg in V1.
> 
> I agree it will be simpler if we can guarantee no file
> add/remove commits occur in a window leading up to 
> April 9, 2019.  So it is not a freeze on all content.
> It would be a freeze on commits that add/remove files.

Good point.

> How does a ~1 week of no commits of file add/remove
> starting April 1 sound?  I can produce a V3 on April 2
> for final review by all package maintainers.

That sounds great. I'd be happy to iterate multiple times during that
freeze, if necessary (so v3 wouldn't have to be the final version even);
what matters is that the reviews should converge.

Thus, considering actions on my end regarding the v2 posting, I'll delay
reviewing v2 (or v3 if you post v3 meanwhile) until April 2.

I'm making a note in my calendar, but please do ping me on April 2 --
I'm the one requesting this freeze, so once it starts, I should be as
responsive as possible.

> I would of course rebase the patch series on April 9 and
> also verify that no files were added/removed.

Thank you!
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-26 10:14       ` Laszlo Ersek
@ 2019-03-26 11:21         ` Laszlo Ersek
  0 siblings, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-26 11:21 UTC (permalink / raw)
  To: Wu, Hao A, edk2-devel@lists.01.org, Julien Grall, Anthony Perard
  Cc: Justen, Jordan L, Ard Biesheuvel, Ni, Ray

On 03/26/19 11:14, Laszlo Ersek wrote:
> On 03/26/19 03:49, Wu, Hao A wrote:
>>> -----Original Message-----
>>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>>> Sent: Monday, March 25, 2019 7:29 PM
>>> To: Wu, Hao A; edk2-devel@lists.01.org; Julien Grall; Anthony Perard
>>> Cc: Justen, Jordan L; Ard Biesheuvel; Ni, Ray
>>> Subject: Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within
>>> IntelFrameworkModulePkg
>>>
>>> On 03/25/19 11:58, Laszlo Ersek wrote:
>>>> On 03/25/19 06:28, Hao Wu wrote:
>>>>> The series is also available at:
>>>>> https://github.com/hwu25/edk2/tree/ovmf_siobus_v2
>>>>>
>>>>> V2 changes:
>>>>> * Introduce a static build flag 'USE_LEGACY_ISA_STACK' in OVMF DSC files
>>>>>   for users to select between the ISA driver stacks.
>>>>> * V1 patch 2/2 is split into 2 patches in V2. The first one will add the
>>>>>   new OVMF SioBusDxe driver and list it in the DSC files. Then second one
>>>>>   will add the whole new ISA stack in DSC/FDF files.
>>>>>
>>>>>
>>>>> V1 history:
>>>>>
>>>>> This series will update the OVMF to stop using the ISA drivers within
>>>>> IntelFrameworkModulePkg.
>>>>>
>>>>> As the replacement, a new OVMF Super I/O bus driver has been add which
>>>>> will install the Super I/O protocol for ISA serial and PS2 keyboard
>>>>> devices. By doing so, these devices can be managed by:
>>>>>
>>>>>   MdeModulePkg/Bus/Pci/PciSioSerialDxe/PciSioSerialDxe.inf
>>>>>   MdeModulePkg/Bus/Isa/Ps2KeyboardDxe/Ps2KeyboardDxe.inf
>>>>>
>>>>> respectively.
>>>>>
>>>>>
>>>>> Tests done:
>>>>> A. GCC5 & VS2015x86 tool chains build pass
>>>>> B. Launch QEMU (2.4.50, Windows) with command:
>>>>>    > qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -serial
>>> file:1.txt -serial file:2.txt
>>>>>
>>>>>    Able to see the ISA COM1/COM2 UART and PS2Keyboard devices under
>>> Shell
>>>>>    using command 'devtree';
>>>>>
>>>>>    Both the serials and PS2 keyboard are working fine;
>>>>
>>>> Can you please confirm the following:
>>>>
>>>> (1) In the PrepareLpcBridgeDevicePath() function, in file
>>>> "OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c", we add
>>>> IsaKeyboard to ConIn, and IsaSerial to ConOut, ConIn, ErrOut.
>>>>
>>>> This function takes the "LPC Bridge device" handle from its caller,
>>>> namely DetectAndPreparePlatformPciDevicePath(), and appends some
>>>> constant device path nodes, from "BdsPlatform.h" / "PlatformData.c".
>>>>
>>>> Can you please confirm that the existing Platform BDS code described
>>>> above is compatible with the new driver?
>>>>
>>>> In other words, do DetectAndPreparePlatformPciDevicePath() +
>>>> PrepareLpcBridgeDevicePath() still add the proper device paths to
>>>> ConIn/ConOut/ErrOut?
>>>>
>>>> (Note, they need not be identical to the previous device paths, but the
>>>> *logic* must continue to work -- i.e. *some* device paths have to be
>>>> added, and they should be correct.)
>>
>> Hello Laszlo,
>>
>> For (1), since I saw in function PrepareLpcBridgeDevicePath() there are
>> already some codes to print out the device path for COM1/COM2. So I just
>> add some logic to print the device path for the PS2 keyboard as well.
>>
>> I ran the qemu with command:
>> qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402
>>
>> For the new and origin ISA stack, their logs both show:
>> Found LPC Bridge device
>> BdsPlatform.c+585: PS2 Keyboard DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Acpi(PNP0303,0x0)
>> BdsPlatform.c+615: COM1 DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Serial(0x0)/Uart(115200,8,N,1)/VenMsg(E0C14753-F9BE-11D2-9A0C-0090273FC14D)
>> BdsPlatform.c+647: COM2 DevPath: PciRoot(0x0)/Pci(0x1,0x0)/Serial(0x1)/Uart(115200,8,N,1)/VenMsg(E0C14753-F9BE-11D2-9A0C-0090273FC14D)
>>
>> Thus, I think the Platform BDS code behavior remains the same.
>>
>>>>
>>>> (2) Can you please confirm if the new build survives repeated
>>>>
>>>>   reconnect -r
>>>>
>>>> commands in the UEFI shell? Both the ISA keyboard and the serial console
>>>> should resume working after "reconnect -r".
>>
>> For (2), I also run the qemu with:
>> qemu-system-x86_64.exe -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402
>>
>> After running 'reconnect -r' under Shell, the PS2 keyboard works fine.
>> Then I switch the display from 'VGA' to 'serial0', and the serial works
>> fine. (I just re-run the 'reconnect -r' under 'serial0', after the command
>> finishes, the keyboard is responding and there is still output on 'serial0'.)
>>
>>>
>>> (3) Hao, can you please verify the above steps on the "q35" machine type
>>> as well?
>>>
>>> (The QEMU command line that you mention selects the "i440fx" machine
>>> type. QEMU provides an ICH9 ISA controller with the q35 board, and a
>>> PIIX4 one with the i440fx board. I think we should regression-test this
>>> work with both.)
>>
>> For (3), I run qemu with:
>> qemu-system-x86_64.exe -machine q35 -pflash <SOME_PATH>\OVMF.fd -debugcon file:boot.log -global isa-debugcon.iobase=0x402
>>
>> Both tests (1) and (2) passed in this case.
>>
>>
>> Please help to let me know if there is anything not right in the above
>> tests. Thanks.
> 
> All of the above sounds great, thank you for the thorough testing.
> 
> Please let's wait for (4) below, and also let's sort out the license
> identifier scheduling.

So it looks like the only remaining question remains (4), i.e. testing
on Xen. If that works out fine, we should push this series before April
2nd, which is when the file addition/removal freeze is going to start,
for a week.

>>> (4) Julien, Anthony: can you please fetch this series (github URL at the
>>> top) and see if the PS/2 keyboard, and the serial port, still work on
>>> Xen, to interact e.g. with the UEFI shell?

Thanks!
Laszlo



^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-26 10:09     ` Julien Grall
@ 2019-03-26 11:53       ` Laszlo Ersek
  0 siblings, 0 replies; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-26 11:53 UTC (permalink / raw)
  To: Julien Grall, Hao Wu, edk2-devel, Anthony Perard
  Cc: Jordan Justen, Ard Biesheuvel, Ray Ni

On 03/26/19 11:09, Julien Grall wrote:
> Hi Laszlo,
> 
> On 3/25/19 11:29 AM, Laszlo Ersek wrote:
>> On 03/25/19 11:58, Laszlo Ersek wrote:
>>> On 03/25/19 06:28, Hao Wu wrote:
>> (4) Julien, Anthony: can you please fetch this series (github URL at the
>> top) and see if the PS/2 keyboard, and the serial port, still work on
>> Xen, to interact e.g. with the UEFI shell?
> 
> Just to confirm, should we test for x86 only? If so, I will leave that
> to Anthony.

Yes, this is x86 only.

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-25 11:29   ` Laszlo Ersek
  2019-03-26  2:49     ` Wu, Hao A
  2019-03-26 10:09     ` Julien Grall
@ 2019-03-26 13:03     ` Anthony PERARD
  2019-03-26 15:01       ` Laszlo Ersek
  2 siblings, 1 reply; 28+ messages in thread
From: Anthony PERARD @ 2019-03-26 13:03 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Hao Wu, edk2-devel, Julien Grall, Jordan Justen, Ard Biesheuvel,
	Ray Ni

On Mon, Mar 25, 2019 at 12:29:25PM +0100, Laszlo Ersek wrote:
> (4) Julien, Anthony: can you please fetch this series (github URL at the
> top) and see if the PS/2 keyboard, and the serial port, still work on
> Xen, to interact e.g. with the UEFI shell?

Yes, I can still interact with the UEFI shell with this series applied,
both via the console and via VNC (which uses respectively the serial
port and PS/2 I think).

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-26 13:03     ` Anthony PERARD
@ 2019-03-26 15:01       ` Laszlo Ersek
  2019-03-26 15:14         ` Anthony PERARD
  2019-03-26 15:15         ` Laszlo Ersek
  0 siblings, 2 replies; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-26 15:01 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Hao Wu, edk2-devel, Julien Grall, Jordan Justen, Ard Biesheuvel,
	Ray Ni

On 03/26/19 14:03, Anthony PERARD wrote:
> On Mon, Mar 25, 2019 at 12:29:25PM +0100, Laszlo Ersek wrote:
>> (4) Julien, Anthony: can you please fetch this series (github URL at the
>> top) and see if the PS/2 keyboard, and the serial port, still work on
>> Xen, to interact e.g. with the UEFI shell?
> 
> Yes, I can still interact with the UEFI shell with this series applied,
> both via the console and via VNC (which uses respectively the serial
> port and PS/2 I think).

Thank you -- can you please follow up with a Tested-by, or do you prefer
if I push the set without it?

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-26 15:01       ` Laszlo Ersek
@ 2019-03-26 15:14         ` Anthony PERARD
  2019-03-26 15:15         ` Laszlo Ersek
  1 sibling, 0 replies; 28+ messages in thread
From: Anthony PERARD @ 2019-03-26 15:14 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Hao Wu, edk2-devel, Julien Grall, Jordan Justen, Ard Biesheuvel,
	Ray Ni

On Tue, Mar 26, 2019 at 04:01:03PM +0100, Laszlo Ersek wrote:
> On 03/26/19 14:03, Anthony PERARD wrote:
> > On Mon, Mar 25, 2019 at 12:29:25PM +0100, Laszlo Ersek wrote:
> >> (4) Julien, Anthony: can you please fetch this series (github URL at the
> >> top) and see if the PS/2 keyboard, and the serial port, still work on
> >> Xen, to interact e.g. with the UEFI shell?
> > 
> > Yes, I can still interact with the UEFI shell with this series applied,
> > both via the console and via VNC (which uses respectively the serial
> > port and PS/2 I think).
> 
> Thank you -- can you please follow up with a Tested-by, or do you prefer
> if I push the set without it?

Tested-by: Anthony PERARD <anthony.perard@citrix.com>

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-26 15:01       ` Laszlo Ersek
  2019-03-26 15:14         ` Anthony PERARD
@ 2019-03-26 15:15         ` Laszlo Ersek
  2019-03-27  0:20           ` Wu, Hao A
  1 sibling, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-26 15:15 UTC (permalink / raw)
  To: Hao Wu; +Cc: Anthony PERARD, Jordan Justen, edk2-devel, Julien Grall

Hi Hao,

On 03/26/19 16:01, Laszlo Ersek wrote:
> On 03/26/19 14:03, Anthony PERARD wrote:
>> On Mon, Mar 25, 2019 at 12:29:25PM +0100, Laszlo Ersek wrote:
>>> (4) Julien, Anthony: can you please fetch this series (github URL at the
>>> top) and see if the PS/2 keyboard, and the serial port, still work on
>>> Xen, to interact e.g. with the UEFI shell?
>>
>> Yes, I can still interact with the UEFI shell with this series applied,
>> both via the console and via VNC (which uses respectively the serial
>> port and PS/2 I think).
> 
> Thank you -- can you please follow up with a Tested-by, or do you prefer
> if I push the set without it?

... actually, I just realize that you can push this series just fine,
after Anthony's answer -- so please excuse my confusing comment; I'll of
course let you push the series.

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-26 15:15         ` Laszlo Ersek
@ 2019-03-27  0:20           ` Wu, Hao A
  2019-03-27  3:37             ` Laszlo Ersek
  0 siblings, 1 reply; 28+ messages in thread
From: Wu, Hao A @ 2019-03-27  0:20 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Anthony PERARD, Justen, Jordan L, edk2-devel@lists.01.org,
	Julien Grall

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, March 26, 2019 11:15 PM
> To: Wu, Hao A
> Cc: Anthony PERARD; Justen, Jordan L; edk2-devel@lists.01.org; Julien Grall
> Subject: Re: [edk2] [PATCH v2 0/3] Ovmf: Stop using ISA drivers within
> IntelFrameworkModulePkg
> 
> Hi Hao,
> 
> On 03/26/19 16:01, Laszlo Ersek wrote:
> > On 03/26/19 14:03, Anthony PERARD wrote:
> >> On Mon, Mar 25, 2019 at 12:29:25PM +0100, Laszlo Ersek wrote:
> >>> (4) Julien, Anthony: can you please fetch this series (github URL at the
> >>> top) and see if the PS/2 keyboard, and the serial port, still work on
> >>> Xen, to interact e.g. with the UEFI shell?
> >>
> >> Yes, I can still interact with the UEFI shell with this series applied,
> >> both via the console and via VNC (which uses respectively the serial
> >> port and PS/2 I think).
> >
> > Thank you -- can you please follow up with a Tested-by, or do you prefer
> > if I push the set without it?
> 
> ... actually, I just realize that you can push this series just fine,
> after Anthony's answer -- so please excuse my confusing comment; I'll of
> course let you push the series.

Hello Laszlo,

Sure, I will push the series.

But one thing to confirm, for patch 2/3, it only got a couple of 'Ack'
tags and a 'Tested-by' tag. I am not sure if this meets the criteria for
pushing.

Best Regards,
Hao Wu

> 
> Thanks
> Laszlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-27  0:20           ` Wu, Hao A
@ 2019-03-27  3:37             ` Laszlo Ersek
  2019-03-27  5:28               ` Wu, Hao A
  0 siblings, 1 reply; 28+ messages in thread
From: Laszlo Ersek @ 2019-03-27  3:37 UTC (permalink / raw)
  To: Wu, Hao A
  Cc: Anthony PERARD, Justen, Jordan L, edk2-devel@lists.01.org,
	Julien Grall

On 03/27/19 01:20, Wu, Hao A wrote:
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Tuesday, March 26, 2019 11:15 PM
>> To: Wu, Hao A
>> Cc: Anthony PERARD; Justen, Jordan L; edk2-devel@lists.01.org; Julien Grall
>> Subject: Re: [edk2] [PATCH v2 0/3] Ovmf: Stop using ISA drivers within
>> IntelFrameworkModulePkg
>>
>> Hi Hao,
>>
>> On 03/26/19 16:01, Laszlo Ersek wrote:
>>> On 03/26/19 14:03, Anthony PERARD wrote:
>>>> On Mon, Mar 25, 2019 at 12:29:25PM +0100, Laszlo Ersek wrote:
>>>>> (4) Julien, Anthony: can you please fetch this series (github URL at the
>>>>> top) and see if the PS/2 keyboard, and the serial port, still work on
>>>>> Xen, to interact e.g. with the UEFI shell?
>>>>
>>>> Yes, I can still interact with the UEFI shell with this series applied,
>>>> both via the console and via VNC (which uses respectively the serial
>>>> port and PS/2 I think).
>>>
>>> Thank you -- can you please follow up with a Tested-by, or do you prefer
>>> if I push the set without it?
>>
>> ... actually, I just realize that you can push this series just fine,
>> after Anthony's answer -- so please excuse my confusing comment; I'll of
>> course let you push the series.
> 
> Hello Laszlo,
> 
> Sure, I will push the series.
> 
> But one thing to confirm, for patch 2/3, it only got a couple of 'Ack'
> tags and a 'Tested-by' tag. I am not sure if this meets the criteria for
> pushing.

Yes, if you get an Acked-by from at least one "M" person for the package
(and no open questions remain), that's sufficient. For edk2, my
interpretation of "Acked-by" is: "although I haven't reviewed in detail,
I agree/approve".

Thanks,
Laszlo


^ permalink raw reply	[flat|nested] 28+ messages in thread

* Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg
  2019-03-27  3:37             ` Laszlo Ersek
@ 2019-03-27  5:28               ` Wu, Hao A
  0 siblings, 0 replies; 28+ messages in thread
From: Wu, Hao A @ 2019-03-27  5:28 UTC (permalink / raw)
  To: Laszlo Ersek, Ard Biesheuvel, Anthony PERARD
  Cc: Justen, Jordan L, edk2-devel@lists.01.org, Julien Grall

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, March 27, 2019 11:37 AM
> To: Wu, Hao A
> Cc: Anthony PERARD; Justen, Jordan L; edk2-devel@lists.01.org; Julien Grall
> Subject: Re: [edk2] [PATCH v2 0/3] Ovmf: Stop using ISA drivers within
> IntelFrameworkModulePkg
> 
> On 03/27/19 01:20, Wu, Hao A wrote:
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Tuesday, March 26, 2019 11:15 PM
> >> To: Wu, Hao A
> >> Cc: Anthony PERARD; Justen, Jordan L; edk2-devel@lists.01.org; Julien Grall
> >> Subject: Re: [edk2] [PATCH v2 0/3] Ovmf: Stop using ISA drivers within
> >> IntelFrameworkModulePkg
> >>
> >> Hi Hao,
> >>
> >> On 03/26/19 16:01, Laszlo Ersek wrote:
> >>> On 03/26/19 14:03, Anthony PERARD wrote:
> >>>> On Mon, Mar 25, 2019 at 12:29:25PM +0100, Laszlo Ersek wrote:
> >>>>> (4) Julien, Anthony: can you please fetch this series (github URL at the
> >>>>> top) and see if the PS/2 keyboard, and the serial port, still work on
> >>>>> Xen, to interact e.g. with the UEFI shell?
> >>>>
> >>>> Yes, I can still interact with the UEFI shell with this series applied,
> >>>> both via the console and via VNC (which uses respectively the serial
> >>>> port and PS/2 I think).
> >>>
> >>> Thank you -- can you please follow up with a Tested-by, or do you prefer
> >>> if I push the set without it?
> >>
> >> ... actually, I just realize that you can push this series just fine,
> >> after Anthony's answer -- so please excuse my confusing comment; I'll of
> >> course let you push the series.
> >
> > Hello Laszlo,
> >
> > Sure, I will push the series.
> >
> > But one thing to confirm, for patch 2/3, it only got a couple of 'Ack'
> > tags and a 'Tested-by' tag. I am not sure if this meets the criteria for
> > pushing.
> 
> Yes, if you get an Acked-by from at least one "M" person for the package
> (and no open questions remain), that's sufficient. For edk2, my
> interpretation of "Acked-by" is: "although I haven't reviewed in detail,
> I agree/approve".

Got it.

Thanks all for the feedbacks and review/test effort.

Series has been pushed via commits:
c455bc8c8d..a068102296

Best Regards,
Hao Wu

> 
> Thanks,
> Laszlo

^ permalink raw reply	[flat|nested] 28+ messages in thread

end of thread, other threads:[~2019-03-27  5:28 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-25  5:28 [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Hao Wu
2019-03-25  5:28 ` [PATCH v2 1/3] OvmfPkg: Drop the ISA Floppy device support Hao Wu
2019-03-25 10:42   ` Laszlo Ersek
2019-03-25  5:28 ` [PATCH v2 2/3] OvmfPkg: Add an Super IO bus driver Hao Wu
2019-03-25 11:22   ` Laszlo Ersek
2019-03-26  2:52     ` Wu, Hao A
2019-03-25 12:00   ` Laszlo Ersek
2019-03-25 17:30     ` Kinney, Michael D
2019-03-25 18:33       ` Laszlo Ersek
2019-03-25 20:02         ` Kinney, Michael D
2019-03-26 11:19           ` Laszlo Ersek
2019-03-25  5:28 ` [PATCH v2 3/3] OvmfPkg: Add a build flag to select ISA driver stack Hao Wu
2019-03-25 11:20   ` Laszlo Ersek
2019-03-25  8:28 ` [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg Ard Biesheuvel
2019-03-25 10:58 ` Laszlo Ersek
2019-03-25 11:29   ` Laszlo Ersek
2019-03-26  2:49     ` Wu, Hao A
2019-03-26 10:14       ` Laszlo Ersek
2019-03-26 11:21         ` Laszlo Ersek
2019-03-26 10:09     ` Julien Grall
2019-03-26 11:53       ` Laszlo Ersek
2019-03-26 13:03     ` Anthony PERARD
2019-03-26 15:01       ` Laszlo Ersek
2019-03-26 15:14         ` Anthony PERARD
2019-03-26 15:15         ` Laszlo Ersek
2019-03-27  0:20           ` Wu, Hao A
2019-03-27  3:37             ` Laszlo Ersek
2019-03-27  5:28               ` Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox