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 <[email protected]> > > --- > > 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; >
