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 91426C433FE for ; Thu, 6 Oct 2022 09:43:40 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 060218492C; Thu, 6 Oct 2022 11:43:38 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=gmx.net header.i=@gmx.net header.b="afywNRcJ"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 7304A84940; Thu, 6 Oct 2022 11:43:36 +0200 (CEST) Received: from mout.gmx.net (mout.gmx.net [212.227.15.15]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 80E3684233 for ; Thu, 6 Oct 2022 11:43:33 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1665049412; bh=qh9qNrg13VrxZi4H2zA6mZnoWYpiSW8B2rCszrKMVBE=; h=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To; b=afywNRcJMpr29HBC8BoKCG079UdDFNK5c7dJJCKqOERRHHhrDZUKnZEMbj74w88Dm 1gNJHI17BiWwO4hnT+WbhwZlm+nvVE3+tAMromj+XLWrAAUPA7QxwWr/P5zivoo1mN zpJCcVG3pADfhAh/TNXybDOyfoOC9e2/BLQBQfWU= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.94] ([84.118.157.2]) by mail.gmx.net (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MHGCo-1oTA0q19Ci-00DKBy; Thu, 06 Oct 2022 11:43:32 +0200 Message-ID: Date: Thu, 6 Oct 2022 11:43:26 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [RFC PATCH 2/2] cmd: replace efi_create_handle/add_protocol with InstallMultipleProtocol Content-Language: en-US To: Ilias Apalodimas Cc: AKASHI Takahiro , u-boot@lists.denx.de References: <20221005152603.3085754-1-ilias.apalodimas@linaro.org> <20221005152603.3085754-3-ilias.apalodimas@linaro.org> <20221006015346.GC38245@laputa> <62e539e4-6c51-b457-5c64-37f71770d3aa@gmx.de> From: Heinrich Schuchardt In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:DZhsyF5mmM1qNTyUnklWx2u6Ra5dpLYj8qFKkXNcLdDDgtAkdER ABF03++rQx4EyxbW9zBojP4VEE8/nytazYRMyihCTc1RuEdX9slTpAAjUjrUZAV6ZGZYfRi 8ytasqVZvJJzsFdnfBihp7zsiP0WR3PVRLW8AQYyz3XvJUtPaDqBvVkqw0R4IlcFE8WzGnt Tyfi261Ztan2n0srn8sGQ== X-UI-Out-Filterresults: notjunk:1;V03:K0:lvdL/Gr7HE4=:sJNTD3yEUk/caTw1KKu2+c 4dvopvkKtxDZ1JU5bxAnzENmA4o0GcwmEgQ/JvIInEeJ1sWflJQcpEv5TAyDlcKtlOPzKpCqG hlxodj4WB0sGLDT49xLgQRJcdg41LKyKa2zD66fP8sD4EC/tujhc95dfLMw1QDeDpd1B9r4Nd OzcpWZnRScWBaGVTM7Vpo7D4sFjHkBEsDKt6apiZeL9PZVqiUCLGrV0B5I9Tfd8hVq7PrBK3/ 0mwaR5X8ROOHmva8hkReDMVhcop2Hq7/IRC9OnksPFPQDydlBDOm2HE9tnyu82ojRYVVTYAyz d7uDPZX4nPW8kM3xXG4Gvf2SvZxHbks2hIN3MDhWlcFz8nQPo5v7MZ7Z76M/C7gNY6mGCSc2G Gu8IcaDGLBF2zRk3zxj+a0801MIOOoEAM6EtBjAnl9CYD+RXQlweBjU18eQVv3RhaH19bfdVl SBe1n619sLVueKOVfhdg8tKFWHaqckr0lf/zAzSqFmVBoJAe/wo9f7KIInqikn0tQavb/Y+Y5 uk9syRqWF2bpBW7VLLalDLL6F4eJ+VhSUOZulG3GO9zKhdDPfFIGCWWHlrd1nZNkCEO1NwVtu s6hFTRaIVEYgyBOkWH+vuI/oZaZCMP9F1HCMngZIytwJG29NgZRfi96MMETBSEes6RjZyTXqt QAaNLrpAdph7jXiZI9vW6mzx/USvCOkxG7jRkVCeCw5+XnncEPyxhFXPYTVbIVo/DOz2Wdkos 6qtr/93dcJG53P0CiefPw44FYgRac1NEw2e9bJq6d3zIHJdJedZFhyhmjbOikdPgKIkihthm2 VwRGKol2V8Zi9PoyMIP6He5cPpuj85kC3UvpoBHDujig2DhqGEnaZmQzSL1NM+3tabVMneKS5 7SqTzwYFe0OqTxKWxheZzKMQosSwR4NOhnwdYo0JG3v8JBA8yHylmJCRL8WNg3bcT3PC312Qw YpSQcdih3A8oMYZhT10jo3WhaVQnW9cCmFvx+Tfw/lPFG3eHm/fLoCG1WM2+JhPRqPrwnkixE zS6SIOAsPicwbOuGwXkeY3Z3d//QJUqlObUmuOMr0KkfpRhA8n44J7ZLdOsc7VohtQpc0YkR0 tGjf+UsmUuywXwwHsfWBiSeqhdu3P6xBoSSdZpT0VnvSDeN9VuhHXaWIIJNXjYLSsTFT1HPy7 Z4QF94hJjr4mMK4ESl40/LQ4A+ 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.6 at phobos.denx.de X-Virus-Status: Clean On 10/6/22 09:38, Ilias Apalodimas wrote: > On Thu, Oct 06, 2022 at 04:26:37AM +0200, Heinrich Schuchardt wrote: >> On 10/6/22 03:53, AKASHI Takahiro wrote: >>> On Wed, Oct 05, 2022 at 06:26:02PM +0300, Ilias Apalodimas wrote: >>>> In general handles should only be deleted if the last remaining proto= col >>>> is removed. Instead of explicitly calling >>>> efi_create_handle -> efi_add_protocol -> efi_delete_handle which blin= dly >>>> removes all protocols from a handle before removing it, use >>>> InstallMultiple/UninstallMultiple which adheres to the EFI spec and o= nly >>>> deletes a handle if there are no additional protocols present >>>> >>>> Signed-off-by: Ilias Apalodimas >>>> --- >>>> cmd/bootefi.c | 13 ++++++------- >>>> 1 file changed, 6 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >>>> index 3041873afbbc..4ab68868cc7e 100644 >>>> --- a/cmd/bootefi.c >>>> +++ b/cmd/bootefi.c >>>> @@ -509,12 +509,9 @@ efi_status_t efi_run_image(void *source_buffer, = efi_uintn_t source_size) >>>> * Make sure that device for device_path exist >>>> * in load_image(). Otherwise, shell and grub will fail. >>>> */ >>>> - ret =3D efi_create_handle(&mem_handle); >>>> - if (ret !=3D EFI_SUCCESS) >>>> - goto out; >>>> - >>>> - ret =3D efi_add_protocol(mem_handle, &efi_guid_device_path, >>>> - file_path); >>>> + ret =3D efi_install_multiple_protocol_interfaces(&mem_handle, >>>> + &efi_guid_device_path, >>>> + file_path, NULL); >>> >>> nitpick: NULL -> NULL, NULL >>> UEFI spec seems to require "A variable argument list containing pairs = of >>> protocol GUIDs and protocol interfaces" even if a protocol interface w= on't be >>> used with GUID as NULL. >> >> The spec also has: >> "The pairs of arguments are removed in order from the variable argument >> list until a NULL protocol GUID value is found." >> >> This is what a calls inside EDK II looks like: >> >> Status =3D CoreInstallMultipleProtocolInterfaces ( >> &mDecompressHandle, >> &gEfiDecompressProtocolGuid, >> &gEfiDecompress, >> NULL >> ); >> >> gBS->UninstallMultipleProtocolInterfaces ( >> Controller, >> &gEfiCallerIdGuid, >> AtaBusDriverData, >> NULL >> ); >> >> So I am happy with a single NULL here. >> >>> >>> -Takahiro Akashi >>> >>>> if (ret !=3D EFI_SUCCESS) >>>> goto out; >>>> msg_path =3D file_path; >>>> @@ -542,7 +539,9 @@ efi_status_t efi_run_image(void *source_buffer, e= fi_uintn_t source_size) >>>> ret =3D do_bootefi_exec(handle, load_options); >>>> >>>> out: >>>> - efi_delete_handle(mem_handle); >>>> + efi_uninstall_multiple_protocol_interfaces(mem_handle, >>>> + &efi_guid_device_path, >>>> + file_path, NULL); >> >> We have installed a lot more protocols. The binary may have installed >> additional protocols. Consider the case of a boottime driver. To delete >> the handle we would have to remove all installed protocols. >> >> UninstallMultipleProtocolInterfaces() may fail if one of the protocols >> was opened with ByDriver or ByChildcontroller. The return value has to >> be considered before freeing file_path. > > Ok I'll fix it in v2 We only install the device path file_path on mem_handle. The loaded image itself is referenced by handle. So the only thing you missed here was checking the return value. Best regards Heinrich > >> >> Best regards >> >> Heinrich >> >>>> efi_free_pool(file_path); >>>> return ret; >>>> } >>>> -- >>>> 2.34.1 >>>> >> > > Thanks > /Ilias