From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 552D4208F7A02 for ; Wed, 29 Mar 2017 10:23:41 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BADD08E24F; Wed, 29 Mar 2017 17:23:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BADD08E24F Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com BADD08E24F Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-134.phx2.redhat.com [10.3.116.134]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4B1FB78B85; Wed, 29 Mar 2017 17:23:39 +0000 (UTC) To: Ard Biesheuvel , Marc Zyngier References: <20170329151940.23397-1-ard.biesheuvel@linaro.org> <4bdde09a-cf2e-33fb-967e-97e69e5be892@redhat.com> <2fb8acda-2786-e3de-e035-32d13e3f5868@arm.com> Cc: "edk2-devel@lists.01.org" , Jon Masters , Drew Jones From: Laszlo Ersek Message-ID: Date: Wed, 29 Mar 2017 19:23:38 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Wed, 29 Mar 2017 17:23:40 +0000 (UTC) Subject: Re: [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 29 Mar 2017 17:23:41 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/29/17 19:07, Ard Biesheuvel wrote: > On 29 March 2017 at 18: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 wrote: >>>>> On 03/29/17 18:02, Ard Biesheuvel wrote: >>>>>> On 29 March 2017 at 17:00, Laszlo Ersek 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 >>>>>>>> --- >>>>>>>> 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 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