Re: [PATCH 2/2] OvmfPkg: Replace the OVMF-specific SataControllerDxe with a generic one

["Laszlo Ersek" <lersek@redhat.com>]

On 5/8/23 23:52, Pedro Falcato wrote:
> Previously, OVMF had forked DuetPkg's SataControllerDxe (see commit
> 12e92a2). However, in commit fda951d a generic SataControllerDxe was
> added to MdeModulePkg/Bus/Pci.
> Since they are most similar (both code-wise and functionally), let's
> unify them and de-duplicate code.
> 
> Tested by booting in QEMU, in both Q35 and PC (to test IDE and AHCI
> functionality).
> 
> 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>
> Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> ---
>  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 -
>  22 files changed, 18 insertions(+), 1887 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

Just to make this patch a bit more tractable, I'd suggest splitting it.

First, update only the DSC/FDF files. In particular, if you do that
alongside review/maintainer responsibilities -- that is, for example,
you create a separate FDF+DSC patch for Bhyve and another FDF+DSC patch
for Xen --, then your reviewers will thank you for the effort, as they
won't have to wade through platform DSC+FDF code they don't care about.

Second, removing "OvmfPkg/SataControllerDxe" can be its own patch at the
very end of the series; and that one need not be CC'd to the various
platform maintainers. With smaller / more focused patches,
"GetMaintainer.py" will provide more targeted CC lists.

Thanks!
Laszlo