On Thu, Apr 30, 2020 at 11:23:34AM +0000, Thanos Makatos wrote: > > > > I've just shared with you the Google doc we've working on with John > > > where we've > > > > been drafting the protocol specification, we think it's time for some > > > > first > > > > comments. Please feel free to comment/edit and suggest more people > > to > > > be on the > > > > reviewers list. > > > > > > > > You can also find the Google doc here: > > > > > > > > > > > https://urldefense.proofpoint.com/v2/url?u=https- > > 3A__docs.google.com_document_d_1FspkL0hVEnZqHbdoqGLUpyC38rSk- > > 5F&d=DwIFAw&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh5Ps2zJvtw6ogtti46a > > tk736SI4vgsJiUKIyDE&m=lJC7YeMMsAaVsr99tmTYncQdjEfOXiJQkRkJW7NMg > > Rg&s=RyyhgVrLX2bBTqpXZnBmllqkCg_wyalxwZKkfcYt50c&e= > > > 7HhY471TsVwyK8/edit?usp=sharing > > > > > > > > If a Google doc doesn't work for you we're open to suggestions. > > > > > > I can't add comments to the document so I've inlined them here: > > > > > > The spec assumes the reader is already familiar with VFIO and does not > > > explain concepts like the device lifecycle, regions, interrupts, etc. > > > We don't need to duplicate detailed VFIO information, but I think the > > > device model should be explained so that anyone can start from the > > > VFIO-user spec and begin working on an implementation. Right now I > > > think they would have to do some serious investigation of VFIO first in > > > order to be able to write code. > > > > I've added a high-level overview of how VFIO is used in this context. > > > > > "only the source header files are used" > > > I notice the current <linux/vfio.h> header is licensed "GPL-2.0 WITH > > > Linux-syscall-note". I'm not a lawyer but I guess this means there are > > > some restrictions on using this header file. The <linux/virtio*.h> > > > header files were explicitly licensed under the BSD license to make it > > > easy to use the non __KERNEL__ parts. > > > > My impression is that this note actually relaxes the licensing > > requirements, so > > that proprietary software can use the system call headers and run on Linux > > without being considered derived work. In any case I'll double check with > > our > > legal team. > > > > > VFIO-user Command Types: please indicate for each request type whether > > > it is client->server, server->client, or both. Also is it a "command" > > > or "request"? > > > > Will do. It's a command. > > > > > > > vfio_user_req_type <-- is this an extension on top of <linux/vfio.h>? > > > Please make it clear what is part of the base <linux/vfio.h> protocol > > > and what is specific to vfio-user. > > > > Correct, it's an extension over <linux/vfio.h>. I've clarified the two > > symbol > > namespaces. > > > > > > > VFIO_USER_READ/WRITE serve completely different purposes depending > > on > > > whether they are sent client->server or server->client. I suggest > > > defining separate request type constants instead of overloading them. > > > > Fixed. > > > > > What is the difference between VFIO_USER_MAP_DMA and > > > VFIO_USER_REG_MEM? > > > They both seem to be client->server messages for setting up memory but > > > I'm not sure why two request types are needed. > > > > John will provide more information on this. > > > > > struct vfio_user_req->data. Is this really a union so that every > > > message has the same size, regardless of how many parameters are > > passed > > > in the data field? > > > > Correct, it's a union so that every message has the same length. > > > > > "a framebuffer where the guest does multiple stores to the virtual > > > device." Do you mean in SMP guests? Or even in a single CPU guest? > > > > @John > > > > > Also, is there any concurrency requirement on the client and server > > > side? Can I implement a client/server that processes requests > > > sequentially and completes them before moving on to the next request or > > > would that deadlock for certain message types? > > > > I believe that this might also depend on the device semantics, will need to > > think about it in greater detail. > > I've looked at this but can't provide a definitive answer yet. I believe the > safest thing to do is for the server to process requests in order. > > > More importantly, considering: > > a) Marc-André's comments about data alignment etc., and > > b) the possibility to run the server on another guest or host, > > we won't be able to use native VFIO types. If we do want to support that > > then > > we'll have to redefine all data formats, similar to > > https://urldefense.proofpoint.com/v2/url?u=https- > > 3A__github.com_qemu_qemu_blob_master_docs_interop_vhost- > > 2Duser.rst&d=DwIFAw&c=s883GpUCOChKOHiocYtGcg&r=XTpYsh5Ps2zJvtw6 > > ogtti46atk736SI4vgsJiUKIyDE&m=lJC7YeMMsAaVsr99tmTYncQdjEfOXiJQkRkJ > > W7NMgRg&s=1d_kB7VWQ-8d4t6Ikga5KSVwws4vwiVMvTyWVaS6PRU&e= . > > > > So the protocol will be more like an enhanced version of the Vhost-user > > protocol > > than VFIO. I'm fine with either direction (VFIO vs. enhanced Vhost-user), > > so we need to decide before proceeding as the request format is > > substantially > > different. > > Regarding the ability to use the protocol on non-AF_UNIX sockets, we can > support this future use case without unnecessarily complicating the protocol > by > defining the C structs and stating that data alignment and endianness for the > non AF_UNIX case must be the one used by GCC on a x86_64 bit machine, or can > be overridden as required.
Defining it to be x86_64 semantics is effectively saying "we're not going to do anything and it is up to other arch maintainers to fix the inevitable portability problems that arise". Since this is a new protocol should we take the opportunity to model it explicitly in some common standard RPC protocol language. This would have the benefit of allowing implementors to use off the shelf APIs for their wire protocol marshalling, and eliminate questions about endianness and alignment across architectures. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
