Bug#963033: linux-image-arm64: kexec loses EFI system tables with Debian kernels
Uwe Kleine-König <uwe@kleine-koenig.org> writes:
> Hello,
>
> On Fri, Jun 26, 2020 at 04:34:28PM -0400, Gabriel Krisman Bertazi wrote:
>> Hi,
>>
>> This is introduced by a Debian specific patch
>> features/all/lockdown/arm64-add-kernel-config-option-to-lock-down-when.patch
>>
>> The following patch fixes it.
>
> Note, I'm not an expert in the area this patch modifies, just some
> general feedback.
Hi, thanks for the review.
>> diff --git a/drivers/firmware/efi/fdtparams.c b/drivers/firmware/efi/fdtparams.c
>> index 152ca7cfccc9..78c36e582408 100644
>> --- a/drivers/firmware/efi/fdtparams.c
>> +++ b/drivers/firmware/efi/fdtparams.c
>> @@ -96,13 +96,15 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm, u32 *secure_boot)
>> struct {
>> void *var;
>> int size;
>> + int required;
>> +
>> } target[] = {
>> - [SYSTAB] = { &systab, sizeof(systab) },
>> - [MMBASE] = { &mm->phys_map, sizeof(mm->phys_map) },
>> - [MMSIZE] = { &mm->size, sizeof(mm->size) },
>> - [DCSIZE] = { &mm->desc_size, sizeof(mm->desc_size) },
>> - [DCVERS] = { &mm->desc_version, sizeof(mm->desc_version) },
>> - [SBMODE] = { secure_boot, sizeof(*secure_boot) },
>> + [SYSTAB] = {&systab, sizeof(systab), 1},
>> + [MMBASE] = {&mm->phys_map, sizeof(mm->phys_map), 1},
>> + [MMSIZE] = {&mm->size, sizeof(mm->size), 1},
>> + [DCSIZE] = {&mm->desc_size, sizeof(mm->desc_size), 1},
>> + [DCVERS] = {&mm->desc_version, sizeof(mm->desc_version), 1},
>> + [SBMODE] = {secure_boot, sizeof(*secure_boot), 0 },
>
> Is the whitespace change intended here?
argh.
> I wonder if it would be easier to drop this hunk and ...
>
>> };
>>
>> BUILD_BUG_ON(ARRAY_SIZE(target) != ARRAY_SIZE(name));
>> @@ -125,8 +127,10 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm, u32 *secure_boot)
>> continue;
>> if (!j)
>> goto notfound;
>> +
>> pr_err("Can't find property '%s' in DT!\n", pname);
>> - return 0;
>> + if (target[j].required)
>> + return 0;
>
> ... do here
>
> if (j != SBMODE)
> return 0;
>
> here.
I don't think this makes the patch more readable, quite the opposite.
But I don't oppose changing it.
>
> Is it right that a missing linux,uefi-secure-boot property still results
> in a pr_err?
It think it is reasonable to print the message in the log, so board
developers know there is something missing for their board. We could
make it a pr_warn, as long as it doesn't fail silently, works for me.
> I wonder what the upstream status of the broken patch is.
It never got merged upstream. There are some lockdown features merged
already, but this is not part of it, as far as I know.
--
Gabriel Krisman Bertazi
Reply to: