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

[Laszlo Ersek <lersek@redhat.com>]

On 03/29/17 19:07, Ard Biesheuvel wrote:
> 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?

If you *really* want to control it within the guest, on the guest
firmware level, while keeping the exclusive nature, then there are
better options for that.

Namely, a variation of your HII-exposed driver added in:

https://github.com/tianocore/edk2/commit/779cc439e881

In this case, PlatformHasAcpiDtDxe should be replaced with a simple HII
driver that exposes the same exclusive toggle as an HII checkbox. The
HII checkbox would control the installation of EdkiiPlatformHasAcpiGuid
vs EdkiiPlatformHasDeviceTreeGuid.

Note that I disagree with *combining* a HII toggle with the current
fw_cfg-based knob (that is, the -no-acpi switch). That means there are
multiple masters for the same decision, which invariably leads to confusion.

Therefore any ArmVirtQemu platform build should either offer the fw_cfg
swtich (== -no-acpi QEMU command line option), *or* the HII toggle.
Never both.

In fact I've fully expected Ard to post such an HII driver down the
road, for the (upcoming) "showcase QEMU" virtualization platform (and
QEMU  machine type). The target environment of that platform wouldn't be
the data center (where you really want to control most things from the
host side) -- the goal would be to model physical hardware very closely,
even as far as human interaction goes.

So, let us figure out what we ultimately want (well, I know what I want,
what do you guys want)?

- For the current (data center virtualization oriented case), the
DT<->ACPI exclusion is controlled by QEMU's -no-acpi flag.
- For the "showcase QEMU" case, another HII driver would be necessary.
DT and ACPI would remain exclusive, but they would be controlled
(visually, interactively) from the guest firmware.
- Both of these control methods should never be enabled in the same
firmware build, at the same time. If Ard writes the HII driver, we can
introduce a build time flag that switches between the two control
methods. In no case would it be possible for DT and ACPI to appear together.

Thanks
Laszlo