diff mbox series

[2/3,v2] efi_loader: Force a sinlge FMP instance per hardware store

Message ID 20210618105115.257938-2-ilias.apalodimas@linaro.org
State New
Headers show
Series [1/3,v2] efi: Fix to use null handle to create new handle for efi_fmp_raw | expand

Commit Message

Ilias Apalodimas June 18, 2021, 10:51 a.m. UTC
Chapter 23 of the EFI spec (rev 2.9) says:
"A specific updatable hardware firmware store must be represented by
exactly one FMP instance".
This is not the case for us, since both of our FMP protocols can be
installed at the same time because they are controlled by a single
'dfu_alt_info' env variable.
So make the config options depend on each other and allow the user to
install one of them at any given time.  If we fix the meta-data provided
by the 'dfu_alt_info' in the future,  to hint about the capsule type
(fit or raw) we can revise this and enable both FMPs to be installed, as
long as they target different firmware hardware stores

Note that we are not using a Kconfig 'choice' on purpose, since we
want to allow both of those to be installed and tested in sandbox

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

---
 configs/xilinx_zynqmp_virt_defconfig |  1 -
 lib/efi_loader/Kconfig               | 45 ++++++++++++++--------------
 lib/efi_loader/efi_capsule.c         | 12 +++-----
 3 files changed, 26 insertions(+), 32 deletions(-)

-- 
2.32.0.rc0

Comments

Masami Hiramatsu June 18, 2021, 1:52 p.m. UTC | #1
Hi Ilias,

2021年6月18日(金) 19:51 Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>

> Chapter 23 of the EFI spec (rev 2.9) says:

> "A specific updatable hardware firmware store must be represented by

> exactly one FMP instance".

> This is not the case for us, since both of our FMP protocols can be

> installed at the same time because they are controlled by a single

> 'dfu_alt_info' env variable.

> So make the config options depend on each other and allow the user to

> install one of them at any given time.  If we fix the meta-data provided

> by the 'dfu_alt_info' in the future,  to hint about the capsule type

> (fit or raw) we can revise this and enable both FMPs to be installed, as

> long as they target different firmware hardware stores

>

> Note that we are not using a Kconfig 'choice' on purpose, since we

> want to allow both of those to be installed and tested in sandbox


This sounds like changing the Kconfig, thus...

[...]
> --- a/lib/efi_loader/efi_capsule.c

> +++ b/lib/efi_loader/efi_capsule.c

> @@ -918,15 +918,15 @@ static void efi_capsule_scan_done(void)

>  }

>

>  /**

> - * arch_efi_load_capsule_drivers - initialize capsule drivers

> + * efi_load_capsule_drivers - initialize capsule drivers

>   *

> - * Architecture or board specific initialization routine

> + * Generic FMP drivers backed by DFU

>   *

>   * Return:     status code

>   */

> -efi_status_t __weak arch_efi_load_capsule_drivers(void)

> +efi_status_t __weak efi_load_capsule_drivers(void)

>  {

> -       __maybe_unused efi_handle_t handle;

> +       __maybe_unused efi_handle_t handle = NULL;

>         efi_status_t ret = EFI_SUCCESS;

>

>         if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)) {

> @@ -975,10 +975,6 @@ efi_status_t efi_launch_capsules(void)

>

>         index = get_last_capsule();

>

> -       /* Load capsule drivers */

> -       ret = arch_efi_load_capsule_drivers();

> -       if (ret != EFI_SUCCESS)

> -               return ret;


I think this part of the change should be included in the next patch.

Thank you,

>

>         /*

>          * Find capsules on disk.

> --

> 2.32.0.rc0

>



-- 
Masami Hiramatsu
Ilias Apalodimas June 18, 2021, 2:01 p.m. UTC | #2
On Fri, Jun 18, 2021 at 10:52:51PM +0900, Masami Hiramatsu wrote:
> Hi Ilias,

> 

> 2021???6???18???(???) 19:51 Ilias Apalodimas <ilias.apalodimas@linaro.org>:

> >

> > Chapter 23 of the EFI spec (rev 2.9) says:

> > "A specific updatable hardware firmware store must be represented by

> > exactly one FMP instance".

> > This is not the case for us, since both of our FMP protocols can be

> > installed at the same time because they are controlled by a single

> > 'dfu_alt_info' env variable.

> > So make the config options depend on each other and allow the user to

> > install one of them at any given time.  If we fix the meta-data provided

> > by the 'dfu_alt_info' in the future,  to hint about the capsule type

> > (fit or raw) we can revise this and enable both FMPs to be installed, as

> > long as they target different firmware hardware stores

> >

> > Note that we are not using a Kconfig 'choice' on purpose, since we

> > want to allow both of those to be installed and tested in sandbox

> 

> This sounds like changing the Kconfig, thus...

> 


It does 

> [...]

> >

> > -       /* Load capsule drivers */

> > -       ret = arch_efi_load_capsule_drivers();

> > -       if (ret != EFI_SUCCESS)

> > -               return ret;

> 

> I think this part of the change should be included in the next patch.

> 

> Thank you,


Yep, I completely missed this during the rebasing.  I'll send a v3.

Thanks!
/Ilias
diff mbox series

Patch

diff --git a/configs/xilinx_zynqmp_virt_defconfig b/configs/xilinx_zynqmp_virt_defconfig
index e939b04ef6a5..0c2d1a70a5a1 100644
--- a/configs/xilinx_zynqmp_virt_defconfig
+++ b/configs/xilinx_zynqmp_virt_defconfig
@@ -188,5 +188,4 @@  CONFIG_EFI_SET_TIME=y
 CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_ON_DISK_EARLY=y
-CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 6242caceb7f9..684adfb62379 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -161,6 +161,28 @@  config EFI_CAPSULE_FIRMWARE_MANAGEMENT
 	  Select this option if you want to enable capsule-based
 	  firmware update using Firmware Management Protocol.
 
+config EFI_CAPSULE_FIRMWARE_FIT
+	bool "FMP driver for FIT images"
+	depends on FIT
+	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
+	select UPDATE_FIT
+	select DFU
+	select EFI_CAPSULE_FIRMWARE
+	help
+	  Select this option if you want to enable firmware management protocol
+	  driver for FIT image
+
+config EFI_CAPSULE_FIRMWARE_RAW
+	bool "FMP driver for raw images"
+	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
+	depends on SANDBOX || (!SANDBOX && !EFI_CAPSULE_FIRMWARE_FIT)
+	select DFU_WRITE_ALT
+	select DFU
+	select EFI_CAPSULE_FIRMWARE
+	help
+	  Select this option if you want to enable firmware management protocol
+	  driver for raw image
+
 config EFI_CAPSULE_AUTHENTICATE
 	bool "Update Capsule authentication"
 	depends on EFI_CAPSULE_FIRMWARE
@@ -181,29 +203,6 @@  config EFI_CAPSULE_AUTHENTICATE
 	  Select this option if you want to enable capsule
 	  authentication
 
-config EFI_CAPSULE_FIRMWARE_FIT
-	bool "FMP driver for FIT image"
-	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
-	depends on FIT
-	select UPDATE_FIT
-	select DFU
-	select EFI_CAPSULE_FIRMWARE
-	default n
-	help
-	  Select this option if you want to enable firmware management protocol
-	  driver for FIT image
-
-config EFI_CAPSULE_FIRMWARE_RAW
-	bool "FMP driver for raw image"
-	depends on EFI_CAPSULE_FIRMWARE_MANAGEMENT
-	select DFU
-	select DFU_WRITE_ALT
-	select EFI_CAPSULE_FIRMWARE
-	default n
-	help
-	  Select this option if you want to enable firmware management protocol
-	  driver for raw image
-
 config EFI_DEVICE_PATH_TO_TEXT
 	bool "Device path to text protocol"
 	default y
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 71d3d1e523ce..6d9f6aee237e 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -918,15 +918,15 @@  static void efi_capsule_scan_done(void)
 }
 
 /**
- * arch_efi_load_capsule_drivers - initialize capsule drivers
+ * efi_load_capsule_drivers - initialize capsule drivers
  *
- * Architecture or board specific initialization routine
+ * Generic FMP drivers backed by DFU
  *
  * Return:	status code
  */
-efi_status_t __weak arch_efi_load_capsule_drivers(void)
+efi_status_t __weak efi_load_capsule_drivers(void)
 {
-	__maybe_unused efi_handle_t handle;
+	__maybe_unused efi_handle_t handle = NULL;
 	efi_status_t ret = EFI_SUCCESS;
 
 	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)) {
@@ -975,10 +975,6 @@  efi_status_t efi_launch_capsules(void)
 
 	index = get_last_capsule();
 
-	/* Load capsule drivers */
-	ret = arch_efi_load_capsule_drivers();
-	if (ret != EFI_SUCCESS)
-		return ret;
 
 	/*
 	 * Find capsules on disk.