[PATCH] efi_loader: move efi_var_collect to common functions
Ilias Apalodimas
ilias.apalodimas at linaro.org
Fri Apr 5 08:37:03 CEST 2024
On Fri, 5 Apr 2024 at 09:33, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 4/4/24 09:48, Ilias Apalodimas wrote:
> > From: Ilias Apalodimas <apalos at gmail.com>
> >
> > efi_var_collect() was initially placed in efi_var_file.c, since back
> > then we only supported efi variables stored in a file. Since then we
> > support variables stored in an RPMB as well and use that function to
> > collect variables that should be present at runtime.
>
> Thanks for cleaning this up.
>
> In the CONFIG_EFI_MM_COMM_TEE=y case we seem not to need efi_var_file.c
> anymore. Can we add this diff:
>
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -72,11 +72,11 @@ obj-y += efi_string.o
> obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
> obj-y += efi_var_common.o
> obj-y += efi_var_mem.o
> -obj-y += efi_var_file.o
> ifeq ($(CONFIG_EFI_MM_COMM_TEE),y)
> obj-y += efi_variable_tee.o
> else
> obj-y += efi_variable.o
> +obj-y += efi_var_file.o
> obj-$(CONFIG_EFI_VARIABLES_PRESEED) += efi_var_seed.o
> endif
> obj-y += efi_watchdog.o
Yes, I'll add it on v2
Thanks
/Ilias
>
> Another comment below.
>
> >
> > So let's move it around in efi_var_common.c which makes more sense
> >
> > Suggested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Signed-off-by: Ilias Apalodimas <apalos at gmail.com>
> > ---
> > lib/efi_loader/efi_var_common.c | 76 +++++++++++++++++++++++++++++++++
> > lib/efi_loader/efi_var_file.c | 64 ---------------------------
> > 2 files changed, 76 insertions(+), 64 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c
> > index 16b2c3d48828..07b9603d49f3 100644
> > --- a/lib/efi_loader/efi_var_common.c
> > +++ b/lib/efi_loader/efi_var_common.c
> > @@ -9,6 +9,7 @@
> > #include <efi_loader.h>
> > #include <efi_variable.h>
> > #include <stdlib.h>
> > +#include <u-boot/crc.h>
> >
> > enum efi_secure_mode {
> > EFI_MODE_SETUP,
> > @@ -416,3 +417,78 @@ void *efi_get_var(const u16 *name, const efi_guid_t *vendor, efi_uintn_t *size)
> >
> > return buf;
> > }
> > +
> > +/**
> > + * efi_var_collect() - Copy EFI variables mstching attributes mask
> > + *
> > + * @bufp: buffer containing variable collection
> > + * @lenp: buffer length
> > + * @attr_mask: mask of matched attributes
> > + *
> > + * Return: Status code
> > + */
> > +efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp,
> > + u32 check_attr_mask)
> > +{
> > + size_t len = EFI_VAR_BUF_SIZE;
> > + struct efi_var_file *buf;
> > + struct efi_var_entry *var, *old_var;
> > + size_t old_var_name_length = 2;
> > +
> > + *bufp = NULL; /* Avoid double free() */
> > + buf = calloc(1, len);
> > + if (!buf)
> > + return EFI_OUT_OF_RESOURCES;
> > + var = buf->var;
> > + old_var = var;
> > + for (;;) {
> > + efi_uintn_t data_length, var_name_length;
> > + u8 *data;
> > + efi_status_t ret;
> > +
> > + if ((uintptr_t)buf + len <=
> > + (uintptr_t)var->name + old_var_name_length)
> > + return EFI_BUFFER_TOO_SMALL;
> > +
> > + var_name_length = (uintptr_t)buf + len - (uintptr_t)var->name;
> > + memcpy(var->name, old_var->name, old_var_name_length);
> > + guidcpy(&var->guid, &old_var->guid);
> > + ret = efi_get_next_variable_name_int(
> > + &var_name_length, var->name, &var->guid);
> > + if (ret == EFI_NOT_FOUND)
> > + break;
> > + if (ret != EFI_SUCCESS) {
> > + free(buf);
> > + return ret;
> > + }
> > + old_var_name_length = var_name_length;
> > + old_var = var;
> > +
> > + data = (u8 *)var->name + old_var_name_length;
> > + data_length = (uintptr_t)buf + len - (uintptr_t)data;
> > + ret = efi_get_variable_int(var->name, &var->guid,
> > + &var->attr, &data_length, data,
> > + &var->time);
> > + if (ret != EFI_SUCCESS) {
> > + free(buf);
> > + return ret;
> > + }
> > + if ((var->attr & check_attr_mask) == check_attr_mask) {
> > + var->length = data_length;
> > + var = (struct efi_var_entry *)ALIGN((uintptr_t)data + data_length, 8);
> > + }
> > + }
> > +
> > + buf->reserved = 0;
> > + buf->magic = EFI_VAR_FILE_MAGIC;
> > + len = (uintptr_t)var - (uintptr_t)buf;
> > + buf->crc32 = crc32(0, (u8 *)buf->var,
> > + len - sizeof(struct efi_var_file));
> > + buf->length = len;
> > + *bufp = buf;
> > + *lenp = len;
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +
>
> This produces a warning
>
> .git/rebase-apply/patch:95: new blank line at EOF.
>
> Could you, please, remove the blank lines at the end of the file.
>
> Best regards
>
> Heinrich
>
> > diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> > index 532b6b40eefe..413e1794e88c 100644
> > --- a/lib/efi_loader/efi_var_file.c
> > +++ b/lib/efi_loader/efi_var_file.c
> > @@ -52,70 +52,6 @@ static efi_status_t __maybe_unused efi_set_blk_dev_to_system_partition(void)
> > return EFI_SUCCESS;
> > }
> >
> > -efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t *lenp,
> > - u32 check_attr_mask)
> > -{
> > - size_t len = EFI_VAR_BUF_SIZE;
> > - struct efi_var_file *buf;
> > - struct efi_var_entry *var, *old_var;
> > - size_t old_var_name_length = 2;
> > -
> > - *bufp = NULL; /* Avoid double free() */
> > - buf = calloc(1, len);
> > - if (!buf)
> > - return EFI_OUT_OF_RESOURCES;
> > - var = buf->var;
> > - old_var = var;
> > - for (;;) {
> > - efi_uintn_t data_length, var_name_length;
> > - u8 *data;
> > - efi_status_t ret;
> > -
> > - if ((uintptr_t)buf + len <=
> > - (uintptr_t)var->name + old_var_name_length)
> > - return EFI_BUFFER_TOO_SMALL;
> > -
> > - var_name_length = (uintptr_t)buf + len - (uintptr_t)var->name;
> > - memcpy(var->name, old_var->name, old_var_name_length);
> > - guidcpy(&var->guid, &old_var->guid);
> > - ret = efi_get_next_variable_name_int(
> > - &var_name_length, var->name, &var->guid);
> > - if (ret == EFI_NOT_FOUND)
> > - break;
> > - if (ret != EFI_SUCCESS) {
> > - free(buf);
> > - return ret;
> > - }
> > - old_var_name_length = var_name_length;
> > - old_var = var;
> > -
> > - data = (u8 *)var->name + old_var_name_length;
> > - data_length = (uintptr_t)buf + len - (uintptr_t)data;
> > - ret = efi_get_variable_int(var->name, &var->guid,
> > - &var->attr, &data_length, data,
> > - &var->time);
> > - if (ret != EFI_SUCCESS) {
> > - free(buf);
> > - return ret;
> > - }
> > - if ((var->attr & check_attr_mask) == check_attr_mask) {
> > - var->length = data_length;
> > - var = (struct efi_var_entry *)ALIGN((uintptr_t)data + data_length, 8);
> > - }
> > - }
> > -
> > - buf->reserved = 0;
> > - buf->magic = EFI_VAR_FILE_MAGIC;
> > - len = (uintptr_t)var - (uintptr_t)buf;
> > - buf->crc32 = crc32(0, (u8 *)buf->var,
> > - len - sizeof(struct efi_var_file));
> > - buf->length = len;
> > - *bufp = buf;
> > - *lenp = len;
> > -
> > - return EFI_SUCCESS;
> > -}
> > -
> > /**
> > * efi_var_to_file() - save non-volatile variables as file
> > *
>
More information about the U-Boot
mailing list