From patchwork Fri Feb 13 16:04:48 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Matt Fleming X-Patchwork-Id: 5825791 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id CA908BF440 for ; Fri, 13 Feb 2015 16:07:31 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id E0B3A20220 for ; Fri, 13 Feb 2015 16:07:30 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E576D200D6 for ; Fri, 13 Feb 2015 16:07:29 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YMIk9-0000JR-DX; Fri, 13 Feb 2015 16:05:25 +0000 Received: from mail-wg0-f45.google.com ([74.125.82.45]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1YMIk3-0007Ur-Qh for linux-arm-kernel@lists.infradead.org; Fri, 13 Feb 2015 16:05:21 +0000 Received: by mail-wg0-f45.google.com with SMTP id k14so14519662wgh.4 for ; Fri, 13 Feb 2015 08:04:56 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-type:content-disposition:in-reply-to :user-agent; bh=TAqFZcMtOZbVzljWrIHsKEuHrjPNkBdnCzZYz1osf70=; b=YTRtXiAEZgZyr99EZt9GTQGOqxi2XWajMMjyB9pbfIYKrjxl44ur4Uz9RXNdDeYYh5 E9R3VoAbM1IFiP4fd5K/9jlQ3eIA6tMBfBBQAwIixvPJLvTGsHJnMi+v5u3DUkLZFik0 Ew5UIXC35uC+oE7LSAYLMuSzdMZMYtPC0XhWtAHpH/O9diyVUMS/H2HS1lbex45y7c+N vPxTXxAgMkNWtEzH+4oI0vdR4zTqYyT08GsvTTaSwxP89R5jGyCApVwwTnCDKqW7yjX7 ZGAVviUD193lqHYvu1sBWJYnnwxzBa1zEt8aNeNRez0J1KLGHsVBUsRiWgPS/ZDxXSrc SKhw== X-Gm-Message-State: ALoCoQmDUq1rMKssOJJEPAP6aL8DzaWpcPujtPatuap5GYnavqTxKyp901fAcyX+W3ad60iKdcwu X-Received: by 10.180.100.104 with SMTP id ex8mr8182641wib.57.1423843496817; Fri, 13 Feb 2015 08:04:56 -0800 (PST) Received: from localhost ([90.203.53.188]) by mx.google.com with ESMTPSA id df8sm3501514wib.2.2015.02.13.08.04.53 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 13 Feb 2015 08:04:56 -0800 (PST) Date: Fri, 13 Feb 2015 16:04:48 +0000 From: Matt Fleming To: Ard Biesheuvel Subject: Re: [PATCH] efi: get_memory_map: add sufficient slack for memory descriptors Message-ID: <20150213160448.GA30567@codeblueprint.co.uk> References: <1423718659-795-1-git-send-email-ard.biesheuvel@linaro.org> <20150212102226.GB1245@leverpostej> <20150212144727.GD4665@codeblueprint.co.uk> <20150212151607.GH1522@leverpostej> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150213_080520_017216_08F587AE X-CRM114-Status: GOOD ( 19.99 ) X-Spam-Score: -0.7 (/) Cc: Mark Rutland , "linux-efi@vger.kernel.org" , "leif.lindholm@linaro.org" , "roy.franz@linaro.org" , "mingo@redhat.com" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 12 Feb, at 11:31:02PM, Ard Biesheuvel wrote: > > Actually, looking again at the original patch, it appears that my > analysis was incorrect regarding the possibility that the loop would > never terminate. The only thing that could happen if desc_size > > sizeof(efi_memory_desc_t) is that you need two iterations instead of > one to get a pool allocation that is of sufficient size. > So perhaps it is better to just revert the patch. > > My apologies for the hassle. This is what I've got queued up, Acked-by: Ard Biesheuvel Acked-by: Mark Rutland --- From 3f281b98ffc99e604a3988aa93304a3a591eeeb5 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 13 Feb 2015 15:46:56 +0000 Subject: [PATCH] Revert "efi/libstub: Call get_memory_map() to obtain map and desc sizes" This reverts commit d1a8d66b9177105e898e73716f97eb61842c457a. Ard reported a boot failure when running UEFI under Qemu and Xen and experimenting with various Tianocore build options, "As it turns out, when allocating room for the UEFI memory map using UEFI's AllocatePool (), it may result in two new memory map entries being created, for instance, when using Tianocore's preallocated region feature. For example, the following region 0x00005ead5000-0x00005ebfffff [Conventional Memory| | | | | |WB|WT|WC|UC] may be split like this 0x00005ead5000-0x00005eae2fff [Conventional Memory| | | | | |WB|WT|WC|UC] 0x00005eae3000-0x00005eae4fff [Loader Data | | | | | |WB|WT|WC|UC] 0x00005eae5000-0x00005ebfffff [Conventional Memory| | | | | |WB|WT|WC|UC] if the preallocated Loader Data region was chosen to be right in the middle of the original free space. After patch d1a8d66b9177 ("efi/libstub: Call get_memory_map() to obtain map and desc sizes"), this is not being dealt with correctly anymore, as the existing logic to allocate room for a single additional entry has become insufficient." Mark requested to reinstate the old loop we had before commit d1a8d66b9177, which grows the memory map buffer until it's big enough to hold the EFI memory map. Cc: Ard Biesheuvel Cc: Mark Rutland Signed-off-by: Matt Fleming --- drivers/firmware/efi/libstub/efi-stub-helper.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index d073e3946383..9bd9fbb5bea8 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -66,29 +66,25 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg, unsigned long key; u32 desc_version; - *map_size = 0; - *desc_size = 0; - key = 0; - status = efi_call_early(get_memory_map, map_size, NULL, - &key, desc_size, &desc_version); - if (status != EFI_BUFFER_TOO_SMALL) - return EFI_LOAD_ERROR; - + *map_size = sizeof(*m) * 32; +again: /* * Add an additional efi_memory_desc_t because we're doing an * allocation which may be in a new descriptor region. */ - *map_size += *desc_size; + *map_size += sizeof(*m); status = efi_call_early(allocate_pool, EFI_LOADER_DATA, *map_size, (void **)&m); if (status != EFI_SUCCESS) goto fail; + *desc_size = 0; + key = 0; status = efi_call_early(get_memory_map, map_size, m, &key, desc_size, &desc_version); if (status == EFI_BUFFER_TOO_SMALL) { efi_call_early(free_pool, m); - return EFI_LOAD_ERROR; + goto again; } if (status != EFI_SUCCESS)