Re: [PATCH 4.4 103/105] Revert "x86/mm/pat: Ensure cpa->pfn only contains page frame numbers"
On Thu, 23 Aug, at 09:37:28AM, Roland Dreier wrote: > > > This is bad enough that 4.4.148 and all newer 4.4.y crash early in > > > boot on some EFI systems that I have. > > > > Ugh, not good. > > > > > For now I am re-applying the "ensure cpa->pfn only contains page frame > > > numbers" patch, ported on top of 4.4.151. > > > > I can try to add it back and see what blows up, want me to attempt that? > > Not sure what to say... the current state is obviously broken. If you > look at what 02ff2769edbc is doing, it's clear that we're now shifting > cpa->pfn by PAGE_SHIFT where we weren't before, so we're putting bogus > values in the page table. And this is enough that my server system > booting with EFI crashes early in boot efi_enter_virtual_mode() with > the symptom that NX is improperly set on some pages (booting with > "noexec=off" fixes things, although obviously I don't want to run that > way). FWIW I can confirm that reverting the single patch 02ff2769edbc > fixes things, as does the cpa->pfn fix I mentioned above. > > It's hard for me to make a call on applying "ensure cpa->pfn only > contains page frame numbers" without knowing the problems it caused > before. The patch looks fine to me and I definitely need it, but > maybe it exposes some other bug elsewhere? Maybe Ben or Matt remember > more above why this was reverted in 4.4.106? Otherwise I'd say yeah, > we should re-apply it, since I don't think we want to revert > 02ff2769edbc. For the record, I wasn't even aware it had been reverted.
Re: [PATCH V4 0/3] Use mm_struct and switch_mm() instead of manually
On Thu, 18 Jan, at 01:01:04PM, Sai Praneeth Prakhya wrote: > From: Sai Praneeth > > Presently, in x86, to invoke any efi function like > efi_set_virtual_address_map() or any efi_runtime_service() the code path > typically involves read_cr3() (save previous pgd), write_cr3() > (write efi_pgd) and calling efi function. Likewise after returning from > efi function the code path typically involves read_cr3() (save efi_pgd), > write_cr3() (write previous pgd). We do this couple of times in efi > subsystem of Linux kernel, instead we can use helper function > efi_switch_mm() to do this. This improves readability and maintainability. > Also, instead of maintaining a separate struct "efi_scratch" to store/restore > efi_pgd, we can use mm_struct to do this. FWIW this series looks OK to me. Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] MAINTAINERS: Remove Matt Fleming as EFI co-maintainer
On Wed, 03 Jan, at 10:13:55AM, Ard Biesheuvel wrote: > On 3 January 2018 at 09:44, Matt Fleming wrote: > > Instate Ard Biesheuvel as the sole EFI maintainer and leave other folks > > as maintainers for the EFI test driver and efivarfs file system. > > > > Signed-off-by: Matt Fleming > > Acked-by: Ard Biesheuvel > > Thanks Matt > > > --- > > MAINTAINERS | 5 + > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > Ard, if you want to add yourself as co-maintainer of the EFI test > > driver or efivarfs, please go ahead. I'm sure no one would object. > > > > I guess that makes sense. Should we just fold that in? Sounds good to me. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] MAINTAINERS: Remove Matt Fleming as EFI co-maintainer
Instate Ard Biesheuvel as the sole EFI maintainer and leave other folks as maintainers for the EFI test driver and efivarfs file system. Signed-off-by: Matt Fleming --- MAINTAINERS | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) Ard, if you want to add yourself as co-maintainer of the EFI test driver or efivarfs, please go ahead. I'm sure no one would object. diff --git a/MAINTAINERS b/MAINTAINERS index d4fdcb12616c..9a41aa072e6a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5150,15 +5150,13 @@ F: sound/usb/misc/ua101.c EFI TEST DRIVER L: linux-efi@vger.kernel.org M: Ivan Hu -M: Matt Fleming S: Maintained F: drivers/firmware/efi/test/ EFI VARIABLE FILESYSTEM M: Matthew Garrett M: Jeremy Kerr -M: Matt Fleming -T: git git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git +T: git git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git L: linux-efi@vger.kernel.org S: Maintained F: fs/efivarfs/ @@ -5319,7 +5317,6 @@ S:Supported F: security/integrity/evm/ EXTENSIBLE FIRMWARE INTERFACE (EFI) -M: Matt Fleming M: Ard Biesheuvel L: linux-efi@vger.kernel.org T: git git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git -- 2.14.2 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] x86/efi: fix kernel param add_efi_memmap regression
On Sat, 16 Dec, at 12:19:53PM, Dave Young wrote: > 'add_efi_memmap' is an early param, but do_add_efi_memmap() has no > chance to run because the code path is before parse_early_param(). > I believe it worked when the param was introduced but probably later > some other changes caused the wrong order and nobody noticed it. > > Move efi_memblock_x86_reserve_range() after parse_early_param() > to fix it. > > Signed-off-by: Dave Young > --- > arch/x86/kernel/setup.c |5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) Thanks Dave, applied to 'urgent'. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
On Sat, 16 Dec, at 03:06:32PM, Ingo Molnar wrote: > > * Matt Fleming wrote: > > > > x86_init.oem.arch_setup(); > > > @@ -962,6 +959,8 @@ void __init setup_arch(char **cmdline_p) > > > > > > parse_early_param(); > > > > > > + if (efi_enabled(EFI_BOOT)) > > > + efi_memblock_x86_reserve_range(); > > > #ifdef CONFIG_MEMORY_HOTPLUG > > > /* > > >* Memory used by the kernel cannot be hot-removed because Linux > > > > > > > I prefer this version. Please re-send a full patch and update the > > subject line to include the "fix" somewhere; it wasn't obvious to me > > from the start that this is a bug fix. > > Agreed. > > I dropped the commit I just applied to tip:efi/core, since you are handling > this, > so this patch should come through the regular EFI channels, right? Yep, that's right. Me or Ard will take care of it. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi: make EFI a menuconfig to ease disabling it all
On Sat, 09 Dec, at 04:52:52PM, Vincent Legoll wrote: > No need to get into the submenu to disable all related > config entries. > > This makes it easier to disable all EFI config options > without entering the submenu. It will also enable one > to see that en/dis-abled state from the outside menu. > > This is only intended to change menuconfig UI, not change > the config dependencies. > > Signed-off-by: Vincent Legoll > --- > drivers/firmware/efi/Kconfig | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) This looks fine to me. Ard? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86: move parse_early_param to earlier code for add_efi_memmap
On Thu, 14 Dec, at 06:41:19PM, Dave Young wrote:
> On 11/30/17 at 01:23pm, Dave Young wrote:
> > 'add_efi_memmap' is an early param, but do_add_efi_memmap() has no
> > chance to run because the code path is before parse_early_param().
> > I believe it worked when the param was introduced but probably later
> > some other changes caused the wrong order and nobody noticed it.
> >
> > Move parse_early_param before efi_memblock_x86_reserve_range to fix
> > this.
> >
> > Signed-off-by: Dave Young
> > ---
> > arch/x86/kernel/setup.c | 55
> >
> > 1 file changed, 28 insertions(+), 27 deletions(-)
> >
> > --- linux-x86.orig/arch/x86/kernel/setup.c
> > +++ linux-x86/arch/x86/kernel/setup.c
> > @@ -897,6 +897,34 @@ void __init setup_arch(char **cmdline_p)
> > rd_prompt = ((boot_params.hdr.ram_size & RAMDISK_PROMPT_FLAG) != 0);
> > rd_doload = ((boot_params.hdr.ram_size & RAMDISK_LOAD_FLAG) != 0);
> > #endif
> > +
> > +#ifdef CONFIG_CMDLINE_BOOL
> > +#ifdef CONFIG_CMDLINE_OVERRIDE
> > + strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> > +#else
> > + if (builtin_cmdline[0]) {
> > + /* append boot loader cmdline to builtin */
> > + strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
> > + strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> > + strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> > + }
> > +#endif
> > +#endif
> > +
> > + strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> > + *cmdline_p = command_line;
> > +
> > + /*
> > +* x86_configure_nx() is called before parse_early_param() to detect
> > +* whether hardware doesn't support NX (so that the early EHCI debug
> > +* console setup can safely call set_fixmap()). It may then be called
> > +* again from within noexec_setup() during parsing early parameters
> > +* to honor the respective command line option.
> > +*/
> > + x86_configure_nx();
> > +
> > + parse_early_param();
> > +
> > #ifdef CONFIG_EFI
> > if (!strncmp((char *)&boot_params.efi_info.efi_loader_signature,
> > EFI32_LOADER_SIGNATURE, 4)) {
> > @@ -935,33 +963,6 @@ void __init setup_arch(char **cmdline_p)
> > bss_resource.start = __pa_symbol(__bss_start);
> > bss_resource.end = __pa_symbol(__bss_stop)-1;
> >
> > -#ifdef CONFIG_CMDLINE_BOOL
> > -#ifdef CONFIG_CMDLINE_OVERRIDE
> > - strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> > -#else
> > - if (builtin_cmdline[0]) {
> > - /* append boot loader cmdline to builtin */
> > - strlcat(builtin_cmdline, " ", COMMAND_LINE_SIZE);
> > - strlcat(builtin_cmdline, boot_command_line, COMMAND_LINE_SIZE);
> > - strlcpy(boot_command_line, builtin_cmdline, COMMAND_LINE_SIZE);
> > - }
> > -#endif
> > -#endif
> > -
> > - strlcpy(command_line, boot_command_line, COMMAND_LINE_SIZE);
> > - *cmdline_p = command_line;
> > -
> > - /*
> > -* x86_configure_nx() is called before parse_early_param() to detect
> > -* whether hardware doesn't support NX (so that the early EHCI debug
> > -* console setup can safely call set_fixmap()). It may then be called
> > -* again from within noexec_setup() during parsing early parameters
> > -* to honor the respective command line option.
> > -*/
> > - x86_configure_nx();
> > -
> > - parse_early_param();
> > -
> > #ifdef CONFIG_MEMORY_HOTPLUG
> > /*
> > * Memory used by the kernel cannot be hot-removed because Linux
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
> Ping for review..
>
> Another way is move "efi_memblock_x86_reserve_range" to later code
> Maybe below is better?
>
> ---
> arch/x86/kernel/setup.c |5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> --- linux-x86.orig/arch/x86/kernel/setup.c
> +++ linux-x86/arch/x86/kernel/setup.c
> @@ -906,9 +906,6 @@ void __init setup_arch(char **cmdline_p)
> set_bit(EFI_BOOT, &efi.flags);
> set_bit(EFI_64BIT, &efi.flags);
> }
> -
> - if (efi_enabled(EFI_BOOT))
> - efi_memblock_x86_reserve_range();
> #endif
>
> x86_init.oem.arch_setup();
> @@ -962,6 +959,8 @@ void __init setup_arch(char **cmdline_p)
>
> parse_early_param();
>
> + if (efi_enabled(EFI_BOOT))
> + efi_memblock_x86_reserve_range();
> #ifdef CONFIG_MEMORY_HOTPLUG
> /*
>* Memory used by the kernel cannot be hot-removed because Linux
>
I prefer this version. Please re-send a full patch and update the
subject line to include the "fix" somewhere; it wasn't obvious to me
from the start that this is a bug fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger
Re: [PATCH] efi: Use PTR_ERR_OR_ZERO()
On Tue, 28 Nov, at 10:39:37PM, Vasyl Gomonovych wrote: > Fix ptr_ret.cocci warnings: > drivers/firmware/efi/efi.c:610:8-14: WARNING: PTR_ERR_OR_ZERO can be used > > Use PTR_ERR_OR_ZERO rather than if(IS_ERR(...)) + PTR_ERR > > Generated by: scripts/coccinelle/api/ptr_ret.cocci > > Signed-off-by: Vasyl Gomonovych > --- > drivers/firmware/efi/efi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Thanks. Applied to the 'next' branch. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] hash addresses printed with %p
(Cc'ing Dave since this is used for kexec on EFI) On Fri, 01 Dec, at 09:54:43AM, Ard Biesheuvel wrote: > On 1 December 2017 at 09:48, Greg Kroah-Hartman > wrote: > > On Thu, Nov 30, 2017 at 05:18:42PM +, Ard Biesheuvel wrote: > >> On 30 November 2017 at 17:10, Greg Kroah-Hartman > >> wrote: > >> > On Thu, Nov 30, 2017 at 04:32:35PM +, Greg Kroah-Hartman wrote: > >> >> On Wed, Nov 29, 2017 at 01:36:25PM -0800, Linus Torvalds wrote: > >> >> > On Wed, Nov 29, 2017 at 1:14 PM, Linus Torvalds > >> >> > wrote: > >> >> > > > >> >> > > Not because %pK itself changed, but because the semantics of %p did. > >> >> > > The baseline moved, and the "safe" version did not. > >> >> > > >> >> > Btw, that baseline for me is now that I can do > >> >> > > >> >> > ./scripts/leaking_addresses.pl | wc -l > >> >> > 18 > >> >> > > >> >> > and of those 18 hits, six are false positives (looks like bitmaps in > >> >> > the uevent keys). > >> >> > > >> >> > The remaining 12 are from the EFI runtime map files > >> >> > (/sys/firmware/efi/runtime-map/*). They should presumably not be > >> >> > world-readable, but sadly the kset_create_and_add() helper seems to do > >> >> > that by default. > >> >> > > >> >> > I think the sysfs code makes it insanely too easy to make things > >> >> > world-readable. You try to be careful, and mark things read-only etc, > >> >> > but __ATTR_RO() jkust means S_IRUGO, which means world-readable. > >> >> > > >> >> > There seems to be no convenient model for kobjects having better > >> >> > permissions. Greg? > >> >> > >> >> They can just use __ATTR() which lets you set the exact mode settings > >> >> that are wanted. > >> >> > >> >> Something like the patch below, which breaks the build as the > >> >> map_attributes are "odd", but you get the idea. The EFI developers can > >> >> fix this up properly :) > >> >> > >> >> Note, this only accounts for 5 attributes, what is the whole list? > >> > > >> > Ah, it's the virt_addr file 12 times, I just ran it on my laptop: > >> > > >> > /sys/firmware/efi/runtime-map/7/virt_addr: 0xfffeea6ea000 > >> > /sys/firmware/efi/runtime-map/5/virt_addr: 0xfffeee88b000 > >> > /sys/firmware/efi/runtime-map/3/virt_addr: 0xfffefea0 > >> > /sys/firmware/efi/runtime-map/11/virt_addr: 0xfffed9c0 > >> > /sys/firmware/efi/runtime-map/1/virt_addr: 0xfffefee0 > >> > /sys/firmware/efi/runtime-map/8/virt_addr: 0xfffedba4e000 > >> > /sys/firmware/efi/runtime-map/6/virt_addr: 0xfffeee2de000 > >> > /sys/firmware/efi/runtime-map/4/virt_addr: 0xfffeeea0 > >> > /sys/firmware/efi/runtime-map/2/virt_addr: 0xfffefec0 > >> > /sys/firmware/efi/runtime-map/10/virt_addr: 0xfffed9c6 > >> > /sys/firmware/efi/runtime-map/0/virt_addr: 0xfffeff00 > >> > /sys/firmware/efi/runtime-map/9/virt_addr: 0xfffedb9c9000 > >> > > >> > So changing it to use __ATTR() should fix this remaning leakage up. > >> > That is if we even really need to export these values at all. What does > >> > userspace do with them? Shouldn't they just be in debugfs instead? > >> > > >> > >> These are the virtual mappings UEFI firmware regions, which must > >> remain in the same place across kexec reboots. So kexec tooling > >> consumes this information and passes it on to the incoming kernel in > >> some way. > >> > >> Note that these are not kernel addresses, so while I agree they should > >> not be world readable, they won't give you any clue as to where the > >> kernel itself is mapped. > >> > >> So the recommendation is to switch to __ATTR( ... 0400 ... ) instead? > >> If so, I'll code up a patch. > > > > If these pointers are not "real", I recommend just leaving them as-is. > > That's not what I said :-) > > These are real pointers, and stuff will actually be mapped there > (although I am not intimately familiar with the way x86 does this, but > on ARM [which does not have these sysfs nodes in the first place], > these mappings are only live during the time a UEFI runtime service > call is in progress, and IIRC, work was underway to do the same for > x86). So while these values don't correlate with the placement of > kernel data structures, they could still be useful for an attacker to > figure out where exploitable firmware memory regions are located, > especially given that some of these may be mapped RWX. These are mappings of the EFI firmware's runtime regions, dynamically allocated by the kernel starting at EFI_VA_START. Because we only get one chance to tell the firmware where we placed its regions (via SetVirtualAddressMap()) we have to guarantee that any subsequent kexec reboots use the same addresses. So that's why they're exported to userspace through sysfs. Like Ard said, these mappings are not mapped into the regular process address space. Instead, they're only used while making EFI runtime calls. But this is still an information leak. And I think _ATTR(..0400) is the right way to fix it. Dave, could you double-check my logic a
Re: ACK: [PATCH] efi/efi_test: Prevent an Oops in efi_runtime_query_capsulecaps()
On Wed, 11 Oct, at 02:26:57PM, Ivan Hu wrote:
>
>
> On 10/06/2017 08:19 PM, Matt Fleming wrote:
> >On Sat, 30 Sep, at 11:17:32AM, Dan Carpenter wrote:
> >>If "qcaps.capsule_count" is ULONG_MAX then "qcaps.capsule_count + 1"
> >>will overflow to zero and kcalloc() will return the ZERO_SIZE_PTR. We
> >>try to dereference it inside the loop and crash.
> >>
> >>Fixes: ff6301dabc3c ("efi: Add efi_test driver for exporting UEFI runtime
> >>service interfaces")
> >>Signed-off-by: Dan Carpenter
> >>
> >>diff --git a/drivers/firmware/efi/test/efi_test.c
> >>b/drivers/firmware/efi/test/efi_test.c
> >>index 08129b7b80ab..41c48a1e8baa 100644
> >>--- a/drivers/firmware/efi/test/efi_test.c
> >>+++ b/drivers/firmware/efi/test/efi_test.c
> >>@@ -593,6 +593,9 @@ static long efi_runtime_query_capsulecaps(unsigned long
> >>arg)
> >>if (copy_from_user(&qcaps, qcaps_user, sizeof(qcaps)))
> >>return -EFAULT;
> >>+ if (qcaps.capsule_count == ULONG_MAX)
> >>+ return -EINVAL;
> >>+
> >>capsules = kcalloc(qcaps.capsule_count + 1,
> >> sizeof(efi_capsule_header_t), GFP_KERNEL);
> >>if (!capsules)
> >
> >This looks OK to me. Ivan?
> >
>
>
> Acked-by: Ivan Hu
Thanks, applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/efi_test: Prevent an Oops in efi_runtime_query_capsulecaps()
On Sat, 30 Sep, at 11:17:32AM, Dan Carpenter wrote:
> If "qcaps.capsule_count" is ULONG_MAX then "qcaps.capsule_count + 1"
> will overflow to zero and kcalloc() will return the ZERO_SIZE_PTR. We
> try to dereference it inside the loop and crash.
>
> Fixes: ff6301dabc3c ("efi: Add efi_test driver for exporting UEFI runtime
> service interfaces")
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/firmware/efi/test/efi_test.c
> b/drivers/firmware/efi/test/efi_test.c
> index 08129b7b80ab..41c48a1e8baa 100644
> --- a/drivers/firmware/efi/test/efi_test.c
> +++ b/drivers/firmware/efi/test/efi_test.c
> @@ -593,6 +593,9 @@ static long efi_runtime_query_capsulecaps(unsigned long
> arg)
> if (copy_from_user(&qcaps, qcaps_user, sizeof(qcaps)))
> return -EFAULT;
>
> + if (qcaps.capsule_count == ULONG_MAX)
> + return -EINVAL;
> +
> capsules = kcalloc(qcaps.capsule_count + 1,
> sizeof(efi_capsule_header_t), GFP_KERNEL);
> if (!capsules)
This looks OK to me. Ivan?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/capsule-loader: pr_err() strings should end with newlines
On Mon, 25 Sep, at 04:17:23PM, Arvind Yadav wrote:
> pr_err() messages should terminated with a new-line to avoid
> other messages being concatenated onto the end.
>
> Signed-off-by: Arvind Yadav
> ---
> drivers/firmware/efi/capsule-loader.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c
> b/drivers/firmware/efi/capsule-loader.c
> index ec8ac5c..51d2874 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -49,7 +49,7 @@ int __efi_capsule_setup_info(struct capsule_info *cap_info)
> pages_needed = ALIGN(cap_info->total_size, PAGE_SIZE) / PAGE_SIZE;
>
> if (pages_needed == 0) {
> - pr_err("invalid capsule size");
> + pr_err("invalid capsule size\n");
> return -EINVAL;
> }
>
Thanks, applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] x86/efi: Use efi_switch_mm() rather than manually twiddling with cr3
On Wed, 16 Aug, at 12:03:22PM, Mark Rutland wrote: > > I'd expect we'd abort at a higher level, not taking any sample. i.e. > we'd have the core overflow handler check in_funny_mm(), and if so, skip > the sample, as with the skid case. FYI, this is my preferred solution for x86 too. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v9 1/2] efi: Introduce efi_early_memdesc_ptr to get pointer to memmap descriptor
On Mon, 14 Aug, at 10:54:23PM, Baoquan He wrote:
> The existing map iteration helper for_each_efi_memory_desc_in_map can
> only be used after OS initializes EFI to fill data of struct efi_memory_map.
Should this say "EFI subsystem"? The firmware doesn't care about the
kernel's internal data structures.
> Before that we also need iterate map descriptors which are stored in several
> intermediate structures, like struct efi_boot_memmap for arch independent
> usage and struct efi_info for x86 arch only.
>
> Introduce efi_early_memdesc_ptr to get pointer to a map descriptor, and
> replace several places of open code with it.
>
> Suggested-by: Ingo Molnar
> Signed-off-by: Baoquan He
> ---
> arch/x86/boot/compressed/eboot.c | 2 +-
> drivers/firmware/efi/libstub/efi-stub-helper.c | 4 ++--
> include/linux/efi.h| 21 +
> 3 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/eboot.c
> b/arch/x86/boot/compressed/eboot.c
> index c3e869eaef0c..e007887a33b0 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -767,7 +767,7 @@ static efi_status_t setup_e820(struct boot_params *params,
> m |= (u64)efi->efi_memmap_hi << 32;
> #endif
>
> - d = (efi_memory_desc_t *)(m + (i * efi->efi_memdesc_size));
> + d = efi_early_memdesc_ptr(m, efi->efi_memdesc_size, i);
> switch (d->type) {
> case EFI_RESERVED_TYPE:
> case EFI_RUNTIME_SERVICES_CODE:
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
> b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index b0184360efc6..50a9cab5a834 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -205,7 +205,7 @@ efi_status_t efi_high_alloc(efi_system_table_t
> *sys_table_arg,
> unsigned long m = (unsigned long)map;
> u64 start, end;
>
> - desc = (efi_memory_desc_t *)(m + (i * desc_size));
> + desc = efi_early_memdesc_ptr(m, desc_size, i);
> if (desc->type != EFI_CONVENTIONAL_MEMORY)
> continue;
>
> @@ -298,7 +298,7 @@ efi_status_t efi_low_alloc(efi_system_table_t
> *sys_table_arg,
> unsigned long m = (unsigned long)map;
> u64 start, end;
>
> - desc = (efi_memory_desc_t *)(m + (i * desc_size));
> + desc = efi_early_memdesc_ptr(m, desc_size, i);
>
> if (desc->type != EFI_CONVENTIONAL_MEMORY)
> continue;
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 8269bcb8ccf7..9783d9e4a4b2 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -1020,6 +1020,27 @@ extern int efi_memattr_init(void);
> extern int efi_memattr_apply_permissions(struct mm_struct *mm,
>efi_memattr_perm_setter fn);
>
> +/*
> + * efi_early_memdesc_ptr - get the n-th efi memmap descriptor
> + * @map: the start of efi memmap
> + * @desc_size: the size of space for each efi memmap descriptor
> + * @n: the index of efi memmap descriptor
> + *
> + * EFI boot service provides function GetMemoryMap() to get a copy of the
> + * current memory map which is an array of memory descriptors, each of
> + * which describes a contiguous block of memory. And also get the size of
> + * map, and the size of each descriptor, etc. Note that per section 6.2 of
> + * UEFI Spec 2.6 Errata A, the returned size of each descriptor might not
> + * be equal to sizeof(efi_memory_memdesc_t) since efi_memory_memdesc_t may
> + * be extended in the future in response to hardware innovation. Thus OS
> + * MUST use the returned size of descriptor to find the start of each
> + * efi_memory_memdesc_t in the memory map array. This should only be used
> + * during bootup since for_each_efi_memory_desc_xxx is suggested after OS
> + * initializes EFI to fill data of struct efi_memory_map.
> + */
Again, please use "EFI subsystem" here.
> +#define efi_early_memdesc_ptr(map, desc_size, n) \
> + (efi_memory_desc_t *)((void *)(map) + ((n) * (desc_size)))
> +
> /* Iterate through an efi_memory_map */
> #define for_each_efi_memory_desc_in_map(m, md)
>\
> for ((md) = (m)->map; \
Otherwise, this looks OK to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86/efi: page align EFI ROM image ranges
On Wed, 02 Aug, at 11:41:38AM, Stuart Hayes wrote:
> (Resend because I mistyped the maintainer's email address the first time.)
>
> The kernel's EFI stub locates and copies EFI ROM images into memory,
> which it allocates using the byte-granular EFI allocate_pool
> function. These memory ranges are then added to setup_data, and
> later to e820 (in e820__reserve_setup_data()). The e820 ranges are
> parsed to create nosave regions (in
> e820__register_nosave_regions()), but when non-page-aligned e820
> regions are parsed, a nosave page is added at the beginning and end
> of each non-page-aligned region, which results in data not getting
> saved or restored during a hibernate/resume. This can result in
> random failures after a hibernate/resume.
>
> Round up the allocation size to a whole number of pages, and use EFI
> allocate_pages to ensure that the EFI ROM copy regions are
> page-aligned.
>
> On a system with six EFI ROM images, before the patch:
>
> e820: update [mem 0x64866020-0x6486e05f] usable ==> usable
> e820: update [mem 0x6147a020-0x61499c5f] usable ==> usable
> e820: update [mem 0x60fff020-0x6105785f] usable ==> usable
> e820: update [mem 0x60fa6020-0x60ffe85f] usable ==> usable
> e820: update [mem 0x60f4d020-0x60fa585f] usable ==> usable
> e820: update [mem 0x60ef4020-0x60f4c85f] usable ==> usable
> ...
> PM: Registered nosave memory: [mem 0x-0x0fff]
> PM: Registered nosave memory: [mem 0x000a-0x000f]
> PM: Registered nosave memory: [mem 0x60ef4000-0x60ef4fff]
> PM: Registered nosave memory: [mem 0x60f4c000-0x60f4cfff]
> PM: Registered nosave memory: [mem 0x60f4d000-0x60f4dfff]
> PM: Registered nosave memory: [mem 0x60fa5000-0x60fa5fff]
> PM: Registered nosave memory: [mem 0x60fa6000-0x60fa6fff]
> PM: Registered nosave memory: [mem 0x60ffe000-0x60ffefff]
> PM: Registered nosave memory: [mem 0x60fff000-0x60ff]
> PM: Registered nosave memory: [mem 0x61057000-0x61057fff]
> PM: Registered nosave memory: [mem 0x6147a000-0x6147afff]
> PM: Registered nosave memory: [mem 0x61499000-0x61499fff]
> PM: Registered nosave memory: [mem 0x64866000-0x64866fff]
> PM: Registered nosave memory: [mem 0x6486e000-0x6486efff]
> PM: Registered nosave memory: [mem 0x6cf6e000-0x6f3ccfff]
>
> After the patch:
>
> e820: update [mem 0x64866000-0x6486efff] usable ==> usable
> e820: update [mem 0x6147a000-0x61499fff] usable ==> usable
> e820: update [mem 0x60fff000-0x61057fff] usable ==> usable
> e820: update [mem 0x60fa6000-0x60ffefff] usable ==> usable
> e820: update [mem 0x60f4d000-0x60fa5fff] usable ==> usable
> e820: update [mem 0x60ef4000-0x60f4cfff] usable ==> usable
> ...
> PM: Registered nosave memory: [mem 0x-0x0fff]
> PM: Registered nosave memory: [mem 0x000a-0x000f]
> PM: Registered nosave memory: [mem 0x6cf6e000-0x6f3ccfff]
>
> Signed-off-by: Stuart Hayes
> ---
>
> --- linux-4.13-rc2/arch/x86/boot/compressed/eboot.c.orig 2017-08-01
> 12:12:04.696049106 -0400
> +++ linux-4.13-rc2/arch/x86/boot/compressed/eboot.c 2017-08-01
> 12:11:33.120182236 -0400
> @@ -235,7 +235,12 @@ __setup_efi_pci64(efi_pci_io_protocol_64
>
> size = pci->romsize + sizeof(*rom);
>
> - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, size, &rom);
> + /*
> + * Get whole pages because this will be added to e820.
> + */
> + size = PAGE_ALIGN(size);
> + status = efi_call_early(allocate_pages, EFI_ALLOCATE_ANY_PAGES,
> + EFI_LOADER_DATA, (size >> PAGE_SHIFT), &rom);
> if (status != EFI_SUCCESS) {
> efi_printk(sys_table, "Failed to alloc mem for rom\n");
> return status;
>
Nice catch. The comment could do with a little more information,
including the fact that it's the e820 nosave code that expects
page-aligned ROM regions.
Also, you'll need the same fix for __setup_efi_pci32().
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 RESEND] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions
On Fri, 04 Aug, at 07:40:05PM, Baoquan He wrote: > On 08/04/17 at 12:23pm, Matt Fleming wrote: > > On Fri, 28 Jul, at 07:26:03PM, Baoquan He wrote: > > > Hi Matt, > > > > > > On 07/28/17 at 11:55am, Ingo Molnar wrote: > > > > > > > > * Matt Fleming wrote: > > > > > > > > > On Fri, 21 Jul, at 09:19:56PM, Baoquan He wrote: > > > > > > > > > > > > There are places where the efi map is getting and used like this. > > > > > > E.g > > > > > > in efi_high_alloc() of > > > > > > drivers/firmware/efi/libstub/efi-stub-helper.c. > > > > > > EFI developers worry the size of efi_memory_desc_t could not be the > > > > > > same > > > > > > as e->efi_memdesc_size? > > > > > > > > > > > > Hi Matt, > > > > > > > > > > > > Could you help have a look at this? > > > > > > > > > > You're exactly right. The code guards against the size of the > > > > > efi_memory_desc_t struct changing. The UEFI spec says to traverse the > > > > > memory map this way. > > > > > > > > This is not obvious and looks pretty ugly as well, and open coded in > > > > several > > > > places. > > > > > > > > At minimum we should have an efi_memdesc_ptr(efi, i) wrapper inline (or > > > > so) that > > > > gives us the entry pointer, plus a comment that points out that > > > > ->memdesc_size > > > > might not be equal to sizeof(efi_memory_memdesc_t). > > > > > > I can make a efi_memdesc_ptr(efi, i) wrapper as Ingo suggested and use > > > it here if you agree. Seems it might be not good to add another > > > for_each_efi_memory_desc_ wrapper since there are different memmap > > > data structures in x86 boot and in general efi libstub. Or any other > > > idea? > > > > I think adding a wrapper is fine, but I'd suggest including the word > > "early" (or something similar) to explain that it should only be used > > during bootup -- we want everyone else to use the > > for_each_efi_memory_*() API. > > Thanks, Matt. I can do that. Do you think below helper definition is OK > to you? If yes, I can upstate with it and post v9. > > #define efi_early_memdesc_ptr(map, desc_size, n) > \ > (efi_memory_desc_t *)((void *)(map) + ((n) * (desc_size))) Looks fine to me, but I'd wait for Ingo's OK before resending. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 RESEND] x86/boot/KASLR: Restrict kernel to be randomized in mirror regions
On Fri, 28 Jul, at 07:26:03PM, Baoquan He wrote: > Hi Matt, > > On 07/28/17 at 11:55am, Ingo Molnar wrote: > > > > * Matt Fleming wrote: > > > > > On Fri, 21 Jul, at 09:19:56PM, Baoquan He wrote: > > > > > > > > There are places where the efi map is getting and used like this. E.g > > > > in efi_high_alloc() of drivers/firmware/efi/libstub/efi-stub-helper.c. > > > > EFI developers worry the size of efi_memory_desc_t could not be the same > > > > as e->efi_memdesc_size? > > > > > > > > Hi Matt, > > > > > > > > Could you help have a look at this? > > > > > > You're exactly right. The code guards against the size of the > > > efi_memory_desc_t struct changing. The UEFI spec says to traverse the > > > memory map this way. > > > > This is not obvious and looks pretty ugly as well, and open coded in > > several > > places. > > > > At minimum we should have an efi_memdesc_ptr(efi, i) wrapper inline (or so) > > that > > gives us the entry pointer, plus a comment that points out that > > ->memdesc_size > > might not be equal to sizeof(efi_memory_memdesc_t). > > I can make a efi_memdesc_ptr(efi, i) wrapper as Ingo suggested and use > it here if you agree. Seems it might be not good to add another > for_each_efi_memory_desc_ wrapper since there are different memmap > data structures in x86 boot and in general efi libstub. Or any other > idea? I think adding a wrapper is fine, but I'd suggest including the word "early" (or something similar) to explain that it should only be used during bootup -- we want everyone else to use the for_each_efi_memory_*() API. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 19/36] x86/mm: Add support to access boot related data in the clear
On Fri, 16 Jun, at 01:53:26PM, Tom Lendacky wrote: > Boot data (such as EFI related data) is not encrypted when the system is > booted because UEFI/BIOS does not run with SME active. In order to access > this data properly it needs to be mapped decrypted. > > Update early_memremap() to provide an arch specific routine to modify the > pagetable protection attributes before they are applied to the new > mapping. This is used to remove the encryption mask for boot related data. > > Update memremap() to provide an arch specific routine to determine if RAM > remapping is allowed. RAM remapping will cause an encrypted mapping to be > generated. By preventing RAM remapping, ioremap_cache() will be used > instead, which will provide a decrypted mapping of the boot related data. > > Signed-off-by: Tom Lendacky > --- > arch/x86/include/asm/io.h |5 + > arch/x86/mm/ioremap.c | 179 > + > include/linux/io.h|2 + > kernel/memremap.c | 20 - > mm/early_ioremap.c| 18 - > 5 files changed, 217 insertions(+), 7 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 18/36] x86/efi: Update EFI pagetable creation to work with SME
On Fri, 16 Jun, at 01:53:17PM, Tom Lendacky wrote: > When SME is active, pagetable entries created for EFI need to have the > encryption mask set as necessary. > > When the new pagetable pages are allocated they are mapped encrypted. So, > update the efi_pgt value that will be used in cr3 to include the encryption > mask so that the PGD table can be read successfully. The pagetable mapping > as well as the kernel are also added to the pagetable mapping as encrypted. > All other EFI mappings are mapped decrypted (tables, etc.). > > Reviewed-by: Borislav Petkov > Signed-off-by: Tom Lendacky > --- > arch/x86/platform/efi/efi_64.c | 15 +++ > 1 file changed, 11 insertions(+), 4 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 16/36] efi: Add an EFI table address match function
On Fri, 16 Jun, at 01:52:53PM, Tom Lendacky wrote: > Add a function that will determine if a supplied physical address matches > the address of an EFI table. > > Reviewed-by: Borislav Petkov > Signed-off-by: Tom Lendacky > --- > drivers/firmware/efi/efi.c | 33 + > include/linux/efi.h|7 +++ > 2 files changed, 40 insertions(+) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 17/36] efi: Update efi_mem_type() to return an error rather than 0
On Fri, 16 Jun, at 01:53:06PM, Tom Lendacky wrote: > The efi_mem_type() function currently returns a 0, which maps to > EFI_RESERVED_TYPE, if the function is unable to find a memmap entry for > the supplied physical address. Returning EFI_RESERVED_TYPE implies that > a memmap entry exists, when it doesn't. Instead of returning 0, change > the function to return a negative error value when no memmap entry is > found. > > Reviewed-by: Borislav Petkov > Signed-off-by: Tom Lendacky > --- > arch/ia64/kernel/efi.c |4 ++-- > arch/x86/platform/efi/efi.c |6 +++--- > include/linux/efi.h |2 +- > 3 files changed, 6 insertions(+), 6 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: efi/reboot: Fall back to original power-off method if EFI_RESET_SHUTDOWN returns
On Wed, 21 Jun, at 03:15:09PM, Hans de Goede wrote:
> HI,
>
> On 23-04-17 14:36, Hans de Goede wrote:
> >Commit 44be28e9dd98 ("x86/reboot: Add EFI reboot quirk for ACPI Hardware
> >Reduced flag") sets pm_power_off to efi_power_off() when the
> >acpi_gbl_reduced_hardware flag is set.
> >
> >According to its commit message this is necessary because: "BayTrail-T
> >class of hardware requires EFI in order to powerdown and reboot and no
> >other reliable method exists"
> >
> >But I have a Bay Trail CR tablet where the EFI_RESET_SHUTDOWN call does
> >not work, it simply returns without doing anything (AFAICT).
> >
> >So it seems that some Bay Trail devices must use EFI for power-off, while
> >for others only ACPI works.
> >
> >Note that efi_power_off() only gets used if the platform code defines
> >efi_poweroff_required() and that returns true, this currently only ever
> >happens on x86.
> >
> >Since on the devices which need ACPI for power-off the EFI_RESET_SHUTDOWN
> >call simply returns, this patch makes the efi-reboot code remember the
> >old pm_power_off handler and if EFI_RESET_SHUTDOWN returns it falls back
> >to calling that.
> >
> >This seems preferable to dmi-quirking our way out of this, since there
> >are likely quite a few devices suffering from this.
> >
> >Cc: Mark Salter
> >Signed-off-by: Hans de Goede
>
> What is the status of this patch ? It has had 2 somewhat favorable
> reviews and then things went silent ?
Sorry about the delay. I've picked this up for the efi-next branch
since we're at -rc6 and it doesn't look like an -rc6-urgent bug.
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] efi: Don't issue error message when booted under xen
From: Juergen Gross
When booted as Xen dom0 there won't be an EFI memmap allocated. Avoid
issuing an error message in this case:
[0.144079] efi: Failed to allocate new EFI memmap
Signed-off-by: Juergen Gross
Cc: "H. Peter Anvin"
Cc: Ingo Molnar
Cc: Thomas Gleixner
Cc: Ard Biesheuvel
Cc: # v4.9+
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/quirks.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 26615991d69c..e0cf95a83f3f 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -360,6 +360,9 @@ void __init efi_free_boot_services(void)
free_bootmem_late(start, size);
}
+ if (!num_entries)
+ return;
+
new_size = efi.memmap.desc_size * num_entries;
new_phys = efi_memmap_alloc(num_entries);
if (!new_phys) {
--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] efi/bgrt: Skip efi_bgrt_init in case of non-efi boot
From: Dave Young
Sabrina Dubroca reported an early panic below, it was introduced by
commit 7b0a911478c7 ("efi/x86: Move the EFI BGRT init code to early init
code"). The cause is on this machine even for legacy boot firmware still
provide the ACPI BGRT table which should be EFI only. Thus the garbage
bgrt data caused the efi_bgrt_init panic.
Add a checking to skip efi_bgrt_init in case non EFI booting solves this
problem.
BUG: unable to handle kernel paging request at ff240001
IP: efi_bgrt_init+0xdc/0x134
PGD 1ac0c067
PUD 1ac0e067
PMD 1aee9067
PTE 938070180163
Oops: 0009 [#1] SMP
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted 4.10.0-rc5-00116-g7b0a911 #19
Hardware name: Hewlett-Packard HP Z220 CMT Workstation/1790, BIOS K51 v01.02
05/03/2012
task: 9fc10500 task.stack: 9fc0
RIP: 0010:efi_bgrt_init+0xdc/0x134
RSP: :9fc03d58 EFLAGS: 00010082
RAX: ff240001 RBX: RCX: 138070180006
RDX: 8163 RSI: 938070180163 RDI: 05be
RBP: 9fc03d70 R08: 138070181000 R09: 0002
R10: 0002d000 R11: 98a3dedd2fc6 R12: 9f9f22b6
R13: 9ff49480 R14: 0010 R15:
FS: () GS:9fd2() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: ff240001 CR3: 1ac09000 CR4: 000406b0
Call Trace:
? acpi_parse_ioapic+0x98/0x98
acpi_parse_bgrt+0x9/0xd
acpi_table_parse+0x7a/0xa9
acpi_boot_init+0x3c7/0x4f9
? acpi_parse_x2apic+0x74/0x74
? acpi_parse_x2apic_nmi+0x46/0x46
setup_arch+0xb4b/0xc6f
? printk+0x52/0x6e
start_kernel+0xb2/0x47b
? early_idt_handler_array+0x120/0x120
x86_64_start_reservations+0x24/0x26
x86_64_start_kernel+0xf7/0x11a
start_cpu+0x14/0x14
Code: 48 c7 c7 10 16 a0 9f e8 4e 94 40 ff eb 62 be 06 00 00 00 e8 f9 ff 00 00
48 85 c0 75 0e 48
c7 c7 40 16 a0 9f e8 31 94 40 ff eb 45 <66> 44 8b 20 be 06 00 00 00 48 89 c7 8b
58 02 e8 87 00
01 00 66
RIP: efi_bgrt_init+0xdc/0x134 RSP: 9fc03d58
CR2: ff240001
---[ end trace f68728a0d3053b52 ]---
Kernel panic - not syncing: Attempted to kill the idle task!
---[ end Kernel panic - not syncing: Attempted to kill the idle task!
Fixes: 7b0a911478c7 ("efi/x86: Move the EFI BGRT init code to early init code")
Signed-off-by: Dave Young
Tested-by: Sabrina Dubroca
Cc: # v4.11+
Signed-off-by: Ard Biesheuvel
Signed-off-by: Matt Fleming
Signed-off-by: Matt Fleming
---
drivers/firmware/efi/efi-bgrt.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
index 04ca8764f0c0..8bf27323f7a3 100644
--- a/drivers/firmware/efi/efi-bgrt.c
+++ b/drivers/firmware/efi/efi-bgrt.c
@@ -36,6 +36,9 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
if (acpi_disabled)
return;
+ if (!efi_enabled(EFI_BOOT))
+ return;
+
if (table->length < sizeof(bgrt_tab)) {
pr_notice("Ignoring BGRT: invalid length %u (expected %zu)\n",
table->length, sizeof(bgrt_tab));
--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] x86/efi: Correct ident mapping of efi old_map when kalsr enabled
From: Baoquan He
For EFI with 'efi=old_map' kernel option specified, Kernel will panic
when kaslr is enabled.
The back trace is:
BUG: unable to handle kernel paging request at 7febd57e
IP: 0x7febd57e
PGD 1025a067
PUD 0
Oops: 0010 [#1] SMP
[ ... ]
Call Trace:
? efi_call+0x58/0x90
? printk+0x58/0x6f
efi_enter_virtual_mode+0x3c5/0x50d
start_kernel+0x40f/0x4b8
? set_init_arg+0x55/0x55
? early_idt_handler_array+0x120/0x120
x86_64_start_reservations+0x24/0x26
x86_64_start_kernel+0x14c/0x16f
start_cpu+0x14/0x14
The root cause is the ident mapping is not built correctly in old_map case.
For nokaslr kernel, PAGE_OFFSET is 0x8800 which is PGDIR_SIZE
aligned. We can borrow the pud table from direct mapping safely. Given a
physical address X, we have pud_index(X) == pud_index(__va(X)). However,
for kaslr kernel, PAGE_OFFSET is PUD_SIZE aligned. For a given physical
address X, pud_index(X) != pud_index(__va(X)). We can't only copy pgd entry
from direct mapping to build ident mapping, instead need copy pud entry
one by one from direct mapping.
Fix it.
Signed-off-by: Baoquan He
Cc: Dave Young
Cc: Ard Biesheuvel
Cc: Thomas Gleixner
Cc: Ingo Molnar
Cc: "H. Peter Anvin"
Cc: Thomas Garnier
Cc: Kees Cook
Cc: Russ Anderson
Cc: Frank Ramsay
Cc: Borislav Petkov
Cc: Bhupesh Sharma
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/efi_64.c | 79 +-
1 file changed, 71 insertions(+), 8 deletions(-)
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index c488625c9712..548728949324 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -71,11 +71,13 @@ static void __init early_code_mapping_set_exec(int
executable)
pgd_t * __init efi_call_phys_prolog(void)
{
- unsigned long vaddress;
- pgd_t *save_pgd;
+ unsigned long vaddr, addr_pgd, addr_p4d, addr_pud;
+ pgd_t *save_pgd, *pgd_k, *pgd_efi;
+ p4d_t *p4d, *p4d_k, *p4d_efi;
+ pud_t *pud;
int pgd;
- int n_pgds;
+ int n_pgds, i, j;
if (!efi_enabled(EFI_OLD_MEMMAP)) {
save_pgd = (pgd_t *)read_cr3();
@@ -88,10 +90,49 @@ pgd_t * __init efi_call_phys_prolog(void)
n_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT), PGDIR_SIZE);
save_pgd = kmalloc_array(n_pgds, sizeof(*save_pgd), GFP_KERNEL);
+ /*
+* Build 1:1 ident mapping for old_map usage. It needs to be noticed
+* that PAGE_OFFSET is PGDIR_SIZE aligned with KASLR disabled, while
+* PUD_SIZE ALIGNED with KASLR enabled. So for a given physical
+* address X, the pud_index(X) != pud_index(__va(X)), we can only copy
+* pud entry of __va(X) to fill in pud entry of X to build 1:1 mapping
+* . Means here we can only reuse pmd table of direct mapping.
+*/
for (pgd = 0; pgd < n_pgds; pgd++) {
- save_pgd[pgd] = *pgd_offset_k(pgd * PGDIR_SIZE);
- vaddress = (unsigned long)__va(pgd * PGDIR_SIZE);
- set_pgd(pgd_offset_k(pgd * PGDIR_SIZE),
*pgd_offset_k(vaddress));
+ addr_pgd = (unsigned long)(pgd * PGDIR_SIZE);
+ vaddr = (unsigned long)__va(pgd * PGDIR_SIZE);
+ pgd_efi = pgd_offset_k(addr_pgd);
+ save_pgd[pgd] = *pgd_efi;
+
+ p4d = p4d_alloc(&init_mm, pgd_efi, addr_pgd);
+ if (!p4d) {
+ pr_err("Failed to allocate p4d table!\n");
+ goto out;
+ }
+
+ for (i = 0; i < PTRS_PER_P4D; i++) {
+ addr_p4d = addr_pgd + i * P4D_SIZE;
+ p4d_efi = p4d + p4d_index(addr_p4d);
+
+ pud = pud_alloc(&init_mm, p4d_efi, addr_p4d);
+ if (!pud) {
+ pr_err("Failed to allocate pud table!\n");
+ goto out;
+ }
+
+ for (j = 0; j < PTRS_PER_PUD; j++) {
+ addr_pud = addr_p4d + j * PUD_SIZE;
+
+ if (addr_pud > (max_pfn << PAGE_SHIFT))
+ break;
+
+ vaddr = (unsigned long)__va(addr_pud);
+
+ pgd_k = pgd_offset_k(vaddr);
+ p4d_k = p4d_offset(pgd_k, vaddr);
+ pud[j] = *pud_offset(p4d_k, vaddr);
+ }
+ }
}
out:
__flush_tlb_all();
@@ -104,8 +145,11 @@ void __init efi_call_phys_epilog(pgd_t *save_pgd)
/*
* After the lock is released, the original page table is restored.
*/
- int pgd_idx;
+ int pgd_idx, i;
int nr_pgds;
+ pgd_t *pgd;
+ p4d_t *p4d;
+ pud_t *pud;
if (!efi
[PATCH 2/5] efi: Remove duplicate 'const' specifiers
From: Arnd Bergmann
gcc-7 shows a harmless warning:
drivers/firmware/efi/libstub/secureboot.c:19:27: error: duplicate 'const'
declaration specifier [-Werror=duplicate-decl-specifier]
static const efi_char16_t const efi_SecureBoot_name[] = {
drivers/firmware/efi/libstub/secureboot.c:22:27: error: duplicate 'const'
declaration specifier [-Werror=duplicate-decl-specifier]
Removing one of the specifiers gives us the expected behavior.
Fixes: de8cb458625c ("efi: Get and store the secure boot status")
Signed-off-by: Arnd Bergmann
Acked-by: Ard Biesheuvel
Reviewed-by: David Howells
Cc: Josh Boyer
Cc: Ingo Molnar
Signed-off-by: Matt Fleming
---
drivers/firmware/efi/libstub/secureboot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 8c34d50a4d80..959777ec8a77 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -16,10 +16,10 @@
/* BIOS variables */
static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-static const efi_char16_t const efi_SecureBoot_name[] = {
+static const efi_char16_t efi_SecureBoot_name[] = {
'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
};
-static const efi_char16_t const efi_SetupMode_name[] = {
+static const efi_char16_t efi_SetupMode_name[] = {
'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
};
--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/5] x86/efi: Disable runtime services on kexec kernel if booted with efi=old_map
From: Sai Praneeth
Booting kexec kernel with "efi=old_map" in kernel command line hits
kernel panic as shown below.
BUG: unable to handle kernel paging request at 88007fe78070
IP: virt_efi_set_variable.part.7+0x63/0x1b0
PGD 7ea28067
PUD 7ea2b067
PMD 7ea2d067
PTE 0
[...]
Call Trace:
virt_efi_set_variable+0x5d/0x70
efi_delete_dummy_variable+0x7a/0x80
efi_enter_virtual_mode+0x3f6/0x4a7
start_kernel+0x375/0x400
x86_64_start_reservations+0x2a/0x2c
x86_64_start_kernel+0x168/0x176
start_cpu+0x14/0x14
[ efi=old_map was never intended to work with kexec. The problem with
using efi=old_map is that the virtual addresses are assigned from the
memory region used by other kernel mappings; vmalloc() space.
Potentially there could be collisions when booting kexec if something
else is mapped at the virtual address we allocated for runtime service
regions in the initial boot - Matt Fleming ]
Since kexec was never intended to work with efi=old_map, disable
runtime services in kexec if booted with efi=old_map, so that we don't
panic.
Signed-off-by: Sai Praneeth Prakhya
Cc: Borislav Petkov
Cc: Ricardo Neri
Cc: Ard Biesheuvel
Cc: Ravi Shankar
Tested-by: Lee Chun-Yi
Acked-by: Dave Young
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/efi.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 7e76a4d8304b..43b96f5f78ba 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -828,9 +828,11 @@ static void __init kexec_enter_virtual_mode(void)
/*
* We don't do virtual mode, since we don't do runtime services, on
-* non-native EFI
+* non-native EFI. With efi=old_map, we don't do runtime services in
+* kexec kernel because in the initial boot something else might
+* have been mapped at these virtual addresses.
*/
- if (!efi_is_native()) {
+ if (!efi_is_native() || efi_enabled(EFI_OLD_MEMMAP)) {
efi_memmap_unmap();
clear_bit(EFI_RUNTIME_SERVICES, &efi.flags);
return;
--
2.12.2
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL 0/5] EFI urgent fixes
Hi folks, Please pull the following fixes. There are patches that resolve a few boot crashes and some minor build and boot log cleanups. The following changes since commit 08332893e37af6ae779367e78e444f8f9571511d: Linux 4.12-rc2 (2017-05-21 19:30:23 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-urgent for you to fetch changes up to 5d36982a80248b2dcce395e7fa4ba342e814ced1: efi/bgrt: Skip efi_bgrt_init in case of non-efi boot (2017-05-26 11:27:38 +0100) - Make the boot console quiet when using Xen on EFI by deleting a pointless error message - Juergen Gross - Silence harmless warnings emitted with GCC 7 - Arnd Bergmann - Prevent a crash when booting kexec with the efi=old_map kernel command line parameter by disabling EFI runtime services - Sai Praneeth - Fix boot crash when using kaslr and efi=old_map. The crash is caused because of assumptions about PAGE_OFFSET alignment which are not true with kaslr enabled - Baoquan He - Fix boot regression when a machine has an ACPI BGRT table and is booted using BIOS, not EFI. - Dave Young Arnd Bergmann (1): efi: Remove duplicate 'const' specifiers Baoquan He (1): x86/efi: Correct ident mapping of efi old_map when kalsr enabled Dave Young (1): efi/bgrt: Skip efi_bgrt_init in case of non-efi boot Juergen Gross (1): efi: Don't issue error message when booted under xen Sai Praneeth (1): x86/efi: Disable runtime services on kexec kernel if booted with efi=old_map arch/x86/platform/efi/efi.c | 6 ++- arch/x86/platform/efi/efi_64.c| 79 +++ arch/x86/platform/efi/quirks.c| 3 ++ drivers/firmware/efi/efi-bgrt.c | 3 ++ drivers/firmware/efi/libstub/secureboot.c | 4 +- 5 files changed, 83 insertions(+), 12 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] x86/efi: Correct ident mapping of efi old_map when kalsr enabled
On Thu, 18 May, at 02:39:30PM, Baoquan He wrote: > For EFI with 'efi=old_map' kernel option specified, Kernel will panic > when kaslr is enabled. > > The back trace is: > > BUG: unable to handle kernel paging request at 7febd57e > IP: 0x7febd57e > PGD 1025a067 > PUD 0 > > Oops: 0010 [#1] SMP > [ ... ] > Call Trace: > ? efi_call+0x58/0x90 > ? printk+0x58/0x6f > efi_enter_virtual_mode+0x3c5/0x50d > start_kernel+0x40f/0x4b8 > ? set_init_arg+0x55/0x55 > ? early_idt_handler_array+0x120/0x120 > x86_64_start_reservations+0x24/0x26 > x86_64_start_kernel+0x14c/0x16f > start_cpu+0x14/0x14 > > The root cause is the ident mapping is not built correctly in old_map case. > > For nokaslr kernel, PAGE_OFFSET is 0x8800 which is PGDIR_SIZE > aligned. We can borrow the pud table from direct mapping safely. Given a > physical address X, we have pud_index(X) == pud_index(__va(X)). However, > for kaslr kernel, PAGE_OFFSET is PUD_SIZE aligned. For a given physical > address X, pud_index(X) != pud_index(__va(X)). We can't only copy pgd entry > from direct mapping to build ident mapping, instead need copy pud entry > one by one from direct mapping. > > Fix it. > > Signed-off-by: Baoquan He > Signed-off-by: Dave Young > Cc: Matt Fleming > Cc: Ard Biesheuvel > Cc: Thomas Gleixner > Cc: Ingo Molnar > Cc: "H. Peter Anvin" > Cc: Thomas Garnier > Cc: Kees Cook > Cc: Russ Anderson > Cc: Frank Ramsay > Cc: Borislav Petkov > Cc: Bhupesh Sharma > Cc: x...@kernel.org > Cc: linux-efi@vger.kernel.org > --- > v3->v4: > 1. Forget running scripts/checkpatch.pl to check patch, there are several > code stype issue. Correct them in this version. > > v2->v3: > 1. Rewrite code to copy pud entry one by one so that code can be > understood > better. Usually we only have less than 1TB or several TB memory, pud entry > copy one by one won't impact efficiency. > > 2. Adding p4d page table handling. > > v1->v2: > Change code and add description according to Thomas's suggestion as below: > > 1. Add checking if pud table is allocated successfully. If not just break > the for loop. > > 2. Add code comment to explain how the 1:1 mapping is built in > efi_call_phys_prolog > > 3. Other minor change > > arch/x86/platform/efi/efi_64.c | 70 > +- > 1 file changed, 62 insertions(+), 8 deletions(-) Thanks, applied. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] x86/efi: Disable runtime services on kexec kernel if booted with efi=old_map
On Tue, 16 May, at 06:14:23PM, Sai Praneeth Prakhya wrote: > From: Sai Praneeth > > Booting kexec kernel with "efi=old_map" in kernel command line hits > kernel panic as shown below. > > [0.001000] BUG: unable to handle kernel paging request at 88007fe78070 > [0.001000] IP: virt_efi_set_variable.part.7+0x63/0x1b0 > [0.001000] PGD 7ea28067 > [0.001000] PUD 7ea2b067 > [0.001000] PMD 7ea2d067 > [0.001000] PTE 0 > [0.001000] > [0.001000] Oops: [#1] SMP > [0.001000] Modules linked in: > [0.001000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted > 4.11.0-rc2-yocto-standard+ #229 > [0.001000] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 > 02/06/2015 > [0.001000] task: 82022500 task.stack: 8200 > [0.001000] RIP: 0010:virt_efi_set_variable.part.7+0x63/0x1b0 > [0.001000] RSP: :82003dc0 EFLAGS: 00010246 > [0.001000] RAX: 88007fe78018 RBX: 82050300 RCX: 0007 > [0.001000] RDX: 82003e50 RSI: 82050300 RDI: 82050300 > [0.001000] RBP: 82003e08 R08: R09: > [0.001000] R10: R11: R12: 82003e50 > [0.001000] R13: 0007 R14: 0246 R15: > [0.001000] FS: () GS:88007fa0() > knlGS: > [0.001000] CS: 0010 DS: ES: CR0: 80050033 > [0.001000] CR2: 88007fe78070 CR3: 7da1d000 CR4: 06b0 > [0.001000] DR0: DR1: DR2: > [0.001000] DR3: DR6: fffe0ff0 DR7: 0400 > [0.001000] Call Trace: > [0.001000] virt_efi_set_variable+0x5d/0x70 > [0.001000] efi_delete_dummy_variable+0x7a/0x80 > [0.001000] efi_enter_virtual_mode+0x3f6/0x4a7 > [0.001000] start_kernel+0x375/0x400 > [0.001000] x86_64_start_reservations+0x2a/0x2c > [0.001000] x86_64_start_kernel+0x168/0x176 > [0.001000] start_cpu+0x14/0x14 > [0.001000] Code: 04 b0 84 ff 80 3d c5 56 b3 00 00 4c 8b 44 24 08 75 > 6b 9c 41 5e 48 8b 05 9c 78 99 00 4d 89 c1 48 89 de 4d 89 f8 44 89 e9 4c > 89 e2 <48> 8b 40 58 48 8b 78 58 e8 b0 2d 88 ff 48 c7 c6 b6 1d f4 81 4c > [0.001000] RIP: virt_efi_set_variable.part.7+0x63/0x1b0 RSP: 82003dc0 > [0.001000] CR2: 88007fe78070 > [0.001000] ---[ end trace ]--- > [0.001000] Kernel panic - not syncing: Attempted to kill the idle task! > [0.001000] ---[ end Kernel panic - not syncing: Attempted to kill the idle > task! > > [efi=old_map was never intended to work with kexec. The problem with > using efi=old_map is that the virtual addresses are assigned from the > memory region used by other kernel mappings; vmalloc() space. > Potentially there could be collisions when booting kexec if something > else is mapped at the virtual address we allocated for runtime service > regions in the initial boot] - Matt Fleming > > Since kexec was never intended to work with efi=old_map, disable > runtime services in kexec if booted with efi=old_map, so that we don't > panic. > > Signed-off-by: Sai Praneeth Prakhya > Cc: Borislav Petkov > Cc: Ricardo Neri > Cc: Matt Fleming > Cc: Ard Biesheuvel > Cc: Ravi Shankar > Cc: Lee Chun-Yi > Cc: Dave Young > > Changes since v1: > Don't fix the panic, because this was never intended to work. > --- > arch/x86/platform/efi/efi.c | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) Thanks Sai, applied to the urgent branch. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.11 00/28] 4.11.1-stable review
On Mon, 15 May, at 11:28:16AM, Shuah Khan wrote:
> Hi Matt,
>
> On 05/15/2017 08:36 AM, Matt Fleming wrote:
> > On Fri, 12 May, at 10:01:41AM, Shuah Khan wrote:
> >>
> >> Greg/Matt,
> >>
> >> I started seeing this maybe since 4.11, so it isn't really a 4.11.1
> >> issue, however sounds like this shouldn't be an error message, since
> >> it is showing up on an older platform.
> >>
> >> efi: EFI_MEMMAP is not enabled.
> >> efi: Failed to lookup EFI memory descriptor for 0xd9e0f018
> >>
> >>
> >> From 816e76129ed5fadd28e526c43397c79775194b5c Mon Sep 17 00:00:00 2001
> >> From: Matt Fleming
> >> Date: Mon, 29 Feb 2016 21:22:52 +
> >> Subject: [PATCH] efi: Allow drivers to reserve boot services forever
> >>
> >> This change was introduced in Commit:
> >> 816e76129ed5fadd28e526c43397c79775194b5c
> >>
> >> Matt!
> >>
> >> Shouldn't this be a debug message?? Is this a really an error??
> >
> > Yes, it's most likely an error. We shouldn't be trying to reserve
> > EFI regions if we can't find them in the memory map.
> >
> > Can you provide the full dmesg, please?
> >
>
> Please see attached dmesg and dmidecode output. Please let me know if you need
> any other information.
Did you use git bisect to find the above commit that introduced this
error message? Because while the error message itself was merged in
the above commit there were no users until commit 8e80632fb23f
("efi/esrt: Use efi_mem_reserve() and avoid a kmalloc()").
Can you apply the attached patch, boot your machine with the efi=debug
command line option and capture the dmesg output? It doesn't look like
your machine is booting using EFI.
---
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 30031d5293c4..7f942df48f42 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -193,6 +193,7 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64 size)
if (efi_mem_desc_lookup(addr, &md)) {
pr_err("Failed to lookup EFI memory descriptor for %pa\n",
&addr);
+ WARN_ON(1);
return;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/reboot: Fall back to original power-off method if EFI_RESET_SHUTDOWN returns
On Fri, 19 May, at 04:45:49PM, Ard Biesheuvel wrote: > > This does not look unreasonable to me, but this is more Matt's turf so > I will let him handle this one. I was hoping that either Len or Rafael would have chimed in by now, but they were probably waiting for me... Doesn't ACPI reduced require that EFI power off be supported? I can't find anything in the spec that makes that connection. Unless the ACPI folks provide a reason that falling back to ACPI poweroff doesn't make sense for ACPI reduced hardware, I think we should apply this patch. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 17/32] x86/mm: Add support to access boot related data in the clear
On Mon, 15 May, at 08:35:17PM, Borislav Petkov wrote: > On Tue, Apr 18, 2017 at 04:19:21PM -0500, Tom Lendacky wrote: > > > + paddr = boot_params.efi_info.efi_memmap_hi; > > + paddr <<= 32; > > + paddr |= boot_params.efi_info.efi_memmap; > > + if (phys_addr == paddr) > > + return true; > > + > > + paddr = boot_params.efi_info.efi_systab_hi; > > + paddr <<= 32; > > + paddr |= boot_params.efi_info.efi_systab; > > So those two above look like could be two global vars which are > initialized somewhere in the EFI init path: > > efi_memmap_phys and efi_systab_phys or so. > > Matt ? > > And then you won't need to create that paddr each time on the fly. I > mean, it's not a lot of instructions but still... We should already have the physical memmap address available in 'efi.memmap.phys_map'. And the physical address of the system table should be in 'efi_phys.systab'. See efi_init(). -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4.11 00/28] 4.11.1-stable review
On Fri, 12 May, at 10:01:41AM, Shuah Khan wrote: > > Greg/Matt, > > I started seeing this maybe since 4.11, so it isn't really a 4.11.1 > issue, however sounds like this shouldn't be an error message, since > it is showing up on an older platform. > > efi: EFI_MEMMAP is not enabled. > efi: Failed to lookup EFI memory descriptor for 0xd9e0f018 > > > From 816e76129ed5fadd28e526c43397c79775194b5c Mon Sep 17 00:00:00 2001 > From: Matt Fleming > Date: Mon, 29 Feb 2016 21:22:52 + > Subject: [PATCH] efi: Allow drivers to reserve boot services forever > > This change was introduced in Commit: 816e76129ed5fadd28e526c43397c79775194b5c > > Matt! > > Shouldn't this be a debug message?? Is this a really an error?? Yes, it's most likely an error. We shouldn't be trying to reserve EFI regions if we can't find them in the memory map. Can you provide the full dmesg, please? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] x86/efi: Add EFI_PGT_DUMP support for x86_32, kexec
On Thu, 11 May, at 12:20:33PM, Sai Praneeth Prakhya wrote: > From: Sai Praneeth > > EFI_PGT_DUMP, as the name suggests dumps efi page tables to dmesg during > kernel boot. This feature is very useful while debugging page > faults/null pointer dereferences to efi related addresses. Presently, > this feature is limited only to x86_64, so let's extend it to other efi > configurations like kexec kernel, efi=old_map and to x86_32 as well. > This doesn't effect normal boot path because this config option should > be used only for debug purposes. > > Signed-off-by: Sai Praneeth Prakhya > Cc: Borislav Petkov > Cc: Ricardo Neri > Cc: Matt Fleming > Cc: Ard Biesheuvel > Cc: Ravi Shankar > > Changes since v1: > 1. Call efi_dump_pagetable() only once from efi_enter_virtual_mode() - > as suggested by Boris > > --- > arch/x86/platform/efi/efi.c| 3 ++- > arch/x86/platform/efi/efi_32.c | 9 - > arch/x86/platform/efi/efi_64.c | 5 - > 3 files changed, 14 insertions(+), 3 deletions(-) Thanks Sai, applied. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi: remove duplicate 'const' specifiers
On Thu, 11 May, at 01:43:17PM, Arnd Bergmann wrote:
> gcc-7 shows a harmless warning:
>
> drivers/firmware/efi/libstub/secureboot.c:19:27: error: duplicate 'const'
> declaration specifier [-Werror=duplicate-decl-specifier]
> static const efi_char16_t const efi_SecureBoot_name[] = {
> drivers/firmware/efi/libstub/secureboot.c:22:27: error: duplicate 'const'
> declaration specifier [-Werror=duplicate-decl-specifier]
>
> Removing one of the specifiers gives us the expected behavior.
>
> Fixes: de8cb458625c ("efi: Get and store the secure boot status")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/firmware/efi/libstub/secureboot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Thanks, applied to the urgent queue.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86/efi: Fix kexec kernel panic when efi=old_map is enabled
(Pulling in Dave, Mr. Kexec on EFI)
On Mon, 08 May, at 12:25:23PM, Sai Praneeth Prakhya wrote:
> From: Sai Praneeth
>
> Booting kexec kernel with "efi=old_map" in kernel command line hits
> kernel panic as shown below.
>
> [0.001000] BUG: unable to handle kernel paging request at 88007fe78070
> [0.001000] IP: virt_efi_set_variable.part.7+0x63/0x1b0
> [0.001000] PGD 7ea28067
> [0.001000] PUD 7ea2b067
> [0.001000] PMD 7ea2d067
> [0.001000] PTE 0
> [0.001000]
> [0.001000] Oops: [#1] SMP
> [0.001000] Modules linked in:
> [0.001000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
> 4.11.0-rc2-yocto-standard+ #229
> [0.001000] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 0.0.0 02/06/2015
> [0.001000] task: 82022500 task.stack: 8200
> [0.001000] RIP: 0010:virt_efi_set_variable.part.7+0x63/0x1b0
> [0.001000] RSP: :82003dc0 EFLAGS: 00010246
> [0.001000] RAX: 88007fe78018 RBX: 82050300 RCX:
> 0007
> [0.001000] RDX: 82003e50 RSI: 82050300 RDI:
> 82050300
> [0.001000] RBP: 82003e08 R08: R09:
>
> [0.001000] R10: R11: R12:
> 82003e50
> [0.001000] R13: 0007 R14: 0246 R15:
>
> [0.001000] FS: () GS:88007fa0()
> knlGS:
> [0.001000] CS: 0010 DS: ES: CR0: 80050033
> [0.001000] CR2: 88007fe78070 CR3: 7da1d000 CR4:
> 06b0
> [0.001000] DR0: DR1: DR2:
>
> [0.001000] DR3: DR6: fffe0ff0 DR7:
> 0400
> [0.001000] Call Trace:
> [0.001000] virt_efi_set_variable+0x5d/0x70
> [0.001000] efi_delete_dummy_variable+0x7a/0x80
> [0.001000] efi_enter_virtual_mode+0x3f6/0x4a7
> [0.001000] start_kernel+0x375/0x400
> [0.001000] x86_64_start_reservations+0x2a/0x2c
> [0.001000] x86_64_start_kernel+0x168/0x176
> [0.001000] start_cpu+0x14/0x14
> [0.001000] Code: 04 b0 84 ff 80 3d c5 56 b3 00 00 4c 8b 44 24 08 75
> 6b 9c 41 5e 48 8b 05 9c 78 99 00 4d 89 c1 48 89 de 4d 89 f8 44 89 e9 4c
> 89 e2 <48> 8b 40 58 48 8b 78 58 e8 b0 2d 88 ff 48 c7 c6 b6 1d f4 81 4c
> [0.001000] RIP: virt_efi_set_variable.part.7+0x63/0x1b0 RSP:
> 82003dc0
> [0.001000] CR2: 88007fe78070
> [0.001000] ---[ end trace ]---
> [0.001000] Kernel panic - not syncing: Attempted to kill the idle task!
> [0.001000] ---[ end Kernel panic - not syncing: Attempted to kill the
> idle task!
>
> This happens because efi=old_map doesn't use efi_pgd but rather it uses
> kernel's pgd. We don't hit the same panic in a regular kernel because
> it uses old_map_region() and not __map_region().
>
> Signed-off-by: Sai Praneeth Prakhya
> Cc: Borislav Petkov
> Cc: Ricardo Neri
> Cc: Matt Fleming
> Cc: Ard Biesheuvel
> Cc: Ravi Shankar
> ---
> arch/x86/platform/efi/efi_64.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 4e043a8c8556..76e1cd6b74dd 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -320,6 +320,9 @@ static void __init __map_region(efi_memory_desc_t *md,
> u64 va)
> unsigned long pfn;
> pgd_t *pgd = efi_pgd;
>
> + if (efi_enabled(EFI_OLD_MEMMAP))
> + pgd = swapper_pg_dir;
> +
> if (!(md->attribute & EFI_MEMORY_WB))
> flags |= _PAGE_PCD;
>
The thing is, efi=old_map was never intended to work with kexec.
Part of the reason for introducing the new EFI runtime services
mapping scheme was so that we could kexec on EFI. See commit
d2f7cbe7b26a ("x86/efi: Runtime services virtual mapping").
The problem with using efi=old_map is that the virtual addresses are
assigned from the memory region used by other kernel mappings;
vmalloc() space.
Potentially there could be collisions when booting kexec if something
else is mapped at the virtual address we allocated for runtime service
regions in the initial boot.
So, while this patch may work for you and Joey, I don't think it's
reliable.
Dave, did I miss anything?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi: don't issue error message when booted under xen
On Mon, 08 May, at 02:56:33PM, Juergen Gross wrote: > When booted as Xen dom0 there won't be an EFI memmap allocated. Avoid > issuing an error message in this case: > > [0.144079] efi: Failed to allocate new EFI memmap > > Signed-off-by: Juergen Gross > --- > arch/x86/platform/efi/quirks.c | 3 +++ > 1 file changed, 3 insertions(+) Thanks. I've applied this for the urgent branch because it's trivial and we've had issues before where these kinds of error messages mess up an otherwise clean boot screen. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] efi/efi_test: use memdup_user
On Mon, 08 May, at 04:18:30PM, Ivan Hu wrote: > > > On 05/06/2017 04:53 AM, Matt Fleming wrote: > >On Sat, 29 Apr, at 09:42:52AM, Geliang Tang wrote: > >>Use memdup_user() helper instead of open-coding to simplify the code. > >> > >>Signed-off-by: Geliang Tang > >>--- > >> drivers/firmware/efi/test/efi_test.c | 11 +++ > >> 1 file changed, 3 insertions(+), 8 deletions(-) > > > >This one looks fine. Ivan, do you want to ACK it? > > > > Looks fine to me too, ACK. Thanks! Applied, thanks everyone. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1 linux-next] efi/capsule: kmap() can't fail
On Tue, 25 Apr, at 08:10:51PM, Fabian Frederick wrote:
> Remove NULL test on kmap()
>
> Signed-off-by: Fabian Frederick
> ---
> drivers/firmware/efi/capsule-loader.c | 5 -
> drivers/firmware/efi/capsule.c| 4
> 2 files changed, 9 deletions(-)
>
> diff --git a/drivers/firmware/efi/capsule-loader.c
> b/drivers/firmware/efi/capsule-loader.c
> index 9ae6c11..bf8b2ef 100644
> --- a/drivers/firmware/efi/capsule-loader.c
> +++ b/drivers/firmware/efi/capsule-loader.c
> @@ -183,11 +183,6 @@ static ssize_t efi_capsule_write(struct file *file,
> const char __user *buff,
> page = cap_info->pages[cap_info->index - 1];
>
> kbuff = kmap(page);
> - if (!kbuff) {
> - pr_debug("%s: kmap() failed\n", __func__);
> - ret = -EFAULT;
> - goto failed;
> - }
> kbuff += PAGE_SIZE - cap_info->page_bytes_remain;
>
> /* Copy capsule binary data from user space to kernel space buffer */
> diff --git a/drivers/firmware/efi/capsule.c b/drivers/firmware/efi/capsule.c
> index 6eedff4..e603ccf 100644
> --- a/drivers/firmware/efi/capsule.c
> +++ b/drivers/firmware/efi/capsule.c
> @@ -247,10 +247,6 @@ int efi_capsule_update(efi_capsule_header_t *capsule,
> struct page **pages)
> efi_capsule_block_desc_t *sglist;
>
> sglist = kmap(sg_pages[i]);
> - if (!sglist) {
> - rv = -ENOMEM;
> - goto out;
> - }
>
> for (j = 0; j < SGLIST_PER_PAGE && count > 0; j++) {
> u64 sz = min_t(u64, imagesize, PAGE_SIZE);
Huh, well I didn't know that.
This looks OK to me. Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] rtc: rtc-efi: Add an enable parameter and make it false for X86 by default
On Tue, 02 May, at 08:51:47AM, Ocean HY1 He wrote:
> The commit 7efe665903d0 ("rtc: Disable EFI rtc for x86") turns off rtc-efi
> option completely for x86 in rtc/Kconfig, to avoid possible crash caused by
> buggy implementations of the time-related EFI runtime services.
>
> In fact, there are more and more UEFI firmware has time-related EFI runtime
> services well done. To meet EFI rtc request for X86, here adds an enable
> parameter which could be explicitly set as true when load rtc-efi module.
> To keep consistency, make this enable parameter false for X86 by default.
>
> The test passes on Lenovo ThinkServer RD350 while the UEFI/BIOS version is
> VB3TS424 and processor is Intel(R) Xeon(R) CPU E5-2660 v3.
> #modprobe rtc_efi enable=1
> #cat /sys/class/rtc/rtc1/name
> rtc-efi
> #hwclock --rtc=/dev/rtc1 -w
> #hwclock --rtc=/dev/rtc1 -r
>
> Signed-off-by: Ocean He
> ---
> drivers/rtc/Kconfig | 2 +-
> drivers/rtc/rtc-efi.c | 11 +++
> 2 files changed, 12 insertions(+), 1 deletion(-)
The key piece of info missing from this changelog is: why?
Why does it make sense to allow this driver to be enabled for x86? All
the RTC functions should be available via other, better features on
x86.
There's no justification for enabling it just because it works on some
platforms, you need to show it is *essential* somehow.
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index ee1b0e9..482200a 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1037,7 +1037,7 @@ config RTC_DRV_DA9063
>
> config RTC_DRV_EFI
> tristate "EFI RTC"
> - depends on EFI && !X86
> + depends on EFI
> help
> If you say yes here you will get support for the EFI
> Real Time Clock.
> diff --git a/drivers/rtc/rtc-efi.c b/drivers/rtc/rtc-efi.c
> index 0130afd..5a8427f 100644
> --- a/drivers/rtc/rtc-efi.c
> +++ b/drivers/rtc/rtc-efi.c
> @@ -23,6 +23,14 @@
> #include
> #include
>
> +/*
> + * Disable the use of rtc-efi as a RTC for X86 by default. This setting can
> be
> + * overridden using this module's enable parameter.
> + */
> +static bool efi_rtc_enable = !IS_ENABLED(CONFIG_X86);
> +
> +module_param_named(enable, efi_rtc_enable, bool, 0644);
> +
> #define EFI_ISDST (EFI_TIME_ADJUST_DAYLIGHT|EFI_TIME_IN_DAYLIGHT)
>
> /*
> @@ -262,6 +270,9 @@ static int __init efi_rtc_probe(struct platform_device
> *dev)
> efi_time_t eft;
> efi_time_cap_t cap;
>
> + if (!efi_rtc_enable)
> + return -ENODEV;
> +
> /* First check if the RTC is usable */
> if (efi.get_time(&eft, &cap) != EFI_SUCCESS)
> return -ENODEV;
> --
> 1.8.3.1
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] efi/efi_test: use memdup_user
On Sat, 29 Apr, at 09:42:52AM, Geliang Tang wrote: > Use memdup_user() helper instead of open-coding to simplify the code. > > Signed-off-by: Geliang Tang > --- > drivers/firmware/efi/test/efi_test.c | 11 +++ > 1 file changed, 3 insertions(+), 8 deletions(-) This one looks fine. Ivan, do you want to ACK it? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] x86/efi: Correct ident mapping of efi old_map when kalsr enabled
(Including the folks from SGI since this was hit on a UV system)
On Thu, 27 Apr, at 08:07:03PM, Baoquan He wrote:
> For EFI with old_map enabled, Kernel will panic when kaslr is enabled.
>
> The root cause is the ident mapping is not built correctly in this case.
>
> For nokaslr kernel, PAGE_OFFSET is 0x8800 which is PGDIR_SIZE
> aligned. We can borrow the pud table from direct mapping safely. Given a
> physical address X, we have pud_index(X) == pud_index(__va(X)). However,
> for kaslr kernel, PAGE_OFFSET is PUD_SIZE aligned. For a given physical
> address X, pud_index(X) != pud_index(__va(X)). We can't only copy pgd entry
> from direct mapping to build ident mapping, instead need copy pud entry
> one by one from direct mapping.
>
> So fix it in this patch.
>
> The panic message is like below, an emty PUD or a wrong PUD.
>
> [0.233007] BUG: unable to handle kernel paging request at 7febd57e
> [0.233899] IP: 0x7febd57e
> [0.234000] PGD 1025a067
> [0.234000] PUD 0
> [0.234000]
> [0.234000] Oops: 0010 [#1] SMP
> [0.234000] Modules linked in:
> [0.234000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.11.0-rc8+ #125
> [0.234000] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 0.0.0 02/06/2015
> [0.234000] task: afe104c0 task.stack: afe0
> [0.234000] RIP: 0010:0x7febd57e
> [0.234000] RSP: :afe03d98 EFLAGS: 00010086
> [0.234000] RAX: 8c9e3fff9540 RBX: 7c4b6000 RCX:
> 0480
> [0.234000] RDX: 0030 RSI: 0480 RDI:
> 7febd57e
> [0.234000] RBP: afe03e40 R08: 0001 R09:
> 7c4b6000
> [0.234000] R10: afa71a40 R11: 20786c6c2478303d R12:
> 0030
> [0.234000] R13: 0246 R14: 8c9e3c4198d8 R15:
> 0480
> [0.234000] FS: () GS:8c9e3fa0()
> knlGS:
> [0.234000] CS: 0010 DS: ES: CR0: 80050033
> [0.234000] CR2: 7febd57e CR3: 0fe09000 CR4:
> 000406b0
> [0.234000] DR0: DR1: DR2:
>
> [0.234000] DR3: DR6: fffe0ff0 DR7:
> 0400
> [0.234000] Call Trace:
> [0.234000] ? efi_call+0x58/0x90
> [0.234000] ? printk+0x58/0x6f
> [0.234000] efi_enter_virtual_mode+0x3c5/0x50d
> [0.234000] start_kernel+0x40f/0x4b8
> [0.234000] ? set_init_arg+0x55/0x55
> [0.234000] ? early_idt_handler_array+0x120/0x120
> [0.234000] x86_64_start_reservations+0x24/0x26
> [0.234000] x86_64_start_kernel+0x14c/0x16f
> [0.234000] start_cpu+0x14/0x14
> [0.234000] Code: Bad RIP value.
> [0.234000] RIP: 0x7febd57e RSP: afe03d98
> [0.234000] CR2: 7febd57e
> [0.234000] ---[ end trace d4ded46ab8ab8ba9 ]---
> [0.234000] Kernel panic - not syncing: Attempted to kill the idle task!
> [0.234000] ---[ end Kernel panic - not syncing: Attempted to kill the
> idle task!
>
> Signed-off-by: Baoquan He
> Signed-off-by: Dave Young
> Cc: Matt Fleming
> Cc: Ard Biesheuvel
> Cc: Thomas Gleixner
> Cc: Ingo Molnar
> Cc: "H. Peter Anvin"
> Cc: Thomas Garnier
> Cc: Kees Cook
> Cc: x...@kernel.org
> Cc: linux-efi@vger.kernel.org
> ---
> v1->v2:
> Change code and add description according to Thomas's suggestion as below:
>
> 1. Add checking if pud table is allocated successfully. If not just break
> the for loop.
>
> 2. Add code comment to explain how the 1:1 mapping is built in
> efi_call_phys_prolog
>
> 3. Other minor change
>
> arch/x86/platform/efi/efi_64.c | 72
> +-
> 1 file changed, 64 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 2ee7694..48de7fd 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -71,11 +71,13 @@ static void __init early_code_mapping_set_exec(int
> executable)
>
> pgd_t * __init efi_call_phys_prolog(void)
> {
> - unsigned long vaddress;
> + unsigned long vaddr, left_vaddr;
> + unsigned int num_entries;
> pgd_t *save_pgd;
> -
> - int pgd;
> + pud_t *pud, *pud_k;
> + int pud_idx;
> int n_pgds;
> + int i;
>
> if (!efi_enabled(EFI_OLD_MEMMAP)) {
> save_pgd = (pgd_t *)read_cr3();
> @@ -88,10 +90,51 @@ pgd_t * __init efi_call_phys_prolog(void)
> n_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT), PG
Re: [PATCH v2 0/3] efi: add support for non-standard capsule headers
On Wed, 19 Apr, at 08:32:59PM, Jan Kiszka wrote: > This picks up the patches Ard send before in [1], including the > "left-over" patches 6..8. > > As Ard suggested, I've taken updated patches 6 and 7 of him from [2] > which address reviewer comments. Furthermore, I've changed patch 8 to > factor out the Quark quirk logic from the overloaded > efi_capsule_setup_info as requested by Matt and also applied Andy's > suggestion to have a quirk dispatcher table with callbacks. > > Tested successfully on the IOT2040 - still without a working Galileo > board. > > Jan > > [1] http://www.spinics.net/lists/linux-efi/msg11194.html > [2] > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=quark-capsule > > Cc: Matt Fleming > > Ard Biesheuvel (2): > efi/capsule-loader: Redirect calls to efi_capsule_setup_info via weak > alias > efi/capsule-loader: Use page addresses rather than struct page > pointers > > Jan Kiszka (1): > efi/capsule: Add support for Quark security header > > arch/x86/platform/efi/quirks.c| 137 > ++ > drivers/firmware/efi/Kconfig | 9 +++ > drivers/firmware/efi/capsule-loader.c | 66 > drivers/firmware/efi/capsule.c| 7 +- > include/linux/efi.h | 14 +++- > 5 files changed, 197 insertions(+), 36 deletions(-) OK, this looks like it's in good shape to me. Ard, are we waiting for anything else before we queue this up for v4.13? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: xen: Implement EFI reset_system callback
On Wed, 19 Apr, at 09:29:06PM, Daniel Kiper wrote: > On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote: > > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote: > > > > > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper, > > > rather than spreading it further. > > > > > > IMO, given reset_system is a *mandatory* function, the Xen wrapper > > > should provide an implementation. > > > > > > I don't see why you can't implement a wrapper that calls the usual Xen > > > poweroff/reset functions. > > > > I realise I'm making a sweeping generalisation, but adding > > EFI_PARAVIRT is almost always the wrong thing to do. > > Why? Because it makes paravirt a special case, and there's usually very little reason to make it special in the EFI code. Special-casing means more branches, more code paths, a bigger testing matrix and more complex code. EFI_PARAVIRT does have its uses, like for those scenarios where we don't have a table of function pointers that can be overidden for paravirt. But we do have such a table for ->reset_system(). -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] arm64: xen: Implement EFI reset_system callback
On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote: > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper, > rather than spreading it further. > > IMO, given reset_system is a *mandatory* function, the Xen wrapper > should provide an implementation. > > I don't see why you can't implement a wrapper that calls the usual Xen > poweroff/reset functions. I realise I'm making a sweeping generalisation, but adding EFI_PARAVIRT is almost always the wrong thing to do. I'd much prefer to see Mark's idea implemented. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] efi/capsule: Add support for Quark security header
On Tue, 18 Apr, at 02:59:43PM, Jan Kiszka wrote: > > I've implemented this, but for the old design, and Ard took over then. > So it never made it to the list. > > Whatever layout of these bits is preferred, it can probably be done. I > just need an indication that there is (likely) a consensus. Post what you already have on top of Ard's series and we'll review it. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] efi/capsule-loader: use page addresses rather than struct page pointers
On Tue, 18 Apr, at 02:01:21PM, Ard Biesheuvel wrote: > On 18 April 2017 at 13:56, Matt Fleming wrote: > > On Wed, 05 Apr, at 10:23:16AM, Ard Biesheuvel wrote: > >> To give some leeway to code that handles non-standard capsule headers, > >> let's keep an array of page addresses rather than struct page pointers. > >> > >> This gives special implementations of efi_capsule_setup_info() the > >> opportunity to mangle the payload a bit before it is presented to the > >> firmware, without putting any knowledge of the nature of such quirks > >> into the generic code. > >> > >> Signed-off-by: Ard Biesheuvel > >> --- > >> drivers/firmware/efi/capsule-loader.c | 12 > >> drivers/firmware/efi/capsule.c| 7 --- > >> include/linux/efi.h | 4 ++-- > >> 3 files changed, 14 insertions(+), 9 deletions(-) > >> > >> diff --git a/drivers/firmware/efi/capsule-loader.c > >> b/drivers/firmware/efi/capsule-loader.c > >> index d68a1ecebbf3..22b2bb73176c 100644 > >> --- a/drivers/firmware/efi/capsule-loader.c > >> +++ b/drivers/firmware/efi/capsule-loader.c > >> @@ -20,6 +20,10 @@ > >> > >> #define NO_FURTHER_WRITE_ACTION -1 > >> > >> +#ifndef phys_to_page > >> +#define phys_to_page(x) virt_to_page((unsigned long)__va(x)) > >> +#endif > > > > Is this going to work with highmem pages, which presumably, is a > > possibility for the 32-bit Quark? > > Good point. Given that we don't really care about the virtual address > anyway, what is the best way to translate physical addresses to struct > page pointers on x86? i suppose pfn_to_page(pa >> PAGE_SHIFT) always > does the trick? Yep, I think that's the way to do it. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 7/8] efi/capsule-loader: use page addresses rather than struct page pointers
On Wed, 05 Apr, at 10:23:16AM, Ard Biesheuvel wrote: > To give some leeway to code that handles non-standard capsule headers, > let's keep an array of page addresses rather than struct page pointers. > > This gives special implementations of efi_capsule_setup_info() the > opportunity to mangle the payload a bit before it is presented to the > firmware, without putting any knowledge of the nature of such quirks > into the generic code. > > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/capsule-loader.c | 12 > drivers/firmware/efi/capsule.c| 7 --- > include/linux/efi.h | 4 ++-- > 3 files changed, 14 insertions(+), 9 deletions(-) > > diff --git a/drivers/firmware/efi/capsule-loader.c > b/drivers/firmware/efi/capsule-loader.c > index d68a1ecebbf3..22b2bb73176c 100644 > --- a/drivers/firmware/efi/capsule-loader.c > +++ b/drivers/firmware/efi/capsule-loader.c > @@ -20,6 +20,10 @@ > > #define NO_FURTHER_WRITE_ACTION -1 > > +#ifndef phys_to_page > +#define phys_to_page(x) virt_to_page((unsigned long)__va(x)) > +#endif Is this going to work with highmem pages, which presumably, is a possibility for the 32-bit Quark? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/8] efi/capsule: Add support for Quark security header
On Wed, 05 Apr, at 10:23:17AM, Ard Biesheuvel wrote:
> From: Jan Kiszka
>
> The firmware for Quark X102x prepends a security header to the capsule
> which is needed to support the mandatory secure boot on this processor.
> The header can be detected by checking for the "_CSH" signature and -
> to avoid any GUID conflict - validating its size field to contain the
> expected value. Then we need to look for the EFI header right after the
> security header and pass the real header to __efi_capsule_setup_info.
>
> To be minimally invasive and maximally safe, the quirk version of
> efi_capsule_identify_image is only effective on Quark processors.
>
> Signed-off-by: Jan Kiszka
> Cc: Matt Fleming
> [ardb: refactor using an override of efi_capsule_setup_info()]
> Signed-off-by: Ard Biesheuvel
> ---
> arch/x86/platform/efi/quirks.c | 112
> drivers/firmware/efi/Kconfig | 9 ++
> 2 files changed, 121 insertions(+)
[...]
> @@ -495,3 +549,61 @@ bool efi_poweroff_required(void)
> {
> return acpi_gbl_reduced_hardware || acpi_no_s5;
> }
> +
> +#ifdef CONFIG_EFI_CAPSULE_QUIRK_QUARK_CSH
> +
> +static const struct x86_cpu_id quark_ids[] = {
> + { X86_VENDOR_INTEL, 5, 9 }, /* Intel Quark X1000 */
> + { }
> +};
> +
> +int efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
> +size_t hdr_bytes)
> +{
> + struct quark_security_header *csh = kbuff;
> +
> + cap_info->total_size = 0;
> +
> + if (!x86_match_cpu(quark_ids))
> + goto fallback;
> +
I'd prefer to see the quark quirk pulled out into its own function and
referenced from the __weak efi_capsule_setup_info() function, which
makes it easier to people to read the EFI capsule code flow if they're
not interested in the Quark quick.
Something like this,
int efi_capsule_setup_info(...)
{
...
if (x86_match_cpu(quark_ids))
return efi_capsule_quark_setup_quirk(cap_info, kbuff,
hdr_bytes);
> + if (hdr_bytes < sizeof(efi_capsule_header_t))
> + return 0;
> +
> + memcpy(&cap_info->header, kbuff, sizeof(cap_info->header));
> +
> + cap_info->total_size += cap_info->header.imagesize;
> +
> + return __efi_capsule_setup_info(cap_info);
> +}
Or something.
Otherwise this looks fine to me.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] efi/capsule-loader: indirect calls to efi_capsule_setup_info via weak alias
On Wed, 05 Apr, at 10:23:15AM, Ard Biesheuvel wrote:
> To allow platform specific code to hook into the capsule loading
> routines, indirect calls to efi_capsule_setup_info() via a weak alias
> of __efi_capsule_setup_info(), allowing platforms to redefine the former
> but still use the latter.
>
> Signed-off-by: Ard Biesheuvel
> ---
> drivers/firmware/efi/capsule-loader.c | 52 +---
> include/linux/efi.h | 12 +
> 2 files changed, 35 insertions(+), 29 deletions(-)
[...]
> @@ -97,6 +71,26 @@ static int efi_capsule_setup_info(struct capsule_info
> *cap_info,
> }
>
> /**
> + * efi_capsule_setup_info - obtain the efi capsule header in the binary and
> + * setup capsule_info structure
> + * @cap_info: pointer to current instance of capsule_info structure
> + * @kbuff: a mapped first page buffer pointer
> + * @hdr_bytes: the total received number of bytes for efi header
> + **/
> +int __weak efi_capsule_setup_info(struct capsule_info *cap_info, void *kbuff,
> + size_t hdr_bytes)
> +{
> + /* Only process data block that is larger than efi header size */
> + if (hdr_bytes < sizeof(efi_capsule_header_t))
> + return 0;
> +
> + memcpy(&cap_info->header, kbuff, sizeof(cap_info->header));
> + cap_info->total_size = cap_info->header.imagesize;
> +
> + return __efi_capsule_setup_info(cap_info);
> +}
It would be good if you provided a little bit of blurb in the function
comment describing why someone might want to override this __weak
function.
Perhaps just something like,
"Platforms with non-standard capsule update mechanisms can override
this __weak function so they can perform any required capsule
image munging. See quark_quirk_function() for an example."
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/8] efi/capsule-loader: use cached copy of capsule header
On Wed, 05 Apr, at 10:23:14AM, Ard Biesheuvel wrote: > Instead of kmapping the capsule data twice, copy the capsule header > into the capsule info struct we keep locally. This is an improvement > by itself, but will also enable handling of non-standard header formats > more easily. > > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/capsule-loader.c | 41 > 1 file changed, 17 insertions(+), 24 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/8] efi/capsule: Adjust return type of efi_capsule_setup_info
On Wed, 05 Apr, at 10:23:13AM, Ard Biesheuvel wrote: > From: Jan Kiszka > > We actually expect int at the caller and never return any size > information. > > Signed-off-by: Jan Kiszka > Cc: Matt Fleming > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/capsule-loader.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/8] efi/capsule: Clean up pr_err/info messages
On Wed, 05 Apr, at 10:23:12AM, Ard Biesheuvel wrote: > From: Jan Kiszka > > Avoid __func__, improve the information provided by some of the > messages. > > Signed-off-by: Jan Kiszka > Cc: Matt Fleming > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/capsule-loader.c | 19 --- > 1 file changed, 8 insertions(+), 11 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] efi/capsule: Fix return code on failing kmap/vmap
On Wed, 05 Apr, at 10:23:10AM, Ard Biesheuvel wrote: > From: Jan Kiszka > > If kmap or vmap fail, it means we ran out of memory. There are no > user-provided addressed involved that would justify EFAULT. > > Signed-off-by: Jan Kiszka > Cc: Matt Fleming > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/capsule-loader.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] efi/capsule: Remove pr_debug on ENOMEM or EFAULT
On Wed, 05 Apr, at 10:23:11AM, Ard Biesheuvel wrote: > From: Jan Kiszka > > Both cases are not worth a debug log message - the error code is telling > enough. > > Signed-off-by: Jan Kiszka > Cc: Matt Fleming > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/capsule-loader.c | 11 ++- > 1 file changed, 2 insertions(+), 9 deletions(-) Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/24] Add a sysrq option to exit secure boot mode
On Fri, 14 Apr, at 08:05:07PM, Thomas Gleixner wrote: > On Wed, 5 Apr 2017, David Howells wrote: > > > From: Kyle McMartin > > > > Make sysrq+x exit secure boot mode on x86_64, thereby allowing the running > > kernel image to be modified. This lifts the lockdown. > > > > Signed-off-by: Kyle McMartin > > Signed-off-by: David Howells > > cc: x...@kernel.org > > Matt, Ard? > > Any opinions on this? Looks OK to me. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[GIT PULL] EFI urgent fix
Folks, please pull the single below fix from Omar which fixes a kexec boot regression. I've based the pull on tip/efi/urgent since the EFI urgent queue hasn't reached Linus' tree yet. The following changes since commit 55d728a40d368ba80443be85c02e641fc9082a3f: efi/fb: Avoid reconfiguration of BAR that covers the framebuffer (2017-04-05 12:25:53 +0200) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-urgent for you to fetch changes up to 09ca0b10e8100a48aa94eb8649f4c6c904e5d196: x86/efi: Don't try to reserve runtime regions (2017-04-12 16:17:20 +0100) - Fix a crash on kexec boot introduced by the recent efi_mem_reserve() code in the ESRT driver, which double-reserved EFI runtime regions - Omar Sandoval Omar Sandoval (1): x86/efi: Don't try to reserve runtime regions arch/x86/platform/efi/quirks.c | 4 1 file changed, 4 insertions(+) -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] x86/efi: Don't try to reserve runtime regions
From: Omar Sandoval
Reserving a runtime region results in splitting the efi memory
descriptors for the runtime region. This results in runtime region
descriptors with bogus memory mappings, leading to interesting crashes
like the following during a kexec:
[0.001000] general protection fault: [#1] SMP
[0.001000] Modules linked in:
[0.001000] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.11.0-rc1 #53
[0.001000] Hardware name: Wiwynn Leopard-Orv2/Leopard-DDR BW, BIOS LBM05
09/30/2016
[0.001000] task: 81e0e4c0 task.stack: 81e0
[0.001000] RIP: 0010:virt_efi_set_variable+0x85/0x1a0
[0.001000] RSP: :81e03e18 EFLAGS: 00010202
[0.001000] RAX: afafafafafafafaf RBX: 81e3a4e0 RCX: 0007
[0.001000] RDX: 81e03e70 RSI: 81e3a4e0 RDI: 88407f8c2de0
[0.001000] RBP: 81e03e60 R08: R09:
[0.001000] R10: R11: R12: 81e03e70
[0.001000] R13: 0007 R14: R15:
[0.001000] FS: () GS:881fff60()
knlGS:
[0.001000] CS: 0010 DS: ES: CR0: 80050033
[0.001000] CR2: 88407f30f000 CR3: 001fff102000 CR4: 000406b0
[0.001000] DR0: DR1: DR2:
[0.001000] DR3: DR6: fffe0ff0 DR7: 0400
[0.001000] Call Trace:
[0.001000] efi_delete_dummy_variable+0x7a/0x80
[0.001000] efi_enter_virtual_mode+0x3e2/0x494
[0.001000] start_kernel+0x392/0x418
[0.001000] ? set_init_arg+0x55/0x55
[0.001000] x86_64_start_reservations+0x2a/0x2c
[0.001000] x86_64_start_kernel+0xea/0xed
[0.001000] start_cpu+0x14/0x14
[0.001000] Code: 42 25 8d ff 80 3d 43 77 95 00 00 75 68 9c 8f 04 24 48 8b
05 3e 7d 7e 00 48 89 de 4d 89 f9 4d 89 f0 44 89 e9 4c 89 e2 48 8b 40 58 <48> 8b
78 58 31 c0 e8 90 e4 92 ff 48 8b 3c 24 48 c7 c6 2b 0a ca
[0.001000] RIP: virt_efi_set_variable+0x85/0x1a0 RSP: 81e03e18
[0.001000] ---[ end trace 0bd213e540e9b19f ]---
[0.001000] Kernel panic - not syncing: Fatal exception
[0.001000] ---[ end Kernel panic - not syncing: Fatal exception
Runtime regions will not be freed and do not need to be reserved, so
skip the memmap modification in this case.
Fixes: 8e80632fb23f ("efi/esrt: Use efi_mem_reserve() and avoid a kmalloc()")
Signed-off-by: Omar Sandoval
Cc: Ard Biesheuvel
Cc: Dave Young
Cc: Ingo Molnar
Cc: Peter Jones
Cc: # v4.9+
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/quirks.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 30031d5293c4..cdfe8c628959 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -201,6 +201,10 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64
size)
return;
}
+ /* No need to reserve regions that will never be freed. */
+ if (md.attribute & EFI_MEMORY_RUNTIME)
+ return;
+
size += addr % EFI_PAGE_SIZE;
size = round_up(size, EFI_PAGE_SIZE);
addr = round_down(addr, EFI_PAGE_SIZE);
--
2.10.0
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kexec regression since 4.9 caused by efi
On Mon, 20 Mar, at 10:14:12AM, Dave Young wrote: > > Matt, I'm fine if you prefer to capture the range checking errors. > Would you like me to post it or just you send it out? Can you please send out the patch with the minimal change to efi_arch_mem_reserve() and we'll get it into urgent ASAP. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kexec regression since 4.9 caused by efi
On Fri, 17 Mar, at 10:09:51AM, Dave Young wrote: > > Matt, I think it should be fine although I think the md type checking in > efi_mem_desc_lookup() is causing confusion and not easy to understand.. Could you make that a separate patch if you think of improvements there? > How about move the if chunk early like below because it seems no need > to sanity check the addr + size any more if the md is still RUNTIME? My original version did as you suggest, but I changed it because we *really* want to know if someone tries to reserve a range that spans regions. That would be totally unexpected and a warning about a potential bug/issue. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kexec regression since 4.9 caused by efi
On Mon, 13 Mar, at 03:37:48PM, Dave Young wrote:
>
> Omar, could you try below patch? Looking at the efi_mem_desc_lookup, it is not
> correct to be used in efi_arch_mem_reserve, if it passed your test, I
> can rewrite patch log with more background and send it out:
>
> for_each_efi_memory_desc(md) {
> [snip]
> if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> md->type != EFI_BOOT_SERVICES_DATA &&
> md->type != EFI_RUNTIME_SERVICES_DATA) {
> continue;
> }
>
> In above code, it meant to get a md of EFI_MEMORY_RUNTIME of either boot
> data or runtime data, this is wrong for efi_mem_reserve, because we are
> reserving boot data which has no EFI_MEMORY_RUNTIME attribute at the
> running time. Just is happened to work and we did not capture the error.
Wouldn't something like this be simpler?
---
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 30031d5293c4..cdfe8c628959 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -201,6 +201,10 @@ void __init efi_arch_mem_reserve(phys_addr_t addr, u64
size)
return;
}
+ /* No need to reserve regions that will never be freed. */
+ if (md.attribute & EFI_MEMORY_RUNTIME)
+ return;
+
size += addr % EFI_PAGE_SIZE;
size = round_up(size, EFI_PAGE_SIZE);
addr = round_down(addr, EFI_PAGE_SIZE);
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kexec regression since 4.9 caused by efi
On Thu, 09 Mar, at 12:53:36PM, Ard Biesheuvel wrote: > > Hi Omar, > > Thanks for tracking this down. > > I wonder if this is an unintended side effect of the way we repurpose > the EFI_MEMORY_RUNTIME attribute in efi_arch_mem_reserve(). AFAIUI, > splitting memory map entries should only be necessary for regions that > are not runtime memory regions to begin with, and so whether their > virtual mapping address makes sense or not should be irrelevant. > > Perhaps this only illustrates my lack of understanding of the x86 way > of doing this, so perhaps Matt can shed some light on this? Sorry for the delay. Yes, Ard is correct. It's not necessary to split/reserve memory regions that already have the EFI_MEMORY_RUNTIME attribute. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] x86/efi: Add missing 1:1 mappings to support buggy firmware
On Tue, 28 Feb, at 07:25:34PM, Sai Praneeth Prakhya wrote: > > > > > > > I don't think we should be adding yet another place in the EFI code > > > where we're modifying the page tables. > > > > > > We already have the ability to map EFI_CONVENTIONAL_MEMORY regions > > > inside of efi_map_regions() via the should_map_region() function. > > > > > > Currently, unless you're booting in mixed mode that function will > > > return 'false' if the region type is EFI_CONVENTIONAL_MEMORY, so to > > > get your machine booting you need to do two things, > > > > > > 1) Modify should_map_region() to allow EFI_CONVENTIONAL_MEMORY to be > > > mapped > > > > > > 2) Modify the 64-bit version of efi_map_region() to *only* create > > > 1:1 mapping for EFI_CONVENTIONAL_MEMORY regions. > > > > Thanks for the suggestions! Will try these and will let you know if that > > fixes the issue. > > > > I have noticed this issue on two machines HP laptop and a Desktop > (Gigabyte). Adding mappings for EFI_CONVENTIONAL_MEMORY and > *EFI_LOADER_DATA* solves the issue. Presently, I only have access to one > machine (Desktop) and as soon as I test this on laptop (hopefully it > does not access any other EFI regions illegally), I will send version 3 > of the patch. > > Since this patch will be mapping EFI_CONVENTIONAL_MEMORY and > EFI_LOADER_DATA in 1:1 mode on 64-bit machines, I think we could also do > the same with EFI_BOOT_SERVICES_DATA and EFI_BOOT_SERVICES_CODE. In > other words we could remove the existing mappings for > EFI_BOOT_SERVICES_CODE and EFI_BOOT_SERVICES_DATA from VA mapping. Do you know what data is being access in the EFI_LOADER_DATA region? Accessing that via the 1:1 mapping is really strange because the firmware will have had to convert any addresses the kernel gave it from virtual to physical (the kernel stores things in EFI_LOADER_DATA regions during boot). > I think; this should not break any machines because firmware could > access illegal addresses only in 1:1 mode and not in VA mode because > that's the only address space firmware has knowledge about. Firmware > doesn't know about virtual addresses until we pass them through > SetVirtualAddressMap(). > > So Matt, could you please confirm if you had come across any machines > that did illegal access to EFI regions using virtual addresses? I don't think I do have access to such machines, but what would removing the virtual mappings buy us? The risk of breaking machines with buggy firmware outweighs any benefit that I can think of. Am I missing something? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] efi: implement interruptible runtime services
On Fri, 18 Dec, at 11:29:51AM, sylvain.choul...@gmail.com wrote:
> From: Sylvain Chouleur
>
> This patch adds an implementation of EFI runtime services wappers which
> allow interrupts and preemption during execution of BIOS code.
>
> It's needed by Broxton platform which requires the OS to do the write
> of the non volatile variables. To do that, the OS will receive an
> interrupt coming from the firmware to notify that it must write the
> data.
>
> BIOS code must be executed using a specific PGD. This is currently
> done by efi_call() which force value of CR3 before calling BIOS and
> restore previous value after.
>
> We use a dedicated kthread which will execute all interruptible runtime
> services. This efi_kthread is special because it has it's own mm_struct
> whereas kthread should not have one. This mm_struct contains the EFI PGD
> so the scheduler will load it before running any BIOS code.
>
> Obviously, an interruptible runtime service must never be called from an
> interrupt context. EFI pstore is the only use case here, that's why we
> require it to be disabled when activating interruptible runtime services.
>
> Signed-off-by: Sylvain Chouleur
> ---
> arch/x86/Kconfig | 14 +++
> arch/x86/include/asm/efi.h| 1 +
> arch/x86/platform/efi/Makefile| 1 +
> arch/x86/platform/efi/efi_32.c| 5 +
> arch/x86/platform/efi/efi_64.c| 5 +
> arch/x86/platform/efi/efi_interruptible.c | 191
> ++
> 6 files changed, 217 insertions(+)
> create mode 100644 arch/x86/platform/efi/efi_interruptible.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index db3622f22b61..3ddd5a9cab30 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1720,6 +1720,20 @@ config EFI_MIXED
>
> If unsure, say N.
>
> +if X86_64
> +config EFI_INTERRUPTIBLE
> + bool "Interruptible Efi Runtime Services"
> + depends on EFI && !EFI_VARS_PSTORE
> + default n
> + help
> + This is an interruptible implementation of efi runtime
> + services wrappers. If you say Y, BIOS code could be
> + interrupted and preempted as a standard process. Enable this
> + only if you are sure that your BIOS will support
> + interruptible efi calls
> + In doubt, say "N".
> +endif
> +
These words should be a little more assertive. Almost everyone is
going to want to turn this feature off.
> +static efi_status_t
> +non_blocking_not_allowed(__attribute__((unused)) efi_char16_t *name,
> + __attribute__((unused)) efi_guid_t *vendor,
> + __attribute__((unused)) u32 attr,
> + __attribute__((unused)) unsigned long data_size,
> + __attribute__((unused)) void *data)
> +{
> + pr_err("efi_interruptible: non bloking operation is not allowed\n");
> + return EFI_UNSUPPORTED;
> +}
Typo, s/bloking/blocking/
> +static int efi_interruptible_panic_notifier_call(
> + struct notifier_block *notifier,
> + unsigned long what, void *data)
> +{
> + static struct efivars generic_efivars;
> + static struct efivar_operations generic_ops;
> +
> + generic_ops.get_variable = efi.get_variable;
> + generic_ops.set_variable = efi.set_variable;
> + generic_ops.get_next_variable = efi.get_next_variable;
> + generic_ops.query_variable_store = efi_query_variable_store;
> +
> + efivars_register(&generic_efivars, &generic_ops, efivars_kobject());
> +
> + return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block panic_nb = {
> + .notifier_call = efi_interruptible_panic_notifier_call,
> + .priority = 100,
> +};
> +
What's the purpose of this? The only reason you'd want to do this that
I can think of is because you'd want efi-pstore to run and attempt to
save some crash data for you. But you can't build with efi-pstore
enable and CONFIG_EFI_INTERRUPTIBLE.
I'd be tempted to just delete this notifier code.
> +static struct task_struct *efi_kworker_task;
> +static struct efivar_operations interruptible_ops;
> +static __init int efi_interruptible_init(void)
> +{
> + int ret;
> +
> + efi_kworker_task = kthread_create(kthread_worker_fn, &efi_kworker,
> + "efi_kthread");
> + if (IS_ERR(efi_kworker_task)) {
> + pr_err("efi_interruptible: Failed to create kthread\n");
> + ret = PTR_ERR(efi_kworker_task);
> + efi_kworker_task = NULL;
> + return ret;
> + }
> +
> + efi_kworker_task->mm = mm_alloc();
> + efi_kworker_task->active_mm = efi_kworker_task->mm;
> + efi_kworker_task->mm->pgd = efi_get_pgd();
> + wake_up_process(efi_kworker_task);
> +
> + atomic_notifier_chain_register(&panic_notifier_list, &panic_nb);
> +
> + interruptible_ops.get_variable = get_variable_interruptible;
> + interruptible_ops.set_variable = set_variable_interruptible;
> +
Re: [PATCH 1/2] efi: Don't use spinlocks for efi vars
(Cc'ing Ard since he has recently been in this area)
On Fri, 18 Dec, at 11:29:50AM, sylvain.choul...@gmail.com wrote:
> From: Sylvain Chouleur
>
> All efivars operations are protected by a spinlock which prevents
> interruptions and preemption. This is too restricted, we just need a
> lock preventing concurency.
> The idea is to use a semaphore of count 1 and to have two ways of
> locking, depending on the context:
> - In interrupt context, we call down_trylock(), if it fails we return
> an error
> - In normal context, we call down_interruptible()
>
> We don't use a mutex here because the mutex_trylock() function must not
> be called from interrupt context, whereas the down_trylock() can.
>
> This patch also remove the efivars lock to use a single lock for the
> whole vars.c file. The goal of this lock is to protect concurrent calls
> to efi variable services, registering and unregistering.
> This allows to register new efivars operations without having
> in-progress call.
>
> Signed-off-by: Sylvain Chouleur
> ---
> drivers/firmware/efi/efi-pstore.c | 34 +++---
> drivers/firmware/efi/efivars.c| 10 ++-
> drivers/firmware/efi/vars.c | 130
> +-
> fs/efivarfs/inode.c | 5 +-
> fs/efivarfs/super.c | 8 ++-
> include/linux/efi.h | 12 +---
> 6 files changed, 130 insertions(+), 69 deletions(-)
This needs splitting into more than 1 patch.
You need separate patches to,
- Split out __efivars->lock into a file local lock
- Convert the lock to a semaphore
- Print a message when efi_operations are registered
Further comments below.
> diff --git a/drivers/firmware/efi/efivars.c b/drivers/firmware/efi/efivars.c
> index 756eca8c4cf8..3998373d92ef 100644
> --- a/drivers/firmware/efi/efivars.c
> +++ b/drivers/firmware/efi/efivars.c
> @@ -509,7 +509,8 @@ static ssize_t efivar_delete(struct file *filp, struct
> kobject *kobj,
> vendor = del_var->VendorGuid;
> }
>
> - efivar_entry_iter_begin();
> + if (efivar_entry_iter_begin())
> + return -EINTR;
> entry = efivar_entry_find(name, vendor, &efivar_sysfs_list, true);
> if (!entry)
> err = -EINVAL;
> @@ -582,7 +583,8 @@ efivar_create_sysfs_entry(struct efivar_entry *new_var)
> return ret;
>
> kobject_uevent(&new_var->kobj, KOBJ_ADD);
> - efivar_entry_add(new_var, &efivar_sysfs_list);
> + if (efivar_entry_add(new_var, &efivar_sysfs_list))
> + return -EINTR;
>
> return 0;
> }
This looks like it's missing a call to efivar_unregister() in the
-EINTR case.
> @@ -697,7 +699,9 @@ static int efivars_sysfs_callback(efi_char16_t *name,
> efi_guid_t vendor,
>
> static int efivar_sysfs_destroy(struct efivar_entry *entry, void *data)
> {
> - efivar_entry_remove(entry);
> + int err = efivar_entry_remove(entry);
> + if (err)
> + return err;
> efivar_unregister(entry);
> return 0;
> }
You now need to return early from efivars_sysfs_exit() if you hit the
error path in efivar_sysfs_destroy().
> diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
> index 70a0fb10517f..8a44351211e5 100644
> --- a/drivers/firmware/efi/vars.c
> +++ b/drivers/firmware/efi/vars.c
> @@ -37,6 +37,13 @@
> /* Private pointer to registered efivars */
> static struct efivars *__efivars;
>
> +/*
> + * ->lock protects two things:
> + * 1) efivarfs_list and efivars_sysfs_list
> + * 2) ->ops calls
> + */
> +DEFINE_SEMAPHORE(efivars_lock);
> +
Now it also protects registration of __efivars, so that needs to be
documented too.
> static bool efivar_wq_enabled = true;
> DECLARE_WORK(efivar_work, NULL);
> EXPORT_SYMBOL_GPL(efivar_work);
> @@ -376,7 +383,10 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t,
> unsigned long, void *),
> return -ENOMEM;
> }
>
> - spin_lock_irq(&__efivars->lock);
> + if (down_interruptible(&efivars_lock)) {
> + err = -EINTR;
> + goto free;
> + }
>
> /*
>* Per EFI spec, the maximum storage allocated for both
> @@ -392,7 +402,7 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t,
> unsigned long, void *),
> switch (status) {
> case EFI_SUCCESS:
> if (!atomic)
> - spin_unlock_irq(&__efivars->lock);
> + up(&efivars_lock);
>
> variable_name_size = var_name_strnsize(variable_name,
>
> variable_name_size);
> @@ -410,7 +420,10 @@ int efivar_init(int (*func)(efi_char16_t *, efi_guid_t,
> unsigned long, void *),
> dup_variable_bug(variable_name, &vendor_guid,
>variable_name_size);
> if (!atomic)
> -
Re: [PATCH] x86/efi-bgrt: Don't ignore the BGRT if the 'valid' bit is 0
On Sun, 20 Dec, at 10:53:11PM, Môshe van der Sterre wrote:
> Unintuitively, the BGRT graphic is apparently meant to be usable if
> the valid bit in not set. The valid bit only conveys uncertainty
> about the validity in relation to the screen state.
>
> Windows 10 actually uses the BGRT image for its boot screen even if
> not 'valid', for example when the user triggered the boot menu.
> Because it is unclear if all firmwares will provide a usable graphic
> in this case, we now look at the BMP magic number as an additional
> check.
> ---
> arch/x86/platform/efi/efi-bgrt.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/platform/efi/efi-bgrt.c
> b/arch/x86/platform/efi/efi-bgrt.c
> index b097066..a243381 100644
> --- a/arch/x86/platform/efi/efi-bgrt.c
> +++ b/arch/x86/platform/efi/efi-bgrt.c
> @@ -57,11 +57,6 @@ void __init efi_bgrt_init(void)
> bgrt_tab->status);
> return;
> }
> - if (bgrt_tab->status != 1) {
> - pr_debug("Ignoring BGRT: invalid status %u (expected 1)\n",
> - bgrt_tab->status);
> - return;
> - }
> if (bgrt_tab->image_type != 0) {
> pr_err("Ignoring BGRT: invalid image type %u (expected 0)\n",
> bgrt_tab->image_type);
> @@ -80,6 +75,11 @@ void __init efi_bgrt_init(void)
>
> memcpy(&bmp_header, image, sizeof(bmp_header));
> memunmap(image);
> + if (bmp_header.id != 0x4d42) {
> + pr_err("Ignoring BGRT: Incorrect BMP magic number 0x%x
> (expected 0x4d42)\n",
> + bmp_header.id);
> + return;
> + }
> bgrt_image_size = bmp_header.size;
>
> bgrt_image = kmalloc(bgrt_image_size, GFP_KERNEL | __GFP_NOWARN);
Shouldn't this be pr_debug() instead of pr_err()?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi: stub: define DISABLE_BRANCH_PROFILING for all architectures
On Tue, 05 Jan, at 03:56:39PM, Ard Biesheuvel wrote: > (+ Arnd) > > On 4 January 2016 at 13:31, Will Deacon wrote: > > On Wed, Dec 23, 2015 at 10:29:28AM +0100, Ard Biesheuvel wrote: > >> This moves the DISABLE_BRANCH_PROFILING define from the x86 specific > >> to the general CFLAGS definition for the stub. This fixes build errors > >> when building for arm64 with CONFIG_PROFILE_ALL_BRANCHES_ENABLED. > >> > >> Reported-by: Will Deacon > >> Signed-off-by: Ard Biesheuvel > >> --- > >> drivers/firmware/efi/libstub/Makefile | 4 ++-- > >> 1 file changed, 2 insertions(+), 2 deletions(-) > > > > Cheers, Ard. The kernel now builds and boots as an EFI application for > > me when the dreaded #define if config option is enabled: > > > > Tested-by: Will Deacon > > > > Will > > > > @Matt: this is causing randconfig build errors for ARM in -next. Mind > if we take this through some other tree? Go ahead. Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86/efi-bgrt: Replace early_memremap() with memremap()
On Mon, 21 Dec, at 02:12:52PM, Matt Fleming wrote:
> Môshe reported the following warning triggered on his machine since
> commit 50a0cb565246 ("x86/efi-bgrt: Fix kernel panic when mapping BGRT
> data"),
>
> [0.026936] [ cut here ]
> [0.026941] WARNING: CPU: 0 PID: 0 at mm/early_ioremap.c:137
> __early_ioremap+0x102/0x1bb()
> [0.026941] Modules linked in:
> [0.026944] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rc1 #2
> [0.026945] Hardware name: Dell Inc. XPS 13 9343/09K8G1, BIOS A05
> 07/14/2015
> [0.026946] 900f03d5a116524d 81c03e60
> 813a3fff
> [0.026948] 81c03e98 810a0852
> d7b76000
> [0.026949] 0001 0001
> 017c
> [0.026951] Call Trace:
> [0.026955] [] dump_stack+0x44/0x55
> [0.026958] [] warn_slowpath_common+0x82/0xc0
> [0.026959] [] warn_slowpath_null+0x1a/0x20
> [0.026961] [] __early_ioremap+0x102/0x1bb
> [0.026962] [] early_memremap+0x13/0x15
> [0.026964] [] efi_bgrt_init+0x162/0x1ad
> [0.026966] [] efi_late_init+0x9/0xb
> [0.026968] [] start_kernel+0x46f/0x49f
> [0.026970] [] ? early_idt_handler_array+0x120/0x120
> [0.026972] [] x86_64_start_reservations+0x2a/0x2c
> [0.026974] [] x86_64_start_kernel+0x14a/0x16d
> [0.026977] ---[ end trace f9b3812eb8e24c58 ]---
> [0.026978] efi_bgrt: Ignoring BGRT: failed to map image memory
>
> early_memremap() has an upper limit on the size of mapping it can
> handle which is ~200KB. Clearly the BGRT image on Môshe's machine is
> much larger than that.
>
> There's actually no reason to restrict ourselves to using the early_*
> version of memremap() - the ACPI BGRT driver is invoked late enough in
> boot that we can use the standard version, with the benefit that the
> late version allows mappings of arbitrary size.
>
> Reported-by: Môshe van der Sterre
> Tested-by: Môshe van der Sterre
> Cc: Josh Triplett
> Cc: Sai Praneeth Prakhya
> Cc: Borislav Petkov
> Signed-off-by: Matt Fleming
> ---
> arch/x86/platform/efi/efi-bgrt.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
Ping? Could someone please pick this up for the 'x86/efi' branch? It
fixes a bug in 50a0cb565246 ("x86/efi-bgrt: Fix kernel panic when
mapping BGRT data").
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] x86/efi: print size and base in binary units in efi_print_memmap
On Wed, 23 Dec, at 12:11:56AM, Elliott, Robert (Persistent Memory) wrote: > > I was trying to make it resemble the memmap=size@address > kernel parameter format for creating e820 entries, which > does accept abbreviations in addition to hex values: > memmap=nn[KMG]@ss[KMG] for usable DRAM > memmap=nn[KMG]#ss[KMG] for ACPI data > memmap=nn[KMG]$ss[KMG] for reserved > memmap=nn[KMG]!ss[KMG] for persistent memory > > Mapping the UEFI type to the corresponding @, #, $, or ! was > more than I wanted to tackle, so it's not a drop-in > replacement string. > > memparse() also accepts T, P, and E units; I guess those > need to be added to Documentation/kernel-parameters.txt. I think the value of the "@ address" portion of the string is questionable. > Thanks for the pointer; I wondered if there was a similar > function somewhere. However, that function throws away > precision in favor of printing just 3 significant digits; > I think that's dangerous. Its non-integer output is not > supported by memmap=, and the function appears to use > assembly code to get CPU divide instructions, losing the > ability to use shifts for these power of two divisions. > > Example results... > > efi: mem01:... range=[0x00093000-0x00093fff] (4 KiB @ 588 KiB) > efi: mem01:... range=[0x00093000-0x00093fff] (4.00 KiB @ 588 > KiB) SGS > > efi: mem03:... range=[0x0010-0x013e8fff] (19364 KiB @ 1 > MiB) > efi: mem03:... range=[0x0010-0x013e8fff] (18.9 MiB @ 1.00 > MiB) SGS > (example of lost precision: 19364 KiB is really 18.91015625 MiB) > > efi: mem04:... range=[0x013e9000-0x01ff] (12380 KiB @ > 20388 KiB) > efi: mem04:... range=[0x013e9000-0x01ff] (12.0 MiB @ 19.9 > MiB) SGS > > efi: mem28:... range=[0x717c2000-0x72acafff] (19492 KiB @ > 1859336 KiB) > efi: mem28:... range=[0x717c2000-0x72acafff] (19.0 MiB @ 1.77 > GiB) SGS > > efi: mem57:... range=[0x00088000-0x000e7fff] (24 GiB @ 34 GiB) > efi: mem57:... range=[0x00088000-0x000e7fff] (24.0 GiB @ 34.0 > GiB) SGS Good points! I agree that string_get_size() (unfortunately) doesn't look useful in this scenario. The code in efi_size_format() looks fine. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] x86/efi: show actual ending addresses in efi_print_memmap
On Mon, 21 Dec, at 04:44:19PM, Elliott, Robert (Persistent Memory) wrote:
>
> The format of that line is architecture-specific and only appears
> under the efi=debug kernel parameter, so I don't know how much
> anyone relies on the specific format. Good question for the lists.
Both kernel and non-kernel developers are consumers of these debug
statements, and people do come to rely on the format of the text.
I have certainly become acustomed to the current format we have,
particularly when debugging issues via other users, i.e. when I'm not
the person running the kernel being debugged.
Just because it's a debug option doesn't mean it's completely open to
modification. Reasonable Justification must be provided. Having said
that, I think you provide a good argument in your other email,
https://lkml.kernel.org/r/94d0cd8314a33a4d9d801c0fe68b40295bec7...@g9w0745.americas.hpqcorp.net
> arch/ia64/kernel/efi.c shares the range=[...) format, but prints
> the range after the bitmask rather than before:
> printk("mem%02d: %s "
> "range=[0x%016lx-0x%016lx) (%4lu%s)\n",
> i, efi_md_typeattr_format(buf, sizeof(buf), md),
> md->phys_addr,
> md->phys_addr + efi_md_size(md), size, unit);
>
> arch/arm64/kernel/efi.c has no mem prefix, or range=[...) text
> surrounding the range:
> pr_info(" 0x%012llx-0x%012llx %s",
> paddr, paddr + (npages << EFI_PAGE_SHIFT) - 1,
> efi_md_typeattr_format(buf, sizeof(buf), md));
> pr_cont("*");
> pr_cont("\n");
>
> The x86 code is inside ifdef EFI_DEBUG, which is always
> defined in that file. I wonder if it was supposed to change
> to efi_enabled(EFI_DBG) to be based off the efi=debug kernel
> parameter? efi_init() qualified the call to this function
> based on that:
> if (efi_enabled(EFI_DBG))
> efi_print_memmap();
The EFI_DEBUG symbol should just be deleted. It's been enabled since
forever and really provides no value, because as you point out,
printing of the memory map is guarded by EFI_DBG anyway.
I'll send out a patch for ripping that out.
> In arch/ia64/kernel/efi.c efi_init(), EFI_DEBUG is set to 0
> so the print doesn't happen at all without editing the
> source code. It doesn't use efi_enabled(EFI_DBG).
>
> arch/arm64/kernel/efi.c uses efi_enabled(EFI_DBG) exclusively.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] x86/efi: show actual ending addresses in efi_print_memmap
On Tue, 22 Dec, at 08:08:19PM, Elliott, Robert (Persistent Memory) wrote: > > While investigating the grub persistent memory corruption > issues, it was helpful to make this table match the ending > address convention used by: > * the kernel's e820 table prints > * the kernel's nosave memory prints > * grub's lsmmap and lsefimmap commands > * the UEFI shell's memmap command > > Using the excerpts from the patch, if you grep all the various > logs for c7fff, you won't find the line with c8000. I like this. This is reasonable justification. Please inclue it in the commit log. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/4] x86/efi: print size and base in binary units in efi_print_memmap
On Thu, 17 Dec, at 07:28:34PM, Robert Elliott wrote:
> Print the base address for each range in decimal alongside the size.
> Use a "(size @ base)" format similar to the fake_memmap kernel parameter.
>
> Print the range and base in the best-fit B, KiB, MiB, etc. units rather
> than always MiB. This avoids rounding, which can be misleading.
>
> Use proper IEC binary units (KiB, MiB, etc.) rather than misuse SI
> decimal units (KB, MB, etc.).
>
> old:
> efi: mem61: [Persistent Memory | | | | | | | |WB|WT|WC|UC]
> range=[0x00088000-0x000c7fff) (16384MB)
>
> new:
> efi: mem61: [Persistent Memory | | | | | | | |WB|WT|WC|UC]
> range=[0x00088000-0x000c7fff] (16 GiB @ 34 GiB)
>
> Signed-off-by: Robert Elliott
> ---
> arch/x86/platform/efi/efi.c | 27 ---
> 1 file changed, 24 insertions(+), 3 deletions(-)
I'm not at all sure of the value of printing the physical address as a
size. I would have thought that you'd have to convert it back to an
address whenever you wanted to use it anyway.
> diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
> index 635a955..030ba91 100644
> --- a/arch/x86/platform/efi/efi.c
> +++ b/arch/x86/platform/efi/efi.c
> @@ -222,6 +222,25 @@ int __init efi_memblock_x86_reserve_range(void)
> return 0;
> }
>
> +char * __init efi_size_format(char *buf, size_t size, u64 bytes)
> +{
> + if (!bytes || (bytes & 0x3ff))
> + snprintf(buf, size, "%llu B", bytes);
> + else if (bytes & 0xf)
> + snprintf(buf, size, "%llu KiB", bytes >> 10);
> + else if (bytes & 0x3fff)
> + snprintf(buf, size, "%llu MiB", bytes >> 20);
> + else if (bytes & 0xff)
> + snprintf(buf, size, "%llu GiB", bytes >> 30);
> + else if (bytes & 0x3)
> + snprintf(buf, size, "%llu TiB", bytes >> 40);
> + else if (bytes & 0xfff)
> + snprintf(buf, size, "%llu PiB", bytes >> 50);
> + else
> + snprintf(buf, size, "%llu EiB", bytes >> 60);
> + return buf;
> +}
> +
Can we use string_get_size() instead of rolling our own function?
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] x86/efi: show actual ending addresses in efi_print_memmap
On Mon, 21 Dec, at 03:50:38PM, Matt Fleming wrote: > On Thu, 17 Dec, at 07:28:31PM, Robert Elliott wrote: > > Adjust efi_print_memmap to print the real end address of each > > range, not 1 byte beyond. This matches other prints like those for > > SRAT and nosave memory. > > > > Change the closing ) to ] to match the opening [. > > > > old: > > efi: mem61: [Persistent Memory | | | | | | | |WB|WT|WC|UC] > > range=[0x00088000-0x000c8000) (16384MB) > > > > new: > > efi: mem61: [Persistent Memory | | | | | | | |WB|WT|WC|UC] > > range=[0x00088000-0x000c7fff] (16384MB) > > > > Example other address range prints: > > SRAT: Node 1 PXM 1 [mem 0x48000-0x87fff] > > PM: Registered nosave memory: [mem 0x88000-0xc7fff] > > > > Signed-off-by: Robert Elliott > > --- > > arch/x86/platform/efi/efi.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > Is this change purely for aesthetic reasons? We're usually not in the > habit of changing the output of print messages without a good reason > because people downstream do rely on them being consistent. I just noticed the bracket change. My comment above only refers to printing the range addresses. Swapping ')' for ']' is a perfectly valid change. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] efi: add NV memory attribute
(Cc'ing people that have worked in this area recently)
On Thu, 17 Dec, at 07:28:32PM, Robert Elliott wrote:
> Add the NV memory attribute introduced in UEFI 2.5 and add a column
> for it in the types and attributes string used when printing the UEFI
> memory map.
>
> old:
> efi: mem61: [type=14| | | | | | | |WB|WT|WC|UC]
> range=[0x00088000-0x000c7fff) (16384MB)
>
> new:
> efi: mem61: [type=14| | |NV| | | | | |WB|WT|WC|UC]
> range=[0x00088000-0x000c7fff) (16384MB)
>
> Signed-off-by: Robert Elliott
> ---
> drivers/firmware/efi/efi.c | 5 -
> include/linux/efi.h| 1 +
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 027ca21..4dd5464 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -611,13 +611,16 @@ char * __init efi_md_typeattr_format(char *buf, size_t
> size,
> if (attr & ~(EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT |
>EFI_MEMORY_WB | EFI_MEMORY_UCE | EFI_MEMORY_RO |
>EFI_MEMORY_WP | EFI_MEMORY_RP | EFI_MEMORY_XP |
> + EFI_MEMORY_NV |
>EFI_MEMORY_RUNTIME | EFI_MEMORY_MORE_RELIABLE))
> snprintf(pos, size, "|attr=0x%016llx]",
>(unsigned long long)attr);
> else
> - snprintf(pos, size,
> "|%3s|%2s|%2s|%2s|%2s|%2s|%3s|%2s|%2s|%2s|%2s]",
> + snprintf(pos, size,
> + "|%3s|%2s|%2s|%2s|%2s|%2s|%2s|%3s|%2s|%2s|%2s|%2s]",
>attr & EFI_MEMORY_RUNTIME ? "RUN" : "",
>attr & EFI_MEMORY_MORE_RELIABLE ? "MR" : "",
> + attr & EFI_MEMORY_NV ? "NV" : "",
>attr & EFI_MEMORY_XP ? "XP" : "",
>attr & EFI_MEMORY_RP ? "RP" : "",
>attr & EFI_MEMORY_WP ? "WP" : "",
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 569b5a8..9ce9e9e 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -97,6 +97,7 @@ typedef struct {
> #define EFI_MEMORY_WP((u64)0x1000ULL)/*
> write-protect */
> #define EFI_MEMORY_RP((u64)0x2000ULL)/*
> read-protect */
> #define EFI_MEMORY_XP((u64)0x4000ULL)/*
> execute-protect */
> +#define EFI_MEMORY_NV((u64)0x8000ULL)/*
> non-volatile */
> #define EFI_MEMORY_MORE_RELIABLE \
> ((u64)0x0001ULL)/* higher
> reliability */
> #define EFI_MEMORY_RO((u64)0x0002ULL)/*
> read-only */
Seems straight forward to me.
Applied, thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] x86/efi: show actual ending addresses in efi_print_memmap
On Thu, 17 Dec, at 07:28:31PM, Robert Elliott wrote: > Adjust efi_print_memmap to print the real end address of each > range, not 1 byte beyond. This matches other prints like those for > SRAT and nosave memory. > > Change the closing ) to ] to match the opening [. > > old: > efi: mem61: [Persistent Memory | | | | | | | |WB|WT|WC|UC] > range=[0x00088000-0x000c8000) (16384MB) > > new: > efi: mem61: [Persistent Memory | | | | | | | |WB|WT|WC|UC] > range=[0x00088000-0x000c7fff] (16384MB) > > Example other address range prints: > SRAT: Node 1 PXM 1 [mem 0x48000-0x87fff] > PM: Registered nosave memory: [mem 0x88000-0xc7fff] > > Signed-off-by: Robert Elliott > --- > arch/x86/platform/efi/efi.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Is this change purely for aesthetic reasons? We're usually not in the habit of changing the output of print messages without a good reason because people downstream do rely on them being consistent. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] x86/efi-bgrt: Replace early_memremap() with memremap()
Môshe reported the following warning triggered on his machine since
commit 50a0cb565246 ("x86/efi-bgrt: Fix kernel panic when mapping BGRT
data"),
[0.026936] [ cut here ]
[0.026941] WARNING: CPU: 0 PID: 0 at mm/early_ioremap.c:137
__early_ioremap+0x102/0x1bb()
[0.026941] Modules linked in:
[0.026944] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rc1 #2
[0.026945] Hardware name: Dell Inc. XPS 13 9343/09K8G1, BIOS A05
07/14/2015
[0.026946] 900f03d5a116524d 81c03e60
813a3fff
[0.026948] 81c03e98 810a0852
d7b76000
[0.026949] 0001 0001
017c
[0.026951] Call Trace:
[0.026955] [] dump_stack+0x44/0x55
[0.026958] [] warn_slowpath_common+0x82/0xc0
[0.026959] [] warn_slowpath_null+0x1a/0x20
[0.026961] [] __early_ioremap+0x102/0x1bb
[0.026962] [] early_memremap+0x13/0x15
[0.026964] [] efi_bgrt_init+0x162/0x1ad
[0.026966] [] efi_late_init+0x9/0xb
[0.026968] [] start_kernel+0x46f/0x49f
[0.026970] [] ? early_idt_handler_array+0x120/0x120
[0.026972] [] x86_64_start_reservations+0x2a/0x2c
[0.026974] [] x86_64_start_kernel+0x14a/0x16d
[0.026977] ---[ end trace f9b3812eb8e24c58 ]---
[0.026978] efi_bgrt: Ignoring BGRT: failed to map image memory
early_memremap() has an upper limit on the size of mapping it can
handle which is ~200KB. Clearly the BGRT image on Môshe's machine is
much larger than that.
There's actually no reason to restrict ourselves to using the early_*
version of memremap() - the ACPI BGRT driver is invoked late enough in
boot that we can use the standard version, with the benefit that the
late version allows mappings of arbitrary size.
Reported-by: Môshe van der Sterre
Tested-by: Môshe van der Sterre
Cc: Josh Triplett
Cc: Sai Praneeth Prakhya
Cc: Borislav Petkov
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/efi-bgrt.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c
index bf51f4c02562..b0970661870a 100644
--- a/arch/x86/platform/efi/efi-bgrt.c
+++ b/arch/x86/platform/efi/efi-bgrt.c
@@ -72,14 +72,14 @@ void __init efi_bgrt_init(void)
return;
}
- image = early_memremap(bgrt_tab->image_address, sizeof(bmp_header));
+ image = memremap(bgrt_tab->image_address, sizeof(bmp_header),
MEMREMAP_WB);
if (!image) {
pr_err("Ignoring BGRT: failed to map image header memory\n");
return;
}
memcpy(&bmp_header, image, sizeof(bmp_header));
- early_memunmap(image, sizeof(bmp_header));
+ memunmap(image);
bgrt_image_size = bmp_header.size;
bgrt_image = kmalloc(bgrt_image_size, GFP_KERNEL | __GFP_NOWARN);
@@ -89,7 +89,7 @@ void __init efi_bgrt_init(void)
return;
}
- image = early_memremap(bgrt_tab->image_address, bmp_header.size);
+ image = memremap(bgrt_tab->image_address, bmp_header.size, MEMREMAP_WB);
if (!image) {
pr_err("Ignoring BGRT: failed to map image memory\n");
kfree(bgrt_image);
@@ -98,5 +98,5 @@ void __init efi_bgrt_init(void)
}
memcpy(bgrt_image, image, bgrt_image_size);
- early_memunmap(image, bmp_header.size);
+ memunmap(image);
}
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 0/7] efi: run UEFI services with interrupts enabled
On Mon, 14 Dec, at 11:40:24AM, Ard Biesheuvel wrote: > The UEFI spec does not require interrupts to be disabled when invoking > runtime services. The reason we have been doing so is because EFI pstore > may call SetVariable() from interrupt context, which may result in deadlock > if another runtime services call was in progress on the same cpu. > The EFI pstore has already been updated to use a non-blocking path and fail > gracefully rather than spin forever, so we can update the ordinary blocking > wrappers to run with interrupts enabled instead. This aims to prevent > excessive > interrupt latencies on uniprocessor platforms with slow variable stores. > > Changes since v2: > - folded Matt's suggestion for patch #4, to bail rather than try to trigger a > garbage collection in the nonblocking case when there is insufficient space > - rebased onto v4.4-rc5 > > Changes since v1: > - added nonblocking QueryVariableInfo() wrapper variant, and updated > efi_query_variable_store() to use it when called in nonblocking context > - add patch to drop the rtc_lock spinlock > - add patch to drop redundant efi_set_variable_nonblocking_t typedef > - drop BUG_ONs in patch #7 > > Ard Biesheuvel (7): > efi: expose non-blocking set_variable() wrapper to efivars > efi: remove redundant efi_set_variable_nonblocking prototype > efi: runtime-wrappers: add a nonblocking version of QueryVariableInfo > efi: add nonblocking option to efi_query_variable_store() > efi: runtime-wrappers: remove out of date comment regarding in_nmi() > efi: runtime-wrapper: get rid of the rtc_lock spinlock > efi: runtime-wrappers: run UEFI Runtime Services with interrupts > enabled > > arch/x86/platform/efi/quirks.c | 33 +- > drivers/firmware/efi/efi.c | 1 + > drivers/firmware/efi/runtime-wrappers.c | 115 +++- > drivers/firmware/efi/vars.c | 16 ++- > include/linux/efi.h | 21 ++-- > 5 files changed, 100 insertions(+), 86 deletions(-) Thanks Ard, this all looks good to me. I've queued this up. It's a little late for v4.5 now, so this will probably be targeting v4.6. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/7] efi: runtime-wrappers: add a nonblocking version of QueryVariableInfo
On Mon, 14 Dec, at 11:40:27AM, Ard Biesheuvel wrote: > From: Ard Biesheuvel > > This introduces a new runtime wrapper for the QueryVariableInfo() UEFI > Runtime Service, which gives up immediately rather than spins on failure > to grab the efi_runtime spinlock. > > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/runtime-wrappers.c | 22 > include/linux/efi.h | 1 + > 2 files changed, 23 insertions(+) Thanks Ard, applied with this additional piece in the commit log, This is required in the non-blocking path of the efi-pstore code. so that we don't have to wonder in 6 months why we introduced this wrapper. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: BGRT doesn't work for me on efi-next
(Pulling in Ingo and Juergen because of commit 954e12f7a800)
On Thu, 17 Dec, at 05:30:45PM, Môshe van der Sterre wrote:
> Hello Sai and others,
>
> The change to use early_mem*() instead of early_io*() in 50a0cb56 does not
> work on my machine. Last week I discussed some BGRT changes and I created a
> patch for that, but can't test it on efi-next because of this.
>
> I get this (when booting 50a0cb56, without any of my changes):
> [0.026936] [ cut here ]
> [0.026941] WARNING: CPU: 0 PID: 0 at mm/early_ioremap.c:137
> __early_ioremap+0x102/0x1bb()
> [0.026941] Modules linked in:
> [0.026944] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.4.0-rc1 #2
> [0.026945] Hardware name: Dell Inc. XPS 13 9343/09K8G1, BIOS A05
> 07/14/2015
> [0.026946] 900f03d5a116524d 81c03e60
> 813a3fff
> [0.026948] 81c03e98 810a0852
> d7b76000
> [0.026949] 0001 0001
> 017c
> [0.026951] Call Trace:
> [0.026955] [] dump_stack+0x44/0x55
> [0.026958] [] warn_slowpath_common+0x82/0xc0
> [0.026959] [] warn_slowpath_null+0x1a/0x20
> [0.026961] [] __early_ioremap+0x102/0x1bb
> [0.026962] [] early_memremap+0x13/0x15
> [0.026964] [] efi_bgrt_init+0x162/0x1ad
> [0.026966] [] efi_late_init+0x9/0xb
> [0.026968] [] start_kernel+0x46f/0x49f
> [0.026970] [] ? early_idt_handler_array+0x120/0x120
> [0.026972] [] x86_64_start_reservations+0x2a/0x2c
> [0.026974] [] x86_64_start_kernel+0x14a/0x16d
> [0.026977] ---[ end trace f9b3812eb8e24c58 ]---
> [0.026978] efi_bgrt: Ignoring BGRT: failed to map image memory
>
> This is the second early_memremap call in efi_bgrt_init triggering
> WARN_ON(nrpages > NR_FIX_BTMAPS). Can you comment on this?
Hmm... yeah NR_FIX_BTMAPS == 64, so early_memremap() is limited to
mapping ~200KB at once. That's not very big, your BGRT data is likely
much larger than that.
Obviously we can't use efi_lookup_mapped_addr() any more, so it makes
sense to come up with a much robust way to memremap the BGRT image.
The immediate solution that comes to mind is using memremap() instead
of the early_* version, since the late version won't use the FIXMAP
area and will allow us to map a much larger region into the kernel
virtual address space.
Digging through the history it appears I was the one who made the
switch from ioremap() to early_memremap() in commit 081cd62a010f
("x86/efi: Allow mapping BGRT on x86-32"), which I suspect was because
a generic memremap() implementation didn't exist at the time. Dan
Williams introduced one in Aug 2015 with commit 92281dee825f ("arch:
introduce memremap()").
Somewhat surprisingly, Juergen switched the BGRT driver from
early_memremap() to early_ioremap() in commit 954e12f7a800 ("x86/mm,
efi: Use early_ioremap() in arch/x86/platform/efi/efi-bgrt.c")
although I can't figure out why because this isn't I/O memory. And now
we're attempting to switch it back.
Juergen, could you provide a rationale for commit 954e12f7a800? All
the commit message says is that it's "an I/O-area", but that isn't
true.
In the meantime Môshe, could you try this patch ontop of the EFI
'next' branch? (Note it may not work/compile, but you get the gist)
---
diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c
index bf51f4c02562..b0970661870a 100644
--- a/arch/x86/platform/efi/efi-bgrt.c
+++ b/arch/x86/platform/efi/efi-bgrt.c
@@ -72,14 +72,14 @@ void __init efi_bgrt_init(void)
return;
}
- image = early_memremap(bgrt_tab->image_address, sizeof(bmp_header));
+ image = memremap(bgrt_tab->image_address, sizeof(bmp_header),
MEMREMAP_WB);
if (!image) {
pr_err("Ignoring BGRT: failed to map image header memory\n");
return;
}
memcpy(&bmp_header, image, sizeof(bmp_header));
- early_memunmap(image, sizeof(bmp_header));
+ memunmap(image);
bgrt_image_size = bmp_header.size;
bgrt_image = kmalloc(bgrt_image_size, GFP_KERNEL | __GFP_NOWARN);
@@ -89,7 +89,7 @@ void __init efi_bgrt_init(void)
return;
}
- image = early_memremap(bgrt_tab->image_address, bmp_header.size);
+ image = memremap(bgrt_tab->image_address, bmp_header.size, MEMREMAP_WB);
if (!image) {
pr_err("Ignoring BGRT: failed to map image memory\n");
kfree(bgrt_image);
@@ -98,5 +98,5 @@ void __init efi_bgrt_init(void)
}
memcpy(bgrt_image, image, bgrt_image_size);
- early_memunmap(image, bmp_header.size);
+ memunmap(image);
}
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/4] efi/esrt: Don't preformat name
From: Rasmus Villemoes
kobject_init_and_add takes a format string+args, so there's no reason
to do this formatting in advance.
Signed-off-by: Rasmus Villemoes
Cc: Peter Jones
Signed-off-by: Matt Fleming
---
drivers/firmware/efi/esrt.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/firmware/efi/esrt.c b/drivers/firmware/efi/esrt.c
index 22c5285f7705..75feb3f5829b 100644
--- a/drivers/firmware/efi/esrt.c
+++ b/drivers/firmware/efi/esrt.c
@@ -167,14 +167,11 @@ static struct kset *esrt_kset;
static int esre_create_sysfs_entry(void *esre, int entry_num)
{
struct esre_entry *entry;
- char name[20];
entry = kzalloc(sizeof(*entry), GFP_KERNEL);
if (!entry)
return -ENOMEM;
- sprintf(name, "entry%d", entry_num);
-
entry->kobj.kset = esrt_kset;
if (esrt->fw_resource_version == 1) {
@@ -182,7 +179,7 @@ static int esre_create_sysfs_entry(void *esre, int
entry_num)
entry->esre.esre1 = esre;
rc = kobject_init_and_add(&entry->kobj, &esre1_ktype, NULL,
- "%s", name);
+ "entry%d", entry_num);
if (rc) {
kfree(entry);
return rc;
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/4] doc: efi-stub.txt: Fix arm64 paths
From: Alan Ott Update documented paths for arm64 files to match current tree. Signed-off-by: Alan Ott Cc: Roy Franz Cc: Jonathan Corbet Cc: Ard Biesheuvel Cc: Leif Lindholm Signed-off-by: Matt Fleming --- Documentation/efi-stub.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/efi-stub.txt b/Documentation/efi-stub.txt index 7747024d3bb7..e15746988261 100644 --- a/Documentation/efi-stub.txt +++ b/Documentation/efi-stub.txt @@ -10,12 +10,12 @@ arch/x86/boot/header.S and arch/x86/boot/compressed/eboot.c, respectively. For ARM the EFI stub is implemented in arch/arm/boot/compressed/efi-header.S and arch/arm/boot/compressed/efi-stub.c. EFI stub code that is shared -between architectures is in drivers/firmware/efi/efi-stub-helper.c. +between architectures is in drivers/firmware/efi/libstub. For arm64, there is no compressed kernel support, so the Image itself masquerades as a PE/COFF image and the EFI stub is linked into the kernel. The arm64 EFI stub lives in arch/arm64/kernel/efi-entry.S -and arch/arm64/kernel/efi-stub.c. +and drivers/firmware/efi/libstub/arm64-stub.c. By using the EFI boot stub it's possible to boot a Linux kernel without the use of a conventional EFI boot loader, such as grub or -- 2.6.2 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/4] x86/efi: Preface all print statements with efi* tag
The pr_*() calls in the x86 EFI code may or may not include a
subsystem tag, which makes it difficult to grep the kernel log for all
relevant EFI messages and leads users to miss important information.
Recently, a bug reporter provided all the EFI print messages from the
kernel log when trying to diagnose an issue but missed the following
statement because it wasn't prefixed with anything indicating it was
related to EFI,
pr_err("Error ident-mapping new memmap (0x%lx)!\n", pa_memmap);
Cc: Borislav Petkov
Reviewed-by: Josh Triplett
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/efi-bgrt.c | 3 +++
arch/x86/platform/efi/efi_64.c | 2 ++
arch/x86/platform/efi/quirks.c | 4 +++-
3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c
index ea48449b2e63..9a52b5c4438f 100644
--- a/arch/x86/platform/efi/efi-bgrt.c
+++ b/arch/x86/platform/efi/efi-bgrt.c
@@ -10,6 +10,9 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
#include
#include
#include
diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
index a0ac0f9c307f..d347e854a5e4 100644
--- a/arch/x86/platform/efi/efi_64.c
+++ b/arch/x86/platform/efi/efi_64.c
@@ -15,6 +15,8 @@
*
*/
+#define pr_fmt(fmt) "efi: " fmt
+
#include
#include
#include
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 1c7380da65ff..6452070f3025 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -1,3 +1,5 @@
+#define pr_fmt(fmt) "efi: " fmt
+
#include
#include
#include
@@ -256,7 +258,7 @@ void __init efi_apply_memmap_quirks(void)
* services.
*/
if (!efi_runtime_supported()) {
- pr_info("efi: Setup done, disabling due to 32/64-bit
mismatch\n");
+ pr_info("Setup done, disabling due to 32/64-bit mismatch\n");
efi_unmap_memmap();
}
--
2.6.2
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/4] x86/efi-bgrt: Fix kernel panic when mapping BGRT data
From: Sai Praneeth
Starting with this commit 35eb8b81edd4 ("x86/efi: Build our own page
table structures") efi regions have a separate page directory called
"efi_pgd". In order to access any efi region we have to first shift %cr3
to this page table. In the bgrt code we are trying to copy bgrt_header
and image, but these regions fall under "EFI_BOOT_SERVICES_DATA"
and to access these regions we have to shift %cr3 to efi_pgd and not
doing so will cause page fault as shown below.
[0.251599] Last level dTLB entries: 4KB 64, 2MB 0, 4MB 0, 1GB 4
[0.259126] Freeing SMP alternatives memory: 32K (8230e000 -
82316000)
[0.271803] BUG: unable to handle kernel paging request at fffefce35002
[0.279740] IP: [] efi_bgrt_init+0x144/0x1fd
[0.286383] PGD 300f067 PUD 0
[0.289879] Oops: [#1] SMP
[0.293566] Modules linked in:
[0.297039] CPU: 0 PID: 0 Comm: swapper/0 Not tainted
4.4.0-rc1-eywa-eywa-built-in-47041+ #2
[0.306619] Hardware name: Intel Corporation Skylake Client platform/Skylake
Y LPDDR3 RVP3, BIOS SKLSE2R1.R00.B104.B01.150114 11/11/2015
[0.320925] task: 820134c0 ti: 8200 task.ti:
8200
[0.329420] RIP: 0010:[] []
efi_bgrt_init+0x144/0x1fd
[0.338821] RSP: :82003f18 EFLAGS: 00010246
[0.344852] RAX: fffefce35000 RBX: fffefce35000 RCX: fffefce2b000
[0.352952] RDX: 8a82b000 RSI: 8235bb80 RDI: 8a835000
[0.361050] RBP: 82003f30 R08: 8a865000 R09: ff202850
[0.369149] R10: 811ad62f R11: R12:
[0.377248] R13: 88016dbaea40 R14: 822622c0 R15: 82003fb0
[0.385348] FS: () GS:88016d80()
knlGS:
[0.394533] CS: 0010 DS: ES: CR0: 80050033
[0.401054] CR2: fffefce35002 CR3: 0300c000 CR4: 003406f0
[0.409153] DR0: DR1: DR2:
[0.417252] DR3: DR6: fffe0ff0 DR7: 0400
[0.425350] Stack:
[0.427638] 82256900 88016dbaea40
82003f40
[0.436086] 821bbce0 82003f88 8219c0c2
[0.444533] 8219ba4a 822622c0 00083000
[0.452978] Call Trace:
[0.455763] [] efi_late_init+0x9/0xb
[0.461697] [] start_kernel+0x463/0x47f
[0.467928] [] ? set_init_arg+0x55/0x55
[0.474159] [] ? early_idt_handler_array+0x120/0x120
[0.481669] [] x86_64_start_reservations+0x2a/0x2c
[0.488982] [] x86_64_start_kernel+0x13d/0x14c
[0.495897] Code: 00 41 b4 01 48 8b 78 28 e8 09 36 01 00 48 85 c0 48 89 c3
75 13 48 c7 c7 f8 ac d3 81 31 c0 e8 d7 3b fb fe e9 b5 00 00 00 45 84 e4 <44> 8b
6b 02 74 0d be 06 00 00 00 48 89 df e8 ae 34 0$
[0.518151] RIP [] efi_bgrt_init+0x144/0x1fd
[0.524888] RSP
[0.528851] CR2: fffefce35002
[0.532615] ---[ end trace 7b06521e6ebf2aea ]---
[0.537852] Kernel panic - not syncing: Attempted to kill the idle task!
As said above one way to fix this bug is to shift %cr3 to efi_pgd but we
are not doing that way because it leaks inner details of how we switch
to EFI page tables into a new call site and it also adds duplicate code.
Instead, we remove the call to efi_lookup_mapped_addr() and always
perform early_mem*() instead of early_io*() because we want to remap RAM
regions and not I/O regions. We also delete efi_lookup_mapped_addr()
because we are no longer using it.
Signed-off-by: Sai Praneeth Prakhya
Reported-by: Wendy Wang
Cc: Borislav Petkov
Cc: Josh Triplett
Cc: Ricardo Neri
Cc: Ravi Shankar
Signed-off-by: Matt Fleming
---
arch/x86/platform/efi/efi-bgrt.c | 39 ++-
drivers/firmware/efi/efi.c | 32
2 files changed, 14 insertions(+), 57 deletions(-)
diff --git a/arch/x86/platform/efi/efi-bgrt.c b/arch/x86/platform/efi/efi-bgrt.c
index 9a52b5c4438f..bf51f4c02562 100644
--- a/arch/x86/platform/efi/efi-bgrt.c
+++ b/arch/x86/platform/efi/efi-bgrt.c
@@ -31,8 +31,7 @@ struct bmp_header {
void __init efi_bgrt_init(void)
{
acpi_status status;
- void __iomem *image;
- bool ioremapped = false;
+ void *image;
struct bmp_header bmp_header;
if (acpi_disabled)
@@ -73,20 +72,14 @@ void __init efi_bgrt_init(void)
return;
}
- image = efi_lookup_mapped_addr(bgrt_tab->image_address);
+ image = early_memremap(bgrt_tab->image_address, sizeof(bmp_header));
if (!image) {
- image = early_ioremap(bgrt_tab->image_address,
- sizeof(bmp_header));
- ioremapped = true;
- if (!image) {
- pr_err("I
[GIT PULL 0/4] EFI changes for v4.5
Folks, Here's a couple of cleanups and a bug fix for the ACPI BGRT driver triggered by the recent isolated EFI page table changes currently sitting in tip. The following changes since commit 8005c49d9aea74d382f474ce11afbbc7d7130bec: Linux 4.4-rc1 (2015-11-15 17:00:27 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git tags/efi-next for you to fetch changes up to 50a0cb565246f20d59cdb161778531e4b19d35ac: x86/efi-bgrt: Fix kernel panic when mapping BGRT data (2015-12-14 15:24:24 +) * We don't need to carry our own formatting code in the esrt driver because the kobject API can do that for us - Rasmus Villemoes * Update the arm64 file paths in Documentation/efi-stub.txt to match the current tree - Alan Ott * Consistently preface all print statements with "efi" arch/x86 so that it's more obvious to users reporting problems which statements in the kernel log are relevant for EFI - Matt Fleming * Fix a boot crash in the ACPI BGRT driver and delete efi_lookup_mapped_addr() since it's useless now that the EFI mappings *only* exist in the 'efi_pgd' page table. Instead we always early_memremap() the BGRT memory - Sai Praneeth Prakhya Alan Ott (1): doc: efi-stub.txt: Fix arm64 paths Matt Fleming (1): x86/efi: Preface all print statements with efi* tag Rasmus Villemoes (1): efi/esrt: Don't preformat name Sai Praneeth (1): x86/efi-bgrt: Fix kernel panic when mapping BGRT data Documentation/efi-stub.txt | 4 ++-- arch/x86/platform/efi/efi-bgrt.c | 42 arch/x86/platform/efi/efi_64.c | 2 ++ arch/x86/platform/efi/quirks.c | 4 +++- drivers/firmware/efi/efi.c | 32 -- drivers/firmware/efi/esrt.c | 5 + 6 files changed, 25 insertions(+), 64 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Remove EFI memmap quirk for UV2+
On Mon, 14 Dec, at 09:41:58AM, Ingo Molnar wrote: > > Ok, this looks good to me and I'll apply it if it looks good to Matt as well. Yep, looks OK to me. Feel free to pick it up directly, Reviewed-by: Matt Fleming -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86/efi-bgrt: Fix kernel panic when mapping BGRT data
On Fri, 11 Dec, at 05:58:09AM, Sai Praneeth Prakhya wrote: > > >The original motivation for efi_lookup_mapped_addr came from > >early_ioremap printing a warning if used on an address range > >already mapped as RAM. Does early_mem* handle that case correctly > >without a warning? > > Thanks a lot Josh for letting me know that. I don't think > early_memremap() does that because early_memremap() and > early_ioremap() both use __early_ioremap() but with different page > protections (and I am not sure how those protections effect warning, > but I will check that). Waiting for comments from Matt and Boris. Right, early_memremap() was introduced for the purpose of mapping RAM, so we shouldn't be seeing any warning here. > >Because not all firmware places the BGRT image in boot services > >memory; some firmware places the BGRT image variously in BIOS > >reserved memory, ACPI reclaim >space, or other strange places. > > > >- Josh Triplett > > I think we should not support buggy firmware implementations because > it's same as encouraging them, instead we could let user know that > he has got a buggy firmware and we skip bgrt code as if bgrt was > disabled and carry on with normal boot process. One way of detecting > buggy implementation without doing page table switch in > efi_bgrt_init() is to enable a chicken bit if we find bgrt header > and image address in EFI_BOOT_SERVICES_DATA regions while we are > switching efi runtime services to virtual mode in > __efi_enter_virtual_mode() and check for the same bit in > efi_bgrt_init(). Trying to inform firmware developers that we noticed some buggy behaviour should definitely be discussed as a separate patch (because it's debatable whether there's any value in that in this scenario). The changes in this patch look OK to me, and I think all of Josh's concerns have been addressed. So unless anyone complains soon, I'm going to apply it and send it to tip. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 7/7] efi: runtime-wrappers: run UEFI Runtime Services with interrupts enabled
On Tue, 01 Dec, at 11:50:20AM, Ard Biesheuvel wrote: > The UEFI spec allows Runtime Services to be invoked with interrupts > enabled. The only reason we were disabling interrupts was to prevent > recursive calls into the services on the same CPU, which will lead to > deadlock. However, the only context where such invocations may occur > legally is from efi-pstore via efivars, and that code has been updated > to call a non-blocking alternative when invoked from a non-interruptible > context. > > So instead, update the ordinary, blocking UEFI Runtime Services wrappers > to execute with interrupts enabled. This aims to prevent excessive interrupt > latencies on uniprocessor platforms with slow variable stores. > > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/runtime-wrappers.c | 73 > 1 file changed, 30 insertions(+), 43 deletions(-) Thanks Ard, this and all the other patches in this series look good to me, apart from the comments I had for patch 4. Once that is resolved I'd like to get this queued up for v4.5. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 4/7] efi: add nonblocking option to efi_query_variable_store()
On Tue, 01 Dec, at 11:50:17AM, Ard Biesheuvel wrote:
> The function efi_query_variable_store() may be invoked by
> efivar_entry_set_nonblocking(), which itself takes care to only call
> a non-blocking version of the SetVariable() runtime wrapper. However,
> efi_query_variable_store() may call the SetVariable() wrapper directly,
> as well as the wrapper for QueryVariableInfo(), both of which could
> deadlock in the same way we are trying to prevent by calling
> efivar_entry_set_nonblocking() in the first place.
>
> So instead, modify efi_query_variable_store() to use the non-blocking
> variants of both SetVariable() and QueryVariableInfo() if invoked with
> the 'nonblocking' argument set to true.
>
> Signed-off-by: Ard Biesheuvel
> ---
> arch/x86/platform/efi/quirks.c | 37 ++--
> drivers/firmware/efi/vars.c| 16 +++--
> include/linux/efi.h| 12 +--
> 3 files changed, 49 insertions(+), 16 deletions(-)
I get sad everytime I read the EFI vars code. Especially the bits I
wrote :-(
> diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
> index 1c7380da65ff..cbe542f5c75d 100644
> --- a/arch/x86/platform/efi/quirks.c
> +++ b/arch/x86/platform/efi/quirks.c
> @@ -60,16 +60,27 @@ void efi_delete_dummy_variable(void)
> * Return EFI_SUCCESS if it is safe to write 'size' bytes to the variable
> * store.
> */
> -efi_status_t efi_query_variable_store(u32 attributes, unsigned long size)
> +efi_status_t efi_query_variable_store(u32 attributes, unsigned long size,
> + bool nonblocking)
> {
> efi_status_t status;
> u64 storage_size, remaining_size, max_size;
> + efi_set_variable_t *set_variable;
> + efi_query_variable_info_t *query_variable_info;
>
> if (!(attributes & EFI_VARIABLE_NON_VOLATILE))
> return 0;
>
> - status = efi.query_variable_info(attributes, &storage_size,
> - &remaining_size, &max_size);
> + if (nonblocking) {
> + set_variable = efi.set_variable_nonblocking;
> + query_variable_info = efi.query_variable_info_nonblocking;
> + } else {
> + set_variable = efi.set_variable;
> + query_variable_info = efi.query_variable_info;
> + }
> +
> + status = query_variable_info(attributes, &storage_size,
> + &remaining_size, &max_size);
Could we just return early in the nonblocking case and skip the
attempted garbage collection, e.g.
diff --git a/arch/x86/platform/efi/quirks.c b/arch/x86/platform/efi/quirks.c
index 1c7380da65ff..4b170c522e8c 100644
--- a/arch/x86/platform/efi/quirks.c
+++ b/arch/x86/platform/efi/quirks.c
@@ -54,13 +54,40 @@ void efi_delete_dummy_variable(void)
}
/*
+ * In the nonblocking case we do not attempt to perform garbage
+ * collection if we do not have enough free space. Rather, we do the
+ * bare minimum check and give up immediately if the available space
+ * is below EFI_MIN_RESERVE.
+ *
+ * This function is intended to be small and simple because it is
+ * invoked from crash handler paths.
+ */
+static inline efi_status_t
+query_variable_store_nonblocking(u32 attributes, unsigned long size)
+{
+ efi_status_t status;
+ u64 storage_size, remaining_size, max_size;
+
+ status = efi.query_variable_info_nonblocking(attributes, &storage_size
+&remaining_size,
&max_size);
+ if (status != EFI_SUCCESS)
+ return status;
+
+ if (remaining_size - size < EFI_MIN_RESERVE)
+ return EFI_OUT_OF_RESOURCES;
+
+ return EFI_SUCCESS;
+}
+
+/*
* Some firmware implementations refuse to boot if there's insufficient space
* in the variable store. Ensure that we never use more than a safe limit.
*
* Return EFI_SUCCESS if it is safe to write 'size' bytes to the variable
* store.
*/
-efi_status_t efi_query_variable_store(u32 attributes, unsigned long size)
+efi_status_t efi_query_variable_store(u32 attributes, unsigned long size,
+ bool nonblocking)
{
efi_status_t status;
u64 storage_size, remaining_size, max_size;
@@ -68,6 +95,9 @@ efi_status_t efi_query_variable_store(u32 attributes,
unsigned long size)
if (!(attributes & EFI_VARIABLE_NON_VOLATILE))
return 0;
+ if (nonblocking)
+ return query_variable_store_nonblocking(attributes, size);
+
status = efi.query_variable_info(attributes, &storage_size,
&remaining_size, &max_size);
if (status != EFI_SUCCESS)
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/7] efi: runtime-wrapper: get rid of the rtc_lock spinlock
On Tue, 01 Dec, at 11:50:19AM, Ard Biesheuvel wrote: > The rtc_lock spinlock aims to serialize access to the CMOS RTC between > the UEFI firmware and the kernel drivers that use it directly. However, > x86 is the only arch that performs such direct accesses, and that never > uses the time related UEFI runtime services. Since no other UEFI enlightened > architectures have a legcay CMOS RTC anyway, we can remove the rtc_lock > spinlock entirely. > > Signed-off-by: Ard Biesheuvel > --- > drivers/firmware/efi/runtime-wrappers.c | 32 +--- > 1 file changed, 8 insertions(+), 24 deletions(-) Is this really true? It's not possible, for instance, for 32-bit ARM systems to use the rtc-cmos driver which would access the same physical device that UEFI would with the GetTime() service? With the pending 32-bit ARM UEFI support coming, this needs to be considered carefully. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware
On Mon, 07 Dec, at 11:10:43PM, Kosuke Tatsukawa wrote:
>
> Thank you pointing that out.
>
> linux-4.4-rc3 booted without a problem on a real server even with XD
> turned off by the firmware. I didn't notice this before because I was
> using an older version of the kernel on the real server, and doing
> investigation on a KVM guest.
>
> The "noexec=off" kernel parameter still seems to come up with EFI
> runtime service disabled though. Do you think this should be left alone
> as an disadvantage for using a bad option?
Good question.
I couldn't find any other examples of code that returns an error if
PAGE_NX isn't supported either by the hardware or via the noexec
command line option. Things like set_memory_x() and set_memory_nx()
return success without actually doing anything.
While I'm all for using hardware security features wherever possible,
such as force-enabling it in commit 04633df0c43d ("x86/cpu: Call
verify_cpu() after having entered long mode too"), if the user has
explicitly turned it off on the kernel command line, we should still
try and provide EFI services.
Borislav, what do you think about stripping PAGE_NX from 'page_flags'
in kernel_map_pages_in_pgd() if NX isn't supported, rather than
returning EINVAL? At least that way EFI runtime services would still
work.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] x86: Fix kernel panic when booting with XD disabled in uEFI firmware
On Thu, 03 Dec, at 11:58:33PM, Kosuke Tatsukawa wrote:
> The kernel panics early in boot on a x86_64 server if the eXecute
> Disable (XD) bit is set to disabled in the uEFI firmware. The message
> in the kernel log buffer looks like below.
>
> [0.00] CPU: 0 PID: 0 Comm: swapper Not tainted 4.4.0-rc3 #1
> [0.00] 261c6fa13723be1b 819b7e40
> 8131f320
> [0.00] 819b7f30 81b261b0
> 001c
> [0.00] 81d77a1c 0010 be35a000
> ff20
> [0.00] Call Trace:
> [0.00] [] dump_stack+0x44/0x64
> [0.00] [] early_idt_handler_common+0x90/0xd0
> [0.00] [] ? setup_arch+0x1f1/0xce0
> [0.00] [] ? setup_arch+0x1f1/0xce0
> [0.00] [] ? early_idt_handler_array+0x120/0x120
> [0.00] [] start_kernel+0xe6/0x4f0
> [0.00] [] ? early_idt_handler_array+0x120/0x120
> [0.00] [] ? early_idt_handler_array+0x120/0x120
> [0.00] [] x86_64_start_reservations+0x2a/0x2c
> [0.00] [] x86_64_start_kernel+0x14c/0x16f
> [0.00] RIP 0x8000be359163
>
>
> The panic occurs because __early_set_fixmap() called from
> parse_setup_data() unconditionally sets the PTE with FIXMAP_PAGE_NORMAL,
> which contains _PAGE_NX and causes an exception.
>
> This patch modifies __early_set_fixmap() to set _PAGE_NX only when the
> hardware supports it. It also moves the call to x86_configure_nx()
> earlier in setup_arch() before __early_set_fixmap() is first called.
>
> The above problem occurs after __early_set_fixmap() is called from
> parse_setup_data(). However, since setup_olpc_ofw_pgd() can also call
> __early_set_fixmap(), the patch moves the call to x86_configure_nx()
> before that.
>
> Signed-off-by: Kosuke Tatsukawa
> ---
> arch/x86/kernel/setup.c | 18 +-
> arch/x86/mm/ioremap.c |3 +++
> 2 files changed, 12 insertions(+), 9 deletions(-)
Could you try booting with the commit 04633df0c43d ("x86/cpu: Call
verify_cpu() after having entered long mode too") instead? It's part
of v4.4-rc1.
Allowing NX to be disabled should be avoided.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] doc: efi-stub.txt: Fix arm64 paths
On Mon, 30 Nov, at 09:36:54AM, Alan Ott wrote: > Update documented paths for arm64 files to match current tree. > > Signed-off-by: Alan Ott > --- > Documentation/efi-stub.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/efi-stub.txt b/Documentation/efi-stub.txt > index 7747024..e157469 100644 > --- a/Documentation/efi-stub.txt > +++ b/Documentation/efi-stub.txt > @@ -10,12 +10,12 @@ arch/x86/boot/header.S and > arch/x86/boot/compressed/eboot.c, > respectively. For ARM the EFI stub is implemented in > arch/arm/boot/compressed/efi-header.S and > arch/arm/boot/compressed/efi-stub.c. EFI stub code that is shared > -between architectures is in drivers/firmware/efi/efi-stub-helper.c. > +between architectures is in drivers/firmware/efi/libstub. > > For arm64, there is no compressed kernel support, so the Image itself > masquerades as a PE/COFF image and the EFI stub is linked into the > kernel. The arm64 EFI stub lives in arch/arm64/kernel/efi-entry.S > -and arch/arm64/kernel/efi-stub.c. > +and drivers/firmware/efi/libstub/arm64-stub.c. > > By using the EFI boot stub it's possible to boot a Linux kernel > without the use of a conventional EFI boot loader, such as grub or Thanks, applied for v4.5. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: efi-stub.txt: Fix arm64 paths
On Mon, 30 Nov, at 11:28:59AM, Matt Fleming wrote: > On Sat, 28 Nov, at 11:03:23AM, Alan Ott wrote: > > Update documented paths for arm64 files to match current tree. > > --- > > Documentation/efi-stub.txt | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/Documentation/efi-stub.txt b/Documentation/efi-stub.txt > > index 7747024..e157469 100644 > > --- a/Documentation/efi-stub.txt > > +++ b/Documentation/efi-stub.txt > > @@ -10,12 +10,12 @@ arch/x86/boot/header.S and > > arch/x86/boot/compressed/eboot.c, > > respectively. For ARM the EFI stub is implemented in > > arch/arm/boot/compressed/efi-header.S and > > arch/arm/boot/compressed/efi-stub.c. EFI stub code that is shared > > -between architectures is in drivers/firmware/efi/efi-stub-helper.c. > > +between architectures is in drivers/firmware/efi/libstub. > > > > For arm64, there is no compressed kernel support, so the Image itself > > masquerades as a PE/COFF image and the EFI stub is linked into the > > kernel. The arm64 EFI stub lives in arch/arm64/kernel/efi-entry.S > > -and arch/arm64/kernel/efi-stub.c. > > +and drivers/firmware/efi/libstub/arm64-stub.c. > > > > By using the EFI boot stub it's possible to boot a Linux kernel > > without the use of a conventional EFI boot loader, such as grub or > > Thanks Alan, applied. Umm.. actually could you resend this with your Signed-off-by tag, please? Then I'll apply it. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] doc: efi-stub.txt: Fix arm64 paths
On Sat, 28 Nov, at 11:03:23AM, Alan Ott wrote: > Update documented paths for arm64 files to match current tree. > --- > Documentation/efi-stub.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/efi-stub.txt b/Documentation/efi-stub.txt > index 7747024..e157469 100644 > --- a/Documentation/efi-stub.txt > +++ b/Documentation/efi-stub.txt > @@ -10,12 +10,12 @@ arch/x86/boot/header.S and > arch/x86/boot/compressed/eboot.c, > respectively. For ARM the EFI stub is implemented in > arch/arm/boot/compressed/efi-header.S and > arch/arm/boot/compressed/efi-stub.c. EFI stub code that is shared > -between architectures is in drivers/firmware/efi/efi-stub-helper.c. > +between architectures is in drivers/firmware/efi/libstub. > > For arm64, there is no compressed kernel support, so the Image itself > masquerades as a PE/COFF image and the EFI stub is linked into the > kernel. The arm64 EFI stub lives in arch/arm64/kernel/efi-entry.S > -and arch/arm64/kernel/efi-stub.c. > +and drivers/firmware/efi/libstub/arm64-stub.c. > > By using the EFI boot stub it's possible to boot a Linux kernel > without the use of a conventional EFI boot loader, such as grub or Thanks Alan, applied. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] efi: efivars: don't rely on blocking operations in non-blocking set_var()
On Fri, 27 Nov, at 09:18:36PM, Matt Fleming wrote: > > I *think* that'd be OK. The thing I wanted to avoid was a > proliferation of nonblocking versions of the standard EFI calls, but > limiting it to adding query_variable_info() doesn't seem too bad. > > Let me think about it over the weekend. I couldn't come up with anything better. Go for it! -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 00/13] UEFI boot and runtime services support for 32-bit ARM
On Mon, 23 Nov, at 10:06:20AM, Ard Biesheuvel wrote: > This series adds support for booting the 32-bit ARM kernel directly from > UEFI firmware using a builtin UEFI stub. It mostly reuses refactored arm64 > code, and the differences (primarily the PE/COFF header and entry point and > the efi_create_mapping() implementation) are split out into arm64 and ARM > versions. For the series, Reviewed-by: Matt Fleming Ard, I think the next EFI area for refactoring could be the pgtable switching code, since we've now got 3 architectures doing it. If I can find some time in the new year I'll add a fourth (i386) and try and pull out the common code. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 13/13] ARM: add UEFI stub support
On Fri, 27 Nov, at 10:38:05AM, Ard Biesheuvel wrote: > > Actually, it is the reservation done a bit earlier that could > potentially end up at 0x0, and the [compressed] kernel is always at > least 32 MB up in memory, so that it can be decompressed as close to > the base of DRAM as possible. > > As far as I can tell, efi_free() deals correctly with allocations at > address 0x0, and that is the only dealing we have with the > reservation. So I don't think there is an issue here. OK, great. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] efi: efivars: don't rely on blocking operations in non-blocking set_var()
On Thu, 26 Nov, at 01:06:27PM, Ard Biesheuvel wrote: > > How would this be affected by things like suspend/resume, hibernation > or variable accesses made in SMM context (i.e., which the kernel knows > nothing about) > If we need to take all of that into account as well, we're probably > better off adding a nonblocking query_variable_info() call after all, > and wiring it up to efi_query_variable_store(), i.e., make it take a > 'bool nonblocking' argument and choose between the two versions of > each of the hooks it uses. I *think* that'd be OK. The thing I wanted to avoid was a proliferation of nonblocking versions of the standard EFI calls, but limiting it to adding query_variable_info() doesn't seem too bad. Let me think about it over the weekend. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
