| Message ID | 20230615065737.323329-1-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] efi_loader: use efi_install_multiple_protocol_interfaces() | expand |
On 6/15/23 08:57, Ilias Apalodimas wrote: > The tcg protocol currently adds and removes protocols with > efi_(add/remove)_protocol(). Although this works fine protocol > interfaces should be installed using the EFI API functions instead > of the internal API ones I would prefer the commit message to explicitly state the reason: Currently DisconnectController() is not called when uninstalling the TCG2 protocol which does not comply to the UEFI specification. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > lib/efi_loader/efi_tcg2.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > index a83ae7a46cf3..49f8a5e77cbf 100644 > --- a/lib/efi_loader/efi_tcg2.c > +++ b/lib/efi_loader/efi_tcg2.c > @@ -1680,8 +1680,8 @@ void tcg2_uninit(void) > if (!is_tcg2_protocol_installed()) > return; > > - ret = efi_remove_protocol(efi_root, &efi_guid_tcg2_protocol, > - (void *)&efi_tcg2_protocol); > + ret = efi_uninstall_multiple_protocol_interfaces(efi_root, &efi_guid_tcg2_protocol, > + &efi_tcg2_protocol, NULL); For a single protocol interface efi_uninstall_protocol() is good enough. > if (ret != EFI_SUCCESS) > log_err("Failed to remove EFI TCG2 protocol\n"); > } > @@ -2507,8 +2507,8 @@ efi_status_t efi_tcg2_register(void) > goto fail; > } > > - ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, > - (void *)&efi_tcg2_protocol); > + ret = efi_install_multiple_protocol_interfaces(&efi_root, &efi_guid_tcg2_protocol, > + &efi_tcg2_protocol, NULL); What is the benefit of this change? Best regards Heinrich > if (ret != EFI_SUCCESS) { > tcg2_uninit(); > goto fail;
On Sun, Jun 18, 2023 at 08:03:16AM +0200, Heinrich Schuchardt wrote: > On 6/15/23 08:57, Ilias Apalodimas wrote: > > The tcg protocol currently adds and removes protocols with > > efi_(add/remove)_protocol(). Although this works fine protocol > > interfaces should be installed using the EFI API functions instead > > of the internal API ones > > I would prefer the commit message to explicitly state the reason: > > Currently DisconnectController() is not called when uninstalling the > TCG2 protocol which does not comply to the UEFI specification. > Sure, I can send a v2 updating the description. We could also add that using UninstallMultiple instead of protocl_remove() also removes the handle if not used > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > lib/efi_loader/efi_tcg2.c | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c > > index a83ae7a46cf3..49f8a5e77cbf 100644 > > --- a/lib/efi_loader/efi_tcg2.c > > +++ b/lib/efi_loader/efi_tcg2.c > > @@ -1680,8 +1680,8 @@ void tcg2_uninit(void) > > if (!is_tcg2_protocol_installed()) > > return; > > > > - ret = efi_remove_protocol(efi_root, &efi_guid_tcg2_protocol, > > - (void *)&efi_tcg2_protocol); > > + ret = efi_uninstall_multiple_protocol_interfaces(efi_root, &efi_guid_tcg2_protocol, > > + &efi_tcg2_protocol, NULL); > > For a single protocol interface efi_uninstall_protocol() is good enough. > I'd rather keep this as is. Since I am using InstallMultiple() using this one to remove is easier to read. > > if (ret != EFI_SUCCESS) > > log_err("Failed to remove EFI TCG2 protocol\n"); > > } > > @@ -2507,8 +2507,8 @@ efi_status_t efi_tcg2_register(void) > > goto fail; > > } > > > > - ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, > > - (void *)&efi_tcg2_protocol); > > + ret = efi_install_multiple_protocol_interfaces(&efi_root, &efi_guid_tcg2_protocol, > > + &efi_tcg2_protocol, NULL); > > What is the benefit of this change? > efi_add_protocol() doesn't check for a class in installed device paths and neither does efi_install_protocol_interface(). But apart from all that I'd like us to move away from using functions interchangeably when installing a protocol. IMHO (and that's what the second path does) we should slowly replace efi_add_protocol -> efi_install_multiple_protocol_interfaces() and make efi_add_protocol() static as well. Similarly we should remove protocols with efi_uninstall_multiple_protocol_interfaces() and remove the handle automatically as well. I also have more patches which I'll send next week [0] which clean the whole interface usage further. [0] https://source.denx.de/u-boot/custodians/u-boot-tpm/-/commit/02ac924bb6db020dc4e4284936069ecd46806542 Thanks /Ilias > Best regards > > Heinrich > > > if (ret != EFI_SUCCESS) { > > tcg2_uninit(); > > goto fail; >
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index a83ae7a46cf3..49f8a5e77cbf 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -1680,8 +1680,8 @@ void tcg2_uninit(void) if (!is_tcg2_protocol_installed()) return; - ret = efi_remove_protocol(efi_root, &efi_guid_tcg2_protocol, - (void *)&efi_tcg2_protocol); + ret = efi_uninstall_multiple_protocol_interfaces(efi_root, &efi_guid_tcg2_protocol, + &efi_tcg2_protocol, NULL); if (ret != EFI_SUCCESS) log_err("Failed to remove EFI TCG2 protocol\n"); } @@ -2507,8 +2507,8 @@ efi_status_t efi_tcg2_register(void) goto fail; } - ret = efi_add_protocol(efi_root, &efi_guid_tcg2_protocol, - (void *)&efi_tcg2_protocol); + ret = efi_install_multiple_protocol_interfaces(&efi_root, &efi_guid_tcg2_protocol, + &efi_tcg2_protocol, NULL); if (ret != EFI_SUCCESS) { tcg2_uninit(); goto fail;
The tcg protocol currently adds and removes protocols with efi_(add/remove)_protocol(). Although this works fine protocol interfaces should be installed using the EFI API functions instead of the internal API ones Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- lib/efi_loader/efi_tcg2.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)