Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add IPC event pool function to strip/encode file descriptor from/to IPC event pool handle #205

Open
zzdanowicz opened this issue Aug 30, 2023 · 10 comments · May be fixed by #220
Open

Add IPC event pool function to strip/encode file descriptor from/to IPC event pool handle #205

zzdanowicz opened this issue Aug 30, 2023 · 10 comments · May be fixed by #220
Labels

Comments

@zzdanowicz
Copy link
Contributor

Equivalents of functions:
zeMemGetIpcHandleFromFileDescriptorExp
zeMemGetFileDescriptorFromIpcHandleExp

Function proposals:
zeEventPoolGetIpcHandleFromFileDescriptorExp(ze_context_handle_t hContext, uint64_t handle, ze_ipc_event_pool_handle_t *pIpcHandle)

ze_result_t zeEventPoolGetFileDescriptorFromIpcHandleExp(ze_context_handle_t hContext, ze_ipc_event_pool_handle_t ipcHandle, uint64_t *pHandle)

@jandres742
Copy link

@wdamon-intel : I think we can do this w/o problems. we have the ones for memory, we could add the ones for pools, following the same semantics and meaning. this should be easy to do for v1.8.

jandres742 pushed a commit to jandres742/level-zero-spec that referenced this issue Sep 18, 2023
Added

zeEventPoolGetFileDescriptorFromIpcHandleExp
zeEventPoolGetIpcHandleFromFileDescriptorExp

To help users with the management of IPC event pool handles.

Resolves: oneapi-src#205

Signed-off-by: Jaime Arteaga <[email protected]>
@jandres742 jandres742 linked a pull request Sep 18, 2023 that will close this issue
@jandres742
Copy link

@zzdanowicz please check #220

@wdamon-intel
Copy link
Contributor

Moving out of v1.8 Release. Offline review highlighted a need for further discussion.

@MichalMrozek
Copy link

We had a discussion on this and IPC should remain opaque, so any API's that expose internal handles / OS details shouldn't be added.

@zzdanowicz
Copy link
Contributor Author

So adding that two functions to strip wanted data out of opaque handles is justified.

@MichalMrozek
Copy link

no , as it would make IPC not opaque.

@zzdanowicz
Copy link
Contributor Author

Retrieving data from opaque handler by dedicated function is making handler not opaque? We do not reveal memory layout that can change and sounds like contradiction to me - can you expand your thinking?

@MichalMrozek
Copy link

to keep it opaque we should never return any handle via any APIs

@zzdanowicz
Copy link
Contributor Author

So retrieving via API call partial data from opaque handler is not an option? OK, but right now applications are stripping that data directly - this is much worse as driver can change memory layout of the opaque handler and applications would break. With these at least the driver allows for some control over applications.

@MichalMrozek
Copy link

yes what is now is much worse, but this is due to the fact that IPC is not opaque.
The solution is not to add more API's that return implementation details, but rather make ipc_handles truly opaque and whenever you use IPC you just use IPC opaque structures and nothing else.
And yes exposing handles requires underlying implementation to implement IPC using single handle, which reveals' the implementation details and solidifies it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

4 participants