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

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

On 29 March 2017 at 18:01, Marc Zyngier <marc.zyngier@arm.com> 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.
>

So to clarify, you want something sticky in the firmware settings
rather than having to use the -no-acpi command line argument to 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
>

DT is always available, so this condenses to

> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==1 and ACPI+DT present -> DT

unless -no-acpi is set, which gives us

> - PreferDT==0 and DT present      -> DT
> - 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).
>

For symmetry, it would make sense to call it ForceNoAcpi then, after
the command line param.

> Could people agree on something like this?
>

Works for me.