Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

[Laszlo Ersek <lersek@redhat.com>]

On 03/29/17 19:01, Marc Zyngier wrote:
> On 29/03/17 17:40, Laszlo Ersek wrote:
>> On 03/29/17 18:07, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:03, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>>>> On 29 March 2017 at 17:00, Laszlo Ersek <lersek@redhat.com> wrote:
>>>>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>>>>>> In general, we should not present two separate (and inevitably different)
>>>>>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>>>>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>>>>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>>>>>> vice versa.
>>>>>>>
>>>>>>> However, this is arguably a regression for those who rely on both
>>>>>>> descriptions being available, even if the use cases in question are
>>>>>>> uncommon.
>>>>>>>
>>>>>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>>>>>> will result in both descriptions being exposed on the next boot, if
>>>>>>> executing in the default 'ACPI-only' mode.
>>>>>>>
>>>>>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>>>>>
>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>>>>> ---
>>>>>>>  ArmVirtPkg/ArmVirtPkg.dec                                | 8 ++++++++
>>>>>>>  ArmVirtPkg/ArmVirtQemu.dsc                               | 3 +++
>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++++++-
>>>>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +++++
>>>>>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>>>>>
> 
> [snip the policy argumentation, I only care about the technical aspects]
> 
>> On the technical side:
>>
>> - I think a dynamic boolean PCD would be superior, if that is possible
>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>> should map to FALSE. I'm unsure though if that were as easy to set from
>> the UEFI shell as a UINT8. So stick with the current data type if you
>> deem that superior (maybe comment on it in the commit message).
>>
>> - please include <Library/PcdLib.h> in the C source, to reflect the
>> [LibraryClasses] update in the INF.
> 
> My personal choice would be *not* to expose both tables at the same
> time, but instead to be able to shift the preference from one side or
> the other, similarly to what a menu on a bare metal system would do.

Umm... Are we in violent agreement? This works already.

If you invoke QEMU with the normal command like, like you've always
done, you get ACPI only (right now).

If you pass the "-no-acpi" switch in addition to your normal command
line, you get DT only. This is documented in detail on the following commit:

https://github.com/tianocore/edk2/commit/110316a995ed

If you pass -no-acpi on your QEMU command line, you can perform the
whole guest kernel bisection using purely DT, without ever having to
re-launch QEMU.

> 
> Lets call the variable PreferDT (rather than ForceDT), with the
> following (exhaustive) behaviour :
> 
> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==0 and ACPI present    -> ACPI
> - PreferDT==0 and DT present      -> DT
> - PreferDT==1 and ACPI+DT present -> DT
> - PreferDT==1 and ACPI present    -> ACPI
> - PreferDT==1 and DT present      -> DT
> 
> It allows people with existing setups to still have something that works
> with minimal effort (still need to set this variable though).
> 
> Could people agree on something like this?

It's overly complex. With QEMU (and the current edk2 master state) you
already have a single (host-side) master knob for controlling the ACPI
vs. DT exposure.

Laszlo