From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9CF2CEB64D7 for ; Sun, 18 Jun 2023 14:15:02 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1573F862B3; Sun, 18 Jun 2023 16:14:59 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=linaro.org header.i=@linaro.org header.b="W8TvjSdR"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id A29BD862CB; Sun, 18 Jun 2023 16:14:56 +0200 (CEST) Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 0370B861A9 for ; Sun, 18 Jun 2023 16:14:53 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-wr1-x434.google.com with SMTP id ffacd0b85a97d-311153ec442so1736758f8f.1 for ; Sun, 18 Jun 2023 07:14:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1687097692; x=1689689692; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=koxUUuVXmipXcV3cj4V3yJsswlnphuRk3h/W97YM8Eg=; b=W8TvjSdRJvFbPy9huvEC3fesbvibqfro3oseJEKBsk4k9lNGQTlfpE8/Txp+kEot+3 UWxmpvuJPY4TXmSxshFLT9tkaQmiA4IcJ7Ekc/u6E3TDL6gqEWyzgoxqxSYBPFhx4n6U ZWc3JWMRC5tSxqdFoFfZkbhYHS/t8hPBBqVsy3a1YHR9UhR/g6G+GOZnQy2SzPo01nfQ snYLz2YLPzNqdnB5Tz+5PcTyf8gEbb9c4PZITmUKXxN8wENQHhLw50xvrItvjA7nn8L0 lbxazqToSqQg0wF0nOJPWsHJoC/8owqcnsNcjQ2AdgJrbHnch9Gs7ynRu+y/iucfKix3 EY+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687097692; x=1689689692; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=koxUUuVXmipXcV3cj4V3yJsswlnphuRk3h/W97YM8Eg=; b=WwMxAzonmRfFDuyxv20vZLy6xhftduibGRN4MBZf2lWqE5Keji+e9uYJR8h6evzWlI 99ThdSoUKEU+HBY9DMktgelY9YV4nLmYMG8xWn1EfRfxg+fuq2jH1lOlkHPzztryxmzx oo0QJMW1i6fjvmclNKtXyCFX6H06sEo2mlBZolKp77RQpjrkOkqyUGJEJ/BlfDFXRtaw vhfQ+y588WmG7xPZPIBXUXfhMB0OY+62oPDJGErOxC3PEDJrs3nDGaMHDGSw/4tQdl9c nnEGLGGSH75+CIE+zg5Gibw9zxJxFkcrwQ264SeZjfIUL5HAdAPJTBHlKKr8VPsAvNcs g/yQ== X-Gm-Message-State: AC+VfDwgnOnLcn5W6gfr10TWiDtKHL/n4rMhMAgEjLhKe92cmH3olIaU YAfk0kZJO+cXegCMixoSgHKvLQVSzHUstJo5BY70GQ== X-Google-Smtp-Source: ACHHUZ78jF1vhKYOGjfK0eUQJHrWrS4Jks75lMBqf80IcIWSE6XyKLDUA7yZ7aY9tO6SP6og8VUF2Q== X-Received: by 2002:a5d:6a46:0:b0:311:10ae:123c with SMTP id t6-20020a5d6a46000000b0031110ae123cmr9781929wrw.16.1687097692342; Sun, 18 Jun 2023 07:14:52 -0700 (PDT) Received: from hades (ppp089210114029.access.hol.gr. [89.210.114.29]) by smtp.gmail.com with ESMTPSA id v18-20020a5d6112000000b003112dbc3257sm3884297wrt.90.2023.06.18.07.14.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 18 Jun 2023 07:14:51 -0700 (PDT) Date: Sun, 18 Jun 2023 17:14:46 +0300 From: Ilias Apalodimas To: Heinrich Schuchardt Cc: u-boot@lists.denx.de Subject: Re: [PATCH 1/2] efi_loader: use efi_install_multiple_protocol_interfaces() Message-ID: References: <20230615065737.323329-1-ilias.apalodimas@linaro.org> <557b7d76-6053-e3d4-1aae-5fd7d0814525@gmx.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <557b7d76-6053-e3d4-1aae-5fd7d0814525@gmx.de> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean 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 > > --- > > 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; >