* [PATCH net V2 2/4] vhost_net: switch to use mutex_trylock() in vhost_net_busy_poll()
From: Jason Wang @ 2018-12-12 10:08 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20181212100819.21295-1-jasowang@redhat.com>
We used to hold the mutex of paired virtqueue in
vhost_net_busy_poll(). But this will results an inconsistent lock
order which may cause deadlock if we try to bring back the protection
of device IOTLB with vq mutex that requires to hold mutex of all
virtqueues at the same time.
Fix this simply by switching to use mutex_trylock(), when fail just
skip the busy polling. This can happen when device IOTLB is under
updating which should be rare.
Fixes: commit 78139c94dc8c ("net: vhost: lock the vqs one by one")
Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/net.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index ab11b2bee273..ad7a6f475a44 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -513,7 +513,13 @@ static void vhost_net_busy_poll(struct vhost_net *net,
struct socket *sock;
struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
- mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
+ /* Try to hold the vq mutex of the paired virtqueue. We can't
+ * use mutex_lock() here since we could not guarantee a
+ * consistenet lock ordering.
+ */
+ if (!mutex_trylock(&vq->mutex))
+ return;
+
vhost_disable_notify(&net->dev, vq);
sock = rvq->private_data;
--
2.17.1
^ permalink raw reply related
* [PATCH net V2 1/4] vhost: make sure used idx is seen before log in vhost_add_used_n()
From: Jason Wang @ 2018-12-12 10:08 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20181212100819.21295-1-jasowang@redhat.com>
We miss a write barrier that guarantees used idx is updated and seen
before log. This will let userspace sync and copy used ring before
used idx is update. Fix this by adding a barrier before log_write().
Fixes: 8dd014adfea6f ("vhost-net: mergeable buffers support")
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
drivers/vhost/vhost.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 6b98d8e3a5bf..5915f240275a 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2220,6 +2220,8 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
return -EFAULT;
}
if (unlikely(vq->log_used)) {
+ /* Make sure used idx is seen before log. */
+ smp_wmb();
/* Log used index update. */
log_write(vq->log_base,
vq->log_addr + offsetof(struct vring_used, idx),
--
2.17.1
^ permalink raw reply related
* [PATCH net V2 0/4] Fix various issue of vhost
From: Jason Wang @ 2018-12-12 10:08 UTC (permalink / raw)
To: mst, jasowang, kvm, virtualization, netdev, linux-kernel
Hi:
This series tries to fix various issues of vhost:
- Patch 1 adds a missing write barrier between used idx updating and
logging.
- Patch 2-3 brings back the protection of device IOTLB through vq
mutex, this fixes possible use after free in device IOTLB entries.
- Patch 4-7 fixes the diry page logging when device IOTLB is
enabled. We should done through GPA instead of GIOVA, this was done
through intorudce HVA->GPA reverse mapping and convert HVA to GPA
during logging dirty pages.
Please consider them for -stable.
Thanks
Changes from V1:
- silent compiler warning for 32bit.
- use mutex_trylock() on slowpath instead of mutex_lock() even on fast
path.
Jason Wang (4):
vhost: make sure used idx is seen before log in vhost_add_used_n()
vhost_net: switch to use mutex_trylock() in vhost_net_busy_poll()
Revert "net: vhost: lock the vqs one by one"
vhost: log dirty page correctly
drivers/vhost/net.c | 11 ++++-
drivers/vhost/vhost.c | 102 ++++++++++++++++++++++++++++++++++--------
drivers/vhost/vhost.h | 3 +-
3 files changed, 95 insertions(+), 21 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH] VSOCK: Send reset control packet when socket is partially bound
From: Jorgen Hansen @ 2018-12-12 9:38 UTC (permalink / raw)
To: netdev, linux-kernel, virtualization
Cc: pv-drivers, gregkh, davem, Jorgen Hansen
If a server side socket is bound to an address, but not in the listening
state yet, incoming connection requests should receive a reset control
packet in response. However, the function used to send the reset
silently drops the reset packet if the sending socket isn't bound
to a remote address (as is the case for a bound socket not yet in
the listening state). This change fixes this by using the src
of the incoming packet as destination for the reset packet in
this case.
Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
Reviewed-by: Adit Ranadive <aditr@vmware.com>
Reviewed-by: Vishnu Dasa <vdasa@vmware.com>
Signed-off-by: Jorgen Hansen <jhansen@vmware.com>
---
net/vmw_vsock/vmci_transport.c | 67 +++++++++++++++++++++++++++++++-----------
1 file changed, 50 insertions(+), 17 deletions(-)
diff --git a/net/vmw_vsock/vmci_transport.c b/net/vmw_vsock/vmci_transport.c
index 0ae3614..402d84e 100644
--- a/net/vmw_vsock/vmci_transport.c
+++ b/net/vmw_vsock/vmci_transport.c
@@ -264,6 +264,31 @@ vmci_transport_send_control_pkt_bh(struct sockaddr_vm *src,
}
static int
+vmci_transport_alloc_send_control_pkt(struct sockaddr_vm *src,
+ struct sockaddr_vm *dst,
+ enum vmci_transport_packet_type type,
+ u64 size,
+ u64 mode,
+ struct vmci_transport_waiting_info *wait,
+ u16 proto,
+ struct vmci_handle handle)
+{
+ struct vmci_transport_packet *pkt;
+ int err;
+
+ pkt = kmalloc(sizeof(*pkt), GFP_KERNEL);
+ if (!pkt)
+ return -ENOMEM;
+
+ err = __vmci_transport_send_control_pkt(pkt, src, dst, type, size,
+ mode, wait, proto, handle,
+ true);
+ kfree(pkt);
+
+ return err;
+}
+
+static int
vmci_transport_send_control_pkt(struct sock *sk,
enum vmci_transport_packet_type type,
u64 size,
@@ -272,9 +297,7 @@ vmci_transport_send_control_pkt(struct sock *sk,
u16 proto,
struct vmci_handle handle)
{
- struct vmci_transport_packet *pkt;
struct vsock_sock *vsk;
- int err;
vsk = vsock_sk(sk);
@@ -284,17 +307,10 @@ vmci_transport_send_control_pkt(struct sock *sk,
if (!vsock_addr_bound(&vsk->remote_addr))
return -EINVAL;
- pkt = kmalloc(sizeof(*pkt), GFP_KERNEL);
- if (!pkt)
- return -ENOMEM;
-
- err = __vmci_transport_send_control_pkt(pkt, &vsk->local_addr,
- &vsk->remote_addr, type, size,
- mode, wait, proto, handle,
- true);
- kfree(pkt);
-
- return err;
+ return vmci_transport_alloc_send_control_pkt(&vsk->local_addr,
+ &vsk->remote_addr,
+ type, size, mode,
+ wait, proto, handle);
}
static int vmci_transport_send_reset_bh(struct sockaddr_vm *dst,
@@ -312,12 +328,29 @@ static int vmci_transport_send_reset_bh(struct sockaddr_vm *dst,
static int vmci_transport_send_reset(struct sock *sk,
struct vmci_transport_packet *pkt)
{
+ struct sockaddr_vm dst;
+ struct sockaddr_vm *dst_ptr;
+ struct vsock_sock *vsk;
+
if (pkt->type == VMCI_TRANSPORT_PACKET_TYPE_RST)
return 0;
- return vmci_transport_send_control_pkt(sk,
- VMCI_TRANSPORT_PACKET_TYPE_RST,
- 0, 0, NULL, VSOCK_PROTO_INVALID,
- VMCI_INVALID_HANDLE);
+
+ vsk = vsock_sk(sk);
+
+ if (!vsock_addr_bound(&vsk->local_addr))
+ return -EINVAL;
+
+ if (vsock_addr_bound(&vsk->remote_addr)) {
+ dst_ptr = &vsk->remote_addr;
+ } else {
+ vsock_addr_init(&dst, pkt->dg.src.context,
+ pkt->src_port);
+ dst_ptr = &dst;
+ }
+ return vmci_transport_alloc_send_control_pkt(&vsk->local_addr, dst_ptr,
+ VMCI_TRANSPORT_PACKET_TYPE_RST,
+ 0, 0, NULL, VSOCK_PROTO_INVALID,
+ VMCI_INVALID_HANDLE);
}
static int vmci_transport_send_negotiate(struct sock *sk, size_t size)
--
2.6.2
^ permalink raw reply related
* [PATCH v2 5/5] VSOCK: batch sending rx buffer to increase bandwidth
From: jiangyiwen @ 2018-12-12 9:35 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang
Cc: netdev, kvm, virtualization
Batch sending rx buffer can improve total bandwidth.
Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
---
drivers/vhost/vsock.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 9600133..a4bf0a1 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -151,9 +151,11 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
struct vhost_virtqueue *vq)
{
struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
- bool added = false;
bool restart_tx = false;
size_t vsock_hlen;
+ int batch_count = 0;
+
+#define VHOST_VSOCK_BATCH 16
mutex_lock(&vq->mutex);
@@ -194,8 +196,9 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
list_del_init(&pkt->list);
spin_unlock_bh(&vsock->send_pkt_list_lock);
- headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
- &in, likely(vsock->mergeable) ? UIO_MAXIOV : 1);
+ headcount = get_rx_bufs(vq, vq->heads + batch_count,
+ vsock_hlen + pkt->len, &in,
+ likely(vsock->mergeable) ? UIO_MAXIOV : 1);
if (headcount <= 0) {
spin_lock_bh(&vsock->send_pkt_list_lock);
list_add(&pkt->list, &vsock->send_pkt_list);
@@ -238,8 +241,12 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
remain_len -= tmp_len;
}
- vhost_add_used_n(vq, vq->heads, headcount);
- added = true;
+ batch_count += headcount;
+ if (batch_count > VHOST_VSOCK_BATCH) {
+ vhost_add_used_and_signal_n(&vsock->dev, vq,
+ vq->heads, batch_count);
+ batch_count = 0;
+ }
if (pkt->reply) {
int val;
@@ -258,8 +265,11 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
virtio_transport_free_pkt(pkt);
}
- if (added)
- vhost_signal(&vsock->dev, vq);
+
+ if (batch_count) {
+ vhost_add_used_and_signal_n(&vsock->dev, vq,
+ vq->heads, batch_count);
+ }
out:
mutex_unlock(&vq->mutex);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 4/5] VSOCK: increase send pkt len in mergeable mode to improve performance
From: jiangyiwen @ 2018-12-12 9:34 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang
Cc: netdev, kvm, virtualization
Since VSOCK already support mergeable rx buffer, so it can
implement the balance with performance and guest memory,
we can increase the sent pkt len to improve performance.
And in order to be compatible with old version, so we still
send max default rx buf size once.
Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
---
drivers/vhost/vsock.c | 33 ++++++++++++++++++++++++++++-----
include/linux/virtio_vsock.h | 3 +++
net/vmw_vsock/virtio_transport.c | 18 ++++++++++++++++++
net/vmw_vsock/virtio_transport_common.c | 7 ++++---
4 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index c7ab0dd..9600133 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -44,6 +44,8 @@ struct vhost_vsock {
atomic_t queued_replies;
u32 guest_cid;
+
+ bool mergeable;
};
static u32 vhost_transport_get_local_cid(void)
@@ -151,7 +153,6 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
bool added = false;
bool restart_tx = false;
- int mergeable;
size_t vsock_hlen;
mutex_lock(&vq->mutex);
@@ -159,12 +160,11 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
if (!vq->private_data)
goto out;
- mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
/*
* Guest fill page for rx vq in mergeable case, so it will not
* allocate pkt structure, we should reserve size of pkt in advance.
*/
- if (likely(mergeable))
+ if (likely(vsock->mergeable))
vsock_hlen = sizeof(struct virtio_vsock_pkt);
else
vsock_hlen = sizeof(struct virtio_vsock_hdr);
@@ -195,7 +195,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
spin_unlock_bh(&vsock->send_pkt_list_lock);
headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
- &in, likely(mergeable) ? UIO_MAXIOV : 1);
+ &in, likely(vsock->mergeable) ? UIO_MAXIOV : 1);
if (headcount <= 0) {
spin_lock_bh(&vsock->send_pkt_list_lock);
list_add(&pkt->list, &vsock->send_pkt_list);
@@ -214,7 +214,7 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
len = iov_length(&vq->iov[out], in);
iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
- if (likely(mergeable))
+ if (likely(vsock->mergeable))
pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
nbytes = copy_to_iter(&pkt->hdr, vsock_hlen, &iov_iter);
if (nbytes != vsock_hlen) {
@@ -303,6 +303,22 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
return len;
}
+static u32
+vhost_transport_get_max_pkt_len(u32 guest_cid)
+{
+ int len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
+ struct vhost_vsock *vsock;
+
+ vsock = vhost_vsock_get(guest_cid);
+ if (!vsock)
+ return len;
+
+ if (vsock->mergeable)
+ len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE;
+
+ return len;
+}
+
static int
vhost_transport_cancel_pkt(struct vsock_sock *vsk)
{
@@ -602,6 +618,8 @@ static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
atomic_set(&vsock->queued_replies, 0);
+ vsock->mergeable = false;
+
vqs[VSOCK_VQ_TX] = &vsock->vqs[VSOCK_VQ_TX];
vqs[VSOCK_VQ_RX] = &vsock->vqs[VSOCK_VQ_RX];
vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick;
@@ -726,6 +744,9 @@ static int vhost_vsock_set_features(struct vhost_vsock *vsock, u64 features)
return -EFAULT;
}
+ if (features & (1 << VIRTIO_VSOCK_F_MRG_RXBUF))
+ vsock->mergeable = true;
+
for (i = 0; i < ARRAY_SIZE(vsock->vqs); i++) {
vq = &vsock->vqs[i];
mutex_lock(&vq->mutex);
@@ -848,6 +869,8 @@ static long vhost_vsock_dev_compat_ioctl(struct file *f, unsigned int ioctl,
},
.send_pkt = vhost_transport_send_pkt,
+
+ .max_pkt_len = vhost_transport_get_max_pkt_len,
};
static int __init vhost_vsock_init(void)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index 734eeed..ad95319 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -82,6 +82,9 @@ struct virtio_transport {
/* Takes ownership of the packet */
int (*send_pkt)(struct virtio_vsock_pkt *pkt);
+
+ /* Get max pkt len, cid only be used for Host */
+ u32 (*max_pkt_len)(u32 cid);
};
ssize_t
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 148b58a..809085a 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -222,6 +222,22 @@ static int virtio_transport_send_pkt_loopback(struct virtio_vsock *vsock,
return len;
}
+static u32
+virtio_transport_get_max_pkt_len(u32 cid)
+{
+ int len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
+ struct virtio_vsock *vsock;
+
+ vsock = virtio_vsock_get();
+ if (!vsock)
+ return len;
+
+ if (vsock->mergeable)
+ len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE;
+
+ return len;
+}
+
static int
virtio_transport_cancel_pkt(struct vsock_sock *vsk)
{
@@ -675,6 +691,8 @@ static void virtio_vsock_rx_done(struct virtqueue *vq)
},
.send_pkt = virtio_transport_send_pkt,
+
+ .max_pkt_len = virtio_transport_get_max_pkt_len,
};
static int virtio_vsock_probe(struct virtio_device *vdev)
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 123a8b6..cc14ace 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -159,7 +159,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
u32 src_cid, src_port, dst_cid, dst_port;
struct virtio_vsock_sock *vvs;
struct virtio_vsock_pkt *pkt;
- u32 pkt_len = info->pkt_len;
+ u32 pkt_len;
+ u32 max_pkt_len;
src_cid = vm_sockets_get_local_cid();
src_port = vsk->local_addr.svm_port;
@@ -174,8 +175,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock *vsk,
vvs = vsk->trans;
/* we can send less than pkt_len bytes */
- if (pkt_len > VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE)
- pkt_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
+ max_pkt_len = virtio_transport_get_ops()->max_pkt_len(dst_cid);
+ pkt_len = min(max_pkt_len, info->pkt_len);
/* virtio_transport_get_credit might return less than pkt_len credit */
pkt_len = virtio_transport_get_credit(vvs, pkt_len);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 3/5] VSOCK: support receive mergeable rx buffer in guest
From: jiangyiwen @ 2018-12-12 9:31 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang
Cc: netdev, kvm, virtualization
Guest receive mergeable rx buffer, it can merge
scatter rx buffer into a big buffer and then copy
to user space.
In addition, it also use iovec to replace buf in struct
virtio_vsock_pkt, keep tx and rx consistency. The only
difference is now tx still uses a segment of continuous
physical memory to implement.
Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
---
drivers/vhost/vsock.c | 31 +++++++---
include/linux/virtio_vsock.h | 6 +-
net/vmw_vsock/virtio_transport.c | 105 ++++++++++++++++++++++++++++----
net/vmw_vsock/virtio_transport_common.c | 59 ++++++++++++++----
4 files changed, 166 insertions(+), 35 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index dc52b0f..c7ab0dd 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -179,6 +179,8 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
size_t nbytes;
size_t len;
s16 headcount;
+ size_t remain_len;
+ int i;
spin_lock_bh(&vsock->send_pkt_list_lock);
if (list_empty(&vsock->send_pkt_list)) {
@@ -221,11 +223,19 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
break;
}
- nbytes = copy_to_iter(pkt->buf, pkt->len, &iov_iter);
- if (nbytes != pkt->len) {
- virtio_transport_free_pkt(pkt);
- vq_err(vq, "Faulted on copying pkt buf\n");
- break;
+ remain_len = pkt->len;
+ for (i = 0; i < pkt->nr_vecs; i++) {
+ int tmp_len;
+
+ tmp_len = min(remain_len, pkt->vec[i].iov_len);
+ nbytes = copy_to_iter(pkt->vec[i].iov_base, tmp_len, &iov_iter);
+ if (nbytes != tmp_len) {
+ virtio_transport_free_pkt(pkt);
+ vq_err(vq, "Faulted on copying pkt buf\n");
+ break;
+ }
+
+ remain_len -= tmp_len;
}
vhost_add_used_n(vq, vq->heads, headcount);
@@ -341,6 +351,7 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
struct iov_iter iov_iter;
size_t nbytes;
size_t len;
+ void *buf;
if (in != 0) {
vq_err(vq, "Expected 0 input buffers, got %u\n", in);
@@ -375,13 +386,17 @@ static void vhost_transport_send_pkt_work(struct vhost_work *work)
return NULL;
}
- pkt->buf = kmalloc(pkt->len, GFP_KERNEL);
- if (!pkt->buf) {
+ buf = kmalloc(pkt->len, GFP_KERNEL);
+ if (!buf) {
kfree(pkt);
return NULL;
}
- nbytes = copy_from_iter(pkt->buf, pkt->len, &iov_iter);
+ pkt->vec[0].iov_base = buf;
+ pkt->vec[0].iov_len = pkt->len;
+ pkt->nr_vecs = 1;
+
+ nbytes = copy_from_iter(buf, pkt->len, &iov_iter);
if (nbytes != pkt->len) {
vq_err(vq, "Expected %u byte payload, got %zu bytes\n",
pkt->len, nbytes);
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index da9e1fe..734eeed 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -13,6 +13,8 @@
#define VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE (1024 * 4)
#define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL
#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
+/* virtio_vsock_pkt + max_pkt_len(default MAX_PKT_BUF_SIZE) */
+#define VIRTIO_VSOCK_MAX_VEC_NUM ((VIRTIO_VSOCK_MAX_PKT_BUF_SIZE / PAGE_SIZE) + 1)
/* Virtio-vsock feature */
#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */
@@ -55,10 +57,12 @@ struct virtio_vsock_pkt {
struct list_head list;
/* socket refcnt not held, only use for cancellation */
struct vsock_sock *vsk;
- void *buf;
+ struct kvec vec[VIRTIO_VSOCK_MAX_VEC_NUM];
+ int nr_vecs;
u32 len;
u32 off;
bool reply;
+ bool mergeable;
};
struct virtio_vsock_pkt_info {
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index c4a465c..148b58a 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -155,8 +155,10 @@ static int virtio_transport_send_pkt_loopback(struct virtio_vsock *vsock,
sg_init_one(&hdr, &pkt->hdr, sizeof(pkt->hdr));
sgs[out_sg++] = &hdr;
- if (pkt->buf) {
- sg_init_one(&buf, pkt->buf, pkt->len);
+ if (pkt->len) {
+ /* Currently only support a segment of memory in tx */
+ BUG_ON(pkt->vec[0].iov_len != pkt->len);
+ sg_init_one(&buf, pkt->vec[0].iov_base, pkt->vec[0].iov_len);
sgs[out_sg++] = &buf;
}
@@ -304,23 +306,28 @@ static int fill_old_rx_buff(struct virtqueue *vq)
struct virtio_vsock_pkt *pkt;
struct scatterlist hdr, buf, *sgs[2];
int ret;
+ void *pkt_buf;
pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
if (!pkt)
return -ENOMEM;
- pkt->buf = kmalloc(buf_len, GFP_KERNEL);
- if (!pkt->buf) {
+ pkt_buf = kmalloc(buf_len, GFP_KERNEL);
+ if (!pkt_buf) {
virtio_transport_free_pkt(pkt);
return -ENOMEM;
}
+ pkt->vec[0].iov_base = pkt_buf;
+ pkt->vec[0].iov_len = buf_len;
+ pkt->nr_vecs = 1;
+
pkt->len = buf_len;
sg_init_one(&hdr, &pkt->hdr, sizeof(pkt->hdr));
sgs[0] = &hdr;
- sg_init_one(&buf, pkt->buf, buf_len);
+ sg_init_one(&buf, pkt->vec[0].iov_base, buf_len);
sgs[1] = &buf;
ret = virtqueue_add_sgs(vq, sgs, 0, 2, pkt, GFP_KERNEL);
if (ret)
@@ -388,11 +395,78 @@ static bool virtio_transport_more_replies(struct virtio_vsock *vsock)
return val < virtqueue_get_vring_size(vq);
}
+static struct virtio_vsock_pkt *receive_mergeable(struct virtqueue *vq,
+ struct virtio_vsock *vsock, unsigned int *total_len)
+{
+ struct virtio_vsock_pkt *pkt;
+ u16 num_buf;
+ void *buf;
+ unsigned int len;
+ size_t vsock_hlen = sizeof(struct virtio_vsock_pkt);
+
+ buf = virtqueue_get_buf(vq, &len);
+ if (!buf)
+ return NULL;
+
+ *total_len = len;
+ vsock->rx_buf_nr--;
+
+ if (unlikely(len < vsock_hlen)) {
+ put_page(virt_to_head_page(buf));
+ return NULL;
+ }
+
+ pkt = buf;
+ num_buf = le16_to_cpu(pkt->mrg_rxbuf_hdr.num_buffers);
+ if (!num_buf || num_buf > VIRTIO_VSOCK_MAX_VEC_NUM) {
+ put_page(virt_to_head_page(buf));
+ return NULL;
+ }
+
+ /* Initialize pkt residual structure */
+ memset(&pkt->work, 0, vsock_hlen - sizeof(struct virtio_vsock_hdr) -
+ sizeof(struct virtio_vsock_mrg_rxbuf_hdr));
+
+ pkt->mergeable = true;
+ pkt->len = le32_to_cpu(pkt->hdr.len);
+ if (!pkt->len)
+ return pkt;
+
+ len -= vsock_hlen;
+ if (len) {
+ pkt->vec[pkt->nr_vecs].iov_base = buf + vsock_hlen;
+ pkt->vec[pkt->nr_vecs].iov_len = len;
+ /* Shared page with pkt, so get page in advance */
+ get_page(virt_to_head_page(buf));
+ pkt->nr_vecs++;
+ }
+
+ while (--num_buf) {
+ buf = virtqueue_get_buf(vq, &len);
+ if (!buf)
+ goto err;
+
+ *total_len += len;
+ vsock->rx_buf_nr--;
+
+ pkt->vec[pkt->nr_vecs].iov_base = buf;
+ pkt->vec[pkt->nr_vecs].iov_len = len;
+ pkt->nr_vecs++;
+ }
+
+ return pkt;
+err:
+ virtio_transport_free_pkt(pkt);
+ return NULL;
+}
+
static void virtio_transport_rx_work(struct work_struct *work)
{
struct virtio_vsock *vsock =
container_of(work, struct virtio_vsock, rx_work);
struct virtqueue *vq;
+ size_t vsock_hlen = vsock->mergeable ? sizeof(struct virtio_vsock_pkt) :
+ sizeof(struct virtio_vsock_hdr);
vq = vsock->vqs[VSOCK_VQ_RX];
@@ -412,21 +486,26 @@ static void virtio_transport_rx_work(struct work_struct *work)
goto out;
}
- pkt = virtqueue_get_buf(vq, &len);
- if (!pkt) {
- break;
- }
+ if (likely(vsock->mergeable)) {
+ pkt = receive_mergeable(vq, vsock, &len);
+ if (!pkt)
+ break;
+ } else {
+ pkt = virtqueue_get_buf(vq, &len);
+ if (!pkt)
+ break;
- vsock->rx_buf_nr--;
+ vsock->rx_buf_nr--;
+ }
/* Drop short/long packets */
- if (unlikely(len < sizeof(pkt->hdr) ||
- len > sizeof(pkt->hdr) + pkt->len)) {
+ if (unlikely(len < vsock_hlen ||
+ len > vsock_hlen + pkt->len)) {
virtio_transport_free_pkt(pkt);
continue;
}
- pkt->len = len - sizeof(pkt->hdr);
+ pkt->len = len - vsock_hlen;
virtio_transport_deliver_tap_pkt(pkt);
virtio_transport_recv_pkt(pkt);
}
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 3ae3a33..123a8b6 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -44,6 +44,7 @@ static const struct virtio_transport *virtio_transport_get_ops(void)
{
struct virtio_vsock_pkt *pkt;
int err;
+ void *buf = NULL;
pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
if (!pkt)
@@ -62,12 +63,16 @@ static const struct virtio_transport *virtio_transport_get_ops(void)
pkt->vsk = info->vsk;
if (info->msg && len > 0) {
- pkt->buf = kmalloc(len, GFP_KERNEL);
- if (!pkt->buf)
+ buf = kmalloc(len, GFP_KERNEL);
+ if (!buf)
goto out_pkt;
- err = memcpy_from_msg(pkt->buf, info->msg, len);
+ err = memcpy_from_msg(buf, info->msg, len);
if (err)
goto out;
+
+ pkt->vec[0].iov_base = buf;
+ pkt->vec[0].iov_len = len;
+ pkt->nr_vecs = 1;
}
trace_virtio_transport_alloc_pkt(src_cid, src_port,
@@ -80,7 +85,7 @@ static const struct virtio_transport *virtio_transport_get_ops(void)
return pkt;
out:
- kfree(pkt->buf);
+ kfree(buf);
out_pkt:
kfree(pkt);
return NULL;
@@ -92,6 +97,7 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
struct virtio_vsock_pkt *pkt = opaque;
struct af_vsockmon_hdr *hdr;
struct sk_buff *skb;
+ int i;
skb = alloc_skb(sizeof(*hdr) + sizeof(pkt->hdr) + pkt->len,
GFP_ATOMIC);
@@ -134,7 +140,8 @@ static struct sk_buff *virtio_transport_build_skb(void *opaque)
skb_put_data(skb, &pkt->hdr, sizeof(pkt->hdr));
if (pkt->len) {
- skb_put_data(skb, pkt->buf, pkt->len);
+ for (i = 0; i < pkt->nr_vecs; i++)
+ skb_put_data(skb, pkt->vec[i].iov_base, pkt->vec[i].iov_len);
}
return skb;
@@ -260,6 +267,9 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
spin_lock_bh(&vvs->rx_lock);
while (total < len && !list_empty(&vvs->rx_queue)) {
+ size_t copy_bytes, last_vec_total = 0, vec_off;
+ int i;
+
pkt = list_first_entry(&vvs->rx_queue,
struct virtio_vsock_pkt, list);
@@ -272,14 +282,28 @@ static int virtio_transport_send_credit_update(struct vsock_sock *vsk,
*/
spin_unlock_bh(&vvs->rx_lock);
- err = memcpy_to_msg(msg, pkt->buf + pkt->off, bytes);
- if (err)
- goto out;
+ for (i = 0; i < pkt->nr_vecs; i++) {
+ if (pkt->off > last_vec_total + pkt->vec[i].iov_len) {
+ last_vec_total += pkt->vec[i].iov_len;
+ continue;
+ }
+
+ vec_off = pkt->off - last_vec_total;
+ copy_bytes = min(pkt->vec[i].iov_len - vec_off, bytes);
+ err = memcpy_to_msg(msg, pkt->vec[i].iov_base + vec_off,
+ copy_bytes);
+ if (err)
+ goto out;
+
+ bytes -= copy_bytes;
+ pkt->off += copy_bytes;
+ total += copy_bytes;
+ last_vec_total += pkt->vec[i].iov_len;
+ if (!bytes)
+ break;
+ }
spin_lock_bh(&vvs->rx_lock);
-
- total += bytes;
- pkt->off += bytes;
if (pkt->off == pkt->len) {
virtio_transport_dec_rx_pkt(vvs, pkt);
list_del(&pkt->list);
@@ -1050,8 +1074,17 @@ void virtio_transport_recv_pkt(struct virtio_vsock_pkt *pkt)
void virtio_transport_free_pkt(struct virtio_vsock_pkt *pkt)
{
- kfree(pkt->buf);
- kfree(pkt);
+ int i;
+
+ if (pkt->mergeable) {
+ for (i = 0; i < pkt->nr_vecs; i++)
+ put_page(virt_to_head_page(pkt->vec[i].iov_base));
+ put_page(virt_to_head_page((void *)pkt));
+ } else {
+ for (i = 0; i < pkt->nr_vecs; i++)
+ kfree(pkt->vec[i].iov_base);
+ kfree(pkt);
+ }
}
EXPORT_SYMBOL_GPL(virtio_transport_free_pkt);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 2/5] VSOCK: support fill data to mergeable rx buffer in host
From: jiangyiwen @ 2018-12-12 9:29 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang
Cc: netdev, kvm, virtualization
When vhost support VIRTIO_VSOCK_F_MRG_RXBUF feature,
it will merge big packet into rx vq.
Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
---
drivers/vhost/vsock.c | 111 ++++++++++++++++++++++++++++++--------
include/linux/virtio_vsock.h | 1 +
include/uapi/linux/virtio_vsock.h | 5 ++
3 files changed, 94 insertions(+), 23 deletions(-)
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index 34bc3ab..dc52b0f 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -22,7 +22,8 @@
#define VHOST_VSOCK_DEFAULT_HOST_CID 2
enum {
- VHOST_VSOCK_FEATURES = VHOST_FEATURES,
+ VHOST_VSOCK_FEATURES = VHOST_FEATURES |
+ (1ULL << VIRTIO_VSOCK_F_MRG_RXBUF),
};
/* Used to track all the vhost_vsock instances on the system. */
@@ -80,6 +81,69 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
return vsock;
}
+/* This segment of codes are copied from drivers/vhost/net.c */
+static int get_rx_bufs(struct vhost_virtqueue *vq,
+ struct vring_used_elem *heads, int datalen,
+ unsigned *iovcount, unsigned int quota)
+{
+ unsigned int out, in;
+ int seg = 0;
+ int headcount = 0;
+ unsigned d;
+ int ret;
+ /*
+ * len is always initialized before use since we are always called with
+ * datalen > 0.
+ */
+ u32 uninitialized_var(len);
+
+ while (datalen > 0 && headcount < quota) {
+ if (unlikely(seg >= UIO_MAXIOV)) {
+ ret = -ENOBUFS;
+ goto err;
+ }
+
+ ret = vhost_get_vq_desc(vq, vq->iov + seg,
+ ARRAY_SIZE(vq->iov) - seg, &out,
+ &in, NULL, NULL);
+ if (unlikely(ret < 0))
+ goto err;
+
+ d = ret;
+ if (d == vq->num) {
+ ret = 0;
+ goto err;
+ }
+
+ if (unlikely(out || in <= 0)) {
+ vq_err(vq, "unexpected descriptor format for RX: "
+ "out %d, in %d\n", out, in);
+ ret = -EINVAL;
+ goto err;
+ }
+
+ heads[headcount].id = cpu_to_vhost32(vq, d);
+ len = iov_length(vq->iov + seg, in);
+ heads[headcount].len = cpu_to_vhost32(vq, len);
+ datalen -= len;
+ ++headcount;
+ seg += in;
+ }
+
+ heads[headcount - 1].len = cpu_to_vhost32(vq, len + datalen);
+ *iovcount = seg;
+
+ /* Detect overrun */
+ if (unlikely(datalen > 0)) {
+ ret = UIO_MAXIOV + 1;
+ goto err;
+ }
+ return headcount;
+err:
+ vhost_discard_vq_desc(vq, headcount);
+ return ret;
+}
+
static void
vhost_transport_do_send_pkt(struct vhost_vsock *vsock,
struct vhost_virtqueue *vq)
@@ -87,22 +151,34 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
struct vhost_virtqueue *tx_vq = &vsock->vqs[VSOCK_VQ_TX];
bool added = false;
bool restart_tx = false;
+ int mergeable;
+ size_t vsock_hlen;
mutex_lock(&vq->mutex);
if (!vq->private_data)
goto out;
+ mergeable = vhost_has_feature(vq, VIRTIO_VSOCK_F_MRG_RXBUF);
+ /*
+ * Guest fill page for rx vq in mergeable case, so it will not
+ * allocate pkt structure, we should reserve size of pkt in advance.
+ */
+ if (likely(mergeable))
+ vsock_hlen = sizeof(struct virtio_vsock_pkt);
+ else
+ vsock_hlen = sizeof(struct virtio_vsock_hdr);
+
/* Avoid further vmexits, we're already processing the virtqueue */
vhost_disable_notify(&vsock->dev, vq);
for (;;) {
struct virtio_vsock_pkt *pkt;
struct iov_iter iov_iter;
- unsigned out, in;
+ unsigned out = 0, in = 0;
size_t nbytes;
size_t len;
- int head;
+ s16 headcount;
spin_lock_bh(&vsock->send_pkt_list_lock);
if (list_empty(&vsock->send_pkt_list)) {
@@ -116,16 +192,9 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
list_del_init(&pkt->list);
spin_unlock_bh(&vsock->send_pkt_list_lock);
- head = vhost_get_vq_desc(vq, vq->iov, ARRAY_SIZE(vq->iov),
- &out, &in, NULL, NULL);
- if (head < 0) {
- spin_lock_bh(&vsock->send_pkt_list_lock);
- list_add(&pkt->list, &vsock->send_pkt_list);
- spin_unlock_bh(&vsock->send_pkt_list_lock);
- break;
- }
-
- if (head == vq->num) {
+ headcount = get_rx_bufs(vq, vq->heads, vsock_hlen + pkt->len,
+ &in, likely(mergeable) ? UIO_MAXIOV : 1);
+ if (headcount <= 0) {
spin_lock_bh(&vsock->send_pkt_list_lock);
list_add(&pkt->list, &vsock->send_pkt_list);
spin_unlock_bh(&vsock->send_pkt_list_lock);
@@ -133,24 +202,20 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
/* We cannot finish yet if more buffers snuck in while
* re-enabling notify.
*/
- if (unlikely(vhost_enable_notify(&vsock->dev, vq))) {
+ if (!headcount && unlikely(vhost_enable_notify(&vsock->dev, vq))) {
vhost_disable_notify(&vsock->dev, vq);
continue;
}
break;
}
- if (out) {
- virtio_transport_free_pkt(pkt);
- vq_err(vq, "Expected 0 output buffers, got %u\n", out);
- break;
- }
-
len = iov_length(&vq->iov[out], in);
iov_iter_init(&iov_iter, READ, &vq->iov[out], in, len);
- nbytes = copy_to_iter(&pkt->hdr, sizeof(pkt->hdr), &iov_iter);
- if (nbytes != sizeof(pkt->hdr)) {
+ if (likely(mergeable))
+ pkt->mrg_rxbuf_hdr.num_buffers = cpu_to_le16(headcount);
+ nbytes = copy_to_iter(&pkt->hdr, vsock_hlen, &iov_iter);
+ if (nbytes != vsock_hlen) {
virtio_transport_free_pkt(pkt);
vq_err(vq, "Faulted on copying pkt hdr\n");
break;
@@ -163,7 +228,7 @@ static struct vhost_vsock *vhost_vsock_get(u32 guest_cid)
break;
}
- vhost_add_used(vq, head, sizeof(pkt->hdr) + pkt->len);
+ vhost_add_used_n(vq, vq->heads, headcount);
added = true;
if (pkt->reply) {
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index bf84418..da9e1fe 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -50,6 +50,7 @@ struct virtio_vsock_sock {
struct virtio_vsock_pkt {
struct virtio_vsock_hdr hdr;
+ struct virtio_vsock_mrg_rxbuf_hdr mrg_rxbuf_hdr;
struct work_struct work;
struct list_head list;
/* socket refcnt not held, only use for cancellation */
diff --git a/include/uapi/linux/virtio_vsock.h b/include/uapi/linux/virtio_vsock.h
index 1d57ed3..2292f30 100644
--- a/include/uapi/linux/virtio_vsock.h
+++ b/include/uapi/linux/virtio_vsock.h
@@ -63,6 +63,11 @@ struct virtio_vsock_hdr {
__le32 fwd_cnt;
} __attribute__((packed));
+/* It add mergeable rx buffers feature */
+struct virtio_vsock_mrg_rxbuf_hdr {
+ __le16 num_buffers; /* number of mergeable rx buffers */
+} __attribute__((packed));
+
enum virtio_vsock_type {
VIRTIO_VSOCK_TYPE_STREAM = 1,
};
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 1/5] VSOCK: support fill mergeable rx buffer in guest
From: jiangyiwen @ 2018-12-12 9:28 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang
Cc: netdev, kvm, virtualization
In driver probing, if virtio has VIRTIO_VSOCK_F_MRG_RXBUF feature,
it will fill mergeable rx buffer, support for host send mergeable
rx buffer. It will fill a fixed size(PAGE_SIZE) everytime to
compact with small packet and big packet.
In addition, it also add one optimizations copied from virtio-net.c.
- Skb_page_frag_refill() which can use high order page and
reduce the stress of page allocator.
Signed-off-by: Yiwen Jiang <jiangyiwen@huawei.com>
---
include/linux/virtio_vsock.h | 3 ++
net/vmw_vsock/virtio_transport.c | 112 +++++++++++++++++++++++++++++++--------
2 files changed, 92 insertions(+), 23 deletions(-)
diff --git a/include/linux/virtio_vsock.h b/include/linux/virtio_vsock.h
index e223e26..bf84418 100644
--- a/include/linux/virtio_vsock.h
+++ b/include/linux/virtio_vsock.h
@@ -14,6 +14,9 @@
#define VIRTIO_VSOCK_MAX_BUF_SIZE 0xFFFFFFFFUL
#define VIRTIO_VSOCK_MAX_PKT_BUF_SIZE (1024 * 64)
+/* Virtio-vsock feature */
+#define VIRTIO_VSOCK_F_MRG_RXBUF 0 /* Host can merge receive buffers. */
+
enum {
VSOCK_VQ_RX = 0, /* for host to guest data */
VSOCK_VQ_TX = 1, /* for guest to host data */
diff --git a/net/vmw_vsock/virtio_transport.c b/net/vmw_vsock/virtio_transport.c
index 5d3cce9..c4a465c 100644
--- a/net/vmw_vsock/virtio_transport.c
+++ b/net/vmw_vsock/virtio_transport.c
@@ -64,6 +64,11 @@ struct virtio_vsock {
struct virtio_vsock_event event_list[8];
u32 guest_cid;
+
+ /* As mergeable rx buffer flag */
+ bool mergeable;
+ /* Page frag for packet buffer allocation. */
+ struct page_frag alloc_frag;
};
static struct virtio_vsock *virtio_vsock_get(void)
@@ -256,39 +261,89 @@ static int virtio_transport_send_pkt_loopback(struct virtio_vsock *vsock,
return 0;
}
-static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
+/* This segment of codes are copied from virtio-net.c */
+static int fill_mergeable_rx_buff(struct virtio_vsock *vsock,
+ struct virtqueue *vq)
+{
+ struct page_frag *alloc_frag = &vsock->alloc_frag;
+ struct scatterlist sg;
+ /* Currently we don't use ewma len, use PAGE_SIZE instead, because too
+ * small size can't fill one full packet, sadly we only 128 vq num now.
+ */
+ unsigned int len = PAGE_SIZE, hole;
+ void *buf;
+ int err;
+
+ if (unlikely(!skb_page_frag_refill(len, alloc_frag, GFP_KERNEL)))
+ return -ENOMEM;
+
+ buf = (void *)page_address(alloc_frag->page) + alloc_frag->offset;
+ get_page(alloc_frag->page);
+ alloc_frag->offset += len;
+ hole = alloc_frag->size - alloc_frag->offset;
+ if (hole < len) {
+ /* To avoid internal fragmentation, if there is very likely not
+ * enough space for another buffer, add the remaining space to
+ * the current buffer.
+ */
+ len += hole;
+ alloc_frag->offset += hole;
+ }
+
+ sg_init_one(&sg, buf, len);
+ err = virtqueue_add_inbuf(vq, &sg, 1, buf, GFP_KERNEL);
+ if (err < 0)
+ put_page(virt_to_head_page(buf));
+
+ return err;
+}
+
+static int fill_old_rx_buff(struct virtqueue *vq)
{
int buf_len = VIRTIO_VSOCK_DEFAULT_RX_BUF_SIZE;
struct virtio_vsock_pkt *pkt;
struct scatterlist hdr, buf, *sgs[2];
- struct virtqueue *vq;
int ret;
- vq = vsock->vqs[VSOCK_VQ_RX];
+ pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
+ if (!pkt)
+ return -ENOMEM;
- do {
- pkt = kzalloc(sizeof(*pkt), GFP_KERNEL);
- if (!pkt)
- break;
+ pkt->buf = kmalloc(buf_len, GFP_KERNEL);
+ if (!pkt->buf) {
+ virtio_transport_free_pkt(pkt);
+ return -ENOMEM;
+ }
- pkt->buf = kmalloc(buf_len, GFP_KERNEL);
- if (!pkt->buf) {
- virtio_transport_free_pkt(pkt);
- break;
- }
+ pkt->len = buf_len;
- pkt->len = buf_len;
+ sg_init_one(&hdr, &pkt->hdr, sizeof(pkt->hdr));
+ sgs[0] = &hdr;
- sg_init_one(&hdr, &pkt->hdr, sizeof(pkt->hdr));
- sgs[0] = &hdr;
+ sg_init_one(&buf, pkt->buf, buf_len);
+ sgs[1] = &buf;
+ ret = virtqueue_add_sgs(vq, sgs, 0, 2, pkt, GFP_KERNEL);
+ if (ret)
+ virtio_transport_free_pkt(pkt);
- sg_init_one(&buf, pkt->buf, buf_len);
- sgs[1] = &buf;
- ret = virtqueue_add_sgs(vq, sgs, 0, 2, pkt, GFP_KERNEL);
- if (ret) {
- virtio_transport_free_pkt(pkt);
+ return ret;
+}
+
+static void virtio_vsock_rx_fill(struct virtio_vsock *vsock)
+{
+ struct virtqueue *vq;
+ int ret;
+
+ vq = vsock->vqs[VSOCK_VQ_RX];
+
+ do {
+ if (vsock->mergeable)
+ ret = fill_mergeable_rx_buff(vsock, vq);
+ else
+ ret = fill_old_rx_buff(vq);
+ if (ret)
break;
- }
+
vsock->rx_buf_nr++;
} while (vq->num_free);
if (vsock->rx_buf_nr > vsock->rx_buf_max_nr)
@@ -588,6 +643,9 @@ static int virtio_vsock_probe(struct virtio_device *vdev)
if (ret < 0)
goto out_vqs;
+ if (virtio_has_feature(vdev, VIRTIO_VSOCK_F_MRG_RXBUF))
+ vsock->mergeable = true;
+
vsock->rx_buf_nr = 0;
vsock->rx_buf_max_nr = 0;
atomic_set(&vsock->queued_replies, 0);
@@ -640,8 +698,15 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
vdev->config->reset(vdev);
mutex_lock(&vsock->rx_lock);
- while ((pkt = virtqueue_detach_unused_buf(vsock->vqs[VSOCK_VQ_RX])))
- virtio_transport_free_pkt(pkt);
+ while ((pkt = virtqueue_detach_unused_buf(vsock->vqs[VSOCK_VQ_RX]))) {
+ if (vsock->mergeable)
+ put_page(virt_to_head_page((void *)pkt));
+ else
+ virtio_transport_free_pkt(pkt);
+ }
+
+ if (vsock->alloc_frag.page)
+ put_page(vsock->alloc_frag.page);
mutex_unlock(&vsock->rx_lock);
mutex_lock(&vsock->tx_lock);
@@ -683,6 +748,7 @@ static void virtio_vsock_remove(struct virtio_device *vdev)
};
static unsigned int features[] = {
+ VIRTIO_VSOCK_F_MRG_RXBUF,
};
static struct virtio_driver virtio_vsock_driver = {
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 0/5] VSOCK: support mergeable rx buffer in vhost-vsock
From: jiangyiwen @ 2018-12-12 9:25 UTC (permalink / raw)
To: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang
Cc: netdev, kvm, virtualization
Now vsock only support send/receive small packet, it can't achieve
high performance. As previous discussed with Jason Wang, I revisit the
idea of vhost-net about mergeable rx buffer and implement the mergeable
rx buffer in vhost-vsock, it can allow big packet to be scattered in
into different buffers and improve performance obviously.
This series of patches mainly did three things：
- mergeable buffer implementation
- increase the max send pkt size
- add used and signal guest in a batch
And I write a tool to test the vhost-vsock performance, mainly send big
packet(64K) included guest->Host and Host->Guest. I test performance
independently and the result as follows:
Before performance:
Single socket Multiple sockets(Max Bandwidth)
Guest->Host ~400MB/s ~480MB/s
Host->Guest ~1450MB/s ~1600MB/s
After performance only use implement mergeable rx buffer:
Single socket Multiple sockets(Max Bandwidth)
Guest->Host ~400MB/s ~480MB/s
Host->Guest ~1280MB/s ~1350MB/s
In this case, max send pkt size is still limited to 4K, so Host->Guest
performance will worse than before.
After performance increase the max send pkt size to 64K:
Single socket Multiple sockets(Max Bandwidth)
Guest->Host ~1700MB/s ~2900MB/s
Host->Guest ~1500MB/s ~2440MB/s
After performance all patches are used:
Single socket Multiple sockets(Max Bandwidth)
Guest->Host ~1700MB/s ~2900MB/s
Host->Guest ~1700MB/s ~2900MB/s
From the test results, the performance is improved obviously, and guest
memory will not be wasted.
In addition, in order to support mergeable rx buffer in virtio-vsock,
we need to add a qemu patch to support parse feature.
---
v1 -> v2:
* Addressed comments from Jason Wang.
* Add performance test result independently.
* Use Skb_page_frag_refill() which can use high order page and reduce
the stress of page allocator.
* Still use fixed size(PAGE_SIZE) to fill rx buffer, because too small
size can't fill one full packet, we only 128 vq num now.
* Use iovec to replace buf in struct virtio_vsock_pkt, keep tx and rx
consistency.
* Add virtio_transport ops to get max pkt len, in order to be compatible
with old version.
---
Yiwen Jiang (5):
VSOCK: support fill mergeable rx buffer in guest
VSOCK: support fill data to mergeable rx buffer in host
VSOCK: support receive mergeable rx buffer in guest
VSOCK: increase send pkt len in mergeable mode to improve performance
VSOCK: batch sending rx buffer to increase bandwidth
drivers/vhost/vsock.c | 183 ++++++++++++++++++++-----
include/linux/virtio_vsock.h | 13 +-
include/uapi/linux/virtio_vsock.h | 5 +
net/vmw_vsock/virtio_transport.c | 229 +++++++++++++++++++++++++++-----
net/vmw_vsock/virtio_transport_common.c | 66 ++++++---
5 files changed, 411 insertions(+), 85 deletions(-)
--
1.8.3.1
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net 2/4] vhost_net: rework on the lock ordering for busy polling
From: Michael S. Tsirkin @ 2018-12-12 3:40 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <aa8f36da-1489-a094-35ce-286bb3f25243@redhat.com>
On Wed, Dec 12, 2018 at 11:03:57AM +0800, Jason Wang wrote:
>
> On 2018/12/11 下午12:04, Michael S. Tsirkin wrote:
> > On Tue, Dec 11, 2018 at 11:06:43AM +0800, Jason Wang wrote:
> > > On 2018/12/11 上午9:34, Michael S. Tsirkin wrote:
> > > > On Mon, Dec 10, 2018 at 05:44:52PM +0800, Jason Wang wrote:
> > > > > When we try to do rx busy polling in tx path in commit 441abde4cd84
> > > > > ("net: vhost: add rx busy polling in tx path"), we lock rx vq mutex
> > > > > after tx vq mutex is held. This may lead deadlock so we try to lock vq
> > > > > one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by
> > > > > one"). With this commit, we avoid the deadlock with the assumption
> > > > > that handle_rx() and handle_tx() run in a same process. But this
> > > > > commit remove the protection for IOTLB updating which requires the
> > > > > mutex of each vq to be held.
> > > > >
> > > > > To solve this issue, the first step is to have a exact same lock
> > > > > ordering for vhost_net. This is done through:
> > > > >
> > > > > - For handle_rx(), if busy polling is enabled, lock tx vq immediately.
> > > > > - For handle_tx(), always lock rx vq before tx vq, and unlock it if
> > > > > busy polling is not enabled.
> > > > > - Remove the tricky locking codes in busy polling.
> > > > >
> > > > > With this, we can have a exact same lock ordering for vhost_net, this
> > > > > allows us to safely revert commit 78139c94dc8c ("net: vhost: lock the
> > > > > vqs one by one") in next patch.
> > > > >
> > > > > The patch will add two more atomic operations on the tx path during
> > > > > each round of handle_tx(). 1 byte TCP_RR does not notice such
> > > > > overhead.
> > > > >
> > > > > Fixes: commit 78139c94dc8c ("net: vhost: lock the vqs one by one")
> > > > > Cc: Tonghao Zhang<xiangxia.m.yue@gmail.com>
> > > > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > > > ---
> > > > > drivers/vhost/net.c | 18 +++++++++++++++---
> > > > > 1 file changed, 15 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > > > index ab11b2bee273..5f272ab4d5b4 100644
> > > > > --- a/drivers/vhost/net.c
> > > > > +++ b/drivers/vhost/net.c
> > > > > @@ -513,7 +513,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> > > > > struct socket *sock;
> > > > > struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
> > > > > - mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
> > > > > vhost_disable_notify(&net->dev, vq);
> > > > > sock = rvq->private_data;
> > > > > @@ -543,8 +542,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> > > > > vhost_net_busy_poll_try_queue(net, vq);
> > > > > else if (!poll_rx) /* On tx here, sock has no rx data. */
> > > > > vhost_enable_notify(&net->dev, rvq);
> > > > > -
> > > > > - mutex_unlock(&vq->mutex);
> > > > > }
> > > > > static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> > > > > @@ -913,10 +910,16 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
> > > > > static void handle_tx(struct vhost_net *net)
> > > > > {
> > > > > struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> > > > > + struct vhost_net_virtqueue *nvq_rx = &net->vqs[VHOST_NET_VQ_RX];
> > > > > struct vhost_virtqueue *vq = &nvq->vq;
> > > > > + struct vhost_virtqueue *vq_rx = &nvq_rx->vq;
> > > > > struct socket *sock;
> > > > > + mutex_lock_nested(&vq_rx->mutex, VHOST_NET_VQ_RX);
> > > > > mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
> > > > > + if (!vq->busyloop_timeout)
> > > > > + mutex_unlock(&vq_rx->mutex);
> > > > > +
> > > > > sock = vq->private_data;
> > > > > if (!sock)
> > > > > goto out;
> > > > > @@ -933,6 +936,8 @@ static void handle_tx(struct vhost_net *net)
> > > > > handle_tx_copy(net, sock);
> > > > > out:
> > > > > + if (vq->busyloop_timeout)
> > > > > + mutex_unlock(&vq_rx->mutex);
> > > > > mutex_unlock(&vq->mutex);
> > > > > }
> > > > So rx mutex taken on tx path now. And tx mutex is on rc path ... This
> > > > is just messed up. Why can't tx polling drop rx lock before
> > > > getting the tx lock and vice versa?
> > >
> > > Because we want to poll both tx and rx virtqueue at the same time
> > > (vhost_net_busy_poll()).
> > >
> > > while (vhost_can_busy_poll(endtime)) {
> > > if (vhost_has_work(&net->dev)) {
> > > *busyloop_intr = true;
> > > break;
> > > }
> > >
> > > if ((sock_has_rx_data(sock) &&
> > > !vhost_vq_avail_empty(&net->dev, rvq)) ||
> > > !vhost_vq_avail_empty(&net->dev, tvq))
> > > break;
> > >
> > > cpu_relax();
> > >
> > > }
> > >
> > >
> > > And we disable kicks and notification for better performance.
> > Right but it's all slow path - it happens when queue is
> > otherwise empty. So this is what I am saying: let's drop the locks
> > we hold around this.
>
>
> Is this really safe? I looks to me it can race with SET_VRING_ADDR. And the
> codes did more:
>
> - access sock object
>
> - access device IOTLB
>
> - enable and disable notification
>
> None of above is safe without the protection of vq mutex.
ys but take another lock. just not nested.
>
> >
> >
> > > > Or if we really wanted to force everything to be locked at
> > > > all times, let's just use a single mutex.
> > > >
> > > >
> > > >
> > > We could, but it might requires more changes which could be done for -next I
> > > believe.
> > >
> > >
> > > Thanks
> > I'd rather we kept the fine grained locking. E.g. people are
> > looking at splitting the tx and rx threads. But if not possible
> > let's fix it cleanly with a coarse-grained one. A mess here will
> > just create more trouble later.
> >
>
> I believe we won't go back to coarse one. Looks like we can solve this by
> using mutex_trylock() for rxq during TX. And don't do polling for rxq is a
> IOTLB updating is pending.
>
> Let me post V2.
>
> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH net 2/4] vhost_net: rework on the lock ordering for busy polling
From: Jason Wang @ 2018-12-12 3:03 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <20181210230106-mutt-send-email-mst@kernel.org>
On 2018/12/11 下午12:04, Michael S. Tsirkin wrote:
> On Tue, Dec 11, 2018 at 11:06:43AM +0800, Jason Wang wrote:
>> On 2018/12/11 上午9:34, Michael S. Tsirkin wrote:
>>> On Mon, Dec 10, 2018 at 05:44:52PM +0800, Jason Wang wrote:
>>>> When we try to do rx busy polling in tx path in commit 441abde4cd84
>>>> ("net: vhost: add rx busy polling in tx path"), we lock rx vq mutex
>>>> after tx vq mutex is held. This may lead deadlock so we try to lock vq
>>>> one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by
>>>> one"). With this commit, we avoid the deadlock with the assumption
>>>> that handle_rx() and handle_tx() run in a same process. But this
>>>> commit remove the protection for IOTLB updating which requires the
>>>> mutex of each vq to be held.
>>>>
>>>> To solve this issue, the first step is to have a exact same lock
>>>> ordering for vhost_net. This is done through:
>>>>
>>>> - For handle_rx(), if busy polling is enabled, lock tx vq immediately.
>>>> - For handle_tx(), always lock rx vq before tx vq, and unlock it if
>>>> busy polling is not enabled.
>>>> - Remove the tricky locking codes in busy polling.
>>>>
>>>> With this, we can have a exact same lock ordering for vhost_net, this
>>>> allows us to safely revert commit 78139c94dc8c ("net: vhost: lock the
>>>> vqs one by one") in next patch.
>>>>
>>>> The patch will add two more atomic operations on the tx path during
>>>> each round of handle_tx(). 1 byte TCP_RR does not notice such
>>>> overhead.
>>>>
>>>> Fixes: commit 78139c94dc8c ("net: vhost: lock the vqs one by one")
>>>> Cc: Tonghao Zhang<xiangxia.m.yue@gmail.com>
>>>> Signed-off-by: Jason Wang<jasowang@redhat.com>
>>>> ---
>>>> drivers/vhost/net.c | 18 +++++++++++++++---
>>>> 1 file changed, 15 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
>>>> index ab11b2bee273..5f272ab4d5b4 100644
>>>> --- a/drivers/vhost/net.c
>>>> +++ b/drivers/vhost/net.c
>>>> @@ -513,7 +513,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
>>>> struct socket *sock;
>>>> struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
>>>> - mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
>>>> vhost_disable_notify(&net->dev, vq);
>>>> sock = rvq->private_data;
>>>> @@ -543,8 +542,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
>>>> vhost_net_busy_poll_try_queue(net, vq);
>>>> else if (!poll_rx) /* On tx here, sock has no rx data. */
>>>> vhost_enable_notify(&net->dev, rvq);
>>>> -
>>>> - mutex_unlock(&vq->mutex);
>>>> }
>>>> static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
>>>> @@ -913,10 +910,16 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
>>>> static void handle_tx(struct vhost_net *net)
>>>> {
>>>> struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
>>>> + struct vhost_net_virtqueue *nvq_rx = &net->vqs[VHOST_NET_VQ_RX];
>>>> struct vhost_virtqueue *vq = &nvq->vq;
>>>> + struct vhost_virtqueue *vq_rx = &nvq_rx->vq;
>>>> struct socket *sock;
>>>> + mutex_lock_nested(&vq_rx->mutex, VHOST_NET_VQ_RX);
>>>> mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
>>>> + if (!vq->busyloop_timeout)
>>>> + mutex_unlock(&vq_rx->mutex);
>>>> +
>>>> sock = vq->private_data;
>>>> if (!sock)
>>>> goto out;
>>>> @@ -933,6 +936,8 @@ static void handle_tx(struct vhost_net *net)
>>>> handle_tx_copy(net, sock);
>>>> out:
>>>> + if (vq->busyloop_timeout)
>>>> + mutex_unlock(&vq_rx->mutex);
>>>> mutex_unlock(&vq->mutex);
>>>> }
>>> So rx mutex taken on tx path now. And tx mutex is on rc path ... This
>>> is just messed up. Why can't tx polling drop rx lock before
>>> getting the tx lock and vice versa?
>>
>> Because we want to poll both tx and rx virtqueue at the same time
>> (vhost_net_busy_poll()).
>>
>> while (vhost_can_busy_poll(endtime)) {
>> if (vhost_has_work(&net->dev)) {
>> *busyloop_intr = true;
>> break;
>> }
>>
>> if ((sock_has_rx_data(sock) &&
>> !vhost_vq_avail_empty(&net->dev, rvq)) ||
>> !vhost_vq_avail_empty(&net->dev, tvq))
>> break;
>>
>> cpu_relax();
>>
>> }
>>
>>
>> And we disable kicks and notification for better performance.
> Right but it's all slow path - it happens when queue is
> otherwise empty. So this is what I am saying: let's drop the locks
> we hold around this.
Is this really safe? I looks to me it can race with SET_VRING_ADDR. And
the codes did more:
- access sock object
- access device IOTLB
- enable and disable notification
None of above is safe without the protection of vq mutex.
>
>
>>> Or if we really wanted to force everything to be locked at
>>> all times, let's just use a single mutex.
>>>
>>>
>>>
>> We could, but it might requires more changes which could be done for -next I
>> believe.
>>
>>
>> Thanks
> I'd rather we kept the fine grained locking. E.g. people are
> looking at splitting the tx and rx threads. But if not possible
> let's fix it cleanly with a coarse-grained one. A mess here will
> just create more trouble later.
>
I believe we won't go back to coarse one. Looks like we can solve this
by using mutex_trylock() for rxq during TX. And don't do polling for rxq
is a IOTLB updating is pending.
Let me post V2.
Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v6 0/7] Add virtio-iommu driver
From: Michael S. Tsirkin @ 2018-12-12 0:39 UTC (permalink / raw)
To: Christoph Hellwig
Cc: mark.rutland, virtio-dev, tnowicki, devicetree,
Jean-Philippe Brucker, linux-pci, joro, will.deacon,
virtualization, marc.zyngier, iommu, robh+dt, kvmarm, bhelgaas,
robin.murphy
In-Reply-To: <20181211183101.GA31936@infradead.org>
On Tue, Dec 11, 2018 at 10:31:01AM -0800, Christoph Hellwig wrote:
> On Tue, Dec 11, 2018 at 06:20:57PM +0000, Jean-Philippe Brucker wrote:
> > Implement the virtio-iommu driver, following specification v0.9 [1].
> >
> > Only minor changes since v5 [2]. I fixed issues reported by Michael and
> > added tags from Eric and Bharat. Thanks!
> >
> > You can find Linux driver and kvmtool device on v0.9 branches [3],
> > module and x86 support on virtio-iommu/devel. Also tested with Eric's
> > QEMU device [4].
>
> Just curious, what is the use case for it?
If what I saw is any indication, it allows a very simple implementation
of page table shadowing on the host, at the cost of not being able to
accelerate with a hardware nested page tables support when available.
--
MST
^ permalink raw reply
* Re: [PATCH v6 0/7] Add virtio-iommu driver
From: Jean-Philippe Brucker @ 2018-12-11 19:07 UTC (permalink / raw)
To: Christoph Hellwig
Cc: mark.rutland, virtio-dev, tnowicki, devicetree, marc.zyngier,
linux-pci, mst, will.deacon, robin.murphy, virtualization, iommu,
robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211183101.GA31936@infradead.org>
On 11/12/2018 18:31, Christoph Hellwig wrote:
> On Tue, Dec 11, 2018 at 06:20:57PM +0000, Jean-Philippe Brucker wrote:
>> Implement the virtio-iommu driver, following specification v0.9 [1].
>>
>> Only minor changes since v5 [2]. I fixed issues reported by Michael and
>> added tags from Eric and Bharat. Thanks!
>>
>> You can find Linux driver and kvmtool device on v0.9 branches [3],
>> module and x86 support on virtio-iommu/devel. Also tested with Eric's
>> QEMU device [4].
>
> Just curious, what is the use case for it?
The main use case is assigning a device to guest userspace, using VFIO
both in the host and in the guest (the most cited example being DPDK).
There are others, and I wrote a little more about them last week:
https://www.spinics.net/lists/linux-pci/msg78529.html
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH v6 0/7] Add virtio-iommu driver
From: Christoph Hellwig @ 2018-12-11 18:31 UTC (permalink / raw)
To: Jean-Philippe Brucker
Cc: mark.rutland, virtio-dev, tnowicki, devicetree, marc.zyngier,
linux-pci, joro, mst, will.deacon, virtualization, iommu, robh+dt,
kvmarm, bhelgaas, robin.murphy
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
On Tue, Dec 11, 2018 at 06:20:57PM +0000, Jean-Philippe Brucker wrote:
> Implement the virtio-iommu driver, following specification v0.9 [1].
>
> Only minor changes since v5 [2]. I fixed issues reported by Michael and
> added tags from Eric and Bharat. Thanks!
>
> You can find Linux driver and kvmtool device on v0.9 branches [3],
> module and x86 support on virtio-iommu/devel. Also tested with Eric's
> QEMU device [4].
Just curious, what is the use case for it?
^ permalink raw reply
* [PATCH v6 7/7] iommu/virtio: Add event queue
From: Jean-Philippe Brucker @ 2018-12-11 18:21 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
The event queue offers a way for the device to report access faults from
endpoints. It is implemented on virtqueue #1. Whenever the host needs to
signal a fault, it fills one of the buffers offered by the guest and
interrupts it.
Tested-by: Bharat Bhushan <bharat.bhushan@nxp.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/virtio-iommu.c | 115 +++++++++++++++++++++++++++---
include/uapi/linux/virtio_iommu.h | 19 +++++
2 files changed, 125 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 0c7a7fa2628d..e6ff515d41c0 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -29,7 +29,8 @@
#define MSI_IOVA_LENGTH 0x100000
#define VIOMMU_REQUEST_VQ 0
-#define VIOMMU_NR_VQS 1
+#define VIOMMU_EVENT_VQ 1
+#define VIOMMU_NR_VQS 2
struct viommu_dev {
struct iommu_device iommu;
@@ -41,6 +42,7 @@ struct viommu_dev {
struct virtqueue *vqs[VIOMMU_NR_VQS];
spinlock_t request_lock;
struct list_head requests;
+ void *evts;
/* Device configuration */
struct iommu_domain_geometry geometry;
@@ -82,6 +84,15 @@ struct viommu_request {
char buf[];
};
+#define VIOMMU_FAULT_RESV_MASK 0xffffff00
+
+struct viommu_event {
+ union {
+ u32 head;
+ struct virtio_iommu_fault fault;
+ };
+};
+
#define to_viommu_domain(domain) \
container_of(domain, struct viommu_domain, domain)
@@ -503,6 +514,68 @@ static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
return ret;
}
+static int viommu_fault_handler(struct viommu_dev *viommu,
+ struct virtio_iommu_fault *fault)
+{
+ char *reason_str;
+
+ u8 reason = fault->reason;
+ u32 flags = le32_to_cpu(fault->flags);
+ u32 endpoint = le32_to_cpu(fault->endpoint);
+ u64 address = le64_to_cpu(fault->address);
+
+ switch (reason) {
+ case VIRTIO_IOMMU_FAULT_R_DOMAIN:
+ reason_str = "domain";
+ break;
+ case VIRTIO_IOMMU_FAULT_R_MAPPING:
+ reason_str = "page";
+ break;
+ case VIRTIO_IOMMU_FAULT_R_UNKNOWN:
+ default:
+ reason_str = "unknown";
+ break;
+ }
+
+ /* TODO: find EP by ID and report_iommu_fault */
+ if (flags & VIRTIO_IOMMU_FAULT_F_ADDRESS)
+ dev_err_ratelimited(viommu->dev, "%s fault from EP %u at %#llx [%s%s%s]\n",
+ reason_str, endpoint, address,
+ flags & VIRTIO_IOMMU_FAULT_F_READ ? "R" : "",
+ flags & VIRTIO_IOMMU_FAULT_F_WRITE ? "W" : "",
+ flags & VIRTIO_IOMMU_FAULT_F_EXEC ? "X" : "");
+ else
+ dev_err_ratelimited(viommu->dev, "%s fault from EP %u\n",
+ reason_str, endpoint);
+ return 0;
+}
+
+static void viommu_event_handler(struct virtqueue *vq)
+{
+ int ret;
+ unsigned int len;
+ struct scatterlist sg[1];
+ struct viommu_event *evt;
+ struct viommu_dev *viommu = vq->vdev->priv;
+
+ while ((evt = virtqueue_get_buf(vq, &len)) != NULL) {
+ if (len > sizeof(*evt)) {
+ dev_err(viommu->dev,
+ "invalid event buffer (len %u != %zu)\n",
+ len, sizeof(*evt));
+ } else if (!(evt->head & VIOMMU_FAULT_RESV_MASK)) {
+ viommu_fault_handler(viommu, &evt->fault);
+ }
+
+ sg_init_one(sg, evt, sizeof(*evt));
+ ret = virtqueue_add_inbuf(vq, sg, 1, evt, GFP_ATOMIC);
+ if (ret)
+ dev_err(viommu->dev, "could not add event buffer\n");
+ }
+
+ virtqueue_kick(vq);
+}
+
/* IOMMU API */
static struct iommu_domain *viommu_domain_alloc(unsigned type)
@@ -885,16 +958,35 @@ static struct iommu_ops viommu_ops = {
static int viommu_init_vqs(struct viommu_dev *viommu)
{
struct virtio_device *vdev = dev_to_virtio(viommu->dev);
- const char *name = "request";
- void *ret;
+ const char *names[] = { "request", "event" };
+ vq_callback_t *callbacks[] = {
+ NULL, /* No async requests */
+ viommu_event_handler,
+ };
- ret = virtio_find_single_vq(vdev, NULL, name);
- if (IS_ERR(ret)) {
- dev_err(viommu->dev, "cannot find VQ\n");
- return PTR_ERR(ret);
- }
+ return virtio_find_vqs(vdev, VIOMMU_NR_VQS, viommu->vqs, callbacks,
+ names, NULL);
+}
- viommu->vqs[VIOMMU_REQUEST_VQ] = ret;
+static int viommu_fill_evtq(struct viommu_dev *viommu)
+{
+ int i, ret;
+ struct scatterlist sg[1];
+ struct viommu_event *evts;
+ struct virtqueue *vq = viommu->vqs[VIOMMU_EVENT_VQ];
+ size_t nr_evts = vq->num_free;
+
+ viommu->evts = evts = devm_kmalloc_array(viommu->dev, nr_evts,
+ sizeof(*evts), GFP_KERNEL);
+ if (!evts)
+ return -ENOMEM;
+
+ for (i = 0; i < nr_evts; i++) {
+ sg_init_one(sg, &evts[i], sizeof(*evts));
+ ret = virtqueue_add_inbuf(vq, sg, 1, &evts[i], GFP_KERNEL);
+ if (ret)
+ return ret;
+ }
return 0;
}
@@ -963,6 +1055,11 @@ static int viommu_probe(struct virtio_device *vdev)
virtio_device_ready(vdev);
+ /* Populate the event queue with buffers */
+ ret = viommu_fill_evtq(viommu);
+ if (ret)
+ goto err_free_vqs;
+
ret = iommu_device_sysfs_add(&viommu->iommu, dev, NULL, "%s",
virtio_bus_name(vdev));
if (ret)
diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
index ae6145cf5928..ba1b460c9944 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -139,4 +139,23 @@ struct virtio_iommu_req_probe {
*/
};
+/* Fault types */
+#define VIRTIO_IOMMU_FAULT_R_UNKNOWN 0
+#define VIRTIO_IOMMU_FAULT_R_DOMAIN 1
+#define VIRTIO_IOMMU_FAULT_R_MAPPING 2
+
+#define VIRTIO_IOMMU_FAULT_F_READ (1 << 0)
+#define VIRTIO_IOMMU_FAULT_F_WRITE (1 << 1)
+#define VIRTIO_IOMMU_FAULT_F_EXEC (1 << 2)
+#define VIRTIO_IOMMU_FAULT_F_ADDRESS (1 << 8)
+
+struct virtio_iommu_fault {
+ __u8 reason;
+ __u8 reserved[3];
+ __le32 flags;
+ __le32 endpoint;
+ __u8 reserved2[4];
+ __le64 address;
+};
+
#endif
--
2.19.1
^ permalink raw reply related
* [PATCH v6 6/7] iommu/virtio: Add probe request
From: Jean-Philippe Brucker @ 2018-12-11 18:21 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
When the device offers the probe feature, send a probe request for each
device managed by the IOMMU. Extract RESV_MEM information. When we
encounter a MSI doorbell region, set it up as a IOMMU_RESV_MSI region.
This will tell other subsystems that there is no need to map the MSI
doorbell in the virtio-iommu, because MSIs bypass it.
Tested-by: Bharat Bhushan <bharat.bhushan@nxp.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/iommu/virtio-iommu.c | 156 ++++++++++++++++++++++++++++--
include/uapi/linux/virtio_iommu.h | 36 +++++++
2 files changed, 186 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 7540dab9c8dc..0c7a7fa2628d 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -46,6 +46,7 @@ struct viommu_dev {
struct iommu_domain_geometry geometry;
u64 pgsize_bitmap;
u8 domain_bits;
+ u32 probe_size;
};
struct viommu_mapping {
@@ -67,8 +68,10 @@ struct viommu_domain {
};
struct viommu_endpoint {
+ struct device *dev;
struct viommu_dev *viommu;
struct viommu_domain *vdomain;
+ struct list_head resv_regions;
};
struct viommu_request {
@@ -119,6 +122,9 @@ static off_t viommu_get_write_desc_offset(struct viommu_dev *viommu,
{
size_t tail_size = sizeof(struct virtio_iommu_req_tail);
+ if (req->type == VIRTIO_IOMMU_T_PROBE)
+ return len - viommu->probe_size - tail_size;
+
return len - tail_size;
}
@@ -393,6 +399,110 @@ static int viommu_replay_mappings(struct viommu_domain *vdomain)
return ret;
}
+static int viommu_add_resv_mem(struct viommu_endpoint *vdev,
+ struct virtio_iommu_probe_resv_mem *mem,
+ size_t len)
+{
+ size_t size;
+ u64 start64, end64;
+ phys_addr_t start, end;
+ struct iommu_resv_region *region = NULL;
+ unsigned long prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+ start = start64 = le64_to_cpu(mem->start);
+ end = end64 = le64_to_cpu(mem->end);
+ size = end64 - start64 + 1;
+
+ /* Catch any overflow, including the unlikely end64 - start64 + 1 = 0 */
+ if (start != start64 || end != end64 || size < end64 - start64)
+ return -EOVERFLOW;
+
+ if (len < sizeof(*mem))
+ return -EINVAL;
+
+ switch (mem->subtype) {
+ default:
+ dev_warn(vdev->dev, "unknown resv mem subtype 0x%x\n",
+ mem->subtype);
+ /* Fall-through */
+ case VIRTIO_IOMMU_RESV_MEM_T_RESERVED:
+ region = iommu_alloc_resv_region(start, size, 0,
+ IOMMU_RESV_RESERVED);
+ break;
+ case VIRTIO_IOMMU_RESV_MEM_T_MSI:
+ region = iommu_alloc_resv_region(start, size, prot,
+ IOMMU_RESV_MSI);
+ break;
+ }
+ if (!region)
+ return -ENOMEM;
+
+ list_add(&vdev->resv_regions, ®ion->list);
+ return 0;
+}
+
+static int viommu_probe_endpoint(struct viommu_dev *viommu, struct device *dev)
+{
+ int ret;
+ u16 type, len;
+ size_t cur = 0;
+ size_t probe_len;
+ struct virtio_iommu_req_probe *probe;
+ struct virtio_iommu_probe_property *prop;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct viommu_endpoint *vdev = fwspec->iommu_priv;
+
+ if (!fwspec->num_ids)
+ return -EINVAL;
+
+ probe_len = sizeof(*probe) + viommu->probe_size +
+ sizeof(struct virtio_iommu_req_tail);
+ probe = kzalloc(probe_len, GFP_KERNEL);
+ if (!probe)
+ return -ENOMEM;
+
+ probe->head.type = VIRTIO_IOMMU_T_PROBE;
+ /*
+ * For now, assume that properties of an endpoint that outputs multiple
+ * IDs are consistent. Only probe the first one.
+ */
+ probe->endpoint = cpu_to_le32(fwspec->ids[0]);
+
+ ret = viommu_send_req_sync(viommu, probe, probe_len);
+ if (ret)
+ goto out_free;
+
+ prop = (void *)probe->properties;
+ type = le16_to_cpu(prop->type) & VIRTIO_IOMMU_PROBE_T_MASK;
+
+ while (type != VIRTIO_IOMMU_PROBE_T_NONE &&
+ cur < viommu->probe_size) {
+ len = le16_to_cpu(prop->length) + sizeof(*prop);
+
+ switch (type) {
+ case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
+ ret = viommu_add_resv_mem(vdev, (void *)prop, len);
+ break;
+ default:
+ dev_err(dev, "unknown viommu prop 0x%x\n", type);
+ }
+
+ if (ret)
+ dev_err(dev, "failed to parse viommu prop 0x%x\n", type);
+
+ cur += len;
+ if (cur >= viommu->probe_size)
+ break;
+
+ prop = (void *)probe->properties + cur;
+ type = le16_to_cpu(prop->type) & VIRTIO_IOMMU_PROBE_T_MASK;
+ }
+
+out_free:
+ kfree(probe);
+ return ret;
+}
+
/* IOMMU API */
static struct iommu_domain *viommu_domain_alloc(unsigned type)
@@ -614,15 +724,33 @@ static void viommu_iotlb_sync(struct iommu_domain *domain)
static void viommu_get_resv_regions(struct device *dev, struct list_head *head)
{
- struct iommu_resv_region *region;
+ struct iommu_resv_region *entry, *new_entry, *msi = NULL;
+ struct viommu_endpoint *vdev = dev->iommu_fwspec->iommu_priv;
int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
- region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH, prot,
- IOMMU_RESV_SW_MSI);
- if (!region)
- return;
+ list_for_each_entry(entry, &vdev->resv_regions, list) {
+ if (entry->type == IOMMU_RESV_MSI)
+ msi = entry;
+
+ new_entry = kmemdup(entry, sizeof(*entry), GFP_KERNEL);
+ if (!new_entry)
+ return;
+ list_add_tail(&new_entry->list, head);
+ }
+
+ /*
+ * If the device didn't register any bypass MSI window, add a
+ * software-mapped region.
+ */
+ if (!msi) {
+ msi = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
+ prot, IOMMU_RESV_SW_MSI);
+ if (!msi)
+ return;
+
+ list_add_tail(&msi->list, head);
+ }
- list_add_tail(®ion->list, head);
iommu_dma_get_resv_regions(dev, head);
}
@@ -670,9 +798,18 @@ static int viommu_add_device(struct device *dev)
if (!vdev)
return -ENOMEM;
+ vdev->dev = dev;
vdev->viommu = viommu;
+ INIT_LIST_HEAD(&vdev->resv_regions);
fwspec->iommu_priv = vdev;
+ if (viommu->probe_size) {
+ /* Get additional information for this endpoint */
+ ret = viommu_probe_endpoint(viommu, dev);
+ if (ret)
+ goto err_free_dev;
+ }
+
ret = iommu_device_link(&viommu->iommu, dev);
if (ret)
goto err_free_dev;
@@ -694,6 +831,7 @@ static int viommu_add_device(struct device *dev)
err_unlink_dev:
iommu_device_unlink(&viommu->iommu, dev);
err_free_dev:
+ viommu_put_resv_regions(dev, &vdev->resv_regions);
kfree(vdev);
return ret;
@@ -711,6 +849,7 @@ static void viommu_remove_device(struct device *dev)
iommu_group_remove_device(dev);
iommu_device_unlink(&vdev->viommu->iommu, dev);
+ viommu_put_resv_regions(dev, &vdev->resv_regions);
kfree(vdev);
}
@@ -810,6 +949,10 @@ static int viommu_probe(struct virtio_device *vdev)
struct virtio_iommu_config, domain_bits,
&viommu->domain_bits);
+ virtio_cread_feature(vdev, VIRTIO_IOMMU_F_PROBE,
+ struct virtio_iommu_config, probe_size,
+ &viommu->probe_size);
+
viommu->geometry = (struct iommu_domain_geometry) {
.aperture_start = input_start,
.aperture_end = input_end,
@@ -891,6 +1034,7 @@ static unsigned int features[] = {
VIRTIO_IOMMU_F_MAP_UNMAP,
VIRTIO_IOMMU_F_DOMAIN_BITS,
VIRTIO_IOMMU_F_INPUT_RANGE,
+ VIRTIO_IOMMU_F_PROBE,
};
static struct virtio_device_id id_table[] = {
diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
index 5e5fd62689fb..ae6145cf5928 100644
--- a/include/uapi/linux/virtio_iommu.h
+++ b/include/uapi/linux/virtio_iommu.h
@@ -14,6 +14,7 @@
#define VIRTIO_IOMMU_F_DOMAIN_BITS 1
#define VIRTIO_IOMMU_F_MAP_UNMAP 2
#define VIRTIO_IOMMU_F_BYPASS 3
+#define VIRTIO_IOMMU_F_PROBE 4
struct virtio_iommu_range {
__u64 start;
@@ -37,6 +38,7 @@ struct virtio_iommu_config {
#define VIRTIO_IOMMU_T_DETACH 0x02
#define VIRTIO_IOMMU_T_MAP 0x03
#define VIRTIO_IOMMU_T_UNMAP 0x04
+#define VIRTIO_IOMMU_T_PROBE 0x05
/* Status types */
#define VIRTIO_IOMMU_S_OK 0x00
@@ -103,4 +105,38 @@ struct virtio_iommu_req_unmap {
struct virtio_iommu_req_tail tail;
};
+#define VIRTIO_IOMMU_PROBE_T_NONE 0
+#define VIRTIO_IOMMU_PROBE_T_RESV_MEM 1
+
+#define VIRTIO_IOMMU_PROBE_T_MASK 0xfff
+
+struct virtio_iommu_probe_property {
+ __le16 type;
+ __le16 length;
+};
+
+#define VIRTIO_IOMMU_RESV_MEM_T_RESERVED 0
+#define VIRTIO_IOMMU_RESV_MEM_T_MSI 1
+
+struct virtio_iommu_probe_resv_mem {
+ struct virtio_iommu_probe_property head;
+ __u8 subtype;
+ __u8 reserved[3];
+ __le64 start;
+ __le64 end;
+};
+
+struct virtio_iommu_req_probe {
+ struct virtio_iommu_req_head head;
+ __le32 endpoint;
+ __u8 reserved[64];
+
+ __u8 properties[];
+
+ /*
+ * Tail follows the variable-length properties array. No padding,
+ * property lengths are all aligned on 8 bytes.
+ */
+};
+
#endif
--
2.19.1
^ permalink raw reply related
* [PATCH v6 5/7] iommu: Add virtio-iommu driver
From: Jean-Philippe Brucker @ 2018-12-11 18:21 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
The virtio IOMMU is a para-virtualized device, allowing to send IOMMU
requests such as map/unmap over virtio transport without emulating page
tables. This implementation handles ATTACH, DETACH, MAP and UNMAP
requests.
The bulk of the code transforms calls coming from the IOMMU API into
corresponding virtio requests. Mappings are kept in an interval tree
instead of page tables. A little more work is required for modular and x86
support, so for the moment the driver depends on CONFIG_VIRTIO=y and
CONFIG_ARM64.
Tested-by: Bharat Bhushan <bharat.bhushan@nxp.com>
Tested-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
MAINTAINERS | 7 +
drivers/iommu/Kconfig | 11 +
drivers/iommu/Makefile | 1 +
drivers/iommu/virtio-iommu.c | 916 ++++++++++++++++++++++++++++++
include/uapi/linux/virtio_ids.h | 1 +
include/uapi/linux/virtio_iommu.h | 106 ++++
6 files changed, 1042 insertions(+)
create mode 100644 drivers/iommu/virtio-iommu.c
create mode 100644 include/uapi/linux/virtio_iommu.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 8119141a926f..6d250bc7a4ae 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16041,6 +16041,13 @@ S: Maintained
F: drivers/virtio/virtio_input.c
F: include/uapi/linux/virtio_input.h
+VIRTIO IOMMU DRIVER
+M: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
+L: virtualization@lists.linux-foundation.org
+S: Maintained
+F: drivers/iommu/virtio-iommu.c
+F: include/uapi/linux/virtio_iommu.h
+
VIRTUAL BOX GUEST DEVICE DRIVER
M: Hans de Goede <hdegoede@redhat.com>
M: Arnd Bergmann <arnd@arndb.de>
diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
index d9a25715650e..d507fd754214 100644
--- a/drivers/iommu/Kconfig
+++ b/drivers/iommu/Kconfig
@@ -435,4 +435,15 @@ config QCOM_IOMMU
help
Support for IOMMU on certain Qualcomm SoCs.
+config VIRTIO_IOMMU
+ bool "Virtio IOMMU driver"
+ depends on VIRTIO=y
+ depends on ARM64
+ select IOMMU_API
+ select INTERVAL_TREE
+ help
+ Para-virtualised IOMMU driver with virtio.
+
+ Say Y here if you intend to run this kernel as a guest.
+
endif # IOMMU_SUPPORT
diff --git a/drivers/iommu/Makefile b/drivers/iommu/Makefile
index a158a68c8ea8..48d831a39281 100644
--- a/drivers/iommu/Makefile
+++ b/drivers/iommu/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_EXYNOS_IOMMU) += exynos-iommu.o
obj-$(CONFIG_FSL_PAMU) += fsl_pamu.o fsl_pamu_domain.o
obj-$(CONFIG_S390_IOMMU) += s390-iommu.o
obj-$(CONFIG_QCOM_IOMMU) += qcom_iommu.o
+obj-$(CONFIG_VIRTIO_IOMMU) += virtio-iommu.o
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
new file mode 100644
index 000000000000..7540dab9c8dc
--- /dev/null
+++ b/drivers/iommu/virtio-iommu.c
@@ -0,0 +1,916 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Virtio driver for the paravirtualized IOMMU
+ *
+ * Copyright (C) 2018 Arm Limited
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/amba/bus.h>
+#include <linux/delay.h>
+#include <linux/dma-iommu.h>
+#include <linux/freezer.h>
+#include <linux/interval_tree.h>
+#include <linux/iommu.h>
+#include <linux/module.h>
+#include <linux/of_iommu.h>
+#include <linux/of_platform.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/virtio.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_ids.h>
+#include <linux/wait.h>
+
+#include <uapi/linux/virtio_iommu.h>
+
+#define MSI_IOVA_BASE 0x8000000
+#define MSI_IOVA_LENGTH 0x100000
+
+#define VIOMMU_REQUEST_VQ 0
+#define VIOMMU_NR_VQS 1
+
+struct viommu_dev {
+ struct iommu_device iommu;
+ struct device *dev;
+ struct virtio_device *vdev;
+
+ struct ida domain_ids;
+
+ struct virtqueue *vqs[VIOMMU_NR_VQS];
+ spinlock_t request_lock;
+ struct list_head requests;
+
+ /* Device configuration */
+ struct iommu_domain_geometry geometry;
+ u64 pgsize_bitmap;
+ u8 domain_bits;
+};
+
+struct viommu_mapping {
+ phys_addr_t paddr;
+ struct interval_tree_node iova;
+ u32 flags;
+};
+
+struct viommu_domain {
+ struct iommu_domain domain;
+ struct viommu_dev *viommu;
+ struct mutex mutex; /* protects viommu pointer */
+ unsigned int id;
+
+ spinlock_t mappings_lock;
+ struct rb_root_cached mappings;
+
+ unsigned long nr_endpoints;
+};
+
+struct viommu_endpoint {
+ struct viommu_dev *viommu;
+ struct viommu_domain *vdomain;
+};
+
+struct viommu_request {
+ struct list_head list;
+ void *writeback;
+ unsigned int write_offset;
+ unsigned int len;
+ char buf[];
+};
+
+#define to_viommu_domain(domain) \
+ container_of(domain, struct viommu_domain, domain)
+
+static int viommu_get_req_errno(void *buf, size_t len)
+{
+ struct virtio_iommu_req_tail *tail = buf + len - sizeof(*tail);
+
+ switch (tail->status) {
+ case VIRTIO_IOMMU_S_OK:
+ return 0;
+ case VIRTIO_IOMMU_S_UNSUPP:
+ return -ENOSYS;
+ case VIRTIO_IOMMU_S_INVAL:
+ return -EINVAL;
+ case VIRTIO_IOMMU_S_RANGE:
+ return -ERANGE;
+ case VIRTIO_IOMMU_S_NOENT:
+ return -ENOENT;
+ case VIRTIO_IOMMU_S_FAULT:
+ return -EFAULT;
+ case VIRTIO_IOMMU_S_IOERR:
+ case VIRTIO_IOMMU_S_DEVERR:
+ default:
+ return -EIO;
+ }
+}
+
+static void viommu_set_req_status(void *buf, size_t len, int status)
+{
+ struct virtio_iommu_req_tail *tail = buf + len - sizeof(*tail);
+
+ tail->status = status;
+}
+
+static off_t viommu_get_write_desc_offset(struct viommu_dev *viommu,
+ struct virtio_iommu_req_head *req,
+ size_t len)
+{
+ size_t tail_size = sizeof(struct virtio_iommu_req_tail);
+
+ return len - tail_size;
+}
+
+/*
+ * __viommu_sync_req - Complete all in-flight requests
+ *
+ * Wait for all added requests to complete. When this function returns, all
+ * requests that were in-flight at the time of the call have completed.
+ */
+static int __viommu_sync_req(struct viommu_dev *viommu)
+{
+ int ret = 0;
+ unsigned int len;
+ size_t write_len;
+ struct viommu_request *req;
+ struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
+
+ assert_spin_locked(&viommu->request_lock);
+
+ virtqueue_kick(vq);
+
+ while (!list_empty(&viommu->requests)) {
+ len = 0;
+ req = virtqueue_get_buf(vq, &len);
+ if (!req)
+ continue;
+
+ if (!len)
+ viommu_set_req_status(req->buf, req->len,
+ VIRTIO_IOMMU_S_IOERR);
+
+ write_len = req->len - req->write_offset;
+ if (req->writeback && len == write_len)
+ memcpy(req->writeback, req->buf + req->write_offset,
+ write_len);
+
+ list_del(&req->list);
+ kfree(req);
+ }
+
+ return ret;
+}
+
+static int viommu_sync_req(struct viommu_dev *viommu)
+{
+ int ret;
+ unsigned long flags;
+
+ spin_lock_irqsave(&viommu->request_lock, flags);
+ ret = __viommu_sync_req(viommu);
+ if (ret)
+ dev_dbg(viommu->dev, "could not sync requests (%d)\n", ret);
+ spin_unlock_irqrestore(&viommu->request_lock, flags);
+
+ return ret;
+}
+
+/*
+ * __viommu_add_request - Add one request to the queue
+ * @buf: pointer to the request buffer
+ * @len: length of the request buffer
+ * @writeback: copy data back to the buffer when the request completes.
+ *
+ * Add a request to the queue. Only synchronize the queue if it's already full.
+ * Otherwise don't kick the queue nor wait for requests to complete.
+ *
+ * When @writeback is true, data written by the device, including the request
+ * status, is copied into @buf after the request completes. This is unsafe if
+ * the caller allocates @buf on stack and drops the lock between add_req() and
+ * sync_req().
+ *
+ * Return 0 if the request was successfully added to the queue.
+ */
+static int __viommu_add_req(struct viommu_dev *viommu, void *buf, size_t len,
+ bool writeback)
+{
+ int ret;
+ off_t write_offset;
+ struct viommu_request *req;
+ struct scatterlist top_sg, bottom_sg;
+ struct scatterlist *sg[2] = { &top_sg, &bottom_sg };
+ struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
+
+ assert_spin_locked(&viommu->request_lock);
+
+ write_offset = viommu_get_write_desc_offset(viommu, buf, len);
+ if (write_offset <= 0)
+ return -EINVAL;
+
+ req = kzalloc(sizeof(*req) + len, GFP_ATOMIC);
+ if (!req)
+ return -ENOMEM;
+
+ req->len = len;
+ if (writeback) {
+ req->writeback = buf + write_offset;
+ req->write_offset = write_offset;
+ }
+ memcpy(&req->buf, buf, write_offset);
+
+ sg_init_one(&top_sg, req->buf, write_offset);
+ sg_init_one(&bottom_sg, req->buf + write_offset, len - write_offset);
+
+ ret = virtqueue_add_sgs(vq, sg, 1, 1, req, GFP_ATOMIC);
+ if (ret == -ENOSPC) {
+ /* If the queue is full, sync and retry */
+ if (!__viommu_sync_req(viommu))
+ ret = virtqueue_add_sgs(vq, sg, 1, 1, req, GFP_ATOMIC);
+ }
+ if (ret)
+ goto err_free;
+
+ list_add_tail(&req->list, &viommu->requests);
+ return 0;
+
+err_free:
+ kfree(req);
+ return ret;
+}
+
+static int viommu_add_req(struct viommu_dev *viommu, void *buf, size_t len)
+{
+ int ret;
+ unsigned long flags;
+
+ spin_lock_irqsave(&viommu->request_lock, flags);
+ ret = __viommu_add_req(viommu, buf, len, false);
+ if (ret)
+ dev_dbg(viommu->dev, "could not add request: %d\n", ret);
+ spin_unlock_irqrestore(&viommu->request_lock, flags);
+
+ return ret;
+}
+
+/*
+ * Send a request and wait for it to complete. Return the request status (as an
+ * errno)
+ */
+static int viommu_send_req_sync(struct viommu_dev *viommu, void *buf,
+ size_t len)
+{
+ int ret;
+ unsigned long flags;
+
+ spin_lock_irqsave(&viommu->request_lock, flags);
+
+ ret = __viommu_add_req(viommu, buf, len, true);
+ if (ret) {
+ dev_dbg(viommu->dev, "could not add request (%d)\n", ret);
+ goto out_unlock;
+ }
+
+ ret = __viommu_sync_req(viommu);
+ if (ret) {
+ dev_dbg(viommu->dev, "could not sync requests (%d)\n", ret);
+ /* Fall-through (get the actual request status) */
+ }
+
+ ret = viommu_get_req_errno(buf, len);
+out_unlock:
+ spin_unlock_irqrestore(&viommu->request_lock, flags);
+ return ret;
+}
+
+/*
+ * viommu_add_mapping - add a mapping to the internal tree
+ *
+ * On success, return the new mapping. Otherwise return NULL.
+ */
+static int viommu_add_mapping(struct viommu_domain *vdomain, unsigned long iova,
+ phys_addr_t paddr, size_t size, u32 flags)
+{
+ unsigned long irqflags;
+ struct viommu_mapping *mapping;
+
+ mapping = kzalloc(sizeof(*mapping), GFP_ATOMIC);
+ if (!mapping)
+ return -ENOMEM;
+
+ mapping->paddr = paddr;
+ mapping->iova.start = iova;
+ mapping->iova.last = iova + size - 1;
+ mapping->flags = flags;
+
+ spin_lock_irqsave(&vdomain->mappings_lock, irqflags);
+ interval_tree_insert(&mapping->iova, &vdomain->mappings);
+ spin_unlock_irqrestore(&vdomain->mappings_lock, irqflags);
+
+ return 0;
+}
+
+/*
+ * viommu_del_mappings - remove mappings from the internal tree
+ *
+ * @vdomain: the domain
+ * @iova: start of the range
+ * @size: size of the range. A size of 0 corresponds to the entire address
+ * space.
+ *
+ * On success, returns the number of unmapped bytes (>= size)
+ */
+static size_t viommu_del_mappings(struct viommu_domain *vdomain,
+ unsigned long iova, size_t size)
+{
+ size_t unmapped = 0;
+ unsigned long flags;
+ unsigned long last = iova + size - 1;
+ struct viommu_mapping *mapping = NULL;
+ struct interval_tree_node *node, *next;
+
+ spin_lock_irqsave(&vdomain->mappings_lock, flags);
+ next = interval_tree_iter_first(&vdomain->mappings, iova, last);
+ while (next) {
+ node = next;
+ mapping = container_of(node, struct viommu_mapping, iova);
+ next = interval_tree_iter_next(node, iova, last);
+
+ /* Trying to split a mapping? */
+ if (mapping->iova.start < iova)
+ break;
+
+ /*
+ * Virtio-iommu doesn't allow UNMAP to split a mapping created
+ * with a single MAP request, so remove the full mapping.
+ */
+ unmapped += mapping->iova.last - mapping->iova.start + 1;
+
+ interval_tree_remove(node, &vdomain->mappings);
+ kfree(mapping);
+ }
+ spin_unlock_irqrestore(&vdomain->mappings_lock, flags);
+
+ return unmapped;
+}
+
+/*
+ * viommu_replay_mappings - re-send MAP requests
+ *
+ * When reattaching a domain that was previously detached from all endpoints,
+ * mappings were deleted from the device. Re-create the mappings available in
+ * the internal tree.
+ */
+static int viommu_replay_mappings(struct viommu_domain *vdomain)
+{
+ int ret = 0;
+ unsigned long flags;
+ struct viommu_mapping *mapping;
+ struct interval_tree_node *node;
+ struct virtio_iommu_req_map map;
+
+ spin_lock_irqsave(&vdomain->mappings_lock, flags);
+ node = interval_tree_iter_first(&vdomain->mappings, 0, -1UL);
+ while (node) {
+ mapping = container_of(node, struct viommu_mapping, iova);
+ map = (struct virtio_iommu_req_map) {
+ .head.type = VIRTIO_IOMMU_T_MAP,
+ .domain = cpu_to_le32(vdomain->id),
+ .virt_start = cpu_to_le64(mapping->iova.start),
+ .virt_end = cpu_to_le64(mapping->iova.last),
+ .phys_start = cpu_to_le64(mapping->paddr),
+ .flags = cpu_to_le32(mapping->flags),
+ };
+
+ ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
+ if (ret)
+ break;
+
+ node = interval_tree_iter_next(node, 0, -1UL);
+ }
+ spin_unlock_irqrestore(&vdomain->mappings_lock, flags);
+
+ return ret;
+}
+
+/* IOMMU API */
+
+static struct iommu_domain *viommu_domain_alloc(unsigned type)
+{
+ struct viommu_domain *vdomain;
+
+ if (type != IOMMU_DOMAIN_UNMANAGED && type != IOMMU_DOMAIN_DMA)
+ return NULL;
+
+ vdomain = kzalloc(sizeof(*vdomain), GFP_KERNEL);
+ if (!vdomain)
+ return NULL;
+
+ mutex_init(&vdomain->mutex);
+ spin_lock_init(&vdomain->mappings_lock);
+ vdomain->mappings = RB_ROOT_CACHED;
+
+ if (type == IOMMU_DOMAIN_DMA &&
+ iommu_get_dma_cookie(&vdomain->domain)) {
+ kfree(vdomain);
+ return NULL;
+ }
+
+ return &vdomain->domain;
+}
+
+static int viommu_domain_finalise(struct viommu_dev *viommu,
+ struct iommu_domain *domain)
+{
+ int ret;
+ struct viommu_domain *vdomain = to_viommu_domain(domain);
+ unsigned int max_domain = viommu->domain_bits > 31 ? ~0 :
+ (1U << viommu->domain_bits) - 1;
+
+ vdomain->viommu = viommu;
+
+ domain->pgsize_bitmap = viommu->pgsize_bitmap;
+ domain->geometry = viommu->geometry;
+
+ ret = ida_alloc_max(&viommu->domain_ids, max_domain, GFP_KERNEL);
+ if (ret >= 0)
+ vdomain->id = (unsigned int)ret;
+
+ return ret > 0 ? 0 : ret;
+}
+
+static void viommu_domain_free(struct iommu_domain *domain)
+{
+ struct viommu_domain *vdomain = to_viommu_domain(domain);
+
+ iommu_put_dma_cookie(domain);
+
+ /* Free all remaining mappings (size 2^64) */
+ viommu_del_mappings(vdomain, 0, 0);
+
+ if (vdomain->viommu)
+ ida_free(&vdomain->viommu->domain_ids, vdomain->id);
+
+ kfree(vdomain);
+}
+
+static int viommu_attach_dev(struct iommu_domain *domain, struct device *dev)
+{
+ int i;
+ int ret = 0;
+ struct virtio_iommu_req_attach req;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+ struct viommu_endpoint *vdev = fwspec->iommu_priv;
+ struct viommu_domain *vdomain = to_viommu_domain(domain);
+
+ mutex_lock(&vdomain->mutex);
+ if (!vdomain->viommu) {
+ /*
+ * Properly initialize the domain now that we know which viommu
+ * owns it.
+ */
+ ret = viommu_domain_finalise(vdev->viommu, domain);
+ } else if (vdomain->viommu != vdev->viommu) {
+ dev_err(dev, "cannot attach to foreign vIOMMU\n");
+ ret = -EXDEV;
+ }
+ mutex_unlock(&vdomain->mutex);
+
+ if (ret)
+ return ret;
+
+ /*
+ * In the virtio-iommu device, when attaching the endpoint to a new
+ * domain, it is detached from the old one and, if as as a result the
+ * old domain isn't attached to any endpoint, all mappings are removed
+ * from the old domain and it is freed.
+ *
+ * In the driver the old domain still exists, and its mappings will be
+ * recreated if it gets reattached to an endpoint. Otherwise it will be
+ * freed explicitly.
+ *
+ * vdev->vdomain is protected by group->mutex
+ */
+ if (vdev->vdomain)
+ vdev->vdomain->nr_endpoints--;
+
+ req = (struct virtio_iommu_req_attach) {
+ .head.type = VIRTIO_IOMMU_T_ATTACH,
+ .domain = cpu_to_le32(vdomain->id),
+ };
+
+ for (i = 0; i < fwspec->num_ids; i++) {
+ req.endpoint = cpu_to_le32(fwspec->ids[i]);
+
+ ret = viommu_send_req_sync(vdomain->viommu, &req, sizeof(req));
+ if (ret)
+ return ret;
+ }
+
+ if (!vdomain->nr_endpoints) {
+ /*
+ * This endpoint is the first to be attached to the domain.
+ * Replay existing mappings (e.g. SW MSI).
+ */
+ ret = viommu_replay_mappings(vdomain);
+ if (ret)
+ return ret;
+ }
+
+ vdomain->nr_endpoints++;
+ vdev->vdomain = vdomain;
+
+ return 0;
+}
+
+static int viommu_map(struct iommu_domain *domain, unsigned long iova,
+ phys_addr_t paddr, size_t size, int prot)
+{
+ int ret;
+ int flags;
+ struct virtio_iommu_req_map map;
+ struct viommu_domain *vdomain = to_viommu_domain(domain);
+
+ flags = (prot & IOMMU_READ ? VIRTIO_IOMMU_MAP_F_READ : 0) |
+ (prot & IOMMU_WRITE ? VIRTIO_IOMMU_MAP_F_WRITE : 0) |
+ (prot & IOMMU_MMIO ? VIRTIO_IOMMU_MAP_F_MMIO : 0);
+
+ ret = viommu_add_mapping(vdomain, iova, paddr, size, flags);
+ if (ret)
+ return ret;
+
+ map = (struct virtio_iommu_req_map) {
+ .head.type = VIRTIO_IOMMU_T_MAP,
+ .domain = cpu_to_le32(vdomain->id),
+ .virt_start = cpu_to_le64(iova),
+ .phys_start = cpu_to_le64(paddr),
+ .virt_end = cpu_to_le64(iova + size - 1),
+ .flags = cpu_to_le32(flags),
+ };
+
+ if (!vdomain->nr_endpoints)
+ return 0;
+
+ ret = viommu_send_req_sync(vdomain->viommu, &map, sizeof(map));
+ if (ret)
+ viommu_del_mappings(vdomain, iova, size);
+
+ return ret;
+}
+
+static size_t viommu_unmap(struct iommu_domain *domain, unsigned long iova,
+ size_t size)
+{
+ int ret = 0;
+ size_t unmapped;
+ struct virtio_iommu_req_unmap unmap;
+ struct viommu_domain *vdomain = to_viommu_domain(domain);
+
+ unmapped = viommu_del_mappings(vdomain, iova, size);
+ if (unmapped < size)
+ return 0;
+
+ /* Device already removed all mappings after detach. */
+ if (!vdomain->nr_endpoints)
+ return unmapped;
+
+ unmap = (struct virtio_iommu_req_unmap) {
+ .head.type = VIRTIO_IOMMU_T_UNMAP,
+ .domain = cpu_to_le32(vdomain->id),
+ .virt_start = cpu_to_le64(iova),
+ .virt_end = cpu_to_le64(iova + unmapped - 1),
+ };
+
+ ret = viommu_add_req(vdomain->viommu, &unmap, sizeof(unmap));
+ return ret ? 0 : unmapped;
+}
+
+static phys_addr_t viommu_iova_to_phys(struct iommu_domain *domain,
+ dma_addr_t iova)
+{
+ u64 paddr = 0;
+ unsigned long flags;
+ struct viommu_mapping *mapping;
+ struct interval_tree_node *node;
+ struct viommu_domain *vdomain = to_viommu_domain(domain);
+
+ spin_lock_irqsave(&vdomain->mappings_lock, flags);
+ node = interval_tree_iter_first(&vdomain->mappings, iova, iova);
+ if (node) {
+ mapping = container_of(node, struct viommu_mapping, iova);
+ paddr = mapping->paddr + (iova - mapping->iova.start);
+ }
+ spin_unlock_irqrestore(&vdomain->mappings_lock, flags);
+
+ return paddr;
+}
+
+static void viommu_iotlb_sync(struct iommu_domain *domain)
+{
+ struct viommu_domain *vdomain = to_viommu_domain(domain);
+
+ viommu_sync_req(vdomain->viommu);
+}
+
+static void viommu_get_resv_regions(struct device *dev, struct list_head *head)
+{
+ struct iommu_resv_region *region;
+ int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
+
+ region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH, prot,
+ IOMMU_RESV_SW_MSI);
+ if (!region)
+ return;
+
+ list_add_tail(®ion->list, head);
+ iommu_dma_get_resv_regions(dev, head);
+}
+
+static void viommu_put_resv_regions(struct device *dev, struct list_head *head)
+{
+ struct iommu_resv_region *entry, *next;
+
+ list_for_each_entry_safe(entry, next, head, list)
+ kfree(entry);
+}
+
+static struct iommu_ops viommu_ops;
+static struct virtio_driver virtio_iommu_drv;
+
+static int viommu_match_node(struct device *dev, void *data)
+{
+ return dev->parent->fwnode == data;
+}
+
+static struct viommu_dev *viommu_get_by_fwnode(struct fwnode_handle *fwnode)
+{
+ struct device *dev = driver_find_device(&virtio_iommu_drv.driver, NULL,
+ fwnode, viommu_match_node);
+ put_device(dev);
+
+ return dev ? dev_to_virtio(dev)->priv : NULL;
+}
+
+static int viommu_add_device(struct device *dev)
+{
+ int ret;
+ struct iommu_group *group;
+ struct viommu_endpoint *vdev;
+ struct viommu_dev *viommu = NULL;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+
+ if (!fwspec || fwspec->ops != &viommu_ops)
+ return -ENODEV;
+
+ viommu = viommu_get_by_fwnode(fwspec->iommu_fwnode);
+ if (!viommu)
+ return -ENODEV;
+
+ vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
+ if (!vdev)
+ return -ENOMEM;
+
+ vdev->viommu = viommu;
+ fwspec->iommu_priv = vdev;
+
+ ret = iommu_device_link(&viommu->iommu, dev);
+ if (ret)
+ goto err_free_dev;
+
+ /*
+ * Last step creates a default domain and attaches to it. Everything
+ * must be ready.
+ */
+ group = iommu_group_get_for_dev(dev);
+ if (IS_ERR(group)) {
+ ret = PTR_ERR(group);
+ goto err_unlink_dev;
+ }
+
+ iommu_group_put(group);
+
+ return PTR_ERR_OR_ZERO(group);
+
+err_unlink_dev:
+ iommu_device_unlink(&viommu->iommu, dev);
+err_free_dev:
+ kfree(vdev);
+
+ return ret;
+}
+
+static void viommu_remove_device(struct device *dev)
+{
+ struct viommu_endpoint *vdev;
+ struct iommu_fwspec *fwspec = dev->iommu_fwspec;
+
+ if (!fwspec || fwspec->ops != &viommu_ops)
+ return;
+
+ vdev = fwspec->iommu_priv;
+
+ iommu_group_remove_device(dev);
+ iommu_device_unlink(&vdev->viommu->iommu, dev);
+ kfree(vdev);
+}
+
+static struct iommu_group *viommu_device_group(struct device *dev)
+{
+ if (dev_is_pci(dev))
+ return pci_device_group(dev);
+ else
+ return generic_device_group(dev);
+}
+
+static int viommu_of_xlate(struct device *dev, struct of_phandle_args *args)
+{
+ return iommu_fwspec_add_ids(dev, args->args, 1);
+}
+
+static struct iommu_ops viommu_ops = {
+ .domain_alloc = viommu_domain_alloc,
+ .domain_free = viommu_domain_free,
+ .attach_dev = viommu_attach_dev,
+ .map = viommu_map,
+ .unmap = viommu_unmap,
+ .iova_to_phys = viommu_iova_to_phys,
+ .iotlb_sync = viommu_iotlb_sync,
+ .add_device = viommu_add_device,
+ .remove_device = viommu_remove_device,
+ .device_group = viommu_device_group,
+ .get_resv_regions = viommu_get_resv_regions,
+ .put_resv_regions = viommu_put_resv_regions,
+ .of_xlate = viommu_of_xlate,
+};
+
+static int viommu_init_vqs(struct viommu_dev *viommu)
+{
+ struct virtio_device *vdev = dev_to_virtio(viommu->dev);
+ const char *name = "request";
+ void *ret;
+
+ ret = virtio_find_single_vq(vdev, NULL, name);
+ if (IS_ERR(ret)) {
+ dev_err(viommu->dev, "cannot find VQ\n");
+ return PTR_ERR(ret);
+ }
+
+ viommu->vqs[VIOMMU_REQUEST_VQ] = ret;
+
+ return 0;
+}
+
+static int viommu_probe(struct virtio_device *vdev)
+{
+ struct device *parent_dev = vdev->dev.parent;
+ struct viommu_dev *viommu = NULL;
+ struct device *dev = &vdev->dev;
+ u64 input_start = 0;
+ u64 input_end = -1UL;
+ int ret;
+
+ if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1) ||
+ !virtio_has_feature(vdev, VIRTIO_IOMMU_F_MAP_UNMAP))
+ return -ENODEV;
+
+ viommu = devm_kzalloc(dev, sizeof(*viommu), GFP_KERNEL);
+ if (!viommu)
+ return -ENOMEM;
+
+ spin_lock_init(&viommu->request_lock);
+ ida_init(&viommu->domain_ids);
+ viommu->dev = dev;
+ viommu->vdev = vdev;
+ INIT_LIST_HEAD(&viommu->requests);
+
+ ret = viommu_init_vqs(viommu);
+ if (ret)
+ return ret;
+
+ virtio_cread(vdev, struct virtio_iommu_config, page_size_mask,
+ &viommu->pgsize_bitmap);
+
+ if (!viommu->pgsize_bitmap) {
+ ret = -EINVAL;
+ goto err_free_vqs;
+ }
+
+ viommu->domain_bits = 32;
+
+ /* Optional features */
+ virtio_cread_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
+ struct virtio_iommu_config, input_range.start,
+ &input_start);
+
+ virtio_cread_feature(vdev, VIRTIO_IOMMU_F_INPUT_RANGE,
+ struct virtio_iommu_config, input_range.end,
+ &input_end);
+
+ virtio_cread_feature(vdev, VIRTIO_IOMMU_F_DOMAIN_BITS,
+ struct virtio_iommu_config, domain_bits,
+ &viommu->domain_bits);
+
+ viommu->geometry = (struct iommu_domain_geometry) {
+ .aperture_start = input_start,
+ .aperture_end = input_end,
+ .force_aperture = true,
+ };
+
+ viommu_ops.pgsize_bitmap = viommu->pgsize_bitmap;
+
+ virtio_device_ready(vdev);
+
+ ret = iommu_device_sysfs_add(&viommu->iommu, dev, NULL, "%s",
+ virtio_bus_name(vdev));
+ if (ret)
+ goto err_free_vqs;
+
+ iommu_device_set_ops(&viommu->iommu, &viommu_ops);
+ iommu_device_set_fwnode(&viommu->iommu, parent_dev->fwnode);
+
+ iommu_device_register(&viommu->iommu);
+
+#ifdef CONFIG_PCI
+ if (pci_bus_type.iommu_ops != &viommu_ops) {
+ pci_request_acs();
+ ret = bus_set_iommu(&pci_bus_type, &viommu_ops);
+ if (ret)
+ goto err_unregister;
+ }
+#endif
+#ifdef CONFIG_ARM_AMBA
+ if (amba_bustype.iommu_ops != &viommu_ops) {
+ ret = bus_set_iommu(&amba_bustype, &viommu_ops);
+ if (ret)
+ goto err_unregister;
+ }
+#endif
+ if (platform_bus_type.iommu_ops != &viommu_ops) {
+ ret = bus_set_iommu(&platform_bus_type, &viommu_ops);
+ if (ret)
+ goto err_unregister;
+ }
+
+ vdev->priv = viommu;
+
+ dev_info(dev, "input address: %u bits\n",
+ order_base_2(viommu->geometry.aperture_end));
+ dev_info(dev, "page mask: %#llx\n", viommu->pgsize_bitmap);
+
+ return 0;
+
+err_unregister:
+ iommu_device_sysfs_remove(&viommu->iommu);
+ iommu_device_unregister(&viommu->iommu);
+err_free_vqs:
+ vdev->config->del_vqs(vdev);
+
+ return ret;
+}
+
+static void viommu_remove(struct virtio_device *vdev)
+{
+ struct viommu_dev *viommu = vdev->priv;
+
+ iommu_device_sysfs_remove(&viommu->iommu);
+ iommu_device_unregister(&viommu->iommu);
+
+ /* Stop all virtqueues */
+ vdev->config->reset(vdev);
+ vdev->config->del_vqs(vdev);
+
+ dev_info(&vdev->dev, "device removed\n");
+}
+
+static void viommu_config_changed(struct virtio_device *vdev)
+{
+ dev_warn(&vdev->dev, "config changed\n");
+}
+
+static unsigned int features[] = {
+ VIRTIO_IOMMU_F_MAP_UNMAP,
+ VIRTIO_IOMMU_F_DOMAIN_BITS,
+ VIRTIO_IOMMU_F_INPUT_RANGE,
+};
+
+static struct virtio_device_id id_table[] = {
+ { VIRTIO_ID_IOMMU, VIRTIO_DEV_ANY_ID },
+ { 0 },
+};
+
+static struct virtio_driver virtio_iommu_drv = {
+ .driver.name = KBUILD_MODNAME,
+ .driver.owner = THIS_MODULE,
+ .id_table = id_table,
+ .feature_table = features,
+ .feature_table_size = ARRAY_SIZE(features),
+ .probe = viommu_probe,
+ .remove = viommu_remove,
+ .config_changed = viommu_config_changed,
+};
+
+module_virtio_driver(virtio_iommu_drv);
+
+MODULE_DESCRIPTION("Virtio IOMMU driver");
+MODULE_AUTHOR("Jean-Philippe Brucker <jean-philippe.brucker@arm.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
index 6d5c3b2d4f4d..cfe47c5d9a56 100644
--- a/include/uapi/linux/virtio_ids.h
+++ b/include/uapi/linux/virtio_ids.h
@@ -43,5 +43,6 @@
#define VIRTIO_ID_INPUT 18 /* virtio input */
#define VIRTIO_ID_VSOCK 19 /* virtio vsock transport */
#define VIRTIO_ID_CRYPTO 20 /* virtio crypto */
+#define VIRTIO_ID_IOMMU 23 /* virtio IOMMU */
#endif /* _LINUX_VIRTIO_IDS_H */
diff --git a/include/uapi/linux/virtio_iommu.h b/include/uapi/linux/virtio_iommu.h
new file mode 100644
index 000000000000..5e5fd62689fb
--- /dev/null
+++ b/include/uapi/linux/virtio_iommu.h
@@ -0,0 +1,106 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Virtio-iommu definition v0.9
+ *
+ * Copyright (C) 2018 Arm Ltd.
+ */
+#ifndef _UAPI_LINUX_VIRTIO_IOMMU_H
+#define _UAPI_LINUX_VIRTIO_IOMMU_H
+
+#include <linux/types.h>
+
+/* Feature bits */
+#define VIRTIO_IOMMU_F_INPUT_RANGE 0
+#define VIRTIO_IOMMU_F_DOMAIN_BITS 1
+#define VIRTIO_IOMMU_F_MAP_UNMAP 2
+#define VIRTIO_IOMMU_F_BYPASS 3
+
+struct virtio_iommu_range {
+ __u64 start;
+ __u64 end;
+};
+
+struct virtio_iommu_config {
+ /* Supported page sizes */
+ __u64 page_size_mask;
+ /* Supported IOVA range */
+ struct virtio_iommu_range input_range;
+ /* Max domain ID size */
+ __u8 domain_bits;
+ __u8 padding[3];
+ /* Probe buffer size */
+ __u32 probe_size;
+};
+
+/* Request types */
+#define VIRTIO_IOMMU_T_ATTACH 0x01
+#define VIRTIO_IOMMU_T_DETACH 0x02
+#define VIRTIO_IOMMU_T_MAP 0x03
+#define VIRTIO_IOMMU_T_UNMAP 0x04
+
+/* Status types */
+#define VIRTIO_IOMMU_S_OK 0x00
+#define VIRTIO_IOMMU_S_IOERR 0x01
+#define VIRTIO_IOMMU_S_UNSUPP 0x02
+#define VIRTIO_IOMMU_S_DEVERR 0x03
+#define VIRTIO_IOMMU_S_INVAL 0x04
+#define VIRTIO_IOMMU_S_RANGE 0x05
+#define VIRTIO_IOMMU_S_NOENT 0x06
+#define VIRTIO_IOMMU_S_FAULT 0x07
+
+struct virtio_iommu_req_head {
+ __u8 type;
+ __u8 reserved[3];
+};
+
+struct virtio_iommu_req_tail {
+ __u8 status;
+ __u8 reserved[3];
+};
+
+struct virtio_iommu_req_attach {
+ struct virtio_iommu_req_head head;
+ __le32 domain;
+ __le32 endpoint;
+ __u8 reserved[8];
+ struct virtio_iommu_req_tail tail;
+};
+
+struct virtio_iommu_req_detach {
+ struct virtio_iommu_req_head head;
+ __le32 domain;
+ __le32 endpoint;
+ __u8 reserved[8];
+ struct virtio_iommu_req_tail tail;
+};
+
+#define VIRTIO_IOMMU_MAP_F_READ (1 << 0)
+#define VIRTIO_IOMMU_MAP_F_WRITE (1 << 1)
+#define VIRTIO_IOMMU_MAP_F_EXEC (1 << 2)
+#define VIRTIO_IOMMU_MAP_F_MMIO (1 << 3)
+
+#define VIRTIO_IOMMU_MAP_F_MASK (VIRTIO_IOMMU_MAP_F_READ | \
+ VIRTIO_IOMMU_MAP_F_WRITE | \
+ VIRTIO_IOMMU_MAP_F_EXEC | \
+ VIRTIO_IOMMU_MAP_F_MMIO)
+
+struct virtio_iommu_req_map {
+ struct virtio_iommu_req_head head;
+ __le32 domain;
+ __le64 virt_start;
+ __le64 virt_end;
+ __le64 phys_start;
+ __le32 flags;
+ struct virtio_iommu_req_tail tail;
+};
+
+struct virtio_iommu_req_unmap {
+ struct virtio_iommu_req_head head;
+ __le32 domain;
+ __le64 virt_start;
+ __le64 virt_end;
+ __u8 reserved[4];
+ struct virtio_iommu_req_tail tail;
+};
+
+#endif
--
2.19.1
^ permalink raw reply related
* [PATCH v6 4/7] PCI: OF: Initialize dev->fwnode appropriately
From: Jean-Philippe Brucker @ 2018-12-11 18:21 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
For PCI devices that have an OF node, set the fwnode as well. This way
drivers that rely on fwnode don't need the special case described by
commit f94277af03ea ("of/platform: Initialise dev->fwnode appropriately").
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/pci/of.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 4c4217d0c3f1..c272ecfcd038 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -21,12 +21,15 @@ void pci_set_of_node(struct pci_dev *dev)
return;
dev->dev.of_node = of_pci_find_child_device(dev->bus->dev.of_node,
dev->devfn);
+ if (dev->dev.of_node)
+ dev->dev.fwnode = &dev->dev.of_node->fwnode;
}
void pci_release_of_node(struct pci_dev *dev)
{
of_node_put(dev->dev.of_node);
dev->dev.of_node = NULL;
+ dev->dev.fwnode = NULL;
}
void pci_set_bus_of_node(struct pci_bus *bus)
@@ -35,12 +38,16 @@ void pci_set_bus_of_node(struct pci_bus *bus)
bus->dev.of_node = pcibios_get_phb_of_node(bus);
else
bus->dev.of_node = of_node_get(bus->self->dev.of_node);
+
+ if (bus->dev.of_node)
+ bus->dev.fwnode = &bus->dev.of_node->fwnode;
}
void pci_release_bus_of_node(struct pci_bus *bus)
{
of_node_put(bus->dev.of_node);
bus->dev.of_node = NULL;
+ bus->dev.fwnode = NULL;
}
struct device_node * __weak pcibios_get_phb_of_node(struct pci_bus *bus)
--
2.19.1
^ permalink raw reply related
* [PATCH v6 3/7] of: Allow the iommu-map property to omit untranslated devices
From: Jean-Philippe Brucker @ 2018-12-11 18:21 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
In PCI root complex nodes, the iommu-map property describes the IOMMU that
translates each endpoint. On some platforms, the IOMMU itself is presented
as a PCI endpoint (e.g. AMD IOMMU and virtio-iommu). This isn't supported
by the current OF driver, which expects all endpoints to have an IOMMU.
Allow the iommu-map property to have gaps.
Relaxing of_map_rid() also allows the msi-map property to have gaps, which
is invalid since MSIs always reach an MSI controller. In that case
pci_msi_setup_msi_irqs() will return an error when attempting to find the
device's MSI domain.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
drivers/of/base.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 09692c9b32a7..99f6bfa9b898 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -2237,8 +2237,12 @@ int of_map_rid(struct device_node *np, u32 rid,
return 0;
}
- pr_err("%pOF: Invalid %s translation - no match for rid 0x%x on %pOF\n",
- np, map_name, rid, target && *target ? *target : NULL);
- return -EFAULT;
+ pr_info("%pOF: no %s translation for rid 0x%x on %pOF\n", np, map_name,
+ rid, target && *target ? *target : NULL);
+
+ /* Bypasses translation */
+ if (id_out)
+ *id_out = rid;
+ return 0;
}
EXPORT_SYMBOL_GPL(of_map_rid);
--
2.19.1
^ permalink raw reply related
* [PATCH v6 2/7] dt-bindings: virtio: Add virtio-pci-iommu node
From: Jean-Philippe Brucker @ 2018-12-11 18:20 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
Some systems implement virtio-iommu as a PCI endpoint. The operating
system needs to discover the relationship between IOMMU and masters long
before the PCI endpoint gets probed. Add a PCI child node to describe the
virtio-iommu device.
The virtio-pci-iommu is conceptually split between a PCI programming
interface and a translation component on the parent bus. The latter
doesn't have a node in the device tree. The virtio-pci-iommu node
describes both, by linking the PCI endpoint to "iommus" property of DMA
master nodes and to "iommu-map" properties of bus nodes.
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
.../devicetree/bindings/virtio/iommu.txt | 66 +++++++++++++++++++
1 file changed, 66 insertions(+)
create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
diff --git a/Documentation/devicetree/bindings/virtio/iommu.txt b/Documentation/devicetree/bindings/virtio/iommu.txt
new file mode 100644
index 000000000000..2407fea0651c
--- /dev/null
+++ b/Documentation/devicetree/bindings/virtio/iommu.txt
@@ -0,0 +1,66 @@
+* virtio IOMMU PCI device
+
+When virtio-iommu uses the PCI transport, its programming interface is
+discovered dynamically by the PCI probing infrastructure. However the
+device tree statically describes the relation between IOMMU and DMA
+masters. Therefore, the PCI root complex that hosts the virtio-iommu
+contains a child node representing the IOMMU device explicitly.
+
+Required properties:
+
+- compatible: Should be "virtio,pci-iommu"
+- reg: PCI address of the IOMMU. As defined in the PCI Bus
+ Binding reference [1], the reg property is a five-cell
+ address encoded as (phys.hi phys.mid phys.lo size.hi
+ size.lo). phys.hi should contain the device's BDF as
+ 0b00000000 bbbbbbbb dddddfff 00000000. The other cells
+ should be zero.
+- #iommu-cells: Each platform DMA master managed by the IOMMU is assigned
+ an endpoint ID, described by the "iommus" property [2].
+ For virtio-iommu, #iommu-cells must be 1.
+
+Notes:
+
+- DMA from the IOMMU device isn't managed by another IOMMU. Therefore the
+ virtio-iommu node doesn't have an "iommus" property, and is omitted from
+ the iommu-map property of the root complex.
+
+Example:
+
+pcie@10000000 {
+ compatible = "pci-host-ecam-generic";
+ ...
+
+ /* The IOMMU programming interface uses slot 00:01.0 */
+ iommu0: iommu@0008 {
+ compatible = "virtio,pci-iommu";
+ reg = <0x00000800 0 0 0 0>;
+ #iommu-cells = <1>;
+ };
+
+ /*
+ * The IOMMU manages all functions in this PCI domain except
+ * itself. Omit BDF 00:01.0.
+ */
+ iommu-map = <0x0 &iommu0 0x0 0x8>
+ <0x9 &iommu0 0x9 0xfff7>;
+};
+
+pcie@20000000 {
+ compatible = "pci-host-ecam-generic";
+ ...
+ /*
+ * The IOMMU also manages all functions from this domain,
+ * with endpoint IDs 0x10000 - 0x1ffff
+ */
+ iommu-map = <0x0 &iommu0 0x10000 0x10000>;
+};
+
+ethernet@fe001000 {
+ ...
+ /* The IOMMU manages this platform device with endpoint ID 0x20000 */
+ iommus = <&iommu0 0x20000>;
+};
+
+[1] Documentation/devicetree/bindings/pci/pci.txt
+[2] Documentation/devicetree/bindings/iommu/iommu.txt
--
2.19.1
^ permalink raw reply related
* [PATCH v6 1/7] dt-bindings: virtio-mmio: Add IOMMU description
From: Jean-Philippe Brucker @ 2018-12-11 18:20 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
In-Reply-To: <20181211182104.18241-1-jean-philippe.brucker@arm.com>
The nature of a virtio-mmio node is discovered by the virtio driver at
probe time. However the DMA relation between devices must be described
statically. When a virtio-mmio node is a virtio-iommu device, it needs an
"#iommu-cells" property as specified by bindings/iommu/iommu.txt.
Otherwise, the virtio-mmio device may perform DMA through an IOMMU, which
requires an "iommus" property. Describe these requirements in the
device-tree bindings documentation.
Reviewed-by: Rob Herring <robh@kernel.org>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm.com>
---
.../devicetree/bindings/virtio/mmio.txt | 30 +++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/Documentation/devicetree/bindings/virtio/mmio.txt b/Documentation/devicetree/bindings/virtio/mmio.txt
index 5069c1b8e193..21af30fbb81f 100644
--- a/Documentation/devicetree/bindings/virtio/mmio.txt
+++ b/Documentation/devicetree/bindings/virtio/mmio.txt
@@ -8,10 +8,40 @@ Required properties:
- reg: control registers base address and size including configuration space
- interrupts: interrupt generated by the device
+Required properties for virtio-iommu:
+
+- #iommu-cells: When the node corresponds to a virtio-iommu device, it is
+ linked to DMA masters using the "iommus" or "iommu-map"
+ properties [1][2]. #iommu-cells specifies the size of the
+ "iommus" property. For virtio-iommu #iommu-cells must be
+ 1, each cell describing a single endpoint ID.
+
+Optional properties:
+
+- iommus: If the device accesses memory through an IOMMU, it should
+ have an "iommus" property [1]. Since virtio-iommu itself
+ does not access memory through an IOMMU, the "virtio,mmio"
+ node cannot have both an "#iommu-cells" and an "iommus"
+ property.
+
Example:
virtio_block@3000 {
compatible = "virtio,mmio";
reg = <0x3000 0x100>;
interrupts = <41>;
+
+ /* Device has endpoint ID 23 */
+ iommus = <&viommu 23>
}
+
+ viommu: iommu@3100 {
+ compatible = "virtio,mmio";
+ reg = <0x3100 0x100>;
+ interrupts = <42>;
+
+ #iommu-cells = <1>
+ }
+
+[1] Documentation/devicetree/bindings/iommu/iommu.txt
+[2] Documentation/devicetree/bindings/pci/pci-iommu.txt
--
2.19.1
^ permalink raw reply related
* [PATCH v6 0/7] Add virtio-iommu driver
From: Jean-Philippe Brucker @ 2018-12-11 18:20 UTC (permalink / raw)
To: iommu, linux-pci, devicetree, virtualization, virtio-dev, joro,
mst
Cc: mark.rutland, lorenzo.pieralisi, tnowicki, marc.zyngier,
robin.murphy, will.deacon, eric.auger, robh+dt, bhelgaas, kvmarm
Implement the virtio-iommu driver, following specification v0.9 [1].
Only minor changes since v5 [2]. I fixed issues reported by Michael and
added tags from Eric and Bharat. Thanks!
You can find Linux driver and kvmtool device on v0.9 branches [3],
module and x86 support on virtio-iommu/devel. Also tested with Eric's
QEMU device [4].
[1] Virtio-iommu specification v0.9, sources and pdf
git://linux-arm.org/virtio-iommu.git virtio-iommu/v0.9
http://jpbrucker.net/virtio-iommu/spec/v0.9/virtio-iommu-v0.9.pdf
[2] [PATCH v5 0/7] Add virtio-iommu driver
https://www.spinics.net/lists/linux-pci/msg78158.html
[3] git://linux-arm.org/linux-jpb.git virtio-iommu/v0.9.1
git://linux-arm.org/kvmtool-jpb.git virtio-iommu/v0.9
[4] [RFC v9 00/17] VIRTIO-IOMMU device
https://www.mail-archive.com/qemu-devel@nongnu.org/msg575578.html
Jean-Philippe Brucker (7):
dt-bindings: virtio-mmio: Add IOMMU description
dt-bindings: virtio: Add virtio-pci-iommu node
of: Allow the iommu-map property to omit untranslated devices
PCI: OF: Initialize dev->fwnode appropriately
iommu: Add virtio-iommu driver
iommu/virtio: Add probe request
iommu/virtio: Add event queue
.../devicetree/bindings/virtio/iommu.txt | 66 +
.../devicetree/bindings/virtio/mmio.txt | 30 +
MAINTAINERS | 7 +
drivers/iommu/Kconfig | 11 +
drivers/iommu/Makefile | 1 +
drivers/iommu/virtio-iommu.c | 1157 +++++++++++++++++
drivers/of/base.c | 10 +-
drivers/pci/of.c | 7 +
include/uapi/linux/virtio_ids.h | 1 +
include/uapi/linux/virtio_iommu.h | 161 +++
10 files changed, 1448 insertions(+), 3 deletions(-)
create mode 100644 Documentation/devicetree/bindings/virtio/iommu.txt
create mode 100644 drivers/iommu/virtio-iommu.c
create mode 100644 include/uapi/linux/virtio_iommu.h
--
2.19.1
^ permalink raw reply
* Re: [PATCH v5 5/7] iommu: Add virtio-iommu driver
From: Jean-Philippe Brucker @ 2018-12-11 16:29 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Mark Rutland, virtio-dev@lists.oasis-open.org, Lorenzo Pieralisi,
tnowicki@caviumnetworks.com, devicetree@vger.kernel.org,
Marc Zyngier, linux-pci@vger.kernel.org, joro@8bytes.org,
Will Deacon, virtualization@lists.linux-foundation.org,
eric.auger@redhat.com, iommu@lists.linux-foundation.org,
robh+dt@kernel.org, bhelgaas@google.com, Robin Murphy,
kvmarm@lists.cs.columbia.edu
In-Reply-To: <20181210175132-mutt-send-email-mst@kernel.org>
On 10/12/2018 22:53, Michael S. Tsirkin wrote:
> On Mon, Dec 10, 2018 at 03:06:47PM +0000, Jean-Philippe Brucker wrote:
>> On 27/11/2018 18:53, Michael S. Tsirkin wrote:
>>> On Tue, Nov 27, 2018 at 06:10:46PM +0000, Jean-Philippe Brucker wrote:
>>>> On 27/11/2018 18:04, Michael S. Tsirkin wrote:
>>>>> On Tue, Nov 27, 2018 at 05:50:50PM +0000, Jean-Philippe Brucker wrote:
>>>>>> On 23/11/2018 22:02, Michael S. Tsirkin wrote:
>>>>>>>> +/*
>>>>>>>> + * __viommu_sync_req - Complete all in-flight requests
>>>>>>>> + *
>>>>>>>> + * Wait for all added requests to complete. When this function returns, all
>>>>>>>> + * requests that were in-flight at the time of the call have completed.
>>>>>>>> + */
>>>>>>>> +static int __viommu_sync_req(struct viommu_dev *viommu)
>>>>>>>> +{
>>>>>>>> + int ret = 0;
>>>>>>>> + unsigned int len;
>>>>>>>> + size_t write_len;
>>>>>>>> + struct viommu_request *req;
>>>>>>>> + struct virtqueue *vq = viommu->vqs[VIOMMU_REQUEST_VQ];
>>>>>>>> +
>>>>>>>> + assert_spin_locked(&viommu->request_lock);
>>>>>>>> +
>>>>>>>> + virtqueue_kick(vq);
>>>>>>>> +
>>>>>>>> + while (!list_empty(&viommu->requests)) {
>>>>>>>> + len = 0;
>>>>>>>> + req = virtqueue_get_buf(vq, &len);
>>>>>>>> + if (!req)
>>>>>>>> + continue;
>>>>>>>> +
>>>>>>>> + if (!len)
>>>>>>>> + viommu_set_req_status(req->buf, req->len,
>>>>>>>> + VIRTIO_IOMMU_S_IOERR);
>>>>>>>> +
>>>>>>>> + write_len = req->len - req->write_offset;
>>>>>>>> + if (req->writeback && len == write_len)
>>>>>>>> + memcpy(req->writeback, req->buf + req->write_offset,
>>>>>>>> + write_len);
>>>>>>>> +
>>>>>>>> + list_del(&req->list);
>>>>>>>> + kfree(req);
>>>>>>>> + }
>>>>>>>
>>>>>>> I didn't notice this in the past but it seems this will spin
>>>>>>> with interrupts disabled until host handles the request.
>>>>>>> Please do not do this - host execution can be another
>>>>>>> task that needs the same host CPU. This will then disable
>>>>>>> interrupts for a very very long time.
>>>>>>
>>>>>> In the guest yes, but that doesn't prevent the host from running another
>>>>>> task right?
>>>>>
>>>>> Doesn't prevent it but it will delay it significantly
>>>>> until scheduler decides to kick the VCPU task out.
>>>>>
>>>>>> My tests run fine when QEMU is bound to a single CPU, even
>>>>>> though vcpu and viommu run in different threads
>>>>>>
>>>>>>> What to do then? Queue in software and wake up task.
>>>>>>
>>>>>> Unfortunately I can't do anything here, because IOMMU drivers can't
>>>>>> sleep in the iommu_map() or iommu_unmap() path.
>>>>>>
>>>>>> The problem is the same
>>>>>> for all IOMMU drivers. That's because the DMA API allows drivers to call
>>>>>> some functions with interrupts disabled. For example
>>>>>> Documentation/DMA-API-HOWTO.txt allows dma_alloc_coherent() and
>>>>>> dma_unmap_single() to be called in interrupt context.
>>>>>
>>>>> In fact I don't really understand how it's supposed to
>>>>> work at all: you only sync when ring is full.
>>>>> So host may not have seen your map request if ring
>>>>> is not full.
>>>>> Why is it safe to use the address with a device then?
>>>>
>>>> viommu_map() calls viommu_send_req_sync(), which does the sync
>>>> immediately after adding the MAP request.
>>>>
>>>> Thanks,
>>>> Jean
>>>
>>> I see. So it happens on every request. Maybe you should clear
>>> event index then. This way if exits are disabled you know that
>>> host is processing the ring. Event index is good for when
>>> you don't care when it will be processed, you just want
>>> to reduce number of exits as much as possible.
>>>
>>
>> I think that's already the case: since we don't attach a callback to the
>> request queue, VRING_AVAIL_F_NO_INTERRUPT is set in avail_flags_shadow,
>> which causes the used event index to stay clear.
>>
>> Thanks,
>> Jean
>
> VRING_AVAIL_F_NO_INTERRUPT has no effect when the event index
> feature has been negotiated. In any case, it also does not
> affect kick notifications from guest - it affects
> device interrupts.
Ok, I thought we were talking about the used event. Then this is a
device-side optimization right?
In QEMU, disabling notifications while processing the queue didn't show
any difference in netperf. Kvmtool already masks events when processing
the ring - if the host is still handling requests while the guest adds
more, then the avail event is at least one behind the new avail index,
and the guest doesn't kick the host.
Anyway I think we can look at optimizations later, since I don't think
there is any trivial one that we can squash into the initial driver.
I'll resend this series with the Kconfig and header change.
Thanks,
Jean
^ permalink raw reply
* Re: [PATCH net 2/4] vhost_net: rework on the lock ordering for busy polling
From: Michael S. Tsirkin @ 2018-12-11 4:04 UTC (permalink / raw)
To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <f2a98f3a-a5c5-b762-8ec3-119a7708795d@redhat.com>
On Tue, Dec 11, 2018 at 11:06:43AM +0800, Jason Wang wrote:
>
> On 2018/12/11 上午9:34, Michael S. Tsirkin wrote:
> > On Mon, Dec 10, 2018 at 05:44:52PM +0800, Jason Wang wrote:
> > > When we try to do rx busy polling in tx path in commit 441abde4cd84
> > > ("net: vhost: add rx busy polling in tx path"), we lock rx vq mutex
> > > after tx vq mutex is held. This may lead deadlock so we try to lock vq
> > > one by one in commit 78139c94dc8c ("net: vhost: lock the vqs one by
> > > one"). With this commit, we avoid the deadlock with the assumption
> > > that handle_rx() and handle_tx() run in a same process. But this
> > > commit remove the protection for IOTLB updating which requires the
> > > mutex of each vq to be held.
> > >
> > > To solve this issue, the first step is to have a exact same lock
> > > ordering for vhost_net. This is done through:
> > >
> > > - For handle_rx(), if busy polling is enabled, lock tx vq immediately.
> > > - For handle_tx(), always lock rx vq before tx vq, and unlock it if
> > > busy polling is not enabled.
> > > - Remove the tricky locking codes in busy polling.
> > >
> > > With this, we can have a exact same lock ordering for vhost_net, this
> > > allows us to safely revert commit 78139c94dc8c ("net: vhost: lock the
> > > vqs one by one") in next patch.
> > >
> > > The patch will add two more atomic operations on the tx path during
> > > each round of handle_tx(). 1 byte TCP_RR does not notice such
> > > overhead.
> > >
> > > Fixes: commit 78139c94dc8c ("net: vhost: lock the vqs one by one")
> > > Cc: Tonghao Zhang<xiangxia.m.yue@gmail.com>
> > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > ---
> > > drivers/vhost/net.c | 18 +++++++++++++++---
> > > 1 file changed, 15 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
> > > index ab11b2bee273..5f272ab4d5b4 100644
> > > --- a/drivers/vhost/net.c
> > > +++ b/drivers/vhost/net.c
> > > @@ -513,7 +513,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> > > struct socket *sock;
> > > struct vhost_virtqueue *vq = poll_rx ? tvq : rvq;
> > > - mutex_lock_nested(&vq->mutex, poll_rx ? VHOST_NET_VQ_TX: VHOST_NET_VQ_RX);
> > > vhost_disable_notify(&net->dev, vq);
> > > sock = rvq->private_data;
> > > @@ -543,8 +542,6 @@ static void vhost_net_busy_poll(struct vhost_net *net,
> > > vhost_net_busy_poll_try_queue(net, vq);
> > > else if (!poll_rx) /* On tx here, sock has no rx data. */
> > > vhost_enable_notify(&net->dev, rvq);
> > > -
> > > - mutex_unlock(&vq->mutex);
> > > }
> > > static int vhost_net_tx_get_vq_desc(struct vhost_net *net,
> > > @@ -913,10 +910,16 @@ static void handle_tx_zerocopy(struct vhost_net *net, struct socket *sock)
> > > static void handle_tx(struct vhost_net *net)
> > > {
> > > struct vhost_net_virtqueue *nvq = &net->vqs[VHOST_NET_VQ_TX];
> > > + struct vhost_net_virtqueue *nvq_rx = &net->vqs[VHOST_NET_VQ_RX];
> > > struct vhost_virtqueue *vq = &nvq->vq;
> > > + struct vhost_virtqueue *vq_rx = &nvq_rx->vq;
> > > struct socket *sock;
> > > + mutex_lock_nested(&vq_rx->mutex, VHOST_NET_VQ_RX);
> > > mutex_lock_nested(&vq->mutex, VHOST_NET_VQ_TX);
> > > + if (!vq->busyloop_timeout)
> > > + mutex_unlock(&vq_rx->mutex);
> > > +
> > > sock = vq->private_data;
> > > if (!sock)
> > > goto out;
> > > @@ -933,6 +936,8 @@ static void handle_tx(struct vhost_net *net)
> > > handle_tx_copy(net, sock);
> > > out:
> > > + if (vq->busyloop_timeout)
> > > + mutex_unlock(&vq_rx->mutex);
> > > mutex_unlock(&vq->mutex);
> > > }
> > So rx mutex taken on tx path now. And tx mutex is on rc path ... This
> > is just messed up. Why can't tx polling drop rx lock before
> > getting the tx lock and vice versa?
>
>
> Because we want to poll both tx and rx virtqueue at the same time
> (vhost_net_busy_poll()).
>
> while (vhost_can_busy_poll(endtime)) {
> if (vhost_has_work(&net->dev)) {
> *busyloop_intr = true;
> break;
> }
>
> if ((sock_has_rx_data(sock) &&
> !vhost_vq_avail_empty(&net->dev, rvq)) ||
> !vhost_vq_avail_empty(&net->dev, tvq))
> break;
>
> cpu_relax();
>
> }
>
>
> And we disable kicks and notification for better performance.
Right but it's all slow path - it happens when queue is
otherwise empty. So this is what I am saying: let's drop the locks
we hold around this.
>
> >
> > Or if we really wanted to force everything to be locked at
> > all times, let's just use a single mutex.
> >
> >
> >
>
> We could, but it might requires more changes which could be done for -next I
> believe.
>
>
> Thanks
I'd rather we kept the fine grained locking. E.g. people are
looking at splitting the tx and rx threads. But if not possible
let's fix it cleanly with a coarse-grained one. A mess here will
just create more trouble later.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ 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