loader: Remove trampoline/terminator dependency in vkEnumeratePhysicalDevices

There was a dependency between the trampoline vkEnumeratePhysicalDevices
and the terminator vkEnumeratePhysicalDevices via the
loader_instance.phys_devs_term array which may break layers that
manipulate the enumerated VkPhysicalDevice list. This dependency assumed
the devices in loader_instance.phys_devs_term and
loader_instance.phys_devs were in the same order and that it could
assume the index of one corresponding to the same VkPhysicalDevice of
the other.

Breaking this dependency allows layers to modify or reorder the
VkPhysicalDevice list by intercepting the vkEnumeratePhysicalDevices
function without causing the loader to crash. In general, there should
never be a dependency between the trampoline code and the terminator
code because it has the potential to break unknown layers between them.

Conflicts:
	loader/loader.c
	loader/trampoline.c

Change-Id: Iafefd6e8b7dd58d398a76533f957123242c01b56

3 files changed, 46 insertions, 53 deletions

diff --git a/loader/loader.c b/loader/loader.c
index 7786f147..eecbc756 100644
--- a/loader/loader.c
+++ b/loader/loader.c
@@ -657,7 +657,7 @@ loader_init_device_extensions(const struct loader_instance *inst,
 }
 
 VkResult loader_add_device_extensions(const struct loader_instance *inst,
-                                      struct loader_icd *icd,
+                                      PFN_vkEnumerateDeviceExtensionProperties fpEnumerateDeviceExtensionProperties,
                                       VkPhysicalDevice physical_device,
                                       const char *lib_name,
                                       struct loader_extension_list *ext_list) {
@@ -665,14 +665,14 @@ VkResult loader_add_device_extensions(const struct loader_instance *inst,
     VkResult res;
     VkExtensionProperties *ext_props;
 
-    res = icd->EnumerateDeviceExtensionProperties(physical_device, NULL, &count,
-                                                  NULL);
+    res = fpEnumerateDeviceExtensionProperties(physical_device, NULL, &count,
+                                               NULL);
     if (res == VK_SUCCESS && count > 0) {
         ext_props = loader_stack_alloc(count * sizeof(VkExtensionProperties));
         if (!ext_props)
             return VK_ERROR_OUT_OF_HOST_MEMORY;
-        res = icd->EnumerateDeviceExtensionProperties(physical_device, NULL,
-                                                      &count, ext_props);
+        res = fpEnumerateDeviceExtensionProperties(physical_device, NULL,
+                                                   &count, ext_props);
         if (res != VK_SUCCESS)
             return res;
         for (i = 0; i < count; i++) {
@@ -1154,8 +1154,7 @@ static void loader_destroy_logical_device(const struct loader_instance *inst,
 }
 
 struct loader_device *
-loader_add_logical_device(const struct loader_instance *inst,
-                          struct loader_device **device_list) {
+loader_create_logical_device(const struct loader_instance *inst) {
     struct loader_device *new_dev;
 
     new_dev = loader_heap_alloc(inst, sizeof(struct loader_device),
@@ -1168,11 +1167,16 @@ loader_add_logical_device(const struct loader_instance *inst,
 
     memset(new_dev, 0, sizeof(struct loader_device));
 
-    new_dev->next = *device_list;
-    *device_list = new_dev;
     return new_dev;
 }
 
+void loader_add_logical_device(const struct loader_instance *inst,
+                               struct loader_icd *icd,
+                               struct loader_device *dev) {
+    dev->next = icd->logical_device_list;
+    icd->logical_device_list = dev;
+}
+
 void loader_remove_logical_device(const struct loader_instance *inst,
                                   struct loader_icd *icd,
                                   struct loader_device *found_dev) {
@@ -2654,6 +2658,13 @@ loader_gpa_instance_internal(VkInstance inst, const char *pName) {
     return NULL;
 }
 
+static VKAPI_ATTR PFN_vkVoidFunction VKAPI_CALL
+loader_gpa_device_internal(VkDevice device, const char* pName) {
+    struct loader_device * dev;
+    struct loader_icd * icd = loader_get_icd_and_device(device, &dev);
+    return icd->GetDeviceProcAddr(device, pName);
+}
+
 /**
  * Initialize device_ext dispatch table entry as follows:
  * If dev == NULL find all logical devices created within this instance and
@@ -3325,7 +3336,6 @@ void loader_activate_instance_layer_extensions(struct loader_instance *inst,
 
 VkResult
 loader_enable_device_layers(const struct loader_instance *inst,
-                            struct loader_icd *icd,
                             struct loader_layer_list *activated_layer_list,
                             const VkDeviceCreateInfo *pCreateInfo,
                             const struct loader_layer_list *device_layers)
@@ -3367,7 +3377,6 @@ VkResult loader_create_device_chain(const struct loader_physical_device_tramp *p
                                     const VkDeviceCreateInfo *pCreateInfo,
                                     const VkAllocationCallbacks *pAllocator,
                                     const struct loader_instance *inst,
-                                    struct loader_icd *icd,
                                     struct loader_device *dev) {
     uint32_t activated_layers = 0;
     VkLayerDeviceLink *layer_device_link_info;
@@ -3375,7 +3384,7 @@ VkResult loader_create_device_chain(const struct loader_physical_device_tramp *p
     VkDeviceCreateInfo loader_create_info;
     VkResult res;
 
-    PFN_vkGetDeviceProcAddr fpGDPA, nextGDPA = icd->GetDeviceProcAddr;
+    PFN_vkGetDeviceProcAddr fpGDPA, nextGDPA = loader_gpa_device_internal;
     PFN_vkGetInstanceProcAddr fpGIPA, nextGIPA = loader_gpa_instance_internal;
 
     memcpy(&loader_create_info, pCreateInfo, sizeof(VkDeviceCreateInfo));
@@ -3391,7 +3400,6 @@ VkResult loader_create_device_chain(const struct loader_physical_device_tramp *p
     }
 
 
-
     if (dev->activated_layer_list.count > 0) {
         chain_info.sType = VK_STRUCTURE_TYPE_LOADER_DEVICE_CREATE_INFO;
         chain_info.function = VK_LAYER_LINK_INFO;
@@ -3589,7 +3597,7 @@ VkResult loader_validate_device_extensions(
         VkStringErrorFlags result = vk_string_validate(
             MaxLoaderStringLength, pCreateInfo->ppEnabledExtensionNames[i]);
         if (result != VK_STRING_ERROR_NONE) {
-            loader_log(phys_dev->this_icd->this_instance,
+            loader_log(phys_dev->this_instance,
                        VK_DEBUG_REPORT_ERROR_BIT_EXT, 0,
                        "Loader: Device ppEnabledExtensionNames contains "
                        "string that is too long or is badly formed");
@@ -3824,7 +3832,7 @@ terminator_CreateDevice(VkPhysicalDevice physicalDevice,
     }
 
     res = loader_add_device_extensions(
-        phys_dev->this_icd->this_instance, phys_dev->this_icd,
+        phys_dev->this_icd->this_instance, phys_dev->this_icd->EnumerateDeviceExtensionProperties,
         phys_dev->phys_dev, phys_dev->this_icd->this_icd_lib->lib_name,
         &icd_exts);
     if (res != VK_SUCCESS) {
@@ -3855,6 +3863,7 @@ terminator_CreateDevice(VkPhysicalDevice physicalDevice,
     }
 
     *pDevice = localDevice;
+    loader_add_logical_device(phys_dev->this_icd->this_instance, phys_dev->this_icd, dev);
 
     /* Init dispatch pointer in new device object */
     loader_init_dispatch(*pDevice, &dev->loader_dispatch);
@@ -3923,7 +3932,6 @@ terminator_EnumeratePhysicalDevices(VkInstance instance,
                      ? inst->total_gpu_count
                      : *pPhysicalDeviceCount;
 
-    // phys_devs_term is used to pass the "this_icd" info to trampoline code
     if (inst->phys_devs_term)
         loader_heap_free(inst, inst->phys_devs_term);
     inst->phys_devs_term = loader_heap_alloc(
@@ -3945,6 +3953,8 @@ terminator_EnumeratePhysicalDevices(VkInstance instance,
     }
     *pPhysicalDeviceCount = copy_count;
 
+    // TODO: Is phys_devs being leaked?
+
     if (copy_count < inst->total_gpu_count) {
         inst->total_gpu_count = copy_count;
         return VK_INCOMPLETE;
diff --git a/loader/loader.h b/loader/loader.h
index 2c3240eb..244066c0 100644
--- a/loader/loader.h
+++ b/loader/loader.h
@@ -323,9 +323,7 @@ struct loader_instance {
  * code must be able to get the struct loader_icd  to call into the proper
  * driver  (multiple ICD/gpu case). This can be accomplished by wrapping the
  * created VkPhysicalDevice in loader terminate_EnumeratePhysicalDevices().
- * Secondly, loader must be able to find the instance and icd in trampoline
- * code.
- * Thirdly, the loader must be able to handle wrapped by layer VkPhysicalDevice
+ * Secondly, the loader must be able to handle wrapped by layer VkPhysicalDevice
  * in trampoline code.  This implies, that the loader trampoline code must also
  * wrap the VkPhysicalDevice object in trampoline code.  Thus, loader has to
  * wrap the VkPhysicalDevice created object twice. In trampoline code it can't
@@ -337,9 +335,8 @@ struct loader_instance {
    also same structure used to wrap in terminator code */
 struct loader_physical_device_tramp {
     VkLayerInstanceDispatchTable *disp; // must be first entry in structure
-    struct loader_icd *this_icd;
+    struct loader_instance *this_instance;
     VkPhysicalDevice phys_dev;      // object from layers/loader terminator
-    VkPhysicalDevice icd_phys_dev;  // object from icd
 };
 
 /* per enumerated PhysicalDevice structure, used to wrap in terminator code */
@@ -462,7 +459,7 @@ VkResult loader_add_to_ext_list(const struct loader_instance *inst,
                                 uint32_t prop_list_count,
                                 const VkExtensionProperties *props);
 VkResult loader_add_device_extensions(const struct loader_instance *inst,
-                                      struct loader_icd *icd,
+                                      PFN_vkEnumerateDeviceExtensionProperties fpEnumerateDeviceExtensionProperties,
                                       VkPhysicalDevice physical_device,
                                       const char *lib_name,
                                       struct loader_extension_list *ext_list);
@@ -510,8 +507,10 @@ void *loader_dev_ext_gpa(struct loader_instance *inst, const char *funcName);
 void *loader_get_dev_ext_trampoline(uint32_t index);
 struct loader_instance *loader_get_instance(const VkInstance instance);
 struct loader_device *
-loader_add_logical_device(const struct loader_instance *inst,
-                          struct loader_device **device_list);
+loader_create_logical_device(const struct loader_instance *inst);
+void loader_add_logical_device(const struct loader_instance *inst,
+                               struct loader_icd *icd,
+                               struct loader_device *found_dev);
 void loader_remove_logical_device(const struct loader_instance *inst,
                                   struct loader_icd *icd,
                                   struct loader_device *found_dev);
@@ -530,7 +529,6 @@ void loader_activate_instance_layer_extensions(struct loader_instance *inst,
                                                VkInstance created_inst);
 VkResult
 loader_enable_device_layers(const struct loader_instance *inst,
-                            struct loader_icd *icd,
                             struct loader_layer_list *activated_layer_list,
                             const VkDeviceCreateInfo *pCreateInfo,
                             const struct loader_layer_list *device_layers);
@@ -539,7 +537,6 @@ VkResult loader_create_device_chain(const struct loader_physical_device_tramp *p
                                     const VkDeviceCreateInfo *pCreateInfo,
                                     const VkAllocationCallbacks *pAllocator,
                                     const struct loader_instance *inst,
-                                    struct loader_icd *icd,
                                     struct loader_device *dev);
 VkResult loader_validate_device_extensions(
     struct loader_physical_device_tramp *phys_dev,
diff --git a/loader/trampoline.c b/loader/trampoline.c
index 0622dcea..9c9a879c 100644
--- a/loader/trampoline.c
+++ b/loader/trampoline.c
@@ -511,8 +511,9 @@ vkEnumeratePhysicalDevices(VkInstance instance, uint32_t *pPhysicalDeviceCount,
     }
     if (inst->phys_devs)
         loader_heap_free(inst, inst->phys_devs);
-    count = min(inst->total_gpu_count, *pPhysicalDeviceCount);
-    *pPhysicalDevices = count;
+    count = (inst->total_gpu_count < *pPhysicalDeviceCount) ?
+	    inst->total_gpu_count : *pPhysicalDeviceCount;
+    *pPhysicalDeviceCount = count;
     inst->phys_devs = (struct loader_physical_device_tramp *)loader_heap_alloc(
         inst, count * sizeof(struct loader_physical_device_tramp),
         VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
@@ -525,9 +526,8 @@ vkEnumeratePhysicalDevices(VkInstance instance, uint32_t *pPhysicalDeviceCount,
 
         // initialize the loader's physicalDevice object
         loader_set_dispatch((void *)&inst->phys_devs[i], inst->disp);
-        inst->phys_devs[i].this_icd = inst->phys_devs_term[i].this_icd;
+        inst->phys_devs[i].this_instance = inst;
         inst->phys_devs[i].phys_dev = pPhysicalDevices[i];
-        inst->phys_devs[i].icd_phys_dev = inst->phys_devs_term[i].phys_dev;
 
         // copy wrapped object into Application provided array
         pPhysicalDevices[i] = (VkPhysicalDevice)&inst->phys_devs[i];
@@ -610,7 +610,6 @@ vkCreateDevice(VkPhysicalDevice physicalDevice,
                const VkAllocationCallbacks *pAllocator, VkDevice *pDevice) {
     VkResult res;
     struct loader_physical_device_tramp *phys_dev;
-    struct loader_icd *icd;
     struct loader_device *dev;
     struct loader_instance *inst;
     struct loader_layer_list activated_layer_list = {0};
@@ -620,18 +619,7 @@ vkCreateDevice(VkPhysicalDevice physicalDevice,
     loader_platform_thread_lock_mutex(&loader_lock);
 
     phys_dev = (struct loader_physical_device_tramp *)physicalDevice;
-    icd = phys_dev->this_icd;
-    if (!icd) {
-        loader_platform_thread_unlock_mutex(&loader_lock);
-        return VK_ERROR_INITIALIZATION_FAILED;
-    }
-
-    inst = (struct loader_instance *)phys_dev->this_icd->this_instance;
-
-    if (!icd->CreateDevice) {
-        loader_platform_thread_unlock_mutex(&loader_lock);
-        return VK_ERROR_INITIALIZATION_FAILED;
-    }
+    inst = (struct loader_instance *)phys_dev->this_instance;
 
     /* validate any app enabled layers are available */
     if (pCreateInfo->enabledLayerCount > 0) {
@@ -653,8 +641,8 @@ vkCreateDevice(VkPhysicalDevice physicalDevice,
     }
 
     res = loader_add_device_extensions(
-        inst, icd, phys_dev->icd_phys_dev,
-        phys_dev->this_icd->this_icd_lib->lib_name, &icd_exts);
+        inst, inst->disp->EnumerateDeviceExtensionProperties, phys_dev->phys_dev,
+        "Unknown", &icd_exts);
     if (res != VK_SUCCESS) {
         loader_platform_thread_unlock_mutex(&loader_lock);
         return res;
@@ -678,7 +666,7 @@ vkCreateDevice(VkPhysicalDevice physicalDevice,
         (char ***)&pCreateInfo->ppEnabledLayerNames);
 
     /* fetch a list of all layers activated, explicit and implicit */
-    res = loader_enable_device_layers(inst, icd, &activated_layer_list,
+    res = loader_enable_device_layers(inst, &activated_layer_list,
                                       pCreateInfo, &inst->device_layer_list);
     if (res != VK_SUCCESS) {
         loader_unexpand_dev_layer_names(inst, saved_layer_count,
@@ -701,7 +689,7 @@ vkCreateDevice(VkPhysicalDevice physicalDevice,
         return res;
     }
 
-    dev = loader_add_logical_device(inst, &icd->logical_device_list);
+    dev = loader_create_logical_device(inst);
     if (dev == NULL) {
         loader_unexpand_dev_layer_names(inst, saved_layer_count,
                                         saved_layer_names, saved_layer_ptr,
@@ -720,24 +708,22 @@ vkCreateDevice(VkPhysicalDevice physicalDevice,
     memset(&activated_layer_list, 0, sizeof(activated_layer_list));
 
     /* activate any layers on device chain which terminates with device*/
-    res = loader_enable_device_layers(inst, icd, &dev->activated_layer_list,
+    res = loader_enable_device_layers(inst, &dev->activated_layer_list,
                                       pCreateInfo, &inst->device_layer_list);
     if (res != VK_SUCCESS) {
         loader_unexpand_dev_layer_names(inst, saved_layer_count,
                                         saved_layer_names, saved_layer_ptr,
                                         pCreateInfo);
-        loader_remove_logical_device(inst, icd, dev);
         loader_platform_thread_unlock_mutex(&loader_lock);
         return res;
     }
 
     res = loader_create_device_chain(phys_dev, pCreateInfo, pAllocator, inst,
-                                     icd, dev);
+                                     dev);
     if (res != VK_SUCCESS) {
         loader_unexpand_dev_layer_names(inst, saved_layer_count,
                                         saved_layer_names, saved_layer_ptr,
                                         pCreateInfo);
-        loader_remove_logical_device(inst, icd, dev);
         loader_platform_thread_unlock_mutex(&loader_lock);
         return res;
     }
@@ -804,7 +790,7 @@ vkEnumerateDeviceExtensionProperties(VkPhysicalDevice physicalDevice,
 
         uint32_t count;
         uint32_t copy_size;
-        const struct loader_instance *inst = phys_dev->this_icd->this_instance;
+        const struct loader_instance *inst = phys_dev->this_instance;
         if (vk_string_validate(MaxLoaderStringLength, pLayerName) ==
             VK_STRING_ERROR_NONE) {
 
@@ -860,7 +846,7 @@ vkEnumerateDeviceLayerProperties(VkPhysicalDevice physicalDevice,
        enumerated and instance chain may not contain all device layers */
 
     phys_dev = (struct loader_physical_device_tramp *)physicalDevice;
-    const struct loader_instance *inst = phys_dev->this_icd->this_instance;
+    const struct loader_instance *inst = phys_dev->this_instance;
     uint32_t count = inst->device_layer_list.count;
 
     if (pProperties == NULL) {