[edk2-devel] [PATCH v1 1/4] ArmVirtPkg: Library: Memory initialization for Cloud Hypervisor

Jianyong Wu jianyong.wu at arm.com
Mon Apr 26 10:29:19 UTC 2021


Hi Laszlo,

> -----Original Message-----
> From: Laszlo Ersek <lersek at redhat.com>
> Sent: Friday, April 23, 2021 8:00 PM
> To: Jianyong Wu <Jianyong.Wu at arm.com>; edk2-devel-groups-io
> <devel at edk2.groups.io>; Sami Mujawar <Sami.Mujawar at arm.com>
> Cc: Justin He <Justin.He at arm.com>; Ard Biesheuvel
> <ardb+tianocore at kernel.org>; Leif Lindholm <leif at nuviainc.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/4] ArmVirtPkg: Library: Memory
> initialization for Cloud Hypervisor
>
> 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.
>
>
Yeah, CloudHv is better, as the original name is too long. I will take it as the abbreviation of Cloud Hypervisor.

> 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/
>
Ok , it seems more coherent. I will reorganize the files according to Acpi.

> (And I don't really see the point of an FDF include file.)

Yeah, I can include them into the fdf file directly.

Thanks
Jianyong

>
> Thanks!
> Laszlo

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#74420): https://edk2.groups.io/g/devel/message/74420
Mute This Topic: https://groups.io/mt/82285671/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-






More information about the edk2-devel-archive mailing list