* [PATCH v4 0/6] virtio core DMA API conversion
From: Andy Lutomirski @ 2015-10-30 1:09 UTC (permalink / raw)
To: linux-kernel, David S. Miller, sparclinux
Cc: linux-s390, Joerg Roedel, KVM, Michael S. Tsirkin, benh,
Sebastian Ott, virtualization, Christian Borntraeger,
Andy Lutomirski, Paolo Bonzini, dwmw2, Christoph Hellwig,
Martin Schwidefsky
This switches virtio to use the DMA API unconditionally. I'm sure
it breaks things, but it seems to work on x86 using virtio-pci, with
and without Xen, and using both the modern 1.0 variant and the
legacy variant.
This appears to work on native and Xen x86_64 using both modern and
legacy virtio-pci. It also appears to work on arm and arm64.
It definitely won't work as-is on s390x, and I haven't been able to
test Christian's patches because I can't get virtio-ccw to work in
QEMU at all. I don't know what I'm doing wrong.
It doesn't work on ppc64. Ben, consider yourself pinged to send me
a patch :)
It doesn't work on sparc64. I didn't realize at Kernel Summit that
sparc64 has the same problem as ppc64.
DaveM, for background, we're trying to fix virtio to use the DMA
API. That will require that every platform that uses virtio
supplies valid DMA operations on devices that use virtio_ring.
Unfortunately, QEMU historically ignores the IOMMU on virtio
devices.
On x86, this isn't really a problem. x86 has a nice way for the
platform to describe which devices are behind an IOMMU, and QEMU
will be adjusted accordingly. The only thing that will break is a
recently-added experimental mode.
Ben's plan for powerpc is to add a quirk for existing virtio-pci
devices and to eventually update the devicetree stuff to allow QEMU
to tell the guest which devices use the IOMMU.
AFAICT sparc has a similar problem to powerpc. DaveM, can you come
up with a straightforward way to get sparc's DMA API to work
correctly for virtio-pci devices?
NB: Sadly, the platforms I've successfully tested on don't include any
big-endian platforms, so there could still be lurking endian problems.
Changes from v3:
- More big-endian fixes.
- Added better virtio-ring APIs that handle allocation and use them in
virtio-mmio and virtio-pci.
- Switch to Michael's virtio-net patch.
Changes from v2:
- Fix vring_mapping_error incorrect argument
Changes from v1:
- Fix an endian conversion error causing a BUG to hit.
- Fix a DMA ordering issue (swiotlb=force works now).
- Minor cleanups.
Andy Lutomirski (5):
virtio_ring: Support DMA APIs
virtio_pci: Use the DMA API
virtio: Add improved queue allocation API
virtio_mmio: Use the DMA API
virtio_pci: Use the DMA API
Michael S. Tsirkin (1):
virtio-net: Stop doing DMA from the stack
drivers/net/virtio_net.c | 34 ++--
drivers/virtio/Kconfig | 2 +-
drivers/virtio/virtio_mmio.c | 67 ++-----
drivers/virtio/virtio_pci_common.h | 6 -
drivers/virtio/virtio_pci_legacy.c | 42 ++---
drivers/virtio/virtio_pci_modern.c | 61 ++-----
drivers/virtio/virtio_ring.c | 348 ++++++++++++++++++++++++++++++-------
include/linux/virtio.h | 23 ++-
include/linux/virtio_ring.h | 35 ++++
tools/virtio/linux/dma-mapping.h | 17 ++
10 files changed, 426 insertions(+), 209 deletions(-)
create mode 100644 tools/virtio/linux/dma-mapping.h
--
2.4.3
^ permalink raw reply
* Re: [PATCH v2 1/3] virtio_net: Stop doing DMA from the stack
From: Andy Lutomirski @ 2015-10-29 23:43 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-s390, Joerg Roedel, KVM, Benjamin Herrenschmidt,
Sebastian Ott, linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, Andy Lutomirski, Network Development,
Paolo Bonzini, David Woodhouse, Christoph Hellwig,
Martin Schwidefsky
In-Reply-To: <20151028084249-mutt-send-email-mst@redhat.com>
On Wed, Oct 28, 2015 at 12:07 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> How about this instead? Less code, more robust.
>
> Warning: untested. If you do like this approach, Tested-by would be
> appreciated.
I like it.
Tested-by: Andy Lutomirski <luto@kernel.org>
--Andy
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: David Woodhouse @ 2015-10-29 16:18 UTC (permalink / raw)
To: Michael S. Tsirkin, Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, Benjamin Herrenschmidt,
Sebastian Ott, linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, Andy Lutomirski, Paolo Bonzini,
Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20151029104301-mutt-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 1026 bytes --]
On Thu, 2015-10-29 at 11:01 +0200, Michael S. Tsirkin wrote:
>
> Example: you have a mix of assigned devices and virtio devices. You
> don't trust your assigned device vendor not to corrupt your memory so
> you want to limit the damage your assigned device can do to your
> guest,
> so you use an IOMMU for that. Thus existing iommu=pt within guest is
> out.
>
> But you trust your hypervisor (you have no choice anyway),
> and you don't want the overhead of tweaking IOMMU
> on data path for virtio. Thus iommu=on is out too.
That's not at all special for virtio or guest VMs. Even with real
hardware, we might want performance from *some* devices, and security
from others. See the DMA_ATTR_IOMMU_BYPASS which is currently being
discussed.
But of course the easy answer in *your* case it just to ask the
hypervisor not to put the virtio devices behind an IOMMU at all. Which
we were planning to remain the default behaviour.
In all cases, the DMA API shall do the right thing.
--
dwmw2
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [PATCH] VSOCK: define VSOCK_SS_LISTEN once only
From: Stefan Hajnoczi @ 2015-10-29 11:57 UTC (permalink / raw)
To: netdev
Cc: pv-drivers, linux-kernel, virtualization, Stefan Hajnoczi, davem,
jhansen
The SS_LISTEN socket state is defined by both af_vsock.c and
vmci_transport.c. This is risky since the value could be changed in one
file and the other would be out of sync.
Rename from SS_LISTEN to VSOCK_SS_LISTEN since the constant is not part
of enum socket_state (SS_CONNECTED, ...). This way it is clear that the
constant is vsock-specific.
The big text reflow in af_vsock.c was necessary to keep to the maximum
line length. Text is unchanged except for s/SS_LISTEN/VSOCK_SS_LISTEN/.
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
include/net/af_vsock.h | 3 +++
net/vmw_vsock/af_vsock.c | 37 ++++++++++++++++++-------------------
net/vmw_vsock/vmci_transport.c | 4 +---
3 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/include/net/af_vsock.h b/include/net/af_vsock.h
index db639a4..e9eb2d6 100644
--- a/include/net/af_vsock.h
+++ b/include/net/af_vsock.h
@@ -22,6 +22,9 @@
#include "vsock_addr.h"
+/* vsock-specific sock->sk_state constants */
+#define VSOCK_SS_LISTEN 255
+
#define LAST_RESERVED_PORT 1023
#define vsock_sk(__sk) ((struct vsock_sock *)__sk)
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index df5fc6b..65f2c9c 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -36,19 +36,20 @@
* not support simultaneous connects (two "client" sockets connecting).
*
* - "Server" sockets are referred to as listener sockets throughout this
- * implementation because they are in the SS_LISTEN state. When a connection
- * request is received (the second kind of socket mentioned above), we create a
- * new socket and refer to it as a pending socket. These pending sockets are
- * placed on the pending connection list of the listener socket. When future
- * packets are received for the address the listener socket is bound to, we
- * check if the source of the packet is from one that has an existing pending
- * connection. If it does, we process the packet for the pending socket. When
- * that socket reaches the connected state, it is removed from the listener
- * socket's pending list and enqueued in the listener socket's accept queue.
- * Callers of accept(2) will accept connected sockets from the listener socket's
- * accept queue. If the socket cannot be accepted for some reason then it is
- * marked rejected. Once the connection is accepted, it is owned by the user
- * process and the responsibility for cleanup falls with that user process.
+ * implementation because they are in the VSOCK_SS_LISTEN state. When a
+ * connection request is received (the second kind of socket mentioned above),
+ * we create a new socket and refer to it as a pending socket. These pending
+ * sockets are placed on the pending connection list of the listener socket.
+ * When future packets are received for the address the listener socket is
+ * bound to, we check if the source of the packet is from one that has an
+ * existing pending connection. If it does, we process the packet for the
+ * pending socket. When that socket reaches the connected state, it is removed
+ * from the listener socket's pending list and enqueued in the listener
+ * socket's accept queue. Callers of accept(2) will accept connected sockets
+ * from the listener socket's accept queue. If the socket cannot be accepted
+ * for some reason then it is marked rejected. Once the connection is
+ * accepted, it is owned by the user process and the responsibility for cleanup
+ * falls with that user process.
*
* - It is possible that these pending sockets will never reach the connected
* state; in fact, we may never receive another packet after the connection
@@ -114,8 +115,6 @@ static struct proto vsock_proto = {
*/
#define VSOCK_DEFAULT_CONNECT_TIMEOUT (2 * HZ)
-#define SS_LISTEN 255
-
static const struct vsock_transport *transport;
static DEFINE_MUTEX(vsock_register_mutex);
@@ -887,7 +886,7 @@ static unsigned int vsock_poll(struct file *file, struct socket *sock,
/* Listening sockets that have connections in their accept
* queue can be read.
*/
- if (sk->sk_state == SS_LISTEN
+ if (sk->sk_state == VSOCK_SS_LISTEN
&& !vsock_is_accept_queue_empty(sk))
mask |= POLLIN | POLLRDNORM;
@@ -1144,7 +1143,7 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr,
err = -EALREADY;
break;
default:
- if ((sk->sk_state == SS_LISTEN) ||
+ if ((sk->sk_state == VSOCK_SS_LISTEN) ||
vsock_addr_cast(addr, addr_len, &remote_addr) != 0) {
err = -EINVAL;
goto out;
@@ -1256,7 +1255,7 @@ static int vsock_accept(struct socket *sock, struct socket *newsock, int flags)
goto out;
}
- if (listener->sk_state != SS_LISTEN) {
+ if (listener->sk_state != VSOCK_SS_LISTEN) {
err = -EINVAL;
goto out;
}
@@ -1348,7 +1347,7 @@ static int vsock_listen(struct socket *sock, int backlog)
}
sk->sk_max_ack_backlog = backlog;
- sk->sk_state = SS_LISTEN;
+ sk->sk_state = VSOCK_SS_LISTEN;
err = 0;
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 1f63daf..70cc89b 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -90,8 +90,6 @@ static int PROTOCOL_OVERRIDE = -1;
*/
#define VSOCK_DEFAULT_CONNECT_TIMEOUT (2 * HZ)
-#define SS_LISTEN 255
-
/* Helper function to convert from a VMCI error code to a VSock error code. */
static s32 vmci_transport_error_to_vsock_error(s32 vmci_error)
@@ -919,7 +917,7 @@ static void vmci_transport_recv_pkt_work(struct work_struct *work)
vsock_sk(sk)->local_addr.svm_cid = pkt->dg.dst.context;
switch (sk->sk_state) {
- case SS_LISTEN:
+ case VSOCK_SS_LISTEN:
vmci_transport_recv_listen(sk, pkt);
break;
case SS_CONNECTING:
--
2.4.3
^ permalink raw reply related
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-29 9:01 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, Benjamin Herrenschmidt,
Sebastian Ott, linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, Andy Lutomirski, Paolo Bonzini,
David Woodhouse, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <CALCETrWd7mK5bW2p_dvvjDRnqE31=ueAz5SAYKwY-PD5uZp76A@mail.gmail.com>
On Wed, Oct 28, 2015 at 03:51:58PM -0700, Andy Lutomirski wrote:
> On Wed, Oct 28, 2015 at 9:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Wed, Oct 28, 2015 at 11:32:34PM +0900, David Woodhouse wrote:
> >> > I don't have a problem with extending DMA API to address
> >> > more usecases.
> >>
> >> No, this isn't an extension. This is fixing a bug, on certain platforms
> >> where the DMA API has currently done the wrong thing.
> >>
> >> We have historically worked around that bug by introducing *another*
> >> bug, which is not to *use* the DMA API in the virtio driver.
> >>
> >> Sure, we can co-ordinate those two bug-fixes. But let's not talk about
> >> them as anything other than bug-fixes.
> >
> > It was pretty practical not to use it. All virtio devices at the time
> > without exception bypassed the IOMMU, so it was a question of omitting a
> > couple of function calls in virtio versus hacking on DMA implementation
> > on multiple platforms. We have more policy options now, so I agree it's
> > time to revisit this.
> >
> > But for me, the most important thing is that we do coordinate.
> >
> >> > > Drivers use DMA API. No more talky.
> >> >
> >> > Well for virtio they don't ATM. And 1:1 mapping makes perfect sense
> >> > for the wast majority of users, so I can't switch them over
> >> > until the DMA API actually addresses all existing usecases.
> >>
> >> That's still not your business; it's the platform's. And there are
> >> hardware implementations of the virtio protocols on real PCI cards. And
> >> we have the option of doing IOMMU translation for the virtio devices
> >> even in a virtual machine. Just don't get involved.
> >>
> >> --
> >> dwmw2
> >>
> >>
> >
> > I'm involved anyway, it's possible not to put all the code in the virtio
> > subsystem in guest though. But I suspect we'll need to find a way for
> > non-linux drivers within guest to work correctly too, and they might
> > have trouble poking at things at the system level. So possibly virtio
> > subsystem will have to tell platform "this device wants to bypass IOMMU"
> > and then DMA API does the right thing.
> >
>
> After some discussion at KS, no one came up with an example where it's
> necessary, and the patches to convert virtqueue to use the DMA API are
> much nicer when they convert it unconditionally.
It's very surprising no one couldn't. I did above, I try again below.
Note: below discusses configuration *within guest*.
Example: you have a mix of assigned devices and virtio devices. You
don't trust your assigned device vendor not to corrupt your memory so
you want to limit the damage your assigned device can do to your guest,
so you use an IOMMU for that. Thus existing iommu=pt within guest is out.
But you trust your hypervisor (you have no choice anyway),
and you don't want the overhead of tweaking IOMMU
on data path for virtio. Thus iommu=on is out too.
> The two interesting cases we thought of were PPC and x86's emulated
> Q35 IOMMU. PPC will look in to architecting a devicetree-based way to
> indicate passthrough status and will add quirks for the existing
> virtio devices.
Isn't this specified by the hypervisor? I don't think this is a good way
to do this: guest security should be up to guest.
> Everyone seems to agree that x86's emulated Q35 thing
> is just buggy right now and should be taught to use the existing ACPI
> mechanism for enumerating passthrough devices.
I'm not sure what ACPI has to do with it.
It's about a way for guest users to specify whether
they want to bypass an IOMMU for a given device.
> I'll send a new version of the series soon.
>
> --Andy
By the way, a bunch of code is missing on the QEMU side
to make this useful:
1. virtio ignores the iommu
2. vhost user ignores the iommu
3. dataplane ignores the iommu
4. vhost-net ignores the iommu
5. VFIO ignores the iommu
I think so far I only saw patches for 1 above.
--
MST
^ permalink raw reply
* [PATCH net-next rfc V2 2/2] vhost_net: basic polling support
From: Jason Wang @ 2015-10-29 8:45 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1446108326-37765-1-git-send-email-jasowang@redhat.com>
This patch tries to poll for new added tx buffer for a while at the
end of tx processing. The maximum time spent on polling were limited
through a module parameter. To avoid block rx, the loop will end it
there's new other works queued on vhost so in fact socket receive
queue is also be polled.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 50 insertions(+), 4 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 9eda69e..30e6d3d 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -31,9 +31,13 @@
#include "vhost.h"
static int experimental_zcopytx = 1;
+static int busyloop_timeout = 50;
module_param(experimental_zcopytx, int, 0444);
MODULE_PARM_DESC(experimental_zcopytx, "Enable Zero Copy TX;"
" 1 -Enable; 0 - Disable");
+module_param(busyloop_timeout, int, 0444);
+MODULE_PARM_DESC(busyloop_timeout, "Maximum number of time (in us) "
+ "could be spend on busy polling");
/* Max number of bytes transferred before requeueing the job.
* Using this limit prevents one virtqueue from starving others. */
@@ -287,6 +291,49 @@ static void vhost_zerocopy_callback(struct ubuf_info *ubuf, bool success)
rcu_read_unlock_bh();
}
+static inline unsigned long busy_clock(void)
+{
+ return local_clock() >> 10;
+}
+
+static bool tx_can_busy_poll(struct vhost_dev *dev,
+ unsigned long endtime)
+{
+ return likely(!need_resched()) &&
+ likely(!time_after(busy_clock(), endtime)) &&
+ likely(!signal_pending(current)) &&
+ !vhost_has_work(dev) &&
+ single_task_running();
+}
+
+static int vhost_net_tx_get_vq_desc(struct vhost_virtqueue *vq,
+ struct iovec iov[], unsigned int iov_size,
+ unsigned int *out_num, unsigned int *in_num)
+{
+ unsigned long uninitialized_var(endtime);
+ int head;
+
+ if (busyloop_timeout) {
+ preempt_disable();
+ endtime = busy_clock() + busyloop_timeout;
+ }
+
+again:
+ head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
+ out_num, in_num, NULL, NULL);
+
+ if (head == vq->num && busyloop_timeout &&
+ tx_can_busy_poll(vq->dev, endtime)) {
+ cpu_relax();
+ goto again;
+ }
+
+ if (busyloop_timeout)
+ preempt_enable();
+
+ return head;
+}
+
/* Expects to be always run from workqueue - which acts as
* read-size critical section for our kind of RCU. */
static void handle_tx(struct vhost_net *net)
@@ -331,10 +378,9 @@ static void handle_tx(struct vhost_net *net)
% UIO_MAXIOV == nvq->done_idx))
break;
- head = vhost_get_vq_desc(vq, vq->iov,
- ARRAY_SIZE(vq->iov),
- &out, &in,
- NULL, NULL);
+ head = vhost_net_tx_get_vq_desc(vq, vq->iov,
+ ARRAY_SIZE(vq->iov),
+ &out, &in);
/* On error, stop handling until the next kick. */
if (unlikely(head < 0))
break;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next rfc V2 1/2] vhost: introduce vhost_has_work()
From: Jason Wang @ 2015-10-29 8:45 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <1446108326-37765-1-git-send-email-jasowang@redhat.com>
This path introduces a helper which can give a hint for whether or not
there's a work queued in the work list.
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 7 +++++++
drivers/vhost/vhost.h | 1 +
2 files changed, 8 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index eec2f11..163b365 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -245,6 +245,13 @@ void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work)
}
EXPORT_SYMBOL_GPL(vhost_work_queue);
+/* A lockless hint for busy polling code to exit the loop */
+bool vhost_has_work(struct vhost_dev *dev)
+{
+ return !list_empty(&dev->work_list);
+}
+EXPORT_SYMBOL_GPL(vhost_has_work);
+
void vhost_poll_queue(struct vhost_poll *poll)
{
vhost_work_queue(poll->dev, &poll->work);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 4772862..ea0327d 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -37,6 +37,7 @@ struct vhost_poll {
void vhost_work_init(struct vhost_work *work, vhost_work_fn_t fn);
void vhost_work_queue(struct vhost_dev *dev, struct vhost_work *work);
+bool vhost_has_work(struct vhost_dev *dev);
void vhost_poll_init(struct vhost_poll *poll, vhost_work_fn_t fn,
unsigned long mask, struct vhost_dev *dev);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next rfc V2 0/2] basic busy polling support for vhost_net
From: Jason Wang @ 2015-10-29 8:45 UTC (permalink / raw)
To: mst, kvm, virtualization, netdev, linux-kernel
Hi all:
This series tries to add basic busy polling for vhost net. The idea is
simple: at the end of tx processing, busy polling for new tx added
descriptor and rx receive socket for a while. The maximum number of
time (in us) could be spent on busy polling was specified through
module parameter.
Test were done through:
- 50 us as busy loop timeout
- Netperf 2.6
- Two machines with back to back connected mlx4
- Guest with 8 vcpus and 1 queue
Result shows very huge improvement on both tx (at most 158%) and rr
(at most 53%) while rx is as much as in the past. Most cases the cpu
utilization is also improved:
Guest TX:
size/session/+thu%/+normalize%
64/ 1/ +17%/ +6%
64/ 4/ +9%/ +17%
64/ 8/ +34%/ +21%
512/ 1/ +48%/ +40%
512/ 4/ +31%/ +20%
512/ 8/ +39%/ +22%
1024/ 1/ +158%/ +99%
1024/ 4/ +20%/ +11%
1024/ 8/ +40%/ +18%
2048/ 1/ +108%/ +74%
2048/ 4/ +21%/ +7%
2048/ 8/ +32%/ +14%
4096/ 1/ +94%/ +77%
4096/ 4/ +7%/ -6%
4096/ 8/ +9%/ -4%
16384/ 1/ +33%/ +9%
16384/ 4/ +10%/ -6%
16384/ 8/ +19%/ +2%
65535/ 1/ +15%/ -6%
65535/ 4/ +8%/ -9%
65535/ 8/ +14%/ 0%
Guest RX:
size/session/+thu%/+normalize%
64/ 1/ -3%/ -3%
64/ 4/ +4%/ +20%
64/ 8/ -1%/ -1%
512/ 1/ +20%/ +12%
512/ 4/ +1%/ +3%
512/ 8/ 0%/ -5%
1024/ 1/ +9%/ -2%
1024/ 4/ 0%/ +5%
1024/ 8/ +1%/ 0%
2048/ 1/ 0%/ +3%
2048/ 4/ -2%/ +3%
2048/ 8/ -1%/ -3%
4096/ 1/ -8%/ +3%
4096/ 4/ 0%/ +2%
4096/ 8/ 0%/ +5%
16384/ 1/ +3%/ 0%
16384/ 4/ +2%/ +2%
16384/ 8/ 0%/ +13%
65535/ 1/ 0%/ +3%
65535/ 4/ +2%/ -1%
65535/ 8/ +1%/ +14%
TCP_RR:
size/session/+thu%/+normalize%
1/ 1/ +8%/ -6%
1/ 50/ +18%/ +15%
1/ 100/ +22%/ +19%
1/ 200/ +25%/ +23%
64/ 1/ +2%/ -19%
64/ 50/ +46%/ +39%
64/ 100/ +47%/ +39%
64/ 200/ +50%/ +44%
512/ 1/ 0%/ -28%
512/ 50/ +50%/ +44%
512/ 100/ +53%/ +47%
512/ 200/ +51%/ +58%
1024/ 1/ +3%/ -14%
1024/ 50/ +45%/ +37%
1024/ 100/ +53%/ +49%
1024/ 200/ +48%/ +55%
Changes from V1:
- Add a comment for vhost_has_work() to explain why it could be
lockless
- Add param description for busyloop_timeout
- Split out the busy polling logic into a new helper
- Check and exit the loop when there's a pending signal
- Disable preemption during busy looping to make sure lock_clock() was
correctly used.
Todo:
- Make the busyloop timeout could be configure per VM through ioctl.
Please review.
Thanks
Jason Wang (2):
vhost: introduce vhost_has_work()
vhost_net: basic polling support
drivers/vhost/net.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++----
drivers/vhost/vhost.c | 7 +++++++
drivers/vhost/vhost.h | 1 +
3 files changed, 58 insertions(+), 4 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Andy Lutomirski @ 2015-10-28 22:51 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-s390, Joerg Roedel, KVM, Benjamin Herrenschmidt,
Sebastian Ott, linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, Andy Lutomirski, Paolo Bonzini,
David Woodhouse, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20151028175136-mutt-send-email-mst@redhat.com>
On Wed, Oct 28, 2015 at 9:12 AM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Wed, Oct 28, 2015 at 11:32:34PM +0900, David Woodhouse wrote:
>> > I don't have a problem with extending DMA API to address
>> > more usecases.
>>
>> No, this isn't an extension. This is fixing a bug, on certain platforms
>> where the DMA API has currently done the wrong thing.
>>
>> We have historically worked around that bug by introducing *another*
>> bug, which is not to *use* the DMA API in the virtio driver.
>>
>> Sure, we can co-ordinate those two bug-fixes. But let's not talk about
>> them as anything other than bug-fixes.
>
> It was pretty practical not to use it. All virtio devices at the time
> without exception bypassed the IOMMU, so it was a question of omitting a
> couple of function calls in virtio versus hacking on DMA implementation
> on multiple platforms. We have more policy options now, so I agree it's
> time to revisit this.
>
> But for me, the most important thing is that we do coordinate.
>
>> > > Drivers use DMA API. No more talky.
>> >
>> > Well for virtio they don't ATM. And 1:1 mapping makes perfect sense
>> > for the wast majority of users, so I can't switch them over
>> > until the DMA API actually addresses all existing usecases.
>>
>> That's still not your business; it's the platform's. And there are
>> hardware implementations of the virtio protocols on real PCI cards. And
>> we have the option of doing IOMMU translation for the virtio devices
>> even in a virtual machine. Just don't get involved.
>>
>> --
>> dwmw2
>>
>>
>
> I'm involved anyway, it's possible not to put all the code in the virtio
> subsystem in guest though. But I suspect we'll need to find a way for
> non-linux drivers within guest to work correctly too, and they might
> have trouble poking at things at the system level. So possibly virtio
> subsystem will have to tell platform "this device wants to bypass IOMMU"
> and then DMA API does the right thing.
>
After some discussion at KS, no one came up with an example where it's
necessary, and the patches to convert virtqueue to use the DMA API are
much nicer when they convert it unconditionally.
The two interesting cases we thought of were PPC and x86's emulated
Q35 IOMMU. PPC will look in to architecting a devicetree-based way to
indicate passthrough status and will add quirks for the existing
virtio devices. Everyone seems to agree that x86's emulated Q35 thing
is just buggy right now and should be taught to use the existing ACPI
mechanism for enumerating passthrough devices.
I'll send a new version of the series soon.
--Andy
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-28 16:12 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <1446042754.3405.237.camel@infradead.org>
On Wed, Oct 28, 2015 at 11:32:34PM +0900, David Woodhouse wrote:
> > I don't have a problem with extending DMA API to address
> > more usecases.
>
> No, this isn't an extension. This is fixing a bug, on certain platforms
> where the DMA API has currently done the wrong thing.
>
> We have historically worked around that bug by introducing *another*
> bug, which is not to *use* the DMA API in the virtio driver.
>
> Sure, we can co-ordinate those two bug-fixes. But let's not talk about
> them as anything other than bug-fixes.
It was pretty practical not to use it. All virtio devices at the time
without exception bypassed the IOMMU, so it was a question of omitting a
couple of function calls in virtio versus hacking on DMA implementation
on multiple platforms. We have more policy options now, so I agree it's
time to revisit this.
But for me, the most important thing is that we do coordinate.
> > > Drivers use DMA API. No more talky.
> >
> > Well for virtio they don't ATM. And 1:1 mapping makes perfect sense
> > for the wast majority of users, so I can't switch them over
> > until the DMA API actually addresses all existing usecases.
>
> That's still not your business; it's the platform's. And there are
> hardware implementations of the virtio protocols on real PCI cards. And
> we have the option of doing IOMMU translation for the virtio devices
> even in a virtual machine. Just don't get involved.
>
> --
> dwmw2
>
>
I'm involved anyway, it's possible not to put all the code in the virtio
subsystem in guest though. But I suspect we'll need to find a way for
non-linux drivers within guest to work correctly too, and they might
have trouble poking at things at the system level. So possibly virtio
subsystem will have to tell platform "this device wants to bypass IOMMU"
and then DMA API does the right thing.
I'll look into this after my vacation ~1.5 weeks from now.
--
MST
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: David Woodhouse @ 2015-10-28 14:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20151028161424-mutt-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 2156 bytes --]
On Wed, 2015-10-28 at 16:22 +0200, Michael S. Tsirkin wrote:
> On Wed, Oct 28, 2015 at 11:13:29PM +0900, David Woodhouse wrote:
> > On Wed, 2015-10-28 at 16:05 +0200, Michael S. Tsirkin wrote:
> > >
> > > Short answer - platforms need a way to discover, and express
> > > different
> > > security requirements of different devices.
> >
> > Sure. PLATFORMS need that. Do not let it go anywhere near your
> > device
> > drivers. Including the virtio drivers.
>
> But would there be any users of this outside the virtio subsystem?
> If no, maybe virtio core is a logical place to keep this.
Users of what? DMA API ops which basically do nothing? Sure — there are
*plenty* of cases where there isn't actually an IOMMU in active use and
the DMA API just returns the same address it was given.
Obviously that happens in platforms without an IOMMU, but it also
happens in cases where an IOMMU exists but is in passthrough mode, and
it also happens in cases where an IOMMU exists somewhere in the system
but only translates for *other* devices.
In all cases, drivers must just use the DMA API and *it* is responsible
for doing the right thing.
> I don't have a problem with extending DMA API to address
> more usecases.
No, this isn't an extension. This is fixing a bug, on certain platforms
where the DMA API has currently done the wrong thing.
We have historically worked around that bug by introducing *another*
bug, which is not to *use* the DMA API in the virtio driver.
Sure, we can co-ordinate those two bug-fixes. But let's not talk about
them as anything other than bug-fixes.
> > Drivers use DMA API. No more talky.
>
> Well for virtio they don't ATM. And 1:1 mapping makes perfect sense
> for the wast majority of users, so I can't switch them over
> until the DMA API actually addresses all existing usecases.
That's still not your business; it's the platform's. And there are
hardware implementations of the virtio protocols on real PCI cards. And
we have the option of doing IOMMU translation for the virtio devices
even in a virtual machine. Just don't get involved.
--
dwmw2
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-28 14:22 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <1446041609.3405.228.camel@infradead.org>
On Wed, Oct 28, 2015 at 11:13:29PM +0900, David Woodhouse wrote:
> On Wed, 2015-10-28 at 16:05 +0200, Michael S. Tsirkin wrote:
> >
> > Short answer - platforms need a way to discover, and express different
> > security requirements of different devices.
>
> Sure. PLATFORMS need that. Do not let it go anywhere near your device
> drivers. Including the virtio drivers.
But would there be any users of this outside the virtio subsystem?
If no, maybe virtio core is a logical place to keep this.
> > If they continue to lack that, we'll need a custom API in virtio,
> > and while this seems a bit less elegant, I would not see that as
> > the end of the world at all, there are not that many virtio drivers.
>
> No. If they continue to lack that, we fix them. This is a *platform*
> issue. The DMA API shall do the right thing. Do not second-guess it.
>
>
> (From the other mail)
I don't have a problem with extending DMA API to address
more usecases.
> > > > OK so I guess that means we should prefer a transport-specific
> > > > interface in virtio-pci then.
> > >
> > > Why?
> >
> > Because you said you are doing something device tree specific for
> > ARM, aren't you?
>
> Nonono. The ARM platform code might do that, and the DMA API on ARM
> *might* give you I/O virtual addresses that look a lot like the
> physical addresses you asked it to map. That's none of your business.
> Drivers use DMA API. No more talky.
Well for virtio they don't ATM. And 1:1 mapping makes perfect sense
for the wast majority of users, so I can't switch them over
until the DMA API actually addresses all existing usecases.
> --
> dwmw2
>
>
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: David Woodhouse @ 2015-10-28 14:13 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20151028155105-mutt-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 1159 bytes --]
On Wed, 2015-10-28 at 16:05 +0200, Michael S. Tsirkin wrote:
>
> Short answer - platforms need a way to discover, and express different
> security requirements of different devices.
Sure. PLATFORMS need that. Do not let it go anywhere near your device
drivers. Including the virtio drivers.
> If they continue to lack that, we'll need a custom API in virtio,
> and while this seems a bit less elegant, I would not see that as
> the end of the world at all, there are not that many virtio drivers.
No. If they continue to lack that, we fix them. This is a *platform*
issue. The DMA API shall do the right thing. Do not second-guess it.
(From the other mail)
> > > OK so I guess that means we should prefer a transport-specific
> > > interface in virtio-pci then.
> >
> > Why?
>
> Because you said you are doing something device tree specific for
> ARM, aren't you?
Nonono. The ARM platform code might do that, and the DMA API on ARM
*might* give you I/O virtual addresses that look a lot like the
physical addresses you asked it to map. That's none of your business.
Drivers use DMA API. No more talky.
--
dwmw2
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-28 14:07 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-s390, Joerg Roedel, KVM, Benjamin Herrenschmidt,
Sebastian Ott, linux-kernel, virtualization,
Christian Borntraeger, Andy Lutomirski, Paolo Bonzini,
Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <1446039476.3405.217.camel@infradead.org>
On Wed, Oct 28, 2015 at 10:37:56PM +0900, David Woodhouse wrote:
> On Wed, 2015-10-28 at 13:23 +0200, Michael S. Tsirkin wrote:
> > On Wed, Oct 28, 2015 at 05:36:53PM +0900, Benjamin Herrenschmidt
> > wrote:
> > > On Wed, 2015-10-28 at 16:40 +0900, Christian Borntraeger wrote:
> > > > We have discussed that at kernel summit. I will try to implement
> > > > a dummy dma_ops for
> > > > s390 that does 1:1 mapping and Ben will look into doing some
> > > > quirk to handle "old"
> > > > code in addition to also make it possible to mark devices as
> > > > iommu bypass (IIRC,
> > > > via device tree, Ben?)
> > >
> > > Something like that yes. I'll look into it when I'm back home.
> > >
> > > Cheers,
> > > Ben.
> >
> > OK so I guess that means we should prefer a transport-specific
> > interface in virtio-pci then.
>
> Why?
Because you said you are doing something device tree specific for ARM,
aren't you?
--
MST
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-28 14:05 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <1446039327.3405.216.camel@infradead.org>
On Wed, Oct 28, 2015 at 10:35:27PM +0900, David Woodhouse wrote:
> On Wed, 2015-10-28 at 13:35 +0200, Michael S. Tsirkin wrote:
> > E.g. on intel x86, there's an option iommu=pt which does the 1:1
> > thing for devices when used by kernel, but enables
> > the iommu if used by userspace/VMs.
>
> That's none of your business.
>
> You call the DMA API when you do DMA. That's all there is to it.
>
> If the IOMMU happens to be in passthrough mode, or your device happens
> to not to be routed through an IOMMU today, then I/O virtual address
> you get back from the DMA API will look a *lot* like the physical
> address you asked the DMA to map. You might think there's no IOMMU. We
> couldn't possibly comment.
>
> Use the DMA API. Always. Let the platform worry about whether it
> actually needs to *do* anything or not.
> --
> dwmw2
>
>
Short answer - platforms need a way to discover, and express different
security requirements of different devices. If they continue to lack
that, we'll need a custom API in virtio, and while this seems a bit less
elegant, I would not see that as the end of the world at all, there are
not that many virtio drivers.
And hey - that's just an internal API. We can change it later at a whim.
Long answer - PV is weird. It's not always the same as real hardware.
For PV, it's generally hypervisor doing writes into memory.
If it's monolitic with device emulation in same memory space as the
hypervisor (e.g. in the case of the current QEMU, or using vhost in host
kernel), then you gain *no security* by "restricting" it by means of the
IOMMU - the IOMMU is part of the same hypervisor.
If it is modular with device emulation in a separate memory space (e.g.
in case of Xen, or vhost-user in modern QEMU) then you do gain security:
the part emulating the IOMMU limits the part doing DMA.
In both cases for assigned devices, it is always modular in a sense, so
you do gain security since that is restricted by the hardware IOMMU.
The way things are set up at the moment, it's mostly global,
with iommu=pt on intel being a kind of exception.
We need host/guest and API interfaces that are more nuanced than that.
--
MST
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: David Woodhouse @ 2015-10-28 13:37 UTC (permalink / raw)
To: Michael S. Tsirkin, Benjamin Herrenschmidt
Cc: linux-s390, Joerg Roedel, KVM, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20151028132115-mutt-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 762 bytes --]
On Wed, 2015-10-28 at 13:23 +0200, Michael S. Tsirkin wrote:
> On Wed, Oct 28, 2015 at 05:36:53PM +0900, Benjamin Herrenschmidt
> wrote:
> > On Wed, 2015-10-28 at 16:40 +0900, Christian Borntraeger wrote:
> > > We have discussed that at kernel summit. I will try to implement
> > > a dummy dma_ops for
> > > s390 that does 1:1 mapping and Ben will look into doing some
> > > quirk to handle "old"
> > > code in addition to also make it possible to mark devices as
> > > iommu bypass (IIRC,
> > > via device tree, Ben?)
> >
> > Something like that yes. I'll look into it when I'm back home.
> >
> > Cheers,
> > Ben.
>
> OK so I guess that means we should prefer a transport-specific
> interface in virtio-pci then.
Why?
--
dwmw2
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: David Woodhouse @ 2015-10-28 13:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <20151028132331-mutt-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 762 bytes --]
On Wed, 2015-10-28 at 13:35 +0200, Michael S. Tsirkin wrote:
> E.g. on intel x86, there's an option iommu=pt which does the 1:1
> thing for devices when used by kernel, but enables
> the iommu if used by userspace/VMs.
That's none of your business.
You call the DMA API when you do DMA. That's all there is to it.
If the IOMMU happens to be in passthrough mode, or your device happens
to not to be routed through an IOMMU today, then I/O virtual address
you get back from the DMA API will look a *lot* like the physical
address you asked the DMA to map. You might think there's no IOMMU. We
couldn't possibly comment.
Use the DMA API. Always. Let the platform worry about whether it
actually needs to *do* anything or not.
--
dwmw2
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-28 11:35 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <1446019787.3405.203.camel@infradead.org>
On Wed, Oct 28, 2015 at 05:09:47PM +0900, David Woodhouse wrote:
> On Wed, 2015-10-28 at 16:40 +0900, Christian Borntraeger wrote:
> > Am 28.10.2015 um 16:17 schrieb Michael S. Tsirkin:
> > > On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> > > > This switches virtio to use the DMA API unconditionally. I'm sure
> > > > it breaks things, but it seems to work on x86 using virtio-pci, with
> > > > and without Xen, and using both the modern 1.0 variant and the
> > > > legacy variant.
> > >
> > > I'm very glad to see work on this making progress.
> > >
> > > I suspect we'll have to find a way to make this optional though, and
> > > keep doing the non-DMA API thing with old devices. And I've been
> > > debating with myself whether a pci specific thing or a feature bit is
> > > preferable.
> > >
> >
> > We have discussed that at kernel summit. I will try to implement a dummy dma_ops for
> > s390 that does 1:1 mapping and Ben will look into doing some quirk to handle "old"
> > code in addition to also make it possible to mark devices as iommu bypass (IIRC,
> > via device tree, Ben?)
>
> Right. You never eschew the DMA API in the *driver* — you just expect
> the DMA API to do the right thing for devices which don't need
> translation (with platforms using per-device dma_ops and generally
> getting their act together).
> We're pushing that on the platforms where it's currently an issue,
> including Power, SPARC and S390.
>
> --
> dwmw2
>
>
Well APIs are just that - internal kernel APIs.
If the only user of an API is virtio, we can strick the
code in virtio.h just as well.
I think controlling this dynamically and not statically
in e.g. devicetree is important though.
E.g. on intel x86, there's an option iommu=pt which does the 1:1
thing for devices when used by kernel, but enables
the iommu if used by userspace/VMs.
Something like this would be needed for other platforms IMHO.
And given that
1. virtio seems the only user so far
2. supporting this per device seems like something that
might become useful in the future
maybe we'd better make this part of virtio transports.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-28 11:23 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linux-s390, Joerg Roedel, KVM, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Andy Lutomirski,
Paolo Bonzini, dwmw2, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <1446021413.1856.28.camel@kernel.crashing.org>
On Wed, Oct 28, 2015 at 05:36:53PM +0900, Benjamin Herrenschmidt wrote:
> On Wed, 2015-10-28 at 16:40 +0900, Christian Borntraeger wrote:
> > We have discussed that at kernel summit. I will try to implement a dummy dma_ops for
> > s390 that does 1:1 mapping and Ben will look into doing some quirk to handle "old"
> > code in addition to also make it possible to mark devices as iommu bypass (IIRC,
> > via device tree, Ben?)
>
> Something like that yes. I'll look into it when I'm back home.
>
> Cheers,
> Ben.
OK so I guess that means we should prefer a transport-specific
interface in virtio-pci then.
--
MST
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Benjamin Herrenschmidt @ 2015-10-28 8:36 UTC (permalink / raw)
To: Christian Borntraeger, Michael S. Tsirkin, Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, Sebastian Ott, linux-kernel,
virtualization, Martin Schwidefsky, Paolo Bonzini, dwmw2,
Christoph Hellwig
In-Reply-To: <56307BD1.6010806@de.ibm.com>
On Wed, 2015-10-28 at 16:40 +0900, Christian Borntraeger wrote:
> We have discussed that at kernel summit. I will try to implement a dummy dma_ops for
> s390 that does 1:1 mapping and Ben will look into doing some quirk to handle "old"
> code in addition to also make it possible to mark devices as iommu bypass (IIRC,
> via device tree, Ben?)
Something like that yes. I'll look into it when I'm back home.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: David Woodhouse @ 2015-10-28 8:09 UTC (permalink / raw)
To: Christian Borntraeger, Michael S. Tsirkin, Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Martin Schwidefsky, Paolo Bonzini,
Christoph Hellwig
In-Reply-To: <56307BD1.6010806@de.ibm.com>
[-- Attachment #1.1: Type: text/plain, Size: 1390 bytes --]
On Wed, 2015-10-28 at 16:40 +0900, Christian Borntraeger wrote:
> Am 28.10.2015 um 16:17 schrieb Michael S. Tsirkin:
> > On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> > > This switches virtio to use the DMA API unconditionally. I'm sure
> > > it breaks things, but it seems to work on x86 using virtio-pci, with
> > > and without Xen, and using both the modern 1.0 variant and the
> > > legacy variant.
> >
> > I'm very glad to see work on this making progress.
> >
> > I suspect we'll have to find a way to make this optional though, and
> > keep doing the non-DMA API thing with old devices. And I've been
> > debating with myself whether a pci specific thing or a feature bit is
> > preferable.
> >
>
> We have discussed that at kernel summit. I will try to implement a dummy dma_ops for
> s390 that does 1:1 mapping and Ben will look into doing some quirk to handle "old"
> code in addition to also make it possible to mark devices as iommu bypass (IIRC,
> via device tree, Ben?)
Right. You never eschew the DMA API in the *driver* — you just expect
the DMA API to do the right thing for devices which don't need
translation (with platforms using per-device dma_ops and generally
getting their act together).
We're pushing that on the platforms where it's currently an issue,
including Power, SPARC and S390.
--
dwmw2
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Christian Borntraeger @ 2015-10-28 7:40 UTC (permalink / raw)
To: Michael S. Tsirkin, Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Martin Schwidefsky, Paolo Bonzini, dwmw2,
Christoph Hellwig
In-Reply-To: <20151028091208-mutt-send-email-mst@redhat.com>
Am 28.10.2015 um 16:17 schrieb Michael S. Tsirkin:
> On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
>> This switches virtio to use the DMA API unconditionally. I'm sure
>> it breaks things, but it seems to work on x86 using virtio-pci, with
>> and without Xen, and using both the modern 1.0 variant and the
>> legacy variant.
>
> I'm very glad to see work on this making progress.
>
> I suspect we'll have to find a way to make this optional though, and
> keep doing the non-DMA API thing with old devices. And I've been
> debating with myself whether a pci specific thing or a feature bit is
> preferable.
>
We have discussed that at kernel summit. I will try to implement a dummy dma_ops for
s390 that does 1:1 mapping and Ben will look into doing some quirk to handle "old"
code in addition to also make it possible to mark devices as iommu bypass (IIRC,
via device tree, Ben?)
Christian
> Thoughts?
>
>> Changes from v2:
>> - Fix really embarrassing bug. This version actually works.
>>
>> Changes from v1:
>> - Fix an endian conversion error causing a BUG to hit.
>> - Fix a DMA ordering issue (swiotlb=force works now).
>> - Minor cleanups.
>>
>> Andy Lutomirski (3):
>> virtio_net: Stop doing DMA from the stack
>> virtio_ring: Support DMA APIs
>> virtio_pci: Use the DMA API
>>
>> drivers/net/virtio_net.c | 53 +++++++----
>> drivers/virtio/Kconfig | 2 +-
>> drivers/virtio/virtio_pci_common.h | 3 +-
>> drivers/virtio/virtio_pci_legacy.c | 19 +++-
>> drivers/virtio/virtio_pci_modern.c | 34 +++++--
>> drivers/virtio/virtio_ring.c | 187 ++++++++++++++++++++++++++++++-------
>> tools/virtio/linux/dma-mapping.h | 17 ++++
>> 7 files changed, 246 insertions(+), 69 deletions(-)
>> create mode 100644 tools/virtio/linux/dma-mapping.h
>>
>> --
>> 2.4.3
>
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Michael S. Tsirkin @ 2015-10-28 7:17 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Paolo Bonzini, dwmw2,
Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <cover.1446014204.git.luto@kernel.org>
On Tue, Oct 27, 2015 at 11:38:57PM -0700, Andy Lutomirski wrote:
> This switches virtio to use the DMA API unconditionally. I'm sure
> it breaks things, but it seems to work on x86 using virtio-pci, with
> and without Xen, and using both the modern 1.0 variant and the
> legacy variant.
I'm very glad to see work on this making progress.
I suspect we'll have to find a way to make this optional though, and
keep doing the non-DMA API thing with old devices. And I've been
debating with myself whether a pci specific thing or a feature bit is
preferable.
Thoughts?
> Changes from v2:
> - Fix really embarrassing bug. This version actually works.
>
> Changes from v1:
> - Fix an endian conversion error causing a BUG to hit.
> - Fix a DMA ordering issue (swiotlb=force works now).
> - Minor cleanups.
>
> Andy Lutomirski (3):
> virtio_net: Stop doing DMA from the stack
> virtio_ring: Support DMA APIs
> virtio_pci: Use the DMA API
>
> drivers/net/virtio_net.c | 53 +++++++----
> drivers/virtio/Kconfig | 2 +-
> drivers/virtio/virtio_pci_common.h | 3 +-
> drivers/virtio/virtio_pci_legacy.c | 19 +++-
> drivers/virtio/virtio_pci_modern.c | 34 +++++--
> drivers/virtio/virtio_ring.c | 187 ++++++++++++++++++++++++++++++-------
> tools/virtio/linux/dma-mapping.h | 17 ++++
> 7 files changed, 246 insertions(+), 69 deletions(-)
> create mode 100644 tools/virtio/linux/dma-mapping.h
>
> --
> 2.4.3
^ permalink raw reply
* Re: [PATCH v3 0/3] virtio DMA API core stuff
From: Andy Lutomirski @ 2015-10-28 7:09 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-s390, Joerg Roedel, KVM, Michael S. Tsirkin,
Benjamin Herrenschmidt, Sebastian Ott,
linux-kernel@vger.kernel.org, Linux Virtualization,
Christian Borntraeger, Andy Lutomirski, Paolo Bonzini,
Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <1446015204.3405.186.camel@infradead.org>
On Tue, Oct 27, 2015 at 11:53 PM, David Woodhouse <dwmw2@infradead.org> wrote:
> On Tue, 2015-10-27 at 23:38 -0700, Andy Lutomirski wrote:
>>
>> Changes from v2:
>> - Fix really embarrassing bug. This version actually works.
>
> So embarrassing you didn't want to tell us what it was? ...
Shhh, it's a secret!
I somehow managed to test-boot a different kernel than I thought I was booting.
>
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -292,7 +292,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
> vq, desc, total_sg * sizeof(struct vring_desc),
> DMA_TO_DEVICE);
>
> - if (vring_mapping_error(vq, vq->vring.desc[head].addr))
> + if (vring_mapping_error(vq, addr))
> goto unmap_release;
>
> vq->vring.desc[head].flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_INDIRECT);
>
> That wasn't going to be the reason for Christian's failure, was it?
>
Not obviously, but it's possible. Now that I'm staring at it, I have
some more big-endian issues, so there'll be a v4. I'll also play with
Michael's thing. Expect a long delay, though -- my flight's about to
leave.
The readme notwithstanding, virtme (https://github.com/amluto/virtme)
actually has s390x support, so I can try to debug when I get home.
I'm not about to try doing this on a laptop :)
--Andy
^ permalink raw reply
* Re: [PATCH v3 1/3] virtio_net: Stop doing DMA from the stack
From: Michael S. Tsirkin @ 2015-10-28 7:08 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-s390, Joerg Roedel, KVM, benh, Sebastian Ott, linux-kernel,
virtualization, Christian Borntraeger, Paolo Bonzini,
Andy Lutomirski, dwmw2, Christoph Hellwig, Martin Schwidefsky
In-Reply-To: <a2b5cd8102594565dca91e9ed665ae2fff5367bb.1446014204.git.luto@kernel.org>
On Tue, Oct 27, 2015 at 11:38:58PM -0700, Andy Lutomirski wrote:
> From: Andy Lutomirski <luto@amacapital.net>
>
> Once virtio starts using the DMA API, we won't be able to safely DMA
> from the stack. virtio-net does a couple of config DMA requests
> from small stack buffers -- switch to using dynamically-allocated
> memory.
>
> This should have no effect on any performance-critical code paths.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Reviewed-by: Joerg Roedel <jroedel@suse.de>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
Same issues as v2 (I only saw v3 now).
I've proposed an alternative patch.
> ---
>
> Hi Michael and DaveM-
>
> This is a prerequisite for the virtio DMA fixing project. It works
> as a standalone patch, though. Would it make sense to apply it to
> an appropriate networking tree now?
>
> (This is unchanged from v2.)
>
> drivers/net/virtio_net.c | 53 ++++++++++++++++++++++++++++++++----------------
> 1 file changed, 36 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index d8838dedb7a4..4f10f8a58811 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -976,31 +976,43 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> struct scatterlist *out)
> {
> struct scatterlist *sgs[4], hdr, stat;
> - struct virtio_net_ctrl_hdr ctrl;
> - virtio_net_ctrl_ack status = ~0;
> +
> + struct {
> + struct virtio_net_ctrl_hdr ctrl;
> + virtio_net_ctrl_ack status;
> + } *buf;
> +
> unsigned out_num = 0, tmp;
> + bool ret;
>
> /* Caller should know better */
> BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ));
>
> - ctrl.class = class;
> - ctrl.cmd = cmd;
> + buf = kmalloc(sizeof(*buf), GFP_ATOMIC);
> + if (!buf)
> + return false;
> + buf->status = ~0;
> +
> + buf->ctrl.class = class;
> + buf->ctrl.cmd = cmd;
> /* Add header */
> - sg_init_one(&hdr, &ctrl, sizeof(ctrl));
> + sg_init_one(&hdr, &buf->ctrl, sizeof(buf->ctrl));
> sgs[out_num++] = &hdr;
>
> if (out)
> sgs[out_num++] = out;
>
> /* Add return status. */
> - sg_init_one(&stat, &status, sizeof(status));
> + sg_init_one(&stat, &buf->status, sizeof(buf->status));
> sgs[out_num] = &stat;
>
> BUG_ON(out_num + 1 > ARRAY_SIZE(sgs));
> virtqueue_add_sgs(vi->cvq, sgs, out_num, 1, vi, GFP_ATOMIC);
>
> - if (unlikely(!virtqueue_kick(vi->cvq)))
> - return status == VIRTIO_NET_OK;
> + if (unlikely(!virtqueue_kick(vi->cvq))) {
> + ret = (buf->status == VIRTIO_NET_OK);
> + goto out;
> + }
>
> /* Spin for a response, the kick causes an ioport write, trapping
> * into the hypervisor, so the request should be handled immediately.
> @@ -1009,7 +1021,11 @@ static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
> !virtqueue_is_broken(vi->cvq))
> cpu_relax();
>
> - return status == VIRTIO_NET_OK;
> + ret = (buf->status == VIRTIO_NET_OK);
> +
> +out:
> + kfree(buf);
> + return ret;
> }
>
> static int virtnet_set_mac_address(struct net_device *dev, void *p)
> @@ -1151,7 +1167,7 @@ static void virtnet_set_rx_mode(struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> struct scatterlist sg[2];
> - u8 promisc, allmulti;
> + u8 *cmdbyte;
> struct virtio_net_ctrl_mac *mac_data;
> struct netdev_hw_addr *ha;
> int uc_count;
> @@ -1163,22 +1179,25 @@ static void virtnet_set_rx_mode(struct net_device *dev)
> if (!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_RX))
> return;
>
> - promisc = ((dev->flags & IFF_PROMISC) != 0);
> - allmulti = ((dev->flags & IFF_ALLMULTI) != 0);
> + cmdbyte = kmalloc(sizeof(*cmdbyte), GFP_ATOMIC);
> + if (!cmdbyte)
> + return;
>
> - sg_init_one(sg, &promisc, sizeof(promisc));
> + sg_init_one(sg, cmdbyte, sizeof(*cmdbyte));
>
> + *cmdbyte = ((dev->flags & IFF_PROMISC) != 0);
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> VIRTIO_NET_CTRL_RX_PROMISC, sg))
> dev_warn(&dev->dev, "Failed to %sable promisc mode.\n",
> - promisc ? "en" : "dis");
> -
> - sg_init_one(sg, &allmulti, sizeof(allmulti));
> + *cmdbyte ? "en" : "dis");
>
> + *cmdbyte = ((dev->flags & IFF_ALLMULTI) != 0);
> if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_RX,
> VIRTIO_NET_CTRL_RX_ALLMULTI, sg))
> dev_warn(&dev->dev, "Failed to %sable allmulti mode.\n",
> - allmulti ? "en" : "dis");
> + *cmdbyte ? "en" : "dis");
> +
> + kfree(cmdbyte);
>
> uc_count = netdev_uc_count(dev);
> mc_count = netdev_mc_count(dev);
> --
> 2.4.3
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox