Hi Jianyong, On 04/22/21 15:56, Laszlo Ersek wrote:
> (2) "Clh" is a catastrophically bad abbreviation. The whole point of > your work is to add Cloud Hypervisor support, so why trash the most > relevant information in the file names with an inane abbreviation? > > (Not to mention that the name "Cloud Hypervisor" itself is as > nondescript as possible. :/) In an attempt to approach this constructively, I've given it more thought. Does "CloudHv" sound acceptable to the community? I've seen "hv" stand for "hypervisor" frequently. I have another high-level note. I could delay it until after you post v2, but I figure I could save you some time by sharing my observation with you right now. I think that the ACPI platform stuff, in patch#2, does not belong in OvmfPkg/AcpiPlatformDxe. What's more, I don't think it belongs in OvmfPkg, even. The CloudHvAcpiPlatformDxe and CloudHvPlatformHasAcpiDtDxe drivers should exist as stand-alone, self-contained drivers; they should be as minimal as possible. This is already a given for "CloudHvPlatformHasAcpiDtDxe", but it should also be possible for "CloudHvAcpiPlatformDxe". OvmfPkg/AcpiPlatformDxe is a complex driver, and the overlap between what OvmfPkg/AcpiPlatformDxe currently does, and what CloudHvAcpiPlatformDxe actually *needs*, is virtually nil. And so, the series shouldn't touch OvmfPkg at all. Ultimately I suggest following the Xen pattern that can be seen under ArmVirtPkg already. In detail, the following files and directories should contain the new platform: ArmVirtPkg/ArmVirtCloudHv.dsc ArmVirtPkg/ArmVirtCloudHv.fdf ArmVirtPkg/CloudHvAcpiPlatformDxe/ ArmVirtPkg/CloudHvPlatformHasAcpiDtDxe/ ArmVirtPkg/Library/CloudHvVirtMemInfoLib/ (And I don't really see the point of an FDF include file.) Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#74391): https://edk2.groups.io/g/devel/message/74391 Mute This Topic: https://groups.io/mt/82285671/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
