Re: [PATCH] efi_loader: Enable run-time variable support for tee based variables

2021-01-15 Thread Andreas Schwab
On Jan 15 2021, Ilias Apalodimas wrote:

> Anyway removing -fpic should work as well, but I'd rather do this [1],
> instead of relying on linker flags.

It's not the linker that breaks this, but the compiler, by forcing GOT
addressing.  And it can easily break again any time.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH] efi_loader: Enable run-time variable support for tee based variables

2021-01-15 Thread Ilias Apalodimas
Hi Andreas,

On Fri, Jan 15, 2021 at 05:34:04PM +0100, Andreas Schwab wrote:
> On Jan 14 2021, Atish Patra wrote:
> 
> > I am a bit confused how this will work. This means it will reside in GOT
> > which is not mapped in virtual address for Linux. Whenever we try to
> > invoke get_variable service, it will panic.
> > Did we miss a trick in RISC-V ?
> 
> I think the problem really is that RISC-V use -fpic for compiling.  If I
> change that to -fpie, there is no longer a GOT reference.

The -fpic explains why the GOT is there to begin with, as you say. Keep in mind 
it's
present in Arm as well. What I am trying to explain in the mail is that 
regardless 
of the -fpic, Arm gets rid of all GOT indirections. The section is actually
empty and they all turn into relative references. That's why that works fine
on Arm.

The reason for that (I think, if I am wrong somebody shout please), is that you 
only 
need a GOT in shared libraries for symbol pre-emption. So if both the library
and the executable define a global 'bar', the lib is supposed to switch the
references to the executable exposed symbol.
So on Arm the linker observes that's not the case, and uses relative
references, while it gets rid of  the GOT section entries (again shout if I am 
wrong :)).

Anyway removing -fpic should work as well, but I'd rather do this [1],
instead of relying on linker flags.

[1] https://lists.denx.de/pipermail/u-boot/2021-January/437478.html

Cheers
/Ilias
> 
> Andreas.
> 
> -- 
> Andreas Schwab, SUSE Labs, sch...@suse.de
> GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
> "And now for something completely different."


Re: [PATCH] efi_loader: Enable run-time variable support for tee based variables

2021-01-15 Thread Andreas Schwab
On Jan 14 2021, Atish Patra wrote:

> I am a bit confused how this will work. This means it will reside in GOT
> which is not mapped in virtual address for Linux. Whenever we try to
> invoke get_variable service, it will panic.
> Did we miss a trick in RISC-V ?

I think the problem really is that RISC-V use -fpic for compiling.  If I
change that to -fpie, there is no longer a GOT reference.

Andreas.

-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."


Re: [PATCH] efi_loader: Enable run-time variable support for tee based variables

2021-01-14 Thread Ilias Apalodimas
Hi Atish,

> >
> > diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c
> > index 7a2dba7dc263..fd97d5b56300 100644
> > --- a/lib/efi_loader/efi_var_mem.c
> > +++ b/lib/efi_loader/efi_var_mem.c
> > @@ -10,7 +10,7 @@
> >  #include 
> >  #include 
> >
> > -static struct efi_var_file __efi_runtime_data *efi_var_buf;
> > +struct efi_var_file __efi_runtime_data *efi_var_buf;
>
> I am a bit confused how this will work. This means it will reside in GOT
> which is not mapped in virtual address for Linux. Whenever we try to
> invoke get_variable service, it will panic.
> Did we miss a trick in RISC-V ?
>
> Here are the details of the issue we are seeing.
>
> http://lists.infradead.org/pipermail/linux-riscv/2021-January/004200.html
>

Thanks for reporting this. I can't make too much from the dump itself. Since
there's a qemu config for riscv I'll reproduce it.

Long shot but, during LTO, the whole executable is compiled in one go. I think
that if in that phase it observes that GOT entries never change it converts
them to relative references. I think we are either looking into some compiler
differences here or maybe a linker script trick. In any case that's not the
right way to go.
FWIW I just tested on arm64 and my .got table is empty.

Since this will work if we switch it back to a static pointer, that
should be easy to
fix and the correct way to do it since we'll be unaffected by
compiler/linker changes.

In U-Boot we support 2 ways for runtime variables. One is generic, by using
a piece of runtime data memory for the variables and the other one is very arm
specific. In both cases though the runtime memory backend is used to expose
the variables to the kernel.  So that variable can remain static and instead
code a function internally to efi_var_mem.c and do the memcpy we need.

I've never tested it on risc-v but apparently I should up more tests for cases
like that.
I'll send a patch tomorrow once I gather all the objdump info to make
sure we aren't
missing anything.

Regards

/Ilias


Re: [PATCH] efi_loader: Enable run-time variable support for tee based variables

2021-01-14 Thread Atish Patra
On Thu, Jul 23, 2020 at 12:53 AM Ilias Apalodimas
 wrote:
>
> We recently added functions for storing/restoring variables
> from a file to a memory backed buffer marked as __efi_runtime_data
> commit f1f990a8c958 ("efi_loader: memory buffer for variables")
> commit 5f7dcf079de8 ("efi_loader: UEFI variable persistence")
>
> Using the same idea we now can support GetVariable() and GetNextVariable()
> on the OP-TEE based variables as well.
>
> So let's re-arrange the code a bit and move the commmon code for
> accessing variables out of efi_variable.c. Create common functions for
> reading variables from memory that both implementations can use on
> run-time. Then just use those functions in the run-time variants of the
> OP-TEE based EFI variable implementation and initialize the memory
> buffer on ExitBootServices()
>
> Signed-off-by: Ilias Apalodimas 
> ---
>  include/efi_variable.h| 45 
>  lib/efi_loader/Makefile   |  2 +-
>  lib/efi_loader/efi_var_file.c | 25 ---
>  lib/efi_loader/efi_var_mem.c  | 70 ++-
>  lib/efi_loader/efi_variable.c | 58 +
>  lib/efi_loader/efi_variable_tee.c | 55 ++--
>  6 files changed, 175 insertions(+), 80 deletions(-)
>
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 2c629e4dca92..6ef24cd05feb 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -142,6 +142,20 @@ struct efi_var_file {
>   */
>  efi_status_t efi_var_to_file(void);
>
> +/**
> + * efi_var_collect() - collect non-volatile variables in buffer
> + *
> + * A buffer is allocated and filled with all non-volatile variables in a
> + * format ready to be written to disk.
> + *
> + * @bufp:  pointer to pointer of buffer with collected variables
> + * @lenp:  pointer to length of buffer
> + * @check_attr_mask:   mask of variable attributes which will be included in 
> the buffer
> + * Return: status code
> + */
> +efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, 
> loff_t *lenp,
> +   u32 check_attr_mask);
> +
>  /**
>   * efi_var_restore() - restore EFI variables from buffer
>   *
> @@ -233,4 +247,35 @@ efi_status_t efi_init_secure_state(void);
>   */
>  enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t 
> *guid);
>
> +/**
> + * efi_get_next_variable_name_mem() - Runtime common code across efi variable
> + *implementations for GetNextVariable()
> + *from the cached memory copy
> + * @variable_name_size:size of variable_name buffer in byte
> + * @variable_name: name of uefi variable's name in u16
> + * @vendor:vendor's guid
> + *
> + * Return: status code
> + */
> +efi_status_t __efi_runtime
> +efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 
> *variable_name,
> +  efi_guid_t *vendor);
> +/**
> + * efi_get_variable_mem() - Runtime common code across efi variable
> + *  implementations for GetVariable() from
> + *  the cached memory copy
> + *
> + * @variable_name: name of the variable
> + * @vendor:vendor GUID
> + * @attributes:attributes of the variable
> + * @data_size: size of the buffer to which the variable value is 
> copied
> + * @data:  buffer to which the variable value is copied
> + * @timep: authentication time (seconds since start of epoch)
> + * Return: status code
> +
> + */
> +efi_status_t __efi_runtime
> +efi_get_variable_mem(u16 *variable_name, const efi_guid_t *vendor, u32 
> *attributes,
> +efi_uintn_t *data_size, void *data, u64 *timep);
> +
>  #endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 441ac9432e99..9bad1d159b03 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -37,11 +37,11 @@ obj-y += efi_setup.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
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> index 6f9d76f2a2d5..b171d2d1a8f7 100644
> --- a/lib/efi_loader/efi_var_file.c
> +++ b/lib/efi_loader/efi_var_file.c
> @@ -46,18 +46,8 @@ static efi_status_t __maybe_unused 
> efi_set_blk_dev_to_system_partition(void)
> return EFI_SUCCESS;
>  }
>
> -/**
> - * efi_var_collect() - collect non-volatile variables in buffer
> - *
> - * A buffer is allocated and filled with all non-volatile variables in a
> - * format read

Re: [PATCH] efi_loader: Enable run-time variable support for tee based variables

2020-07-23 Thread ilias . apalodimas
On Thu, Jul 23, 2020 at 12:32:01PM +0200, Heinrich Schuchardt wrote:
> On 23.07.20 09:53, Ilias Apalodimas wrote:
> > We recently added functions for storing/restoring variables
> > from a file to a memory backed buffer marked as __efi_runtime_data
> > commit f1f990a8c958 ("efi_loader: memory buffer for variables")
> > commit 5f7dcf079de8 ("efi_loader: UEFI variable persistence")
> >
> > Using the same idea we now can support GetVariable() and GetNextVariable()
> > on the OP-TEE based variables as well.
> >
> > So let's re-arrange the code a bit and move the commmon code for
> > accessing variables out of efi_variable.c. Create common functions for
> > reading variables from memory that both implementations can use on
> > run-time. Then just use those functions in the run-time variants of the
> > OP-TEE based EFI variable implementation and initialize the memory
> > buffer on ExitBootServices()
> >
> > Signed-off-by: Ilias Apalodimas 
> 
> Overall the changes look good. Some cleanup is needed.

Thanks, I'll sent a v2 shortly. All of your remarks made sense

Cheers
/Ilias
> 
> > ---
> >  include/efi_variable.h| 45 
> >  lib/efi_loader/Makefile   |  2 +-
> >  lib/efi_loader/efi_var_file.c | 25 ---
> >  lib/efi_loader/efi_var_mem.c  | 70 ++-
> >  lib/efi_loader/efi_variable.c | 58 +
> >  lib/efi_loader/efi_variable_tee.c | 55 ++--
> >  6 files changed, 175 insertions(+), 80 deletions(-)
> >
> > diff --git a/include/efi_variable.h b/include/efi_variable.h
> > index 2c629e4dca92..6ef24cd05feb 100644
> > --- a/include/efi_variable.h
> > +++ b/include/efi_variable.h
> > @@ -142,6 +142,20 @@ struct efi_var_file {
> >   */
> >  efi_status_t efi_var_to_file(void);
> >
> > +/**
> > + * efi_var_collect() - collect non-volatile variables in buffer
> 
> Please, remove the reference to non-volatile here.
> 
> > + *
> > + * A buffer is allocated and filled with all non-volatile variables in a
> 
> Same here.
> 
> > + * format ready to be written to disk.
> 
> Please, describe that the bits set in check_attr_mask must *all* be set
> in the attributes of the variable to have the variable collected, e.g.
> 
> @check_attr_mask is a bitmask with required variable attributes.
> Variables are only collected if all of the required attributes are set.
> 
> > + *
> > + * @bufp:  pointer to pointer of buffer with collected variables
> > + * @lenp:  pointer to length of buffer
> > + * @check_attr_mask:   mask of variable attributes which will be 
> > included in the buffer
> 
> bitmask with required attributes of variables to be collected
> 
> > + * Return: status code
> > + */
> > +efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, 
> > loff_t *lenp,
> > +   u32 check_attr_mask);
> > +
> >  /**
> >   * efi_var_restore() - restore EFI variables from buffer
> >   *
> > @@ -233,4 +247,35 @@ efi_status_t efi_init_secure_state(void);
> >   */
> >  enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t 
> > *guid);
> >
> > +/**
> > + * efi_get_next_variable_name_mem() - Runtime common code across efi 
> > variable
> > + *implementations for GetNextVariable()
> > + *from the cached memory copy
> > + * @variable_name_size:size of variable_name buffer in byte
> > + * @variable_name: name of uefi variable's name in u16
> > + * @vendor:vendor's guid
> > + *
> > + * Return: status code
> > + */
> > +efi_status_t __efi_runtime
> > +efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 
> > *variable_name,
> > +  efi_guid_t *vendor);
> > +/**
> > + * efi_get_variable_mem() - Runtime common code across efi variable
> > + *  implementations for GetVariable() from
> > + *  the cached memory copy
> > + *
> > + * @variable_name: name of the variable
> > + * @vendor:vendor GUID
> > + * @attributes:attributes of the variable
> > + * @data_size: size of the buffer to which the variable value 
> > is copied
> > + * @data:  buffer to which the variable value is copied
> > + * @timep: authentication time (seconds since start of epoch)
> > + * Return: status code
> > +
> > + */
> > +efi_status_t __efi_runtime
> > +efi_get_variable_mem(u16 *variable_name, const efi_guid_t *vendor, u32 
> > *attributes,
> > +efi_uintn_t *data_size, void *data, u64 *timep);
> > +
> >  #endif
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index 441ac9432e99..9bad1d159b03 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -37,11 +37,11 @@ obj-y += efi_setup.o
> >  obj-$(CONFIG_EFI_UNICODE_COLLATION_PROTOCOL2) += efi_unicode_collation.o
> >  o

Re: [PATCH] efi_loader: Enable run-time variable support for tee based variables

2020-07-23 Thread Heinrich Schuchardt
On 23.07.20 09:53, Ilias Apalodimas wrote:
> We recently added functions for storing/restoring variables
> from a file to a memory backed buffer marked as __efi_runtime_data
> commit f1f990a8c958 ("efi_loader: memory buffer for variables")
> commit 5f7dcf079de8 ("efi_loader: UEFI variable persistence")
>
> Using the same idea we now can support GetVariable() and GetNextVariable()
> on the OP-TEE based variables as well.
>
> So let's re-arrange the code a bit and move the commmon code for
> accessing variables out of efi_variable.c. Create common functions for
> reading variables from memory that both implementations can use on
> run-time. Then just use those functions in the run-time variants of the
> OP-TEE based EFI variable implementation and initialize the memory
> buffer on ExitBootServices()
>
> Signed-off-by: Ilias Apalodimas 

Overall the changes look good. Some cleanup is needed.

> ---
>  include/efi_variable.h| 45 
>  lib/efi_loader/Makefile   |  2 +-
>  lib/efi_loader/efi_var_file.c | 25 ---
>  lib/efi_loader/efi_var_mem.c  | 70 ++-
>  lib/efi_loader/efi_variable.c | 58 +
>  lib/efi_loader/efi_variable_tee.c | 55 ++--
>  6 files changed, 175 insertions(+), 80 deletions(-)
>
> diff --git a/include/efi_variable.h b/include/efi_variable.h
> index 2c629e4dca92..6ef24cd05feb 100644
> --- a/include/efi_variable.h
> +++ b/include/efi_variable.h
> @@ -142,6 +142,20 @@ struct efi_var_file {
>   */
>  efi_status_t efi_var_to_file(void);
>
> +/**
> + * efi_var_collect() - collect non-volatile variables in buffer

Please, remove the reference to non-volatile here.

> + *
> + * A buffer is allocated and filled with all non-volatile variables in a

Same here.

> + * format ready to be written to disk.

Please, describe that the bits set in check_attr_mask must *all* be set
in the attributes of the variable to have the variable collected, e.g.

@check_attr_mask is a bitmask with required variable attributes.
Variables are only collected if all of the required attributes are set.

> + *
> + * @bufp:pointer to pointer of buffer with collected variables
> + * @lenp:pointer to length of buffer
> + * @check_attr_mask: mask of variable attributes which will be included in 
> the buffer

bitmask with required attributes of variables to be collected

> + * Return:   status code
> + */
> +efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, 
> loff_t *lenp,
> + u32 check_attr_mask);
> +
>  /**
>   * efi_var_restore() - restore EFI variables from buffer
>   *
> @@ -233,4 +247,35 @@ efi_status_t efi_init_secure_state(void);
>   */
>  enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t 
> *guid);
>
> +/**
> + * efi_get_next_variable_name_mem() - Runtime common code across efi variable
> + *implementations for GetNextVariable()
> + *from the cached memory copy
> + * @variable_name_size:  size of variable_name buffer in byte
> + * @variable_name:   name of uefi variable's name in u16
> + * @vendor:  vendor's guid
> + *
> + * Return: status code
> + */
> +efi_status_t __efi_runtime
> +efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 
> *variable_name,
> +efi_guid_t *vendor);
> +/**
> + * efi_get_variable_mem() - Runtime common code across efi variable
> + *  implementations for GetVariable() from
> + *  the cached memory copy
> + *
> + * @variable_name:   name of the variable
> + * @vendor:  vendor GUID
> + * @attributes:  attributes of the variable
> + * @data_size:   size of the buffer to which the variable value 
> is copied
> + * @data:buffer to which the variable value is copied
> + * @timep:   authentication time (seconds since start of epoch)
> + * Return:   status code
> +
> + */
> +efi_status_t __efi_runtime
> +efi_get_variable_mem(u16 *variable_name, const efi_guid_t *vendor, u32 
> *attributes,
> +  efi_uintn_t *data_size, void *data, u64 *timep);
> +
>  #endif
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 441ac9432e99..9bad1d159b03 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -37,11 +37,11 @@ obj-y += efi_setup.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
> diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
> 

[PATCH] efi_loader: Enable run-time variable support for tee based variables

2020-07-23 Thread Ilias Apalodimas
We recently added functions for storing/restoring variables
from a file to a memory backed buffer marked as __efi_runtime_data
commit f1f990a8c958 ("efi_loader: memory buffer for variables")
commit 5f7dcf079de8 ("efi_loader: UEFI variable persistence")

Using the same idea we now can support GetVariable() and GetNextVariable()
on the OP-TEE based variables as well.

So let's re-arrange the code a bit and move the commmon code for
accessing variables out of efi_variable.c. Create common functions for
reading variables from memory that both implementations can use on
run-time. Then just use those functions in the run-time variants of the
OP-TEE based EFI variable implementation and initialize the memory
buffer on ExitBootServices()

Signed-off-by: Ilias Apalodimas 
---
 include/efi_variable.h| 45 
 lib/efi_loader/Makefile   |  2 +-
 lib/efi_loader/efi_var_file.c | 25 ---
 lib/efi_loader/efi_var_mem.c  | 70 ++-
 lib/efi_loader/efi_variable.c | 58 +
 lib/efi_loader/efi_variable_tee.c | 55 ++--
 6 files changed, 175 insertions(+), 80 deletions(-)

diff --git a/include/efi_variable.h b/include/efi_variable.h
index 2c629e4dca92..6ef24cd05feb 100644
--- a/include/efi_variable.h
+++ b/include/efi_variable.h
@@ -142,6 +142,20 @@ struct efi_var_file {
  */
 efi_status_t efi_var_to_file(void);
 
+/**
+ * efi_var_collect() - collect non-volatile variables in buffer
+ *
+ * A buffer is allocated and filled with all non-volatile variables in a
+ * format ready to be written to disk.
+ *
+ * @bufp:  pointer to pointer of buffer with collected variables
+ * @lenp:  pointer to length of buffer
+ * @check_attr_mask:   mask of variable attributes which will be included in 
the buffer
+ * Return: status code
+ */
+efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp, loff_t 
*lenp,
+   u32 check_attr_mask);
+
 /**
  * efi_var_restore() - restore EFI variables from buffer
  *
@@ -233,4 +247,35 @@ efi_status_t efi_init_secure_state(void);
  */
 enum efi_auth_var_type efi_auth_var_get_type(u16 *name, const efi_guid_t 
*guid);
 
+/**
+ * efi_get_next_variable_name_mem() - Runtime common code across efi variable
+ *implementations for GetNextVariable()
+ *from the cached memory copy
+ * @variable_name_size:size of variable_name buffer in byte
+ * @variable_name: name of uefi variable's name in u16
+ * @vendor:vendor's guid
+ *
+ * Return: status code
+ */
+efi_status_t __efi_runtime
+efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 
*variable_name,
+  efi_guid_t *vendor);
+/**
+ * efi_get_variable_mem() - Runtime common code across efi variable
+ *  implementations for GetVariable() from
+ *  the cached memory copy
+ *
+ * @variable_name: name of the variable
+ * @vendor:vendor GUID
+ * @attributes:attributes of the variable
+ * @data_size: size of the buffer to which the variable value is copied
+ * @data:  buffer to which the variable value is copied
+ * @timep: authentication time (seconds since start of epoch)
+ * Return: status code
+
+ */
+efi_status_t __efi_runtime
+efi_get_variable_mem(u16 *variable_name, const efi_guid_t *vendor, u32 
*attributes,
+efi_uintn_t *data_size, void *data, u64 *timep);
+
 #endif
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 441ac9432e99..9bad1d159b03 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -37,11 +37,11 @@ obj-y += efi_setup.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
diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c
index 6f9d76f2a2d5..b171d2d1a8f7 100644
--- a/lib/efi_loader/efi_var_file.c
+++ b/lib/efi_loader/efi_var_file.c
@@ -46,18 +46,8 @@ static efi_status_t __maybe_unused 
efi_set_blk_dev_to_system_partition(void)
return EFI_SUCCESS;
 }
 
-/**
- * efi_var_collect() - collect non-volatile variables in buffer
- *
- * A buffer is allocated and filled with all non-volatile variables in a
- * format ready to be written to disk.
- *
- * @bufp:  pointer to pointer of buffer with collected variables
- * @lenp:  pointer to length of buffer
- * Return: status code
- */
-static efi_status_t __maybe_unused efi_var_collect(struct efi_var_file **bufp,
-