All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Wang, Wei W" <wei.w.wang@intel.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	"Yang, Zhiyong" <zhiyong.yang@intel.com>,
	"jan.kiszka@siemens.com" <jan.kiszka@siemens.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"avi.cohen@huawei.com" <avi.cohen@huawei.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"marcandre.lureau@redhat.com" <marcandre.lureau@redhat.com>
Subject: Re: [Qemu-devel] [virtio-dev] [PATCH v3 0/7] Vhost-pci for inter-VM communication
Date: Sat, 9 Dec 2017 16:23:17 +0000	[thread overview]
Message-ID: <286AC319A985734F985F78AFA26841F73937E001@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <CAJSP0QUAqCzFgVtM1cg_KybdyrZa_FRUHhDN7oLfRjZ2ZVkp4g@mail.gmail.com>

On Friday, December 8, 2017 4:34 PM, Stefan Hajnoczi wrote:
> On Fri, Dec 8, 2017 at 6:43 AM, Wei Wang <wei.w.wang@intel.com> wrote:
> > On 12/08/2017 07:54 AM, Michael S. Tsirkin wrote:
> >>
> >> On Thu, Dec 07, 2017 at 06:28:19PM +0000, Stefan Hajnoczi wrote:
> >>>
> >>> On Thu, Dec 7, 2017 at 5:38 PM, Michael S. Tsirkin <mst@redhat.com>
> > Thanks Stefan and Michael for the sharing and discussion. I think
> > above 3 and 4 are debatable (e.g. whether it is simpler really
> > depends). 1 and 2 are implementations, I think both approaches could
> > implement the device that way. We originally thought about one device
> > and driver to support all types (called it transformer sometimes :-)
> > ), that would look interesting from research point of view, but from
> > real usage point of view, I think it would be better to have them separated,
> because:
> > - different device types have different driver logic, mixing them
> > together would cause the driver to look messy. Imagine that a
> > networking driver developer has to go over the block related code to
> > debug, that also increases the difficulty.
> 
> I'm not sure I understand where things get messy because:
> 1. The vhost-pci device implementation in QEMU relays messages but has no
> device logic, so device-specific messages like VHOST_USER_NET_SET_MTU are
> trivial at this layer.
> 2. vhost-user slaves only handle certain vhost-user protocol messages.
> They handle device-specific messages for their device type only.  This is like
> vhost drivers today where the ioctl() function returns an error if the ioctl is
> not supported by the device.  It's not messy.
> 
> Where are you worried about messy driver logic?

Probably I didn’t explain well, please let me summarize my thought a little bit, from the perspective of the control path and data path.

Control path: the vhost-user messages - I would prefer just have the interaction between QEMUs, instead of relaying to the GuestSlave, because
1) I think the claimed advantage (easier to debug and develop) doesn’t seem very convincing
2) some messages can be directly answered by QemuSlave , and some messages are not useful to give to the GuestSlave (inside the VM), e.g. fds, VhostUserMemoryRegion from SET_MEM_TABLE msg (the device first maps the master memory and gives the offset (in terms of the bar, i.e., where does it sit in the bar) of the mapped gpa to the guest. if we give the raw VhostUserMemoryRegion to the guest, that wouldn’t be usable).


Data path: that's the discussion we had about one driver or separate driver for different device types, and this is not related to the control path.
I meant if we have one driver for all the types, that driver would look messy, because each type has its own data sending/receiving logic. For example, net type deals with a pair of tx and rx, and transmission is skb based (e.g. xmit_skb), while block type deals with a request queue. If we have one driver, then the driver will include all the things together.


The last part is whether to make it a virtio device or a regular pci device
I don’t have a strong preference. I think virtio device works fine (e.g. use some bar area to create ioevenfds to solve the "no virtqueue no fds" issue if you and Michael think that's acceptable),  and we can reuse some other things like feature negotiation from virtio. But if Michael and you have a decision to make it a regular PCI device, I think that would also work though.

Best,
Wei

WARNING: multiple messages have this Message-ID (diff)
From: "Wang, Wei W" <wei.w.wang@intel.com>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>,
	"virtio-dev@lists.oasis-open.org"
	<virtio-dev@lists.oasis-open.org>,
	"Yang, Zhiyong" <zhiyong.yang@intel.com>,
	"jan.kiszka@siemens.com" <jan.kiszka@siemens.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"avi.cohen@huawei.com" <avi.cohen@huawei.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@redhat.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>,
	"marcandre.lureau@redhat.com" <marcandre.lureau@redhat.com>
Subject: [virtio-dev] RE: [Qemu-devel] [virtio-dev] [PATCH v3 0/7] Vhost-pci for inter-VM communication
Date: Sat, 9 Dec 2017 16:23:17 +0000	[thread overview]
Message-ID: <286AC319A985734F985F78AFA26841F73937E001@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <CAJSP0QUAqCzFgVtM1cg_KybdyrZa_FRUHhDN7oLfRjZ2ZVkp4g@mail.gmail.com>

On Friday, December 8, 2017 4:34 PM, Stefan Hajnoczi wrote:
> On Fri, Dec 8, 2017 at 6:43 AM, Wei Wang <wei.w.wang@intel.com> wrote:
> > On 12/08/2017 07:54 AM, Michael S. Tsirkin wrote:
> >>
> >> On Thu, Dec 07, 2017 at 06:28:19PM +0000, Stefan Hajnoczi wrote:
> >>>
> >>> On Thu, Dec 7, 2017 at 5:38 PM, Michael S. Tsirkin <mst@redhat.com>
> > Thanks Stefan and Michael for the sharing and discussion. I think
> > above 3 and 4 are debatable (e.g. whether it is simpler really
> > depends). 1 and 2 are implementations, I think both approaches could
> > implement the device that way. We originally thought about one device
> > and driver to support all types (called it transformer sometimes :-)
> > ), that would look interesting from research point of view, but from
> > real usage point of view, I think it would be better to have them separated,
> because:
> > - different device types have different driver logic, mixing them
> > together would cause the driver to look messy. Imagine that a
> > networking driver developer has to go over the block related code to
> > debug, that also increases the difficulty.
> 
> I'm not sure I understand where things get messy because:
> 1. The vhost-pci device implementation in QEMU relays messages but has no
> device logic, so device-specific messages like VHOST_USER_NET_SET_MTU are
> trivial at this layer.
> 2. vhost-user slaves only handle certain vhost-user protocol messages.
> They handle device-specific messages for their device type only.  This is like
> vhost drivers today where the ioctl() function returns an error if the ioctl is
> not supported by the device.  It's not messy.
> 
> Where are you worried about messy driver logic?

Probably I didn’t explain well, please let me summarize my thought a little bit, from the perspective of the control path and data path.

Control path: the vhost-user messages - I would prefer just have the interaction between QEMUs, instead of relaying to the GuestSlave, because
1) I think the claimed advantage (easier to debug and develop) doesn’t seem very convincing
2) some messages can be directly answered by QemuSlave , and some messages are not useful to give to the GuestSlave (inside the VM), e.g. fds, VhostUserMemoryRegion from SET_MEM_TABLE msg (the device first maps the master memory and gives the offset (in terms of the bar, i.e., where does it sit in the bar) of the mapped gpa to the guest. if we give the raw VhostUserMemoryRegion to the guest, that wouldn’t be usable).


Data path: that's the discussion we had about one driver or separate driver for different device types, and this is not related to the control path.
I meant if we have one driver for all the types, that driver would look messy, because each type has its own data sending/receiving logic. For example, net type deals with a pair of tx and rx, and transmission is skb based (e.g. xmit_skb), while block type deals with a request queue. If we have one driver, then the driver will include all the things together.


The last part is whether to make it a virtio device or a regular pci device
I don’t have a strong preference. I think virtio device works fine (e.g. use some bar area to create ioevenfds to solve the "no virtqueue no fds" issue if you and Michael think that's acceptable),  and we can reuse some other things like feature negotiation from virtio. But if Michael and you have a decision to make it a regular PCI device, I think that would also work though.

Best,
Wei

  reply	other threads:[~2017-12-09 16:23 UTC|newest]

Thread overview: 170+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-05  3:33 [Qemu-devel] [PATCH v3 0/7] Vhost-pci for inter-VM communication Wei Wang
2017-12-05  3:33 ` [virtio-dev] " Wei Wang
2017-12-05  3:33 ` [Qemu-devel] [PATCH v3 1/7] vhost-user: share the vhost-user protocol related structures Wei Wang
2017-12-05  3:33   ` [virtio-dev] " Wei Wang
2017-12-05  3:33 ` [Qemu-devel] [PATCH v3 2/7] vhost-pci-net: add vhost-pci-net Wei Wang
2017-12-05  3:33   ` [virtio-dev] " Wei Wang
2017-12-05 14:59   ` [Qemu-devel] " Stefan Hajnoczi
2017-12-05 14:59     ` Stefan Hajnoczi
2017-12-05 15:17     ` [Qemu-devel] " Michael S. Tsirkin
2017-12-05 15:17       ` Michael S. Tsirkin
2017-12-05 15:55     ` [Qemu-devel] " Michael S. Tsirkin
2017-12-05 15:55       ` Michael S. Tsirkin
2017-12-05 16:41       ` [Qemu-devel] " Stefan Hajnoczi
2017-12-05 16:41         ` Stefan Hajnoczi
2017-12-05 16:53         ` [Qemu-devel] " Michael S. Tsirkin
2017-12-05 16:53           ` Michael S. Tsirkin
2017-12-05 17:00           ` [Qemu-devel] " Cornelia Huck
2017-12-05 17:00             ` [virtio-dev] " Cornelia Huck
2017-12-05 18:06             ` Michael S. Tsirkin
2017-12-05 18:06               ` [virtio-dev] " Michael S. Tsirkin
2017-12-06 10:17     ` Wei Wang
2017-12-06 10:17       ` Wei Wang
2017-12-06 12:01       ` [Qemu-devel] " Stefan Hajnoczi
2017-12-06 12:01         ` Stefan Hajnoczi
2017-12-05  3:33 ` [Qemu-devel] [PATCH v3 3/7] virtio/virtio-pci.c: add vhost-pci-net-pci Wei Wang
2017-12-05  3:33   ` [virtio-dev] " Wei Wang
2017-12-05  3:33 ` [Qemu-devel] [PATCH v3 4/7] vhost-pci-slave: add vhost-pci slave implementation Wei Wang
2017-12-05  3:33   ` [virtio-dev] " Wei Wang
2017-12-05 15:56   ` [Qemu-devel] " Stefan Hajnoczi
2017-12-05 15:56     ` [virtio-dev] " Stefan Hajnoczi
2017-12-14 17:30   ` [Qemu-devel] " Stefan Hajnoczi
2017-12-14 17:30     ` [virtio-dev] " Stefan Hajnoczi
2017-12-14 17:48   ` [Qemu-devel] " Stefan Hajnoczi
2017-12-14 17:48     ` [virtio-dev] " Stefan Hajnoczi
2017-12-05  3:33 ` [Qemu-devel] [PATCH v3 5/7] vhost-user: VHOST_USER_SET_VHOST_PCI msg Wei Wang
2017-12-05  3:33   ` [virtio-dev] " Wei Wang
2017-12-05 16:00   ` [Qemu-devel] " Stefan Hajnoczi
2017-12-05 16:00     ` [virtio-dev] " Stefan Hajnoczi
2017-12-06 10:32     ` [Qemu-devel] " Wei Wang
2017-12-06 10:32       ` Wei Wang
2017-12-15 12:40       ` [Qemu-devel] " Stefan Hajnoczi
2017-12-15 12:40         ` Stefan Hajnoczi
2017-12-05  3:33 ` [Qemu-devel] [PATCH v3 6/7] vhost-pci-slave: handle VHOST_USER_SET_VHOST_PCI Wei Wang
2017-12-05  3:33   ` [virtio-dev] " Wei Wang
2017-12-05  3:33 ` [Qemu-devel] [PATCH v3 7/7] virtio/vhost.c: vhost-pci needs remote gpa Wei Wang
2017-12-05  3:33   ` [virtio-dev] " Wei Wang
2017-12-05 16:05   ` [Qemu-devel] " Stefan Hajnoczi
2017-12-05 16:05     ` [virtio-dev] " Stefan Hajnoczi
2017-12-06 10:46     ` [Qemu-devel] " Wei Wang
2017-12-06 10:46       ` Wei Wang
2017-12-05  4:13 ` [Qemu-devel] [PATCH v3 0/7] Vhost-pci for inter-VM communication no-reply
2017-12-05  7:01 ` [Qemu-devel] [virtio-dev] " Jason Wang
2017-12-05  7:01   ` Jason Wang
2017-12-05  7:15   ` [Qemu-devel] " Wei Wang
2017-12-05  7:15     ` Wei Wang
2017-12-05  7:19     ` [Qemu-devel] " Jason Wang
2017-12-05  7:19       ` Jason Wang
2017-12-05  8:49       ` [Qemu-devel] " Avi Cohen (A)
2017-12-05  8:49         ` Avi Cohen (A)
2017-12-05 10:36         ` [Qemu-devel] " Wei Wang
2017-12-05 10:36           ` Wei Wang
2017-12-05 14:30 ` [Qemu-devel] " Stefan Hajnoczi
2017-12-05 14:30   ` Stefan Hajnoczi
2017-12-05 15:20 ` [Qemu-devel] " Michael S. Tsirkin
2017-12-05 15:20   ` [virtio-dev] " Michael S. Tsirkin
2017-12-05 16:06 ` [Qemu-devel] [virtio-dev] " Stefan Hajnoczi
2017-12-05 16:06   ` Stefan Hajnoczi
2017-12-06 13:49 ` [Qemu-devel] " Stefan Hajnoczi
2017-12-06 13:49   ` Stefan Hajnoczi
2017-12-06 16:09   ` [Qemu-devel] " Wang, Wei W
2017-12-06 16:09     ` Wang, Wei W
2017-12-06 16:27     ` [Qemu-devel] " Stefan Hajnoczi
2017-12-07  3:57       ` Wei Wang
2017-12-07  3:57         ` [virtio-dev] " Wei Wang
2017-12-07  5:11         ` Michael S. Tsirkin
2017-12-07  5:11           ` [virtio-dev] " Michael S. Tsirkin
2017-12-07  5:34           ` Wei Wang
2017-12-07  5:34             ` [virtio-dev] " Wei Wang
2017-12-07  6:31         ` Stefan Hajnoczi
2017-12-07  7:54           ` Avi Cohen (A)
2017-12-07  7:54             ` [virtio-dev] " Avi Cohen (A)
2017-12-07  8:04             ` Stefan Hajnoczi
2017-12-07  8:31               ` Jason Wang
2017-12-07  8:31                 ` [virtio-dev] " Jason Wang
2017-12-07 10:24                 ` [Qemu-devel] [virtio-dev] " Stefan Hajnoczi
2017-12-07 10:24                   ` [virtio-dev] Re: [Qemu-devel] " Stefan Hajnoczi
2017-12-07 13:33             ` Michael S. Tsirkin
2017-12-07 13:33               ` [virtio-dev] " Michael S. Tsirkin
2017-12-07  9:02           ` Wei Wang
2017-12-07  9:02             ` [virtio-dev] " Wei Wang
2017-12-07 13:08             ` Stefan Hajnoczi
2017-12-07 14:02               ` Michael S. Tsirkin
2017-12-07 14:02                 ` [virtio-dev] " Michael S. Tsirkin
2017-12-07 16:29                 ` Stefan Hajnoczi
2017-12-07 16:47                   ` Michael S. Tsirkin
2017-12-07 16:47                     ` [virtio-dev] " Michael S. Tsirkin
2017-12-07 17:29                     ` Stefan Hajnoczi
2017-12-07 17:38                       ` Michael S. Tsirkin
2017-12-07 17:38                         ` [virtio-dev] " Michael S. Tsirkin
2017-12-07 18:28                         ` Stefan Hajnoczi
2017-12-07 23:54                           ` Michael S. Tsirkin
2017-12-07 23:54                             ` [virtio-dev] " Michael S. Tsirkin
2017-12-08  6:08                             ` Stefan Hajnoczi
2017-12-08 14:27                               ` Michael S. Tsirkin
2017-12-08 14:27                                 ` [virtio-dev] " Michael S. Tsirkin
2017-12-08 16:15                                 ` Stefan Hajnoczi
2017-12-09 16:08                                 ` Wang, Wei W
2017-12-09 16:08                                   ` [virtio-dev] " Wang, Wei W
2017-12-08  6:43                             ` Wei Wang
2017-12-08  6:43                               ` [virtio-dev] " Wei Wang
2017-12-08  8:33                               ` Stefan Hajnoczi
2017-12-09 16:23                                 ` Wang, Wei W [this message]
2017-12-09 16:23                                   ` [virtio-dev] " Wang, Wei W
2017-12-11 11:11                                   ` Stefan Hajnoczi
2017-12-11 11:11                                     ` [virtio-dev] " Stefan Hajnoczi
2017-12-11 13:53                                     ` Wang, Wei W
2017-12-11 13:53                                       ` [virtio-dev] " Wang, Wei W
2017-12-12 10:14                                       ` Stefan Hajnoczi
2017-12-12 10:14                                         ` [virtio-dev] " Stefan Hajnoczi
2017-12-13  8:11                                         ` Wei Wang
2017-12-13  8:11                                           ` [virtio-dev] " Wei Wang
2017-12-13 12:35                                           ` Stefan Hajnoczi
2017-12-13 15:01                                             ` Michael S. Tsirkin
2017-12-13 15:01                                               ` [virtio-dev] " Michael S. Tsirkin
2017-12-13 20:08                                               ` Stefan Hajnoczi
2017-12-13 20:59                                                 ` Michael S. Tsirkin
2017-12-13 20:59                                                   ` [virtio-dev] " Michael S. Tsirkin
2017-12-14 15:06                                                   ` Stefan Hajnoczi
2017-12-14 15:06                                                     ` [virtio-dev] " Stefan Hajnoczi
2017-12-15 10:33                                                     ` Wei Wang
2017-12-15 10:33                                                       ` [virtio-dev] " Wei Wang
2017-12-15 12:37                                                       ` Stefan Hajnoczi
2017-12-15 12:37                                                         ` [virtio-dev] " Stefan Hajnoczi
2017-12-13 21:50                                                 ` Maxime Coquelin
2017-12-13 21:50                                                   ` [virtio-dev] " Maxime Coquelin
2017-12-14 15:46                                                   ` [Qemu-devel] [virtio-dev] " Stefan Hajnoczi
2017-12-14 15:46                                                     ` [virtio-dev] Re: [Qemu-devel] " Stefan Hajnoczi
2017-12-14 16:27                                                     ` [Qemu-devel] [virtio-dev] " Michael S. Tsirkin
2017-12-14 16:27                                                       ` [virtio-dev] Re: [Qemu-devel] " Michael S. Tsirkin
2017-12-14 16:39                                                       ` [Qemu-devel] [virtio-dev] " Maxime Coquelin
2017-12-14 16:39                                                         ` [virtio-dev] Re: [Qemu-devel] " Maxime Coquelin
2017-12-14 16:40                                                         ` [Qemu-devel] [virtio-dev] " Michael S. Tsirkin
2017-12-14 16:40                                                           ` [virtio-dev] Re: [Qemu-devel] " Michael S. Tsirkin
2017-12-14 16:50                                                           ` [Qemu-devel] [virtio-dev] " Maxime Coquelin
2017-12-14 16:50                                                             ` [virtio-dev] Re: [Qemu-devel] " Maxime Coquelin
2017-12-14 18:11                                                             ` [Qemu-devel] [virtio-dev] " Stefan Hajnoczi
2017-12-14 18:11                                                               ` [virtio-dev] Re: [Qemu-devel] " Stefan Hajnoczi
2017-12-14  5:53                                             ` Wei Wang
2017-12-14  5:53                                               ` [virtio-dev] " Wei Wang
2017-12-14 17:32                                               ` Stefan Hajnoczi
2017-12-14 17:32                                                 ` [virtio-dev] " Stefan Hajnoczi
2017-12-15  9:10                                                 ` Wei Wang
2017-12-15  9:10                                                   ` [virtio-dev] " Wei Wang
2017-12-15 12:26                                                   ` Stefan Hajnoczi
2017-12-15 12:26                                                     ` [virtio-dev] " Stefan Hajnoczi
2017-12-14 18:04                                               ` Stefan Hajnoczi
2017-12-14 18:04                                                 ` [virtio-dev] " Stefan Hajnoczi
2017-12-15 10:33                                                 ` [Qemu-devel] [virtio-dev] " Wei Wang
2017-12-15 10:33                                                   ` [virtio-dev] Re: [Qemu-devel] " Wei Wang
2017-12-15 12:00                                                   ` [Qemu-devel] [virtio-dev] " Stefan Hajnoczi
2017-12-15 12:00                                                     ` [virtio-dev] Re: [Qemu-devel] " Stefan Hajnoczi
2017-12-06 16:13   ` Stefan Hajnoczi
2017-12-19 11:35 ` Stefan Hajnoczi
2017-12-19 11:35   ` Stefan Hajnoczi
2017-12-19 14:56   ` [Qemu-devel] " Michael S. Tsirkin
2017-12-19 14:56     ` Michael S. Tsirkin
2017-12-19 17:05     ` [Qemu-devel] " Stefan Hajnoczi
2017-12-20  4:06       ` Michael S. Tsirkin
2017-12-20  4:06         ` [virtio-dev] " Michael S. Tsirkin
2017-12-20  6:26         ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=286AC319A985734F985F78AFA26841F73937E001@shsmsx102.ccr.corp.intel.com \
    --to=wei.w.wang@intel.com \
    --cc=avi.cohen@huawei.com \
    --cc=jan.kiszka@siemens.com \
    --cc=jasowang@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@gmail.com \
    --cc=stefanha@redhat.com \
    --cc=virtio-dev@lists.oasis-open.org \
    --cc=zhiyong.yang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.