From patchwork Tue Nov 28 19:10:31 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ilias Apalodimas X-Patchwork-Id: 747832 Delivered-To: patch@linaro.org Received: by 2002:a5d:6706:0:b0:32d:baff:b0ca with SMTP id o6csp4018782wru; Tue, 28 Nov 2023 11:10:46 -0800 (PST) X-Google-Smtp-Source: AGHT+IGrsU4B+wVkepC3ukbFVGddNaIrIhjhUfCf39ME6yVMkPLH9WjzRKyWPXXvU6UYRmbGjv3B X-Received: by 2002:a05:6808:1416:b0:3b8:4e37:42b with SMTP id w22-20020a056808141600b003b84e37042bmr22455590oiv.7.1701198645762; Tue, 28 Nov 2023 11:10:45 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701198645; cv=none; d=google.com; s=arc-20160816; b=b0AOU/JUr+f2SfDhTiIobsffpoqphWfE9KirrxZerSFuZJ3kEqtMlOOpYL6x7IhSNX HqUFVMm9aHiuXLtuwRl+vhjEYBUmrDSGWqc/acHx0V12pLS0GqmYvXjQYRmGi8ngsGwP K/UtpvO7xF9TIEI3tuFaIqyjP7m5+CzaII7pB+OyyIteB4OWXDg5ZmZDtJ65ec3wsKBc CPajotc1XrFJXYdBuzmzd92oTqx49SRqOBzwLXimNeNIyZFFbxAFdp4fNakaE7mCgNAS D3XTRgTZyEC9ZklQgtYnNEAvJw0sia0iAO2indkcuItFwreFyGgwNinsJXiMfsbLn0gI lOLQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:cc:to:from:dkim-signature; bh=ofu0ZAm1ZzWGmYPlNN/W3cNC+DAOirmPYPc799aAufc=; fh=B93XzViNTXJLikfPVgKgwxGe21hLlL3emL7ujSYHwTM=; b=U/JxG4HJnflDSTgQULxbGMmtnf6pHnQ+K6tG3fwyi7uv2HhCvbFVmVOMUQhsLedqRB 2kAz4srSoJYc0rmp4FAHwKpAtfzb2xN09K3ohQ7bR2wAc0w+5FiPJ4KwYF/9IChmVUB/ Q+aLjvS99pOIFyHqfK1HjygTZAnMhqPj+aqrz8mfHzsp6AeqQj2nYESDcHrLZ6Hwx3vh jgMNc2Qdll5fDzjvgiMB69H7g8rzo/aeba0lWoWCp27LOS4rXdlyZXNhdAzF6Q/6RURV cfTZqtgKhR6G6cB/ZiuWS1YeyvQnxEYwpJkvAOjH2tI7RPJE5dW7lUVGBpDy8+xdDKkr /JXA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b="X/NxtJ64"; spf=pass (google.com: domain of u-boot-bounces@lists.denx.de designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender) smtp.mailfrom=u-boot-bounces@lists.denx.de; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from phobos.denx.de (phobos.denx.de. [2a01:238:438b:c500:173d:9f52:ddab:ee01]) by mx.google.com with ESMTPS id k2-20020a634b42000000b005641315d956si13009076pgl.147.2023.11.28.11.10.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Nov 2023 11:10:45 -0800 (PST) Received-SPF: pass (google.com: domain of u-boot-bounces@lists.denx.de designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender) client-ip=2a01:238:438b:c500:173d:9f52:ddab:ee01; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b="X/NxtJ64"; spf=pass (google.com: domain of u-boot-bounces@lists.denx.de designates 2a01:238:438b:c500:173d:9f52:ddab:ee01 as permitted sender) smtp.mailfrom=u-boot-bounces@lists.denx.de; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id EC3AA87759; Tue, 28 Nov 2023 20:10:40 +0100 (CET) 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="X/NxtJ64"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0F0C187759; Tue, 28 Nov 2023 20:10:39 +0100 (CET) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on phobos.denx.de X-Spam-Level: X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_PASS, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.2 Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) (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 2048A8776E for ; Tue, 28 Nov 2023 20:10:35 +0100 (CET) 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-wm1-x331.google.com with SMTP id 5b1f17b1804b1-40b4746ae51so19051695e9.2 for ; Tue, 28 Nov 2023 11:10:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1701198634; x=1701803434; darn=lists.denx.de; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=ofu0ZAm1ZzWGmYPlNN/W3cNC+DAOirmPYPc799aAufc=; b=X/NxtJ64xt/6NPyynyZks/RK7bthWxre+mUZFnx6syp0Yz97cCEWeCdOKkxwJ/5lM8 FSo4effoANwZN/ZJS2Gy33/8q9Cobqo9olRyMbElb9B/pWxIqBHfgBaCIxBDDVRBFgHD Fk5FpappL1aQcKJpBQjl8kNSmF1M67608Weg9NJ09R6RMv956XXULsYsfa1MNN3Ee/VI SdhilZcYBD7UPrxaCUukWkQTRwuA/BuEVKWUx1FeS7GBUnJ122x/C4w5M27NJYuJEYv2 DC6lKIFbefHbVMbk2zFhjYYtnZ8c1g8wdLGd1SNwGACNMRYRWa72DI891NOG7A657y9q 0Hrw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701198634; x=1701803434; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=ofu0ZAm1ZzWGmYPlNN/W3cNC+DAOirmPYPc799aAufc=; b=Guj70qkb9gk9BKZ18IWSd7WVm0FOgYpDD3n14dUukK76PGaOe6VMdmTjMz91iZ1ANb D5+KNgyh/P7GILcVcHvT0OWdOY/Q+HxU7FYf8JQCgWn9R5PMIZQ4TsBVb0FPVrBq/e2u PyjzGldLT+/lfaWPM9WL3RT7kpAEM1FoqydCTZjZppWkR1zKmz6C5Efj81o6N87gYS87 UWHyzEbvxQDTG9ym4QrS1zhTgR43As6afVOs0036k5bMzgsc7FGtbGIlLj7zanVO2tv5 105PZeKJqIkm5lxeTdSJxOKY/kxRWEbxbq6PUMOJiFLXygcLACROgwBAvXq9bcfJ3879 F90A== X-Gm-Message-State: AOJu0Yy+OBCPGByYi6M0AejJyFhOVoY/dgSe+pg/z8HNU9guL/w8Mo+0 32MBZAqorbnPTtibiIGR5J/WrQ== X-Received: by 2002:adf:ffd0:0:b0:32f:7a65:da64 with SMTP id x16-20020adfffd0000000b0032f7a65da64mr11884637wrs.65.1701198634542; Tue, 28 Nov 2023 11:10:34 -0800 (PST) Received: from hades.. (ppp046103219117.access.hol.gr. [46.103.219.117]) by smtp.gmail.com with ESMTPSA id cr6-20020a05600004e600b00332cc24a59bsm15653028wrb.109.2023.11.28.11.10.33 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 28 Nov 2023 11:10:34 -0800 (PST) From: Ilias Apalodimas To: xypron.glpk@gmx.de Cc: masahisa.kojima@linaro.org, ryosuke.saito@linaro.org, Ilias Apalodimas , u-boot@lists.denx.de Subject: [PATCH v2] efi_loader: Make DisconnectController follow the EFI spec Date: Tue, 28 Nov 2023 21:10:31 +0200 Message-Id: <20231128191031.467222-1-ilias.apalodimas@linaro.org> X-Mailer: git-send-email 2.37.2 MIME-Version: 1.0 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 commit 239d59a65e20 ("efi_loader: reconnect drivers on failure") tried to fix the UninstallProtocol interface which must reconnect any controllers it disconnected by calling ConnectController() in case of failure. However, the reconnect functionality was wired in efi_disconnect_all_drivers() instead of efi_uninstall_protocol(). As a result some SCT tests started failing. Specifically, BBTestOpenProtocolInterfaceTest333CheckPoint3() test - Calls ConnectController for DriverImageHandle1 - Calls DisconnectController for DriverImageHandle1 which will disconnect everything apart from TestProtocol4. That will remain open on purpose. - Calls ConnectController for DriverImageHandle2. TestProtocol4 which was explicitly preserved was installed wth BY_DRIVER attributes. The new protocol will call DisconnectController since its attributes are BY_DRIVER|EXCLUSIVE, but TestProtocol4 will not be removed. The test expects EFI_ACCESS_DENIED which works fine. The problem is that DisconnectController, will eventually call EFI_DRIVER_BINDING_PROTOCOL.Stop(). But on the aforementioned test this will call CloseProtocol -- the binding protocol is defined in 'DBindingDriver3.c' and the .Stop function uses CloseProtocol. If that close protocol call fails with EFI_NOT_FOUND, the current code will try to mistakenly reconnect all drivers and the subsequent tests that rely on the device being disconnected will fail. Move the reconnection in efi_uninstall_protocol() were it belongs. Fixes: commit 239d59a65e20 ("efi_loader: reconnect drivers on failure") Signed-off-by: Ilias Apalodimas --- Apologies for the fast resend, but since Heinrich reviewed it and we want it in 2024.01 resending Changes since v1: - return ret instead of (ret != EFI_SUCCESS ? ret : EFI_SUCCESS) which does the same thing... - Add a comment about reconnecting all controllers lib/efi_loader/efi_boottime.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) -- 2.37.2 diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 0b7579cb5af1..ea711919630b 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -1339,7 +1339,7 @@ static efi_status_t efi_disconnect_all_drivers const efi_guid_t *protocol, efi_handle_t child_handle) { - efi_uintn_t number_of_drivers, tmp; + efi_uintn_t number_of_drivers; efi_handle_t *driver_handle_buffer; efi_status_t r, ret; @@ -1350,27 +1350,13 @@ static efi_status_t efi_disconnect_all_drivers if (!number_of_drivers) return EFI_SUCCESS; - tmp = number_of_drivers; while (number_of_drivers) { - ret = EFI_CALL(efi_disconnect_controller( + r = EFI_CALL(efi_disconnect_controller( handle, driver_handle_buffer[--number_of_drivers], child_handle)); - if (ret != EFI_SUCCESS) - goto reconnect; - } - - free(driver_handle_buffer); - return ret; - -reconnect: - /* Reconnect all disconnected drivers */ - for (; number_of_drivers < tmp; number_of_drivers++) { - r = EFI_CALL(efi_connect_controller(handle, - &driver_handle_buffer[number_of_drivers], - NULL, true)); if (r != EFI_SUCCESS) - EFI_PRINT("Failed to reconnect controller\n"); + ret = r; } free(driver_handle_buffer); @@ -1409,6 +1395,13 @@ static efi_status_t efi_uninstall_protocol r = efi_disconnect_all_drivers(handle, protocol, NULL); if (r != EFI_SUCCESS) { r = EFI_ACCESS_DENIED; + /* + * This will reconnect all controllers of the handle, even ones + * that were not connected before. This can be done better + * but we are following the EDKII implementation on this for + * now + */ + EFI_CALL(efi_connect_controller(handle, NULL, NULL, true)); goto out; } /* Close protocol */