Re: [PATCH v2 0/3] Ovmf: Stop using ISA drivers within IntelFrameworkModulePkg

[Laszlo Ersek <lersek@redhat.com>]

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
>