Skip to contentSkip to conversation

efi_loader: Disable env_save() call on boot

PATCH In review Alexander Graf 4 participants 4 messages Oct 19, 2017 – Oct 29, 2017

lore
Share & tools
lore

Alexander Graf

author
#0 lore raw

PATCH 1/1 efi_loader: Disable env_save() call on boot

With the introduction of EFI variable support, we also wanted to persist these EFI variables. However, the way it was implemented we ended up persisting all U-Boot environment variables on every EFI boot.

That could potentially lead to unexpected side effects because variables that were not supposed to be written to persisted env get written. It also means we may end up writing the environment more often than we should.

For this release, let's just disable EFI variable persistence and instead implement it properly for the next one.

Reported-by Heinrich Schuchardt <[email protected]>
Fixes ad644e7c182 ("efi_loader: efi variable support")
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_boottime.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
lib/efi_loader/efi_boottime.c +1 -4
index f627340de4..743b84864f 100644
@@ -1439,10 +1439,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
 	/* Make sure that notification functions are not called anymore */
 	efi_tpl = TPL_HIGH_LEVEL;
 
-#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
-	/* save any EFI variables that have been written: */
-	env_save();
-#endif
+	/* XXX Should persist EFI variables here */
 
 	board_quiesce_devices();
Signature · 1 line
2.12.3
#1 lore raw

PATCH 1/1 efi_loader: Disable env_save() call on boot

Quoted 11 lines · Alexander Graf

With the introduction of EFI variable support, we also wanted to persist these EFI variables. However, the way it was implemented we ended up persisting all U-Boot environment variables on every EFI boot.

That could potentially lead to unexpected side effects because variables that were not supposed to be written to persisted env get written. It also means we may end up writing the environment more often than we should.

For this release, let's just disable EFI variable persistence and instead implement it properly for the next one.

Acked-by Rob Clark <[email protected]>
Quoted 26 lines
Reported-by Heinrich Schuchardt <[email protected]>
Fixes ad644e7c182 ("efi_loader: efi variable support")
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_boottime.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
lib/efi_loader/efi_boottime.c +1 -5
index f627340de4..743b84864f 100644
@@ -1439,10 +1439,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
        /* Make sure that notification functions are not called anymore */
        efi_tpl = TPL_HIGH_LEVEL;
 
-#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
-       /* save any EFI variables that have been written: */
-       env_save();
-#endif
+       /* XXX Should persist EFI variables here */
 
        board_quiesce_devices();
 
--

2.12.3

#2 lore raw

PATCH 1/1 efi_loader: Disable env_save() call on boot

In reply to Alexander Graf
Quoted 25 lines · Alexander Graf

With the introduction of EFI variable support, we also wanted to persist these EFI variables. However, the way it was implemented we ended up persisting all U-Boot environment variables on every EFI boot.

That could potentially lead to unexpected side effects because variables that were not supposed to be written to persisted env get written. It also means we may end up writing the environment more often than we should.

For this release, let's just disable EFI variable persistence and instead implement it properly for the next one.

Reported-by Heinrich Schuchardt <[email protected]>
Fixes ad644e7c182 ("efi_loader: efi variable support")
Signed-off-by Alexander Graf <[email protected]>
---
 lib/efi_loader/efi_boottime.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)
lib/efi_loader/efi_boottime.c +0 -0
index f627340de4..743b84864f 100644
@@ -1439,10 +1439,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
 	/* Make sure that notification functions are not called anymore */

lib/efi_loader/efi_boottime.c static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle, · line 1445

Quoted 10 lines
lib/efi_loader/efi_boottime.c +1 -4
 	efi_tpl = TPL_HIGH_LEVEL;
 
-#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_ENV_IS_NOWHERE)
-	/* save any EFI variables that have been written: */
-	env_save();
-#endif
+	/* XXX Should persist EFI variables here */
 
 	board_quiesce_devices();
Reviewed-by Heinrich Schuchardt <[email protected]>
#3 lore raw
In reply to Alexander Graf
Quoted 8 lines · Alexander Graf

With the introduction of EFI variable support, we also wanted to persist these EFI variables. However, the way it was implemented we ended up persisting all U-Boot environment variables on every EFI boot.

That could potentially lead to unexpected side effects because variables that were not supposed to be written to persisted env get written. It also means we may end up writing the environment more often than we should.

Quoted 8 lines

For this release, let's just disable EFI variable persistence and instead implement it properly for the next one.

Reported-by Heinrich Schuchardt <[email protected]>
Fixes ad644e7c182 ("efi_loader: efi variable support")
Signed-off-by Alexander Graf <[email protected]>
Acked-by Rob Clark <[email protected]>
Reviewed-by Heinrich Schuchardt <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20171029/b161764b/attachment.sig>