Update loader documentation. #685
Conversation
|
CI Vulkan-Loader build queued with queue ID 36055. |
|
CI Vulkan-Loader build # 668 running. |
|
CI Vulkan-Loader build # 668 passed. |
|
CI Vulkan-Loader build queued with queue ID 38405. |
|
CI Vulkan-Loader build # 673 running. |
|
CI Vulkan-Loader build # 673 passed. |
|
CI Vulkan-Loader build queued with queue ID 41353. |
|
CI Vulkan-Loader build # 691 running. |
|
CI Vulkan-Loader build queued with queue ID 41359. |
|
CI Vulkan-Loader build # 692 running. |
|
CI Vulkan-Loader build # 692 passed. |
|
CI Vulkan-Loader build queued with queue ID 41404. |
|
CI Vulkan-Loader build # 693 running. |
|
CI Vulkan-Loader build # 693 passed. |
|
CI Vulkan-Loader build queued with queue ID 42573. |
|
CI Vulkan-Loader build # 695 running. |
|
CI Vulkan-Loader build # 695 passed. |
| <td><b>This path is ignored when running an application with root or | ||
| super-user access</b>.<br/> |
There was a problem hiding this comment.
Is this really true, or is what you mean: this path is ignored when running with elevated privileges such as setuid, setgid or filesystem capabilities?
If you're still (correctly) using glibc secure_getenv(), then that's based on the AT_SECURE flag from the kernel, which indicates that the executable ran with some sort of privilege elevation, and is actually nothing to do with whether it's root (uid 0, super-user).
If someone runs a Vulkan application while logged in as root, in principle it's fine to load layers from (root's!) $HOME/.config (usually /root/.config), as long as the environment variables are trustworthy.
If someone runs a Vulkan application that is setuid, setgid, has filesystem capabilities, has undergone a Cx/Px/Ux AppArmor transition, or has any other elevated privilege compared with the parent process from which it inherited its environment variables, then that is the situation where it isn't safe to trust environment variables to be non-malicious - which is what secure_getenv() does for us.
There was a problem hiding this comment.
Thanks for the clarification. We're using secure_getenv based on requests from Chromium, so it sounds like my wording is wrong and should be what you suggest in the first sentence.
There was a problem hiding this comment.
Am I right in thinking that every time Vulkan-Loader looks at an environment variable, including $XDG_*, $VULKAN_* and $HOME, it's using secure_getenv() or equivalent?
If yes, then it might be better to say that once, as a general statement about environment variables, rather than putting it in a subset of the places where environment variables are mentioned (and perhaps making people think that other environment variable references are done unconditionally)?
There was a problem hiding this comment.
I do have a global mention of it as well as individual locations. The intent was people tend to focus solely on the sections they're looking at and by mentioning it multiple times, they may see the comment in one of those locations. If you look in the top-level LoaderInterfaceArchitecture.md file, you'll see that there's now a "Elevated Privileges Caveats" section. I can try to specifically call out the XDG and other items there if you believe it's useful.
|
CI Vulkan-Loader build queued with queue ID 43784. |
|
CI Vulkan-Loader build # 704 running. |
|
CI Vulkan-Loader build # 704 passed. |
|
CI Vulkan-Loader build queued with queue ID 44711. |
|
CI Vulkan-Loader build # 712 running. |
|
CI Vulkan-Loader build # 712 passed. |
|
CI Vulkan-Loader build queued with queue ID 44742. |
|
CI Vulkan-Loader build # 713 running. |
| All implementations (supporting interface version 2 or higher) must export the | ||
| following function that is used for determination of the interface version that | ||
| will be used. | ||
| This entry point is not a part of the Vulkan API itself, only a private | ||
| interface between the loader and implementations. | ||
|
|
||
| ```cpp | ||
| VKAPI_ATTR VkResult VKAPI_CALL | ||
| vk_icdNegotiateLoaderICDInterfaceVersion( |
There was a problem hiding this comment.
Shouldn't this be in the 'version 2' section? Feels like we start with v2, then go to v6 and descend from there.
There was a problem hiding this comment.
I'm not sure what your saying. Are you saying cover Interface v2, then Interface v6, then 5-3 and then 1-0? The reason it's written this way is that the negotiate is the expected mechanism for newly written implementations, so we want to lead with that. Maybe this section should just be labeled "Current Preferred Negotiation Process" or something like that?
There was a problem hiding this comment.
Sorry for the confusion, it stems from the order of the versions information being out of order. I agree that this should be first so maybe I just want to see a link to this section in the Interface Version 2 section.
|
CI Vulkan-Loader build queued with queue ID 1455. |
|
CI Vulkan-Loader build # 749 running. |
|
CI Vulkan-Loader build # 749 passed. |
|
CI Vulkan-Loader build queued with queue ID 1462. |
|
CI Vulkan-Loader build # 750 running. |
|
CI Vulkan-Loader build # 750 passed. |
|
CI Vulkan-Loader build queued with queue ID 331. |
|
CI Vulkan-Loader build # 751 running. |
|
CI Vulkan-Loader build # 751 failed. |
|
CI Vulkan-Loader build queued with queue ID 665. |
|
CI Vulkan-Loader build # 754 running. |
|
CI Vulkan-Loader build # 754 passed. |
|
CI Vulkan-Loader build queued with queue ID 1002. |
|
CI Vulkan-Loader build # 757 running. |
|
CI Vulkan-Loader build # 757 passed. |
| All implementations (supporting interface version 2 or higher) must export the | ||
| following function that is used for determination of the interface version that | ||
| will be used. | ||
| This entry point is not a part of the Vulkan API itself, only a private | ||
| interface between the loader and implementations. | ||
|
|
||
| ```cpp | ||
| VKAPI_ATTR VkResult VKAPI_CALL | ||
| vk_icdNegotiateLoaderICDInterfaceVersion( |
There was a problem hiding this comment.
Sorry for the confusion, it stems from the order of the versions information being out of order. I agree that this should be first so maybe I just want to see a link to this section in the Interface Version 2 section.
| If multiple instances of the same library exist in different locations | ||
| throughout the user's system, then the one appearing first in the search order | ||
| will be used. |
There was a problem hiding this comment.
Unsure of this behavior. Does de-duplication happen if the library_path is the same, if the layer name is the same, or both? (I need to look at the loader source code for an actual answer, but writing a comment here to remember to do so)
There was a problem hiding this comment.
Looks like we only search for layer names (loader.c loader_add_layer_names_to_list).
There was a problem hiding this comment.
I think we have to only do names. If two layers are in the same library, don't we need to support two separate JSONs that could load it? I know it would point to the same instance, but each one might have their own interface functions. Like LayerA_GetProcAddr, and LayerB_GetProcAddr. So it technically could work. I don't like it but... We can discuss it off-line.
docs/LoaderLayerInterface.md
Outdated
| ## Layer Version Negotiation | ||
|
|
||
| Now that a layer has been discovered, an application can choose to load it (or | ||
| it is loaded by default if it is an Implicit layer). |
There was a problem hiding this comment.
| it is loaded by default if it is an Implicit layer). | |
| if it is an Implicit layer load it by default). |
There was a problem hiding this comment.
I like this instead:
Now that a layer has been discovered, an application can choose to load it, or in the case of implicit layers, it can be loaded by default.
|
CI Vulkan-Loader build queued with queue ID 1442. |
|
CI Vulkan-Loader build # 759 running. |
|
CI Vulkan-Loader build # 759 passed. |
|
CI Vulkan-Loader build queued with queue ID 1521. |
|
CI Vulkan-Loader build # 760 running. |
|
CI Vulkan-Loader build # 760 passed. |
Update the loader documentation in the following ways:
1. Move documentation to new "docs" folder"
2. Split LoaderAndLayerInterface.md into multiple docs to focus
on target audience:
- Top-Level LoaderInterfaceArchitecture.md
- Applications LoaderApplicationInterface.md
- Layers LoaderLayerInterface.md
- ICDs LoaderImplementationInterface.md
3. Upload newer images and their corresponding original Inkscape files.
4. Cleanup and update sections on Linux directory search
5. Add new sections to detail items missing
- VkConfig
- Handling undef Fuchsia
6. Language cleanup
Thanks to @charles-lunarg and @smcv for feedback and various section language
corrections.
|
CI Vulkan-Loader build queued with queue ID 1586. |
|
CI Vulkan-Loader build # 761 running. |
|
CI Vulkan-Loader build # 761 passed. |
* Update release number to 197 for this update.
Github Issues:
* Align some of the provisional video standard headers enums and bitfields
to have predictable sizes (public issue 1571).
* Remove exporting of D3D memory handles from
slink:VkExportMemoryWin32HandleInfoKHR (public pull request 1612).
* Add language to slink:VkAccelerationStructureBuildGeometryInfoKHR
explicitly stating that source and target acceleration structures are
allowed to be the same or different during an update (public issue
1641).
* Fix typos (public pull request 1662).
* Register remaining newly introduced `vk_video` types in `vk.xml` (public
pull request 1663).
Internal Issues:
* Clarify <<resources-external-sharing, ownership transfers on external
resources>> (internal issue 2692).
* Changes to (nearly) eliminate dead internal links and improve scripts:
** Correctly generate API dependencies on extensions and core versions in
cases where "`spelling aliases`" were present
** Clean up a few incorrectly marked up links, anchors, and refpage block
alias= attributes
** Use an API alias map to substitute promoted API names for promoted-to
APIs when an older or restricted spec is being generated and the
promoted-to API is not included
* Tag sname:VkVideo{Encode,Decode}H26{4,5}ProfileEXT structures as
extending slink:VkQueryPoolCreateInfo in `vk.xml` (internal issue 2861).
* Grammar edits to slink:VkAccelerationStructureKHR (internal issue 2887).
* Change the cited title of the <<LoaderInterfaceArchitecture>> document
to "`Architecture of the Vulkan Loader Interfaces`" matching a recent
change in KhronosGroup/Vulkan-Loader#685
(internal merge request 4823).
* Re-remove etext:VkVideoEncodeH265CapabilityFlagBitsEXT, which was
accidentally reintroduced but is still unused (internal merge request
4885).
* Update wording for
ename:VK_BUILD_ACCELERATION_STRUCTURE_ALLOW_UPDATE_BIT_KHR since it no
longer contains `"update`" (internal merge request 4886).
* Consistency edits to remove "`instance of`" when referring to a specific
structure, and use "`render pass`" instead of "`renderpass`" as a noun
(internal merge request 4896).
* Add -version option to 'makeSpec' frontend build script.
New Extensions
* `<<VK_KHR_dynamic_rendering>>`
Update the loader documentation in the following ways:
on target audience: