From 87edb95cbba91c28fb9c0bc4977b50c5e1c04940 Mon Sep 17 00:00:00 2001 From: Adam Bodnar Date: Wed, 17 Jul 2019 12:35:53 -0700 Subject: Check if Vulkan layers have a valid looking GIPA Bug: 137862180 Test: flash, remove vkGetInstanceProcAddr from layer symbols, run Change-Id: I4d4d75585623d2c15c57b4d8ad0243a2fc347fce --- vulkan/libvulkan/api.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 71048db920..368130d13b 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -664,6 +664,12 @@ VkResult LayerChain::LoadLayer(ActiveLayer& layer, const char* name) { return VK_ERROR_LAYER_NOT_PRESENT; } + if (!layer.ref.GetGetInstanceProcAddr()) { + ALOGW("Failed to locate vkGetInstanceProcAddr in layer %s", name); + layer.ref.~LayerRef(); + return VK_ERROR_LAYER_NOT_PRESENT; + } + ALOGI("Loaded layer %s", name); return VK_SUCCESS; -- cgit v1.2.3-59-g8ed1b From 9dfc93ace276102af4973b62823454fcc5a653cd Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 9 Aug 2019 17:25:24 -0700 Subject: libvulkan: refactor the layer discovery logic Previously, the layer discovery logic is only executed once along with the driver loading. However, once Vulkan driver is preloaded in Zygote, new layers pushed to the system or shipped with the App's apk won't be discovered at runtime. This change refactors the logic of layer discovery. It doesn't hurt to keep finding new layers available in the layer search path, because the app apk itself won't change at runtime. Even if we push new layers to the system search path, that's only on the debug build of the system. Bug: 135536511 Test: preload Vulkan and atest CtsGpuToolsHostTestCases Change-Id: I915b78dacfd9b637a202f76969d559a31eded686 --- vulkan/libvulkan/api.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 368130d13b..4608be2907 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -1172,11 +1172,16 @@ bool EnsureInitialized() { std::call_once(once_flag, []() { if (driver::OpenHAL()) { - DiscoverLayers(); initialized = true; } }); + { + static std::mutex layer_lock; + std::lock_guard lock(layer_lock); + DiscoverLayers(); + } + return initialized; } -- cgit v1.2.3-59-g8ed1b From 519b44c8398e1b237f72dbd8106cd0ea0a00d936 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Wed, 14 Aug 2019 23:15:02 -0700 Subject: libvulkan: ensure layer discovery is triggered only once for a new process After decoupling layer discovery from driver loading, the layer discovery is triggered at each call of vkCreateInstance, vkEnumerateInstanceLayerProperties and vkEnumerateInstanceExtensionProperties. However, it takes non-trivial time to traverse the layer search path for priviledged apps and non-updated system apps. So this change just makes sure the layer discovery logic is triggered only once for a new process. Bug: 139443653 Bug: 135536511 Test: preload Vulkan and atest CtsGpuToolsHostTestCases Change-Id: Ibe502fd4b089acbbff6f4a2485fa61c736a484b5 --- vulkan/libvulkan/api.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 4608be2907..48f26e7e43 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -1177,9 +1177,13 @@ bool EnsureInitialized() { }); { + static pid_t pid = getpid() + 1; static std::mutex layer_lock; std::lock_guard lock(layer_lock); - DiscoverLayers(); + if (pid != getpid()) { + pid = getpid(); + DiscoverLayers(); + } } return initialized; -- cgit v1.2.3-59-g8ed1b From 7cc36a50e15cc781d9b1260d2cf418780173b2ee Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 11 Oct 2019 19:02:09 -0700 Subject: Vulkan: correctly expose Vulkan entry points This change fixes the advertisement of core Vulkan entry points as below: 1. GIPA returns a valid checked_proc for 1.1 core device APIs. 2. GDPA returns NULL for 1.1 core device APIs on a 1.0 physical device. Bug: 134185757 Bug: 142266108 Test: dEQP-VK.memory.binding on 1.1 loader and 1.0 device ICD Test: dEQP-VK.api.info.instance on 1.1 loader and 1.0 instance ICD Change-Id: I0a3e06dc04bade4f36a7e68ee2f53979c656ee4e --- vulkan/libvulkan/api.cpp | 6 +++- vulkan/libvulkan/driver.cpp | 60 +++++++++++++++++++++++++++-------- vulkan/libvulkan/driver.h | 8 ++--- vulkan/libvulkan/driver_gen.cpp | 51 ++++++++++++++++++++---------- vulkan/libvulkan/driver_gen.h | 3 +- vulkan/scripts/driver_generator.py | 65 ++++++++++++++++++++++++++++---------- vulkan/scripts/generator_common.py | 19 +++++++++++ 7 files changed, 158 insertions(+), 54 deletions(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 48f26e7e43..1578d9ffce 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -519,7 +519,11 @@ LayerChain::LayerChain(bool is_instance, get_device_proc_addr_(nullptr), driver_extensions_(nullptr), driver_extension_count_(0) { - enabled_extensions_.set(driver::ProcHook::EXTENSION_CORE); + // advertise the loader supported core Vulkan API version at vulkan::api + for (uint32_t i = driver::ProcHook::EXTENSION_CORE_1_0; + i != driver::ProcHook::EXTENSION_COUNT; ++i) { + enabled_extensions_.set(i); + } } LayerChain::~LayerChain() { diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index b413ac9375..c3c19ecdff 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -99,6 +99,7 @@ class CreateInfoWrapper { VkResult Validate(); void DowngradeApiVersion(); + void UpgradeDeviceCoreApiVersion(uint32_t api_version); const std::bitset& GetHookExtensions() const; const std::bitset& GetHalExtensions() const; @@ -328,8 +329,12 @@ CreateInfoWrapper::CreateInfoWrapper(const VkInstanceCreateInfo& create_info, physical_dev_(VK_NULL_HANDLE), instance_info_(create_info), extension_filter_() { - hook_extensions_.set(ProcHook::EXTENSION_CORE); - hal_extensions_.set(ProcHook::EXTENSION_CORE); + // instance core versions need to match the loader api version + for (uint32_t i = ProcHook::EXTENSION_CORE_1_0; + i != ProcHook::EXTENSION_COUNT; ++i) { + hook_extensions_.set(i); + hal_extensions_.set(i); + } } CreateInfoWrapper::CreateInfoWrapper(VkPhysicalDevice physical_dev, @@ -340,8 +345,9 @@ CreateInfoWrapper::CreateInfoWrapper(VkPhysicalDevice physical_dev, physical_dev_(physical_dev), dev_info_(create_info), extension_filter_() { - hook_extensions_.set(ProcHook::EXTENSION_CORE); - hal_extensions_.set(ProcHook::EXTENSION_CORE); + // initialize with baseline core API version + hook_extensions_.set(ProcHook::EXTENSION_CORE_1_0); + hal_extensions_.set(ProcHook::EXTENSION_CORE_1_0); } CreateInfoWrapper::~CreateInfoWrapper() { @@ -540,7 +546,8 @@ void CreateInfoWrapper::FilterExtension(const char* name) { case ProcHook::ANDROID_external_memory_android_hardware_buffer: case ProcHook::ANDROID_native_buffer: case ProcHook::GOOGLE_display_timing: - case ProcHook::EXTENSION_CORE: + case ProcHook::EXTENSION_CORE_1_0: + case ProcHook::EXTENSION_CORE_1_1: case ProcHook::EXTENSION_COUNT: // Device and meta extensions. If we ever get here it's a bug in // our code. But enumerating them lets us avoid having a default @@ -588,7 +595,8 @@ void CreateInfoWrapper::FilterExtension(const char* name) { case ProcHook::EXT_debug_report: case ProcHook::EXT_swapchain_colorspace: case ProcHook::ANDROID_native_buffer: - case ProcHook::EXTENSION_CORE: + case ProcHook::EXTENSION_CORE_1_0: + case ProcHook::EXTENSION_CORE_1_1: case ProcHook::EXTENSION_COUNT: // Instance and meta extensions. If we ever get here it's a bug // in our code. But enumerating them lets us avoid having a @@ -636,6 +644,31 @@ void CreateInfoWrapper::DowngradeApiVersion() { } } +void CreateInfoWrapper::UpgradeDeviceCoreApiVersion(uint32_t api_version) { + ALOG_ASSERT(!is_instance_, "Device only API called by instance wrapper."); + +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wold-style-cast" + api_version ^= VK_VERSION_PATCH(api_version); +#pragma clang diagnostic pop + + // cap the API version to the loader supported highest version + if (api_version > VK_API_VERSION_1_1) + api_version = VK_API_VERSION_1_1; + + switch (api_version) { + case VK_API_VERSION_1_1: + hook_extensions_.set(ProcHook::EXTENSION_CORE_1_1); + hal_extensions_.set(ProcHook::EXTENSION_CORE_1_1); + [[clang::fallthrough]]; + case VK_API_VERSION_1_0: + break; + default: + ALOGD("Unknown upgrade API version[%u]", api_version); + break; + } +} + VKAPI_ATTR void* DefaultAllocate(void*, size_t size, size_t alignment, @@ -771,7 +804,7 @@ PFN_vkVoidFunction GetInstanceProcAddr(VkInstance instance, const char* pName) { : nullptr; break; case ProcHook::DEVICE: - proc = (hook->extension == ProcHook::EXTENSION_CORE) + proc = (hook->extension == ProcHook::EXTENSION_CORE_1_0) ? hook->proc : hook->checked_proc; break; @@ -1117,6 +1150,13 @@ VkResult CreateDevice(VkPhysicalDevice physicalDevice, if (!data) return VK_ERROR_OUT_OF_HOST_MEMORY; + VkPhysicalDeviceProperties properties; + ATRACE_BEGIN("driver.GetPhysicalDeviceProperties"); + instance_data.driver.GetPhysicalDeviceProperties(physicalDevice, + &properties); + ATRACE_END(); + + wrapper.UpgradeDeviceCoreApiVersion(properties.apiVersion); data->hook_extensions |= wrapper.GetHookExtensions(); // call into the driver @@ -1161,12 +1201,6 @@ VkResult CreateDevice(VkPhysicalDevice physicalDevice, return VK_ERROR_INCOMPATIBLE_DRIVER; } - VkPhysicalDeviceProperties properties; - ATRACE_BEGIN("driver.GetPhysicalDeviceProperties"); - instance_data.driver.GetPhysicalDeviceProperties(physicalDevice, - &properties); - ATRACE_END(); - if (properties.deviceType == VK_PHYSICAL_DEVICE_TYPE_CPU) { // Log that the app is hitting software Vulkan implementation android::GraphicsEnv::getInstance().setTargetStats( diff --git a/vulkan/libvulkan/driver.h b/vulkan/libvulkan/driver.h index f058c47d54..61e1818f28 100644 --- a/vulkan/libvulkan/driver.h +++ b/vulkan/libvulkan/driver.h @@ -67,9 +67,7 @@ struct InstanceData { : opaque_api_data(), allocator(alloc), driver(), - get_device_proc_addr(nullptr) { - hook_extensions.set(ProcHook::EXTENSION_CORE); - } + get_device_proc_addr(nullptr) {} api::InstanceData opaque_api_data; @@ -89,9 +87,7 @@ struct DeviceData { : opaque_api_data(), allocator(alloc), debug_report_callbacks(debug_report_callbacks_), - driver() { - hook_extensions.set(ProcHook::EXTENSION_CORE); - } + driver() {} api::DeviceData opaque_api_data; diff --git a/vulkan/libvulkan/driver_gen.cpp b/vulkan/libvulkan/driver_gen.cpp index f676573de9..52205e9e79 100644 --- a/vulkan/libvulkan/driver_gen.cpp +++ b/vulkan/libvulkan/driver_gen.cpp @@ -74,6 +74,15 @@ VKAPI_ATTR VkResult checkedQueuePresentKHR(VkQueue queue, const VkPresentInfoKHR } } +VKAPI_ATTR VkResult checkedBindImageMemory2(VkDevice device, uint32_t bindInfoCount, const VkBindImageMemoryInfo* pBindInfos) { + if (GetData(device).hook_extensions[ProcHook::EXTENSION_CORE_1_1]) { + return BindImageMemory2(device, bindInfoCount, pBindInfos); + } else { + Logger(device).Err(device, "VK_VERSION_1_1 not enabled. vkBindImageMemory2 not executed."); + return VK_SUCCESS; + } +} + VKAPI_ATTR VkResult checkedBindImageMemory2KHR(VkDevice device, uint32_t bindInfoCount, const VkBindImageMemoryInfo* pBindInfos) { if (GetData(device).hook_extensions[ProcHook::KHR_bind_memory2]) { return BindImageMemory2KHR(device, bindInfoCount, pBindInfos); @@ -145,6 +154,14 @@ VKAPI_ATTR VkResult checkedGetPastPresentationTimingGOOGLE(VkDevice device, VkSw } } +VKAPI_ATTR void checkedGetDeviceQueue2(VkDevice device, const VkDeviceQueueInfo2* pQueueInfo, VkQueue* pQueue) { + if (GetData(device).hook_extensions[ProcHook::EXTENSION_CORE_1_1]) { + GetDeviceQueue2(device, pQueueInfo, pQueue); + } else { + Logger(device).Err(device, "VK_VERSION_1_1 not enabled. vkGetDeviceQueue2 not executed."); + } +} + // clang-format on const ProcHook g_proc_hooks[] = { @@ -173,16 +190,16 @@ const ProcHook g_proc_hooks[] = { { "vkAllocateCommandBuffers", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(AllocateCommandBuffers), nullptr, }, { "vkBindImageMemory2", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_1, reinterpret_cast(BindImageMemory2), - nullptr, + reinterpret_cast(checkedBindImageMemory2), }, { "vkBindImageMemory2KHR", @@ -208,14 +225,14 @@ const ProcHook g_proc_hooks[] = { { "vkCreateDevice", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(CreateDevice), nullptr, }, { "vkCreateInstance", ProcHook::GLOBAL, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(CreateInstance), nullptr, }, @@ -243,14 +260,14 @@ const ProcHook g_proc_hooks[] = { { "vkDestroyDevice", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(DestroyDevice), nullptr, }, { "vkDestroyInstance", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(DestroyInstance), nullptr, }, @@ -271,28 +288,28 @@ const ProcHook g_proc_hooks[] = { { "vkEnumerateDeviceExtensionProperties", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(EnumerateDeviceExtensionProperties), nullptr, }, { "vkEnumerateInstanceExtensionProperties", ProcHook::GLOBAL, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(EnumerateInstanceExtensionProperties), nullptr, }, { "vkEnumeratePhysicalDeviceGroups", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_1, reinterpret_cast(EnumeratePhysicalDeviceGroups), nullptr, }, { "vkEnumeratePhysicalDevices", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(EnumeratePhysicalDevices), nullptr, }, @@ -313,28 +330,28 @@ const ProcHook g_proc_hooks[] = { { "vkGetDeviceProcAddr", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(GetDeviceProcAddr), nullptr, }, { "vkGetDeviceQueue", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(GetDeviceQueue), nullptr, }, { "vkGetDeviceQueue2", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_1, reinterpret_cast(GetDeviceQueue2), - nullptr, + reinterpret_cast(checkedGetDeviceQueue2), }, { "vkGetInstanceProcAddr", ProcHook::INSTANCE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(GetInstanceProcAddr), nullptr, }, @@ -446,7 +463,7 @@ const ProcHook g_proc_hooks[] = { { "vkQueueSubmit", ProcHook::DEVICE, - ProcHook::EXTENSION_CORE, + ProcHook::EXTENSION_CORE_1_0, reinterpret_cast(QueueSubmit), nullptr, }, diff --git a/vulkan/libvulkan/driver_gen.h b/vulkan/libvulkan/driver_gen.h index cd7d8f82e0..43c4d1469e 100644 --- a/vulkan/libvulkan/driver_gen.h +++ b/vulkan/libvulkan/driver_gen.h @@ -49,7 +49,8 @@ struct ProcHook { KHR_bind_memory2, KHR_get_physical_device_properties2, - EXTENSION_CORE, // valid bit + EXTENSION_CORE_1_0, + EXTENSION_CORE_1_1, EXTENSION_COUNT, EXTENSION_UNKNOWN, }; diff --git a/vulkan/scripts/driver_generator.py b/vulkan/scripts/driver_generator.py index 0f3d760d5f..a64a7026db 100644 --- a/vulkan/scripts/driver_generator.py +++ b/vulkan/scripts/driver_generator.py @@ -177,8 +177,12 @@ struct ProcHook { for ext in _KNOWN_EXTENSIONS: f.write(gencom.indent(2) + gencom.base_ext_name(ext) + ',\n') - f.write(""" - EXTENSION_CORE, // valid bit + f.write('\n') + for version in gencom.version_code_list: + f.write(gencom.indent(2) + 'EXTENSION_CORE_' + version + ',\n') + + # EXTENSION_COUNT must be the next enum after the highest API version. + f.write("""\ EXTENSION_COUNT, EXTENSION_UNKNOWN, }; @@ -249,6 +253,18 @@ def _is_intercepted(cmd): return False +def _get_proc_hook_enum(cmd): + """Returns the ProcHook enumeration for the corresponding core function. + + Args: + cmd: Vulkan function name. + """ + assert cmd in gencom.version_dict + for version in gencom.version_code_list: + if gencom.version_dict[cmd] == 'VK_VERSION_' + version: + return 'ProcHook::EXTENSION_CORE_' + version + + def _need_proc_hook_stub(cmd): """Returns true if a function needs a ProcHook stub. @@ -259,6 +275,8 @@ def _need_proc_hook_stub(cmd): if cmd in gencom.extension_dict: if not gencom.is_extension_internal(gencom.extension_dict[cmd]): return True + elif gencom.version_dict[cmd] != 'VK_VERSION_1_0': + return True return False @@ -271,8 +289,16 @@ def _define_proc_hook_stub(cmd, f): """ if _need_proc_hook_stub(cmd): return_type = gencom.return_type_dict[cmd] - ext_name = gencom.extension_dict[cmd] - ext_hook = 'ProcHook::' + gencom.base_ext_name(ext_name) + + ext_name = '' + ext_hook = '' + if cmd in gencom.extension_dict: + ext_name = gencom.extension_dict[cmd] + ext_hook = 'ProcHook::' + gencom.base_ext_name(ext_name) + else: + ext_name = gencom.version_dict[cmd] + ext_hook = _get_proc_hook_enum(cmd) + handle = gencom.param_dict[cmd][0][1] param_types = ', '.join([''.join(i) for i in gencom.param_dict[cmd]]) param_names = ', '.join([''.join(i[1]) for i in gencom.param_dict[cmd]]) @@ -306,12 +332,12 @@ def _define_global_proc_hook(cmd, f): f.write(gencom.indent(1) + '{\n') f.write(gencom.indent(2) + '\"' + cmd + '\",\n') - f.write("""\ - ProcHook::GLOBAL, - ProcHook::EXTENSION_CORE, - reinterpret_cast(""" + gencom.base_name(cmd) + """), - nullptr, - },\n""") + f.write(gencom.indent(2) + 'ProcHook::GLOBAL,\n') + f.write(gencom.indent(2) + _get_proc_hook_enum(cmd) + ',\n') + f.write(gencom.indent(2) + 'reinterpret_cast(' + + gencom.base_name(cmd) + '),\n') + f.write(gencom.indent(2) + 'nullptr,\n') + f.write(gencom.indent(1) + '},\n') def _define_instance_proc_hook(cmd, f): @@ -339,8 +365,8 @@ def _define_instance_proc_hook(cmd, f): reinterpret_cast(""" + gencom.base_name(cmd) + """), nullptr,\n""") else: + f.write(gencom.indent(2) + _get_proc_hook_enum(cmd) + ',\n') f.write("""\ - ProcHook::EXTENSION_CORE, reinterpret_cast(""" + gencom.base_name(cmd) + """), nullptr,\n""") @@ -358,10 +384,17 @@ def _define_device_proc_hook(cmd, f): f.write(gencom.indent(2) + '\"' + cmd + '\",\n') f.write(gencom.indent(2) + 'ProcHook::DEVICE,\n') - if cmd in gencom.extension_dict: - ext_name = gencom.extension_dict[cmd] - f.write(gencom.indent(2) + 'ProcHook::' + - gencom.base_ext_name(ext_name) + ',\n') + if (cmd in gencom.extension_dict or + gencom.version_dict[cmd] != 'VK_VERSION_1_0'): + ext_name = '' + ext_hook = '' + if cmd in gencom.extension_dict: + ext_name = gencom.extension_dict[cmd] + ext_hook = 'ProcHook::' + gencom.base_ext_name(ext_name) + else: + ext_name = gencom.version_dict[cmd] + ext_hook = _get_proc_hook_enum(cmd) + f.write(gencom.indent(2) + ext_hook + ',\n') if gencom.is_extension_internal(ext_name): f.write("""\ @@ -374,8 +407,8 @@ def _define_device_proc_hook(cmd, f): gencom.base_name(cmd) + '),\n') else: + f.write(gencom.indent(2) + _get_proc_hook_enum(cmd) + ',\n') f.write("""\ - ProcHook::EXTENSION_CORE, reinterpret_cast(""" + gencom.base_name(cmd) + """), nullptr,\n""") diff --git a/vulkan/scripts/generator_common.py b/vulkan/scripts/generator_common.py index 670ba66d1b..cf370fafe5 100644 --- a/vulkan/scripts/generator_common.py +++ b/vulkan/scripts/generator_common.py @@ -91,6 +91,9 @@ param_dict = {} # Dict for mapping a function to its return type. return_type_dict = {} +# List of the sorted Vulkan version codes. e.g. '1_0', '1_1'. +version_code_list = [] + # Dict for mapping a function to the core Vulkan API version. version_dict = {} @@ -171,6 +174,15 @@ def base_ext_name(ext): return ext[3:] +def version_code(version): + """Returns the version code from a version string. + + Args: + version: Vulkan version string. + """ + return version[11:] + + def is_function_supported(cmd): """Returns true if a function is core or from a supportable extension. @@ -313,6 +325,7 @@ def parse_vulkan_registry(): extension_dict param_dict return_type_dict + version_code_list version_dict """ registry = os.path.join(os.path.dirname(__file__), '..', '..', '..', '..', @@ -385,3 +398,9 @@ def parse_vulkan_registry(): cmd_name = command.get('name') if cmd_name in command_list: version_dict[cmd_name] = apiversion + + version_code_set = set() + for version in version_dict.values(): + version_code_set.add(version_code(version)) + for code in sorted(version_code_set): + version_code_list.append(code) -- cgit v1.2.3-59-g8ed1b From a885c06c07e9a0838b185daed58bc7626c2b961e Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Thu, 24 Oct 2019 12:07:57 -0700 Subject: Vulkan: convert all the TODOs into actual issues to chase against Bug: 134185757 Test: build Change-Id: Ib9a27ba8b8da53707337d3d48e6da502f38670e5 --- vulkan/libvulkan/api.cpp | 4 ++-- vulkan/libvulkan/debug_report.h | 2 +- vulkan/libvulkan/driver.cpp | 2 +- vulkan/libvulkan/layers_extensions.cpp | 8 +------- vulkan/libvulkan/swapchain.cpp | 8 ++++---- 5 files changed, 9 insertions(+), 15 deletions(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 1578d9ffce..24039b1fee 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -1279,7 +1279,7 @@ VkResult EnumerateInstanceExtensionProperties( return *pPropertyCount < count ? VK_INCOMPLETE : VK_SUCCESS; } - // TODO how about extensions from implicitly enabled layers? + // TODO(b/143293104): expose extensions from implicitly enabled layers return vulkan::driver::EnumerateInstanceExtensionProperties( nullptr, pPropertyCount, pProperties); } @@ -1333,7 +1333,7 @@ VkResult EnumerateDeviceExtensionProperties( return *pPropertyCount < count ? VK_INCOMPLETE : VK_SUCCESS; } - // TODO how about extensions from implicitly enabled layers? + // TODO(b/143293104): expose extensions from implicitly enabled layers const InstanceData& data = GetData(physicalDevice); return data.dispatch.EnumerateDeviceExtensionProperties( physicalDevice, nullptr, pPropertyCount, pProperties); diff --git a/vulkan/libvulkan/debug_report.h b/vulkan/libvulkan/debug_report.h index 3d8bd50c28..e5b1587b4f 100644 --- a/vulkan/libvulkan/debug_report.h +++ b/vulkan/libvulkan/debug_report.h @@ -78,7 +78,7 @@ class DebugReportCallbackList { VkDebugReportCallbackEXT driver_handle; }; - // TODO(jessehall): replace with std::shared_mutex when available in libc++ + // TODO(b/143295577): use std::shared_mutex when available in libc++ mutable std::shared_timed_mutex rwmutex_; Node head_; }; diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 4aa7d55e12..733e823bed 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -692,7 +692,7 @@ VKAPI_ATTR void* DefaultReallocate(void*, return nullptr; } - // TODO(jessehall): Right now we never shrink allocations; if the new + // TODO(b/143295633): Right now we never shrink allocations; if the new // request is smaller than the existing chunk, we just continue using it. // Right now the loader never reallocs, so this doesn't matter. If that // changes, or if this code is copied into some other project, this should diff --git a/vulkan/libvulkan/layers_extensions.cpp b/vulkan/libvulkan/layers_extensions.cpp index 5679412732..2f33fee8ac 100644 --- a/vulkan/libvulkan/layers_extensions.cpp +++ b/vulkan/libvulkan/layers_extensions.cpp @@ -38,13 +38,7 @@ #include #include -// TODO(jessehall): The whole way we deal with extensions is pretty hokey, and -// not a good long-term solution. Having a hard-coded enum of extensions is -// bad, of course. Representing sets of extensions (requested, supported, etc.) -// as a bitset isn't necessarily bad, if the mapping from extension to bit were -// dynamic. Need to rethink this completely when there's a little more time. - -// TODO(jessehall): This file currently builds up global data structures as it +// TODO(b/143296676): This file currently builds up global data structures as it // loads, and never cleans them up. This means we're doing heap allocations // without going through an app-provided allocator, but worse, we'll leak those // allocations if the loader is unloaded. diff --git a/vulkan/libvulkan/swapchain.cpp b/vulkan/libvulkan/swapchain.cpp index bbf50a1529..d234e177d9 100644 --- a/vulkan/libvulkan/swapchain.cpp +++ b/vulkan/libvulkan/swapchain.cpp @@ -694,8 +694,8 @@ VkResult GetPhysicalDeviceSurfaceFormatsKHR(VkPhysicalDevice pdev, const InstanceData& instance_data = GetData(pdev); - // TODO(jessehall): Fill out the set of supported formats. Longer term, add - // a new gralloc method to query whether a (format, usage) pair is + // TODO(b/143296550): Fill out the set of supported formats. Longer term, + // add a new gralloc method to query whether a (format, usage) pair is // supported, and check that for each gralloc format that corresponds to a // Vulkan format. Shorter term, just add a few more formats to the ones // hardcoded below. @@ -953,7 +953,7 @@ VkResult GetPhysicalDevicePresentRectanglesKHR(VkPhysicalDevice, strerror(-err), err); } - // TODO: Return something better than "whole window" + // TODO(b/143294545): Return something better than "whole window" pRects[0].offset.x = 0; pRects[0].offset.y = 0; pRects[0].extent = VkExtent2D{static_cast(width), @@ -1812,7 +1812,7 @@ VkResult GetSwapchainStatusKHR( return VK_ERROR_OUT_OF_DATE_KHR; } - // TODO(chrisforbes): Implement this function properly + // TODO(b/143296009): Implement this function properly return result; } -- cgit v1.2.3-59-g8ed1b From 94b18d55fa60a9c3a5b135b6237436d44b9ebeb7 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Thu, 7 Nov 2019 17:12:11 -0800 Subject: Vulkan: fix a typo Fixes: 140609421 Test: build, flash and boot Change-Id: I7548bf733b32861c8007cee63df954022c111a1d --- vulkan/libvulkan/api.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 24039b1fee..47253440d4 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -144,7 +144,7 @@ class OverrideLayerNames { } void GetLayersFromSettings() { - // These will only be available if conditions are met in GraphicsEnvironemnt + // These will only be available if conditions are met in GraphicsEnvironment // gpu_debug_layers = layer1:layer2:layerN const std::string layers = android::GraphicsEnv::getInstance().getDebugLayers(); if (!layers.empty()) { -- cgit v1.2.3-59-g8ed1b From 6a674c9e105bdc5d736c06a4500dcdac1c6c4006 Mon Sep 17 00:00:00 2001 From: Yiwei Zhang Date: Fri, 8 Nov 2019 11:55:36 -0800 Subject: GraphicsEnv: refactor to unify the debuggable logic By default, PR_SET_DUMPABLE is 0 for zygote spawned apps, except in the following circumstances: 1. ro.debuggable=1 (global debuggable enabled, i.e., userdebug or eng builds). 2. android:debuggable="true" in the manifest for an individual application. 3. An app which explicitly calls prctl(PR_SET_DUMPABLE, 1). 4. GraphicsEnv calls prctl(PR_SET_DUMPABLE, 1) in the presence of in the application manifest. So checking both ro.debuggable=1 and PR_GET_DUMPABLE is redundant. Bug: 144186877 Test: CtsAngleIntegrationHostTestCases Test: CtsRootlessGpuDebugHostTest Change-Id: I934f64315b67db77ee2c2a9dff50fb23bc0a546a --- libs/graphicsenv/GraphicsEnv.cpp | 8 ++------ libs/graphicsenv/include/graphicsenv/GraphicsEnv.h | 12 ++++++++++-- opengl/libs/EGL/egl_layers.cpp | 2 +- vulkan/libvulkan/api.cpp | 6 ++++-- vulkan/libvulkan/driver.cpp | 4 ---- vulkan/libvulkan/driver.h | 1 - vulkan/libvulkan/layers_extensions.cpp | 3 +-- 7 files changed, 18 insertions(+), 18 deletions(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/libs/graphicsenv/GraphicsEnv.cpp b/libs/graphicsenv/GraphicsEnv.cpp index 28591110f0..354703b9e4 100644 --- a/libs/graphicsenv/GraphicsEnv.cpp +++ b/libs/graphicsenv/GraphicsEnv.cpp @@ -124,12 +124,8 @@ static const std::string getSystemNativeLibraries(NativeLibrary type) { return env; } -int GraphicsEnv::getCanLoadSystemLibraries() { - if (property_get_bool("ro.debuggable", false) && prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) { - // Return an integer value since this crosses library boundaries - return 1; - } - return 0; +bool GraphicsEnv::isDebuggable() { + return prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) > 0; } void GraphicsEnv::setDriverPathAndSphalLibraries(const std::string path, diff --git a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h index 83448d4ce7..c6dc1f831a 100644 --- a/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h +++ b/libs/graphicsenv/include/graphicsenv/GraphicsEnv.h @@ -33,8 +33,16 @@ class GraphicsEnv { public: static GraphicsEnv& getInstance(); - // Check if device is debuggable. - int getCanLoadSystemLibraries(); + // Check if the process is debuggable. It returns false except in any of the + // following circumstances: + // 1. ro.debuggable=1 (global debuggable enabled). + // 2. android:debuggable="true" in the manifest for an individual app. + // 3. An app which explicitly calls prctl(PR_SET_DUMPABLE, 1). + // 4. GraphicsEnv calls prctl(PR_SET_DUMPABLE, 1) in the presence of + // + // in the application manifest. + bool isDebuggable(); /* * Apis for updatable driver diff --git a/opengl/libs/EGL/egl_layers.cpp b/opengl/libs/EGL/egl_layers.cpp index ac01dc8f96..ba7cacdbf2 100644 --- a/opengl/libs/EGL/egl_layers.cpp +++ b/opengl/libs/EGL/egl_layers.cpp @@ -337,7 +337,7 @@ void LayerLoader::LoadLayers() { // Only enable the system search path for non-user builds std::string system_path; - if (property_get_bool("ro.debuggable", false) && prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) { + if (android::GraphicsEnv::getInstance().isDebuggable()) { system_path = kSystemLayerLibraryDir; } diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index 24039b1fee..a1207d324f 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -124,7 +124,8 @@ class OverrideLayerNames { }; void AddImplicitLayers() { - if (!is_instance_ || !driver::Debuggable()) + if (!is_instance_ || + !android::GraphicsEnv::getInstance().isDebuggable()) return; GetLayersFromSettings(); @@ -370,7 +371,8 @@ class OverrideExtensionNames { private: bool EnableDebugCallback() const { - return (is_instance_ && driver::Debuggable() && + return (is_instance_ && + android::GraphicsEnv::getInstance().isDebuggable() && property_get_bool("debug.vulkan.enable_callback", false)); } diff --git a/vulkan/libvulkan/driver.cpp b/vulkan/libvulkan/driver.cpp index 0b686f189a..c1994f9976 100644 --- a/vulkan/libvulkan/driver.cpp +++ b/vulkan/libvulkan/driver.cpp @@ -755,10 +755,6 @@ void FreeDeviceData(DeviceData* data, const VkAllocationCallbacks& allocator) { } // anonymous namespace -bool Debuggable() { - return prctl(PR_GET_DUMPABLE, 0, 0, 0, 0) > 0; -} - bool OpenHAL() { return Hal::Open(); } diff --git a/vulkan/libvulkan/driver.h b/vulkan/libvulkan/driver.h index 7edadea09d..23c717cf5d 100644 --- a/vulkan/libvulkan/driver.h +++ b/vulkan/libvulkan/driver.h @@ -100,7 +100,6 @@ struct DeviceData { DeviceDriverTable driver; }; -bool Debuggable(); bool OpenHAL(); const VkAllocationCallbacks& GetDefaultAllocator(); diff --git a/vulkan/libvulkan/layers_extensions.cpp b/vulkan/libvulkan/layers_extensions.cpp index 2f33fee8ac..3ebfd7b629 100644 --- a/vulkan/libvulkan/layers_extensions.cpp +++ b/vulkan/libvulkan/layers_extensions.cpp @@ -475,8 +475,7 @@ void* GetLayerGetProcAddr(const Layer& layer, void DiscoverLayers() { ATRACE_CALL(); - if (property_get_bool("ro.debuggable", false) && - prctl(PR_GET_DUMPABLE, 0, 0, 0, 0)) { + if (android::GraphicsEnv::getInstance().isDebuggable()) { DiscoverLayersInPathList(kSystemLayerLibraryDir); } if (!android::GraphicsEnv::getInstance().getLayerPaths().empty()) -- cgit v1.2.3-59-g8ed1b From 8f4435a3370ec3a7ad56ecb3721d29a0292c06a9 Mon Sep 17 00:00:00 2001 From: Peiyong Lin Date: Wed, 11 Mar 2020 17:43:28 -0700 Subject: [Vulkan] Expose instance extensions from implicit layers. Previously the vulkan loader only exposes instance extensions from the driver implementation. Per vkEnumerateInstanceExtensionProperties spec the loader must also advertise instance extensions from implicitly enabled layers. Bug: b/143293104 Test: vkEnumerateInstanceExtensionProperties returns the instance extensions from implicit layers. Change-Id: I17f4ce370bf5f4bba295165a28836e2b7c03a318 --- vulkan/libvulkan/api.cpp | 65 +++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 62 insertions(+), 3 deletions(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index aae72db0fe..e607b058eb 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -29,6 +29,8 @@ #include #include #include +#include +#include #include #include @@ -1280,9 +1282,66 @@ VkResult EnumerateInstanceExtensionProperties( return *pPropertyCount < count ? VK_INCOMPLETE : VK_SUCCESS; } - // TODO(b/143293104): expose extensions from implicitly enabled layers - return vulkan::driver::EnumerateInstanceExtensionProperties( - nullptr, pPropertyCount, pProperties); + // If the pLayerName is nullptr, we must advertise all instance extensions + // from all implicitly enabled layers and the driver implementation. If + // there are duplicates among layers and the driver implementation, always + // only preserve the top layer closest to the application regardless of the + // spec version. + std::vector properties; + std::unordered_set extensionNames; + + // Expose extensions from implicitly enabled layers. + const std::string layersSetting = + android::GraphicsEnv::getInstance().getDebugLayers(); + if (!layersSetting.empty()) { + std::vector layers = + android::base::Split(layersSetting, ":"); + for (uint32_t i = 0; i < layers.size(); i++) { + const Layer* layer = FindLayer(layers[i].c_str()); + if (!layer) { + continue; + } + uint32_t count = 0; + const VkExtensionProperties* props = + GetLayerInstanceExtensions(*layer, count); + if (count > 0) { + for (uint32_t i = 0; i < count; ++i) { + if (extensionNames.emplace(props[i].extensionName).second) { + properties.push_back(props[i]); + } + } + } + } + } + + // TODO(b/143293104): Parse debug.vulkan.layers properties + + // Expose extensions from driver implementation. + { + uint32_t count = 0; + VkResult result = vulkan::driver::EnumerateInstanceExtensionProperties( + nullptr, &count, nullptr); + if (result == VK_SUCCESS && count > 0) { + std::vector props(count); + result = vulkan::driver::EnumerateInstanceExtensionProperties( + nullptr, &count, props.data()); + for (auto prop : props) { + if (extensionNames.emplace(prop.extensionName).second) { + properties.push_back(prop); + } + } + } + } + + uint32_t totalCount = properties.size(); + if (!pProperties || *pPropertyCount > totalCount) { + *pPropertyCount = totalCount; + } + if (pProperties) { + std::copy(properties.data(), properties.data() + *pPropertyCount, + pProperties); + } + return *pPropertyCount < totalCount ? VK_INCOMPLETE : VK_SUCCESS; } VkResult EnumerateDeviceLayerProperties(VkPhysicalDevice physicalDevice, -- cgit v1.2.3-59-g8ed1b From 27800499ab20551a2b2e64d360c5e2130c881dd2 Mon Sep 17 00:00:00 2001 From: Peiyong Lin Date: Wed, 25 Mar 2020 15:44:28 -0700 Subject: [Vulkan] Expose device extensions from implicit layers. Previously the vulkan loader only exposes device extensions from the driver implementation when the layer name is not specified. Per vkEnumerateDeviceExtensionProperties spec the loader must also advertise device extensions from implicitly enabled layers. Bug: b/143293104 Test: atest android.gputools.cts.CtsRootlessGpuDebugHostTest Change-Id: Iaf5786ba7e371a290ecd1764af69b5298371cfdd --- vulkan/libvulkan/api.cpp | 100 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 23 deletions(-) (limited to 'vulkan/libvulkan/api.cpp') diff --git a/vulkan/libvulkan/api.cpp b/vulkan/libvulkan/api.cpp index e607b058eb..5b9affd03a 100644 --- a/vulkan/libvulkan/api.cpp +++ b/vulkan/libvulkan/api.cpp @@ -1196,6 +1196,23 @@ bool EnsureInitialized() { return initialized; } +template +void ForEachLayerFromSettings(Functor functor) { + const std::string layersSetting = + android::GraphicsEnv::getInstance().getDebugLayers(); + if (!layersSetting.empty()) { + std::vector layers = + android::base::Split(layersSetting, ":"); + for (uint32_t i = 0; i < layers.size(); i++) { + const Layer* layer = FindLayer(layers[i].c_str()); + if (!layer) { + continue; + } + functor(layer); + } + } +} + } // anonymous namespace VkResult CreateInstance(const VkInstanceCreateInfo* pCreateInfo, @@ -1291,28 +1308,18 @@ VkResult EnumerateInstanceExtensionProperties( std::unordered_set extensionNames; // Expose extensions from implicitly enabled layers. - const std::string layersSetting = - android::GraphicsEnv::getInstance().getDebugLayers(); - if (!layersSetting.empty()) { - std::vector layers = - android::base::Split(layersSetting, ":"); - for (uint32_t i = 0; i < layers.size(); i++) { - const Layer* layer = FindLayer(layers[i].c_str()); - if (!layer) { - continue; - } - uint32_t count = 0; - const VkExtensionProperties* props = - GetLayerInstanceExtensions(*layer, count); - if (count > 0) { - for (uint32_t i = 0; i < count; ++i) { - if (extensionNames.emplace(props[i].extensionName).second) { - properties.push_back(props[i]); - } + ForEachLayerFromSettings([&](const Layer* layer) { + uint32_t count = 0; + const VkExtensionProperties* props = + GetLayerInstanceExtensions(*layer, count); + if (count > 0) { + for (uint32_t i = 0; i < count; ++i) { + if (extensionNames.emplace(props[i].extensionName).second) { + properties.push_back(props[i]); } } } - } + }); // TODO(b/143293104): Parse debug.vulkan.layers properties @@ -1393,10 +1400,57 @@ VkResult EnumerateDeviceExtensionProperties( return *pPropertyCount < count ? VK_INCOMPLETE : VK_SUCCESS; } - // TODO(b/143293104): expose extensions from implicitly enabled layers - const InstanceData& data = GetData(physicalDevice); - return data.dispatch.EnumerateDeviceExtensionProperties( - physicalDevice, nullptr, pPropertyCount, pProperties); + // If the pLayerName is nullptr, we must advertise all device extensions + // from all implicitly enabled layers and the driver implementation. If + // there are duplicates among layers and the driver implementation, always + // only preserve the top layer closest to the application regardless of the + // spec version. + std::vector properties; + std::unordered_set extensionNames; + + // Expose extensions from implicitly enabled layers. + ForEachLayerFromSettings([&](const Layer* layer) { + uint32_t count = 0; + const VkExtensionProperties* props = + GetLayerDeviceExtensions(*layer, count); + if (count > 0) { + for (uint32_t i = 0; i < count; ++i) { + if (extensionNames.emplace(props[i].extensionName).second) { + properties.push_back(props[i]); + } + } + } + }); + + // TODO(b/143293104): Parse debug.vulkan.layers properties + + // Expose extensions from driver implementation. + { + const InstanceData& data = GetData(physicalDevice); + uint32_t count = 0; + VkResult result = data.dispatch.EnumerateDeviceExtensionProperties( + physicalDevice, nullptr, &count, nullptr); + if (result == VK_SUCCESS && count > 0) { + std::vector props(count); + result = data.dispatch.EnumerateDeviceExtensionProperties( + physicalDevice, nullptr, &count, props.data()); + for (auto prop : props) { + if (extensionNames.emplace(prop.extensionName).second) { + properties.push_back(prop); + } + } + } + } + + uint32_t totalCount = properties.size(); + if (!pProperties || *pPropertyCount > totalCount) { + *pPropertyCount = totalCount; + } + if (pProperties) { + std::copy(properties.data(), properties.data() + *pPropertyCount, + pProperties); + } + return *pPropertyCount < totalCount ? VK_INCOMPLETE : VK_SUCCESS; } VkResult EnumerateInstanceVersion(uint32_t* pApiVersion) { -- cgit v1.2.3-59-g8ed1b