All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw
@ 2021-06-18 10:51 Ilias Apalodimas
  2021-06-18 10:51 ` [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store Ilias Apalodimas
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2021-06-18 10:51 UTC (permalink / raw)
  To: xypron.glpk
  Cc: masami.hiramatsu, takahiro.akashi, Ilias Apalodimas,
	Michal Simek, Alexander Graf, u-boot

From: Masami Hiramatsu <masami.hiramatsu@linaro.org>

When running the efidebug capsule disk-update command, the efi_fmp_raw
protocol installation is failed with 2 (EFI_INVALID_PARAMETER) as
below. This is because the code passes efi_root instaed of handle.

=> efidebug capsule disk-update
EFI: Call: efi_install_multiple_protocol_interfaces( &handle, &efi_guid_firmware_management_protocol, &efi_fmp_fit, NULL)
  EFI: Entry efi_install_multiple_protocol_interfaces(00000000fbaf5988)
    EFI: Call: efi_install_protocol_interface( handle, protocol, EFI_NATIVE_INTERFACE, protocol_interface)
      EFI: Entry efi_install_protocol_interface(00000000fbaf5988, 86c77a67-0b97-4633-a187-49104d0685c7, 0, 00000000fbfa6ee8)
        EFI: new handle 00000000fbb37520
      EFI: Exit: efi_install_protocol_interface: 0
    EFI: 0 returned by efi_install_protocol_interface( handle, protocol, EFI_NATIVE_INTERFACE, protocol_interface)
  EFI: Exit: efi_install_multiple_protocol_interfaces: 0
EFI: 0 returned by efi_install_multiple_protocol_interfaces( &handle, &efi_guid_firmware_management_protocol, &efi_fmp_fit, NULL)
EFI: Call: efi_install_multiple_protocol_interfaces( &efi_root, &efi_guid_firmware_management_protocol, &efi_fmp_raw, NULL)
  EFI: Entry efi_install_multiple_protocol_interfaces(00000000fbfec648)
    EFI: Call: efi_install_protocol_interface( handle, protocol, EFI_NATIVE_INTERFACE, protocol_interface)
      EFI: Entry efi_install_protocol_interface(00000000fbfec648, 86c77a67-0b97-4633-a187-49104d0685c7, 0, 00000000fbfa6f18)
        EFI: handle 00000000fbaf8520
      EFI: Exit: efi_install_protocol_interface: 2
    EFI: 2 returned by efi_install_protocol_interface( handle, protocol, EFI_NATIVE_INTERFACE, protocol_interface)
  EFI: Exit: efi_install_multiple_protocol_interfaces: 2
EFI: 2 returned by efi_install_multiple_protocol_interfaces( &efi_root, &efi_guid_firmware_management_protocol, &efi_fmp_raw, NULL)
Command failed, result=1

To fix this issue, pass the handle variable as same as the efi_fmp_fit
installation.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
v1 is at https://lists.denx.de/pipermail/u-boot/2021-June/452097.html
changes since v1:
- split patches
- pick up Masami's patch fixing the efi_root issue
- Don't use 'choice' on Kconfig sicne we need both of the FMPs installed for sandbox testing
  instead just make them mutually exclusive with 'depends on'
- Only install the FMPs if CONFIG_EFI_HAVE_CAPSULE_SUPPORT is selected
 lib/efi_loader/efi_capsule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 9ead0d2c7816..71d3d1e523ce 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -939,7 +939,7 @@ efi_status_t __weak arch_efi_load_capsule_drivers(void)
 	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) {
 		handle = NULL;
 		ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
-				&efi_root,
+				&handle,
 				&efi_guid_firmware_management_protocol,
 				&efi_fmp_raw, NULL));
 	}
-- 
2.32.0.rc0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store
  2021-06-18 10:51 [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Ilias Apalodimas
@ 2021-06-18 10:51 ` Ilias Apalodimas
  2021-06-18 13:52   ` Masami Hiramatsu
  2021-06-18 10:51 ` [PATCH 3/3 v2] efi_loader: Always install FMPs Ilias Apalodimas
  2021-06-18 19:22 ` [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Heinrich Schuchardt
  2 siblings, 1 reply; 8+ messages in thread
From: Ilias Apalodimas @ 2021-06-18 10:51 UTC (permalink / raw)
  To: xypron.glpk
  Cc: masami.hiramatsu, takahiro.akashi, Ilias Apalodimas,
	Michal Simek, Alexander Graf, u-boot

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(-)

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.
-- 
2.32.0.rc0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3 v2] efi_loader: Always install FMPs
  2021-06-18 10:51 [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Ilias Apalodimas
  2021-06-18 10:51 ` [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store Ilias Apalodimas
@ 2021-06-18 10:51 ` Ilias Apalodimas
  2021-06-18 19:22 ` [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Heinrich Schuchardt
  2 siblings, 0 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2021-06-18 10:51 UTC (permalink / raw)
  To: xypron.glpk
  Cc: masami.hiramatsu, takahiro.akashi, Ilias Apalodimas,
	Michal Simek, Alexander Graf, u-boot

We only install FMPs if a CapsuleUpdate is requested.  Since we now have an
ESRT table which relies on FMPs to build the required information,  it
makes more sense to unconditionally install them. This will allow userspace
applications (e.g fwupd) to make use of the ERST and provide us with files
we can use to run CapsuleUpdate on-disk

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 include/efi_loader.h       | 1 +
 lib/efi_loader/efi_setup.c | 6 ++++++
 2 files changed, 7 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 0a9c82a257e1..b81180cfda8b 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -972,4 +972,5 @@ efi_status_t efi_esrt_register(void);
  * - error code otherwise.
  */
 efi_status_t efi_esrt_populate(void);
+efi_status_t efi_load_capsule_drivers(void);
 #endif /* _EFI_LOADER_H */
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 3c5cf9a4357e..2dc6ba8337a7 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -254,6 +254,12 @@ efi_status_t efi_init_obj_list(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
+	if (IS_ENABLED(CONFIG_EFI_HAVE_CAPSULE_SUPPORT)) {
+		ret = efi_load_capsule_drivers();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 #if defined(CONFIG_LCD) || defined(CONFIG_DM_VIDEO)
 	ret = efi_gop_register();
 	if (ret != EFI_SUCCESS)
-- 
2.32.0.rc0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store
  2021-06-18 10:51 ` [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store Ilias Apalodimas
@ 2021-06-18 13:52   ` Masami Hiramatsu
  2021-06-18 14:01     ` Ilias Apalodimas
  0 siblings, 1 reply; 8+ messages in thread
From: Masami Hiramatsu @ 2021-06-18 13:52 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: Heinrich Schuchardt, Takahiro Akashi, Michal Simek,
	Alexander Graf, U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store
  2021-06-18 13:52   ` Masami Hiramatsu
@ 2021-06-18 14:01     ` Ilias Apalodimas
  0 siblings, 0 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2021-06-18 14:01 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Heinrich Schuchardt, Takahiro Akashi, Michal Simek,
	Alexander Graf, U-Boot Mailing List

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw
  2021-06-18 10:51 [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Ilias Apalodimas
  2021-06-18 10:51 ` [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store Ilias Apalodimas
  2021-06-18 10:51 ` [PATCH 3/3 v2] efi_loader: Always install FMPs Ilias Apalodimas
@ 2021-06-18 19:22 ` Heinrich Schuchardt
  2021-06-19  4:23   ` Masami Hiramatsu
  2021-06-22  4:46   ` Ilias Apalodimas
  2 siblings, 2 replies; 8+ messages in thread
From: Heinrich Schuchardt @ 2021-06-18 19:22 UTC (permalink / raw)
  To: Ilias Apalodimas
  Cc: masami.hiramatsu, takahiro.akashi, Michal Simek, Alexander Graf, u-boot

Am 18. Juni 2021 12:51:12 MESZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>:
>From: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>
>When running the efidebug capsule disk-update command, the efi_fmp_raw
>protocol installation is failed with 2 (EFI_INVALID_PARAMETER) as
>below. This is because the code passes efi_root instaed of handle.

instead

Is the problem that two protocols with the same GUID are installed on the same handle?


>
>=> efidebug capsule disk-update
>EFI: Call: efi_install_multiple_protocol_interfaces( &handle,
>&efi_guid_firmware_management_protocol, &efi_fmp_fit, NULL)
>  EFI: Entry efi_install_multiple_protocol_interfaces(00000000fbaf5988)
>EFI: Call: efi_install_protocol_interface( handle, protocol,
>EFI_NATIVE_INTERFACE, protocol_interface)
>EFI: Entry efi_install_protocol_interface(00000000fbaf5988,
>86c77a67-0b97-4633-a187-49104d0685c7, 0, 00000000fbfa6ee8)
>        EFI: new handle 00000000fbb37520
>      EFI: Exit: efi_install_protocol_interface: 0
>EFI: 0 returned by efi_install_protocol_interface( handle, protocol,
>EFI_NATIVE_INTERFACE, protocol_interface)
>  EFI: Exit: efi_install_multiple_protocol_interfaces: 0
>EFI: 0 returned by efi_install_multiple_protocol_interfaces( &handle,
>&efi_guid_firmware_management_protocol, &efi_fmp_fit, NULL)
>EFI: Call: efi_install_multiple_protocol_interfaces( &efi_root,
>&efi_guid_firmware_management_protocol, &efi_fmp_raw, NULL)
>  EFI: Entry efi_install_multiple_protocol_interfaces(00000000fbfec648)
>EFI: Call: efi_install_protocol_interface( handle, protocol,
>EFI_NATIVE_INTERFACE, protocol_interface)
>EFI: Entry efi_install_protocol_interface(00000000fbfec648,
>86c77a67-0b97-4633-a187-49104d0685c7, 0, 00000000fbfa6f18)
>        EFI: handle 00000000fbaf8520
>      EFI: Exit: efi_install_protocol_interface: 2
>EFI: 2 returned by efi_install_protocol_interface( handle, protocol,
>EFI_NATIVE_INTERFACE, protocol_interface)
>  EFI: Exit: efi_install_multiple_protocol_interfaces: 2
>EFI: 2 returned by efi_install_multiple_protocol_interfaces( &efi_root,
>&efi_guid_firmware_management_protocol, &efi_fmp_raw, NULL)
>Command failed, result=1
>
>To fix this issue, pass the handle variable as same as the efi_fmp_fit
>installation.


You can not install twice protocols with the same GUID one one handle
 What do you mean by same variable?

>
>Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
>Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>---
>v1 is at https://lists.denx.de/pipermail/u-boot/2021-June/452097.html
>changes since v1:
>- split patches
>- pick up Masami's patch fixing the efi_root issue
>- Don't use 'choice' on Kconfig sicne we need both of the FMPs
>installed for sandbox testing
>  instead just make them mutually exclusive with 'depends on'

I can't see ''depends on" in this patch.

Best regards

Heinrich


>- Only install the FMPs if CONFIG_EFI_HAVE_CAPSULE_SUPPORT is selected
> lib/efi_loader/efi_capsule.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/efi_loader/efi_capsule.c
>b/lib/efi_loader/efi_capsule.c
>index 9ead0d2c7816..71d3d1e523ce 100644
>--- a/lib/efi_loader/efi_capsule.c
>+++ b/lib/efi_loader/efi_capsule.c
>@@ -939,7 +939,7 @@ efi_status_t __weak
>arch_efi_load_capsule_drivers(void)
> 	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) {
> 		handle = NULL;
> 		ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
>-				&efi_root,
>+				&handle,
> 				&efi_guid_firmware_management_protocol,
> 				&efi_fmp_raw, NULL));
> 	}


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw
  2021-06-18 19:22 ` [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Heinrich Schuchardt
@ 2021-06-19  4:23   ` Masami Hiramatsu
  2021-06-22  4:46   ` Ilias Apalodimas
  1 sibling, 0 replies; 8+ messages in thread
From: Masami Hiramatsu @ 2021-06-19  4:23 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: Ilias Apalodimas, Takahiro Akashi, Michal Simek, Alexander Graf,
	U-Boot Mailing List

Hi,

2021年6月19日(土) 4:22 Heinrich Schuchardt <xypron.glpk@gmx.de>:
>
> Am 18. Juni 2021 12:51:12 MESZ schrieb Ilias Apalodimas <ilias.apalodimas@linaro.org>:
> >From: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> >
> >When running the efidebug capsule disk-update command, the efi_fmp_raw
> >protocol installation is failed with 2 (EFI_INVALID_PARAMETER) as
> >below. This is because the code passes efi_root instaed of handle.
>
> instead

Oops, it's my typo.

>
> Is the problem that two protocols with the same GUID are installed on the same handle?

Actually, the returned handle is ignored in both cases. That "handle"
is the local variable, and the handle is set to NULL before installing
FMP in both cases.

>
>
> >
> >=> efidebug capsule disk-update
> >EFI: Call: efi_install_multiple_protocol_interfaces( &handle,
> >&efi_guid_firmware_management_protocol, &efi_fmp_fit, NULL)
> >  EFI: Entry efi_install_multiple_protocol_interfaces(00000000fbaf5988)
> >EFI: Call: efi_install_protocol_interface( handle, protocol,
> >EFI_NATIVE_INTERFACE, protocol_interface)
> >EFI: Entry efi_install_protocol_interface(00000000fbaf5988,
> >86c77a67-0b97-4633-a187-49104d0685c7, 0, 00000000fbfa6ee8)
> >        EFI: new handle 00000000fbb37520
> >      EFI: Exit: efi_install_protocol_interface: 0
> >EFI: 0 returned by efi_install_protocol_interface( handle, protocol,
> >EFI_NATIVE_INTERFACE, protocol_interface)
> >  EFI: Exit: efi_install_multiple_protocol_interfaces: 0
> >EFI: 0 returned by efi_install_multiple_protocol_interfaces( &handle,
> >&efi_guid_firmware_management_protocol, &efi_fmp_fit, NULL)
> >EFI: Call: efi_install_multiple_protocol_interfaces( &efi_root,
> >&efi_guid_firmware_management_protocol, &efi_fmp_raw, NULL)
> >  EFI: Entry efi_install_multiple_protocol_interfaces(00000000fbfec648)
> >EFI: Call: efi_install_protocol_interface( handle, protocol,
> >EFI_NATIVE_INTERFACE, protocol_interface)
> >EFI: Entry efi_install_protocol_interface(00000000fbfec648,
> >86c77a67-0b97-4633-a187-49104d0685c7, 0, 00000000fbfa6f18)
> >        EFI: handle 00000000fbaf8520
> >      EFI: Exit: efi_install_protocol_interface: 2
> >EFI: 2 returned by efi_install_protocol_interface( handle, protocol,
> >EFI_NATIVE_INTERFACE, protocol_interface)
> >  EFI: Exit: efi_install_multiple_protocol_interfaces: 2
> >EFI: 2 returned by efi_install_multiple_protocol_interfaces( &efi_root,
> >&efi_guid_firmware_management_protocol, &efi_fmp_raw, NULL)
> >Command failed, result=1
> >
> >To fix this issue, pass the handle variable as same as the efi_fmp_fit
> >installation.
>
>
> You can not install twice protocols with the same GUID one one handle
>  What do you mean by same variable?

Ah, yes. The description might not be correct. The handle is
initialized right before installing a new protocol with the same GUID
(previous one is discarded at that point).

>
> >
> >Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> >Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> >---
> >v1 is at https://lists.denx.de/pipermail/u-boot/2021-June/452097.html
> >changes since v1:
> >- split patches
> >- pick up Masami's patch fixing the efi_root issue
> >- Don't use 'choice' on Kconfig sicne we need both of the FMPs
> >installed for sandbox testing
> >  instead just make them mutually exclusive with 'depends on'
>
> I can't see ''depends on" in this patch.

Aha, I think that comment is for the 2nd patch in the series.

Thank you,

>
> Best regards
>
> Heinrich
>
>
> >- Only install the FMPs if CONFIG_EFI_HAVE_CAPSULE_SUPPORT is selected
> > lib/efi_loader/efi_capsule.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lib/efi_loader/efi_capsule.c
> >b/lib/efi_loader/efi_capsule.c
> >index 9ead0d2c7816..71d3d1e523ce 100644
> >--- a/lib/efi_loader/efi_capsule.c
> >+++ b/lib/efi_loader/efi_capsule.c
> >@@ -939,7 +939,7 @@ efi_status_t __weak
> >arch_efi_load_capsule_drivers(void)
> >       if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) {
> >               handle = NULL;
> >               ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
> >-                              &efi_root,
> >+                              &handle,
> >                               &efi_guid_firmware_management_protocol,
> >                               &efi_fmp_raw, NULL));
> >       }
>


-- 
Masami Hiramatsu

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw
  2021-06-18 19:22 ` [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Heinrich Schuchardt
  2021-06-19  4:23   ` Masami Hiramatsu
@ 2021-06-22  4:46   ` Ilias Apalodimas
  1 sibling, 0 replies; 8+ messages in thread
From: Ilias Apalodimas @ 2021-06-22  4:46 UTC (permalink / raw)
  To: Heinrich Schuchardt
  Cc: masami.hiramatsu, takahiro.akashi, Michal Simek, Alexander Graf, u-boot

[...]
> >v1 is at https://lists.denx.de/pipermail/u-boot/2021-June/452097.html
> >changes since v1:
> >- split patches
> >- pick up Masami's patch fixing the efi_root issue
> >- Don't use 'choice' on Kconfig sicne we need both of the FMPs
> >installed for sandbox testing
> >  instead just make them mutually exclusive with 'depends on'
> 
> I can't see ''depends on" in this patch.

V1 was a single patch, so I included all the changes in the first patch.
I'll split them in v3

Thanks
/Ilias
> 
> Best regards
> 
> Heinrich
> 
> 
> >- Only install the FMPs if CONFIG_EFI_HAVE_CAPSULE_SUPPORT is selected
> > lib/efi_loader/efi_capsule.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/lib/efi_loader/efi_capsule.c
> >b/lib/efi_loader/efi_capsule.c
> >index 9ead0d2c7816..71d3d1e523ce 100644
> >--- a/lib/efi_loader/efi_capsule.c
> >+++ b/lib/efi_loader/efi_capsule.c
> >@@ -939,7 +939,7 @@ efi_status_t __weak
> >arch_efi_load_capsule_drivers(void)
> > 	if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) {
> > 		handle = NULL;
> > 		ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
> >-				&efi_root,
> >+				&handle,
> > 				&efi_guid_firmware_management_protocol,
> > 				&efi_fmp_raw, NULL));
> > 	}
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-06-22  4:46 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-18 10:51 [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Ilias Apalodimas
2021-06-18 10:51 ` [PATCH 2/3 v2] efi_loader: Force a sinlge FMP instance per hardware store Ilias Apalodimas
2021-06-18 13:52   ` Masami Hiramatsu
2021-06-18 14:01     ` Ilias Apalodimas
2021-06-18 10:51 ` [PATCH 3/3 v2] efi_loader: Always install FMPs Ilias Apalodimas
2021-06-18 19:22 ` [PATCH 1/3 v2] efi: Fix to use null handle to create new handle for efi_fmp_raw Heinrich Schuchardt
2021-06-19  4:23   ` Masami Hiramatsu
2021-06-22  4:46   ` Ilias Apalodimas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.