diff mbox series

[1/3,v2] efi: Fix to use null handle to create new handle for efi_fmp_raw

Message ID 20210618105115.257938-1-ilias.apalodimas@linaro.org
State Superseded
Delegated to: Heinrich Schuchardt
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 
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(-)

Comments

Heinrich Schuchardt June 18, 2021, 7:22 p.m. UTC | #1 
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));
> 	}
Masami Hiramatsu June 19, 2021, 4:23 a.m. UTC | #2 
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));
> >       }
>
Ilias Apalodimas June 22, 2021, 4:46 a.m. UTC | #3 
[...]
> >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));
> > 	}
>
diff mbox series

Patch

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));
 	}