[efi] Open device path protocol only at point of use

Some EFI 1.10 systems (observed on an Apple iMac) do not allow us to
open the device path protocol with an attribute of
EFI_OPEN_PROTOCOL_BY_DRIVER and so we cannot maintain a safe,
long-lived pointer to the device path.  Work around this by instead
opening the device path protocol with an attribute of
EFI_OPEN_PROTOCOL_GET_PROTOCOL whenever we need to use it.

Debugged-by: Curtis Larsen <larsen@dixie.edu>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/drivers/net/efi/snpnet.c b/src/drivers/net/efi/snpnet.c
index 75574526f08780a94b4628835611ba4f33527c5e..79b4946c4297723cc41f7df4da1ef4bc3ff190e7 100644 (file)
--- a/src/drivers/net/efi/snpnet.c
+++ b/src/drivers/net/efi/snpnet.c
@@ -389,33 +389,56 @@ static struct net_device_operations snpnet_operations = {
  */
 static int snpnet_pci_info ( struct efi_device *efidev, struct device *dev ) {
        EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
-       EFI_DEVICE_PATH_PROTOCOL *devpath = efidev->path;
        EFI_HANDLE device = efidev->device;
+       union {
+               EFI_DEVICE_PATH_PROTOCOL *path;
+               void *interface;
+       } path;
+       EFI_DEVICE_PATH_PROTOCOL *devpath;
        struct pci_device pci;
        EFI_HANDLE pci_device;
        EFI_STATUS efirc;
        int rc;
 
+       /* Get device path */
+       if ( ( efirc = bs->OpenProtocol ( device,
+                                         &efi_device_path_protocol_guid,
+                                         &path.interface,
+                                         efi_image_handle, device,
+                                         EFI_OPEN_PROTOCOL_GET_PROTOCOL ))!=0){
+               rc = -EEFI ( efirc );
+               DBGC ( device, "SNP %p %s cannot open device path: %s\n",
+                      device, efi_handle_name ( device ), strerror ( rc ) );
+               goto err_open_device_path;
+       }
+       devpath = path.path;
+
        /* Check for presence of PCI I/O protocol */
        if ( ( efirc = bs->LocateDevicePath ( &efi_pci_io_protocol_guid,
                                              &devpath, &pci_device ) ) != 0 ) {
+               rc = -EEFI ( efirc );
                DBGC ( device, "SNP %p %s is not a PCI device\n",
                       device, efi_handle_name ( device ) );
-               return -EEFI ( efirc );
+               goto err_locate_pci_io;
        }
 
        /* Get PCI device information */
        if ( ( rc = efipci_info ( pci_device, &pci ) ) != 0 ) {
                DBGC ( device, "SNP %p %s could not get PCI information: %s\n",
                       device, efi_handle_name ( device ), strerror ( rc ) );
-               return rc;
+               goto err_efipci_info;
        }
 
        /* Populate SNP device information */
        memcpy ( &dev->desc, &pci.dev.desc, sizeof ( dev->desc ) );
        snprintf ( dev->name, sizeof ( dev->name ), "SNP-%s", pci.dev.name );
 
-       return 0;
+ err_efipci_info:
+ err_locate_pci_io:
+       bs->CloseProtocol ( device, &efi_device_path_protocol_guid,
+                           efi_image_handle, device );
+ err_open_device_path:
+       return rc;
 }
 
 /**

diff --git a/src/image/efi_image.c b/src/image/efi_image.c
index f5ce102f73a4bb18f76c473c2c8eab4357a8aa1f..613c9d923e826272f3ba26b8d22fec065e80d627 100644 (file)
--- a/src/image/efi_image.c
+++ b/src/image/efi_image.c
@@ -167,7 +167,7 @@ static int efi_image_exec ( struct image *image ) {
        }
 
        /* Create device path for image */
-       path = efi_image_path ( image, &snpdev->path );
+       path = efi_image_path ( image, snpdev->path );
        if ( ! path ) {
                DBGC ( image, "EFIIMAGE %p could not create device path\n",
                       image );

diff --git a/src/include/ipxe/efi/efi_driver.h b/src/include/ipxe/efi/efi_driver.h
index 21bff0e55034d941e5ce82f77ab334f4cc96e336..1b890b0613a6083cbd36d519fec35d53c84765e9 100644 (file)
--- a/src/include/ipxe/efi/efi_driver.h
+++ b/src/include/ipxe/efi/efi_driver.h
@@ -19,8 +19,6 @@ struct efi_device {
        struct device dev;
        /** EFI device handle */
        EFI_HANDLE device;
-       /** Device path */
-       EFI_DEVICE_PATH_PROTOCOL *path;
        /** Driver for this device */
        struct efi_driver *driver;
        /** Driver-private data */

diff --git a/src/include/ipxe/efi/efi_snp.h b/src/include/ipxe/efi/efi_snp.h
index 87798ccc2238933d141213723241feee8355c963..a18bced5ff428379b99ef142a25e85495f6a11cb 100644 (file)
--- a/src/include/ipxe/efi/efi_snp.h
+++ b/src/include/ipxe/efi/efi_snp.h
@@ -66,12 +66,8 @@ struct efi_snp_device {
        wchar_t driver_name[16];
        /** Controller name */
        wchar_t controller_name[64];
-       /** The device path
-        *
-        * This field is variable in size and must appear at the end
-        * of the structure.
-        */
-       EFI_DEVICE_PATH_PROTOCOL path;
+       /** The device path */
+       EFI_DEVICE_PATH_PROTOCOL *path;
 };
 
 extern int efi_snp_hii_install ( struct efi_snp_device *snpdev );

diff --git a/src/interface/efi/efi_driver.c b/src/interface/efi/efi_driver.c
index 424bbc315ba8cce30af50fd4e36168bfb114c6a9..3a1945a5f2f62a1215c68a76cc1a3841edf309ab 100644 (file)
--- a/src/interface/efi/efi_driver.c
+++ b/src/interface/efi/efi_driver.c
@@ -208,13 +208,8 @@ efi_driver_supported ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
 static EFI_STATUS EFIAPI
 efi_driver_start ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
                   EFI_HANDLE device, EFI_DEVICE_PATH_PROTOCOL *child ) {
-       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
        struct efi_driver *efidrv;
        struct efi_device *efidev;
-       union {
-               EFI_DEVICE_PATH_PROTOCOL *devpath;
-               void *interface;
-       } devpath;
        EFI_STATUS efirc;
        int rc;
 
@@ -244,22 +239,6 @@ efi_driver_start ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
        INIT_LIST_HEAD ( &efidev->dev.children );
        list_add ( &efidev->dev.siblings, &efi_devices );
 
-       /* Open device path protocol */
-       if ( ( efirc = bs->OpenProtocol ( device,
-                                         &efi_device_path_protocol_guid,
-                                         &devpath.interface,
-                                         efi_image_handle, device,
-                                         EFI_OPEN_PROTOCOL_BY_DRIVER ) ) != 0){
-               rc = -EEFI ( efirc );
-               DBGC ( device, "EFIDRV %p %s could not open device path: %s\n",
-                      device, efi_handle_name ( device ),
-                      strerror ( rc ) );
-               DBGC_EFI_OPENERS ( device, device,
-                                  &efi_device_path_protocol_guid );
-               goto err_no_device_path;
-       }
-       efidev->path = devpath.devpath;
-
        /* Try to start this device */
        for_each_table_entry ( efidrv, EFI_DRIVERS ) {
                if ( ( rc = efidrv->supported ( device ) ) != 0 ) {
@@ -282,9 +261,6 @@ efi_driver_start ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
        }
        efirc = EFI_UNSUPPORTED;
 
-       bs->CloseProtocol ( device, &efi_device_path_protocol_guid,
-                           efi_image_handle, device );
- err_no_device_path:
        list_del ( &efidev->dev.siblings );
        free ( efidev );
  err_alloc:
@@ -306,7 +282,6 @@ static EFI_STATUS EFIAPI
 efi_driver_stop ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
                  EFI_HANDLE device, UINTN num_children,
                  EFI_HANDLE *children ) {
-       EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
        struct efi_driver *efidrv;
        struct efi_device *efidev;
        UINTN i;
@@ -331,8 +306,6 @@ efi_driver_stop ( EFI_DRIVER_BINDING_PROTOCOL *driver __unused,
        efidrv = efidev->driver;
        assert ( efidrv != NULL );
        efidrv->stop ( efidev );
-       bs->CloseProtocol ( efidev->device, &efi_device_path_protocol_guid,
-                           efi_image_handle, efidev->device );
        list_del ( &efidev->dev.siblings );
        free ( efidev );
 

diff --git a/src/interface/efi/efi_snp.c b/src/interface/efi/efi_snp.c
index b5eeab21289a919961b2e30cc3fa88c5b041d42d..2eec29cab9a292a89fdb6f760a551dc5e42c27a7 100644 (file)
--- a/src/interface/efi/efi_snp.c
+++ b/src/interface/efi/efi_snp.c
@@ -909,6 +909,10 @@ static int efi_snp_probe ( struct net_device *netdev ) {
        EFI_BOOT_SERVICES *bs = efi_systab->BootServices;
        struct efi_device *efidev;
        struct efi_snp_device *snpdev;
+       union {
+               EFI_DEVICE_PATH_PROTOCOL *path;
+               void *interface;
+       } path;
        EFI_DEVICE_PATH_PROTOCOL *path_end;
        MAC_ADDR_DEVICE_PATH *macpath;
        size_t path_prefix_len = 0;
@@ -923,14 +927,8 @@ static int efi_snp_probe ( struct net_device *netdev ) {
                goto err_no_efidev;
        }
 
-       /* Calculate device path prefix length */
-       path_end = efi_devpath_end ( efidev->path );
-       path_prefix_len = ( ( ( void * ) path_end ) -
-                           ( ( void * ) efidev->path ) );
-
        /* Allocate the SNP device */
-       snpdev = zalloc ( sizeof ( *snpdev ) + path_prefix_len +
-                         sizeof ( *macpath ) );
+       snpdev = zalloc ( sizeof ( *snpdev ) );
        if ( ! snpdev ) {
                rc = -ENOMEM;
                goto err_alloc_snp;
@@ -993,9 +991,32 @@ static int efi_snp_probe ( struct net_device *netdev ) {
                                       sizeof ( snpdev->name[0] ) ),
                       "%s", netdev->name );
 
+       /* Get the parent device path */
+       if ( ( efirc = bs->OpenProtocol ( efidev->device,
+                                         &efi_device_path_protocol_guid,
+                                         &path.interface, efi_image_handle,
+                                         efidev->device,
+                                         EFI_OPEN_PROTOCOL_GET_PROTOCOL ))!=0){
+               rc = -EEFI ( efirc );
+               DBGC ( snpdev, "SNPDEV %p cannot get %p %s device path: %s\n",
+                      snpdev, efidev->device,
+                      efi_handle_name ( efidev->device ), strerror ( rc ) );
+               goto err_open_device_path;
+       }
+
+       /* Allocate the new device path */
+       path_end = efi_devpath_end ( path.path );
+       path_prefix_len = ( ( ( void * ) path_end ) - ( ( void * ) path.path ));
+       snpdev->path = zalloc ( path_prefix_len + sizeof ( *macpath ) +
+                               sizeof ( *path_end ) );
+       if ( ! snpdev->path ) {
+               rc = -ENOMEM;
+               goto err_alloc_device_path;
+       }
+
        /* Populate the device path */
-       memcpy ( &snpdev->path, efidev->path, path_prefix_len );
-       macpath = ( ( ( void * ) &snpdev->path ) + path_prefix_len );
+       memcpy ( snpdev->path, path.path, path_prefix_len );
+       macpath = ( ( ( void * ) snpdev->path ) + path_prefix_len );
        path_end = ( ( void * ) ( macpath + 1 ) );
        memset ( macpath, 0, sizeof ( *macpath ) );
        macpath->Header.Type = MESSAGING_DEVICE_PATH;
@@ -1013,7 +1034,7 @@ static int efi_snp_probe ( struct net_device *netdev ) {
        if ( ( efirc = bs->InstallMultipleProtocolInterfaces (
                        &snpdev->handle,
                        &efi_simple_network_protocol_guid, &snpdev->snp,
-                       &efi_device_path_protocol_guid, &snpdev->path,
+                       &efi_device_path_protocol_guid, snpdev->path,
                        &efi_nii_protocol_guid, &snpdev->nii,
                        &efi_nii31_protocol_guid, &snpdev->nii,
                        &efi_component_name2_protocol_guid, &snpdev->name2,
@@ -1046,6 +1067,10 @@ static int efi_snp_probe ( struct net_device *netdev ) {
        /* Add to list of SNP devices */
        list_add ( &snpdev->list, &efi_snp_devices );
 
+       /* Close device path */
+       bs->CloseProtocol ( efidev->device, &efi_device_path_protocol_guid,
+                           efi_image_handle, efidev->device );
+
        DBGC ( snpdev, "SNPDEV %p installed for %s as device %p %s\n",
               snpdev, netdev->name, snpdev->handle,
               efi_handle_name ( snpdev->handle ) );
@@ -1058,13 +1083,18 @@ static int efi_snp_probe ( struct net_device *netdev ) {
        bs->UninstallMultipleProtocolInterfaces (
                        snpdev->handle,
                        &efi_simple_network_protocol_guid, &snpdev->snp,
-                       &efi_device_path_protocol_guid, &snpdev->path,
+                       &efi_device_path_protocol_guid, snpdev->path,
                        &efi_nii_protocol_guid, &snpdev->nii,
                        &efi_nii31_protocol_guid, &snpdev->nii,
                        &efi_component_name2_protocol_guid, &snpdev->name2,
                        &efi_load_file_protocol_guid, &snpdev->load_file,
                        NULL );
  err_install_protocol_interface:
+       free ( snpdev->path );
+ err_alloc_device_path:
+       bs->CloseProtocol ( efidev->device, &efi_device_path_protocol_guid,
+                           efi_image_handle, efidev->device );
+ err_open_device_path:
        bs->CloseEvent ( snpdev->snp.WaitForPacket );
  err_create_event:
  err_ll_addr_len:
@@ -1124,12 +1154,13 @@ static void efi_snp_remove ( struct net_device *netdev ) {
        bs->UninstallMultipleProtocolInterfaces (
                        snpdev->handle,
                        &efi_simple_network_protocol_guid, &snpdev->snp,
-                       &efi_device_path_protocol_guid, &snpdev->path,
+                       &efi_device_path_protocol_guid, snpdev->path,
                        &efi_nii_protocol_guid, &snpdev->nii,
                        &efi_nii31_protocol_guid, &snpdev->nii,
                        &efi_component_name2_protocol_guid, &snpdev->name2,
                        &efi_load_file_protocol_guid, &snpdev->load_file,
                        NULL );
+       free ( snpdev->path );
        bs->CloseEvent ( snpdev->snp.WaitForPacket );
        netdev_put ( snpdev->netdev );
        free ( snpdev );