On 01/26/15 22:32, Jordan Justen wrote:
> On 2015-01-24 15:04:52, Laszlo Ersek wrote:
>> +EFI_STATUS
>> +EFIAPI
>> +InstallAllQemuLinkedTables (
>> + IN EFI_ACPI_TABLE_PROTOCOL *AcpiProtocol
>> + );
>
> What do you think about moving this to QemuFwCfgLib instead?
This crossed my mind earlier, but I don't think it's a good idea. Just
because it depends on FwCfg, I don't want to fuse it with the base
library. (Same for QemuBootOrderLib.) ("Base" meaning "foundational",
not "available to all phases", in this context, clearly.)
(1) Main reason is it makes it harder to port the various independent
features gradually, and/or to port them selectively even for the longer
term.
A good negative example is the QemuFwCfgS3Enabled() API. Because it was
so small and so easy to implement for OVMF / x86, we fused it with the
QemuFwCfgLib interface. When implementing the library class for ARM
guests, I had no choice but to implement it too (as "return FALSE")
because it had already been part of the interface. The implementation is
quite useless, and worse, nothing at all calls it in ArmVirtualizationPkg.
> How about:
> RETURN_STATUS
> EFIAPI
> QemuFwCfgInstallAcpiTables (
> VOID
> );
>
> Obviously this should just assert if called in SEC or PEI.
I can rename the function if you'd like, but I think build-time (ie.
interface-level) constraints of an API are superior to runtime asserts.
This holds for both different UEFI phases and different architectures.
You probably don't like the proliferation of small, QEMU-specific
libraries in OVMF. I can appreciate that from an aesthetic POV, but
these features are really this fine-grained, and exposing their
dependencies on the library class level allows me to port them more
flexibly.
(2) Another reason is that by making the QemuFwCfgLib lib class more
comprehensive, code duplication would worsen. The QemuBootOrderLib and
QemuLoaderLib functionality is identical between ARM and x86. The
underlying fw_cfg access / transfer methods are different. If I fused
these all together, I'd either need two full library instances
(duplicating QemuBootOrderLib & QemuLoaderLib just for the sake of the
different fw_cfg access methods), *or* I'd have to keep these all in one
subdirectory somewhere, and use [Sources.AARCH64] and [Sources.X64,
Sources.Ia32] sections in the INF file just for the transfer &
constructor code. I find those sections unwieldy.
What do you think?
Thanks
Laszlo
------------------------------------------------------------------------------
Dive into the World of Parallel Programming. The Go Parallel Website,
sponsored by Intel and developed in partnership with Slashdot Media, is your
hub for all things parallel software development, from weekly thought
leadership blogs to news, videos, case studies, tutorials and more. Take a
look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel
