From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-it0-f48.google.com (mail-it0-f48.google.com [209.85.214.48]) (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 E2912803B6 for ; Mon, 13 Mar 2017 08:00:18 -0700 (PDT) Received: by mail-it0-f48.google.com with SMTP id g138so32975459itb.0 for ; Mon, 13 Mar 2017 08:00:18 -0700 (PDT) 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:content-transfer-encoding; bh=tnOX5JGr1AMfGv7j780DApEi9rHUvXgI5yl1MyyebFc=; b=EoFhnichT8jFmdBgC1pKdioYKGKyPeFjYLJJ70Nir1mnnGbISquvKegm4qy4cjH24K +9qYZyWqsuhj2AV33hVtfQhF0Xi7N5jkPUHlY9lT+7zB1RWBoyQSwkWZ4a088SModyTe han6MVXDEi+bvjcLAh491fEUSJFbKB4KPFANc= 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:content-transfer-encoding; bh=tnOX5JGr1AMfGv7j780DApEi9rHUvXgI5yl1MyyebFc=; b=FVkhBSxaesyCNNXor6rScJRuF9mNKiGnGglL3s0XXMewlRmA5hwT757L4UH+KF7TGZ AYF/hAXNhpdC0htsLnQcLWfJ9c8zMW8rD2nmobblm1N/scEZM1XeVSVqVp3tOW4Vb8bB nVUnEodE9LXkxZmovtQjSR5ihrjuKXBXJDEruOGZMZpJ8CC1S17CsOkftT2fzEeiKPwg mkwoW1mQObHWPrMWn8kcaWROXZY3ahYYhjzVw0qjG71SkJa1AkhlA4M+ifaiKIj4mUtl SuarHxuebZIPpyY/+za8We9/61Hf8nXaSzR/tge/VKpcMKBzjR+5cXUXy3MWYw3B32AG lItw== X-Gm-Message-State: AFeK/H2a4d3ikAHBcGz8wC8Nsy+ISsG0LLDhoIMwlG3HaO4dKJul7BwLSzFQfVlZdP+FyHPKDw+MVrg1KJYVNilH X-Received: by 10.36.137.4 with SMTP id s4mr10607056itd.63.1489417158214; Mon, 13 Mar 2017 07:59:18 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.10.27 with HTTP; Mon, 13 Mar 2017 07:59:17 -0700 (PDT) In-Reply-To: <7561917e-fc1c-1675-e672-e0f8b716e4c4@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> <5bdc3cc1-b292-8da3-f858-83c752016941@redhat.com> <20170311102640.GK16034@bivouac.eciton.net> <7561917e-fc1c-1675-e672-e0f8b716e4c4@redhat.com> From: Ard Biesheuvel Date: Mon, 13 Mar 2017 14:59:17 +0000 Message-ID: To: Laszlo Ersek Cc: Leif Lindholm , "edk2-devel@lists.01.org" , Andrew Jones 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: Mon, 13 Mar 2017 15:00:19 -0000 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 13 March 2017 at 14:53, Laszlo Ersek wrote: > On 03/13/17 11:23, Ard Biesheuvel wrote: >> On 11 March 2017 at 10:26, Leif Lindholm wrot= e: >>> On Sat, Mar 11, 2017 at 08:38:19AM +0100, Laszlo Ersek wrote: >>>>>> "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 lift= ed >>>>>> 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? >>>> >>>> I think the neutered ACPI tables (on -no-acpi) should be fine. The >>>> upstream Linux guest will prefer DT if it is present; >>> >>> Yes, but we've already determined that this situation is suboptimal, >>> which was what triggered this changeset to begin with. >>> >> >> Indeed. Having an ACPI entry point config table installed, but not >> having the essential tables that allow you to boot is a situation that >> we should try very hard to avoid IMO >> > > That requires dynamic disabling of AcpiTableDxe -- so that EFI_ACPI_TABLE= _PROTOCOL is not produced, despite the driver being loaded --, and equippin= g all clients of EFI_ACPI_TABLE_PROTOCOL to fail gracefully when the protoc= ol is not found. Alternatively, let AcpiTableDxe install the protocol, but = prevent all clients from calling it. > Not really. We only care about what the OS gets to see, not what the firmware ends up doing internally. So, say, at ReadyToBoot, we could check that the ACPI entry point points to what we need minimally to boot successfully, otherwise we rip it out. This will trigger the recently added change to install the DT if no ACPI entry point was found. Not pretty, I know, but in this case, the handover state when invoking the OS is much more important than clean handling in the firmware itself. > We've done something like in the past with PcdAcpiS3Enable. It required I= ntelFrameworkModulePkg, MdeModulePkg and UefiCpuPkg changes. > > $ git grep -l -w InstallAcpiTable > > ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/AcpiTables.c > ArmVirtPkg/XenAcpiPlatformDxe/XenAcpiPlatformDxe.c > EdkCompatibilityPkg/Foundation/Efi/Protocol/AcpiTable/AcpiTable.h > EmbeddedPkg/Library/AcpiLib/AcpiLib.c > IntelFrameworkModulePkg/Universal/Acpi/AcpiSupportDxe/AcpiSupportAcpiSupp= ortProtocol.c > MdeModulePkg/Universal/Acpi/AcpiPlatformDxe/AcpiPlatform.c > MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.c > MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiSdt.h > MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c > MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsReso= urceTableDxe.c > MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerfo= rmanceDxe.c > MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c > MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c > MdePkg/Include/Protocol/AcpiSystemDescriptionTable.h > MdePkg/Include/Protocol/AcpiTable.h > NetworkPkg/IScsiDxe/IScsiIbft.c > OvmfPkg/AcpiPlatformDxe/AcpiPlatform.c > OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h > OvmfPkg/AcpiPlatformDxe/Qemu.c > OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c > OvmfPkg/AcpiPlatformDxe/Xen.c > QuarkPlatformPkg/Acpi/Dxe/AcpiPlatform/AcpiPlatform.c > QuarkPlatformPkg/Acpi/DxeSmm/SmmPowerManagement/Ppm.c > SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c > SecurityPkg/Tcg/TcgDxe/TcgDxe.c > SecurityPkg/Tcg/TcgSmm/TcgSmm.c > SecurityPkg/Tcg/TrEEDxe/TrEEDxe.c > SecurityPkg/Tcg/TrEESmm/TrEESmm.c > UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.c > > (Some of these hits don't belong to EFI_ACPI_TABLE_PROTOCOL clients, of c= ourse.) > > Thanks > Laszlo