From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1aftMI-0007cK-0x for mharc-grub-devel@gnu.org; Tue, 15 Mar 2016 14:06:18 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37655) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aftME-0007YE-Sk for grub-devel@gnu.org; Tue, 15 Mar 2016 14:06:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aftMB-0003Lu-M1 for grub-devel@gnu.org; Tue, 15 Mar 2016 14:06:14 -0400 Received: from mail-lf0-x22f.google.com ([2a00:1450:4010:c07::22f]:33540) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aftMB-0003Lo-AN for grub-devel@gnu.org; Tue, 15 Mar 2016 14:06:11 -0400 Received: by mail-lf0-x22f.google.com with SMTP id h198so1641204lfh.0 for ; Tue, 15 Mar 2016 11:06:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=lpJnQFrAhcW4LDha3S4RI9WqlvVvQPMp2wEpDzfFvGc=; b=y+8VlQmOf9+8+Y7+60tG8EO5AxQojHMdgHLthkguJJuUc1GbHa5/oereDOUJhOMimU vUjn0T8cPjLzgx+20cvm6nmfaCOUPw8Usoc8U4qn6qXEBlRd1yAMcq2OB+tjU6DbiJKX v6ki/vQuxAFlwkpfgkGqnFRzWfT2bZLXhrP6wdYwhsTZTE7HV1cNO9DzbOB3ew4Ls7ue GbSUMytXXe9H+c8ttGZR/zK2ikD6T0ZiQPb3gMWlDx1MCH9AKaR8PCoWYf/5iYtzgb+0 nHFMe3G5z6xEBI+3GMlwntDB9+QE9Mt36wEhFl1eYi44vwrAKwEJ9fYfGeX7S+/dOT8d OfjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=lpJnQFrAhcW4LDha3S4RI9WqlvVvQPMp2wEpDzfFvGc=; b=XcG5MueL649IVLbCf8V/6tenPNvgZyixiWFFbQfim6/F1cA38/AJXoU4KCWCbS/P08 +O1i5rJp6A8k5fzs2S+yE/1me9MBc4DoNf7rKRlWCqnkpqiJqzOyKEoIEcEpn+22eLFD PrZm4kDBLMj/4A9vlczC6RKcBYqaevagkdLlKwVRH0N1Cy8tKHXi9Kxx1TgD+8Kpo9ud efANgdAyAQqr/7Qd5oaDVC3ewlQjx87XBUzyLLhsodacPeYIqRcSYXIEWUReWORi/85J iuCG8nORisoJYLUalWiDyytKhXUfeMzjrKnau1dql4rsB1AhPT/xPyL2W9U8QRKhcrX/ GcJw== X-Gm-Message-State: AD7BkJIDG2GYxf9h32ZNjowOLnG51SeEuAxRAMWcxpMv+ECBgLfK7AFNwVJACZMr9j1F1Q== X-Received: by 10.25.85.145 with SMTP id j139mr10359293lfb.131.1458065170508; Tue, 15 Mar 2016 11:06:10 -0700 (PDT) Received: from [192.168.1.42] (ppp109-252-76-159.pppoe.spdop.ru. [109.252.76.159]) by smtp.gmail.com with ESMTPSA id 95sm1855963lfu.15.2016.03.15.11.06.09 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 15 Mar 2016 11:06:09 -0700 (PDT) Subject: Re: [GRUB2 PATCH v4 3/4 - FOR REVIEW ONLY] multiboot2: Do not pass memory maps to image if EFI boot services are enabled To: Vladimir 'phcoder' Serbinenko , Daniel Kiper References: <1458055562-24950-1-git-send-email-daniel.kiper@oracle.com> <1458055562-24950-5-git-send-email-daniel.kiper@oracle.com> From: Andrei Borzenkov X-Enigmail-Draft-Status: N1110 Message-ID: <56E84F10.5080804@gmail.com> Date: Tue, 15 Mar 2016 21:06:08 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:4010:c07::22f Cc: "jgross@suse.com" , "eric.snowberg@oracle.com" , "grub-devel@gnu.org" , "andrew.cooper3@citrix.com" , "cardoe@cardoe.com" , "pgnet.dev@gmail.com" , "roy.franz@linaro.org" , "ning.sun@intel.com" , "david.vrabel@citrix.com" , "jbeulich@suse.com" , "stefano.stabellini@eu.citrix.com" , "xen-devel@lists.xenproject.org" , "qiaowei.ren@intel.com" , "richard.l.maliszewski@intel.com" , "gang.wei@intel.com" , "fu.wei@linaro.org" , "seth.goldberg@oracle.com" X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Mar 2016 18:06:16 -0000 15.03.2016 19:07, Vladimir 'phcoder' Serbinenko пишет: > Looks good. Let's give a day for others to comment. Is the second email the > version for commit? > > On Tuesday, March 15, 2016, Daniel Kiper wrote: > >> Do not pass memory maps to image if it asked for EFI boot services. >> Main reason for not providing maps is because they will likely be >> invalid. We do a few allocations after filling them, e.g. for relocator >> needs. Usually we do not care as we would already finish boot services. >> If we keep boot services then it is easier to not provide maps. However, >> if image needs memory maps and they are not provided by bootloader then >> it should get them itself just before ExitBootServices() call. >> Are there any existing users of it that rely on memory map provided by bootloader? What if image explicitly requested non-optional memory map? According to multiboot specification it is valid and bootloader must either provide requested information or fail load. >> Signed-off-by: Daniel Kiper > >> Reviewed-by: Konrad Rzeszutek Wilk > >> --- >> v3 - suggestions/fixes: >> - improve commit message >> (suggested by Konrad Rzeszutek Wilk and Vladimir 'phcoder' >> Serbinenko). >> --- >> grub-core/loader/multiboot_mbi2.c | 71 >> ++++++++++++++++++------------------- >> 1 file changed, 35 insertions(+), 36 deletions(-) >> >> diff --git a/grub-core/loader/multiboot_mbi2.c >> b/grub-core/loader/multiboot_mbi2.c >> index 6c04402..ad1553b 100644 >> --- a/grub-core/loader/multiboot_mbi2.c >> +++ b/grub-core/loader/multiboot_mbi2.c >> @@ -390,7 +390,7 @@ static grub_size_t >> grub_multiboot_get_mbi_size (void) >> { >> #ifdef GRUB_MACHINE_EFI >> - if (!efi_mmap_size) >> + if (!keep_bs && !efi_mmap_size) >> find_efi_mmap_size (); >> #endif >> return 2 * sizeof (grub_uint32_t) + sizeof (struct multiboot_tag) >> @@ -755,12 +755,13 @@ grub_multiboot_make_mbi (grub_uint32_t *target) >> } >> } >> >> - { >> - struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *) >> ptrorig; >> - grub_fill_multiboot_mmap (tag); >> - ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) >> - / sizeof (grub_properly_aligned_t); >> - } >> + if (!keep_bs) >> + { >> + struct multiboot_tag_mmap *tag = (struct multiboot_tag_mmap *) >> ptrorig; >> + grub_fill_multiboot_mmap (tag); >> + ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) >> + / sizeof (grub_properly_aligned_t); >> + } >> >> { >> struct multiboot_tag_elf_sections *tag >> @@ -776,18 +777,19 @@ grub_multiboot_make_mbi (grub_uint32_t *target) >> / sizeof (grub_properly_aligned_t); >> } >> >> - { >> - struct multiboot_tag_basic_meminfo *tag >> - = (struct multiboot_tag_basic_meminfo *) ptrorig; >> - tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO; >> - tag->size = sizeof (struct multiboot_tag_basic_meminfo); >> + if (!keep_bs) >> + { >> + struct multiboot_tag_basic_meminfo *tag >> + = (struct multiboot_tag_basic_meminfo *) ptrorig; >> + tag->type = MULTIBOOT_TAG_TYPE_BASIC_MEMINFO; >> + tag->size = sizeof (struct multiboot_tag_basic_meminfo); >> >> - /* Convert from bytes to kilobytes. */ >> - tag->mem_lower = grub_mmap_get_lower () / 1024; >> - tag->mem_upper = grub_mmap_get_upper () / 1024; >> - ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) >> - / sizeof (grub_properly_aligned_t); >> - } >> + /* Convert from bytes to kilobytes. */ >> + tag->mem_lower = grub_mmap_get_lower () / 1024; >> + tag->mem_upper = grub_mmap_get_upper () / 1024; >> + ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) >> + / sizeof (grub_properly_aligned_t); >> + } >> >> { >> struct grub_net_network_level_interface *net; >> @@ -886,27 +888,24 @@ grub_multiboot_make_mbi (grub_uint32_t *target) >> grub_efi_uintn_t efi_desc_size; >> grub_efi_uint32_t efi_desc_version; >> >> - tag->type = MULTIBOOT_TAG_TYPE_EFI_MMAP; >> - tag->size = sizeof (*tag) + efi_mmap_size; >> - >> if (!keep_bs) >> - err = grub_efi_finish_boot_services (&efi_mmap_size, tag->efi_mmap, >> NULL, >> - &efi_desc_size, >> &efi_desc_version); >> - else >> { >> - if (grub_efi_get_memory_map (&efi_mmap_size, (void *) >> tag->efi_mmap, >> - NULL, >> - &efi_desc_size, &efi_desc_version) <= >> 0) >> - err = grub_error (GRUB_ERR_IO, "couldn't retrieve memory map"); >> + tag->type = MULTIBOOT_TAG_TYPE_EFI_MMAP; >> + tag->size = sizeof (*tag) + efi_mmap_size; >> + >> + err = grub_efi_finish_boot_services (&efi_mmap_size, >> tag->efi_mmap, NULL, >> + &efi_desc_size, >> &efi_desc_version); >> + >> + if (err) >> + return err; >> + >> + tag->descr_size = efi_desc_size; >> + tag->descr_vers = efi_desc_version; >> + tag->size = sizeof (*tag) + efi_mmap_size; >> + >> + ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) >> + / sizeof (grub_properly_aligned_t); >> } >> - if (err) >> - return err; >> - tag->descr_size = efi_desc_size; >> - tag->descr_vers = efi_desc_version; >> - tag->size = sizeof (*tag) + efi_mmap_size; >> - >> - ptrorig += ALIGN_UP (tag->size, MULTIBOOT_TAG_ALIGN) >> - / sizeof (grub_properly_aligned_t); >> } >> >> if (keep_bs) >> -- >> 1.7.10.4 >> >> >