From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-x22b.google.com (mail-it0-x22b.google.com [IPv6:2607:f8b0:4001:c0b::22b]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 31FBF803A7 for ; Fri, 10 Mar 2017 22:57:14 -0800 (PST) Received: by mail-it0-x22b.google.com with SMTP id m27so8349949iti.1 for ; Fri, 10 Mar 2017 22:57:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=DyiqEJf+1vTXr/UpOuVLc+TlzBffZwC4JjkeJe8SY8E=; b=WIewNKwyWuwdGicvm5EThgLhpETmXVkUp4HvSWgSK5sPSqkGSJmA0lKtMbOBSZXJwI 4MwHL6nzs0XWz2OLyoMJ3GQXLkrpJ15DCVM6VQ30Jw+92wLHyOHD6HDDJcH3l6wDSJ9l AFcue31zwbSeoO/ykE7s2JmZy1occiCpXTb+4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=DyiqEJf+1vTXr/UpOuVLc+TlzBffZwC4JjkeJe8SY8E=; b=DGsCRdC0EAjIVz5RUxX5YWYBLW1WfKVv+I5Mu2tKN3gamzcA4oycNjG3D7j0s6MGdy +iQFPDohVskTWweB2ePf11EbmzOG6WO75fvfE6R55ieUhXLzILWM1z7CRdK3S55EbxrP onphI5OW2AZg10bRQG02a2dYqpOngcKhT6R9eBxF2KCyKzjBNKvi3rgeAWwfHZ/TE12b BqZcHujZrpyXIRc3JRPtWbaZhht984z0OyEt7qI0Bx6/ZclcPw3be96ZwLbVDNLVeMm5 j4EM1ksXaDQvb1zILcR/AmdJpWRwRk+3mCq8nKR74tI4RV66aiJ8FiP9eTohNDRWQDlW bRWw== X-Gm-Message-State: AFeK/H2/dBTR0ZF6y3GiZ7tcwoPkqq84i8YmkLo4fB5QYdAMwrf6FXzdBjvAQDFJqUZ3C6/8vyonSEGlMSOAphgO X-Received: by 10.36.23.74 with SMTP id 71mr2411752ith.37.1489215433412; Fri, 10 Mar 2017 22:57:13 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Fri, 10 Mar 2017 22:57:13 -0800 (PST) In-Reply-To: <19f70c3d-4220-957b-24cd-bd2d41e45909@redhat.com> References: <1489075441-23745-1-git-send-email-ard.biesheuvel@linaro.org> <1489075441-23745-4-git-send-email-ard.biesheuvel@linaro.org> <19f70c3d-4220-957b-24cd-bd2d41e45909@redhat.com> From: Ard Biesheuvel Date: Sat, 11 Mar 2017 07:57:13 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Andrew Jones , Leif Lindholm Subject: Re: [PATCH 3/3] ArmVirtPkg/FdtClientDxe: make DT table installation !ACPI dependent X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 11 Mar 2017 06:57:14 -0000 Content-Type: text/plain; charset=UTF-8 On 11 March 2017 at 06:55, Laszlo Ersek wrote: > On 03/09/17 17:04, Ard Biesheuvel wrote: >> Instead of having a build time switch to prevent the FDT configuration >> table from being installed, make this behavior dependent on whether we >> are passing ACPI tables to the OS. This is done by looking for the >> ACPI 2.0 configuration table, and only installing the FDT one if the >> ACPI one cannot be found. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel >> --- >> ArmVirtPkg/ArmVirtPkg.dec | 10 ---------- >> ArmVirtPkg/ArmVirtQemu.dsc | 5 ----- >> ArmVirtPkg/FdtClientDxe/FdtClientDxe.c | 15 ++++++++++----- >> ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf | 5 ++--- >> 4 files changed, 12 insertions(+), 23 deletions(-) >> >> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec >> index a5ec42166445..efe83a383d55 100644 >> --- a/ArmVirtPkg/ArmVirtPkg.dec >> +++ b/ArmVirtPkg/ArmVirtPkg.dec >> @@ -58,13 +58,3 @@ [PcdsFixedAtBuild, PcdsPatchableInModule] >> # EFI_VT_100_GUID. >> # >> gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 0x4D}|VOID*|0x00000007 >> - >> -[PcdsFeatureFlag] >> - # >> - # Pure ACPI boot >> - # >> - # Inhibit installation of the FDT as a configuration table if this feature >> - # PCD is TRUE. Otherwise, the OS is presented with both a DT and an ACPI >> - # description of the platform, and it is up to the OS to choose. >> - # >> - gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|FALSE|BOOLEAN|0x0000000a >> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc >> index 477dfdcfc764..7b266b98b949 100644 >> --- a/ArmVirtPkg/ArmVirtQemu.dsc >> +++ b/ArmVirtPkg/ArmVirtQemu.dsc >> @@ -34,7 +34,6 @@ [Defines] >> # -D FLAG=VALUE >> # >> DEFINE SECURE_BOOT_ENABLE = FALSE >> - DEFINE PURE_ACPI_BOOT_ENABLE = FALSE >> >> !include ArmVirtPkg/ArmVirt.dsc.inc >> >> @@ -94,10 +93,6 @@ [PcdsFeatureFlag.common] >> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE >> gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE >> >> -!if $(PURE_ACPI_BOOT_ENABLE) == TRUE >> - gArmVirtTokenSpaceGuid.PcdPureAcpiBoot|TRUE >> -!endif >> - >> [PcdsFixedAtBuild.common] >> gArmPlatformTokenSpaceGuid.PcdCoreCount|1 >> !if $(ARCH) == AARCH64 >> diff --git a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c >> index 0327af5739f2..2981977f3d20 100644 >> --- a/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c >> +++ b/ArmVirtPkg/FdtClientDxe/FdtClientDxe.c >> @@ -17,6 +17,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> >> @@ -312,12 +313,16 @@ OnReadyToBoot ( >> ) >> { >> EFI_STATUS Status; >> + VOID *Table; >> >> - if (!FeaturePcdGet (PcdPureAcpiBoot)) { >> - // >> - // Only install the FDT as a configuration table if we want to leave it up >> - // to the OS to decide whether it prefers ACPI over DT. >> - // >> + // >> + // Only install the FDT as a configuration table if we are not exposing >> + // ACPI 2.0 (or later) tables. Note that the legacy ACPI table GUID has >> + // no meaning on ARM since we need at least ACPI 5.0 support, and the >> + // 64-bit ACPI 2.0 table GUID is mandatory in that case. >> + // >> + Status = EfiGetSystemConfigurationTable (&gEfiAcpi20TableGuid, &Table); >> + if (EFI_ERROR (Status) || Table == NULL) { >> Status = gBS->InstallConfigurationTable (&gFdtTableGuid, mDeviceTreeBase); >> ASSERT_EFI_ERROR (Status); >> } > > This code breaks the DT-only ("-no-acpi") boot with boot graphics > (virtio-gpu) enabled. > > Namely, we recently included BootGraphicsResourceTableDxe in the build. > That driver calls InstallAcpiTable() for BGRT, which in turn causes > AcpiTableDxe to call InstallConfigurationTable(), via PublishTables(). > > We all missed that just because QEMU doesn't produce ACPI payload (and > we consequently don't install it), other drivers in edk2 may > unconditionally install "auxiliary" ACPI tables, which minimally trigger > the presence of the RSD PTR, RSDT and XSDT, and prevent DT installation. > Such a crippled set of ACPI tables isn't sufficient for booting however. > > We have functions like FindAcpiFacsTableByAcpiGuid(), ScanTableInXSDT() > and ScanTableInRSDT() in > "MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c". I > think the above check should be reworked to look for the FADT > (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) with code lifted > from these helper functions. No driver outside of > QemuFwCfgAcpiPlatformDxe will install the FADT. And, the FADT will > always be part of QEMU's ACPI payload, if it generates one. > OK, that would get things working again, I suppose. But do we want neutered ACPI tables to be exposed at all, even if there is a DT in that case to boot from?