Re: [edk2-devel] [PATCH v2 00/12] OvmfPkg: Replace the OVMF-specific SataControllerDxe with the generic one

Re: [edk2-devel] [PATCH v2 00/12] OvmfPkg: Replace the OVMF-specific SataControllerDxe with the generic one

[Pedro Falcato]

On Tue, May 9, 2023 at 5:32 PM Pedro Falcato via groups.io
<pedro.falcato=gmail.com@groups.io> wrote:
>
> This patch-set replaces the OVMF specific SataControllerDxe with the MdeModulePkg/Bus/Pci one.
> They were both forked from the same code, and are code-and-functionality similar. As such, there
> seems to be no need for duplication here.
>
> The first four pages both replay OvmfPkg/SataControllerDxe commits and fix up a couple of details.
> The rest of the patches (minus the last) replace OvmfPkg/SataControllerDxe with the MdeModulePkg variant.
> The last patch, finally, removes the thing.
>
> Tested by booting in QEMU (Q35 (AHCI) and PC (IDE)).
> More testing from other, alternative platforms is desired, although breakage seems unlikely.
>
> This patchset (or at least, the last patch) depends on the edk2-platforms patch series to get merged.
>
> (+CC Laszlo as the author of the original SataControllerDxe patches)
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc :Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Cc: Rebecca Cran <rebecca@bsdio.com>
> Cc: Peter Grehan <grehan@freebsd.org>
> Cc: Corvin Köhne <corvink@freebsd.org>
> Cc: Sebastien Boeuf <sebastien.boeuf@intel.com>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien@xen.org>
> Cc: Laszlo Ersek <lersek@redhat.com>
>
> Changes:
> v2:
>   - Address Laszlo's feedback wrt commit to port into MdeModulePkg
>   - Address Laszlo's feedback wrt how to split the removal of the driver and the patches for each sub-platform maintainer
>   - Address Mike Maslenkin's feedback on the preexisting ASSERTs (Private != NULL)
>   - Add Gerd's ACK and Tested-by (conservatively) to patches 6 and 12 (since they are remotely QEMU related)
>
> Pedro Falcato (12):
>   MdeModulePkg/SataControllerDxe: Clean up error handling in Start()
>   MdeModulePkg/SataControllerDxe: Log expected errors at DEBUG_INFO
>     level
>   MdeModulePkg/SataControllerDxe: Remove useless null check
>   MdeModulePkg/SataControllerDxe: Fix up ASSERTS (Private != NULL)
>   OvmfPkg: Replace the OVMF-specific SataControllerDxe
>   OvmfPkg/Microvm: Replace the OVMF-specific SataControllerDxe
>   OvmfPkg/Bhyve: Replace the OVMF-specific SataControllerDxe
>   OvmfPkg/CloudHv: Replace the OVMF-specific SataControllerDxe
>   OvmfPkg/IntelTdx: Replace the OVMF-specific SataControllerDxe
>   OvmfPkg/AmdSev: Replace the OVMF-specific SataControllerDxe
>   OvmfPkg/Xen: Replace the OVMF-specific SataControllerDxe
>   OvmfPkg: Remove SataControllerDxe
>
>  .../Pci/SataControllerDxe/SataController.c    |  146 ++-
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |    2 +-
>  OvmfPkg/AmdSev/AmdSevX64.fdf                  |    2 +-
>  OvmfPkg/Bhyve/BhyveX64.dsc                    |    2 +-
>  OvmfPkg/Bhyve/BhyveX64.fdf                    |    2 +-
>  OvmfPkg/CloudHv/CloudHvX64.dsc                |    2 +-
>  OvmfPkg/CloudHv/CloudHvX64.fdf                |    2 +-
>  OvmfPkg/IntelTdx/IntelTdxX64.dsc              |    2 +-
>  OvmfPkg/IntelTdx/IntelTdxX64.fdf              |    2 +-
>  OvmfPkg/Microvm/MicrovmX64.dsc                |    2 +-
>  OvmfPkg/Microvm/MicrovmX64.fdf                |    2 +-
>  OvmfPkg/OvmfPkgIa32.dsc                       |    2 +-
>  OvmfPkg/OvmfPkgIa32.fdf                       |    2 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |    2 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf                    |    2 +-
>  OvmfPkg/OvmfPkgX64.dsc                        |    2 +-
>  OvmfPkg/OvmfPkgX64.fdf                        |    2 +-
>  OvmfPkg/OvmfXen.dsc                           |    2 +-
>  OvmfPkg/OvmfXen.fdf                           |    2 +-
>  OvmfPkg/SataControllerDxe/ComponentName.c     |  170 ---
>  OvmfPkg/SataControllerDxe/SataController.c    | 1112 -----------------
>  OvmfPkg/SataControllerDxe/SataController.h    |  544 --------
>  .../SataControllerDxe/SataControllerDxe.inf   |   43 -
>  23 files changed, 90 insertions(+), 1961 deletions(-)
>  delete mode 100644 OvmfPkg/SataControllerDxe/ComponentName.c
>  delete mode 100644 OvmfPkg/SataControllerDxe/SataController.c
>  delete mode 100644 OvmfPkg/SataControllerDxe/SataController.h
>  delete mode 100644 OvmfPkg/SataControllerDxe/SataControllerDxe.inf

... I knew I forgot something...
PR + git branch: https://github.com/tianocore/edk2/pull/4370

-- 
Pedro