On Wed, Mar 01, 2023 at 04:31:41PM +0000, Alex Bennée wrote:
Stefan Hajnoczi stefanha@redhat.com writes:
[[PGP Signed Part:Undecided]] Resend - for some reason my email didn't make it out.
From: Stefan Hajnoczi stefanha@redhat.com Subject: Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support To: Viresh Kumar viresh.kumar@linaro.org Cc: qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org, "Michael S. Tsirkin" mst@redhat.com, Vincent Guittot vincent.guittot@linaro.org, Alex Bennée alex.bennee@linaro.org, stratos-dev@op-lists.linaro.org, Oleksandr Tyshchenko olekstysh@gmail.com, xen-devel@lists.xen.org, Andrew Cooper andrew.cooper3@citrix.com, Juergen Gross jgross@suse.com, Sebastien Boeuf sebastien.boeuf@intel.com, Liu Jiang gerry@linux.alibaba.com, Mathieu Poirier mathieu.poirier@linaro.org Date: Tue, 21 Feb 2023 10:17:01 -0500 (1 week, 1 day, 1 hour ago) Flags: seen, signed, personal
On Tue, Feb 21, 2023 at 03:20:41PM +0530, Viresh Kumar wrote:
The current model of memory mapping at the back-end works fine with Qemu, where a standard call to mmap() for the respective file descriptor, passed from front-end, is generally all we need to do before the front-end can start accessing the guest memory.
There are other complex cases though, where we need more information at the backend and need to do more than just an mmap() call. For example, Xen, a type-1 hypervisor, currently supports memory mapping via two different methods, foreign-mapping (via /dev/privcmd) and grant-dev (via /dev/gntdev). In both these cases, the back-end needs to call mmap() followed by an ioctl() (or vice-versa), and need to pass extra information via the ioctl(), like the Xen domain-id of the guest whose memory we are trying to map.
Add a new protocol feature, 'VHOST_USER_PROTOCOL_F_CUSTOM_MMAP', which lets the back-end know about the additional memory mapping requirements. When this feature is negotiated, the front-end can send the 'VHOST_USER_CUSTOM_MMAP' message type to provide the additional information to the back-end.
Signed-off-by: Viresh Kumar viresh.kumar@linaro.org
docs/interop/vhost-user.rst | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)
The alternative to an in-band approach is to configure these details out-of-band. For example, via command-line options to the vhost-user back-end:
$ my-xen-device --mapping-type=foreign-mapping --domain-id=123
I was thinking about both approaches and don't see an obvious reason to choose one or the other. What do you think?
In-band has the nice property of being dynamic and not having to have some other thing construct command lines. We are also trying to keep the daemons from being Xen specific and keep the type of mmap as an implementation detail that is mostly elided by the rust-vmm memory traits.
Okay.
diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst index 3f18ab424eb0..f2b1d705593a 100644 --- a/docs/interop/vhost-user.rst +++ b/docs/interop/vhost-user.rst @@ -258,6 +258,23 @@ Inflight description :queue size: a 16-bit size of virtqueues +Custom mmap description +^^^^^^^^^^^^^^^^^^^^^^^
++-------+-------+ +| flags | value | ++-------+-------+
+:flags: 64-bit bit field
+- Bit 0 is Xen foreign memory access flag - needs Xen foreign memory mapping. +- Bit 1 is Xen grant memory access flag - needs Xen grant memory mapping.
+:value: a 64-bit hypervisor specific value.
+- For Xen foreign or grant memory access, this is set with guest's xen domain
- id.
This is highly Xen-specific. How about naming the feature XEN_MMAP instead of CUSTOM_MMAP? If someone needs to add other mmap data later, they should define their own struct instead of trying to squeeze into the same fields as Xen.
We hope to support additional mmap mechanisms in the future - one proposal is to use the hypervisor specific interface to support an ioctl() that creates a domain specific device which can then be treated more generically.
Could we not declare the message as flag + n bytes of domain specific message as defined my msglen?
What is the advantage over defining separate messages? Separate messages are cleaner and more typesafe.
There is an assumption in this design that a single VHOST_USER_CUSTOM_MMAP message provides the information necessary for all mmaps. Are you sure the limitation that every mmap belongs to the same domain will be workable in the future?
Like a daemon servicing multiple VMs? Won't those still be separate vhost-user control channels though?
I don't have a concrete example, but was thinking of a guest that shares memory with other guests (like the experimental virtio-vhost-user device). Maybe there would be a scenario where some memory belongs to one domain and some belongs to another (but has been mapped into the first domain), and the vhost-user back-end needs to access both.
The other thing that comes to mind is that the spec must clearly state which mmaps are affected by the Xen domain information. For example, just mem table memory regions and not the VHOST_USER_PROTOCOL_F_LOG_SHMFD feature?
C structure
@@ -867,6 +884,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS 14 #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 #define VHOST_USER_PROTOCOL_F_STATUS 16
- #define VHOST_USER_PROTOCOL_F_CUSTOM_MMAP 17
Front-end message types
@@ -1422,6 +1440,20 @@ Front-end message types query the back-end for its device status as defined in the Virtio specification. +``VHOST_USER_CUSTOM_MMAP``
Most vhost-user protocol messages have a verb like get/set/close/add/listen/etc. I suggest renaming this to VHOST_USER_SET_XEN_MMAP_INFO.
VHOST_USER_CONFIG_MMAP_QUIRKS?
VHOST_USER_CONFIG_MMAP_TYPE?
SET is the verb that's often used when the front-end provides configuration parameters to the back-end (e.g. VHOST_USER_SET_MEM_TABLE, VHOST_USER_SET_FEATURES, etc).
Stefan