Skip to content

posix: implement PRIME for fd <-> handle conversion #419

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

Merged
merged 6 commits into from
Jul 6, 2022

Conversation

no92
Copy link
Member

@no92 no92 commented Feb 4, 2022

This is rough code for making the PRIME ioctls work for sway. The code is not optimal and the naming is weird, but I couldn't come up with something better.

Posting as draft, as I don't expect this to survive review unchanged.

mlibc counterpart is managarm/mlibc#367.

64 pushed a commit to 64/managarm that referenced this pull request May 15, 2022
options/posix: Implement uname
@no92 no92 marked this pull request as ready for review May 15, 2022 23:43
@no92 no92 force-pushed the PRIME branch 2 times, most recently from d67deea to 81b97fd Compare July 4, 2022 17:52
Copy link
Member

@avdgrinten avdgrinten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Architecturally, the PR is good now. Some minor issues still need to be fixed.

Comment on lines +945 to +948
auto ret = _buffers.insert({handle, bo});
assert(ret.second);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this ever fail?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully not, but this is a sanity check to catch the (unlikely) case of duplicate handles.

Comment on lines 220 to 222
void writeToState(const Assignment assignment, std::unique_ptr<AtomicState> &state) override {
state->crtc(assignment.object->id())->active = assignment.intValue;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change? Seems that this function is never called.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While unused now, I have some patches to drivers that make this necessary as some piece of software relied on this property.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then this function should be added in a future PR that also adds consumers.

Copy link
Member

@avdgrinten avdgrinten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, the handle-related issue is solved now, but now I don't understand why we need _primeBufferMap at all. The only place where this is read is importBufferObject(). Can't we just adjust importBufferObject() to either return the existing handle (that we get via getHandle()) or create a new handle (via createHandle()), without looking at _primeBufferMap? Or am I missing something?

The remainder of the PR now looks good to me.

no92 and others added 2 commits July 6, 2022 12:41
Co-authored-by: Kacper Słomiński <kacper.slominski72@gmail.com>
no92 added 3 commits July 6, 2022 12:43
This commit adds support for the `PRIME_HANDLE_TO_FD` and
`PRIME_FD_TO_HANDLE` ioctls. These convert between a DRM handle (as used
for the MAP_DUMB ioctl) and a fd, which supports the regular operations,
including being sent over a UNIX socket and being `dup`ed. This is
achieved by managing the shared buffers at the device level by being
exported and imported. The shared buffers are identified by the
credentials (a unique identifier) of the passthrough lane servicing the
file operations, as fds are obviously not stable across threads.
@no92
Copy link
Member Author

no92 commented Jul 6, 2022

Removed _primeBufferMap, squashed the fixups, dropped one commit as requested.

Copy link
Member

@avdgrinten avdgrinten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you for the PR!

@no92 no92 merged commit f787abf into managarm:master Jul 6, 2022
@no92 no92 deleted the PRIME branch July 6, 2022 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants