public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Marc Zyngier <marc.zyngier@arm.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	Jon Masters <jcm@redhat.com>, Drew Jones <drjones@redhat.com>
Subject: Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation
Date: Wed, 29 Mar 2017 20:04:42 +0200	[thread overview]
Message-ID: <e818ba75-1c81-1455-68cd-4fa06c908778@redhat.com> (raw)
In-Reply-To: <010325b6-1c23-5da7-9df5-337619c519bb@arm.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


  parent reply	other threads:[~2017-03-29 18:04 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-29 15:19 [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation Ard Biesheuvel
2017-03-29 16:00 ` Laszlo Ersek
2017-03-29 16:02   ` Ard Biesheuvel
2017-03-29 16:03     ` Laszlo Ersek
2017-03-29 16:07       ` Ard Biesheuvel
2017-03-29 16:40         ` Laszlo Ersek
     [not found]           ` <2fb8acda-2786-e3de-e035-32d13e3f5868@arm.com>
2017-03-29 17:07             ` Ard Biesheuvel
2017-03-29 17:23               ` Laszlo Ersek
2017-03-29 17:30                 ` Ard Biesheuvel
2017-03-29 17:50                   ` Laszlo Ersek
2017-03-29 17:15             ` Laszlo Ersek
     [not found]               ` <010325b6-1c23-5da7-9df5-337619c519bb@arm.com>
2017-03-29 18:04                 ` Laszlo Ersek [this message]
2017-03-29 17:16           ` Ard Biesheuvel
     [not found]       ` <e78f315f-1e84-cc6c-ab41-fe98b842af21@arm.com>
2017-03-29 17:03         ` Laszlo Ersek
     [not found]   ` <7F72156F-3814-4CF1-8847-A7272409A85E@redhat.com>
2017-03-29 16:17     ` Ard Biesheuvel
2017-03-29 16:55       ` Laszlo Ersek
2017-03-29 17:44         ` Mark Rutland
2017-03-29 17:58           ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e818ba75-1c81-1455-68cd-4fa06c908778@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox