From mboxrd@z Thu Jan 1 00:00:00 1970 From: Doug Goldstein Subject: Re: [PATCH v11 07/13] x86: add multiboot2 protocol support for EFI platforms Date: Thu, 12 Jan 2017 16:20:00 -0600 Message-ID: <55cdf2e1-10e9-2068-ab52-e60611c64afd@cardoe.com> References: <1480976718-12198-1-git-send-email-daniel.kiper@oracle.com> <1480976718-12198-8-git-send-email-daniel.kiper@oracle.com> <96e86b22-8940-dce2-d1b9-92d1c132ad0e@cardoe.com> <20170111190821.GY32675@olila.local.net-space.pl> <5a54299d-240d-5783-9a3b-5c3504d6fa4e@cardoe.com> <20170112121807.GF32675@olila.local.net-space.pl> <216aee7d-f0ab-3ba1-3c00-c16e211fecc7@cardoe.com> <20170112193037.GI32675@olila.local.net-space.pl> <20170112214516.GN32675@olila.local.net-space.pl> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0246652848312384187==" Return-path: Received: from mail6.bemta6.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cRnj7-0007BN-9n for xen-devel@lists.xenproject.org; Thu, 12 Jan 2017 22:20:09 +0000 Received: by mail-it0-f66.google.com with SMTP id o138so3311849ito.3 for ; Thu, 12 Jan 2017 14:20:07 -0800 (PST) In-Reply-To: <20170112214516.GN32675@olila.local.net-space.pl> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" To: Daniel Kiper Cc: jgross@suse.com, sstabellini@kernel.org, andrew.cooper3@citrix.com, pgnet.dev@gmail.com, ning.sun@intel.com, julien.grall@arm.com, jbeulich@suse.com, xen-devel@lists.xenproject.org, qiaowei.ren@intel.com, gang.wei@intel.com, fu.wei@linaro.org List-Id: xen-devel@lists.xenproject.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --===============0246652848312384187== Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="oXIhWtgoSGbL6THDqJsTAWgoQqOlmvMjg" This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --oXIhWtgoSGbL6THDqJsTAWgoQqOlmvMjg Content-Type: multipart/mixed; boundary="P9wFWk97IBgG3m7BaRA5rJ8SWLa7n0Xe3"; protected-headers="v1" From: Doug Goldstein To: Daniel Kiper Cc: xen-devel@lists.xenproject.org, andrew.cooper3@citrix.com, fu.wei@linaro.org, gang.wei@intel.com, jbeulich@suse.com, jgross@suse.com, julien.grall@arm.com, konrad.wilk@oracle.com, ning.sun@intel.com, pgnet.dev@gmail.com, qiaowei.ren@intel.com, sstabellini@kernel.org Message-ID: <55cdf2e1-10e9-2068-ab52-e60611c64afd@cardoe.com> Subject: Re: [PATCH v11 07/13] x86: add multiboot2 protocol support for EFI platforms References: <1480976718-12198-1-git-send-email-daniel.kiper@oracle.com> <1480976718-12198-8-git-send-email-daniel.kiper@oracle.com> <96e86b22-8940-dce2-d1b9-92d1c132ad0e@cardoe.com> <20170111190821.GY32675@olila.local.net-space.pl> <5a54299d-240d-5783-9a3b-5c3504d6fa4e@cardoe.com> <20170112121807.GF32675@olila.local.net-space.pl> <216aee7d-f0ab-3ba1-3c00-c16e211fecc7@cardoe.com> <20170112193037.GI32675@olila.local.net-space.pl> <20170112214516.GN32675@olila.local.net-space.pl> In-Reply-To: <20170112214516.GN32675@olila.local.net-space.pl> --P9wFWk97IBgG3m7BaRA5rJ8SWLa7n0Xe3 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 1/12/17 3:45 PM, Daniel Kiper wrote: > On Thu, Jan 12, 2017 at 01:46:41PM -0600, Doug Goldstein wrote: >> On 1/12/17 1:30 PM, Daniel Kiper wrote: >>> On Thu, Jan 12, 2017 at 09:44:59AM -0600, Doug Goldstein wrote: >> >>> >>>> view there's no reason for adding MB2 support for BIOS since it prov= ides >>>> no advantage over MB1 when booting from the BIOS. Now MB2 solves a >>> >>> From your point of view maybe it does not. However, from user point o= f view it may. >>> If you have support for MB2 on legacy BIOS and EFI platforms then you= can boot Xen >>> on both platforms without changing anything in boot config files. Oth= erwise you have >>> to prepare separate configuration for different platforms. >> >> Neither Grub nor iPXE require different configs for MB1 vs MB2 so I'm >> not seeing the validity of this logic. >=20 > Hmmm... This is interesting. I do not know iPXE, however, in GRUB you m= ust > use multiboot/module for MB1 and multiboot2/module2 for MB2. I suppose = that > you have to differentiate between both of them in iPXE somehow too. Hen= ce, > there is pretty good chance that configs for MB1 and MB2 are different.= multiboot/multiboot2 and module/module2 are aliases of each other. They work interchangeably. Its the same way in iPXE. >=20 >>>> problem with booting over EFI vs MB1 so they'll be willing to take a= >>>> change there. I'll also disagree that BIOS is easier than EFI since = with >>>> EFI its just load the ELF into memory and set a few pointers in tags= =2E >>>> With BIOS it requires me to build up the memory map into a MB2 struc= ture. >>> >>> Xen uses only these tags on legacy BIOS platforms: MULTIBOOT2_TAG_TYP= E_BASIC_MEMINFO >>> (well, nice to have but it can be also not provided), MULTIBOOT2_TAG_= TYPE_MMAP (same >>> as MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO), MULTIBOOT2_TAG_TYPE_BOOT_LOADE= R_NAME >>> (same as MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO) ,MULTIBOOT2_TAG_TYPE_CMDL= INE, >>> MULTIBOOT2_TAG_TYPE_MODULE. I do not mention MULTIBOOT2_TAG_TYPE_END = which >>> is obvious. So, if you are real hardcore minimalist then you have to = provide >>> MULTIBOOT2_TAG_TYPE_CMDLINE and MULTIBOOT2_TAG_TYPE_MODULE. All of th= em >>> are provided also on EFI. So, I do not see any reason to not provide = MB2 >>> for legacy BIOS. And I do not think that it is very difficult to prov= ide >>> all optional tags mentioned above. >> >> I don't understand what you're attempting to convey here. You've liste= d >> out a number of tags that I mentioned in my message that I don't have = to >> implement for EFI. You've basically reinforced my point that its easie= r >> to implement this for EFI than BIOS. MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO= >> and MULTIBOOT2_TAG_TYPE_MMAP are unused by Xen on EFI. It gets this in= fo >=20 > I showed you that if you are real minimalist you can enable the same MB= 2 code > on legacy BIOS and EFI. I do not understand your objection against prov= iding > MB2 in iPXE on legacy BIOS if you do not need extra code (maybe a few #= ifdefs). > Though I am not going to convince you. It is your choice but I am still= thinking > that it is wrong choice. Its not my choice. Its the feedback I've received from upstream. >=20 > By the way, does iPXE check MULTIBOOT2_HEADER_TAG_INFORMATION_REQUEST i= n Xen header. > If it does (it should) and do not understand MULTIBOOT2_TAG_TYPE_BASIC_= MEMINFO and > MULTIBOOT2_TAG_TYPE_MMAP then it should fail. It does but I know that Xen doesn't use that information if Boot Services are available by code inspection. Which is what my comments are related to. >=20 >> from a call to GetMemoryMap(). You actually reminded me of another bug= =2E >> Calling ExitBootServices() on Grub and letting it pass the memory info= >> causes Xen to fail to load. >=20 > How come... Which GRUB version do you use? Xen clearly says that it nee= ds > boot services (look into MB2 header). So, GRUB is not allowed to call > ExitBootServices(). If it does then it is GRUB bug. No. That's not how it works at all. To quote 3.1.12 of the Multiboot2 spec... "This tag indicates that payload supports starting without terminating boot services." This tag is not required to be respected but instead means that the payload supports using boot services. Additionally section 3.6.3 which talks about passing MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO states... "This tag may not be provided by some boot loaders on EFI platforms if EFI boot services are enabled and available for the loaded image (EFI boot services not terminated tag exists in Multiboot2 information structure)." And section 3.6.8 talks about passing MULTIBOOT2_TAG_TYPE_MMAP states... "This tag may not be provided by some boot loaders on EFI platforms if EFI boot services are enabled and available for the loaded image (EFI boot services not terminated tag exists in Multiboot2 information structure)." So for my iPXE support if the payload (in this case Xen) reports that it supports not having boot services exited then I don't exit it and I don't provide MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO or MULTIBOOT2_TAG_TYPE_MMAP. Considering the fact that if Xen has boot services available it ignores these two tags it seems what I've done complies with the spec and complies with the only known full implementation of MB2. (fwiw, the only implementations I know about are tboot and your Xen patches. I've written a small application which dumps out the data that was received but that was purely for debugging). >=20 >> Andrew helped me troubleshoot this and he discovered the fix. You've g= ot >> code: >> >> /* Store Xen image load base address in place accessible for 32-bit co= de. */ >> mov %r15d,%esi >> >> But if any of the checks under the run_bs: label specifically: >> - /* Are EFI boot services available? */ >> - /* Is EFI SystemTable address provided by boot loader? */ >> - /* Is EFI ImageHandle address provided by boot loader? */ >> >> Will not run the mov instruction and then fail to boot. Its only if an= y >> of these are false will it attempt to use the tags mentioned above as = well. >=20 > OK, this is a bug and I will fix it. However, this is not related to > ExitBootServices() call in GRUB2. Well it is. Because if boot services are not available then to goes the path of the bug. --=20 Doug Goldstein --P9wFWk97IBgG3m7BaRA5rJ8SWLa7n0Xe3-- --oXIhWtgoSGbL6THDqJsTAWgoQqOlmvMjg Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG/MacGPG2 v2.0 iQJ8BAEBCgBmBQJYeAETXxSAAAAAAC4AKGlzc3Vlci1mcHJAbm90YXRpb25zLm9w ZW5wZ3AuZmlmdGhob3JzZW1hbi5uZXRBNTM5MEQ2RTNFMTkyNzlCNzVDMzIwOTVB MkJDMDNEQzg3RUQxQkQ0AAoJEKK8A9yH7RvUB9AP/jqKGgdgJ/C+hoodDDX6e+iW iqESPlNDmvzpqljX4TFKvJqbOF5EQoPBH/L7yOueBWhfkNzDStcb3MmNLoRtBGsf dzMS+CP5USFUm2Yj9zXmbEZA6KBO6T14Yt/G3LRwDX60ekbLBHU18x6TR4+P5thS /O5LFQ/hFUDF9ghEKBf/St0pv1sHHseLdM6miYvtEvZ4BPS+jm1MkxUzSM5xik72 DF8OFzK6yKkVwuH/K685ovV2wC69gHALFHCtRQC+ibR/HD2WsK30Mi/13gp2dZuk /zx/pZTBfVjJTAOpgvPlktvFZMv01kDNEAZpyYiA3NTK8B+xGxRmprgFLNfRwGsf a96GUSw7eJNEt9s94LqwsIME5zJcVw5z805Mb8TpV1/AqfaRC8Nn3jSv1hxpe4Hf nSsJPz9v5McEj4njQx/IDMgMZa1Ez/lLrTPfgJXHmyxTfKX8JLhr1mE+nnkN0S4b xpi/l7PI4r2/Az0n/i5fHxWWH5jfbVbpBvuvIS+YLtUeJKFQ5b7EVrh9Bdows/LP HwA7Sl9LoZgNUkitDD8vUvFoMr+cfRFq/9pKN9iy/AeQMtYI1FfuKBcYwwZT0VQC hG7B3NIx44/48IydfJotSRYmhD/xBeDBZEJ/E5wCLPXNXUNRNS4iT3o1scMQCe3Z D6SM2f9NP6PSUlP379Mg =oG8m -----END PGP SIGNATURE----- --oXIhWtgoSGbL6THDqJsTAWgoQqOlmvMjg-- --===============0246652848312384187== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KWGVuLWRldmVs IG1haWxpbmcgbGlzdApYZW4tZGV2ZWxAbGlzdHMueGVuLm9yZwpodHRwczovL2xpc3RzLnhlbi5v cmcveGVuLWRldmVsCg== --===============0246652848312384187==--