Re: [PATCH 0/6] ArmVirtPkg: don't forward the DT to the OS if QEMU provides ACPI

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

On 9 March 2017 at 12:01, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/09/17 09:16, Ard Biesheuvel wrote:
>> On 8 March 2017 at 20:05, Laszlo Ersek <lersek@redhat.com> wrote:
>>> This series replaces the PURE_ACPI_BOOT_ENABLE build option with dynamic
>>> behavior, matching QEMU's (inverse sense) "-no-acpi" switch. In
>>> particular, DT and ACPI are no longer exposed to the guest at the same
>>> time. (DT is exposed with "-no-acpi", or else ACPI is exposed without
>>> "-no-acpi".)
>>>
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: dynamic_pure_acpi
>>> RHBZ:   https://bugzilla.redhat.com/show_bug.cgi?id=1430262
>>>
>>> Tested with RHEL-7.3 for ARM and Fedora 24 guests (DT vs. ACPI).
>>>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>
>>
>> Hi Laszlo,
>>
>> This looks complicated to me. Given that it is arguably a policy to
>> only expose on h/w description or the other, couldn't we simply remove
>> the FDT config table in BDS if an ACPI/ACPI2.0 config table is
>> present?
>
> Technically we could do that, but I dislike it for two reasons:
>
> - BDS is often the first victim found when looking for a driver to add
> new code to that doesn't seem to fit very well elsewhere. That doesn't
> make BDS any better a recipient, however. "For lack of a better driver"
> is not a strong enough argument to dump code into BDS. If there's really
> no better "topical" driver, then the code usually goes to PlatformDxe.
>
> - Installing a sysconfig table (or any other system-wide resource) in
> one driver, then undoing it in another driver, should be avoided as much
> as possible, because it leads to non-trivial lifecycles and boggles our
> minds over the longer term. If we can come to a decision that the table
> shouldn't be installed in the first place, we should pursue that.
>
> Another approach we could look into is: move the installation of the
> sysconfig table from FdtClientDxe to AcpiPlatformDxe. Look for the ACPI
> payload first, and fall back to installing DT (from within
> AcpiPlatformDxe). However, DT should be installed even in builds (like
> ARM32) that don't contain AcpiPlatformDxe at all.
>

Or we could hook to the ReadyToBoot event in FdtClientDxe, and install
the DT config table if there is no ACPI/ACPI2.0 table registered.

> This series indeed turned out a bit more complex than I had expected,
> but it was the one I could post with a good conscience. Can you perhaps
> identify the part(s) in more detail that seem overly complex to you?
>

Building the same library in two different ways, having to call a
library constructor explicitly in some cases and muck about with TPL
levels to prevent a protocol notify from triggering are all things I
would really like to avoid tbh