Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
@ 2023-03-01 15:47 Stefan Hajnoczi
  2023-03-01 16:31 ` Alex Bennée
  2023-03-03  8:11 ` Viresh Kumar
  0 siblings, 2 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-03-01 15:47 UTC (permalink / raw
  To: Viresh Kumar
  Cc: qemu-devel, virtio-dev, Michael S. Tsirkin, Vincent Guittot,
	Alex Bennée, stratos-dev, Oleksandr Tyshchenko, xen-devel,
	Andrew Cooper, Juergen Gross, Sebastien Boeuf, Liu Jiang,
	Mathieu Poirier

[-- Attachment #1: Type: text/plain, Size: 5559 bytes --]

Resend - for some reason my email didn't make it out.

----- Forwarded message from Stefan Hajnoczi <stefanha@redhat.com> -----

Date: Tue, 21 Feb 2023 10:17:01 -0500
From: Stefan Hajnoczi <stefanha@redhat.com>
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>
Subject: Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
Message-ID: <Y/TgbZLY894p4a1S@fedora>

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?

> 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.

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?

> +
>  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.

> +  :id: 41
> +  :equivalent ioctl: N/A
> +  :request payload: Custom mmap description
> +  :reply payload: N/A
> +
> +  When the ``VHOST_USER_PROTOCOL_F_CUSTOM_MMAP`` protocol feature has been
> +  successfully negotiated, this message is submitted by the front-end to
> +  notify the back-end of the special memory mapping requirements, that the
> +  back-end needs to take care of, while mapping any memory regions sent
> +  over by the front-end. The front-end must send this message before
> +  any memory-regions are sent to the back-end via ``VHOST_USER_SET_MEM_TABLE``
> +  or ``VHOST_USER_ADD_MEM_REG`` message types.
> +
>  
>  Back-end message types
>  ----------------------
> -- 
> 2.31.1.272.g89b43f80a514
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 



----- End forwarded message -----

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
  2023-03-01 15:47 [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support Stefan Hajnoczi
@ 2023-03-01 16:31 ` Alex Bennée
  2023-03-01 17:29   ` Stefan Hajnoczi
  2023-03-03  8:11 ` Viresh Kumar
  1 sibling, 1 reply; 6+ messages in thread
From: Alex Bennée @ 2023-03-01 16:31 UTC (permalink / raw
  To: Stefan Hajnoczi
  Cc: Viresh Kumar, qemu-devel, Michael S. Tsirkin, Vincent Guittot,
	stratos-dev, Oleksandr Tyshchenko, xen-devel, Andrew Cooper,
	Juergen Gross, Sebastien Boeuf, Liu Jiang, Mathieu Poirier,
	virtio-dev


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.

>
>> 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?

> 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?

>
>> +
>>  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?

>
>> +  :id: 41
>> +  :equivalent ioctl: N/A
>> +  :request payload: Custom mmap description
>> +  :reply payload: N/A
>> +
>> +  When the ``VHOST_USER_PROTOCOL_F_CUSTOM_MMAP`` protocol feature has been
>> +  successfully negotiated, this message is submitted by the front-end to
>> +  notify the back-end of the special memory mapping requirements, that the
>> +  back-end needs to take care of, while mapping any memory regions sent
>> +  over by the front-end. The front-end must send this message before
>> +  any memory-regions are sent to the back-end via ``VHOST_USER_SET_MEM_TABLE``
>> +  or ``VHOST_USER_ADD_MEM_REG`` message types.
>> +
>>  
>>  Back-end message types
>>  ----------------------
>> -- 
>> 2.31.1.272.g89b43f80a514
>> 
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
>> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>> 
>
>
>
> ----------
>
> [[End of PGP Signed Part]]


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
  2023-03-01 16:31 ` Alex Bennée
@ 2023-03-01 17:29   ` Stefan Hajnoczi
  2023-03-02  8:19     ` Viresh Kumar
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-03-01 17:29 UTC (permalink / raw
  To: Alex Bennée
  Cc: Viresh Kumar, qemu-devel, Michael S. Tsirkin, Vincent Guittot,
	stratos-dev, Oleksandr Tyshchenko, xen-devel, Andrew Cooper,
	Juergen Gross, Sebastien Boeuf, Liu Jiang, Mathieu Poirier,
	virtio-dev

[-- Attachment #1: Type: text/plain, Size: 6737 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
  2023-03-01 17:29   ` Stefan Hajnoczi
@ 2023-03-02  8:19     ` Viresh Kumar
  2023-03-02 12:08       ` Stefan Hajnoczi
  0 siblings, 1 reply; 6+ messages in thread
From: Viresh Kumar @ 2023-03-02  8:19 UTC (permalink / raw
  To: Stefan Hajnoczi
  Cc: Alex Bennée, qemu-devel, Michael S. Tsirkin, Vincent Guittot,
	stratos-dev, Oleksandr Tyshchenko, xen-devel, Andrew Cooper,
	Juergen Gross, Sebastien Boeuf, Liu Jiang, Mathieu Poirier,
	virtio-dev

On 01-03-23, 12:29, Stefan Hajnoczi wrote:
> What is the advantage over defining separate messages? Separate messages
> are cleaner and more typesafe.

I thought we wanted to keep single message for one kind of functionality, which
is mmap related quirks here. And so it would be better if we can reuse the same
for next hypervisor which may need this.

The value parameter is not fixed and is hypervisor specific, for Xen this is the
domain id, for others it may mean something else.

> 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.

These look tricky (and real) and I am not sure how we would want to handle
these. Maybe wait until we have a real use-case ?

> 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?

Maybe we can mention that only the mmap's performed via /dev/xen/privcmd and
/dev/xen/gntdev files are affected by this ?

-- 
viresh


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
  2023-03-02  8:19     ` Viresh Kumar
@ 2023-03-02 12:08       ` Stefan Hajnoczi
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Hajnoczi @ 2023-03-02 12:08 UTC (permalink / raw
  To: Viresh Kumar
  Cc: Alex Bennée, qemu-devel, Michael S. Tsirkin, Vincent Guittot,
	stratos-dev, Oleksandr Tyshchenko, xen-devel, Andrew Cooper,
	Juergen Gross, Sebastien Boeuf, Liu Jiang, Mathieu Poirier,
	virtio-dev

[-- Attachment #1: Type: text/plain, Size: 2194 bytes --]

On Thu, Mar 02, 2023 at 01:49:07PM +0530, Viresh Kumar wrote:
> On 01-03-23, 12:29, Stefan Hajnoczi wrote:
> > What is the advantage over defining separate messages? Separate messages
> > are cleaner and more typesafe.
> 
> I thought we wanted to keep single message for one kind of functionality, which
> is mmap related quirks here. And so it would be better if we can reuse the same
> for next hypervisor which may need this.
> 
> The value parameter is not fixed and is hypervisor specific, for Xen this is the
> domain id, for others it may mean something else.

mmap-related quirks have no parameters or behavior in common so there's
no advantage in sharing a single vhost-user protocol message. Sharing
the same message just makes it awkward to build and parse the message.

> > 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.
> 
> These look tricky (and real) and I am not sure how we would want to handle
> these. Maybe wait until we have a real use-case ?

A way to deal with that is to include mmap information every time fds
are passed with a message instead of sending one global message at the
start of the vhost-user connection. This would allow each mmap to
associate extra information instead of forcing them all to use the same
information.

> > 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?
> 
> Maybe we can mention that only the mmap's performed via /dev/xen/privcmd and
> /dev/xen/gntdev files are affected by this ?

No, this doesn't explain when mmap must be performed via
/dev/xen/privcmd and /dev/xen/gntdev. The spec should be explicit about
this instead of assuming that the device implementer already knows this.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support
  2023-03-01 15:47 [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support Stefan Hajnoczi
  2023-03-01 16:31 ` Alex Bennée
@ 2023-03-03  8:11 ` Viresh Kumar
  1 sibling, 0 replies; 6+ messages in thread
From: Viresh Kumar @ 2023-03-03  8:11 UTC (permalink / raw
  To: Stefan Hajnoczi
  Cc: qemu-devel, virtio-dev, Michael S. Tsirkin, Vincent Guittot,
	Alex Bennée, stratos-dev, Oleksandr Tyshchenko, xen-devel,
	Andrew Cooper, Juergen Gross, Sebastien Boeuf, Liu Jiang,
	Mathieu Poirier

On 01-03-23, 10:47, Stefan Hajnoczi wrote:
> Resend - for some reason my email didn't make it out.

How about this (will send a formal patch later).

Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Tue Feb 21 14:36:30 2023 +0530

    docs: vhost-user: Add Xen specific memory mapping support

    The current model of memory mapping at the back-end works fine where a
    standard call to mmap() (for the respective file descriptor) is enough
    before the front-end can start accessing the guest memory.

    There are other complex cases though where the back-end needs more
    information and simple mmap() isn't enough. 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() and ioctl(), 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_XEN_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_SET_XEN_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 | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 3f18ab424eb0..8be5f5eae941 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -258,6 +258,24 @@ Inflight description

 :queue size: a 16-bit size of virtqueues

+Xen mmap description
+^^^^^^^^^^^^^^^^^^^^
+
++-------+-------+
+| flags | domid |
++-------+-------+
+
+:flags: 64-bit bit field
+
+- Bit 0 is set for Xen foreign memory memory mapping.
+- Bit 1 is set for Xen grant memory memory mapping.
+- Bit 2 is set if the back-end can directly map additional memory (like
+  descriptor buffers or indirect descriptors, which aren't part of already
+  shared memory regions) without the need of front-end sending an additional
+  memory region first.
+
+:domid: a 64-bit Xen hypervisor specific domain id.
+
 C structure
 -----------

@@ -867,6 +885,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_XEN_MMAP             17

 Front-end message types
 -----------------------
@@ -1422,6 +1441,23 @@ Front-end message types
   query the back-end for its device status as defined in the Virtio
   specification.

+``VHOST_USER_SET_XEN_MMAP``
+  :id: 41
+  :equivalent ioctl: N/A
+  :request payload: Xen mmap description
+  :reply payload: N/A
+
+  When the ``VHOST_USER_PROTOCOL_F_XEN_MMAP`` protocol feature has been
+  successfully negotiated, this message is submitted by the front-end to set the
+  Xen hypervisor specific memory mapping configurations at the back-end.  These
+  configurations should be used to mmap memory regions, virtqueues, descriptors
+  and descriptor buffers. The front-end must send this message before any
+  memory-regions are sent to the back-end via ``VHOST_USER_SET_MEM_TABLE`` or
+  ``VHOST_USER_ADD_MEM_REG`` message types. The front-end can send this message
+  multiple times, if different mmap configurations are required for different
+  memory regions, where the most recent ``VHOST_USER_SET_XEN_MMAP`` must be used
+  by the back-end to map any newly shared memory regions.
+

 Back-end message types
 ----------------------

-- 
viresh


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-03-03  8:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-01 15:47 [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support Stefan Hajnoczi
2023-03-01 16:31 ` Alex Bennée
2023-03-01 17:29   ` Stefan Hajnoczi
2023-03-02  8:19     ` Viresh Kumar
2023-03-02 12:08       ` Stefan Hajnoczi
2023-03-03  8:11 ` Viresh Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).