rust-vmm / vhost

Apache License 2.0
130 stars 69 forks source link

Support for VHOST_USER_GPU_SET_SOCKET, and the protocol on this socket #236

Open mtjhrc opened 6 months ago

mtjhrc commented 6 months ago

Hi, me and @dorindabassey are working on a vhost-user-gpu implementation, see: https://github.com/rust-vmm/vhost-device/issues/598.

We are aiming at being compatible with QEMU's displays, which means supporting the VHOST_USER_GPU_SET_SOCKET message and the protocol on this socket. For specification see: https://www.qemu.org/docs/master/interop/vhost-user-gpu.html.

The VHOST_USER_GPU_SET_SOCKET is currently defined in the FrontendReq enum: https://github.com/rust-vmm/vhost/blob/6ce9d36b1e7b84e53ece51acd87e5cd8f65ef12c/vhost/src/vhost_user/message.rs#L162)

but is not being handled here: https://github.com/rust-vmm/vhost/blob/6ce9d36b1e7b84e53ece51acd87e5cd8f65ef12c/vhost/src/vhost_user/backend_req_handler.rs#L370-L376

The protocol on this socket is nearly the same as the normal vhost user protocol and the header structure is exactly the same as the existing VhostUserMsgHeader. The problem are the flags, because only the REPLY flag is specified, not even a VERSION flag is used.

I would like to implement this protocol, but my main problem is I don't know if adding new device specific protocol code into this project is ok. I see that there are FS specific things, but they might be removed (see https://github.com/rust-vmm/vhost/issues/213)

I can think of 3 solution with diferent levels genericness:

I am not sure on what is the stance on device-specific and/or non-standard features in this project, any suggestions?

Ablu commented 6 months ago

To me it looks like providing the infrastructure to receive and send messages could be useful to have here (or in a new subcrate potentially). Then devices could implement against that interface. So I think the first option sounds best to me.

elmarco commented 6 months ago

Just a quick note: having VHOST_USER_GPU_SET_SOCKET support is nice, but even better would be to have org.qemu.Display1 dbus support, to avoid QEMU roundtrip when displaying with libmks for example. The two are fairly similar, so it shouldn't be hard to have both anyway.