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 F290E80429 for ; Fri, 17 Mar 2017 14:25:47 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6BA39804E2; Fri, 17 Mar 2017 21:25:48 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 6BA39804E2 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=lersek@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 6BA39804E2 Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-70.phx2.redhat.com [10.3.116.70]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6D7645C6C9; Fri, 17 Mar 2017 21:25:47 +0000 (UTC) To: Ard Biesheuvel References: <20170317204731.31488-1-lersek@redhat.com> <20170317204731.31488-12-lersek@redhat.com> Cc: edk2-devel-01 , Leif Lindholm From: Laszlo Ersek Message-ID: Date: Fri, 17 Mar 2017 22:25:45 +0100 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.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Fri, 17 Mar 2017 21:25:48 +0000 (UTC) Subject: Re: [PATCH v2 11/12] ArmVirtPkg/PlatformHasAcpiDtDxe: don't expose DT if QEMU provides ACPI 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: Fri, 17 Mar 2017 21:25:48 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 03/17/17 22:10, Ard Biesheuvel wrote: > On 17 March 2017 at 20:47, Laszlo Ersek wrote: >> This will let QEMU's "-no-acpi" option exclusively expose DT vs. ACPI to >> the guest. Showing both is never needed (it is actually detrimental to the >> adoption of standards, such as SBSA / SBBR). >> >> * Without "-no-acpi", the firmware logs (from PlatformHasAcpiDtDxe) >> >>> Found FwCfg @ 0x9020008/0x9020000 >>> Found FwCfg DMA @ 0x9020010 >>> InstallProtocolInterface: [EdkiiPlatformHasAcpiProtocol] 0 >> >> plus the usual messages. Later the guest kernel logs >> >>> [ 0.000000] efi: SMBIOS 3.0=0x13bdb0000 ACPI 2.0=0x138440000 > > Why is there no MEMATTR table in this case? Or was it omitted for brevity? I don't have the slightest clue. What is the MEMATTR table? ... Hm, grepping the kernel, it's apparently the memory attributes table. You added it in the following kernel commit: a604af075a32 ("efi: Add support for the EFI_MEMORY_ATTRIBUTES_TABLE config table", 2016-04-25) and it is part of release v4.7. Ah, I understand now. I used two kernels (two guests) for testing this, namely "4.7.7-200.fc24.aarch64" and "4.5.0-15.el7.aarch64". And, from the logs I collected, the "-no-acpi" log belongs to the Fedora kernel, which has your MEMATTR patch; while the log without "-no-acpi" (above) belongs to the RHEL-7.3 kernel, which does *not* have your MEMATTR patch. I've now removed "-no-acpi" from the Fedora 24 guest's config, and repeated the test. It prints: [ 0.000000] efi: SMBIOS 3.0=0x13bdb0000 ACPI 2.0=0x138440000 MEMATTR=0x13a689018 If you wish (and I don't have to submit a v3 for any other reason), I can refresh the above line in the commit message. Thanks! Laszlo > >> >> before it lists the ACPI tables one by one. >> >> * With "-no-acpi", the firmware logs: >> >>> PlatformHasAcpiDtDxe | Found FwCfg @ 0x9020008/0x9020000 >>> PlatformHasAcpiDtDxe | Found FwCfg DMA @ 0x9020010 >>> PlatformHasAcpiDtDxe | InstallProtocolInterface: >>> PlatformHasAcpiDtDxe | [EdkiiPlatformHasDeviceTreeProtocol] 0 >>> FdtClientDxe | OnPlatformHasDeviceTree: exposing DTB @ >>> FdtClientDxe | 0x13FFBF000 to OS >>> ... >>> DXE_CORE | Driver [AcpiTableDxe] was discovered but not >>> DXE_CORE | loaded!! >>> DXE_CORE | Driver [QemuFwCfgAcpiPlatform] was discovered but >>> DXE_CORE | not loaded!! >>> ... >>> RamDiskDxe | RamDiskAcpiCheck: Cannot locate the EFI ACPI >>> RamDiskDxe | Table Protocol, unable to publish RAM disks to >>> RamDiskDxe | NFIT. >> >> (BootGraphicsResourceTableDxe's ReadyToBoot callback -- >> InstallBootGraphicsResourceTable() -- handles the lack of >> EFI_ACPI_TABLE_PROTOCOL silently.) Later the guest kernel logs >> >>> [ 0.000000] efi: SMBIOS 3.0=0x13bdb0000 MEMATTR=0x138b3c018 >> >> Cc: Ard Biesheuvel >> Cc: Leif Lindholm >> Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1430262 >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Laszlo Ersek >> --- >> ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 6 +-- >> ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c | 45 ++++++++++++-------- >> 2 files changed, 29 insertions(+), 22 deletions(-) >> >> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf >> index 7724cf215dda..95a56a10bb5c 100644 >> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf >> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf >> @@ -28,11 +28,12 @@ [Packages] >> ArmPkg/ArmPkg.dec >> ArmVirtPkg/ArmVirtPkg.dec >> MdePkg/MdePkg.dec >> + OvmfPkg/OvmfPkg.dec >> >> [LibraryClasses] >> BaseLib >> DebugLib >> - PcdLib >> + QemuFwCfgLib >> UefiBootServicesTableLib >> UefiDriverEntryPoint >> >> @@ -40,8 +41,5 @@ [Protocols] >> gEdkiiPlatformHasAcpiProtocolGuid ## SOMETIMES_PRODUCES >> gEdkiiPlatformHasDeviceTreeProtocolGuid ## SOMETIMES_PRODUCES >> >> -[FeaturePcd] >> - gArmVirtTokenSpaceGuid.PcdPureAcpiBoot ## CONSUMES >> - >> [Depex] >> TRUE >> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c >> index 8681f813a6b5..46f96401632c 100644 >> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c >> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c >> @@ -15,7 +15,7 @@ >> >> #include >> #include >> -#include >> +#include >> #include >> #include >> #include >> @@ -27,18 +27,27 @@ PlatformHasAcpiDt ( >> IN EFI_SYSTEM_TABLE *SystemTable >> ) >> { >> - EFI_STATUS Status; >> - >> - Status = EFI_SUCCESS; >> + EFI_STATUS Status; >> + FIRMWARE_CONFIG_ITEM FwCfgItem; >> + UINTN FwCfgSize; >> >> // >> // If we fail to install any of the necessary protocols below, the OS will be >> // unbootable anyway (due to lacking hardware description), so tolerate no >> // errors here. >> // >> - // Always make ACPI available on 64-bit systems. >> - // >> - if (MAX_UINTN == MAX_UINT64) { >> + if (MAX_UINTN == MAX_UINT64 && >> + !EFI_ERROR ( >> + QemuFwCfgFindFile ( >> + "etc/table-loader", >> + &FwCfgItem, >> + &FwCfgSize >> + ) >> + )) { >> + // >> + // Only make ACPI available on 64-bit systems, and only if QEMU generates >> + // (a subset of) the ACPI tables. >> + // >> Status = gBS->InstallProtocolInterface ( >> &ImageHandle, >> &gEdkiiPlatformHasAcpiProtocolGuid, >> @@ -48,21 +57,21 @@ PlatformHasAcpiDt ( >> if (EFI_ERROR (Status)) { >> goto Failed; >> } >> + >> + return Status; >> } >> >> // >> - // Expose the Device Tree unless PcdPureAcpiBoot is set. >> + // Expose the Device Tree otherwise. >> // >> - if (!FeaturePcdGet (PcdPureAcpiBoot)) { >> - Status = gBS->InstallProtocolInterface ( >> - &ImageHandle, >> - &gEdkiiPlatformHasDeviceTreeProtocolGuid, >> - EFI_NATIVE_INTERFACE, >> - NULL >> - ); >> - if (EFI_ERROR (Status)) { >> - goto Failed; >> - } >> + Status = gBS->InstallProtocolInterface ( >> + &ImageHandle, >> + &gEdkiiPlatformHasDeviceTreeProtocolGuid, >> + EFI_NATIVE_INTERFACE, >> + NULL >> + ); >> + if (EFI_ERROR (Status)) { >> + goto Failed; >> } >> >> return Status; >> -- >> 2.9.3 >> >>