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

[Laszlo Ersek <lersek@xxxxxxxxxx>]

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@xxxxxxxxxx>
>> ---
>>  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.

Thanks
Laszlo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel