[Qemu-devel] [PATCH 0/5] vhost: enable for all targets

Paolo Bonzini posted 5 patches 5 years, 5 months ago
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test asan passed
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/1543238443-4993-1-git-send-email-pbonzini@redhat.com
There is a newer version of this series
backends/Makefile.objs     |   5 +--
configure                  | 101 ++++++++++++++++++++++++++++-----------------
default-configs/virtio.mak |   4 +-
hw/net/Makefile.objs       |   4 +-
hw/net/vhost_net-stub.c    |  95 ++++++++++++++++++++++++++++++++++++++++++
hw/net/vhost_net.c         |  78 ++--------------------------------
hw/virtio/Makefile.objs    |   5 ++-
hw/virtio/vhost-backend.c  |  11 ++++-
include/exec/poison.h      |   1 -
net/Makefile.objs          |   4 +-
net/net.c                  |   2 +-
net/vhost-user-stub.c      |  23 +++++++++++
tests/Makefile.include     |   5 +--
13 files changed, 209 insertions(+), 129 deletions(-)
create mode 100644 hw/net/vhost_net-stub.c
create mode 100644 net/vhost-user-stub.c
[Qemu-devel] [PATCH 0/5] vhost: enable for all targets
Posted by Paolo Bonzini 5 years, 5 months ago
vhost does not have to be supported only if KVM is present, in fact
vhost-user does not even need any kind of kernel support.  This series
changes this.  The rationale is that, when vhost-user-test.c will be
converted to qgraph, it will be able to test vhost-user support
for virtio-mmio backend even on x86.

Paolo Bonzini (5):
  vhost-net: move stubs to a separate file
  vhost-net-user: add stubs for when no virtio-net device is present
  vhost: restrict Linux dependency to kernel vhost
  vhost-net: compile it on all targets that have virtio-net.
  vhost-net: revamp configure logic

 backends/Makefile.objs     |   5 +--
 configure                  | 101 ++++++++++++++++++++++++++++-----------------
 default-configs/virtio.mak |   4 +-
 hw/net/Makefile.objs       |   4 +-
 hw/net/vhost_net-stub.c    |  95 ++++++++++++++++++++++++++++++++++++++++++
 hw/net/vhost_net.c         |  78 ++--------------------------------
 hw/virtio/Makefile.objs    |   5 ++-
 hw/virtio/vhost-backend.c  |  11 ++++-
 include/exec/poison.h      |   1 -
 net/Makefile.objs          |   4 +-
 net/net.c                  |   2 +-
 net/vhost-user-stub.c      |  23 +++++++++++
 tests/Makefile.include     |   5 +--
 13 files changed, 209 insertions(+), 129 deletions(-)
 create mode 100644 hw/net/vhost_net-stub.c
 create mode 100644 net/vhost-user-stub.c

-- 
1.8.3.1


Re: [Qemu-devel] [PATCH 0/5] vhost: enable for all targets
Posted by Michael S. Tsirkin 5 years, 4 months ago
On Mon, Nov 26, 2018 at 02:20:38PM +0100, Paolo Bonzini wrote:
> vhost does not have to be supported only if KVM is present, in fact
> vhost-user does not even need any kind of kernel support.  This series
> changes this.  The rationale is that, when vhost-user-test.c will be
> converted to qgraph, it will be able to test vhost-user support
> for virtio-mmio backend even on x86.

The reason for limiting it to KVM was very simple:
it has the same set of problems with ordering
as mttcg.

So I guess it's fine but I think we must then limit it
to when tcg emits fence instructions.

Otherwise we'll get subtle race conditions.



> Paolo Bonzini (5):
>   vhost-net: move stubs to a separate file
>   vhost-net-user: add stubs for when no virtio-net device is present
>   vhost: restrict Linux dependency to kernel vhost
>   vhost-net: compile it on all targets that have virtio-net.
>   vhost-net: revamp configure logic
> 
>  backends/Makefile.objs     |   5 +--
>  configure                  | 101 ++++++++++++++++++++++++++++-----------------
>  default-configs/virtio.mak |   4 +-
>  hw/net/Makefile.objs       |   4 +-
>  hw/net/vhost_net-stub.c    |  95 ++++++++++++++++++++++++++++++++++++++++++
>  hw/net/vhost_net.c         |  78 ++--------------------------------
>  hw/virtio/Makefile.objs    |   5 ++-
>  hw/virtio/vhost-backend.c  |  11 ++++-
>  include/exec/poison.h      |   1 -
>  net/Makefile.objs          |   4 +-
>  net/net.c                  |   2 +-
>  net/vhost-user-stub.c      |  23 +++++++++++
>  tests/Makefile.include     |   5 +--
>  13 files changed, 209 insertions(+), 129 deletions(-)
>  create mode 100644 hw/net/vhost_net-stub.c
>  create mode 100644 net/vhost-user-stub.c
> 
> -- 
> 1.8.3.1

Re: [Qemu-devel] [PATCH 0/5] vhost: enable for all targets
Posted by Paolo Bonzini 5 years, 4 months ago
On 26/11/18 23:43, Michael S. Tsirkin wrote:
> On Mon, Nov 26, 2018 at 02:20:38PM +0100, Paolo Bonzini wrote:
>> vhost does not have to be supported only if KVM is present, in fact
>> vhost-user does not even need any kind of kernel support.  This series
>> changes this.  The rationale is that, when vhost-user-test.c will be
>> converted to qgraph, it will be able to test vhost-user support
>> for virtio-mmio backend even on x86.
> 
> The reason for limiting it to KVM was very simple:
> it has the same set of problems with ordering
> as mttcg.
> 
> So I guess it's fine but I think we must then limit it
> to when tcg emits fence instructions.
> 
> Otherwise we'll get subtle race conditions.

Got it, I'll take a look.  However, right now you _can_ use vhost with
tcg as long as e.g. qemu-system-x86_64 is compiled with KVM support, so
this patch is not changing anything in this respect; the bug is
independent of this series.

Paolo


Re: [Qemu-devel] [PATCH 0/5] vhost: enable for all targets
Posted by Michael S. Tsirkin 5 years, 4 months ago
On Tue, Nov 27, 2018 at 10:04:59AM +0100, Paolo Bonzini wrote:
> On 26/11/18 23:43, Michael S. Tsirkin wrote:
> > On Mon, Nov 26, 2018 at 02:20:38PM +0100, Paolo Bonzini wrote:
> >> vhost does not have to be supported only if KVM is present, in fact
> >> vhost-user does not even need any kind of kernel support.  This series
> >> changes this.  The rationale is that, when vhost-user-test.c will be
> >> converted to qgraph, it will be able to test vhost-user support
> >> for virtio-mmio backend even on x86.
> > 
> > The reason for limiting it to KVM was very simple:
> > it has the same set of problems with ordering
> > as mttcg.
> > 
> > So I guess it's fine but I think we must then limit it
> > to when tcg emits fence instructions.
> > 
> > Otherwise we'll get subtle race conditions.
> 
> Got it, I'll take a look.  However, right now you _can_ use vhost with
> tcg as long as e.g. qemu-system-x86_64 is compiled with KVM support, so
> this patch is not changing anything in this respect; the bug is
> independent of this series.
> 
> Paolo
> 

Well qemu built with KVM implies host architecture == guest
architecture, right? That should ensure race conditions do not happen in
a very hacky way.

-- 
MST

Re: [Qemu-devel] [PATCH 0/5] vhost: enable for all targets
Posted by Paolo Bonzini 5 years, 4 months ago
On 27/11/18 17:27, Michael S. Tsirkin wrote:
> On Tue, Nov 27, 2018 at 10:04:59AM +0100, Paolo Bonzini wrote:
>> On 26/11/18 23:43, Michael S. Tsirkin wrote:
>>> On Mon, Nov 26, 2018 at 02:20:38PM +0100, Paolo Bonzini wrote:
>>>> vhost does not have to be supported only if KVM is present, in fact
>>>> vhost-user does not even need any kind of kernel support.  This series
>>>> changes this.  The rationale is that, when vhost-user-test.c will be
>>>> converted to qgraph, it will be able to test vhost-user support
>>>> for virtio-mmio backend even on x86.
>>>
>>> The reason for limiting it to KVM was very simple:
>>> it has the same set of problems with ordering
>>> as mttcg.
>>>
>>> So I guess it's fine but I think we must then limit it
>>> to when tcg emits fence instructions.
>>>
>>> Otherwise we'll get subtle race conditions.
>>
>> Got it, I'll take a look.  However, right now you _can_ use vhost with
>> tcg as long as e.g. qemu-system-x86_64 is compiled with KVM support, so
>> this patch is not changing anything in this respect; the bug is
>> independent of this series.
> 
> Well qemu built with KVM implies host architecture == guest
> architecture, right? That should ensure race conditions do not happen in
> a very hacky way.

It's not enough, event_idx requires smp_mb and single-threaded TCG does
not emit them.  It is enough for smp_rmb/smp_wmb however, that makes sense.

Paolo