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

[Laszlo Ersek <lersek@redhat.com>]

On 03/29/17 19:30, Marc Zyngier wrote:
> On 29/03/17 18:15, Laszlo Ersek wrote:
>> 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.
> 
> You're missing the essential bit. I don't want a knob in QEMU. Having to
> mess with QEMU means that I can't have a uniform way of running a VM. I
> want one way of booting a VM, and let the VM pick the description it has
> been configured for.
> 
> On bare metal, I can switch UEFI from a menu entry to pick ACPI or DT.
> And that's good. I want the same freedom in a VM, at the same level.
> There is no technical reason to have a different behaviour.

Correct, you can have the exact same menu entry (which is what I've been
calling the HII checkbox) in virtual firmware, assuming someone writes
the platform DXE driver that implements this kind of policy.

As I wrote, I expected Ard to submit (at some undeterminate time in the
future) such a driver, which would replace the current policy drivers in:

- the Xen build of ArmVirtQemu, which has no access to fw_cfg, and

- the upcoming "showcase QEMU" platform, which would entirely ignore
fw_cfg for this purpose.

Laszlo