From: Ard Biesheuvel <ard.biesheuvel@linaro.org> To: Ingo Molnar <mingo@kernel.org> Cc: Matt Fleming <matt@codeblueprint.co.uk>, Thomas Gleixner <tglx@linutronix.de>, "H. Peter Anvin" <hpa@zytor.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-efi@vger.kernel.org" <linux-efi@vger.kernel.org>, Leif Lindholm <leif.lindholm@linaro.org>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, "stable@vger.kernel.org" <stable@vger.kernel.org>, Matt Fleming <matt.fleming@intel.com>, Mark Rutland <mark.rutland@arm.com>, Mark Salter <msalter@redhat.com>, Linus Torvalds <torvalds@linux-foundation.org>, Andrew Morton <akpm@linux-foundation.org>, Andy Lutomirski <luto@kernel.org>, Borislav Petkov <bp@alien8.de>, Denys Vlasenko <dvlasenk@redhat.com>, Brian Gerst <brgerst@gmail.com> Subject: Re: [PATCH 2/2] arm64/efi: Don't pad between EFI_MEMORY_RUNTIME regions Date: Sat, 26 Sep 2015 00:08:03 -0700 [thread overview] Message-ID: <CAKv+Gu_JRCmx5qKq7SDDq73BegVs3LKZwe+OdvTKb4YD_CURyw@mail.gmail.com> (raw) In-Reply-To: <20150926060159.GB25877@gmail.com> On 25 September 2015 at 23:01, Ingo Molnar <mingo@kernel.org> wrote: > > * Matt Fleming <matt@codeblueprint.co.uk> wrote: > >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> The new Properties Table feature introduced in UEFIv2.5 may split >> memory regions that cover PE/COFF memory images into separate code >> and data regions. Since these regions only differ in the type (runtime >> code vs runtime data) and the permission bits, but not in the memory >> type attributes (UC/WC/WT/WB), the spec does not require them to be >> aligned to 64 KB. >> >> Since the relative offset of PE/COFF .text and .data segments cannot >> be changed on the fly, this means that we can no longer pad out those >> regions to be mappable using 64 KB pages. >> Unfortunately, there is no annotation in the UEFI memory map that >> identifies data regions that were split off from a code region, so we >> must apply this logic to all adjacent runtime regions whose attributes >> only differ in the permission bits. >> >> So instead of rounding each memory region to 64 KB alignment at both >> ends, only round down regions that are not directly preceded by another >> runtime region with the same type attributes. Since the UEFI spec does >> not mandate that the memory map be sorted, this means we also need to >> sort it first. > > So I think this is fundamentally wrong as well, similarly to the related x86 fix. > > I think for compatibility reasons the whole 'EFI runtime image' should be mapped > in a single go, as closely matching the EFI layouts and offsets as possible. We > are not talking about gigabytes here, right? > As I explained in the other thread, this is really not necessary, and never has been until the firmware started splitting up PE/COFF images into several sections each. As long as we keep those PE/COFF images together, everything will work as before, and the only complication is that the memory map does not contain any clues about which regions belong to a single PE/COFF image, so we need to keep all adjacent EFI_MEMORY_RUNTIME regions adjacent in the VA mapping. > Even if technically they are 'separate sections', the x86 bug shows that they > aren't. So we should not pretend so on the Linux side either and we should not > tear them apart (and then work hard to preserve the interdependencies, some > declared, some hidden!). > This is about relocations and interdependencies at the symbol level, and such interdependencies only exist internally inside PE/COFF images. > If we allocate the EFI runtime as a single virtual memory block then issues like > rounding between sections does not even come up as a problem: we map the original > offsets and sizes byte by byte. > Well, by that reasoning, we should not call SetVirtualAddressMap() in the first place, and just use the 1:1 mapping UEFI uses natively. This is more than feasible on arm64, and I actually fought hard against using SetVirtualAddressMap() at all, but I was overruled by others. I think this is also trivially possible on X64, since the 1:1 mapping is already active alongside the VA mapping. -- Ard.
WARNING: multiple messages have this Message-ID (diff)
From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> To: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>, "H. Peter Anvin" <hpa-YMNOUZJC4hwAvxtiuMwx3w@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>, Catalin Marinas <catalin.marinas-5wv7dgnIgG8@public.gmane.org>, Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>, "stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>, Mark Salter <msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Andrew Morton <akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>, Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>, Borislav Petkov <bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org>, Denys Vlasenko <dvlasenk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Brian Gerst <brgerst-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Subject: Re: [PATCH 2/2] arm64/efi: Don't pad between EFI_MEMORY_RUNTIME regions Date: Sat, 26 Sep 2015 00:08:03 -0700 [thread overview] Message-ID: <CAKv+Gu_JRCmx5qKq7SDDq73BegVs3LKZwe+OdvTKb4YD_CURyw@mail.gmail.com> (raw) In-Reply-To: <20150926060159.GB25877-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> On 25 September 2015 at 23:01, Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote: > > * Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote: > >> From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >> >> The new Properties Table feature introduced in UEFIv2.5 may split >> memory regions that cover PE/COFF memory images into separate code >> and data regions. Since these regions only differ in the type (runtime >> code vs runtime data) and the permission bits, but not in the memory >> type attributes (UC/WC/WT/WB), the spec does not require them to be >> aligned to 64 KB. >> >> Since the relative offset of PE/COFF .text and .data segments cannot >> be changed on the fly, this means that we can no longer pad out those >> regions to be mappable using 64 KB pages. >> Unfortunately, there is no annotation in the UEFI memory map that >> identifies data regions that were split off from a code region, so we >> must apply this logic to all adjacent runtime regions whose attributes >> only differ in the permission bits. >> >> So instead of rounding each memory region to 64 KB alignment at both >> ends, only round down regions that are not directly preceded by another >> runtime region with the same type attributes. Since the UEFI spec does >> not mandate that the memory map be sorted, this means we also need to >> sort it first. > > So I think this is fundamentally wrong as well, similarly to the related x86 fix. > > I think for compatibility reasons the whole 'EFI runtime image' should be mapped > in a single go, as closely matching the EFI layouts and offsets as possible. We > are not talking about gigabytes here, right? > As I explained in the other thread, this is really not necessary, and never has been until the firmware started splitting up PE/COFF images into several sections each. As long as we keep those PE/COFF images together, everything will work as before, and the only complication is that the memory map does not contain any clues about which regions belong to a single PE/COFF image, so we need to keep all adjacent EFI_MEMORY_RUNTIME regions adjacent in the VA mapping. > Even if technically they are 'separate sections', the x86 bug shows that they > aren't. So we should not pretend so on the Linux side either and we should not > tear them apart (and then work hard to preserve the interdependencies, some > declared, some hidden!). > This is about relocations and interdependencies at the symbol level, and such interdependencies only exist internally inside PE/COFF images. > If we allocate the EFI runtime as a single virtual memory block then issues like > rounding between sections does not even come up as a problem: we map the original > offsets and sizes byte by byte. > Well, by that reasoning, we should not call SetVirtualAddressMap() in the first place, and just use the 1:1 mapping UEFI uses natively. This is more than feasible on arm64, and I actually fought hard against using SetVirtualAddressMap() at all, but I was overruled by others. I think this is also trivially possible on X64, since the 1:1 mapping is already active alongside the VA mapping. -- Ard.
next prev parent reply other threads:[~2015-09-26 7:08 UTC|newest] Thread overview: 80+ messages / expand[flat|nested] mbox.gz Atom feed top 2015-09-25 22:02 [GIT PULL 0/2] EFI urgent fixes Matt Fleming 2015-09-25 22:02 ` Matt Fleming 2015-09-25 22:02 ` [PATCH 1/2] x86/efi: Map EFI memmap entries in-order at runtime Matt Fleming 2015-09-25 22:02 ` Matt Fleming 2015-09-26 5:56 ` Ingo Molnar 2015-09-26 5:56 ` Ingo Molnar 2015-09-26 6:44 ` Ard Biesheuvel 2015-09-26 6:44 ` Ard Biesheuvel 2015-09-26 13:43 ` Matt Fleming 2015-09-27 7:03 ` Ingo Molnar 2015-09-27 7:03 ` Ingo Molnar 2015-09-28 6:49 ` Ard Biesheuvel 2015-09-28 8:22 ` Ingo Molnar 2015-09-28 8:22 ` Ingo Molnar 2015-09-28 9:51 ` Ard Biesheuvel 2015-09-28 9:51 ` Ard Biesheuvel 2015-09-29 9:12 ` Ingo Molnar 2015-09-29 10:41 ` Ard Biesheuvel 2015-09-29 14:18 ` Matt Fleming 2015-09-29 14:18 ` Matt Fleming 2015-09-29 13:52 ` Matt Fleming 2015-09-29 13:52 ` Matt Fleming 2015-09-26 17:01 ` Andy Lutomirski 2015-09-26 17:01 ` Andy Lutomirski 2015-09-26 17:20 ` H. Peter Anvin 2015-09-26 18:15 ` Ard Biesheuvel 2015-09-26 18:15 ` Ard Biesheuvel 2015-09-26 19:49 ` H. Peter Anvin 2015-09-26 19:57 ` Matt Fleming 2015-09-26 20:09 ` Ard Biesheuvel 2015-09-26 20:09 ` Ard Biesheuvel 2015-09-26 20:19 ` H. Peter Anvin 2015-09-27 16:30 ` Andy Lutomirski 2015-09-27 18:06 ` Matthew Garrett 2015-09-27 18:06 ` Matthew Garrett 2015-09-28 6:16 ` Ingo Molnar 2015-09-28 6:16 ` Ingo Molnar 2015-09-28 6:41 ` Matthew Garrett 2015-09-29 21:58 ` Laszlo Ersek 2015-09-29 21:58 ` Laszlo Ersek 2015-09-30 9:30 ` Ard Biesheuvel 2015-09-30 16:43 ` Andy Lutomirski 2015-09-30 16:43 ` Andy Lutomirski 2015-09-30 17:24 ` James Bottomley 2015-09-30 17:24 ` James Bottomley 2015-09-30 17:24 ` James Bottomley 2015-09-30 0:54 ` H. Peter Anvin 2015-09-30 0:54 ` H. Peter Anvin 2015-09-26 19:55 ` Matt Fleming 2015-09-26 19:55 ` Matt Fleming 2015-09-27 6:50 ` Ingo Molnar 2015-10-01 12:48 ` [tip:core/urgent] x86/efi: Fix boot crash by mapping EFI memmap entries bottom-up at runtime, instead of top-down tip-bot for Matt Fleming 2015-10-02 9:44 ` Matt Fleming 2015-09-25 22:02 ` [PATCH 2/2] arm64/efi: Don't pad between EFI_MEMORY_RUNTIME regions Matt Fleming 2015-09-25 22:02 ` Matt Fleming 2015-09-26 6:01 ` Ingo Molnar 2015-09-26 6:01 ` Ingo Molnar 2015-09-26 7:08 ` Ard Biesheuvel [this message] 2015-09-26 7:08 ` Ard Biesheuvel 2015-09-27 7:06 ` Ingo Molnar 2015-09-27 7:06 ` Ingo Molnar 2015-09-27 10:40 ` Borislav Petkov 2015-09-28 6:20 ` Ingo Molnar 2015-09-29 9:31 ` Dave Young 2015-09-29 10:24 ` Borislav Petkov 2015-09-29 14:36 ` Matt Fleming 2015-09-29 14:36 ` Matt Fleming 2015-09-30 0:56 ` H. Peter Anvin 2015-09-30 0:56 ` H. Peter Anvin 2015-09-30 8:33 ` Borislav Petkov 2015-09-30 8:33 ` Borislav Petkov 2015-09-30 1:03 ` H. Peter Anvin 2015-09-30 1:16 ` Andy Lutomirski 2015-09-30 1:19 ` H. Peter Anvin 2015-09-30 4:24 ` Ard Biesheuvel 2015-09-30 4:24 ` Ard Biesheuvel 2015-10-01 10:44 ` Ingo Molnar 2015-10-01 12:49 ` [tip:core/urgent] arm64/efi: Fix boot crash by not padding " tip-bot for Ard Biesheuvel -- strict thread matches above, loose matches on Subject: below -- 2015-06-30 10:17 [PATCH 0/2] arm64/efi: adapt to UEFI 2.5 properties table changes Ard Biesheuvel [not found] ` <1435659443-17625-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> 2015-06-30 10:17 ` [PATCH 2/2] arm64/efi: don't pad between EFI_MEMORY_RUNTIME regions Ard Biesheuvel 2015-06-30 10:17 ` Ard Biesheuvel
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAKv+Gu_JRCmx5qKq7SDDq73BegVs3LKZwe+OdvTKb4YD_CURyw@mail.gmail.com \ --to=ard.biesheuvel@linaro.org \ --cc=akpm@linux-foundation.org \ --cc=bp@alien8.de \ --cc=brgerst@gmail.com \ --cc=catalin.marinas@arm.com \ --cc=dvlasenk@redhat.com \ --cc=hpa@zytor.com \ --cc=leif.lindholm@linaro.org \ --cc=linux-efi@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@kernel.org \ --cc=mark.rutland@arm.com \ --cc=matt.fleming@intel.com \ --cc=matt@codeblueprint.co.uk \ --cc=mingo@kernel.org \ --cc=msalter@redhat.com \ --cc=stable@vger.kernel.org \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=will.deacon@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.