Re: [edk2-devel] [PATCH v2 02/31] OvmfPkg: Create platform XenOvmf

["Ard Biesheuvel" <ard.biesheuvel@linaro.org>]

On Wed, 10 Apr 2019 at 07:27, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 04/10/19 11:48, Jordan Justen wrote:
> > On 2019-04-09 04:08:15, Anthony PERARD wrote:
> >> This is a copy of OvmfX64, removing VirtIO and some SMM.
> >>
> >> This new platform will be changed to make it works on two types of Xen
> >> guest: HVM and PVH.
> >>
> >> Compare to OvmfX64, this patch:
> >>
> >> - changed: PLATFORM_GUID, OUTPUT_DIRECTORY, FLASH_DEFINITION
> >> - removed: VirtioLib class resolution
> >> - removed: all UEFI_DRIVER modules for virtio devices
> >> - removed: DXE_SMM_DRIVER and SMM_CORE lib class resolutions
> >> - removed: DXE_SMM_DRIVER and SMM_CORE FDF rules
> >> - removed: Everything related to SMM_REQUIRE==true
> >> - removed: Everything related to SECURE_BOOT_ENABLE==true
> >> - removed: Everything related to TPM2_ENABLE==true
> >> - changed: PcdPciDisableBusEnumeration dynamic default flipped to TRUE
> >> - changed: default FD_SIZE_IN_KB to 2M.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> >> ---
> >>  OvmfPkg/{OvmfPkgX64.dsc => XenOvmf.dsc}     | 202 +-------------------
> >
> > I guess we all want our name to be first :), but OvmfXen seems more
> > like the pattern that edk2 uses when making sub-platforms.
> >
> > Also, in some cases we've used !ifdef type things to keep dsc and fdf
> > files common. Would that not be workable here? Maybe it would get too
> > ugly. :\
>
> I've been happy with a similar Xen<->QEMU split under ArmVirtPkg.
> Duplications in updates are usually not a huge burden, and keeping the
> files separate has proved very helpful for determining
> maintainer/reviewer/tester responsibility.
>
> > It could help to prevent having to sync dsc changes across, and
> > prevent changes from being omitted for Xen on accident.
>
> True, but in my experience that's been the smaller problem. The larger
> problem has been modifying Xen stuff in unintended ways (= regressing
> Xen), because we can't test (or don't even notice) the Xen-side
> implications of changes made to common source.
>
> Personally I prefer another (DSC + FDF) pair under OvmfPkg (beyond the
> three we already have) to another layer of (conditional?) includes. The
> !if's that are being eliminated in this Xen-customized copy (i.e., in
> this patch) are complex enough already :)
>
> ... It's not that I *generally* prefer duplication; as you say, we do
> have a number of !includes already. I do prefer duplication specifically
> for Xen vs. QEMU however.
>

+1