* Re: EFI regression for efi=noruntime support
From: Branden Sherrell @ 2020-09-14 19:08 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi, linux-rockchip
In-Reply-To: <CAMj1kXEmw08Sed7CfgzBcjD1-WSNW9=Z27-0m4UKdfR5VCmFGg@mail.gmail.com>
That fixed it. Thanks for the quick look and patch.
Branden
> On Sep 14, 2020, at 2:27 PM, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 14 Sep 2020 at 20:15, Branden Sherrell <sherrellbc@gmail.com> wrote:
>>
>> An EFI-related regression appears to have made its way in from f88814cc2578c121e6edef686365036db72af0ed:
>>
>> Author: Ard Biesheuvel <ardb@kernel.org> Date: Wed Jul 8 13:01:57 2020 +0300 efi/efivars:
>> Expose RT service availability via efivars abstraction
>>
>>
>> On the RockChip rk3399 (ARMv8) port the EFI subsystem claims the reboot callback despite no runtime support. Ordinarily one might use efi=noruntime or noefi to disable this, but the boot augment appears to now be outright ignored (on this system, at least). You might imagine this causes issue on a uboot'd system that does not have runtime services. The manifestation of this bug is an iabt with PC=0 originating in efivar_entry_set_safe. This function eventually attempts to call the NULL entry ops->set_variable.
>>
>> [ 41.231673] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
>> [ 41.232436] Mem abort info:
>> [ 41.232682] ESR = 0x86000004
>> [ 41.232951] EC = 0x21: IABT (current EL), IL = 32 bits
>> [ 41.233413] SET = 0, FnV = 0
>> [ 41.233681] EA = 0, S1PTW = 0
>> [ 41.233956] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000f02eb000
>> [ 41.234516] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
>> [ 41.235111] Internal error: Oops: 86000004 [#1] SMP
>> [ 41.235536] Modules linked in: rt2800usb rt2x00usb rt2800lib rt2x00lib rc_cec mac80211 snd_soc_hdmi_codec dw_hdmi_i2s_audio dw_hdmi_cec rockchipdrm realtek dw_mipi_dsi hantro_vpu(C) dw_hdmi hci_uart rockchip_vdec(C) rockchip_rga analogix_dp cfg80211 cec btqca btbcm v4l2_h264 videobuf2_dma_sg pwm_fan btintel rc_core v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig bluetooth dwmac_rk videobuf2_memops videobuf2_v4l2 stmmac_platform videobuf2_common libarc4 drm_kms_helper snd_soc_audio_graph_card snd_soc_simple_card stmmac snd_soc_simple_card_utils syscopyarea sysfillrect sysimgblt panfrost fb_sys_fops ecdh_generic mdio_xpcs videodev phylink gpu_sched ecc snd_soc_rockchip_i2s rfkill mc snd_soc_rockchip_pcm dw_wdt rockchip_thermal rtc_rk808 rockchip_saradc drm gpio_keys
>> [ 41.241462] CPU: 5 PID: 1 Comm: shutdown Tainted: G C 5.8.8-1-ARCH #1
>> [ 41.242135] Hardware name: pine64 rockpro64_rk3399/rockpro64_rk3399, BIOS 2020.10-rc4-dirty 09/11/2020
>> [ 41.242944] pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
>> [ 41.243431] pc : 0x0
>> [ 41.243631] lr : efivar_entry_set_safe+0xc8/0x1a0
>> [ 41.244041] sp : ffff80001254bbe0
>> [ 41.244331] x29: ffff80001254bbe0 x28: ffff0000f2f8e3c0
>> [ 41.244794] x27: 0000000000000000 x26: ffff0000f11af418
>> [ 41.245257] x25: ffff8000124c0000 x24: ffff8000124c0b50
>> [ 41.245720] x23: ffff0000f11af418 x22: ffff800012387230
>> [ 41.246183] x21: 0000000000000007 x20: ffff0000f11af000
>> [ 41.246646] x19: 000000000000000e x18: 0000000000000030
>> [ 41.247109] x17: 0000000000000000 x16: 0000000000000000
>> [ 41.247572] x15: ffff0000f2f8e8e0 x14: ffffffffffffffff
>> [ 41.248034] x13: ffff80009254b9e7 x12: 0000000000000030
>> [ 41.248497] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
>> [ 41.248960] x9 : ffff800010d8bb94 x8 : 41cf0a4c4a67b082
>> [ 41.249423] x7 : ffff8000124c0b78 x6 : ffff0000f11af418
>> [ 41.249886] x5 : 0000000000000000 x4 : ffff0000f11af418
>> [ 41.250349] x3 : 000000000000000e x2 : 0000000000000007
>> [ 41.250811] x1 : ffff80001254bc30 x0 : ffff0000f11af000
>> [ 41.251275] Call trace:
>> [ 41.251491] 0x0
>> [ 41.251655] efibc_set_variable+0xf0/0x170
>> [ 41.252013] efibc_reboot_notifier_call+0x44/0x80
>> [ 41.252427] blocking_notifier_call_chain+0x78/0xb0
>> [ 41.252852] __do_sys_reboot+0x1cc/0x290
>> [ 41.253195] __arm64_sys_reboot+0x30/0x40
>> [ 41.253547] el0_svc_common.constprop.0+0x7c/0x140
>> [ 41.253965] do_el0_svc+0x28/0xb0
>> [ 41.254257] el0_sync_handler+0x98/0x278
>> [ 41.254599] el0_sync+0x158/0x180
>> [ 41.254891] Code: bad PC value
>> [ 41.255160] ---[ end trace 1fafcf21a783a644 ]---
>> [ 41.255617] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>> [ 41.256283] SMP: stopping secondary CPUs
>> [ 41.256735] Kernel Offset: disabled
>> [ 41.257040] CPU features: 0x240022,21006008
>> [ 41.257406] Memory Limit: none
>> [ 41.257681] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
>>
>> I did not spend too much time poking around, but I did note that if one builds a kernel that forces the efibc_init to return ENODEV to prevent reboot registration, the system will properly consult PSCI to handle the event (as is expected for the rk3399 per the device tree). This observed behavior is accurate at least through 5.9.0-rc4. A test of the previous commit (8778eb0927ddcd3f431805c37b78fa56481aeed9) confirms an uneventful reboot for this device.
>> The board configuration is the RockPro64, which has a device tree present in the kernel sources. My configuration in particular was using the latest Grub to EFI boot Linux after having itself been loaded by uboot.
>>
>>
>
> This looks like an oversight in the efibc driver.
>
> Does the change below fix your issue?
>
> diff --git a/drivers/firmware/efi/efibc.c b/drivers/firmware/efi/efibc.c
> index 35dccc88ac0a..15a47539dc56 100644
> --- a/drivers/firmware/efi/efibc.c
> +++ b/drivers/firmware/efi/efibc.c
> @@ -84,7 +84,7 @@ static int __init efibc_init(void)
> {
> int ret;
>
> - if (!efi_enabled(EFI_RUNTIME_SERVICES))
> + if (!efivars_kobject() || !efivar_supports_writes())
> return -ENODEV;
>
> ret = register_reboot_notifier(&efibc_reboot_notifier);
^ permalink raw reply
* RE: [RFC PATCH 1/2] efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
From: Limonciello, Mario @ 2020-09-14 18:45 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Arvind Sankar, Jacobo Pantoja, Peter Jones, linux-efi
In-Reply-To: <CAMj1kXFueikQBrv1xS-Csz+kHfjA+diaZ4ut2FZSO9JXxBPPhA@mail.gmail.com>
> > Should re-order to reverse xmas tree order.
> >
>
> We have no such requirement in the EFI subsystem.
>
My apologies, I didn't realize that this subsystem didn't use that.
> > > +
> > > + if (size < offsetof(efi_load_option_t, variable_data))
> > > + return false;
> > > + pos = src->variable_data;
> > > + size -= offsetof(efi_load_option_t, variable_data);
> > > +
> > > + if ((src->attributes & ~EFI_LOAD_OPTION_MASK) != 0)
> > > + return false;
> > > +
> > > + /* Scan description. */
> > > + description = pos;
> > > + do {
> > > + if (size < sizeof(c))
> > > + return false;
> > > + c = *(const u16 *)pos;
> > > + pos += sizeof(c);
> > > + size -= sizeof(c);
> > > + } while (c != L'\0');
> > > +
> > > + /* Scan file_path_list. */
> > > + file_path_list = pos;
> > > + do {
> > > + if (size < sizeof(header))
> > > + return false;
> > > + header = *(const efi_device_path_protocol_t *)pos;
> > > + if (header.length < sizeof(header))
> > > + return false;
> > > + if (size < header.length)
> > > + return false;
> > > + pos += header.length;
> > > + size -= header.length;
> > > + } while ((header.type != EFI_DEV_END_PATH && header.type !=
> > > EFI_DEV_END_PATH2) ||
> > > + (header.sub_type != EFI_DEV_END_ENTIRE));
> > > + if (pos != (const void *)file_path_list + src-
> >file_path_list_length)
> > > + return false;
> > > +
> > > + dest->attributes = src->attributes;
> > > + dest->file_path_list_length = src->file_path_list_length;
> > > + dest->description = description;
> > > + dest->file_path_list = file_path_list;
> > > + dest->optional_data_size = size;
> > > + dest->optional_data = size ? pos : NULL;
> > > +
> > > + return true;
> > > +}
> > > +
> > > +/*
> > > + * At least some versions of Dell firmware pass the entire contents of
> the
> > > + * Boot#### variable, i.e. the EFI_LOAD_OPTION descriptor, rather than
> just
> > > the
> > > + * OptionalData field.
> > > + *
> > > + * Detect this case and extract OptionalData.
> > > + */
> > > +void efi_apply_loadoptions_quirk(const void **load_options, int
> > > *load_options_size)
> > > +{
> > > + const efi_load_option_t *load_option = *load_options;
> > > + efi_load_option_unpacked_t load_option_unpacked;
> > > +
> > > + if (!IS_ENABLED(CONFIG_X86))
> > > + return;
> > > + if (!load_option)
> > > + return;
> > > + if (*load_options_size < sizeof(*load_option))
> > > + return;
> > > + if ((load_option->attributes & ~EFI_LOAD_OPTION_BOOT_MASK) != 0)
> > > + return;
> > > +
> > > + if (!efi_load_option_unpack(&load_option_unpacked, load_option,
> > > *load_options_size))
> > > + return;
> > > +
> >
> > In case this was ever to be attributed to a cause for someone to fail to
> > boot, it may be useful to drop a pr_debug here that could be easily turned
> > on.
> >
>
> Agree that adding a efi_info() here makes sense, not only for
> potential failures, but also simply to have some confirmation that the
> quirk is taking effect.
Should a change be developed in the firmware side to resolve it, it's a nice way to
confirm that it landed properly too, and to come up with a timeline to eventually
sunset this type of quirk.
This is some precedence of putting stuff like this which Linux kernel has worked around
what is generally viewed as a firmware bug as pr_notice_once (*):
https://github.com/torvalds/linux/blob/89d57dddd7d319ded00415790a0bb3c954b7e386/drivers/acpi/osi.c#L77
Stuff of the like can flow into test tools like the firmware test suite (FWTS) which can
help identify them earlier and to resolve during development before codebases generally lock down.
>
> Note that I am not 100% convinced yet that we need this change to
> begin with. How large is the intersection of the set of affected
> systems and the set of systems that are likely to run future kernels
> that carry this quirk?
Right now there is no confirmation that "current" systems have been fixed in the
current firmware codebase. I don't have access to that codebase, so I have some inquiries
to those that do.
So it's anywhere in between "all Dell X86 client systems manufactured in 2017 and older" to
"all Dell systems up until 2020-2021 when/if this behavior is changed".
A lot of distributions uses other bootloaders, and won't be affected by this.
Even if it does get fixed in firmware (which I'll lobby for internally if it's still a problem)
I would personally rather see the quirk land as I think it's harder to see distributions have
the potential to migrate away from GRUB to other solutions with large numbers of known machines
in the field that can't boot the other solution.
>
>
> > > + *load_options = load_option_unpacked.optional_data;
> > > + *load_options_size = load_option_unpacked.optional_data_size;
> > > +}
> > > +
> > > /*
> > > * Convert the unicode UEFI command line to ASCII to pass to kernel.
> > > * Size of memory allocated return in *cmd_line_len.
> > > @@ -247,12 +341,15 @@ char *efi_convert_cmdline(efi_loaded_image_t *image,
> int
> > > *cmd_line_len)
> > > {
> > > const u16 *s2;
> > > unsigned long cmdline_addr = 0;
> > > - int options_chars = efi_table_attr(image, load_options_size) / 2;
> > > + int options_chars = efi_table_attr(image, load_options_size);
> > > const u16 *options = efi_table_attr(image, load_options);
> > > int options_bytes = 0, safe_options_bytes = 0; /* UTF-8 bytes */
> > > bool in_quote = false;
> > > efi_status_t status;
> > >
> > > + efi_apply_loadoptions_quirk((const void **)&options,
> &options_chars);
> > > + options_chars /= sizeof(*options);
> > > +
> > > if (options) {
> > > s2 = options;
> > > while (options_bytes < COMMAND_LINE_SIZE && options_chars--)
> {
> > > diff --git a/drivers/firmware/efi/libstub/efistub.h
> > > b/drivers/firmware/efi/libstub/efistub.h
> > > index 85050f5a1b28..589d07acb22d 100644
> > > --- a/drivers/firmware/efi/libstub/efistub.h
> > > +++ b/drivers/firmware/efi/libstub/efistub.h
> > > @@ -688,6 +688,35 @@ union efi_load_file_protocol {
> > > } mixed_mode;
> > > };
> > >
> > > +typedef struct {
> > > + u32 attributes;
> > > + u16 file_path_list_length;
> > > + u8 variable_data[];
> > > + // efi_char16_t description[];
> > > + // efi_device_path_protocol_t file_path_list[];
> > > + // u8 optional_data[];
> > > +} __packed efi_load_option_t;
> > > +
> > > +#define EFI_LOAD_OPTION_ACTIVE 0x0001U
> > > +#define EFI_LOAD_OPTION_FORCE_RECONNECT 0x0002U
> > > +#define EFI_LOAD_OPTION_HIDDEN 0x0008U
> > > +#define EFI_LOAD_OPTION_CATEGORY 0x1f00U
> > > +#define EFI_LOAD_OPTION_CATEGORY_BOOT 0x0000U
> > > +#define EFI_LOAD_OPTION_CATEGORY_APP 0x0100U
> > > +
> > > +#define EFI_LOAD_OPTION_BOOT_MASK \
> > > +
> (EFI_LOAD_OPTION_ACTIVE|EFI_LOAD_OPTION_HIDDEN|EFI_LOAD_OPTION_CATEGORY)
> > > +#define EFI_LOAD_OPTION_MASK
> > > (EFI_LOAD_OPTION_FORCE_RECONNECT|EFI_LOAD_OPTION_BOOT_MASK)
> > > +
> > > +typedef struct {
> > > + u32 attributes;
> > > + u16 file_path_list_length;
> > > + const efi_char16_t *description;
> > > + const efi_device_path_protocol_t *file_path_list;
> > > + size_t optional_data_size;
> > > + const void *optional_data;
> > > +} efi_load_option_unpacked_t;
> > > +
> > > void efi_pci_disable_bridge_busmaster(void);
> > >
> > > typedef efi_status_t (*efi_exit_boot_map_processing)(
> > > @@ -730,6 +759,8 @@ __printf(1, 2) int efi_printk(char const *fmt, ...);
> > >
> > > void efi_free(unsigned long size, unsigned long addr);
> > >
> > > +void efi_apply_loadoptions_quirk(const void **load_options, int
> > > *load_options_size);
> > > +
> > > char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len);
> > >
> > > efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
> > > diff --git a/drivers/firmware/efi/libstub/file.c
> > > b/drivers/firmware/efi/libstub/file.c
> > > index 630caa6b1f4c..4e81c6077188 100644
> > > --- a/drivers/firmware/efi/libstub/file.c
> > > +++ b/drivers/firmware/efi/libstub/file.c
> > > @@ -136,7 +136,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t
> > > *image,
> > > unsigned long *load_size)
> > > {
> > > const efi_char16_t *cmdline = image->load_options;
> > > - int cmdline_len = image->load_options_size / 2;
> > > + int cmdline_len = image->load_options_size;
> > > unsigned long efi_chunk_size = ULONG_MAX;
> > > efi_file_protocol_t *volume = NULL;
> > > efi_file_protocol_t *file;
> > > @@ -148,6 +148,9 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t
> > > *image,
> > > if (!load_addr || !load_size)
> > > return EFI_INVALID_PARAMETER;
> > >
> > > + efi_apply_loadoptions_quirk((const void **)&cmdline, &cmdline_len);
> > > + cmdline_len /= sizeof(*cmdline);
> > > +
> > > if (IS_ENABLED(CONFIG_X86) && !efi_nochunk)
> > > efi_chunk_size = EFI_READ_CHUNK_SIZE;
> > >
> > > --
> > > 2.26.2
> >
^ permalink raw reply
* Re: [PATCH v7 5/9] RISC-V: Add PE/COFF header for EFI stub
From: Ard Biesheuvel @ 2020-09-14 18:32 UTC (permalink / raw)
To: Atish Patra
Cc: Atish Patra, linux-efi, Anup Patel, Masahiro Yamada, Anup Patel,
Nick Desaulniers, Linux Kernel Mailing List, Arvind Sankar,
Palmer Dabbelt, Alistair Francis, Paul Walmsley, Greentime Hu,
linux-riscv, Ingo Molnar, Mike Rapoport
In-Reply-To: <CAOnJCUJhxLVNi74F7n6+ZaqkL36urh+4oybozmK4ypgD0-=Cxw@mail.gmail.com>
On Sat, 12 Sep 2020 at 05:04, Atish Patra <atishp@atishpatra.org> wrote:
>
> On Fri, Sep 11, 2020 at 6:09 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Fri, 28 Aug 2020 at 20:20, Atish Patra <atish.patra@wdc.com> wrote:
> > >
> > > Linux kernel Image can appear as an EFI application With appropriate
> > > PE/COFF header fields in the beginning of the Image header. An EFI
> > > application loader can directly load a Linux kernel Image and an EFI
> > > stub residing in kernel can boot Linux kernel directly.
> > >
> > > Add the necessary PE/COFF header.
> > >
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > > Link: https://lore.kernel.org/r/20200421033336.9663-3-atish.patra@wdc.com
> > > [ardb: - use C prefix for c.li to ensure the expected opcode is emitted
> > > - align all image sections according to PE/COFF section alignment ]
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > Reviewed-by: Anup Patel <anup@brainfault.org>
> >
> > Since you need to respin this anyway, one comment below on a thing
> > that I spotted while revisiting these patches.
> >
> > > ---
> > > arch/riscv/include/asm/sections.h | 13 ++++
> > > arch/riscv/kernel/Makefile | 4 ++
> > > arch/riscv/kernel/efi-header.S | 104 ++++++++++++++++++++++++++++++
> > > arch/riscv/kernel/head.S | 16 +++++
> > > arch/riscv/kernel/image-vars.h | 51 +++++++++++++++
> > > arch/riscv/kernel/vmlinux.lds.S | 22 ++++++-
> > > 6 files changed, 208 insertions(+), 2 deletions(-)
> > > create mode 100644 arch/riscv/include/asm/sections.h
> > > create mode 100644 arch/riscv/kernel/efi-header.S
> > > create mode 100644 arch/riscv/kernel/image-vars.h
> > >
> > > diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
> > > new file mode 100644
> > > index 000000000000..3a9971b1210f
> > > --- /dev/null
> > > +++ b/arch/riscv/include/asm/sections.h
> > > @@ -0,0 +1,13 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> > > + */
> > > +#ifndef __ASM_SECTIONS_H
> > > +#define __ASM_SECTIONS_H
> > > +
> > > +#include <asm-generic/sections.h>
> > > +
> > > +extern char _start[];
> > > +extern char _start_kernel[];
> > > +
> > > +#endif /* __ASM_SECTIONS_H */
> > > diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> > > index dc93710f0b2f..41e3895a3192 100644
> > > --- a/arch/riscv/kernel/Makefile
> > > +++ b/arch/riscv/kernel/Makefile
> > > @@ -31,6 +31,10 @@ obj-y += cacheinfo.o
> > > obj-y += patch.o
> > > obj-$(CONFIG_MMU) += vdso.o vdso/
> > >
> > > +OBJCOPYFLAGS := --prefix-symbols=__efistub_
> > > +$(obj)/%.stub.o: $(obj)/%.o FORCE
> > > + $(call if_changed,objcopy)
> > > +
> > > obj-$(CONFIG_RISCV_M_MODE) += traps_misaligned.o
> > > obj-$(CONFIG_FPU) += fpu.o
> > > obj-$(CONFIG_SMP) += smpboot.o
> > > diff --git a/arch/riscv/kernel/efi-header.S b/arch/riscv/kernel/efi-header.S
> > > new file mode 100644
> > > index 000000000000..822b4c9ff2bb
> > > --- /dev/null
> > > +++ b/arch/riscv/kernel/efi-header.S
> > > @@ -0,0 +1,104 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> > > + * Adapted from arch/arm64/kernel/efi-header.S
> > > + */
> > > +
> > > +#include <linux/pe.h>
> > > +#include <linux/sizes.h>
> > > +
> > > + .macro __EFI_PE_HEADER
> > > + .long PE_MAGIC
> > > +coff_header:
> > > +#ifdef CONFIG_64BIT
> > > + .short IMAGE_FILE_MACHINE_RISCV64 // Machine
> > > +#else
> > > + .short IMAGE_FILE_MACHINE_RISCV32 // Machine
> > > +#endif
> > > + .short section_count // NumberOfSections
> > > + .long 0 // TimeDateStamp
> > > + .long 0 // PointerToSymbolTable
> > > + .long 0 // NumberOfSymbols
> > > + .short section_table - optional_header // SizeOfOptionalHeader
> > > + .short IMAGE_FILE_DEBUG_STRIPPED | \
> > > + IMAGE_FILE_EXECUTABLE_IMAGE | \
> > > + IMAGE_FILE_LINE_NUMS_STRIPPED // Characteristics
> > > +
> > > +optional_header:
> > > + .short PE_OPT_MAGIC_PE32PLUS // PE32+ format
> >
> > Are you sure both riscv32 and riscv64 use PE32+? IIUC, 32-bit
> > architectures use PE32 not PE32+ (but I could be wrong)
> >
>
> Ahh yes. You are correct. Thanks for noticing it.
> I just followed the U-Boot implementation [1]. I will update this in
> the next revision and update the U-Boot code as well.
>
> As per the specification, we also need to add a BaseOfData[2] entry for RV32.
> Does any of the efi application loader actually use BaseOfData or can
> we set it to zero for RV32 ?
>
Just do whatever ARM and 32-bit x86 do.
> [1] https://gitlab.denx.de/u-boot/custodians/u-boot-amlogic/-/blob/u-boot-amlogic/arch/riscv/lib/crt0_riscv_efi.S#L51
> [2] https://docs.microsoft.com/en-us/windows/win32/debug/pe-format
>
> > > + .byte 0x02 // MajorLinkerVersion
> > > + .byte 0x14 // MinorLinkerVersion
> > > + .long __pecoff_text_end - efi_header_end // SizeOfCode
> > > + .long __pecoff_data_virt_size // SizeOfInitializedData
> > > + .long 0 // SizeOfUninitializedData
> > > + .long __efistub_efi_pe_entry - _start // AddressOfEntryPoint
> > > + .long efi_header_end - _start // BaseOfCode
> > > +
> > > +extra_header_fields:
> > > + .quad 0 // ImageBase
> > > + .long PECOFF_SECTION_ALIGNMENT // SectionAlignment
> > > + .long PECOFF_FILE_ALIGNMENT // FileAlignment
> > > + .short 0 // MajorOperatingSystemVersion
> > > + .short 0 // MinorOperatingSystemVersion
> > > + .short LINUX_EFISTUB_MAJOR_VERSION // MajorImageVersion
> > > + .short LINUX_EFISTUB_MINOR_VERSION // MinorImageVersion
> > > + .short 0 // MajorSubsystemVersion
> > > + .short 0 // MinorSubsystemVersion
> > > + .long 0 // Win32VersionValue
> > > +
> > > + .long _end - _start // SizeOfImage
> > > +
> > > + // Everything before the kernel image is considered part of the header
> > > + .long efi_header_end - _start // SizeOfHeaders
> > > + .long 0 // CheckSum
> > > + .short IMAGE_SUBSYSTEM_EFI_APPLICATION // Subsystem
> > > + .short 0 // DllCharacteristics
> > > + .quad 0 // SizeOfStackReserve
> > > + .quad 0 // SizeOfStackCommit
> > > + .quad 0 // SizeOfHeapReserve
> > > + .quad 0 // SizeOfHeapCommit
> > > + .long 0 // LoaderFlags
> > > + .long (section_table - .) / 8 // NumberOfRvaAndSizes
> > > +
> > > + .quad 0 // ExportTable
> > > + .quad 0 // ImportTable
> > > + .quad 0 // ResourceTable
> > > + .quad 0 // ExceptionTable
> > > + .quad 0 // CertificationTable
> > > + .quad 0 // BaseRelocationTable
> > > +
> > > + // Section table
> > > +section_table:
> > > + .ascii ".text\0\0\0"
> > > + .long __pecoff_text_end - efi_header_end // VirtualSize
> > > + .long efi_header_end - _start // VirtualAddress
> > > + .long __pecoff_text_end - efi_header_end // SizeOfRawData
> > > + .long efi_header_end - _start // PointerToRawData
> > > +
> > > + .long 0 // PointerToRelocations
> > > + .long 0 // PointerToLineNumbers
> > > + .short 0 // NumberOfRelocations
> > > + .short 0 // NumberOfLineNumbers
> > > + .long IMAGE_SCN_CNT_CODE | \
> > > + IMAGE_SCN_MEM_READ | \
> > > + IMAGE_SCN_MEM_EXECUTE // Characteristics
> > > +
> > > + .ascii ".data\0\0\0"
> > > + .long __pecoff_data_virt_size // VirtualSize
> > > + .long __pecoff_text_end - _start // VirtualAddress
> > > + .long __pecoff_data_raw_size // SizeOfRawData
> > > + .long __pecoff_text_end - _start // PointerToRawData
> > > +
> > > + .long 0 // PointerToRelocations
> > > + .long 0 // PointerToLineNumbers
> > > + .short 0 // NumberOfRelocations
> > > + .short 0 // NumberOfLineNumbers
> > > + .long IMAGE_SCN_CNT_INITIALIZED_DATA | \
> > > + IMAGE_SCN_MEM_READ | \
> > > + IMAGE_SCN_MEM_WRITE // Characteristics
> > > +
> > > + .set section_count, (. - section_table) / 40
> > > +
> > > + .balign 0x1000
> > > +efi_header_end:
> > > + .endm
> > > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > > index c6a37e8231a8..3631147732ee 100644
> > > --- a/arch/riscv/kernel/head.S
> > > +++ b/arch/riscv/kernel/head.S
> > > @@ -13,6 +13,7 @@
> > > #include <asm/csr.h>
> > > #include <asm/hwcap.h>
> > > #include <asm/image.h>
> > > +#include "efi-header.S"
> > >
> > > __HEAD
> > > ENTRY(_start)
> > > @@ -22,10 +23,18 @@ ENTRY(_start)
> > > * Do not modify it without modifying the structure and all bootloaders
> > > * that expects this header format!!
> > > */
> > > +#ifdef CONFIG_EFI
> > > + /*
> > > + * This instruction decodes to "MZ" ASCII required by UEFI.
> > > + */
> > > + c.li s4,-13
> > > + j _start_kernel
> > > +#else
> > > /* jump to start kernel */
> > > j _start_kernel
> > > /* reserved */
> > > .word 0
> > > +#endif
> > > .balign 8
> > > #if __riscv_xlen == 64
> > > /* Image load offset(2MB) from start of RAM */
> > > @@ -43,7 +52,14 @@ ENTRY(_start)
> > > .ascii RISCV_IMAGE_MAGIC
> > > .balign 4
> > > .ascii RISCV_IMAGE_MAGIC2
> > > +#ifdef CONFIG_EFI
> > > + .word pe_head_start - _start
> > > +pe_head_start:
> > > +
> > > + __EFI_PE_HEADER
> > > +#else
> > > .word 0
> > > +#endif
> > >
> > > .align 2
> > > #ifdef CONFIG_MMU
> > > diff --git a/arch/riscv/kernel/image-vars.h b/arch/riscv/kernel/image-vars.h
> > > new file mode 100644
> > > index 000000000000..8c212efb37a6
> > > --- /dev/null
> > > +++ b/arch/riscv/kernel/image-vars.h
> > > @@ -0,0 +1,51 @@
> > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > +/*
> > > + * Copyright (C) 2020 Western Digital Corporation or its affiliates.
> > > + * Linker script variables to be set after section resolution, as
> > > + * ld.lld does not like variables assigned before SECTIONS is processed.
> > > + * Based on arch/arm64/kerne/image-vars.h
> > > + */
> > > +#ifndef __RISCV_KERNEL_IMAGE_VARS_H
> > > +#define __RISCV_KERNEL_IMAGE_VARS_H
> > > +
> > > +#ifndef LINKER_SCRIPT
> > > +#error This file should only be included in vmlinux.lds.S
> > > +#endif
> > > +
> > > +#ifdef CONFIG_EFI
> > > +
> > > +/*
> > > + * The EFI stub has its own symbol namespace prefixed by __efistub_, to
> > > + * isolate it from the kernel proper. The following symbols are legally
> > > + * accessed by the stub, so provide some aliases to make them accessible.
> > > + * Only include data symbols here, or text symbols of functions that are
> > > + * guaranteed to be safe when executed at another offset than they were
> > > + * linked at. The routines below are all implemented in assembler in a
> > > + * position independent manner
> > > + */
> > > +__efistub_memcmp = memcmp;
> > > +__efistub_memchr = memchr;
> > > +__efistub_memcpy = memcpy;
> > > +__efistub_memmove = memmove;
> > > +__efistub_memset = memset;
> > > +__efistub_strlen = strlen;
> > > +__efistub_strnlen = strnlen;
> > > +__efistub_strcmp = strcmp;
> > > +__efistub_strncmp = strncmp;
> > > +__efistub_strrchr = strrchr;
> > > +
> > > +#ifdef CONFIG_KASAN
> > > +__efistub___memcpy = memcpy;
> > > +__efistub___memmove = memmove;
> > > +__efistub___memset = memset;
> > > +#endif
> > > +
> > > +__efistub__start = _start;
> > > +__efistub__start_kernel = _start_kernel;
> > > +__efistub__end = _end;
> > > +__efistub__edata = _edata;
> > > +__efistub_screen_info = screen_info;
> > > +
> > > +#endif
> > > +
> > > +#endif /* __RISCV_KERNEL_IMAGE_VARS_H */
> > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> > > index f3586e31ed1e..6dcf790282dd 100644
> > > --- a/arch/riscv/kernel/vmlinux.lds.S
> > > +++ b/arch/riscv/kernel/vmlinux.lds.S
> > > @@ -10,6 +10,7 @@
> > > #include <asm/cache.h>
> > > #include <asm/thread_info.h>
> > > #include <asm/set_memory.h>
> > > +#include "image-vars.h"
> > >
> > > #include <linux/sizes.h>
> > > OUTPUT_ARCH(riscv)
> > > @@ -17,6 +18,9 @@ ENTRY(_start)
> > >
> > > jiffies = jiffies_64;
> > >
> > > +PECOFF_SECTION_ALIGNMENT = 0x1000;
> > > +PECOFF_FILE_ALIGNMENT = 0x200;
> > > +
> > > SECTIONS
> > > {
> > > /* Beginning of code and text segment */
> > > @@ -76,6 +80,10 @@ SECTIONS
> > >
> > > EXCEPTION_TABLE(0x10)
> > >
> > > +#ifdef CONFIG_EFI
> > > + . = ALIGN(PECOFF_SECTION_ALIGNMENT);
> > > + __pecoff_text_end = .;
> > > +#endif
> > > . = ALIGN(SECTION_ALIGN);
> > > _data = .;
> > >
> > > @@ -83,16 +91,26 @@ SECTIONS
> > > .sdata : {
> > > __global_pointer$ = . + 0x800;
> > > *(.sdata*)
> > > - /* End of data section */
> > > - _edata = .;
> > > }
> > >
> > > +#ifdef CONFIG_EFI
> > > + .pecoff_edata_padding : { BYTE(0); . = ALIGN(PECOFF_FILE_ALIGNMENT); }
> > > + __pecoff_data_raw_size = ABSOLUTE(. - __pecoff_text_end);
> > > +#endif
> > > +
> > > + /* End of data section */
> > > + _edata = .;
> > > +
> > > BSS_SECTION(PAGE_SIZE, PAGE_SIZE, 0)
> > >
> > > .rel.dyn : {
> > > *(.rel.dyn*)
> > > }
> > >
> > > +#ifdef CONFIG_EFI
> > > + . = ALIGN(PECOFF_SECTION_ALIGNMENT);
> > > + __pecoff_data_virt_size = ABSOLUTE(. - __pecoff_text_end);
> > > +#endif
> > > _end = .;
> > >
> > > STABS_DEBUG
> > > --
> > > 2.24.0
> > >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
>
>
>
> --
> Regards,
> Atish
^ permalink raw reply
* Re: [RFC PATCH 1/2] efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
From: Ard Biesheuvel @ 2020-09-14 18:30 UTC (permalink / raw)
To: Limonciello, Mario; +Cc: Arvind Sankar, Jacobo Pantoja, Peter Jones, linux-efi
In-Reply-To: <DM6PR19MB26366FAF28A730412DC505EDFA230@DM6PR19MB2636.namprd19.prod.outlook.com>
On Mon, 14 Sep 2020 at 19:57, Limonciello, Mario
<Mario.Limonciello@dell.com> wrote:
>
> > -----Original Message-----
> > From: Arvind Sankar <nivedita@alum.mit.edu>
> > Sent: Saturday, September 12, 2020 12:51
> > To: Jacobo Pantoja
> > Cc: Limonciello, Mario; Ard Biesheuvel; Peter Jones; linux-efi
> > Subject: [RFC PATCH 1/2] efi/x86: Add a quirk to support command line
> > arguments on Dell EFI firmware
> >
> >
> > [EXTERNAL EMAIL]
> >
> > At least some versions of Dell EFI firmware pass the entire
> > EFI_LOAD_OPTION descriptor, rather than just the OptionalData part, to
> > the loaded image.
> >
> > To handle this, add a quirk to check if the options look like a valid
> > EFI_LOAD_OPTION descriptor, and if so, use the OptionalData part as the
> > command line.
>
> I think it would be useful to document in the commit message the specifics
> of at least the failure reported by Jacobo (Precision T3620 FW 2.15.0).
>
Agreed.
> >
> > Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> > Reported-by: Jacobo Pantoja <jacobopantoja@gmail.com>
> > Link: https://lore.kernel.org/linux-
> > efi/20200907170021.GA2284449@rani.riverdale.lan/
> > ---
> > .../firmware/efi/libstub/efi-stub-helper.c | 99 ++++++++++++++++++-
> > drivers/firmware/efi/libstub/efistub.h | 31 ++++++
> > drivers/firmware/efi/libstub/file.c | 5 +-
> > 3 files changed, 133 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > index f735db55adc0..294958ff1ee6 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -238,6 +238,100 @@ efi_status_t efi_parse_options(char const *cmdline)
> > return EFI_SUCCESS;
> > }
> >
> > +/*
> > + * The EFI_LOAD_OPTION descriptor has the following layout:
> > + * u32 Attributes;
> > + * u16 FilePathListLength;
> > + * u16 Description[];
> > + * efi_device_path_protocol_t FilePathList[];
> > + * u8 OptionalData[];
> > + *
> > + * This function validates and unpacks the variable-size data fields.
> > + */
> > +static
> > +bool efi_load_option_unpack(efi_load_option_unpacked_t *dest,
> > + const efi_load_option_t *src, size_t size)
> > +{
> > +
> > + const void *pos;
> > + u16 c;
> > + efi_device_path_protocol_t header;
> > + const efi_char16_t *description;
> > + const efi_device_path_protocol_t *file_path_list;
>
> Should re-order to reverse xmas tree order.
>
We have no such requirement in the EFI subsystem.
> > +
> > + if (size < offsetof(efi_load_option_t, variable_data))
> > + return false;
> > + pos = src->variable_data;
> > + size -= offsetof(efi_load_option_t, variable_data);
> > +
> > + if ((src->attributes & ~EFI_LOAD_OPTION_MASK) != 0)
> > + return false;
> > +
> > + /* Scan description. */
> > + description = pos;
> > + do {
> > + if (size < sizeof(c))
> > + return false;
> > + c = *(const u16 *)pos;
> > + pos += sizeof(c);
> > + size -= sizeof(c);
> > + } while (c != L'\0');
> > +
> > + /* Scan file_path_list. */
> > + file_path_list = pos;
> > + do {
> > + if (size < sizeof(header))
> > + return false;
> > + header = *(const efi_device_path_protocol_t *)pos;
> > + if (header.length < sizeof(header))
> > + return false;
> > + if (size < header.length)
> > + return false;
> > + pos += header.length;
> > + size -= header.length;
> > + } while ((header.type != EFI_DEV_END_PATH && header.type !=
> > EFI_DEV_END_PATH2) ||
> > + (header.sub_type != EFI_DEV_END_ENTIRE));
> > + if (pos != (const void *)file_path_list + src->file_path_list_length)
> > + return false;
> > +
> > + dest->attributes = src->attributes;
> > + dest->file_path_list_length = src->file_path_list_length;
> > + dest->description = description;
> > + dest->file_path_list = file_path_list;
> > + dest->optional_data_size = size;
> > + dest->optional_data = size ? pos : NULL;
> > +
> > + return true;
> > +}
> > +
> > +/*
> > + * At least some versions of Dell firmware pass the entire contents of the
> > + * Boot#### variable, i.e. the EFI_LOAD_OPTION descriptor, rather than just
> > the
> > + * OptionalData field.
> > + *
> > + * Detect this case and extract OptionalData.
> > + */
> > +void efi_apply_loadoptions_quirk(const void **load_options, int
> > *load_options_size)
> > +{
> > + const efi_load_option_t *load_option = *load_options;
> > + efi_load_option_unpacked_t load_option_unpacked;
> > +
> > + if (!IS_ENABLED(CONFIG_X86))
> > + return;
> > + if (!load_option)
> > + return;
> > + if (*load_options_size < sizeof(*load_option))
> > + return;
> > + if ((load_option->attributes & ~EFI_LOAD_OPTION_BOOT_MASK) != 0)
> > + return;
> > +
> > + if (!efi_load_option_unpack(&load_option_unpacked, load_option,
> > *load_options_size))
> > + return;
> > +
>
> In case this was ever to be attributed to a cause for someone to fail to
> boot, it may be useful to drop a pr_debug here that could be easily turned
> on.
>
Agree that adding a efi_info() here makes sense, not only for
potential failures, but also simply to have some confirmation that the
quirk is taking effect.
Note that I am not 100% convinced yet that we need this change to
begin with. How large is the intersection of the set of affected
systems and the set of systems that are likely to run future kernels
that carry this quirk?
> > + *load_options = load_option_unpacked.optional_data;
> > + *load_options_size = load_option_unpacked.optional_data_size;
> > +}
> > +
> > /*
> > * Convert the unicode UEFI command line to ASCII to pass to kernel.
> > * Size of memory allocated return in *cmd_line_len.
> > @@ -247,12 +341,15 @@ char *efi_convert_cmdline(efi_loaded_image_t *image, int
> > *cmd_line_len)
> > {
> > const u16 *s2;
> > unsigned long cmdline_addr = 0;
> > - int options_chars = efi_table_attr(image, load_options_size) / 2;
> > + int options_chars = efi_table_attr(image, load_options_size);
> > const u16 *options = efi_table_attr(image, load_options);
> > int options_bytes = 0, safe_options_bytes = 0; /* UTF-8 bytes */
> > bool in_quote = false;
> > efi_status_t status;
> >
> > + efi_apply_loadoptions_quirk((const void **)&options, &options_chars);
> > + options_chars /= sizeof(*options);
> > +
> > if (options) {
> > s2 = options;
> > while (options_bytes < COMMAND_LINE_SIZE && options_chars--) {
> > diff --git a/drivers/firmware/efi/libstub/efistub.h
> > b/drivers/firmware/efi/libstub/efistub.h
> > index 85050f5a1b28..589d07acb22d 100644
> > --- a/drivers/firmware/efi/libstub/efistub.h
> > +++ b/drivers/firmware/efi/libstub/efistub.h
> > @@ -688,6 +688,35 @@ union efi_load_file_protocol {
> > } mixed_mode;
> > };
> >
> > +typedef struct {
> > + u32 attributes;
> > + u16 file_path_list_length;
> > + u8 variable_data[];
> > + // efi_char16_t description[];
> > + // efi_device_path_protocol_t file_path_list[];
> > + // u8 optional_data[];
> > +} __packed efi_load_option_t;
> > +
> > +#define EFI_LOAD_OPTION_ACTIVE 0x0001U
> > +#define EFI_LOAD_OPTION_FORCE_RECONNECT 0x0002U
> > +#define EFI_LOAD_OPTION_HIDDEN 0x0008U
> > +#define EFI_LOAD_OPTION_CATEGORY 0x1f00U
> > +#define EFI_LOAD_OPTION_CATEGORY_BOOT 0x0000U
> > +#define EFI_LOAD_OPTION_CATEGORY_APP 0x0100U
> > +
> > +#define EFI_LOAD_OPTION_BOOT_MASK \
> > + (EFI_LOAD_OPTION_ACTIVE|EFI_LOAD_OPTION_HIDDEN|EFI_LOAD_OPTION_CATEGORY)
> > +#define EFI_LOAD_OPTION_MASK
> > (EFI_LOAD_OPTION_FORCE_RECONNECT|EFI_LOAD_OPTION_BOOT_MASK)
> > +
> > +typedef struct {
> > + u32 attributes;
> > + u16 file_path_list_length;
> > + const efi_char16_t *description;
> > + const efi_device_path_protocol_t *file_path_list;
> > + size_t optional_data_size;
> > + const void *optional_data;
> > +} efi_load_option_unpacked_t;
> > +
> > void efi_pci_disable_bridge_busmaster(void);
> >
> > typedef efi_status_t (*efi_exit_boot_map_processing)(
> > @@ -730,6 +759,8 @@ __printf(1, 2) int efi_printk(char const *fmt, ...);
> >
> > void efi_free(unsigned long size, unsigned long addr);
> >
> > +void efi_apply_loadoptions_quirk(const void **load_options, int
> > *load_options_size);
> > +
> > char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len);
> >
> > efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
> > diff --git a/drivers/firmware/efi/libstub/file.c
> > b/drivers/firmware/efi/libstub/file.c
> > index 630caa6b1f4c..4e81c6077188 100644
> > --- a/drivers/firmware/efi/libstub/file.c
> > +++ b/drivers/firmware/efi/libstub/file.c
> > @@ -136,7 +136,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t
> > *image,
> > unsigned long *load_size)
> > {
> > const efi_char16_t *cmdline = image->load_options;
> > - int cmdline_len = image->load_options_size / 2;
> > + int cmdline_len = image->load_options_size;
> > unsigned long efi_chunk_size = ULONG_MAX;
> > efi_file_protocol_t *volume = NULL;
> > efi_file_protocol_t *file;
> > @@ -148,6 +148,9 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t
> > *image,
> > if (!load_addr || !load_size)
> > return EFI_INVALID_PARAMETER;
> >
> > + efi_apply_loadoptions_quirk((const void **)&cmdline, &cmdline_len);
> > + cmdline_len /= sizeof(*cmdline);
> > +
> > if (IS_ENABLED(CONFIG_X86) && !efi_nochunk)
> > efi_chunk_size = EFI_READ_CHUNK_SIZE;
> >
> > --
> > 2.26.2
>
^ permalink raw reply
* Re: EFI regression for efi=noruntime support
From: Ard Biesheuvel @ 2020-09-14 18:27 UTC (permalink / raw)
To: Branden Sherrell; +Cc: linux-efi
In-Reply-To: <AE217103-C96F-4AFC-8417-83EC11962004@gmail.com>
On Mon, 14 Sep 2020 at 20:15, Branden Sherrell <sherrellbc@gmail.com> wrote:
>
> An EFI-related regression appears to have made its way in from f88814cc2578c121e6edef686365036db72af0ed:
>
> Author: Ard Biesheuvel <ardb@kernel.org> Date: Wed Jul 8 13:01:57 2020 +0300 efi/efivars:
> Expose RT service availability via efivars abstraction
>
>
> On the RockChip rk3399 (ARMv8) port the EFI subsystem claims the reboot callback despite no runtime support. Ordinarily one might use efi=noruntime or noefi to disable this, but the boot augment appears to now be outright ignored (on this system, at least). You might imagine this causes issue on a uboot'd system that does not have runtime services. The manifestation of this bug is an iabt with PC=0 originating in efivar_entry_set_safe. This function eventually attempts to call the NULL entry ops->set_variable.
>
> [ 41.231673] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [ 41.232436] Mem abort info:
> [ 41.232682] ESR = 0x86000004
> [ 41.232951] EC = 0x21: IABT (current EL), IL = 32 bits
> [ 41.233413] SET = 0, FnV = 0
> [ 41.233681] EA = 0, S1PTW = 0
> [ 41.233956] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000f02eb000
> [ 41.234516] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
> [ 41.235111] Internal error: Oops: 86000004 [#1] SMP
> [ 41.235536] Modules linked in: rt2800usb rt2x00usb rt2800lib rt2x00lib rc_cec mac80211 snd_soc_hdmi_codec dw_hdmi_i2s_audio dw_hdmi_cec rockchipdrm realtek dw_mipi_dsi hantro_vpu(C) dw_hdmi hci_uart rockchip_vdec(C) rockchip_rga analogix_dp cfg80211 cec btqca btbcm v4l2_h264 videobuf2_dma_sg pwm_fan btintel rc_core v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig bluetooth dwmac_rk videobuf2_memops videobuf2_v4l2 stmmac_platform videobuf2_common libarc4 drm_kms_helper snd_soc_audio_graph_card snd_soc_simple_card stmmac snd_soc_simple_card_utils syscopyarea sysfillrect sysimgblt panfrost fb_sys_fops ecdh_generic mdio_xpcs videodev phylink gpu_sched ecc snd_soc_rockchip_i2s rfkill mc snd_soc_rockchip_pcm dw_wdt rockchip_thermal rtc_rk808 rockchip_saradc drm gpio_keys
> [ 41.241462] CPU: 5 PID: 1 Comm: shutdown Tainted: G C 5.8.8-1-ARCH #1
> [ 41.242135] Hardware name: pine64 rockpro64_rk3399/rockpro64_rk3399, BIOS 2020.10-rc4-dirty 09/11/2020
> [ 41.242944] pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
> [ 41.243431] pc : 0x0
> [ 41.243631] lr : efivar_entry_set_safe+0xc8/0x1a0
> [ 41.244041] sp : ffff80001254bbe0
> [ 41.244331] x29: ffff80001254bbe0 x28: ffff0000f2f8e3c0
> [ 41.244794] x27: 0000000000000000 x26: ffff0000f11af418
> [ 41.245257] x25: ffff8000124c0000 x24: ffff8000124c0b50
> [ 41.245720] x23: ffff0000f11af418 x22: ffff800012387230
> [ 41.246183] x21: 0000000000000007 x20: ffff0000f11af000
> [ 41.246646] x19: 000000000000000e x18: 0000000000000030
> [ 41.247109] x17: 0000000000000000 x16: 0000000000000000
> [ 41.247572] x15: ffff0000f2f8e8e0 x14: ffffffffffffffff
> [ 41.248034] x13: ffff80009254b9e7 x12: 0000000000000030
> [ 41.248497] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
> [ 41.248960] x9 : ffff800010d8bb94 x8 : 41cf0a4c4a67b082
> [ 41.249423] x7 : ffff8000124c0b78 x6 : ffff0000f11af418
> [ 41.249886] x5 : 0000000000000000 x4 : ffff0000f11af418
> [ 41.250349] x3 : 000000000000000e x2 : 0000000000000007
> [ 41.250811] x1 : ffff80001254bc30 x0 : ffff0000f11af000
> [ 41.251275] Call trace:
> [ 41.251491] 0x0
> [ 41.251655] efibc_set_variable+0xf0/0x170
> [ 41.252013] efibc_reboot_notifier_call+0x44/0x80
> [ 41.252427] blocking_notifier_call_chain+0x78/0xb0
> [ 41.252852] __do_sys_reboot+0x1cc/0x290
> [ 41.253195] __arm64_sys_reboot+0x30/0x40
> [ 41.253547] el0_svc_common.constprop.0+0x7c/0x140
> [ 41.253965] do_el0_svc+0x28/0xb0
> [ 41.254257] el0_sync_handler+0x98/0x278
> [ 41.254599] el0_sync+0x158/0x180
> [ 41.254891] Code: bad PC value
> [ 41.255160] ---[ end trace 1fafcf21a783a644 ]---
> [ 41.255617] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
> [ 41.256283] SMP: stopping secondary CPUs
> [ 41.256735] Kernel Offset: disabled
> [ 41.257040] CPU features: 0x240022,21006008
> [ 41.257406] Memory Limit: none
> [ 41.257681] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]---
>
> I did not spend too much time poking around, but I did note that if one builds a kernel that forces the efibc_init to return ENODEV to prevent reboot registration, the system will properly consult PSCI to handle the event (as is expected for the rk3399 per the device tree). This observed behavior is accurate at least through 5.9.0-rc4. A test of the previous commit (8778eb0927ddcd3f431805c37b78fa56481aeed9) confirms an uneventful reboot for this device.
> The board configuration is the RockPro64, which has a device tree present in the kernel sources. My configuration in particular was using the latest Grub to EFI boot Linux after having itself been loaded by uboot.
>
>
This looks like an oversight in the efibc driver.
Does the change below fix your issue?
diff --git a/drivers/firmware/efi/efibc.c b/drivers/firmware/efi/efibc.c
index 35dccc88ac0a..15a47539dc56 100644
--- a/drivers/firmware/efi/efibc.c
+++ b/drivers/firmware/efi/efibc.c
@@ -84,7 +84,7 @@ static int __init efibc_init(void)
{
int ret;
- if (!efi_enabled(EFI_RUNTIME_SERVICES))
+ if (!efivars_kobject() || !efivar_supports_writes())
return -ENODEV;
ret = register_reboot_notifier(&efibc_reboot_notifier);
^ permalink raw reply related
* EFI regression for efi=noruntime support
From: Branden Sherrell @ 2020-09-14 17:15 UTC (permalink / raw)
To: ardb; +Cc: linux-efi
An EFI-related regression appears to have made its way in from f88814cc2578c121e6edef686365036db72af0ed:
Author: Ard Biesheuvel <ardb@kernel.org> Date: Wed Jul 8 13:01:57 2020 +0300 efi/efivars:
Expose RT service availability via efivars abstraction
On the RockChip rk3399 (ARMv8) port the EFI subsystem claims the reboot callback despite no runtime support. Ordinarily one might use efi=noruntime or noefi to disable this, but the boot augment appears to now be outright ignored (on this system, at least). You might imagine this causes issue on a uboot'd system that does not have runtime services. The manifestation of this bug is an iabt with PC=0 originating in efivar_entry_set_safe. This function eventually attempts to call the NULL entry ops->set_variable.
[ 41.231673] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[ 41.232436] Mem abort info:
[ 41.232682] ESR = 0x86000004
[ 41.232951] EC = 0x21: IABT (current EL), IL = 32 bits
[ 41.233413] SET = 0, FnV = 0
[ 41.233681] EA = 0, S1PTW = 0
[ 41.233956] user pgtable: 4k pages, 48-bit VAs, pgdp=00000000f02eb000
[ 41.234516] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
[ 41.235111] Internal error: Oops: 86000004 [#1] SMP
[ 41.235536] Modules linked in: rt2800usb rt2x00usb rt2800lib rt2x00lib rc_cec mac80211 snd_soc_hdmi_codec dw_hdmi_i2s_audio dw_hdmi_cec rockchipdrm realtek dw_mipi_dsi hantro_vpu(C) dw_hdmi hci_uart rockchip_vdec(C) rockchip_rga analogix_dp cfg80211 cec btqca btbcm v4l2_h264 videobuf2_dma_sg pwm_fan btintel rc_core v4l2_mem2mem videobuf2_vmalloc videobuf2_dma_contig bluetooth dwmac_rk videobuf2_memops videobuf2_v4l2 stmmac_platform videobuf2_common libarc4 drm_kms_helper snd_soc_audio_graph_card snd_soc_simple_card stmmac snd_soc_simple_card_utils syscopyarea sysfillrect sysimgblt panfrost fb_sys_fops ecdh_generic mdio_xpcs videodev phylink gpu_sched ecc snd_soc_rockchip_i2s rfkill mc snd_soc_rockchip_pcm dw_wdt rockchip_thermal rtc_rk808 rockchip_saradc drm gpio_keys
[ 41.241462] CPU: 5 PID: 1 Comm: shutdown Tainted: G C 5.8.8-1-ARCH #1
[ 41.242135] Hardware name: pine64 rockpro64_rk3399/rockpro64_rk3399, BIOS 2020.10-rc4-dirty 09/11/2020
[ 41.242944] pstate: 00000005 (nzcv daif -PAN -UAO BTYPE=--)
[ 41.243431] pc : 0x0
[ 41.243631] lr : efivar_entry_set_safe+0xc8/0x1a0
[ 41.244041] sp : ffff80001254bbe0
[ 41.244331] x29: ffff80001254bbe0 x28: ffff0000f2f8e3c0
[ 41.244794] x27: 0000000000000000 x26: ffff0000f11af418
[ 41.245257] x25: ffff8000124c0000 x24: ffff8000124c0b50
[ 41.245720] x23: ffff0000f11af418 x22: ffff800012387230
[ 41.246183] x21: 0000000000000007 x20: ffff0000f11af000
[ 41.246646] x19: 000000000000000e x18: 0000000000000030
[ 41.247109] x17: 0000000000000000 x16: 0000000000000000
[ 41.247572] x15: ffff0000f2f8e8e0 x14: ffffffffffffffff
[ 41.248034] x13: ffff80009254b9e7 x12: 0000000000000030
[ 41.248497] x11: 0101010101010101 x10: 7f7f7f7f7f7f7f7f
[ 41.248960] x9 : ffff800010d8bb94 x8 : 41cf0a4c4a67b082
[ 41.249423] x7 : ffff8000124c0b78 x6 : ffff0000f11af418
[ 41.249886] x5 : 0000000000000000 x4 : ffff0000f11af418
[ 41.250349] x3 : 000000000000000e x2 : 0000000000000007
[ 41.250811] x1 : ffff80001254bc30 x0 : ffff0000f11af000
[ 41.251275] Call trace:
[ 41.251491] 0x0
[ 41.251655] efibc_set_variable+0xf0/0x170
[ 41.252013] efibc_reboot_notifier_call+0x44/0x80
[ 41.252427] blocking_notifier_call_chain+0x78/0xb0
[ 41.252852] __do_sys_reboot+0x1cc/0x290
[ 41.253195] __arm64_sys_reboot+0x30/0x40
[ 41.253547] el0_svc_common.constprop.0+0x7c/0x140
[ 41.253965] do_el0_svc+0x28/0xb0
[ 41.254257] el0_sync_handler+0x98/0x278
[ 41.254599] el0_sync+0x158/0x180
[ 41.254891] Code: bad PC value
[ 41.255160] ---[ end trace 1fafcf21a783a644 ]---
[ 41.255617] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
[ 41.256283] SMP: stopping secondary CPUs
[ 41.256735] Kernel Offset: disabled
[ 41.257040] CPU features: 0x240022,21006008
[ 41.257406] Memory Limit: none
[ 41.257681] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]—
TI did not spend too much time poking around, but I did note that if one builds a kernel that forces the efibc_init to return ENODEV to prevent reboot registration, the system will properly consult PSCI to handle the event (as is expected for the rk3399 per the device tree). This observed behavior is accurate at least through 5.9.0-rc4. A test of the previous commit (8778eb0927ddcd3f431805c37b78fa56481aeed9) confirms an uneventful reboot for this device.
The board configuration is the RockPro64, which has a device tree present in the kernel sources. My configuration in particular was using the latest Grub to EFI boot Linux after having itself been loaded by uboot.
Branden
(sent again due to mailing reject on original)
^ permalink raw reply
* RE: [RFC PATCH 1/2] efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
From: Limonciello, Mario @ 2020-09-14 16:56 UTC (permalink / raw)
To: Arvind Sankar, Jacobo Pantoja; +Cc: Ard Biesheuvel, Peter Jones, linux-efi
In-Reply-To: <20200912175105.2085299-2-nivedita@alum.mit.edu>
> -----Original Message-----
> From: Arvind Sankar <nivedita@alum.mit.edu>
> Sent: Saturday, September 12, 2020 12:51
> To: Jacobo Pantoja
> Cc: Limonciello, Mario; Ard Biesheuvel; Peter Jones; linux-efi
> Subject: [RFC PATCH 1/2] efi/x86: Add a quirk to support command line
> arguments on Dell EFI firmware
>
>
> [EXTERNAL EMAIL]
>
> At least some versions of Dell EFI firmware pass the entire
> EFI_LOAD_OPTION descriptor, rather than just the OptionalData part, to
> the loaded image.
>
> To handle this, add a quirk to check if the options look like a valid
> EFI_LOAD_OPTION descriptor, and if so, use the OptionalData part as the
> command line.
I think it would be useful to document in the commit message the specifics
of at least the failure reported by Jacobo (Precision T3620 FW 2.15.0).
>
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> Reported-by: Jacobo Pantoja <jacobopantoja@gmail.com>
> Link: https://lore.kernel.org/linux-
> efi/20200907170021.GA2284449@rani.riverdale.lan/
> ---
> .../firmware/efi/libstub/efi-stub-helper.c | 99 ++++++++++++++++++-
> drivers/firmware/efi/libstub/efistub.h | 31 ++++++
> drivers/firmware/efi/libstub/file.c | 5 +-
> 3 files changed, 133 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c
> b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index f735db55adc0..294958ff1ee6 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -238,6 +238,100 @@ efi_status_t efi_parse_options(char const *cmdline)
> return EFI_SUCCESS;
> }
>
> +/*
> + * The EFI_LOAD_OPTION descriptor has the following layout:
> + * u32 Attributes;
> + * u16 FilePathListLength;
> + * u16 Description[];
> + * efi_device_path_protocol_t FilePathList[];
> + * u8 OptionalData[];
> + *
> + * This function validates and unpacks the variable-size data fields.
> + */
> +static
> +bool efi_load_option_unpack(efi_load_option_unpacked_t *dest,
> + const efi_load_option_t *src, size_t size)
> +{
> +
> + const void *pos;
> + u16 c;
> + efi_device_path_protocol_t header;
> + const efi_char16_t *description;
> + const efi_device_path_protocol_t *file_path_list;
Should re-order to reverse xmas tree order.
> +
> + if (size < offsetof(efi_load_option_t, variable_data))
> + return false;
> + pos = src->variable_data;
> + size -= offsetof(efi_load_option_t, variable_data);
> +
> + if ((src->attributes & ~EFI_LOAD_OPTION_MASK) != 0)
> + return false;
> +
> + /* Scan description. */
> + description = pos;
> + do {
> + if (size < sizeof(c))
> + return false;
> + c = *(const u16 *)pos;
> + pos += sizeof(c);
> + size -= sizeof(c);
> + } while (c != L'\0');
> +
> + /* Scan file_path_list. */
> + file_path_list = pos;
> + do {
> + if (size < sizeof(header))
> + return false;
> + header = *(const efi_device_path_protocol_t *)pos;
> + if (header.length < sizeof(header))
> + return false;
> + if (size < header.length)
> + return false;
> + pos += header.length;
> + size -= header.length;
> + } while ((header.type != EFI_DEV_END_PATH && header.type !=
> EFI_DEV_END_PATH2) ||
> + (header.sub_type != EFI_DEV_END_ENTIRE));
> + if (pos != (const void *)file_path_list + src->file_path_list_length)
> + return false;
> +
> + dest->attributes = src->attributes;
> + dest->file_path_list_length = src->file_path_list_length;
> + dest->description = description;
> + dest->file_path_list = file_path_list;
> + dest->optional_data_size = size;
> + dest->optional_data = size ? pos : NULL;
> +
> + return true;
> +}
> +
> +/*
> + * At least some versions of Dell firmware pass the entire contents of the
> + * Boot#### variable, i.e. the EFI_LOAD_OPTION descriptor, rather than just
> the
> + * OptionalData field.
> + *
> + * Detect this case and extract OptionalData.
> + */
> +void efi_apply_loadoptions_quirk(const void **load_options, int
> *load_options_size)
> +{
> + const efi_load_option_t *load_option = *load_options;
> + efi_load_option_unpacked_t load_option_unpacked;
> +
> + if (!IS_ENABLED(CONFIG_X86))
> + return;
> + if (!load_option)
> + return;
> + if (*load_options_size < sizeof(*load_option))
> + return;
> + if ((load_option->attributes & ~EFI_LOAD_OPTION_BOOT_MASK) != 0)
> + return;
> +
> + if (!efi_load_option_unpack(&load_option_unpacked, load_option,
> *load_options_size))
> + return;
> +
In case this was ever to be attributed to a cause for someone to fail to
boot, it may be useful to drop a pr_debug here that could be easily turned
on.
> + *load_options = load_option_unpacked.optional_data;
> + *load_options_size = load_option_unpacked.optional_data_size;
> +}
> +
> /*
> * Convert the unicode UEFI command line to ASCII to pass to kernel.
> * Size of memory allocated return in *cmd_line_len.
> @@ -247,12 +341,15 @@ char *efi_convert_cmdline(efi_loaded_image_t *image, int
> *cmd_line_len)
> {
> const u16 *s2;
> unsigned long cmdline_addr = 0;
> - int options_chars = efi_table_attr(image, load_options_size) / 2;
> + int options_chars = efi_table_attr(image, load_options_size);
> const u16 *options = efi_table_attr(image, load_options);
> int options_bytes = 0, safe_options_bytes = 0; /* UTF-8 bytes */
> bool in_quote = false;
> efi_status_t status;
>
> + efi_apply_loadoptions_quirk((const void **)&options, &options_chars);
> + options_chars /= sizeof(*options);
> +
> if (options) {
> s2 = options;
> while (options_bytes < COMMAND_LINE_SIZE && options_chars--) {
> diff --git a/drivers/firmware/efi/libstub/efistub.h
> b/drivers/firmware/efi/libstub/efistub.h
> index 85050f5a1b28..589d07acb22d 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -688,6 +688,35 @@ union efi_load_file_protocol {
> } mixed_mode;
> };
>
> +typedef struct {
> + u32 attributes;
> + u16 file_path_list_length;
> + u8 variable_data[];
> + // efi_char16_t description[];
> + // efi_device_path_protocol_t file_path_list[];
> + // u8 optional_data[];
> +} __packed efi_load_option_t;
> +
> +#define EFI_LOAD_OPTION_ACTIVE 0x0001U
> +#define EFI_LOAD_OPTION_FORCE_RECONNECT 0x0002U
> +#define EFI_LOAD_OPTION_HIDDEN 0x0008U
> +#define EFI_LOAD_OPTION_CATEGORY 0x1f00U
> +#define EFI_LOAD_OPTION_CATEGORY_BOOT 0x0000U
> +#define EFI_LOAD_OPTION_CATEGORY_APP 0x0100U
> +
> +#define EFI_LOAD_OPTION_BOOT_MASK \
> + (EFI_LOAD_OPTION_ACTIVE|EFI_LOAD_OPTION_HIDDEN|EFI_LOAD_OPTION_CATEGORY)
> +#define EFI_LOAD_OPTION_MASK
> (EFI_LOAD_OPTION_FORCE_RECONNECT|EFI_LOAD_OPTION_BOOT_MASK)
> +
> +typedef struct {
> + u32 attributes;
> + u16 file_path_list_length;
> + const efi_char16_t *description;
> + const efi_device_path_protocol_t *file_path_list;
> + size_t optional_data_size;
> + const void *optional_data;
> +} efi_load_option_unpacked_t;
> +
> void efi_pci_disable_bridge_busmaster(void);
>
> typedef efi_status_t (*efi_exit_boot_map_processing)(
> @@ -730,6 +759,8 @@ __printf(1, 2) int efi_printk(char const *fmt, ...);
>
> void efi_free(unsigned long size, unsigned long addr);
>
> +void efi_apply_loadoptions_quirk(const void **load_options, int
> *load_options_size);
> +
> char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len);
>
> efi_status_t efi_get_memory_map(struct efi_boot_memmap *map);
> diff --git a/drivers/firmware/efi/libstub/file.c
> b/drivers/firmware/efi/libstub/file.c
> index 630caa6b1f4c..4e81c6077188 100644
> --- a/drivers/firmware/efi/libstub/file.c
> +++ b/drivers/firmware/efi/libstub/file.c
> @@ -136,7 +136,7 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t
> *image,
> unsigned long *load_size)
> {
> const efi_char16_t *cmdline = image->load_options;
> - int cmdline_len = image->load_options_size / 2;
> + int cmdline_len = image->load_options_size;
> unsigned long efi_chunk_size = ULONG_MAX;
> efi_file_protocol_t *volume = NULL;
> efi_file_protocol_t *file;
> @@ -148,6 +148,9 @@ efi_status_t handle_cmdline_files(efi_loaded_image_t
> *image,
> if (!load_addr || !load_size)
> return EFI_INVALID_PARAMETER;
>
> + efi_apply_loadoptions_quirk((const void **)&cmdline, &cmdline_len);
> + cmdline_len /= sizeof(*cmdline);
> +
> if (IS_ENABLED(CONFIG_X86) && !efi_nochunk)
> efi_chunk_size = EFI_READ_CHUNK_SIZE;
>
> --
> 2.26.2
^ permalink raw reply
* Re: [PATCH v2 0/2] UEFI v2.8 Memory Error Record Updates
From: Russ Anderson @ 2020-09-14 16:44 UTC (permalink / raw)
To: Alex Kluver
Cc: linux-edac, linux-efi, linux-kernel, ardb, mchehab, bp,
russ.anderson, dimitri.sivanich, kluveralex
In-Reply-To: <20200819143544.155096-1-alex.kluver@hpe.com>
On Wed, Aug 19, 2020 at 09:35:42AM -0500, Alex Kluver wrote:
> The UEFI Specification v2.8, Table 299, Memory Error Record has
> several changes from previous versions. Bits 18 through 21 have been
> added to the memory validation bits to include an extended version
> of row, an option to print bank address and group separately, and chip id.
> These patches implement bits 18 through 21 into the Memory Error Record.
>
> Change reserved field to extended field in cper_sec_mem_err structure
> and added the extended field to the cper_mem_err_compact structure.
>
> Print correct versions of row, bank, and chip ID.
Are there any community comment on this patch set?
Questions/comments/concerns?
Thanks.
> ---
> v1 -> v2:
> * Add static inline cper_get_mem_extension to make
> it more readable, as suggested by Borislav Petkov
>
> * Add second patch for bank field, bank group, and chip id.
> ---
> Alex Kluver (2):
> edac,ghes,cper: Add Row Extension to Memory Error Record
> cper,edac,efi: Memory Error Record: bank group/address and chip id
>
> drivers/edac/ghes_edac.c | 17 +++++++++++++++--
> drivers/firmware/efi/cper.c | 18 ++++++++++++++++--
> include/linux/cper.h | 24 ++++++++++++++++++++++--
> 3 files changed, 53 insertions(+), 6 deletions(-)
>
> --
> 2.26.2
>
--
Russ Anderson, SuperDome Flex Linux Kernel Group Manager
HPE - Hewlett Packard Enterprise (formerly SGI) rja@hpe.com
^ permalink raw reply
* Re: [PATCH 03/12] ARM: module: add support for place relative relocations
From: Russell King - ARM Linux admin @ 2020-09-14 16:11 UTC (permalink / raw)
To: Nicolas Pitre
Cc: Ard Biesheuvel, linux-efi, linux-arm-kernel, Linus Walleij,
Nick Desaulniers, Stefan Agner, Peter Smith, Marc Zyngier,
Will Deacon
In-Reply-To: <nycvar.YSQ.7.78.906.2009140927490.4095746@knanqh.ubzr>
On Mon, Sep 14, 2020 at 09:35:41AM -0400, Nicolas Pitre wrote:
> On Mon, 14 Sep 2020, Ard Biesheuvel wrote:
>
> > When using the new adr_l/ldr_l/str_l macros to refer to external symbols
> > from modules, the linker may emit place relative ELF relocations that
> > need to be fixed up by the module loader. So add support for these.
>
> Just wondering if that capability requirement should be added to the
> module signature somehow...?
>
> Maybe not. The MODULE_ARCH_VERMAGIC definition only contains things that
> are configurable for a given build.
It doesn't need to be. If a module contains a relocation we don't know
how to handle, it will fail to load.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [RFC PATCH 0/2] Quirk to handle Dell BIOS
From: Jacobo Pantoja @ 2020-09-14 15:59 UTC (permalink / raw)
To: Arvind Sankar; +Cc: Limonciello, Mario, Ard Biesheuvel, Peter Jones, linux-efi
In-Reply-To: <20200912175105.2085299-1-nivedita@alum.mit.edu>
On Sat, 12 Sep 2020 at 19:51, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> I coded up the quirk to see what it looks like.
>
> First patch is the quirk itself, and the second one is a slightly
> rejiggered version of the dumping code for testing.
>
> Jacobo, can you check if this fixes the efi stub boot?
I tested the patch and works perfectly. I also tested in a non-Dell
machine, and it
also works properly.
I simplified a bit the cmdline so that it fits on the screen during testing.
This is the output of the Dell machine with the helper patch also applied:
https://ibb.co/HdDh6HK
And this is the output of the non-Dell machine:
https://ibb.co/whh1g9D
It seems that both are properly parsed.
I've also tested (just in case) the patch alone, without the helper
debug text, and it
works properly in both machines as well, as expected.
>
> Thanks.
>
> Arvind Sankar (2):
> efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
> efi/libstub: Dump command line before/after conversion
>
> .../firmware/efi/libstub/efi-stub-helper.c | 139 +++++++++++++++++-
> drivers/firmware/efi/libstub/efistub.h | 31 ++++
> drivers/firmware/efi/libstub/file.c | 5 +-
> 3 files changed, 173 insertions(+), 2 deletions(-)
>
> --
> 2.26.2
>
^ permalink raw reply
* Re: [PATCH v3 2/2] x86/mce/dev-mcelog: Fix updating kflags in AMD systems
From: Borislav Petkov @ 2020-09-14 15:34 UTC (permalink / raw)
To: Smita Koralahalli
Cc: x86, linux-kernel, linux-pm, linux-edac, linux-efi, linux-acpi,
devel, Tony Luck, Rafael J . Wysocki, Len Brown, Ard Biesheuvel,
Yazen Ghannam
In-Reply-To: <20200903234531.162484-3-Smita.KoralahalliChannabasappa@amd.com>
On Thu, Sep 03, 2020 at 06:45:31PM -0500, Smita Koralahalli wrote:
> The mcelog utility is not commonly used on AMD systems. Therefore, errors
> logged only by the dev_mce_log() notifier will be missed. This may occur
> if the EDAC modules are not loaded in which case it's preferable to print
> the error record by the default notifier.
>
> However, the mce->kflags set by dev_mce_log() notifier makes the default
> notifier to skip over the errors assuming they are processed by
> dev_mce_log().
>
> Do not update kflags in the dev_mce_log() notifier on AMD systems.
>
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> ---
> Link:
> https://lkml.kernel.org/r/20200828203332.11129-3-Smita.KoralahalliChannabasappa@amd.com
>
> v3:
> No change
> v2:
> No change
> ---
> arch/x86/kernel/cpu/mce/dev-mcelog.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/mce/dev-mcelog.c b/arch/x86/kernel/cpu/mce/dev-mcelog.c
> index 03e51053592a..100fbeebdc72 100644
> --- a/arch/x86/kernel/cpu/mce/dev-mcelog.c
> +++ b/arch/x86/kernel/cpu/mce/dev-mcelog.c
> @@ -67,7 +67,9 @@ static int dev_mce_log(struct notifier_block *nb, unsigned long val,
> unlock:
> mutex_unlock(&mce_chrdev_read_mutex);
>
> - mce->kflags |= MCE_HANDLED_MCELOG;
> + if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> + mce->kflags |= MCE_HANDLED_MCELOG;
> +
> return NOTIFY_OK;
> }
>
> --
This one is not related to your 1/2 so it sounds to me like I should
take this one now, independently?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v3 1/2] cper, apei, mce: Pass x86 CPER through the MCA handling chain
From: Borislav Petkov @ 2020-09-14 15:30 UTC (permalink / raw)
To: Smita Koralahalli
Cc: x86, linux-kernel, linux-pm, linux-edac, linux-efi, linux-acpi,
devel, Tony Luck, Rafael J . Wysocki, Len Brown, Ard Biesheuvel,
Yazen Ghannam
In-Reply-To: <20200903234531.162484-2-Smita.KoralahalliChannabasappa@amd.com>
On Thu, Sep 03, 2020 at 06:45:30PM -0500, Smita Koralahalli wrote:
> Linux Kernel uses ACPI Boot Error Record Table (BERT) to report fatal
> errors that occurred in a previous boot. The MCA errors in the BERT are
> reported using the x86 Processor Error Common Platform Error Record (CPER)
> format. Currently, the record prints out the raw MSR values and AMD relies
> on the raw record to provide MCA information.
>
> Extract the raw MSR values of MCA registers from the BERT and feed it into
> the standard mce_log() function through the existing x86/MCA RAS
> infrastructure. This will result in better decoding from the EDAC MCE
> decoder or the default notifier.
>
> The implementation is SMCA specific as the raw MCA register values are
> given in the register offset order of the MCAX address space.
>
> Reported-by: kernel test robot <lkp@intel.com>
What's that Reported-by for?
Pls put in [] brackets over it what the 0day robot has reported.
> Signed-off-by: Smita Koralahalli <Smita.KoralahalliChannabasappa@amd.com>
> ---
> Link:
> https://lkml.kernel.org/r/20200828203332.11129-2-Smita.KoralahalliChannabasappa@amd.com
>
> v3:
> Moved arch specific declarations from generic header file to arch
> specific header file.
> Cleaned additional declarations which are unnecessary.
> Included the check for context type.
> Added a check to verify for the first MSR address in the register
> layout.
> v2:
> Fixed build error reported by kernel test robot.
> Passed struct variable as function argument instead of entire struct
> ---
> arch/x86/include/asm/acpi.h | 11 +++++++++
> arch/x86/include/asm/mce.h | 3 +++
> arch/x86/kernel/acpi/apei.c | 9 +++++++
> arch/x86/kernel/cpu/mce/apei.c | 42 +++++++++++++++++++++++++++++++++
> drivers/firmware/efi/cper-x86.c | 10 +++++---
> 5 files changed, 72 insertions(+), 3 deletions(-)
...
> diff --git a/arch/x86/kernel/acpi/apei.c b/arch/x86/kernel/acpi/apei.c
> index c22fb55abcfd..13d60a91eaa0 100644
> --- a/arch/x86/kernel/acpi/apei.c
> +++ b/arch/x86/kernel/acpi/apei.c
> @@ -43,3 +43,12 @@ void arch_apei_report_mem_error(int sev, struct cper_sec_mem_err *mem_err)
> apei_mce_report_mem_error(sev, mem_err);
> #endif
> }
> +
> +int arch_apei_report_x86_error(struct cper_ia_proc_ctx *ctx_info, u64 lapic_id)
> +{
> + int err = -EINVAL;
> +#ifdef CONFIG_X86_MCE
> + err = apei_mce_report_x86_error(ctx_info, lapic_id);
> +#endif
> + return err;
> +}
Add a stub for apei_mce_report_x86_error() in
arch/x86/include/asm/mce.h, in one of the !CONFIG_X86_MCE ifdeffery
which returns -EINVAL and get rid of this ifdeffery and simply do:
return apei_mce_report_x86_error(ctx_info, lapic_id);
here.
If you wanna fix the above apei_mce_report_mem_error() too, you can do
that in a separate patch.
> diff --git a/arch/x86/kernel/cpu/mce/apei.c b/arch/x86/kernel/cpu/mce/apei.c
> index af8d37962586..65001d342302 100644
> --- a/arch/x86/kernel/cpu/mce/apei.c
> +++ b/arch/x86/kernel/cpu/mce/apei.c
> @@ -26,6 +26,8 @@
>
> #include "internal.h"
>
> +#define MASK_MCA_STATUS 0xC0002001
What does that mask mean? Either here or where it is used needs a
comment.
> void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
> {
> struct mce m;
> @@ -51,6 +53,46 @@ void apei_mce_report_mem_error(int severity, struct cper_sec_mem_err *mem_err)
> }
> EXPORT_SYMBOL_GPL(apei_mce_report_mem_error);
>
> +int apei_mce_report_x86_error(struct cper_ia_proc_ctx *ctx_info, u64 lapic_id)
> +{
> + const u64 *i_mce = ((const void *) (ctx_info + 1));
> + unsigned int cpu;
> + struct mce m;
> +
> + if (!boot_cpu_has(X86_FEATURE_SMCA))
If this function you're adding is SMCA-specific, then its name cannot be
as generic as it is now.
> + return -EINVAL;
> +
> + if ((ctx_info->msr_addr & MASK_MCA_STATUS) != MASK_MCA_STATUS)
> + return -EINVAL;
> +
> + mce_setup(&m);
> +
> + m.extcpu = -1;
> + m.socketid = -1;
> +
> + for_each_possible_cpu(cpu) {
> + if (cpu_data(cpu).initial_apicid == lapic_id) {
I don't like that but I don't think we have a reverse mapping from LAPIC
ID to logical CPU numbers in the kernel...
> + m.extcpu = cpu;
> + m.socketid = cpu_data(m.extcpu).phys_proc_id;
m.socketid = cpu_data(cpu).phys_proc_id;
> + break;
> + }
> + }
> +
> + m.apicid = lapic_id;
> + m.bank = (ctx_info->msr_addr >> 4) & 0xFF;
> + m.status = *i_mce;
> + m.addr = *(i_mce + 1);
> + m.misc = *(i_mce + 2);
> + /* Skipping MCA_CONFIG */
> + m.ipid = *(i_mce + 4);
> + m.synd = *(i_mce + 5);
Is that structure after cper_ia_proc_ctx defined somewhere in the UEFI
spec so that you can cast to it directly instead of doing this ugly
pointer arithmetic?
> +
> + mce_log(&m);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(apei_mce_report_x86_error);
Why is this function exported?
If "no reason", you can fix the above one too, with a separate commit.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 00/12] ARM: use adr_l/ldr_l macros for PC-relative references
From: Nicolas Pitre @ 2020-09-14 14:06 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, linux-arm-kernel, Russell King, Linus Walleij,
Nick Desaulniers, Stefan Agner, Peter Smith, Marc Zyngier,
Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
On Mon, 14 Sep 2020, Ard Biesheuvel wrote:
> This is a respin of the adr_l/ldr_l code I wrote some years ago in the
> context of my KASLR proof of concept for 32-bit ARM.
>
> A new use case came up, in the form of Clang, which does not implement
> the 'adrl' pseudo-instruction in its assembler, and so for PC-relative
> references that don't fit into a ARM adr instruction, we need something
> else. Patch #2 addresses an actual Clang build issue of this nature, by
> replacing an occurrence of adrl with adr_l.
>
> I have included my existing cleanup patches that were built on top of the
> adr_l macro, which replace several occurrences of open coded arithmetic to
> calculate runtime addresses based on link time virtual addresses stored
> in literals.
>
> Note that all of these patches with the exception of #2 were reviewed or
> acked by Nico before, but given that this was a while ago (and the fact
Certainly it must have been, as I didn't remember much of it.
> that neither of us work for Linaro anymore), I have dropped these. Note
> that only patch #1 deviates significantly from the last version that I
> sent out, the remaining ones were just freshened up (and their commit
> logs slightly expanded).
Reviewed-by: Nicolas Pitre <nico@fluxnic.net>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Nicolas Pitre <nico@fluxnic.net>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Peter Smith <Peter.Smith@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
>
> Ard Biesheuvel (12):
> ARM: assembler: introduce adr_l, ldr_l and str_l macros
> ARM: efistub: replace adrl pseudo-op with adr_l macro invocation
> ARM: module: add support for place relative relocations
> ARM: head-common.S: use PC-relative insn sequence for __proc_info
> ARM: head-common.S: use PC-relative insn sequence for idmap creation
> ARM: head.S: use PC-relative insn sequence for secondary_data
> ARM: kernel: use relative references for UP/SMP alternatives
> ARM: head: use PC-relative insn sequence for __smp_alt
> ARM: sleep.S: use PC-relative insn sequence for
> sleep_save_sp/mpidr_hash
> ARM: head.S: use PC-relative insn sequences for __fixup_pv_table
> ARM: head.S: use PC relative insn sequence to calculate PHYS_OFFSET
> ARM: kvm: replace open coded VA->PA calculations with adr_l call
>
> arch/arm/boot/compressed/head.S | 18 +---
> arch/arm/include/asm/assembler.h | 88 ++++++++++++++++++-
> arch/arm/include/asm/elf.h | 5 ++
> arch/arm/include/asm/processor.h | 2 +-
> arch/arm/kernel/head-common.S | 22 ++---
> arch/arm/kernel/head.S | 90 +++++---------------
> arch/arm/kernel/hyp-stub.S | 27 +++---
> arch/arm/kernel/module.c | 20 ++++-
> arch/arm/kernel/sleep.S | 19 ++---
> 9 files changed, 159 insertions(+), 132 deletions(-)
>
> --
> 2.17.1
>
>
^ permalink raw reply
* Re: [PATCH 03/12] ARM: module: add support for place relative relocations
From: Nicolas Pitre @ 2020-09-14 13:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, linux-arm-kernel, Russell King, Linus Walleij,
Nick Desaulniers, Stefan Agner, Peter Smith, Marc Zyngier,
Will Deacon
In-Reply-To: <20200914095706.3985-4-ardb@kernel.org>
On Mon, 14 Sep 2020, Ard Biesheuvel wrote:
> When using the new adr_l/ldr_l/str_l macros to refer to external symbols
> from modules, the linker may emit place relative ELF relocations that
> need to be fixed up by the module loader. So add support for these.
Just wondering if that capability requirement should be added to the
module signature somehow...?
Maybe not. The MODULE_ARCH_VERMAGIC definition only contains things that
are configurable for a given build.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm/include/asm/elf.h | 5 +++++
> arch/arm/kernel/module.c | 20 ++++++++++++++++++--
> 2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
> index b078d992414b..0ac62a54b73c 100644
> --- a/arch/arm/include/asm/elf.h
> +++ b/arch/arm/include/asm/elf.h
> @@ -51,6 +51,7 @@ typedef struct user_fp elf_fpregset_t;
> #define R_ARM_NONE 0
> #define R_ARM_PC24 1
> #define R_ARM_ABS32 2
> +#define R_ARM_REL32 3
> #define R_ARM_CALL 28
> #define R_ARM_JUMP24 29
> #define R_ARM_TARGET1 38
> @@ -58,11 +59,15 @@ typedef struct user_fp elf_fpregset_t;
> #define R_ARM_PREL31 42
> #define R_ARM_MOVW_ABS_NC 43
> #define R_ARM_MOVT_ABS 44
> +#define R_ARM_MOVW_PREL_NC 45
> +#define R_ARM_MOVT_PREL 46
>
> #define R_ARM_THM_CALL 10
> #define R_ARM_THM_JUMP24 30
> #define R_ARM_THM_MOVW_ABS_NC 47
> #define R_ARM_THM_MOVT_ABS 48
> +#define R_ARM_THM_MOVW_PREL_NC 49
> +#define R_ARM_THM_MOVT_PREL 50
>
> /*
> * These are used to set parameters in the core dumps.
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index e15444b25ca0..beac45e89ba6 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -185,14 +185,24 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
> *(u32 *)loc |= offset & 0x7fffffff;
> break;
>
> + case R_ARM_REL32:
> + *(u32 *)loc += sym->st_value - loc;
> + break;
> +
> case R_ARM_MOVW_ABS_NC:
> case R_ARM_MOVT_ABS:
> + case R_ARM_MOVW_PREL_NC:
> + case R_ARM_MOVT_PREL:
> offset = tmp = __mem_to_opcode_arm(*(u32 *)loc);
> offset = ((offset & 0xf0000) >> 4) | (offset & 0xfff);
> offset = (offset ^ 0x8000) - 0x8000;
>
> offset += sym->st_value;
> - if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_ABS)
> + if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_PREL ||
> + ELF32_R_TYPE(rel->r_info) == R_ARM_MOVW_PREL_NC)
> + offset -= loc;
> + if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_ABS ||
> + ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_PREL)
> offset >>= 16;
>
> tmp &= 0xfff0f000;
> @@ -283,6 +293,8 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
>
> case R_ARM_THM_MOVW_ABS_NC:
> case R_ARM_THM_MOVT_ABS:
> + case R_ARM_THM_MOVW_PREL_NC:
> + case R_ARM_THM_MOVT_PREL:
> upper = __mem_to_opcode_thumb16(*(u16 *)loc);
> lower = __mem_to_opcode_thumb16(*(u16 *)(loc + 2));
>
> @@ -302,7 +314,11 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
> offset = (offset ^ 0x8000) - 0x8000;
> offset += sym->st_value;
>
> - if (ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_ABS)
> + if (ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_PREL ||
> + ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVW_PREL_NC)
> + offset -= loc;
> + if (ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_ABS ||
> + ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_PREL)
> offset >>= 16;
>
> upper = (u16)((upper & 0xfbf0) |
> --
> 2.17.1
>
>
^ permalink raw reply
* [PATCH 12/12] ARM: kvm: replace open coded VA->PA calculations with adr_l call
From: Ard Biesheuvel @ 2020-09-14 9:57 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Replace the open coded calculations of the actual physical address
of the KVM stub vector table with a single adr_l invocation.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/boot/compressed/head.S | 15 ++---------
arch/arm/kernel/hyp-stub.S | 27 +++++++++-----------
2 files changed, 14 insertions(+), 28 deletions(-)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 6a430d1f4d31..ab800f7e0dc1 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -472,15 +472,10 @@ dtb_check_done:
/*
* Compute the address of the hyp vectors after relocation.
- * This requires some arithmetic since we cannot directly
- * reference __hyp_stub_vectors in a PC-relative way.
* Call __hyp_set_vectors with the new address so that we
* can HVC again after the copy.
*/
-0: adr r0, 0b
- movw r1, #:lower16:__hyp_stub_vectors - 0b
- movt r1, #:upper16:__hyp_stub_vectors - 0b
- add r0, r0, r1
+ adr_l r0, __hyp_stub_vectors
sub r0, r0, r5
add r0, r0, r10
bl __hyp_set_vectors
@@ -631,17 +626,11 @@ not_relocated: mov r0, #0
cmp r0, #HYP_MODE @ if not booted in HYP mode...
bne __enter_kernel @ boot kernel directly
- adr r12, .L__hyp_reentry_vectors_offset
- ldr r0, [r12]
- add r0, r0, r12
-
+ adr_l r0, __hyp_reentry_vectors
bl __hyp_set_vectors
__HVC(0) @ otherwise bounce to hyp mode
b . @ should never be reached
-
- .align 2
-.L__hyp_reentry_vectors_offset: .long __hyp_reentry_vectors - .
#else
b __enter_kernel
#endif
diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index 26d8e03b1dd3..103d0bdb2b7e 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S
@@ -24,41 +24,38 @@ ENTRY(__boot_cpu_mode)
.text
/*
- * Save the primary CPU boot mode. Requires 3 scratch registers.
+ * Save the primary CPU boot mode. Requires 2 scratch registers.
*/
- .macro store_primary_cpu_mode reg1, reg2, reg3
+ .macro store_primary_cpu_mode reg1, reg2
mrs \reg1, cpsr
and \reg1, \reg1, #MODE_MASK
- adr \reg2, .L__boot_cpu_mode_offset
- ldr \reg3, [\reg2]
- str \reg1, [\reg2, \reg3]
+ str_l \reg1, __boot_cpu_mode, \reg2
.endm
/*
* Compare the current mode with the one saved on the primary CPU.
* If they don't match, record that fact. The Z bit indicates
* if there's a match or not.
- * Requires 3 additionnal scratch registers.
+ * Requires 2 additional scratch registers.
*/
- .macro compare_cpu_mode_with_primary mode, reg1, reg2, reg3
- adr \reg2, .L__boot_cpu_mode_offset
- ldr \reg3, [\reg2]
- ldr \reg1, [\reg2, \reg3]
+ .macro compare_cpu_mode_with_primary mode, reg1, reg2
+ adr_l \reg2, __boot_cpu_mode
+ ldr \reg1, [\reg2]
cmp \mode, \reg1 @ matches primary CPU boot mode?
orrne \reg1, \reg1, #BOOT_CPU_MODE_MISMATCH
- strne \reg1, [\reg2, \reg3] @ record what happened and give up
+ strne \reg1, [\reg2] @ record what happened and give up
.endm
#else /* ZIMAGE */
- .macro store_primary_cpu_mode reg1:req, reg2:req, reg3:req
+ .macro store_primary_cpu_mode reg1:req, reg2:req
.endm
/*
* The zImage loader only runs on one CPU, so we don't bother with mult-CPU
* consistency checking:
*/
- .macro compare_cpu_mode_with_primary mode, reg1, reg2, reg3
+ .macro compare_cpu_mode_with_primary mode, reg1, reg2
cmp \mode, \mode
.endm
@@ -73,7 +70,7 @@ ENTRY(__boot_cpu_mode)
*/
@ Call this from the primary CPU
ENTRY(__hyp_stub_install)
- store_primary_cpu_mode r4, r5, r6
+ store_primary_cpu_mode r4, r5
ENDPROC(__hyp_stub_install)
@ fall through...
@@ -87,7 +84,7 @@ ENTRY(__hyp_stub_install_secondary)
* If the secondary has booted with a different mode, give up
* immediately.
*/
- compare_cpu_mode_with_primary r4, r5, r6, r7
+ compare_cpu_mode_with_primary r4, r5, r6
retne lr
/*
--
2.17.1
^ permalink raw reply related
* [PATCH 11/12] ARM: head.S: use PC relative insn sequence to calculate PHYS_OFFSET
From: Ard Biesheuvel @ 2020-09-14 9:57 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Replace the open coded arithmetic with a simple adr_l/sub pair. This
removes some open coded arithmetic involving virtual addresses, avoids
literal pools on v7+, and slightly reduces the footprint of the code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/kernel/head.S | 10 ++--------
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 6f334df5d3b9..2e6127b2662e 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -103,10 +103,8 @@ ENTRY(stext)
#endif
#ifndef CONFIG_XIP_KERNEL
- adr r3, 2f
- ldmia r3, {r4, r8}
- sub r4, r3, r4 @ (PHYS_OFFSET - PAGE_OFFSET)
- add r8, r8, r4 @ PHYS_OFFSET
+ adr_l r8, _text @ __pa(_text)
+ sub r8, r8, #TEXT_OFFSET @ PHYS_OFFSET
#else
ldr r8, =PLAT_PHYS_OFFSET @ always constant in this case
#endif
@@ -158,10 +156,6 @@ ENTRY(stext)
1: b __enable_mmu
ENDPROC(stext)
.ltorg
-#ifndef CONFIG_XIP_KERNEL
-2: .long .
- .long PAGE_OFFSET
-#endif
/*
* Setup the initial page tables. We only setup the barest
--
2.17.1
^ permalink raw reply related
* [PATCH 10/12] ARM: head.S: use PC-relative insn sequences for __fixup_pv_table
From: Ard Biesheuvel @ 2020-09-14 9:57 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Replace the open coded PC relative offset calculations with adr_l
and mov_l invocations. This removes some open coded arithmetic involving
virtual addresses and avoids literal pools on v7+. Note that the footprint
of the code increases slightly, but the resulting code is a bit easier to
follow.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/kernel/head.S | 27 ++++++--------------
1 file changed, 8 insertions(+), 19 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 5f6436a40db1..6f334df5d3b9 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -576,14 +576,11 @@ ENDPROC(fixup_smp)
*/
__HEAD
__fixup_pv_table:
- adr r0, 1f
- ldmia r0, {r3-r7}
+ adr_l r6, __pv_phys_pfn_offset
+ adr_l r7, __pv_offset @ __pa(__pv_offset)
+ mov_l r3, __pv_offset @ __va(__pv_offset)
mvn ip, #0
- subs r3, r0, r3 @ PHYS_OFFSET - PAGE_OFFSET
- add r4, r4, r3 @ adjust table start address
- add r5, r5, r3 @ adjust table end address
- add r6, r6, r3 @ adjust __pv_phys_pfn_offset address
- add r7, r7, r3 @ adjust __pv_offset address
+ subs r3, r7, r3 @ PHYS_OFFSET - PAGE_OFFSET
mov r0, r8, lsr #PAGE_SHIFT @ convert to PFN
str r0, [r6] @ save computed PHYS_OFFSET to __pv_phys_pfn_offset
strcc ip, [r7, #HIGH_OFFSET] @ save to __pv_offset high bits
@@ -592,20 +589,15 @@ __fixup_pv_table:
THUMB( it ne @ cross section branch )
bne __error
str r3, [r7, #LOW_OFFSET] @ save to __pv_offset low bits
+ adr_l r4, __pv_table_begin
+ adr_l r5, __pv_table_end
b __fixup_a_pv_table
ENDPROC(__fixup_pv_table)
-
- .align
-1: .long .
- .long __pv_table_begin
- .long __pv_table_end
-2: .long __pv_phys_pfn_offset
- .long __pv_offset
+ .ltorg
.text
__fixup_a_pv_table:
- adr r0, 3f
- ldr r6, [r0]
+ mov_l r6, __pv_offset
add r6, r6, r3
ldr r0, [r6, #HIGH_OFFSET] @ pv_offset high word
ldr r6, [r6, #LOW_OFFSET] @ pv_offset low word
@@ -674,9 +666,6 @@ ARM_BE8(rev16 ip, ip)
#endif
ENDPROC(__fixup_a_pv_table)
- .align
-3: .long __pv_offset
-
ENTRY(fixup_pv_table)
stmfd sp!, {r4 - r7, lr}
mov r3, #0 @ no offset
--
2.17.1
^ permalink raw reply related
* [PATCH 09/12] ARM: sleep.S: use PC-relative insn sequence for sleep_save_sp/mpidr_hash
From: Ard Biesheuvel @ 2020-09-14 9:57 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Replace the open coded PC relative offset calculations with adr_l and
ldr_l invocations. This removes some open coded PC relative arithmetic,
avoids literal pools on v7+, and slightly reduces the footprint of the
code. Note that ALT_SMP() expects a single instruction so move the macro
invocation after it.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/kernel/sleep.S | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/arch/arm/kernel/sleep.S b/arch/arm/kernel/sleep.S
index 5dc8b80bb693..43077e11dafd 100644
--- a/arch/arm/kernel/sleep.S
+++ b/arch/arm/kernel/sleep.S
@@ -72,8 +72,9 @@ ENTRY(__cpu_suspend)
ldr r3, =sleep_save_sp
stmfd sp!, {r0, r1} @ save suspend func arg and pointer
ldr r3, [r3, #SLEEP_SAVE_SP_VIRT]
- ALT_SMP(ldr r0, =mpidr_hash)
+ ALT_SMP(W(nop)) @ don't use adr_l inside ALT_SMP()
ALT_UP_B(1f)
+ adr_l r0, mpidr_hash
/* This ldmia relies on the memory layout of the mpidr_hash struct */
ldmia r0, {r1, r6-r8} @ r1 = mpidr mask (r6,r7,r8) = l[0,1,2] shifts
compute_mpidr_hash r0, r6, r7, r8, r2, r1
@@ -147,9 +148,8 @@ no_hyp:
mov r1, #0
ALT_SMP(mrc p15, 0, r0, c0, c0, 5)
ALT_UP_B(1f)
- adr r2, mpidr_hash_ptr
- ldr r3, [r2]
- add r2, r2, r3 @ r2 = struct mpidr_hash phys address
+ adr_l r2, mpidr_hash @ r2 = struct mpidr_hash phys address
+
/*
* This ldmia relies on the memory layout of the mpidr_hash
* struct mpidr_hash.
@@ -157,10 +157,7 @@ no_hyp:
ldmia r2, { r3-r6 } @ r3 = mpidr mask (r4,r5,r6) = l[0,1,2] shifts
compute_mpidr_hash r1, r4, r5, r6, r0, r3
1:
- adr r0, _sleep_save_sp
- ldr r2, [r0]
- add r0, r0, r2
- ldr r0, [r0, #SLEEP_SAVE_SP_PHYS]
+ ldr_l r0, sleep_save_sp + SLEEP_SAVE_SP_PHYS
ldr r0, [r0, r1, lsl #2]
@ load phys pgd, stack, resume fn
@@ -177,12 +174,6 @@ ENDPROC(cpu_resume_arm)
ENDPROC(cpu_resume_no_hyp)
#endif
- .align 2
-_sleep_save_sp:
- .long sleep_save_sp - .
-mpidr_hash_ptr:
- .long mpidr_hash - . @ mpidr_hash struct offset
-
.data
.align 2
.type sleep_save_sp, #object
--
2.17.1
^ permalink raw reply related
* [PATCH 08/12] ARM: head: use PC-relative insn sequence for __smp_alt
From: Ard Biesheuvel @ 2020-09-14 9:57 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Now that calling __do_fixup_smp_on_up() can be done without passing
the physical-to-virtual offset in r3, we can replace the open coded
PC relative offset calculations with a pair of adr_l invocations. This
removes some open coded arithmetic involving virtual addresses, avoids
literal pools on v7+, and slightly reduces the footprint of the code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/kernel/head.S | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 3199d29f4480..5f6436a40db1 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -520,19 +520,11 @@ ARM_BE8(rev r0, r0) @ byteswap if big endian
retne lr
__fixup_smp_on_up:
- adr r0, 1f
- ldmia r0, {r3 - r5}
- sub r3, r0, r3
- add r4, r4, r3
- add r5, r5, r3
+ adr_l r4, __smpalt_begin
+ adr_l r5, __smpalt_end
b __do_fixup_smp_on_up
ENDPROC(__fixup_smp)
- .align
-1: .word .
- .word __smpalt_begin
- .word __smpalt_end
-
.pushsection .data
.align 2
.globl smp_on_up
--
2.17.1
^ permalink raw reply related
* [PATCH 07/12] ARM: kernel: use relative references for UP/SMP alternatives
From: Ard Biesheuvel @ 2020-09-14 9:57 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Currently, the .alt.smp.init section contains the virtual addresses
of the patch sites. Since patching may occur both before and after
switching into virtual mode, this requires some manual handling of
the address when applying the UP alternative.
Let's simplify this by using relative offsets in the table entries:
this allows us to simply add each entry's address to its contents,
regardless of whether we are running in virtual mode or not.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/include/asm/assembler.h | 4 ++--
arch/arm/include/asm/processor.h | 2 +-
arch/arm/kernel/head.S | 10 +++++-----
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 39e972eaaa3f..bf7501dfcb71 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -259,7 +259,7 @@
*/
#define ALT_UP(instr...) \
.pushsection ".alt.smp.init", "a" ;\
- .long 9998b ;\
+ .long 9998b - . ;\
9997: instr ;\
.if . - 9997b == 2 ;\
nop ;\
@@ -270,7 +270,7 @@
.popsection
#define ALT_UP_B(label) \
.pushsection ".alt.smp.init", "a" ;\
- .long 9998b ;\
+ .long 9998b - . ;\
W(b) . + (label - 9998b) ;\
.popsection
#else
diff --git a/arch/arm/include/asm/processor.h b/arch/arm/include/asm/processor.h
index b9241051e5cb..9e6b97286307 100644
--- a/arch/arm/include/asm/processor.h
+++ b/arch/arm/include/asm/processor.h
@@ -96,7 +96,7 @@ unsigned long get_wchan(struct task_struct *p);
#define __ALT_SMP_ASM(smp, up) \
"9998: " smp "\n" \
" .pushsection \".alt.smp.init\", \"a\"\n" \
- " .long 9998b\n" \
+ " .long 9998b - .\n" \
" " up "\n" \
" .popsection\n"
#else
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 40407a4727e0..3199d29f4480 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -546,14 +546,15 @@ smp_on_up:
__do_fixup_smp_on_up:
cmp r4, r5
reths lr
- ldmia r4!, {r0, r6}
- ARM( str r6, [r0, r3] )
- THUMB( add r0, r0, r3 )
+ ldmia r4, {r0, r6}
+ ARM( str r6, [r0, r4] )
+ THUMB( add r0, r0, r4 )
+ add r4, r4, #8
#ifdef __ARMEB__
THUMB( mov r6, r6, ror #16 ) @ Convert word order for big-endian.
#endif
THUMB( strh r6, [r0], #2 ) @ For Thumb-2, store as two halfwords
- THUMB( mov r6, r6, lsr #16 ) @ to be robust against misaligned r3.
+ THUMB( mov r6, r6, lsr #16 ) @ to be robust against misaligned r0.
THUMB( strh r6, [r0] )
b __do_fixup_smp_on_up
ENDPROC(__do_fixup_smp_on_up)
@@ -562,7 +563,6 @@ ENTRY(fixup_smp)
stmfd sp!, {r4 - r6, lr}
mov r4, r0
add r5, r0, r1
- mov r3, #0
bl __do_fixup_smp_on_up
ldmfd sp!, {r4 - r6, pc}
ENDPROC(fixup_smp)
--
2.17.1
^ permalink raw reply related
* [PATCH 06/12] ARM: head.S: use PC-relative insn sequence for secondary_data
From: Ard Biesheuvel @ 2020-09-14 9:57 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Replace the open coded PC relative offset calculations with adr_l
and ldr_l invocations. This removes some open coded arithmetic
involving virtual addresses, avoids literal pools on v7+, and slightly
reduces the footprint of the code.
Note that it also removes a stale comment about the contents of r6.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/kernel/head.S | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 7d8e2a296216..40407a4727e0 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -383,10 +383,8 @@ ENTRY(secondary_startup)
/*
* Use the page tables supplied from __cpu_up.
*/
- adr r4, __secondary_data
- ldmia r4, {r5, r7, r12} @ address to jump to after
- sub lr, r4, r5 @ mmu has been enabled
- add r3, r7, lr
+ adr_l r3, secondary_data
+ mov_l r12, __secondary_switched
ldrd r4, r5, [r3, #0] @ get secondary_data.pgdir
ARM_BE8(eor r4, r4, r5) @ Swap r5 and r4 in BE:
ARM_BE8(eor r5, r4, r5) @ it can be done in 3 steps
@@ -401,22 +399,13 @@ ARM_BE8(eor r4, r4, r5) @ without using a temp reg.
ENDPROC(secondary_startup)
ENDPROC(secondary_startup_arm)
- /*
- * r6 = &secondary_data
- */
ENTRY(__secondary_switched)
- ldr sp, [r7, #12] @ get secondary_data.stack
+ ldr_l r7, secondary_data + 12 @ get secondary_data.stack
+ mov sp, r7
mov fp, #0
b secondary_start_kernel
ENDPROC(__secondary_switched)
- .align
-
- .type __secondary_data, %object
-__secondary_data:
- .long .
- .long secondary_data
- .long __secondary_switched
#endif /* defined(CONFIG_SMP) */
--
2.17.1
^ permalink raw reply related
* [PATCH 05/12] ARM: head-common.S: use PC-relative insn sequence for idmap creation
From: Ard Biesheuvel @ 2020-09-14 9:56 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Replace the open coded PC relative offset calculations involving
__turn_mmu_on and __turn_mmu_on_end with a pair of adr_l invocations.
This removes some open coded arithmetic involving virtual addresses,
avoids literal pools on v7+, and slightly reduces the footprint of the
code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/kernel/head.S | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index f8904227e7fd..7d8e2a296216 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -224,11 +224,8 @@ __create_page_tables:
* Create identity mapping to cater for __enable_mmu.
* This identity mapping will be removed by paging_init().
*/
- adr r0, __turn_mmu_on_loc
- ldmia r0, {r3, r5, r6}
- sub r0, r0, r3 @ virt->phys offset
- add r5, r5, r0 @ phys __turn_mmu_on
- add r6, r6, r0 @ phys __turn_mmu_on_end
+ adr_l r5, __turn_mmu_on @ _pa(__turn_mmu_on)
+ adr_l r6, __turn_mmu_on_end @ _pa(__turn_mmu_on_end)
mov r5, r5, lsr #SECTION_SHIFT
mov r6, r6, lsr #SECTION_SHIFT
@@ -351,11 +348,6 @@ __create_page_tables:
ret lr
ENDPROC(__create_page_tables)
.ltorg
- .align
-__turn_mmu_on_loc:
- .long .
- .long __turn_mmu_on
- .long __turn_mmu_on_end
#if defined(CONFIG_SMP)
.text
--
2.17.1
^ permalink raw reply related
* [PATCH 04/12] ARM: head-common.S: use PC-relative insn sequence for __proc_info
From: Ard Biesheuvel @ 2020-09-14 9:56 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
Replace the open coded PC relative offset calculations with a pair of
adr_l invocations. This removes some open coded arithmetic involving
virtual addresses, avoids literal pools on v7+, and slightly reduces
the footprint of the code.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/kernel/head-common.S | 22 ++++++--------------
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
index 4a3982812a40..9a5ab6c19568 100644
--- a/arch/arm/kernel/head-common.S
+++ b/arch/arm/kernel/head-common.S
@@ -170,11 +170,12 @@ ENDPROC(lookup_processor_type)
* r9 = cpuid (preserved)
*/
__lookup_processor_type:
- adr r3, __lookup_processor_type_data
- ldmia r3, {r4 - r6}
- sub r3, r3, r4 @ get offset between virt&phys
- add r5, r5, r3 @ convert virt addresses to
- add r6, r6, r3 @ physical address space
+ /*
+ * Look in <asm/procinfo.h> for information about the __proc_info
+ * structure.
+ */
+ adr_l r5, __proc_info_begin
+ adr_l r6, __proc_info_end
1: ldmia r5, {r3, r4} @ value, mask
and r4, r4, r9 @ mask wanted bits
teq r3, r4
@@ -186,17 +187,6 @@ __lookup_processor_type:
2: ret lr
ENDPROC(__lookup_processor_type)
-/*
- * Look in <asm/procinfo.h> for information about the __proc_info structure.
- */
- .align 2
- .type __lookup_processor_type_data, %object
-__lookup_processor_type_data:
- .long .
- .long __proc_info_begin
- .long __proc_info_end
- .size __lookup_processor_type_data, . - __lookup_processor_type_data
-
__error_lpae:
#ifdef CONFIG_DEBUG_LL
adr r0, str_lpae
--
2.17.1
^ permalink raw reply related
* [PATCH 03/12] ARM: module: add support for place relative relocations
From: Ard Biesheuvel @ 2020-09-14 9:56 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
When using the new adr_l/ldr_l/str_l macros to refer to external symbols
from modules, the linker may emit place relative ELF relocations that
need to be fixed up by the module loader. So add support for these.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/include/asm/elf.h | 5 +++++
arch/arm/kernel/module.c | 20 ++++++++++++++++++--
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
index b078d992414b..0ac62a54b73c 100644
--- a/arch/arm/include/asm/elf.h
+++ b/arch/arm/include/asm/elf.h
@@ -51,6 +51,7 @@ typedef struct user_fp elf_fpregset_t;
#define R_ARM_NONE 0
#define R_ARM_PC24 1
#define R_ARM_ABS32 2
+#define R_ARM_REL32 3
#define R_ARM_CALL 28
#define R_ARM_JUMP24 29
#define R_ARM_TARGET1 38
@@ -58,11 +59,15 @@ typedef struct user_fp elf_fpregset_t;
#define R_ARM_PREL31 42
#define R_ARM_MOVW_ABS_NC 43
#define R_ARM_MOVT_ABS 44
+#define R_ARM_MOVW_PREL_NC 45
+#define R_ARM_MOVT_PREL 46
#define R_ARM_THM_CALL 10
#define R_ARM_THM_JUMP24 30
#define R_ARM_THM_MOVW_ABS_NC 47
#define R_ARM_THM_MOVT_ABS 48
+#define R_ARM_THM_MOVW_PREL_NC 49
+#define R_ARM_THM_MOVT_PREL 50
/*
* These are used to set parameters in the core dumps.
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index e15444b25ca0..beac45e89ba6 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -185,14 +185,24 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
*(u32 *)loc |= offset & 0x7fffffff;
break;
+ case R_ARM_REL32:
+ *(u32 *)loc += sym->st_value - loc;
+ break;
+
case R_ARM_MOVW_ABS_NC:
case R_ARM_MOVT_ABS:
+ case R_ARM_MOVW_PREL_NC:
+ case R_ARM_MOVT_PREL:
offset = tmp = __mem_to_opcode_arm(*(u32 *)loc);
offset = ((offset & 0xf0000) >> 4) | (offset & 0xfff);
offset = (offset ^ 0x8000) - 0x8000;
offset += sym->st_value;
- if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_ABS)
+ if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_PREL ||
+ ELF32_R_TYPE(rel->r_info) == R_ARM_MOVW_PREL_NC)
+ offset -= loc;
+ if (ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_ABS ||
+ ELF32_R_TYPE(rel->r_info) == R_ARM_MOVT_PREL)
offset >>= 16;
tmp &= 0xfff0f000;
@@ -283,6 +293,8 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
case R_ARM_THM_MOVW_ABS_NC:
case R_ARM_THM_MOVT_ABS:
+ case R_ARM_THM_MOVW_PREL_NC:
+ case R_ARM_THM_MOVT_PREL:
upper = __mem_to_opcode_thumb16(*(u16 *)loc);
lower = __mem_to_opcode_thumb16(*(u16 *)(loc + 2));
@@ -302,7 +314,11 @@ apply_relocate(Elf32_Shdr *sechdrs, const char *strtab, unsigned int symindex,
offset = (offset ^ 0x8000) - 0x8000;
offset += sym->st_value;
- if (ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_ABS)
+ if (ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_PREL ||
+ ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVW_PREL_NC)
+ offset -= loc;
+ if (ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_ABS ||
+ ELF32_R_TYPE(rel->r_info) == R_ARM_THM_MOVT_PREL)
offset >>= 16;
upper = (u16)((upper & 0xfbf0) |
--
2.17.1
^ permalink raw reply related
* [PATCH 02/12] ARM: efistub: replace adrl pseudo-op with adr_l macro invocation
From: Ard Biesheuvel @ 2020-09-14 9:56 UTC (permalink / raw)
To: linux-efi
Cc: linux-arm-kernel, Ard Biesheuvel, Russell King, Linus Walleij,
Nicolas Pitre, Nick Desaulniers, Stefan Agner, Peter Smith,
Marc Zyngier, Will Deacon
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
The ARM 'adrl' pseudo instruction is a bit problematic, as it does not
exist in Thumb mode, and it is not implemented by Clang either. Since
the Thumb variant has a slightly bigger range, it is sometimes necessary
to emit the 'adrl' variant in ARM mode where Thumb mode can use adr just
fine. However, that still leaves the Clang issue, which does not appear
to be supporting this any time soon.
So let's switch to the adr_l macro, which works for both ARM and Thumb,
and has unlimited range.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
arch/arm/boot/compressed/head.S | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 434a16982e34..6a430d1f4d31 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -1444,8 +1444,7 @@ ENTRY(efi_enter_kernel)
mov r4, r0 @ preserve image base
mov r8, r1 @ preserve DT pointer
- ARM( adrl r0, call_cache_fn )
- THUMB( adr r0, call_cache_fn )
+ adr_l r0, call_cache_fn
adr r1, 0f @ clean the region of code we
bl cache_clean_flush @ may run with the MMU off
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox