Re: [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux

2020-10-23 Thread Leif Lindholm
initrd typo in subject minor style comments below On Fri, Oct 23, 2020 at 14:08:24 +0200, Ard Biesheuvel wrote: > Recent Linux kernels will invoke the LoadFile2 protocol installed on > a well-known vendor media path to load the initrd if it is exposed by > the firmware. Using this method is prefe

Re: [PATCH 4/4] linux: ignore FDT unless we need to modify it

2020-10-23 Thread Leif Lindholm
On Fri, Oct 23, 2020 at 14:08:25 +0200, Ard Biesheuvel wrote: > Now that we implemented supported for the LoadFile2 protocol for initrd > loading, there is no longer a need to pass the initrd parameters via > the device tree. This means there is no longer a reason to update the > device tree in the

Re: [PATCH 0/4] linux: implement LoadFile2 initrd loading

2020-10-23 Thread Leif Lindholm
On Fri, Oct 23, 2020 at 14:08:21 +0200, Ard Biesheuvel wrote: > This implements the LoadFile2 initrd loading protocol, which is > essentially a callback face into the bootloader to load the initrd > data into a caller provided buffer. This means the bootloader no > longer has to contain any policy

Re: [PATCH 4/4] linux: ignore FDT unless we need to modify it

2020-10-23 Thread Ard Biesheuvel
On Fri, 23 Oct 2020 at 14:47, Leif Lindholm wrote: > > On Fri, Oct 23, 2020 at 14:08:25 +0200, Ard Biesheuvel wrote: > > Now that we implemented supported for the LoadFile2 protocol for initrd > > loading, there is no longer a need to pass the initrd parameters via > > the device tree. This means

Re: [PATCH 4/4] linux: ignore FDT unless we need to modify it

2020-10-23 Thread Leif Lindholm
On Fri, Oct 23, 2020 at 15:12:50 +0200, Ard Biesheuvel wrote: > On Fri, 23 Oct 2020 at 14:47, Leif Lindholm wrote: > > > > On Fri, Oct 23, 2020 at 14:08:25 +0200, Ard Biesheuvel wrote: > > > Now that we implemented supported for the LoadFile2 protocol for initrd > > > loading, there is no longer a

[PATCH 0/4] linux: implement LoadFile2 initrd loading

2020-10-23 Thread Ard Biesheuvel
This implements the LoadFile2 initrd loading protocol, which is essentially a callback face into the bootloader to load the initrd data into a caller provided buffer. This means the bootloader no longer has to contain any policy regarding where to load the initrd (which differs between architecture

[PATCH 4/4] linux: ignore FDT unless we need to modify it

2020-10-23 Thread Ard Biesheuvel
Now that we implemented supported for the LoadFile2 protocol for initrd loading, there is no longer a need to pass the initrd parameters via the device tree. This means there is no longer a reason to update the device tree in the first place, and so we can ignore it entirely. The only remaining re

[PATCH 2/4] efi: add definition of LoadFile2 protocol

2020-10-23 Thread Ard Biesheuvel
Incorporate the EFI_LOAD_FILE2_PROTOCOL GUID and C types from the UEFI spec. Signed-off-by: Ard Biesheuvel --- grub-core/commands/efi/lsefi.c | 1 + include/grub/efi/api.h | 15 +++ 2 files changed, 16 insertions(+) diff --git a/grub-core/commands/efi/lsefi.c b/grub-core/co

[PATCH 1/4] loader/linux: permit NULL argument for argv[] in grub_initrd_load()

2020-10-23 Thread Ard Biesheuvel
grub_initrd_load() takes a char *argv[] argument which is only used when an error occurs, to print the name of the file that caused the error. In order to be able to split initrd loading from handling the initrd command, let's permit argv to be NULL, and fall back to the file names recorded in the

[PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux

2020-10-23 Thread Ard Biesheuvel
Recent Linux kernels will invoke the LoadFile2 protocol installed on a well-known vendor media path to load the initrd if it is exposed by the firmware. Using this method is preferred for two reasons: - the Linux kernel is in charge of allocating the memory, and so it can implement any placement

Re: [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_error() messages.

2020-10-23 Thread Daniel Kiper
On Mon, Oct 19, 2020 at 06:09:49PM -0500, Glenn Washburn wrote: > When looping over the digests and segments, the loop variable is j, but the > variable i is used to index in the the digests and segments json array. The > variable i is the keyslot index. Similarly, there are several grub_error() >

Re: [PATCH v3 02/10] luks2: Improve readability in luks2_get_keyslot.

2020-10-23 Thread Daniel Kiper
On Mon, Oct 19, 2020 at 06:09:50PM -0500, Glenn Washburn wrote: > Introduce new variables keyslot_key, digest_key, and segment_key which > represent the integer key of the item in the respective associative array > when looping over the array items. This replaces using a generically > named variabl

Re: [PATCH v3 05/10] cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'.

2020-10-23 Thread Daniel Kiper
On Mon, Oct 19, 2020 at 06:09:53PM -0500, Glenn Washburn wrote: > When setting cipher IV mode, detection is done by prefix matching the > cipher IV mode part of the cipher mode string. Since "plain" matches > "plain64", we must check for "plain64" first. Otherwise, "plain64" will > be detected as

Re: [PATCH 3/4] efi: implemented LoadFile2 initr loading protocol for Linux

2020-10-23 Thread Ard Biesheuvel
On Fri, 23 Oct 2020 at 14:32, Leif Lindholm wrote: > > initrd typo in subject > > minor style comments below > > On Fri, Oct 23, 2020 at 14:08:24 +0200, Ard Biesheuvel wrote: > > Recent Linux kernels will invoke the LoadFile2 protocol installed on > > a well-known vendor media path to load the ini

Re: [PATCH v2 10/10] luks2: Rename source disk variabled named 'disk' to 'source' as in luks.c.

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 11:27:12AM -0500, Glenn Washburn wrote: > On Fri, 9 Oct 2020 12:00:47 +0200 > Patrick Steinhardt wrote: > > > On Sat, Oct 03, 2020 at 05:55:34PM -0500, Glenn Washburn wrote: > > > This makes it more obvious to the reader that the disk referred to > > > is the source disk,

Re: [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_error() messages.

2020-10-23 Thread Patrick Steinhardt
On Fri, Oct 23, 2020 at 02:08:18PM +0200, Daniel Kiper wrote: > On Mon, Oct 19, 2020 at 06:09:49PM -0500, Glenn Washburn wrote: > > When looping over the digests and segments, the loop variable is j, but the > > variable i is used to index in the the digests and segments json array. The > > variabl

Re: [PATCH v3 03/10] luks2: Use more intuitive keyslot key instead of index when naming keyslot.

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 06:09:51PM -0500, Glenn Washburn wrote: > Use the keyslot key value in the keyslot json array rather than the index of > the keyslot in the json array. This is less confusing for the end user. For > example, say you have a LUKS2 device with a key in slot 1 and slot 4. When >

Re: [PATCH v3 09/10] cryptodisk: Rename offset in grub_cryptodisk_t to offset_sectors.

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 06:09:57PM -0500, Glenn Washburn wrote: > This makes it clear that the offset represents sectors, not bytes, in order > to improve readability. > > Signed-off-by: Glenn Washburn Reviewed-by: Patrick Steinhardt Patrick > --- > grub-core/disk/cryptodisk.c | 10 +

Re: [PATCH v3 08/10] cryptodisk: Rename total_length field in grub_cryptodisk_t to total_sectors.

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 06:09:56PM -0500, Glenn Washburn wrote: > This creates an alignment with grub_disk_t naming of the same field and is > more intuitive as to how it should be used. > > Signed-off-by: Glenn Washburn Reviewed-by: Patrick Steinhardt Patrick > --- > grub-core/disk/cryptodi

Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt.

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 06:09:55PM -0500, Glenn Washburn wrote: > This should improve readability of code by providing clues as to what the > value represents. > > Signed-off-by: Glenn Washburn > --- > grub-core/disk/cryptodisk.c | 12 +++- > include/grub/types.h| 3 +++ > 2 fil

Re: [PATCH v3 10/10] luks2: Rename source disk variabled named 'disk' to 'source' as in luks.c.

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 06:09:58PM -0500, Glenn Washburn wrote: > This makes it more obvious to the reader that the disk referred to is the > source disk, as opposed to say the disk holding the cryptodisk. > > Signed-off-by: Glenn Washburn Reviewed-by: Patrick Steinhardt Patrick > --- > grub

Re: [PATCH v3 04/10] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 06:09:52PM -0500, Glenn Washburn wrote: > The total_length field is named confusingly because length usually refers to > bytes, whereas in this case its really the total number of sectors on the > device. Also counter-intuitively, grub_disk_get_size returns the total > numbe

Re: [PATCH v3 06/10] cryptodisk: Properly handle non-512 byte sized sectors.

2020-10-23 Thread Patrick Steinhardt
On Mon, Oct 19, 2020 at 06:09:54PM -0500, Glenn Washburn wrote: > By default, dm-crypt internally uses an IV that corresponds to 512-byte > sectors, even when a larger sector size is specified. What this means is > that when using a larger sector size, the IV is incremented every sector. > However,