* 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: [PATCH v3 1/2] cper, apei, mce: Pass x86 CPER through the MCA handling chain
From: Smita Koralahalli Channabasappa @ 2020-09-14 21:16 UTC (permalink / raw)
To: Borislav Petkov
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: <20200914153024.GC680@zn.tnic>
On 9/14/20 10:30 AM, Borislav Petkov wrote:
> 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.
Okay, I will add it here. May be I should have just put it over here
rather than in cover letter.
>> Signed-off-by: Smita Koralahalli <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.
I have addressed this and will be sending it out as 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.
The mask value checks for MSR address of the first expected register
in the register layout of MCAX register space. Since the register
layout is implementation specific I'm just checking for the first
register. I will be adding a comment for this.
>> 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.
May be something like apei_smca_report_x86_error? Or probably an additional
SMCA-specific function call inside this generic APEI and MCA function call?
>> + 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...
Yes, we do not have a reverse mapping.
>> + 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?
The registers here are implementation specific and applies only for
SMCA systems. So I have used pointer arithmetic as it is not defined
in the spec.
>> +
>> + 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.
I will fix this..
Thanks,
Smita
>
^ permalink raw reply
* Re: [PATCH v3 2/2] x86/mce/dev-mcelog: Fix updating kflags in AMD systems
From: Smita Koralahalli Channabasappa @ 2020-09-14 21:20 UTC (permalink / raw)
To: Borislav Petkov
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: <20200914153420.GD680@zn.tnic>
On 9/14/20 10:34 AM, Borislav Petkov wrote:
> 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://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.kernel.org%2Fr%2F20200828203332.11129-3-Smita.KoralahalliChannabasappa%40amd.com&data=02%7C01%7CSmita.KoralahalliChannabasappa%40amd.com%7Cc452e9f80fe9459839c708d858c3a763%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637356944652485754&sdata=%2FhYQbBBNld1GtNX8%2FI6PERD0icYfy0e1k5zukQYI%2Fa4%3D&reserved=0
>>
>> 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?
Yes, this can be taken independently. I just tagged it along as I came
across the issue of missing error logs while trying to print error
records in the previous patch.
Thanks,
Smita
^ permalink raw reply
* [PATCH 1/2] efi/libstub: Add efi_warn and *_once logging helpers
From: Arvind Sankar @ 2020-09-14 21:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Limonciello, Mario, Jacobo Pantoja, Peter Jones, Arvind Sankar,
linux-efi
Add an efi_warn logging helper for warnings, and implement an analog of
printk_once for once-only logging.
Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
---
drivers/firmware/efi/libstub/efistub.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 85050f5a1b28..e33b9395bc23 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -55,11 +55,34 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle,
#define efi_info(fmt, ...) \
efi_printk(KERN_INFO fmt, ##__VA_ARGS__)
+#define efi_warn(fmt, ...) \
+ efi_printk(KERN_WARNING "WARNING: " fmt, ##__VA_ARGS__)
#define efi_err(fmt, ...) \
efi_printk(KERN_ERR "ERROR: " fmt, ##__VA_ARGS__)
#define efi_debug(fmt, ...) \
efi_printk(KERN_DEBUG "DEBUG: " fmt, ##__VA_ARGS__)
+#define efi_printk_once(fmt, ...) \
+({ \
+ static bool __print_once; \
+ bool __ret_print_once = !__print_once; \
+ \
+ if (!__print_once) { \
+ __print_once = true; \
+ efi_printk(fmt, ##__VA_ARGS__); \
+ } \
+ __ret_print_once; \
+})
+
+#define efi_info_once(fmt, ...) \
+ efi_printk_once(KERN_INFO fmt, ##__VA_ARGS__)
+#define efi_warn_once(fmt, ...) \
+ efi_printk_once(KERN_WARNING "WARNING: " fmt, ##__VA_ARGS__)
+#define efi_err_once(fmt, ...) \
+ efi_printk_once(KERN_ERR "ERROR: " fmt, ##__VA_ARGS__)
+#define efi_debug_once(fmt, ...) \
+ efi_printk_once(KERN_DEBUG "DEBUG: " fmt, ##__VA_ARGS__)
+
/* Helper macros for the usual case of using simple C variables: */
#ifndef fdt_setprop_inplace_var
#define fdt_setprop_inplace_var(fdt, node_offset, name, var) \
--
2.26.2
^ permalink raw reply related
* [PATCH 2/2] efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
From: Arvind Sankar @ 2020-09-14 21:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Limonciello, Mario, Jacobo Pantoja, Peter Jones, Arvind Sankar,
linux-efi
In-Reply-To: <20200914213535.933454-1-nivedita@alum.mit.edu>
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. This was verified with firmware revision 2.15.0 on a
Dell Precision T3620 by Jacobo Pontaja.
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.
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 | 101 +++++++++++++++++-
drivers/firmware/efi/libstub/efistub.h | 31 ++++++
drivers/firmware/efi/libstub/file.c | 5 +-
3 files changed, 135 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..aa8da0a49829 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -238,6 +238,102 @@ 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;
+
+ 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;
+
+ efi_warn_once(FW_BUG "LoadOptions is an EFI_LOAD_OPTION descriptor\n");
+ efi_warn_once(FW_BUG "Using OptionalData as a workaround\n");
+
+ *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 +343,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 e33b9395bc23..7997890c756d 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -711,6 +711,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)(
@@ -753,6 +782,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 related
* Re: [PATCH 01/12] ARM: assembler: introduce adr_l, ldr_l and str_l macros
From: Ard Biesheuvel @ 2020-09-15 7:35 UTC (permalink / raw)
To: linux-efi
Cc: Linux ARM, Russell King, Linus Walleij, Nicolas Pitre,
Nick Desaulniers, Stefan Agner, Peter Smith, Marc Zyngier,
Will Deacon
In-Reply-To: <20200914095706.3985-2-ardb@kernel.org>
On Mon, 14 Sep 2020 at 12:57, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Like arm64, ARM supports position independent code sequences that
> produce symbol references with a greater reach than the ordinary
> adr/ldr instructions. Since on ARM, the adrl pseudo-instruction is
> only supported in ARM mode (and not at all when using Clang), having
> a adr_l macro like we do on arm64 is useful, and increases symmetry
> as well.
>
> Currently, we use open coded instruction sequences involving literals
> and arithmetic operations. Instead, we can use movw/movt pairs on v7
> CPUs, circumventing the D-cache entirely.
>
> E.g., on v7+ CPUs, we can emit a PC-relative reference as follows:
>
> movw <reg>, #:lower16:<sym> - (1f + 8)
> movt <reg>, #:upper16:<sym> - (1f + 8)
> 1: add <reg>, <reg>, pc
>
> For older CPUs, we can emit the literal into a subsection, allowing it
> to be emitted out of line while retaining the ability to perform
> arithmetic on label offsets.
>
> E.g., on pre-v7 CPUs, we can emit a PC-relative reference as follows:
>
> ldr <reg>, 2f
> 1: add <reg>, <reg>, pc
> .subsection 1
> 2: .long <sym> - (1b + 8)
> .previous
>
> This is allowed by the assembler because, unlike ordinary sections,
> subsections are combined into a single section in the object file, and
> so the label references are not true cross-section references that are
> visible as relocations. (Subsections have been available in binutils
> since 2004 at least, so they should not cause any issues with older
> toolchains.)
>
> So use the above to implement the macros mov_l, adr_l, ldr_l and str_l,
> all of which will use movw/movt pairs on v7 and later CPUs, and use
> PC-relative literals otherwise.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/arm/include/asm/assembler.h | 84 ++++++++++++++++++++
> 1 file changed, 84 insertions(+)
>
> diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> index feac2c8b86f2..39e972eaaa3f 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -494,4 +494,88 @@ THUMB( orr \reg , \reg , #PSR_T_BIT )
> #define _ASM_NOKPROBE(entry)
> #endif
>
> + .macro __adldst_l, op, reg, sym, tmp, c
> + .if __LINUX_ARM_ARCH__ < 7
> + ldr\c \tmp, .La\@
> + .subsection 1
> + .align 2
> +.La\@: .long \sym - .Lpc\@
> + .previous
> + .else
> + .ifnb \c
> + THUMB( ittt \c )
> + .endif
> + movw\c \tmp, #:lower16:\sym - .Lpc\@
> + movt\c \tmp, #:upper16:\sym - .Lpc\@
> + .endif
> +
> +#ifndef CONFIG_THUMB2_KERNEL
> + .set .Lpc\@, . + 8 // PC bias
> + .ifc \op, add
> + add\c \reg, \tmp, pc
> + .else
> + \op\c \reg, [\tmp, pc]
This should be
\op\c \reg, [pc, \tmp]
> + .endif
> +#else
> +.Lb\@: add\c \tmp, \tmp, pc
> + /*
> + * In Thumb-2 builds, the PC bias depends on whether we are currently
> + * emitting into a .arm or a .thumb section. The size of the add opcode
> + * above will be 2 bytes when emitting in Thumb mode and 4 bytes when
> + * emitting in ARM mode, so let's use this to account for the bias.
> + */
> + .set .Lpc\@, . + (. - .Lb\@)
> +
> + .ifnc \op, add
> + \op\c \reg, [\tmp]
> + .endif
> +#endif
> + .endm
> +
> + /*
> + * mov_l - move a constant value or [relocated] address into a register
> + */
> + .macro mov_l, dst:req, imm:req
> + .if __LINUX_ARM_ARCH__ < 7
> + ldr \dst, =\imm
> + .else
> + movw \dst, #:lower16:\imm
> + movt \dst, #:upper16:\imm
> + .endif
> + .endm
> +
> + /*
> + * adr_l - adr pseudo-op with unlimited range
> + *
> + * @dst: destination register
> + * @sym: name of the symbol
> + * @cond: conditional opcode suffix
> + */
> + .macro adr_l, dst:req, sym:req, cond
> + __adldst_l add, \dst, \sym, \dst, \cond
> + .endm
> +
> + /*
> + * ldr_l - ldr <literal> pseudo-op with unlimited range
> + *
> + * @dst: destination register
> + * @sym: name of the symbol
> + * @cond: conditional opcode suffix
> + */
> + .macro ldr_l, dst:req, sym:req, cond
> + __adldst_l ldr, \dst, \sym, \dst, \cond
> + .endm
> +
> + /*
> + * str_l - str <literal> pseudo-op with unlimited range
> + *
> + * @src: source register
> + * @sym: name of the symbol
> + * @tmp: mandatory scratch register
> + * @cond: conditional opcode suffix
> + */
> + .macro str_l, src:req, sym:req, tmp:req, cond
> + __adldst_l str, \src, \sym, \tmp, \cond
> + .endm
> +
> #endif /* __ASM_ASSEMBLER_H__ */
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH v3 1/2] cper, apei, mce: Pass x86 CPER through the MCA handling chain
From: Borislav Petkov @ 2020-09-15 8:15 UTC (permalink / raw)
To: Smita Koralahalli Channabasappa
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: <a28aa613-8353-0052-31f6-34bc733abf59@amd.com>
On Mon, Sep 14, 2020 at 04:16:10PM -0500, Smita Koralahalli Channabasappa wrote:
> May be something like apei_smca_report_x86_error? Or probably an additional
> SMCA-specific function call inside this generic APEI and MCA function call?
apei_smca_report_x86_error() sounds ok. If there's need for any
additional MCA handling from BERT, then that can be extended/refactored
then.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 2/2] efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
From: Ard Biesheuvel @ 2020-09-15 15:09 UTC (permalink / raw)
To: Arvind Sankar; +Cc: Limonciello, Mario, Jacobo Pantoja, Peter Jones, linux-efi
In-Reply-To: <20200914213535.933454-2-nivedita@alum.mit.edu>
On Tue, 15 Sep 2020 at 00:35, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> 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. This was verified with firmware revision 2.15.0 on a
> Dell Precision T3620 by Jacobo Pontaja.
>
> 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.
>
> 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/
I'll queue these up for v5.10
Thanks all
> ---
> .../firmware/efi/libstub/efi-stub-helper.c | 101 +++++++++++++++++-
> drivers/firmware/efi/libstub/efistub.h | 31 ++++++
> drivers/firmware/efi/libstub/file.c | 5 +-
> 3 files changed, 135 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..aa8da0a49829 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -238,6 +238,102 @@ 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;
> +
> + 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;
> +
> + efi_warn_once(FW_BUG "LoadOptions is an EFI_LOAD_OPTION descriptor\n");
> + efi_warn_once(FW_BUG "Using OptionalData as a workaround\n");
> +
> + *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 +343,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 e33b9395bc23..7997890c756d 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -711,6 +711,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)(
> @@ -753,6 +782,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 1/2] edac,ghes,cper: Add Row Extension to Memory Error Record
From: Ard Biesheuvel @ 2020-09-15 17:12 UTC (permalink / raw)
To: Borislav Petkov
Cc: Alex Kluver, linux-edac, linux-efi, Linux Kernel Mailing List,
mchehab, russ.anderson, Dimitri Sivanich, kluveralex
In-Reply-To: <CAMj1kXHmVhB88qZc-1mHAD1ovNJQnWRBncmQJTR_4+kV0fXG5w@mail.gmail.com>
On Tue, 15 Sep 2020 at 20:07, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 15 Sep 2020 at 19:33, Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Wed, Aug 19, 2020 at 09:35:43AM -0500, Alex Kluver wrote:
> > > Memory errors could be printed with incorrect row values since the DIMM
> > > size has outgrown the 16 bit row field in the CPER structure. UEFI
> > > Specification Version 2.8 has increased the size of row by allowing it to
> > > use the first 2 bits from a previously reserved space within the structure.
> > >
> > > When needed, add the extension bits to the row value printed.
> > >
> > > Based on UEFI 2.8 Table 299. Memory Error Record
> > >
> > > Reviewed-by: Kyle Meyer <kyle.meyer@hpe.com>
> > > Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> > > Tested-by: Russ Anderson <russ.anderson@hpe.com>
> > > Signed-off-by: Alex Kluver <alex.kluver@hpe.com>
> > > ---
> > >
> > > 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.
> > >
> > > ---
> > > drivers/edac/ghes_edac.c | 8 ++++++--
> > > drivers/firmware/efi/cper.c | 9 +++++++--
> > > include/linux/cper.h | 16 ++++++++++++++--
> > > 3 files changed, 27 insertions(+), 6 deletions(-)
> >
> > For the EDAC bits:
> >
> > Acked-by: Borislav Petkov <bp@suse.de>
> >
> > Also, I could take both through the EDAC tree, if people prefer.
> >
>
> I'll take this via the EFI tree - I was just preparing the branch for
> a PR anyways.
Alex - these patches do not apply cleanly. Could you please respin
them on top of the next branch in
https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git?
Boris - do you anticipate any conflicts? If so, please take these via
the EDAC tree - the CPER code is mostly self contained so I don't
expect any conflicts with the EFI tree in that case.
^ permalink raw reply
* Re: [PATCH v2 1/2] edac,ghes,cper: Add Row Extension to Memory Error Record
From: Borislav Petkov @ 2020-09-15 17:19 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Alex Kluver, linux-edac, linux-efi, Linux Kernel Mailing List,
mchehab, russ.anderson, Dimitri Sivanich, kluveralex
In-Reply-To: <CAMj1kXGvfiqZz-j5=LU0Z6yYCkr24pCz6aJS62QL8cBYUP_S=w@mail.gmail.com>
On Tue, Sep 15, 2020 at 08:12:31PM +0300, Ard Biesheuvel wrote:
> Boris - do you anticipate any conflicts? If so, please take these via
> the EDAC tree - the CPER code is mostly self contained so I don't
> expect any conflicts with the EFI tree in that case.
None so far, and I applied them for testing ontop of my EDAC queue for
5.10 so it should be all good. But if you want me, I can test-merge your
branch once ready, just in case...
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH 01/12] ARM: assembler: introduce adr_l, ldr_l and str_l macros
From: Nick Desaulniers @ 2020-09-15 17:51 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, Linux ARM, Russell King, Linus Walleij, Nicolas Pitre,
Stefan Agner, Peter Smith, Marc Zyngier, Will Deacon
In-Reply-To: <CAMj1kXFmF_24d-7W8AWTJR-PCcja7bUdHhYKptGQmsV4vNp=sA@mail.gmail.com>
On Tue, Sep 15, 2020 at 12:35 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 14 Sep 2020 at 12:57, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
> > index feac2c8b86f2..39e972eaaa3f 100644
> > --- a/arch/arm/include/asm/assembler.h
> > +++ b/arch/arm/include/asm/assembler.h
> > @@ -494,4 +494,88 @@ THUMB( orr \reg , \reg , #PSR_T_BIT )
> > #define _ASM_NOKPROBE(entry)
> > #endif
> >
> > + .macro __adldst_l, op, reg, sym, tmp, c
> > + .if __LINUX_ARM_ARCH__ < 7
> > + ldr\c \tmp, .La\@
> > + .subsection 1
> > + .align 2
> > +.La\@: .long \sym - .Lpc\@
> > + .previous
> > + .else
> > + .ifnb \c
> > + THUMB( ittt \c )
> > + .endif
> > + movw\c \tmp, #:lower16:\sym - .Lpc\@
> > + movt\c \tmp, #:upper16:\sym - .Lpc\@
> > + .endif
> > +
> > +#ifndef CONFIG_THUMB2_KERNEL
> > + .set .Lpc\@, . + 8 // PC bias
> > + .ifc \op, add
> > + add\c \reg, \tmp, pc
> > + .else
> > + \op\c \reg, [\tmp, pc]
>
> This should be
>
> \op\c \reg, [pc, \tmp]
That looks better. I had tested this series yesterday and wasn't able
to build with either toolchains. Was going to ask if I was using the
wrong base or if there was an issue with my version binutils. All
good now, I'll try to hammer on this a bit.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH 00/12] ARM: use adr_l/ldr_l macros for PC-relative references
From: Nick Desaulniers @ 2020-09-15 19:32 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, Linux ARM, Russell King, Linus Walleij, Nicolas Pitre,
Stefan Agner, Peter Smith, Marc Zyngier, Will Deacon,
clang-built-linux, Jian Cai
In-Reply-To: <20200914095706.3985-1-ardb@kernel.org>
On Mon, Sep 14, 2020 at 2:57 AM Ard Biesheuvel <ardb@kernel.org> 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
> 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).
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Thanks for the series, Ard. I was able to compile and boot the
following with this series (and the fixup to 01/12 applied):
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 aspeed_g5_defconfig
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 multi_v5_defconfig
(So ARM v7/GCC, ARM v5,6,7/LLVM). (Technically, the v6 is not
booting, but it's not related to this series and fails to boot without
the series as well. Our CI on -next is red for an unrelated issue
which is masking the regression).
I was also able to build+boot the defconfig with v5 and v7 with some
configs disabled and a few hacks with LLVM_IAS=1. This series allowed
me to get further in the build/testing, and I have a few new bugs to
go chase. If anyone's interested:
https://github.com/ClangBuiltLinux/linux/issues/1154
https://github.com/ClangBuiltLinux/linux/issues/1155
so we're still a handful of bugs away from LLVM_IAS=1 with ARCH=arm,
but we're steadily chipping away at it, and this series is a big help.
Lest it look like there's only kernel fixes in this area, Jian's
https://reviews.llvm.org/D69411 recently was a big help, specifically
for ARCH=arm LLVM_IAS=1.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH v2 1/2] edac,ghes,cper: Add Row Extension to Memory Error Record
From: Ard Biesheuvel @ 2020-09-15 17:07 UTC (permalink / raw)
To: Borislav Petkov
Cc: Alex Kluver, linux-edac, linux-efi, Linux Kernel Mailing List,
mchehab, russ.anderson, Dimitri Sivanich, kluveralex
In-Reply-To: <20200915163312.GO14436@zn.tnic>
On Tue, 15 Sep 2020 at 19:33, Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Aug 19, 2020 at 09:35:43AM -0500, Alex Kluver wrote:
> > Memory errors could be printed with incorrect row values since the DIMM
> > size has outgrown the 16 bit row field in the CPER structure. UEFI
> > Specification Version 2.8 has increased the size of row by allowing it to
> > use the first 2 bits from a previously reserved space within the structure.
> >
> > When needed, add the extension bits to the row value printed.
> >
> > Based on UEFI 2.8 Table 299. Memory Error Record
> >
> > Reviewed-by: Kyle Meyer <kyle.meyer@hpe.com>
> > Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> > Tested-by: Russ Anderson <russ.anderson@hpe.com>
> > Signed-off-by: Alex Kluver <alex.kluver@hpe.com>
> > ---
> >
> > 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.
> >
> > ---
> > drivers/edac/ghes_edac.c | 8 ++++++--
> > drivers/firmware/efi/cper.c | 9 +++++++--
> > include/linux/cper.h | 16 ++++++++++++++--
> > 3 files changed, 27 insertions(+), 6 deletions(-)
>
> For the EDAC bits:
>
> Acked-by: Borislav Petkov <bp@suse.de>
>
> Also, I could take both through the EDAC tree, if people prefer.
>
I'll take this via the EFI tree - I was just preparing the branch for
a PR anyways.
^ permalink raw reply
* Re: [PATCH 00/12] ARM: use adr_l/ldr_l macros for PC-relative references
From: Ard Biesheuvel @ 2020-09-15 21:29 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-efi, Linux ARM, Russell King, Linus Walleij, Nicolas Pitre,
Stefan Agner, Peter Smith, Marc Zyngier, Will Deacon,
clang-built-linux, Jian Cai
In-Reply-To: <CAKwvOdmSXm7cV3hB_Yp=DD0RwwDHtPzzDBU8Xj5kBREn3xqYdA@mail.gmail.com>
On Tue, 15 Sep 2020 at 22:32, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Mon, Sep 14, 2020 at 2:57 AM Ard Biesheuvel <ardb@kernel.org> 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
> > 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).
>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Thanks for the series, Ard. I was able to compile and boot the
> following with this series (and the fixup to 01/12 applied):
>
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make defconfig
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 defconfig
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 aspeed_g5_defconfig
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make LLVM=1 multi_v5_defconfig
>
> (So ARM v7/GCC, ARM v5,6,7/LLVM). (Technically, the v6 is not
> booting, but it's not related to this series and fails to boot without
> the series as well. Our CI on -next is red for an unrelated issue
> which is masking the regression).
>
Excellent, thanks for testing. Do you have any coverage for Thumb2
builds as well? (CONFIG_THUMB2_KERNEL=y)
> I was also able to build+boot the defconfig with v5 and v7 with some
> configs disabled and a few hacks with LLVM_IAS=1. This series allowed
> me to get further in the build/testing, and I have a few new bugs to
> go chase. If anyone's interested:
> https://github.com/ClangBuiltLinux/linux/issues/1154
> https://github.com/ClangBuiltLinux/linux/issues/1155
> so we're still a handful of bugs away from LLVM_IAS=1 with ARCH=arm,
> but we're steadily chipping away at it, and this series is a big help.
> Lest it look like there's only kernel fixes in this area, Jian's
> https://reviews.llvm.org/D69411 recently was a big help, specifically
> for ARCH=arm LLVM_IAS=1.
> --
> Thanks,
> ~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH v2 2/2] cper,edac,efi: Memory Error Record: bank group/address and chip id
From: Borislav Petkov @ 2020-09-15 16:36 UTC (permalink / raw)
To: Alex Kluver
Cc: linux-edac, linux-efi, linux-kernel, ardb, mchehab, russ.anderson,
dimitri.sivanich, kluveralex
In-Reply-To: <20200819143544.155096-3-alex.kluver@hpe.com>
On Wed, Aug 19, 2020 at 09:35:44AM -0500, Alex Kluver wrote:
> Updates to the UEFI 2.8 Memory Error Record allow splitting the bank field
> into bank address and bank group, and using the last 3 bits of the extended
> field as a chip identifier.
>
> When needed, print correct version of bank field, bank group, and chip
> identification
>
> Based on UEFI 2.8 Table 299. Memory Error Record
Whoever commits this - those last two sentences need fullstops.
> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> Reviewed-by: Kyle Meyer <kyle.meyer@hpe.com>
> Reviewed-by: Russ Anderson <russ.anderson@hpe.com>
> Signed-off-by: Alex Kluver <alex.kluver@hpe.com>
> ---
>
> 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.
>
> ---
> drivers/edac/ghes_edac.c | 9 +++++++++
> drivers/firmware/efi/cper.c | 9 +++++++++
> include/linux/cper.h | 8 ++++++++
> 3 files changed, 26 insertions(+)
For the EDAC bits:
Acked-by: Borislav Petkov <bp@suse.de>
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 1/2] edac,ghes,cper: Add Row Extension to Memory Error Record
From: Borislav Petkov @ 2020-09-15 16:33 UTC (permalink / raw)
To: Alex Kluver
Cc: linux-edac, linux-efi, linux-kernel, ardb, mchehab, russ.anderson,
dimitri.sivanich, kluveralex
In-Reply-To: <20200819143544.155096-2-alex.kluver@hpe.com>
On Wed, Aug 19, 2020 at 09:35:43AM -0500, Alex Kluver wrote:
> Memory errors could be printed with incorrect row values since the DIMM
> size has outgrown the 16 bit row field in the CPER structure. UEFI
> Specification Version 2.8 has increased the size of row by allowing it to
> use the first 2 bits from a previously reserved space within the structure.
>
> When needed, add the extension bits to the row value printed.
>
> Based on UEFI 2.8 Table 299. Memory Error Record
>
> Reviewed-by: Kyle Meyer <kyle.meyer@hpe.com>
> Reviewed-by: Steve Wahl <steve.wahl@hpe.com>
> Tested-by: Russ Anderson <russ.anderson@hpe.com>
> Signed-off-by: Alex Kluver <alex.kluver@hpe.com>
> ---
>
> 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.
>
> ---
> drivers/edac/ghes_edac.c | 8 ++++++--
> drivers/firmware/efi/cper.c | 9 +++++++--
> include/linux/cper.h | 16 ++++++++++++++--
> 3 files changed, 27 insertions(+), 6 deletions(-)
For the EDAC bits:
Acked-by: Borislav Petkov <bp@suse.de>
Also, I could take both through the EDAC tree, if people prefer.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 0/2] UEFI v2.8 Memory Error Record Updates
From: Ard Biesheuvel @ 2020-09-15 15:07 UTC (permalink / raw)
To: Alex Kluver
Cc: linux-edac, linux-efi, Linux Kernel Mailing List, mchehab,
Borislav Petkov, russ.anderson, Dimitri Sivanich, kluveralex
In-Reply-To: <20200819143544.155096-1-alex.kluver@hpe.com>
On Wed, 19 Aug 2020 at 17:36, Alex Kluver <alex.kluver@hpe.com> 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.
> ---
> 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(-)
>
For the series,
Acked-by: Ard Biesheuvel <ardb@kernel.org>
^ permalink raw reply
* Re: [PATCH 00/12] ARM: use adr_l/ldr_l macros for PC-relative references
From: Nick Desaulniers @ 2020-09-15 23:31 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-efi, Linux ARM, Russell King, Linus Walleij, Nicolas Pitre,
Stefan Agner, Peter Smith, Marc Zyngier, Will Deacon,
clang-built-linux, Jian Cai
In-Reply-To: <CAMj1kXGuFFUyT48EYvzFmjCP4QZi_Sk_GpBrBCaHjP7HKLhjBA@mail.gmail.com>
On Tue, Sep 15, 2020 at 2:30 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Excellent, thanks for testing. Do you have any coverage for Thumb2
> builds as well? (CONFIG_THUMB2_KERNEL=y)
Ah, right, manually testing ARCH=arm defconfig with
CONFIG_THUMB2_KERNEL enabled via menuconfig:
Builds and boots for clang.
(Also needed https://lore.kernel.org/lkml/20200915225751.274531-1-ndesaulniers@google.com/T/#u
for an unrelated issue).
For GCC, I observe:
arch/arm/vfp/vfphw.o: in function `vfp_support_entry':
(.text+0xa): relocation truncated to fit: R_ARM_THM_JUMP19 against
symbol `vfp_kmode_exception' defined in .text.unlikely section in
arch/arm/vfp/vfpmodule.o
There doesn't seem to be a config option to work around that for now.
Though it's not caused by your series; if I drop your series, I can
still reproduce.
Is there a different config I should be testing rather than
defconfig+manual testing, like one of the existing configs? Looks like
only milbeaut_m10v_defconfig enable THUMB2 by default. I should add
that to my CI and kernelCI for coverage with clang.
https://github.com/ClangBuiltLinux/continuous-integration/issues/94#issuecomment-693030376
milbeaut_m10v_defconfig
builds with your series for clang. Looks like that config may be
currently broken for GCC?
Looks like it doesn't boot with Clang, so I'll need to debug that.
Again, both of the two past sentences are regardless of your series.
So your series still LGTM.
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* [efi:urgent] BUILD SUCCESS 46908326c6b801201f1e46f5ed0db6e85bef74ae
From: kernel test robot @ 2020-09-16 4:33 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git urgent
branch HEAD: 46908326c6b801201f1e46f5ed0db6e85bef74ae efi: efibc: check for efivars write capability
elapsed time: 723m
configs tested: 173
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
mips rt305x_defconfig
arm corgi_defconfig
sparc64 defconfig
powerpc ppc6xx_defconfig
m68k mac_defconfig
sh sh03_defconfig
arm prima2_defconfig
m68k m5272c3_defconfig
arm pxa168_defconfig
powerpc walnut_defconfig
powerpc ppa8548_defconfig
mips bcm63xx_defconfig
arm integrator_defconfig
arc nsimosci_hs_defconfig
sh urquell_defconfig
powerpc ep8248e_defconfig
arm assabet_defconfig
powerpc tqm8548_defconfig
sh hp6xx_defconfig
arm sunxi_defconfig
powerpc mvme5100_defconfig
arm footbridge_defconfig
arm palmz72_defconfig
arm vexpress_defconfig
mips ci20_defconfig
sh j2_defconfig
arm ixp4xx_defconfig
xtensa defconfig
mips cobalt_defconfig
sh rsk7264_defconfig
mips bmips_stb_defconfig
powerpc fsp2_defconfig
arc nps_defconfig
arm dove_defconfig
sh sh7710voipgw_defconfig
arm shannon_defconfig
sh kfr2r09-romimage_defconfig
powerpc g5_defconfig
powerpc storcenter_defconfig
m68k sun3_defconfig
i386 alldefconfig
powerpc tqm5200_defconfig
m68k m5407c3_defconfig
mips loongson1c_defconfig
powerpc holly_defconfig
mips maltaaprp_defconfig
arm ebsa110_defconfig
arm gemini_defconfig
arc tb10x_defconfig
powerpc tqm8560_defconfig
powerpc ppc64e_defconfig
xtensa iss_defconfig
sh se7619_defconfig
powerpc64 alldefconfig
powerpc mpc836x_mds_defconfig
xtensa xip_kc705_defconfig
arm spear6xx_defconfig
mips jazz_defconfig
powerpc rainier_defconfig
arm lpc18xx_defconfig
sh secureedge5410_defconfig
mips decstation_64_defconfig
powerpc powernv_defconfig
powerpc mpc832x_mds_defconfig
arm pcm027_defconfig
powerpc eiger_defconfig
powerpc mpc834x_mds_defconfig
powerpc tqm8xx_defconfig
powerpc katmai_defconfig
sh apsh4ad0a_defconfig
powerpc wii_defconfig
arm exynos_defconfig
ia64 bigsur_defconfig
arm bcm2835_defconfig
sh se7724_defconfig
mips db1xxx_defconfig
arm rpc_defconfig
powerpc warp_defconfig
sh rts7751r2d1_defconfig
arm shmobile_defconfig
powerpc taishan_defconfig
c6x evmc6474_defconfig
powerpc makalu_defconfig
mips malta_kvm_guest_defconfig
sh se7751_defconfig
sh sh7770_generic_defconfig
ia64 zx1_defconfig
powerpc mpc8315_rdb_defconfig
powerpc iss476-smp_defconfig
mips tb0219_defconfig
arm mini2440_defconfig
arm lpd270_defconfig
mips sb1250_swarm_defconfig
mips bcm47xx_defconfig
sh dreamcast_defconfig
powerpc tqm8540_defconfig
powerpc kmeter1_defconfig
arm simpad_defconfig
mips tb0287_defconfig
sh defconfig
powerpc ep88xc_defconfig
sh sh7724_generic_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a006-20200916
x86_64 randconfig-a004-20200916
x86_64 randconfig-a003-20200916
x86_64 randconfig-a002-20200916
x86_64 randconfig-a001-20200916
x86_64 randconfig-a005-20200916
i386 randconfig-a004-20200916
i386 randconfig-a006-20200916
i386 randconfig-a003-20200916
i386 randconfig-a001-20200916
i386 randconfig-a002-20200916
i386 randconfig-a005-20200916
i386 randconfig-a015-20200916
i386 randconfig-a014-20200916
i386 randconfig-a011-20200916
i386 randconfig-a013-20200916
i386 randconfig-a016-20200916
i386 randconfig-a012-20200916
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a014-20200916
x86_64 randconfig-a011-20200916
x86_64 randconfig-a016-20200916
x86_64 randconfig-a012-20200916
x86_64 randconfig-a015-20200916
x86_64 randconfig-a013-20200916
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH 00/12] ARM: use adr_l/ldr_l macros for PC-relative references
From: Ard Biesheuvel @ 2020-09-16 5:54 UTC (permalink / raw)
To: Nick Desaulniers
Cc: linux-efi, Linux ARM, Russell King, Linus Walleij, Nicolas Pitre,
Stefan Agner, Peter Smith, Marc Zyngier, Will Deacon,
clang-built-linux, Jian Cai
In-Reply-To: <CAKwvOdmkNgi_+kfauTSLwtpVChipW851_KGJG+gBbhwRxJJORA@mail.gmail.com>
On Wed, 16 Sep 2020 at 02:31, Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Tue, Sep 15, 2020 at 2:30 PM Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > Excellent, thanks for testing. Do you have any coverage for Thumb2
> > builds as well? (CONFIG_THUMB2_KERNEL=y)
>
> Ah, right, manually testing ARCH=arm defconfig with
> CONFIG_THUMB2_KERNEL enabled via menuconfig:
>
> Builds and boots for clang.
>
> (Also needed https://lore.kernel.org/lkml/20200915225751.274531-1-ndesaulniers@google.com/T/#u
> for an unrelated issue).
>
> For GCC, I observe:
>
> arch/arm/vfp/vfphw.o: in function `vfp_support_entry':
> (.text+0xa): relocation truncated to fit: R_ARM_THM_JUMP19 against
> symbol `vfp_kmode_exception' defined in .text.unlikely section in
> arch/arm/vfp/vfpmodule.o
>
Interesting. And this is using ld.bfd ?
> There doesn't seem to be a config option to work around that for now.
> Though it's not caused by your series; if I drop your series, I can
> still reproduce.
>
> Is there a different config I should be testing rather than
> defconfig+manual testing, like one of the existing configs? Looks like
> only milbeaut_m10v_defconfig enable THUMB2 by default. I should add
> that to my CI and kernelCI for coverage with clang.
> https://github.com/ClangBuiltLinux/continuous-integration/issues/94#issuecomment-693030376
>
> milbeaut_m10v_defconfig
> builds with your series for clang. Looks like that config may be
> currently broken for GCC?
> Looks like it doesn't boot with Clang, so I'll need to debug that.
> Again, both of the two past sentences are regardless of your series.
> So your series still LGTM.
Cheers.
I usually build multi_v7_defconfig with/wihout CONFIG_LPAE x
with/without CONFIG_THUMB2_KERNEL (LPAE affects the size of
dma_addr_t, and uses a different page table format, so it is a useful
bit to flick in testing)
^ permalink raw reply
* [efi:next] BUILD SUCCESS 973c2d60254e9548b4f8568e1a349f3852315dd5
From: kernel test robot @ 2020-09-16 13:00 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-efi
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
branch HEAD: 973c2d60254e9548b4f8568e1a349f3852315dd5 efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
elapsed time: 1230m
configs tested: 126
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
arm socfpga_defconfig
sh r7785rp_defconfig
s390 alldefconfig
mips nlm_xlp_defconfig
microblaze nommu_defconfig
sh se7780_defconfig
i386 alldefconfig
powerpc tqm5200_defconfig
m68k m5407c3_defconfig
mips loongson1c_defconfig
powerpc holly_defconfig
mips maltaaprp_defconfig
powerpc ep88xc_defconfig
arm zx_defconfig
h8300 alldefconfig
microblaze defconfig
sh rts7751r2dplus_defconfig
mips maltasmvp_defconfig
arm shannon_defconfig
arm ebsa110_defconfig
arm gemini_defconfig
arc tb10x_defconfig
powerpc tqm8560_defconfig
xtensa xip_kc705_defconfig
arm spear6xx_defconfig
mips jazz_defconfig
powerpc rainier_defconfig
powerpc ppc64e_defconfig
powerpc pq2fads_defconfig
sh rsk7269_defconfig
mips db1xxx_defconfig
powerpc ppc64_defconfig
mips rb532_defconfig
powerpc powernv_defconfig
powerpc mpc832x_mds_defconfig
arm pcm027_defconfig
powerpc eiger_defconfig
arm integrator_defconfig
powerpc mpc834x_mds_defconfig
powerpc tqm8xx_defconfig
powerpc katmai_defconfig
sh apsh4ad0a_defconfig
powerpc wii_defconfig
arm exynos_defconfig
ia64 bigsur_defconfig
powerpc mpc8313_rdb_defconfig
um i386_defconfig
arm assabet_defconfig
mips pistachio_defconfig
m68k multi_defconfig
powerpc chrp32_defconfig
sh landisk_defconfig
mips ar7_defconfig
mips jmr3927_defconfig
mips lemote2f_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
csky defconfig
alpha defconfig
nios2 allyesconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
x86_64 randconfig-a006-20200916
x86_64 randconfig-a004-20200916
x86_64 randconfig-a003-20200916
x86_64 randconfig-a002-20200916
x86_64 randconfig-a001-20200916
x86_64 randconfig-a005-20200916
i386 randconfig-a004-20200916
i386 randconfig-a006-20200916
i386 randconfig-a003-20200916
i386 randconfig-a001-20200916
i386 randconfig-a002-20200916
i386 randconfig-a005-20200916
i386 randconfig-a015-20200916
i386 randconfig-a014-20200916
i386 randconfig-a011-20200916
i386 randconfig-a013-20200916
i386 randconfig-a016-20200916
i386 randconfig-a012-20200916
riscv allyesconfig
riscv allnoconfig
riscv defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 kexec
clang tested configs:
x86_64 randconfig-a014-20200916
x86_64 randconfig-a011-20200916
x86_64 randconfig-a016-20200916
x86_64 randconfig-a012-20200916
x86_64 randconfig-a015-20200916
x86_64 randconfig-a013-20200916
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH 2/2] efi/x86: Add a quirk to support command line arguments on Dell EFI firmware
From: Jacobo Pantoja @ 2020-09-16 16:50 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: Arvind Sankar, Limonciello, Mario, Peter Jones, linux-efi
In-Reply-To: <CAMj1kXHy4d6zEJhJtdkHyYx-jnhJJzJ4Xi+qyawhjg6hXhAQgw@mail.gmail.com>
Hi, I'd like to update my testing and share my thoughts.
Regarding the patches:
1) The patches in email 1/2 (the functions "efi_warn", etc.) are not working
properly. I got some suggestions for testing from Ard in a separate email.
3a. If, in this 2nd patch, I switch the "efi_warn_once" with an
"efi_printk", the
messages appear.
1a. I've set CONFIG_CONSOLE_LOGLEVEL_DEFAULT=5, same result
2a. I've switched from "efi_warn_once" to "efi_warn", same result.
2) Even if they would be working, since it is not logged anywhere, I
don't really
think these messages make sense. Idk if these can be made available to dmesg.
3) The function "efi_apply_loadoptions_quirk" is called twice, it seems to me
that calling it from the "file.c" is redundant, but probably I'm
missing something.
Regarding the quirk itself, in my opinion we should wait for Mario's
news, since,
again in my opinion, this is something that should be fixed in the
firmware itself.
Being Dell a serious company, I think it is feasible that, at least
for their enterprise
products, they might fix it.
On Tue, 15 Sep 2020 at 17:09, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 15 Sep 2020 at 00:35, Arvind Sankar <nivedita@alum.mit.edu> wrote:
> >
> > 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. This was verified with firmware revision 2.15.0 on a
> > Dell Precision T3620 by Jacobo Pontaja.
Please be so kind to correct my name, if it's being included in the commit msg.
> >
> > 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.
> >
> > 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/
>
> I'll queue these up for v5.10
>
> Thanks all
>
> > ---
> > .../firmware/efi/libstub/efi-stub-helper.c | 101 +++++++++++++++++-
> > drivers/firmware/efi/libstub/efistub.h | 31 ++++++
> > drivers/firmware/efi/libstub/file.c | 5 +-
> > 3 files changed, 135 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..aa8da0a49829 100644
> > --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> > +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> > @@ -238,6 +238,102 @@ 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;
> > +
> > + 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;
> > +
> > + efi_warn_once(FW_BUG "LoadOptions is an EFI_LOAD_OPTION descriptor\n");
> > + efi_warn_once(FW_BUG "Using OptionalData as a workaround\n");
> > +
> > + *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 +343,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 e33b9395bc23..7997890c756d 100644
> > --- a/drivers/firmware/efi/libstub/efistub.h
> > +++ b/drivers/firmware/efi/libstub/efistub.h
> > @@ -711,6 +711,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)(
> > @@ -753,6 +782,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 RFC/RFT 0/3] efi/libstub: arm32: Remove dependency on dram_base
From: Maxim Uvarov @ 2020-09-16 15:43 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Palmer Dabbelt, Atish Patra, linux-efi, Linux ARM,
Heinrich Schuchardt, Atish Patra, Jens Wiklander,
François Ozog, Etienne CARRIERE, Takahiro Akashi,
Patrice CHOTARD, Sumit Garg, Grant Likely, Ilias Apalodimas,
Christophe Priouzeau, Rouven Czerwinski, Patrick Delaunay
In-Reply-To: <CAMj1kXECRr+E4=r+7fuaRAXQUyLi5Z1_HgNWgLGZVHdPSABE0A@mail.gmail.com>
On Fri, 11 Sep 2020 at 21:45, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 11 Sep 2020 at 13:27, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> >
> > Changes look fine for and should fix booting, while I can test them in
> > my environment next week.
> >
>
> Thanks
>
> Please use the version below for testing:
> https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/tag/?h=efi-riscv-shared-for-v5.10
>
tested this one, qemu 5.0 ( qemu-system-arm -machine virt,secure=on
-cpu cortex-a15 -m 2048 ..)
atf - fc721f83085
optee - d1c635434
uboot - 9f04a634ef
All components are almost the latest versions for the current moment.
Works fine for me.
Reviewed-and-Tested-by: Maxim Uvarov <maxim.uvarov@linaro.org
Maxim.
>
> > On Fri, 11 Sep 2020 at 10:56, Ard Biesheuvel <ardb@kernel.org> wrote:
> > >
> > > On Fri, 11 Sep 2020 at 05:16, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > >
> > > > On Thu, 10 Sep 2020 07:08:07 PDT (-0700), ardb@kernel.org wrote:
> > > > > On Thu, 10 Sep 2020 at 13:04, Ard Biesheuvel <ardb@kernel.org> wrote:
> > > > >>
> > > > >> On Thu, 10 Sep 2020 at 04:34, Atish Patra <atishp@atishpatra.org> wrote:
> > > > >> >
> > > > >> > On Wed, Sep 9, 2020 at 2:44 PM Atish Patra <atishp@atishpatra.org> wrote:
> > > > >> > >
> > > > >> > > On Wed, Sep 9, 2020 at 1:52 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > > > >> > > >
> > > > >> > > > On Wed, 09 Sep 2020 08:16:20 PDT (-0700), ardb@kernel.org wrote:
> > > > >> > > > > Maxim reports boot failures on platforms that describe reserved memory
> > > > >> > > > > regions in DT that are disjoint from system DRAM, and which are converted
> > > > >> > > > > to EfiReservedMemory regions by the EFI subsystem in u-boot.
> > > > >> > > > >
> > > > >> > > > > As it turns out, the whole notion of discovering the base of DRAM is
> > > > >> > > > > problematic, and it would be better to simply rely on the EFI memory
> > > > >> > > > > allocation routines instead, and derive the FDT and initrd allocation
> > > > >> > > > > limits from the actual placement of the kernel (which is what defines
> > > > >> > > > > the start of the linear region anyway)
> > > > >> > > > >
> > > > >> > > > > Finally, we should be able to get rid of get_dram_base() entirely.
> > > > >> > > > > However, as RISC-V only just started using it, we will need to address
> > > > >> > > > > that at a later time.
> > > > >> > > >
> > > > >> > > > Looks like we're using dram_base to derive two argumets to
> > > > >> > > > efi_relocate_kernel(): the preferred load address and the minimum load address.
> > > > >> > > > I don't see any reason why we can't use the same PAGE_OFFSET-like logic that
> > > > >> > > > x86 uses for the minimum load address, but I don't think we have any mechanism
> > > > >> > > > like "struct boot_params" so we'd need to come up with something.
> > > > >> > > >
> > > > >> > >
> > > > >> > > As discussed in the other thread
> > > > >> > > (https://www.spinics.net/lists/linux-efi/msg20262.html),
> > > > >> > > we don't need to do anything special. efi_relocate_kernel can just
> > > > >> > > take preferred address as 0
> > > > >> > > so that efi_bs_alloc will fail and efi_low_alloc_above will be used to
> > > > >> > > allocate 2MB/4MB aligned address as per requirement.
> > > > >> > >
> > > > >> > > I don't think the other changes in this series will cause any issue
> > > > >> > > for RISC-V. I will test it and update anyways.
> > > > >> > >
> > > > >> > > > > Cc: Maxim Uvarov <maxim.uvarov@linaro.org>
> > > > >> > > > > Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > >> > > > > Cc: Atish Patra <atish.patra@wdc.com>
> > > > >> > > > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > > >> > > > > Cc: Jens Wiklander <jens.wiklander@linaro.org>
> > > > >> > > > > Cc: Francois Ozog <francois.ozog@linaro.org>
> > > > >> > > > > Cc: Etienne CARRIERE <etienne.carriere@st.com>
> > > > >> > > > > Cc: Takahiro Akashi <takahiro.akashi@linaro.org>
> > > > >> > > > > Cc: Patrice CHOTARD <patrice.chotard@st.com>
> > > > >> > > > > Cc: Sumit Garg <sumit.garg@linaro.org>
> > > > >> > > > > Cc: Grant Likely <Grant.Likely@arm.com>
> > > > >> > > > > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > > > >> > > > > Cc: Christophe Priouzeau <christophe.priouzeau@linaro.org>
> > > > >> > > > > Cc: Rouven Czerwinski <r.czerwinski@pengutronix.de>
> > > > >> > > > > Cc: Patrick DELAUNAY <patrick.delaunay@st.com>
> > > > >> > > > >
> > > > >> > > > > Ard Biesheuvel (3):
> > > > >> > > > > efi/libstub: Export efi_low_alloc_above() to other units
> > > > >> > > > > efi/libstub: Use low allocation for the uncompressed kernel
> > > > >> > > > > efi/libstub: base FDT and initrd placement on image address not DRAM
> > > > >> > > > > base
> > > > >> > > > >
> > > > >> > > > > arch/arm/include/asm/efi.h | 6 +-
> > > > >> > > > > arch/arm64/include/asm/efi.h | 2 +-
> > > > >> > > > > drivers/firmware/efi/libstub/arm32-stub.c | 177 ++++----------------
> > > > >> > > > > drivers/firmware/efi/libstub/efi-stub.c | 2 +-
> > > > >> > > > > drivers/firmware/efi/libstub/efistub.h | 3 +
> > > > >> > > > > drivers/firmware/efi/libstub/relocate.c | 4 +-
> > > > >> > > > > 6 files changed, 47 insertions(+), 147 deletions(-)
> > > > >> > >
> > > > >> >
> > > > >> > I verified the above patches along with the following RISC-V specific changes.
> > > > >> >
> > > > >> > diff --git a/arch/riscv/include/asm/efi.h b/arch/riscv/include/asm/efi.h
> > > > >> > index 93c305a638f4..dd6ceea9d548 100644
> > > > >> > --- a/arch/riscv/include/asm/efi.h
> > > > >> > +++ b/arch/riscv/include/asm/efi.h
> > > > >> > @@ -37,7 +37,7 @@ static inline unsigned long
> > > > >> > efi_get_max_fdt_addr(unsigned long dram_base)
> > > > >> > static inline unsigned long efi_get_max_initrd_addr(unsigned long dram_base,
> > > > >> > unsigned long image_addr)
> > > > >> > {
> > > > >> > - return dram_base + SZ_256M;
> > > > >> > + return image_addr + SZ_256M;
> > > > >> > }
> > > > >> >
> > > > >>
> > > > >> Ah yes, we need this change as well - this is a bit unfortunate since
> > > > >> that creates a conflict with the RISC-V tree.
> > > > >>
> > > > >> > --- a/drivers/firmware/efi/libstub/riscv-stub.c
> > > > >> > +++ b/drivers/firmware/efi/libstub/riscv-stub.c
> > > > >> > @@ -100,7 +100,7 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
> > > > >> > */
> > > > >> > preferred_addr = round_up(dram_base, MIN_KIMG_ALIGN) + MIN_KIMG_ALIGN;
> > > > >> > status = efi_relocate_kernel(image_addr, kernel_size, *image_size,
> > > > >> > - preferred_addr, MIN_KIMG_ALIGN, dram_base);
> > > > >> > + 0, MIN_KIMG_ALIGN, 0);
> > > > >> >
> > > > >> > FWIW: Tested-by: Atish Patra <atish.patra@wdc.com>
> > > > >>
> > > > >> Thanks for confirming.
> > > > >
> > > > > OK,
> > > > >
> > > > > So, just to annoy Palmer and you more than I already have up to this
> > > > > point: any chance we could do a final respin of the RISC-V code on top
> > > > > of these changes? They are important for ARM, and I would prefer these
> > > > > to be merged in a way that makes it easy to backport them to -stable
> > > > > if needed.
> > > > >
> > > > > So what I would suggest is:
> > > > > - I will create a new 'shared-efi' tag/stable branch containing the
> > > > > existing two patches, as well as these changes (in a slightly updated
> > > > > form)
> > > > > - Palmer creates a new topic branch in the riscv repo based on this
> > > > > shared tag, and applies the [updated] RISC-V patches on top
> > > > > - Palmer drops the current version of the riscv patches from
> > > > > riscv/for-next, and merges the topic branch into it instead.
> > > > >
> > > > > Again, sorry to be a pain, but I think this is the cleanest way to get
> > > > > these changes queued up for v5.10 without painting ourselves into a
> > > > > corner too much when it comes to future follow-up changes.
> > > >
> > > > That's fine for me.
> > >
> > > Excellent.
> > >
> > > I have created a signed tag efi-riscv-shared-for-v5.10 in the EFI repo
> > > [0], which is based on v5.9-rc1. Please merge that at the start of
> > > your for-next/efi topic branch, and apply the reworked EFI for RISC-V
> > > series on top.
> > >
> > > I have created a preliminary version of that branch as 'riscv-tmp' on
> > > [1], incorporating some changes that are needed for the rebase. Note
> > > that I applied some other tweaks as well - one is in a separate patch
> > > on top, the other is that I omitted the Makefile rule for .stub.o
> > > objects under arch/riscv/Makefile, as it is not actually used.
> > >
> > > Atish - please pick whatever seems useful to you from that branch when
> > > you do the respin.
> > >
> > > Thanks,
> > > Ard.
> > >
> > >
> > >
> > > [0] git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git
> > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git
^ permalink raw reply
* Re: [PATCH v2 1/2] edac,ghes,cper: Add Row Extension to Memory Error Record
From: Borislav Petkov @ 2020-09-16 18:10 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Alex Kluver, linux-edac, linux-efi, Linux Kernel Mailing List,
mchehab, russ.anderson, Dimitri Sivanich, kluveralex
In-Reply-To: <CAMj1kXE6PKb==h_154hRKwZLr3Ek+4z4A8FdTHx=co18ww5d3Q@mail.gmail.com>
On Wed, Sep 16, 2020 at 04:09:36PM +0300, Ard Biesheuvel wrote:
> git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
Looks good and no conflicts, builds fine too.
[boris@zn: ~/kernel/linux> git fetch efi
remote: Enumerating objects: 85, done.
remote: Counting objects: 100% (85/85), done.
remote: Compressing objects: 100% (14/14), done.
remote: Total 131 (delta 71), reused 85 (delta 71), pack-reused 46
Receiving objects: 100% (131/131), 113.14 KiB | 1.69 MiB/s, done.
Resolving deltas: 100% (89/89), completed with 33 local objects.
From git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi
+ 84780c5438ef...744de4180a43 next -> efi/next (forced update)
fb1201aececc..46908326c6b8 urgent -> efi/urgent
* [new tag] efi-next-for-v5.10 -> efi-next-for-v5.10
* [new tag] efi-urgent-for-v5.9-rc5 -> efi-urgent-for-v5.9-rc5
* [new tag] efi-riscv-shared-for-v5.10 -> efi-riscv-shared-for-v5.10
[boris@zn: ~/kernel/linux> git checkout -b test-merge ras/edac-for-next
Branch 'test-merge' set up to track remote branch 'edac-for-next' from 'ras'.
Switched to a new branch 'test-merge'
[boris@zn: ~/kernel/linux> git merge efi/next
Auto-merging drivers/firmware/efi/libstub/efi-stub-helper.c
Auto-merging drivers/firmware/efi/efi.c
Auto-merging drivers/edac/ghes_edac.c
Auto-merging arch/x86/platform/efi/efi.c
Merge made by the 'recursive' strategy.
arch/arm/include/asm/efi.h | 23 +++--
arch/arm64/include/asm/efi.h | 5 +-
arch/x86/kernel/setup.c | 1 +
arch/x86/platform/efi/efi.c | 3 +
drivers/edac/ghes_edac.c | 17 +++-
drivers/firmware/efi/Makefile | 3 +-
drivers/firmware/efi/cper.c | 18 +++-
drivers/firmware/efi/{arm-init.c => efi-init.c} | 1 +
drivers/firmware/efi/efi.c | 6 ++
drivers/firmware/efi/libstub/arm32-stub.c | 178 +++++++---------------------------
drivers/firmware/efi/libstub/arm64-stub.c | 1 -
drivers/firmware/efi/libstub/efi-stub-helper.c | 101 +++++++++++++++++++-
drivers/firmware/efi/libstub/efi-stub.c | 48 +---------
drivers/firmware/efi/libstub/efistub.h | 61 +++++++++++-
drivers/firmware/efi/libstub/file.c | 5 +-
drivers/firmware/efi/libstub/relocate.c | 4 +-
drivers/firmware/efi/libstub/vsprintf.c | 2 +-
drivers/firmware/efi/mokvar-table.c | 360 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/cper.h | 24 ++++-
include/linux/efi.h | 34 +++++++
include/linux/pe.h | 3 +
security/integrity/platform_certs/load_uefi.c | 85 +++++++++++++----
22 files changed, 746 insertions(+), 237 deletions(-)
rename drivers/firmware/efi/{arm-init.c => efi-init.c} (99%)
create mode 100644 drivers/firmware/efi/mokvar-table.c
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [PATCH v2 1/2] edac,ghes,cper: Add Row Extension to Memory Error Record
From: Russ Anderson @ 2020-09-16 18:12 UTC (permalink / raw)
To: Borislav Petkov
Cc: Ard Biesheuvel, linux-edac, linux-efi, Linux Kernel Mailing List,
mchehab, russ.anderson, Dimitri Sivanich, kluveralex
In-Reply-To: <20200916181030.GR2643@zn.tnic>
On Wed, Sep 16, 2020 at 08:10:30PM +0200, Borislav Petkov wrote:
> On Wed, Sep 16, 2020 at 04:09:36PM +0300, Ard Biesheuvel wrote:
> > git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
>
> Looks good and no conflicts, builds fine too.
Excellent.
Thanks!
> [boris@zn: ~/kernel/linux> git fetch efi
> remote: Enumerating objects: 85, done.
> remote: Counting objects: 100% (85/85), done.
> remote: Compressing objects: 100% (14/14), done.
> remote: Total 131 (delta 71), reused 85 (delta 71), pack-reused 46
> Receiving objects: 100% (131/131), 113.14 KiB | 1.69 MiB/s, done.
> Resolving deltas: 100% (89/89), completed with 33 local objects.
> From git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi
> + 84780c5438ef...744de4180a43 next -> efi/next (forced update)
> fb1201aececc..46908326c6b8 urgent -> efi/urgent
> * [new tag] efi-next-for-v5.10 -> efi-next-for-v5.10
> * [new tag] efi-urgent-for-v5.9-rc5 -> efi-urgent-for-v5.9-rc5
> * [new tag] efi-riscv-shared-for-v5.10 -> efi-riscv-shared-for-v5.10
> [boris@zn: ~/kernel/linux> git checkout -b test-merge ras/edac-for-next
> Branch 'test-merge' set up to track remote branch 'edac-for-next' from 'ras'.
> Switched to a new branch 'test-merge'
> [boris@zn: ~/kernel/linux> git merge efi/next
> Auto-merging drivers/firmware/efi/libstub/efi-stub-helper.c
> Auto-merging drivers/firmware/efi/efi.c
> Auto-merging drivers/edac/ghes_edac.c
> Auto-merging arch/x86/platform/efi/efi.c
> Merge made by the 'recursive' strategy.
> arch/arm/include/asm/efi.h | 23 +++--
> arch/arm64/include/asm/efi.h | 5 +-
> arch/x86/kernel/setup.c | 1 +
> arch/x86/platform/efi/efi.c | 3 +
> drivers/edac/ghes_edac.c | 17 +++-
> drivers/firmware/efi/Makefile | 3 +-
> drivers/firmware/efi/cper.c | 18 +++-
> drivers/firmware/efi/{arm-init.c => efi-init.c} | 1 +
> drivers/firmware/efi/efi.c | 6 ++
> drivers/firmware/efi/libstub/arm32-stub.c | 178 +++++++---------------------------
> drivers/firmware/efi/libstub/arm64-stub.c | 1 -
> drivers/firmware/efi/libstub/efi-stub-helper.c | 101 +++++++++++++++++++-
> drivers/firmware/efi/libstub/efi-stub.c | 48 +---------
> drivers/firmware/efi/libstub/efistub.h | 61 +++++++++++-
> drivers/firmware/efi/libstub/file.c | 5 +-
> drivers/firmware/efi/libstub/relocate.c | 4 +-
> drivers/firmware/efi/libstub/vsprintf.c | 2 +-
> drivers/firmware/efi/mokvar-table.c | 360 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/cper.h | 24 ++++-
> include/linux/efi.h | 34 +++++++
> include/linux/pe.h | 3 +
> security/integrity/platform_certs/load_uefi.c | 85 +++++++++++++----
> 22 files changed, 746 insertions(+), 237 deletions(-)
> rename drivers/firmware/efi/{arm-init.c => efi-init.c} (99%)
> create mode 100644 drivers/firmware/efi/mokvar-table.c
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
--
Russ Anderson, SuperDome Flex Linux Kernel Group Manager
HPE - Hewlett Packard Enterprise (formerly SGI) rja@hpe.com
^ permalink raw reply
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