[efi] Match chainloaded device by uppermost matching handle

Commit 4c5b794 ("[efi] Use the SNP protocol instance to match the SNP
chainloading device") switched the chainloaded device matching logic
to use a target protocol instance rather than the loaded image's
device handle, on the basis that we want to bind to the parent SNP
device rather than to a duplicate SNP protocol instance installed onto
an IPv4 or IPv6 child device handle.

It is possible that our calls to DisconnectController() and
ConnectController() will cause the target protocol instance to be
uninstalled and reinstalled, which may change the value of the
protocol instance pointer.  Allow for this by identifying and matching
against the uppermost handle that initially has this target protocol
instance installed.

Signed-off-by: Michael Brown <mcb30@ipxe.org>

1 files changed, 70 insertions, 42 deletions

diff --git a/src/drivers/net/efi/snponly.c b/src/drivers/net/efi/snponly.c
index 674e0a050..163704636 100644
--- a/src/drivers/net/efi/snponly.c
+++ b/src/drivers/net/efi/snponly.c
@@ -45,14 +45,23 @@ struct chained_protocol {
 	/** Protocol GUID */
 	EFI_GUID *protocol;
 	/**
-	 * Protocol instance installed on the loaded image's device handle
+	 * Target device handle
+	 *
+	 * This is the uppermost handle on which the same protocol
+	 * instance is installed as we find on the loaded image's
+	 * device handle.
 	 *
 	 * We match against the protocol instance (rather than simply
 	 * matching against the device handle itself) because some
 	 * systems load us via a child of the underlying device, with
 	 * a duplicate protocol installed on the child handle.
+	 *
+	 * We record the handle rather than the protocol instance
+	 * pointer since the calls to DisconnectController() and
+	 * ConnectController() may end up uninstalling and
+	 * reinstalling the protocol instance.
 	 */
-	void *interface;
+	EFI_HANDLE device;
 };
 
 /** Chainloaded SNP protocol */
@@ -66,49 +75,68 @@ static struct chained_protocol chained_nii = {
 };
 
 /**
- * Locate chainloaded protocol instance
+ * Locate chainloaded protocol
  *
  * @v chained		Chainloaded protocol
- * @ret rc		Return status code
  */
-static int chained_locate ( struct chained_protocol *chained ) {
+static void chained_locate ( struct chained_protocol *chained ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
 	EFI_HANDLE device = efi_loaded_image->DeviceHandle;
-	EFI_HANDLE parent;
+	EFI_HANDLE handle;
+	void *match = NULL;
+	void *interface;
+	unsigned int skip;
 	EFI_STATUS efirc;
 	int rc;
 
-	/* Locate handle supporting this protocol */
-	if ( ( rc = efi_locate_device ( device, chained->protocol,
-					&parent, 0 ) ) != 0 ) {
-		DBGC ( device, "CHAINED %s does not support %s: %s\n",
-		       efi_handle_name ( device ),
-		       efi_guid_ntoa ( chained->protocol ), strerror ( rc ) );
-		goto err_locate_device;
-	}
-	DBGC ( device, "CHAINED %s found %s on ", efi_handle_name ( device ),
-	       efi_guid_ntoa ( chained->protocol ) );
-	DBGC ( device, "%s\n", efi_handle_name ( parent ) );
-
-	/* Get protocol instance */
-	if ( ( efirc = bs->OpenProtocol ( parent, chained->protocol,
-					  &chained->interface, efi_image_handle,
-					  device,
-					  EFI_OPEN_PROTOCOL_GET_PROTOCOL ))!=0){
-		rc = -EEFI ( efirc );
-		DBGC ( device, "CHAINED %s could not open %s on ",
+	/* Identify target device handle */
+	for ( skip = 0 ; ; skip++ ) {
+
+		/* Locate handle supporting this protocol */
+		if ( ( rc = efi_locate_device ( device, chained->protocol,
+						&handle, skip ) ) != 0 ) {
+			if ( skip == 0 ) {
+				DBGC ( device, "CHAINED %s does not support "
+				       "%s: %s\n", efi_handle_name ( device ),
+				       efi_guid_ntoa ( chained->protocol ),
+				       strerror ( rc ) );
+			}
+			break;
+		}
+
+		/* Get protocol instance */
+		if ( ( efirc = bs->OpenProtocol (
+					handle, chained->protocol, &interface,
+					efi_image_handle, handle,
+					EFI_OPEN_PROTOCOL_GET_PROTOCOL )) != 0){
+			rc = -EEFI ( efirc );
+			DBGC ( device, "CHAINED %s could not open %s on ",
+			       efi_handle_name ( device ),
+			       efi_guid_ntoa ( chained->protocol ) );
+			DBGC ( device, "%s: %s\n",
+			       efi_handle_name ( handle ), strerror ( rc ) );
+			break;
+		}
+		bs->CloseProtocol ( handle, chained->protocol,
+				    efi_image_handle, handle );
+
+		/* Stop if we reach a non-matching protocol instance */
+		if ( match && ( match != interface ) ) {
+			DBGC ( device, "CHAINED %s found non-matching %s on ",
+			       efi_handle_name ( device ),
+			       efi_guid_ntoa ( chained->protocol ) );
+			DBGC ( device, "%s\n", efi_handle_name ( handle ) );
+			break;
+		}
+
+		/* Record this handle */
+		chained->device = handle;
+		match = interface;
+		DBGC ( device, "CHAINED %s found %s on ",
 		       efi_handle_name ( device ),
 		       efi_guid_ntoa ( chained->protocol ) );
-		DBGC ( device, "%s: %s\n",
-		       efi_handle_name ( parent ), strerror ( rc ) );
-		goto err_open_protocol;
+		DBGC ( device, "%s\n", efi_handle_name ( chained->device ) );
 	}
-
- err_locate_device:
-	bs->CloseProtocol ( parent, chained->protocol, efi_image_handle,
-			    device );
- err_open_protocol:
-	return rc;
 }
 
 /**
@@ -121,8 +149,8 @@ static int chained_locate ( struct chained_protocol *chained ) {
 static int chained_supported ( EFI_HANDLE device,
 			       struct chained_protocol *chained ) {
 	EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-	EFI_STATUS efirc;
 	void *interface;
+	EFI_STATUS efirc;
 	int rc;
 
 	/* Get protocol */
@@ -136,19 +164,19 @@ static int chained_supported ( EFI_HANDLE device,
 		goto err_open_protocol;
 	}
 
-	/* Test for a match against the chainloading device */
-	if ( interface != chained->interface ) {
-		DBGC ( device, "CHAINED %s %p is not the chainloaded %s\n",
-		       efi_handle_name ( device ), interface,
-		       efi_guid_ntoa ( chained->protocol ) );
+	/* Ignore non-matching handles */
+	if ( device != chained->device ) {
+		DBGC2 ( device, "CHAINED %s is not the chainloaded %s\n",
+			efi_handle_name ( device ),
+			efi_guid_ntoa ( chained->protocol ) );
 		rc = -ENOTTY;
 		goto err_no_match;
 	}
 
 	/* Success */
 	rc = 0;
-	DBGC ( device, "CHAINED %s %p is the chainloaded %s\n",
-	       efi_handle_name ( device ), interface,
+	DBGC ( device, "CHAINED %s is the chainloaded %s\n",
+	       efi_handle_name ( device ),
 	       efi_guid_ntoa ( chained->protocol ) );
 
  err_no_match: