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 EC2D021AE30CF for ; Sat, 3 Jun 2017 08:41:08 -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 2157380C0C; Sat, 3 Jun 2017 15:42:12 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 2157380C0C 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 2157380C0C Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-25.phx2.redhat.com [10.3.116.25]) by smtp.corp.redhat.com (Postfix) with ESMTP id 90B1051DF4; Sat, 3 Jun 2017 15:42:10 +0000 (UTC) From: Laszlo Ersek To: edk2-devel-01 Cc: Ard Biesheuvel , Gerd Hoffmann , Igor Mammedov , Jordan Justen Date: Sat, 3 Jun 2017 17:42:03 +0200 Message-Id: <20170603154203.29907-2-lersek@redhat.com> In-Reply-To: <20170603154203.29907-1-lersek@redhat.com> References: <20170603154203.29907-1-lersek@redhat.com> 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]); Sat, 03 Jun 2017 15:42:12 +0000 (UTC) Subject: [PATCH 1/1] OvmfPkg/AcpiPlatformDxe: alloc blobs from 64-bit space unless restricted 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: Sat, 03 Jun 2017 15:41:09 -0000 ... by narrower than 8-byte ADD_POINTER references. Introduce the CollectRestrictedAllocations() function, which iterates over the linker/loader script, and collects the names of the fw_cfg blobs that are referenced by QEMU_LOADER_ADD_POINTER.PointeeFile fields, such that QEMU_LOADER_ADD_POINTER.PointerSize is less than 8. This means that the pointee blob's address will have to be patched into a narrower-than-8 byte pointer field, hence the pointee blob must not be allocated from 64-bit address space. In ProcessCmdAllocate(), consult these restrictions when setting the maximum address for gBS->AllocatePages(). The default is now MAX_UINT64, unless restricted like described above to the pre-patch MAX_UINT32 limit. In combination with Ard's QEMU commit cb51ac2ffe36 ("hw/arm/virt: generate 64-bit addressable ACPI objects", 2017-04-10), this patch enables OvmfPkg/AcpiPlatformDxe to work entirely above the 4GB mark. (An upcoming / planned aarch64 QEMU machine type will have no RAM under 4GB at all. Plus, moving the allocations higher is beneficial to the current "virt" machine type as well; in Ard's words: "having all firmware allocations inside the same 1 GB (or 512 MB for 64k pages) frame reduces the TLB footprint".) Cc: Ard Biesheuvel Cc: Gerd Hoffmann Cc: Igor Mammedov Cc: Jordan Justen Suggested-by: Igor Mammedov Suggested-by: Gerd Hoffmann Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 190 +++++++++++++++++++- 1 file changed, 183 insertions(+), 7 deletions(-) diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c index 1bc5fe297a96..7aeb6472188b 100644 --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c @@ -132,14 +132,169 @@ PointerCompare ( } /** + Comparator function for two ASCII strings. Can be used as both Key and + UserStruct comparator. + + This function exists solely so we can avoid casting &AsciiStrCmp to + ORDERED_COLLECTION_USER_COMPARE and ORDERED_COLLECTION_KEY_COMPARE. + + @param[in] AsciiString1 Pointer to the first ASCII string. + + @param[in] AsciiString2 Pointer to the second ASCII string. + + @return The return value of AsciiStrCmp (AsciiString1, AsciiString2). +**/ +STATIC +INTN +EFIAPI +AsciiStringCompare ( + IN CONST VOID *AsciiString1, + IN CONST VOID *AsciiString2 + ) +{ + return AsciiStrCmp (AsciiString1, AsciiString2); +} + + +/** + Release the ORDERED_COLLECTION structure populated by + CollectRestrictedAllocations() (below). + + This function may be called by CollectRestrictedAllocations() itself, on + the error path. + + @param[in] RestrictedAllocations The ORDERED_COLLECTION structure to + release. +**/ +STATIC +VOID +ReleaseRestrictedAllocations ( + IN ORDERED_COLLECTION *RestrictedAllocations +) +{ + ORDERED_COLLECTION_ENTRY *Entry, *Entry2; + + for (Entry = OrderedCollectionMin (RestrictedAllocations); + Entry != NULL; + Entry = Entry2) { + Entry2 = OrderedCollectionNext (Entry); + OrderedCollectionDelete (RestrictedAllocations, Entry, NULL); + } + OrderedCollectionUninit (RestrictedAllocations); +} + + +/** + Iterate over the linker/loader script, and collect the names of the fw_cfg + blobs that are referenced by QEMU_LOADER_ADD_POINTER.PointeeFile fields, such + that QEMU_LOADER_ADD_POINTER.PointerSize is less than 8. This means that the + pointee blob's address will have to be patched into a narrower-than-8 byte + pointer field, hence the pointee blob must not be allocated from 64-bit + address space. + + @param[out] RestrictedAllocations The ORDERED_COLLECTION structure linking + (not copying / owning) such + QEMU_LOADER_ADD_POINTER.PointeeFile fields + that name the blobs restricted from 64-bit + allocation. + + @param[in] LoaderStart Points to the first entry in the + linker/loader script. + + @param[in] LoaderEnd Points one past the last entry in the + linker/loader script. + + @retval EFI_SUCCESS RestrictedAllocations has been populated. + + @retval EFI_OUT_OF_RESOURCES Memory allocation failed. + + @retval EFI_PROTOCOL_ERROR Invalid linker/loader script contents. +**/ +STATIC +EFI_STATUS +CollectRestrictedAllocations ( + OUT ORDERED_COLLECTION **RestrictedAllocations, + IN CONST QEMU_LOADER_ENTRY *LoaderStart, + IN CONST QEMU_LOADER_ENTRY *LoaderEnd +) +{ + ORDERED_COLLECTION *Collection; + CONST QEMU_LOADER_ENTRY *LoaderEntry; + EFI_STATUS Status; + + Collection = OrderedCollectionInit (AsciiStringCompare, AsciiStringCompare); + if (Collection == NULL) { + return EFI_OUT_OF_RESOURCES; + } + + for (LoaderEntry = LoaderStart; LoaderEntry < LoaderEnd; ++LoaderEntry) { + CONST QEMU_LOADER_ADD_POINTER *AddPointer; + + if (LoaderEntry->Type != QemuLoaderCmdAddPointer) { + continue; + } + AddPointer = &LoaderEntry->Command.AddPointer; + + if (AddPointer->PointerSize >= 8) { + continue; + } + + if (AddPointer->PointeeFile[QEMU_LOADER_FNAME_SIZE - 1] != '\0') { + DEBUG ((DEBUG_ERROR, "%a: malformed file name\n", __FUNCTION__)); + Status = EFI_PROTOCOL_ERROR; + goto RollBack; + } + + Status = OrderedCollectionInsert ( + Collection, + NULL, // Entry + (VOID *)AddPointer->PointeeFile + ); + switch (Status) { + case EFI_SUCCESS: + DEBUG (( + DEBUG_VERBOSE, + "%a: restricting blob \"%a\" from 64-bit allocation\n", + __FUNCTION__, + AddPointer->PointeeFile + )); + break; + case EFI_ALREADY_STARTED: + // + // The restriction has been recorded already. + // + break; + case EFI_OUT_OF_RESOURCES: + goto RollBack; + default: + ASSERT (FALSE); + } + } + + *RestrictedAllocations = Collection; + return EFI_SUCCESS; + +RollBack: + ReleaseRestrictedAllocations (Collection); + return Status; +} + + +/** Process a QEMU_LOADER_ALLOCATE command. - @param[in] Allocate The QEMU_LOADER_ALLOCATE command to process. + @param[in] Allocate The QEMU_LOADER_ALLOCATE command to + process. - @param[in,out] Tracker The ORDERED_COLLECTION tracking the BLOB user - structures created thus far. + @param[in,out] Tracker The ORDERED_COLLECTION tracking the BLOB + user structures created thus far. + + @param[in] RestrictedAllocations The ORDERED_COLLECTION populated by + CollectRestrictedAllocations(), naming the + fw_cfg blobs that must not be allocated + from 64-bit address space. @retval EFI_SUCCESS An area of whole AcpiNVS pages has been allocated for the blob contents, and the contents have been saved. A BLOB object (user @@ -163,9 +318,10 @@ STATIC EFI_STATUS EFIAPI ProcessCmdAllocate ( IN CONST QEMU_LOADER_ALLOCATE *Allocate, - IN OUT ORDERED_COLLECTION *Tracker + IN OUT ORDERED_COLLECTION *Tracker, + IN ORDERED_COLLECTION *RestrictedAllocations ) { FIRMWARE_CONFIG_ITEM FwCfgItem; UINTN FwCfgSize; @@ -192,9 +348,12 @@ ProcessCmdAllocate ( return Status; } NumPages = EFI_SIZE_TO_PAGES (FwCfgSize); - Address = 0xFFFFFFFF; + Address = MAX_UINT64; + if (OrderedCollectionFind (RestrictedAllocations, Allocate->File) != NULL) { + Address = MAX_UINT32; + } Status = gBS->AllocatePages (AllocateMaxAddress, EfiACPIMemoryNVS, NumPages, &Address); if (EFI_ERROR (Status)) { return Status; @@ -805,8 +964,9 @@ InstallQemuFwCfgTables ( CONST QEMU_LOADER_ENTRY *LoaderEntry, *LoaderEnd; CONST QEMU_LOADER_ENTRY *WritePointerSubsetEnd; ORIGINAL_ATTRIBUTES *OriginalPciAttributes; UINTN OriginalPciAttributesCount; + ORDERED_COLLECTION *RestrictedAllocations; S3_CONTEXT *S3Context; ORDERED_COLLECTION *Tracker; UINTN *InstalledKey; INT32 Installed; @@ -833,17 +993,26 @@ InstallQemuFwCfgTables ( QemuFwCfgReadBytes (FwCfgSize, LoaderStart); RestorePciDecoding (OriginalPciAttributes, OriginalPciAttributesCount); LoaderEnd = LoaderStart + FwCfgSize / sizeof *LoaderEntry; + Status = CollectRestrictedAllocations ( + &RestrictedAllocations, + LoaderStart, + LoaderEnd + ); + if (EFI_ERROR (Status)) { + goto FreeLoader; + } + S3Context = NULL; if (QemuFwCfgS3Enabled ()) { // // Size the allocation pessimistically, assuming that all commands in the // script are QEMU_LOADER_WRITE_POINTER commands. // Status = AllocateS3Context (&S3Context, LoaderEnd - LoaderStart); if (EFI_ERROR (Status)) { - goto FreeLoader; + goto FreeRestrictedAllocations; } } Tracker = OrderedCollectionInit (BlobCompare, BlobKeyCompare); @@ -862,9 +1031,13 @@ InstallQemuFwCfgTables ( WritePointerSubsetEnd = LoaderStart; for (LoaderEntry = LoaderStart; LoaderEntry < LoaderEnd; ++LoaderEntry) { switch (LoaderEntry->Type) { case QemuLoaderCmdAllocate: - Status = ProcessCmdAllocate (&LoaderEntry->Command.Allocate, Tracker); + Status = ProcessCmdAllocate ( + &LoaderEntry->Command.Allocate, + Tracker, + RestrictedAllocations + ); break; case QemuLoaderCmdAddPointer: Status = ProcessCmdAddPointer (&LoaderEntry->Command.AddPointer, @@ -1009,8 +1182,11 @@ FreeS3Context: if (S3Context != NULL) { ReleaseS3Context (S3Context); } +FreeRestrictedAllocations: + ReleaseRestrictedAllocations (RestrictedAllocations); + FreeLoader: FreePool (LoaderStart); return Status; -- 2.9.3