[PATCH v8 18/19] vhost: use batched version by default
As testing shows no performance change, switch to that now.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Eugenio Pérez
Link: https://lore.kernel.org/r/20200401183118.8334-3-epere...@redhat.com
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.c | 251 +-
drivers/vhost/vhost.h | 4 -
2 files changed, 2 insertions(+), 253 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 56593ba6decc..6ca658c21e15 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2038,253 +2038,6 @@ static unsigned next_desc(struct vhost_virtqueue *vq,
struct vring_desc *desc)
return next;
}
-static int get_indirect(struct vhost_virtqueue *vq,
- struct iovec iov[], unsigned int iov_size,
- unsigned int *out_num, unsigned int *in_num,
- struct vhost_log *log, unsigned int *log_num,
- struct vring_desc *indirect)
-{
- struct vring_desc desc;
- unsigned int i = 0, count, found = 0;
- u32 len = vhost32_to_cpu(vq, indirect->len);
- struct iov_iter from;
- int ret, access;
-
- /* Sanity check */
- if (unlikely(len % sizeof desc)) {
- vq_err(vq, "Invalid length in indirect descriptor: "
- "len 0x%llx not multiple of 0x%zx\n",
- (unsigned long long)len,
- sizeof desc);
- return -EINVAL;
- }
-
- ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len,
vq->indirect,
-UIO_MAXIOV, VHOST_ACCESS_RO);
- if (unlikely(ret < 0)) {
- if (ret != -EAGAIN)
- vq_err(vq, "Translation failure %d in indirect.\n",
ret);
- return ret;
- }
- iov_iter_init(, READ, vq->indirect, ret, len);
-
- /* We will use the result as an address to read from, so most
-* architectures only need a compiler barrier here. */
- read_barrier_depends();
-
- count = len / sizeof desc;
- /* Buffers are chained via a 16 bit next field, so
-* we can have at most 2^16 of these. */
- if (unlikely(count > USHRT_MAX + 1)) {
- vq_err(vq, "Indirect buffer length too big: %d\n",
- indirect->len);
- return -E2BIG;
- }
-
- do {
- unsigned iov_count = *in_num + *out_num;
- if (unlikely(++found > count)) {
- vq_err(vq, "Loop detected: last one at %u "
- "indirect size %u\n",
- i, count);
- return -EINVAL;
- }
- if (unlikely(!copy_from_iter_full(, sizeof(desc), )))
{
- vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
- i, (size_t)vhost64_to_cpu(vq, indirect->addr) +
i * sizeof desc);
- return -EINVAL;
- }
- if (unlikely(desc.flags & cpu_to_vhost16(vq,
VRING_DESC_F_INDIRECT))) {
- vq_err(vq, "Nested indirect descriptor: idx %d, %zx\n",
- i, (size_t)vhost64_to_cpu(vq, indirect->addr) +
i * sizeof desc);
- return -EINVAL;
- }
-
- if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_WRITE))
- access = VHOST_ACCESS_WO;
- else
- access = VHOST_ACCESS_RO;
-
- ret = translate_desc(vq, vhost64_to_cpu(vq, desc.addr),
-vhost32_to_cpu(vq, desc.len), iov +
iov_count,
-iov_size - iov_count, access);
- if (unlikely(ret < 0)) {
- if (ret != -EAGAIN)
- vq_err(vq, "Translation failure %d indirect idx
%d\n",
- ret, i);
- return ret;
- }
- /* If this is an input descriptor, increment that count. */
- if (access == VHOST_ACCESS_WO) {
- *in_num += ret;
- if (unlikely(log && ret)) {
- log[*log_num].addr = vhost64_to_cpu(vq,
desc.addr);
- log[*log_num].len = vhost32_to_cpu(vq,
desc.len);
- ++*log_num;
- }
- } else {
- /* If it's an output descriptor, they're all supposed
-* to come before any input descriptors. */
- if (unlikely(*in_num)) {
- vq_err(vq, "Indirect descriptor "
- "has out after in: idx %d\n", i);
- return -EINVAL;
- }
-
[PATCH v8 13/19] tools/virtio: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs that make this
explicit.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/ringtest/virtio_ring_0_9.c | 6 +++---
tools/virtio/virtio_test.c | 6 +++---
tools/virtio/vringh_test.c | 18 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/tools/virtio/ringtest/virtio_ring_0_9.c
b/tools/virtio/ringtest/virtio_ring_0_9.c
index 13a035a390e9..e2ab6ac53966 100644
--- a/tools/virtio/ringtest/virtio_ring_0_9.c
+++ b/tools/virtio/ringtest/virtio_ring_0_9.c
@@ -67,13 +67,13 @@ void alloc_ring(void)
int i;
void *p;
- ret = posix_memalign(, 0x1000, vring_size(ring_size, 0x1000));
+ ret = posix_memalign(, 0x1000, vring_legacy_size(ring_size, 0x1000));
if (ret) {
perror("Unable to allocate ring buffer.\n");
exit(3);
}
- memset(p, 0, vring_size(ring_size, 0x1000));
- vring_init(, ring_size, p, 0x1000);
+ memset(p, 0, vring_legacy_size(ring_size, 0x1000));
+ vring_legacy_init(, ring_size, p, 0x1000);
guest.avail_idx = 0;
guest.kicked_avail_idx = -1;
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 93d81cd64ba0..25be607d8711 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -102,10 +102,10 @@ static void vq_info_add(struct vdev_info *dev, int num)
info->idx = dev->nvqs;
info->kick = eventfd(0, EFD_NONBLOCK);
info->call = eventfd(0, EFD_NONBLOCK);
- r = posix_memalign(>ring, 4096, vring_size(num, 4096));
+ r = posix_memalign(>ring, 4096, vring_legacy_size(num, 4096));
assert(r >= 0);
- memset(info->ring, 0, vring_size(num, 4096));
- vring_init(>vring, num, info->ring, 4096);
+ memset(info->ring, 0, vring_legacy_size(num, 4096));
+ vring_legacy_init(>vring, num, info->ring, 4096);
info->vq = vring_new_virtqueue(info->idx,
info->vring.num, 4096, >vdev,
true, false, info->ring,
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 293653463303..8ee2c9a6ad46 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -151,7 +151,7 @@ static int parallel_test(u64 features,
err(1, "Opening /tmp/vringh_test-file");
/* Extra room at the end for some data, and indirects */
- mapsize = vring_size(RINGSIZE, ALIGN)
+ mapsize = vring_legacy_size(RINGSIZE, ALIGN)
+ RINGSIZE * 2 * sizeof(int)
+ RINGSIZE * 6 * sizeof(struct vring_desc);
mapsize = (mapsize + getpagesize() - 1) & ~(getpagesize() - 1);
@@ -185,7 +185,7 @@ static int parallel_test(u64 features,
close(to_guest[0]);
close(to_host[1]);
- vring_init(, RINGSIZE, host_map, ALIGN);
+ vring_legacy_init(, RINGSIZE, host_map, ALIGN);
vringh_init_user(, features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail,
vrh.vring.used);
CPU_SET(first_cpu, _set);
@@ -297,7 +297,7 @@ static int parallel_test(u64 features,
unsigned int finished = 0;
/* We pass sg[]s pointing into here, but we need RINGSIZE+1 */
- data = guest_map + vring_size(RINGSIZE, ALIGN);
+ data = guest_map + vring_legacy_size(RINGSIZE, ALIGN);
indirects = (void *)data + (RINGSIZE + 1) * 2 * sizeof(int);
/* We are the guest. */
@@ -478,7 +478,7 @@ int main(int argc, char *argv[])
if (posix_memalign(&__user_addr_min, PAGE_SIZE, USER_MEM) != 0)
abort();
__user_addr_max = __user_addr_min + USER_MEM;
- memset(__user_addr_min, 0, vring_size(RINGSIZE, ALIGN));
+ memset(__user_addr_min, 0, vring_legacy_size(RINGSIZE, ALIGN));
/* Set up guest side. */
vq = vring_new_virtqueue(0, RINGSIZE, ALIGN, , true, false,
@@ -487,7 +487,7 @@ int main(int argc, char *argv[])
"guest vq");
/* Set up host side. */
- vring_init(, RINGSIZE, __user_addr_min, ALIGN);
+ vring_legacy_init(, RINGSIZE, __user_addr_min, ALIGN);
vringh_init_user(, vdev.features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
@@ -506,7 +506,7 @@ int main(int argc, char *argv[])
sgs[1] = _sg[1];
/* May allocate an indirect, so force it to allocate user addr */
- __kmalloc_fake = __user_addr_min + vring_size(RINGSIZE, ALIGN);
+ __kmalloc_fake = __user_addr_min + vring_legacy_size(RINGSIZE, ALIGN);
err = virtqueue_add_sgs(vq, sgs, 1, 1, , GFP_KERNEL);
if (err)
errx(1, "virtqueue_add_sgs: %i", err);
@@ -556,7 +556,7 @@ int main(int argc, char *argv[])
errx(1,
[PATCH v8 11/19] virtio: add legacy init/size APIs
vring_init/vring_size (in the uapi directory) are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only vring_legacy_init/vring_legacy_size.
Signed-off-by: Michael S. Tsirkin
---
include/linux/virtio_ring.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index c3f9ca054250..766d70bca492 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -140,4 +140,22 @@ void vring_del_virtqueue(struct virtqueue *vq);
void vring_transport_features(struct virtio_device *vdev);
irqreturn_t vring_interrupt(int irq, void *_vq);
+
+static inline void vring_legacy_init(struct vring *vr, unsigned int num, void
*p,
+unsigned long align)
+{
+ vr->num = num;
+ vr->desc = p;
+ vr->avail = (struct vring_avail *)((char *)p + num * sizeof(struct
vring_desc));
+ vr->used = (void *)(((uintptr_t)>avail->ring[num] +
sizeof(__virtio16)
+ + align-1) & ~(align - 1));
+}
+
+static inline unsigned vring_legacy_size(unsigned int num, unsigned long align)
+{
+ return ((sizeof(struct vring_desc) * num + sizeof(__virtio16) * (3 +
num)
++ align - 1) & ~(align - 1))
+ + sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
+}
+
#endif /* _LINUX_VIRTIO_RING_H */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 19/19] vhost: batching fetches
With this patch applied, new and old code perform identically.
Lots of extra optimizations are now possible, e.g.
we can fetch multiple heads with copy_from/to_user now.
We can get rid of maintaining the log array. Etc etc.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Eugenio Pérez
Link: https://lore.kernel.org/r/20200401183118.8334-4-epere...@redhat.com
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/test.c | 2 +-
drivers/vhost/vhost.c | 47 ++-
drivers/vhost/vhost.h | 5 -
3 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index b06680833f03..251ca723ac3f 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -119,7 +119,7 @@ static int vhost_test_open(struct inode *inode, struct file
*f)
dev = >dev;
vqs[VHOST_TEST_VQ] = >vqs[VHOST_TEST_VQ];
n->vqs[VHOST_TEST_VQ].handle_kick = handle_vq_kick;
- vhost_dev_init(dev, vqs, VHOST_TEST_VQ_MAX, UIO_MAXIOV,
+ vhost_dev_init(dev, vqs, VHOST_TEST_VQ_MAX, UIO_MAXIOV + 64,
VHOST_TEST_PKT_WEIGHT, VHOST_TEST_WEIGHT, NULL);
f->private_data = n;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 6ca658c21e15..0395229486a9 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -299,6 +299,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
{
vq->num = 1;
vq->ndescs = 0;
+ vq->first_desc = 0;
vq->desc = NULL;
vq->avail = NULL;
vq->used = NULL;
@@ -367,6 +368,11 @@ static int vhost_worker(void *data)
return 0;
}
+static int vhost_vq_num_batch_descs(struct vhost_virtqueue *vq)
+{
+ return vq->max_descs - UIO_MAXIOV;
+}
+
static void vhost_vq_free_iovecs(struct vhost_virtqueue *vq)
{
kfree(vq->descs);
@@ -389,6 +395,9 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
for (i = 0; i < dev->nvqs; ++i) {
vq = dev->vqs[i];
vq->max_descs = dev->iov_limit;
+ if (vhost_vq_num_batch_descs(vq) < 0) {
+ return -EINVAL;
+ }
vq->descs = kmalloc_array(vq->max_descs,
sizeof(*vq->descs),
GFP_KERNEL);
@@ -1570,6 +1579,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int
ioctl, void __user *arg
vq->last_avail_idx = s.num;
/* Forget the cached index value. */
vq->avail_idx = vq->last_avail_idx;
+ vq->ndescs = vq->first_desc = 0;
break;
case VHOST_GET_VRING_BASE:
s.index = idx;
@@ -2136,7 +2146,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue
*vq,
return 0;
}
-static int fetch_descs(struct vhost_virtqueue *vq)
+static int fetch_buf(struct vhost_virtqueue *vq)
{
unsigned int i, head, found = 0;
struct vhost_desc *last;
@@ -2149,7 +2159,11 @@ static int fetch_descs(struct vhost_virtqueue *vq)
/* Check it isn't doing very strange things with descriptor numbers. */
last_avail_idx = vq->last_avail_idx;
- if (vq->avail_idx == vq->last_avail_idx) {
+ if (unlikely(vq->avail_idx == vq->last_avail_idx)) {
+ /* If we already have work to do, don't bother re-checking. */
+ if (likely(vq->ndescs))
+ return vq->num;
+
if (unlikely(vhost_get_avail_idx(vq, _idx))) {
vq_err(vq, "Failed to access avail idx at %p\n",
>avail->idx);
@@ -2240,6 +2254,24 @@ static int fetch_descs(struct vhost_virtqueue *vq)
return 0;
}
+static int fetch_descs(struct vhost_virtqueue *vq)
+{
+ int ret = 0;
+
+ if (unlikely(vq->first_desc >= vq->ndescs)) {
+ vq->first_desc = 0;
+ vq->ndescs = 0;
+ }
+
+ if (vq->ndescs)
+ return 0;
+
+ while (!ret && vq->ndescs <= vhost_vq_num_batch_descs(vq))
+ ret = fetch_buf(vq);
+
+ return vq->ndescs ? 0 : ret;
+}
+
/* This looks in the virtqueue and for the first available buffer, and converts
* it to an iovec for convenient access. Since descriptors consist of some
* number of output then some number of input descriptors, it's actually two
@@ -2265,7 +2297,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
if (unlikely(log))
*log_num = 0;
- for (i = 0; i < vq->ndescs; ++i) {
+ for (i = vq->first_desc; i < vq->ndescs; ++i) {
unsigned iov_count = *in_num + *out_num;
struct vhost_desc *desc = >descs[i];
int access;
@@ -2311,14 +2343,19 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
}
ret = desc->id;
+
+ if (!(desc->flags & VRING_DESC_F_NEXT))
+
[PATCH v8 17/19] vhost: option to fetch descriptors through an independent struct
The idea is to support multiple ring formats by converting
to a format-independent array of descriptors.
This costs extra cycles, but we gain in ability
to fetch a batch of descriptors in one go, which
is good for code cache locality.
When used, this causes a minor performance degradation,
it's been kept as simple as possible for ease of review.
A follow-up patch gets us back the performance by adding batching.
To simplify benchmarking, I kept the old code around so one can switch
back and forth between old and new code. This will go away in the final
submission.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Eugenio Pérez
Link: https://lore.kernel.org/r/20200401183118.8334-2-epere...@redhat.com
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.c | 297 +-
drivers/vhost/vhost.h | 16 +++
2 files changed, 312 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d450e16c5c25..56593ba6decc 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -298,6 +298,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
struct vhost_virtqueue *vq)
{
vq->num = 1;
+ vq->ndescs = 0;
vq->desc = NULL;
vq->avail = NULL;
vq->used = NULL;
@@ -368,6 +369,9 @@ static int vhost_worker(void *data)
static void vhost_vq_free_iovecs(struct vhost_virtqueue *vq)
{
+ kfree(vq->descs);
+ vq->descs = NULL;
+ vq->max_descs = 0;
kfree(vq->indirect);
vq->indirect = NULL;
kfree(vq->log);
@@ -384,6 +388,10 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
for (i = 0; i < dev->nvqs; ++i) {
vq = dev->vqs[i];
+ vq->max_descs = dev->iov_limit;
+ vq->descs = kmalloc_array(vq->max_descs,
+ sizeof(*vq->descs),
+ GFP_KERNEL);
vq->indirect = kmalloc_array(UIO_MAXIOV,
sizeof(*vq->indirect),
GFP_KERNEL);
@@ -391,7 +399,7 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
GFP_KERNEL);
vq->heads = kmalloc_array(dev->iov_limit, sizeof(*vq->heads),
GFP_KERNEL);
- if (!vq->indirect || !vq->log || !vq->heads)
+ if (!vq->indirect || !vq->log || !vq->heads || !vq->descs)
goto err_nomem;
}
return 0;
@@ -2277,6 +2285,293 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
}
EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
+static struct vhost_desc *peek_split_desc(struct vhost_virtqueue *vq)
+{
+ BUG_ON(!vq->ndescs);
+ return >descs[vq->ndescs - 1];
+}
+
+static void pop_split_desc(struct vhost_virtqueue *vq)
+{
+ BUG_ON(!vq->ndescs);
+ --vq->ndescs;
+}
+
+#define VHOST_DESC_FLAGS (VRING_DESC_F_INDIRECT | VRING_DESC_F_WRITE | \
+ VRING_DESC_F_NEXT)
+static int push_split_desc(struct vhost_virtqueue *vq, struct vring_desc
*desc, u16 id)
+{
+ struct vhost_desc *h;
+
+ if (unlikely(vq->ndescs >= vq->max_descs))
+ return -EINVAL;
+ h = >descs[vq->ndescs++];
+ h->addr = vhost64_to_cpu(vq, desc->addr);
+ h->len = vhost32_to_cpu(vq, desc->len);
+ h->flags = vhost16_to_cpu(vq, desc->flags) & VHOST_DESC_FLAGS;
+ h->id = id;
+
+ return 0;
+}
+
+static int fetch_indirect_descs(struct vhost_virtqueue *vq,
+ struct vhost_desc *indirect,
+ u16 head)
+{
+ struct vring_desc desc;
+ unsigned int i = 0, count, found = 0;
+ u32 len = indirect->len;
+ struct iov_iter from;
+ int ret;
+
+ /* Sanity check */
+ if (unlikely(len % sizeof desc)) {
+ vq_err(vq, "Invalid length in indirect descriptor: "
+ "len 0x%llx not multiple of 0x%zx\n",
+ (unsigned long long)len,
+ sizeof desc);
+ return -EINVAL;
+ }
+
+ ret = translate_desc(vq, indirect->addr, len, vq->indirect,
+UIO_MAXIOV, VHOST_ACCESS_RO);
+ if (unlikely(ret < 0)) {
+ if (ret != -EAGAIN)
+ vq_err(vq, "Translation failure %d in indirect.\n",
ret);
+ return ret;
+ }
+ iov_iter_init(, READ, vq->indirect, ret, len);
+
+ /* We will use the result as an address to read from, so most
+* architectures only need a compiler barrier here. */
+ read_barrier_depends();
+
+ count = len / sizeof desc;
+ /* Buffers are chained via a 16 bit next field, so
+* we can have at most 2^16 of these. */
+ if (unlikely(count > USHRT_MAX + 1)) {
+ vq_err(vq,
[PATCH v8 12/19] virtio_ring: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs
that make this explicit.
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_ring.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 58b96baa8d48..2a2b12449bac 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -867,8 +867,10 @@ static struct virtqueue *vring_create_virtqueue_split(
}
/* TODO: allocate each queue chunk individually */
- for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ for (; num && vring_legacy_size(num, vring_align) > PAGE_SIZE;
+num /= 2) {
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr,
GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
if (queue)
@@ -882,14 +884,15 @@ static struct virtqueue *vring_create_virtqueue_split(
if (!queue) {
/* Try to get a single page. You are my only hope! */
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr, GFP_KERNEL|__GFP_ZERO);
}
if (!queue)
return NULL;
- queue_size_in_bytes = vring_size(num, vring_align);
- vring_init(, num, queue, vring_align);
+ queue_size_in_bytes = vring_legacy_size(num, vring_align);
+ vring_legacy_init(, num, queue, vring_align);
vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
@@ -2169,7 +2172,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
return NULL;
- vring_init(, num, pages, vring_align);
+ vring_legacy_init(, num, pages, vring_align);
return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 05/19] virtgpu: pull in uaccess.h
In preparation to virtio header changes, include uaccess.h directly as this file is using copy to/from user. Signed-off-by: Michael S. Tsirkin --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 205ec4abae2b..289dabbce477 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -27,6 +27,7 @@ #include #include +#include #include #include -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 02/19] tools/virtio: make asm/barrier.h self contained
We are using abort() so let's include stdlib.h
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/asm/barrier.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
index d0351f83aebe..04d563fc9b95 100644
--- a/tools/virtio/asm/barrier.h
+++ b/tools/virtio/asm/barrier.h
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include
#if defined(__i386__) || defined(__x86_64__)
#define barrier() asm volatile("" ::: "memory")
#define virt_mb() __sync_synchronize()
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 03/19] tools/virtio: define __KERNEL__
Even though we are building in userspace, we are emulating kernel bits.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index b587b9a7a124..ea6674307507 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -4,7 +4,7 @@ test: virtio_test vringh_test
virtio_test: virtio_ring.o virtio_test.o
vringh_test: vringh_test.o vringh.o virtio_ring.o
-CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h
+CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h -D__KERNEL__
vpath %.c ../../drivers/virtio ../../drivers/vhost
mod:
${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test V=${V}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 09/19] virtio: stop using legacy struct vring in kernel
struct vring (in the uapi directory) and supporting APIs are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only struct vring and
switch everyone to use that.
Signed-off-by: Michael S. Tsirkin
---
drivers/block/virtio_blk.c | 1 +
include/linux/virtio.h | 1 -
include/linux/virtio_ring.h | 10 ++
include/linux/vringh.h | 1 +
include/uapi/linux/virtio_ring.h | 26 --
5 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 0736248999b0..dd5732dc4b07 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#define PART_BITS 4
#define VQ_NAME_LEN 16
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 15f906e4a748..a493eac08393 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -9,7 +9,6 @@
#include
#include
#include
-#include
/**
* virtqueue - a queue to register buffers for sending or receiving.
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..11680e74761a 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,6 +60,16 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
/*
* Creates a virtqueue and allocates the descriptor ring. If
* may_reduce_num is set, then this may allocate a smaller ring than
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index 9e2763d7c159..d71b3710f58e 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -11,6 +11,7 @@
#ifndef _LINUX_VRINGH_H
#define _LINUX_VRINGH_H
#include
+#include
#include
#include
#include
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 9223c3a5c46a..8961a4adda5c 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -166,6 +156,21 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+#ifndef __KERNEL__
+/*
+ * The following definitions have been put in the UAPI header by mistake. We
+ * keep them around to avoid breaking old userspace builds.
+ */
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
@@ -182,6 +187,7 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ align - 1) & ~(align - 1))
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif
#endif /* VIRTIO_RING_NO_LEGACY */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 10/19] vhost: force spec specified alignment on types
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.h | 6 +++---
include/linux/virtio_ring.h | 24 +---
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index f8403bd46b85..60cab4c78229 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -67,9 +67,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct file *kick;
struct eventfd_ctx *call_ctx;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 11680e74761a..c3f9ca054250 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,14 +60,32 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __aligned(VRING_DESC_ALIGN_SIZE) vring_desc_t;
+typedef struct vring_avail __aligned(VRING_AVAIL_ALIGN_SIZE) vring_avail_t;
+typedef struct vring_used __aligned(VRING_USED_ALIGN_SIZE) vring_used_t;
+
struct vring {
unsigned int num;
- struct vring_desc *desc;
+ vring_desc_t *desc;
- struct vring_avail *avail;
+ vring_avail_t *avail;
- struct vring_used *used;
+ vring_used_t *used;
};
/*
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 04/19] virtio: add VIRTIO_RING_NO_LEGACY
From: Matej Genci
Add macro to disable legacy vring functions.
Signed-off-by: Matej Genci
Link: https://lore.kernel.org/r/20190911124942.243713-1-matej.ge...@nutanix.com
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_pci_modern.c | 1 +
include/uapi/linux/virtio_ring.h | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 7abcc50838b8..db93cedd262f 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -16,6 +16,7 @@
#include
#define VIRTIO_PCI_NO_LEGACY
+#define VIRTIO_RING_NO_LEGACY
#include "virtio_pci_common.h"
/*
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..9223c3a5c46a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -135,6 +135,8 @@ struct vring {
#define VRING_USED_ALIGN_SIZE 4
#define VRING_DESC_ALIGN_SIZE 16
+#ifndef VIRTIO_RING_NO_LEGACY
+
/* The standard layout for the ring is a continuous chunk of memory which looks
* like this. We assume num is a power of 2.
*
@@ -181,6 +183,8 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif /* VIRTIO_RING_NO_LEGACY */
+
/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
/* Assuming a given event_idx value from the other side, if
* we have just incremented index from old to new_idx,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 08/19] virtio_input: pull in slab.h
In preparation to virtio header changes, include slab.h directly as this module is using it. Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_input.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c index 5ae529671b3d..efaf65b0f42d 100644 --- a/drivers/virtio/virtio_input.c +++ b/drivers/virtio/virtio_input.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v8 01/19] tools/virtio: define aligned attribute
Signed-off-by: Michael S. Tsirkin --- tools/virtio/linux/compiler.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h index 903dc9c4bd11..2c51bccb97bb 100644 --- a/tools/virtio/linux/compiler.h +++ b/tools/virtio/linux/compiler.h @@ -7,4 +7,5 @@ #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var +#define __aligned(x) __attribute((__aligned__(x))) #endif -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 04/19] virtio: add VIRTIO_RING_NO_LEGACY
From: Matej Genci
Add macro to disable legacy vring functions.
Signed-off-by: Matej Genci
Link: https://lore.kernel.org/r/20190911124942.243713-1-matej.ge...@nutanix.com
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_pci_modern.c | 1 +
include/uapi/linux/virtio_ring.h | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 7abcc50838b8..db93cedd262f 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -16,6 +16,7 @@
#include
#define VIRTIO_PCI_NO_LEGACY
+#define VIRTIO_RING_NO_LEGACY
#include "virtio_pci_common.h"
/*
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..9223c3a5c46a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -135,6 +135,8 @@ struct vring {
#define VRING_USED_ALIGN_SIZE 4
#define VRING_DESC_ALIGN_SIZE 16
+#ifndef VIRTIO_RING_NO_LEGACY
+
/* The standard layout for the ring is a continuous chunk of memory which looks
* like this. We assume num is a power of 2.
*
@@ -181,6 +183,8 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif /* VIRTIO_RING_NO_LEGACY */
+
/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
/* Assuming a given event_idx value from the other side, if
* we have just incremented index from old to new_idx,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 18/19] vhost: use batched version by default
As testing shows no performance change, switch to that now.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Eugenio Pérez
Link: https://lore.kernel.org/r/20200401183118.8334-3-epere...@redhat.com
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.c | 251 +-
drivers/vhost/vhost.h | 4 -
2 files changed, 2 insertions(+), 253 deletions(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 56593ba6decc..6ca658c21e15 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2038,253 +2038,6 @@ static unsigned next_desc(struct vhost_virtqueue *vq,
struct vring_desc *desc)
return next;
}
-static int get_indirect(struct vhost_virtqueue *vq,
- struct iovec iov[], unsigned int iov_size,
- unsigned int *out_num, unsigned int *in_num,
- struct vhost_log *log, unsigned int *log_num,
- struct vring_desc *indirect)
-{
- struct vring_desc desc;
- unsigned int i = 0, count, found = 0;
- u32 len = vhost32_to_cpu(vq, indirect->len);
- struct iov_iter from;
- int ret, access;
-
- /* Sanity check */
- if (unlikely(len % sizeof desc)) {
- vq_err(vq, "Invalid length in indirect descriptor: "
- "len 0x%llx not multiple of 0x%zx\n",
- (unsigned long long)len,
- sizeof desc);
- return -EINVAL;
- }
-
- ret = translate_desc(vq, vhost64_to_cpu(vq, indirect->addr), len,
vq->indirect,
-UIO_MAXIOV, VHOST_ACCESS_RO);
- if (unlikely(ret < 0)) {
- if (ret != -EAGAIN)
- vq_err(vq, "Translation failure %d in indirect.\n",
ret);
- return ret;
- }
- iov_iter_init(, READ, vq->indirect, ret, len);
-
- /* We will use the result as an address to read from, so most
-* architectures only need a compiler barrier here. */
- read_barrier_depends();
-
- count = len / sizeof desc;
- /* Buffers are chained via a 16 bit next field, so
-* we can have at most 2^16 of these. */
- if (unlikely(count > USHRT_MAX + 1)) {
- vq_err(vq, "Indirect buffer length too big: %d\n",
- indirect->len);
- return -E2BIG;
- }
-
- do {
- unsigned iov_count = *in_num + *out_num;
- if (unlikely(++found > count)) {
- vq_err(vq, "Loop detected: last one at %u "
- "indirect size %u\n",
- i, count);
- return -EINVAL;
- }
- if (unlikely(!copy_from_iter_full(, sizeof(desc), )))
{
- vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
- i, (size_t)vhost64_to_cpu(vq, indirect->addr) +
i * sizeof desc);
- return -EINVAL;
- }
- if (unlikely(desc.flags & cpu_to_vhost16(vq,
VRING_DESC_F_INDIRECT))) {
- vq_err(vq, "Nested indirect descriptor: idx %d, %zx\n",
- i, (size_t)vhost64_to_cpu(vq, indirect->addr) +
i * sizeof desc);
- return -EINVAL;
- }
-
- if (desc.flags & cpu_to_vhost16(vq, VRING_DESC_F_WRITE))
- access = VHOST_ACCESS_WO;
- else
- access = VHOST_ACCESS_RO;
-
- ret = translate_desc(vq, vhost64_to_cpu(vq, desc.addr),
-vhost32_to_cpu(vq, desc.len), iov +
iov_count,
-iov_size - iov_count, access);
- if (unlikely(ret < 0)) {
- if (ret != -EAGAIN)
- vq_err(vq, "Translation failure %d indirect idx
%d\n",
- ret, i);
- return ret;
- }
- /* If this is an input descriptor, increment that count. */
- if (access == VHOST_ACCESS_WO) {
- *in_num += ret;
- if (unlikely(log && ret)) {
- log[*log_num].addr = vhost64_to_cpu(vq,
desc.addr);
- log[*log_num].len = vhost32_to_cpu(vq,
desc.len);
- ++*log_num;
- }
- } else {
- /* If it's an output descriptor, they're all supposed
-* to come before any input descriptors. */
- if (unlikely(*in_num)) {
- vq_err(vq, "Indirect descriptor "
- "has out after in: idx %d\n", i);
- return -EINVAL;
- }
-
[PATCH v7 17/19] vhost: option to fetch descriptors through an independent struct
The idea is to support multiple ring formats by converting
to a format-independent array of descriptors.
This costs extra cycles, but we gain in ability
to fetch a batch of descriptors in one go, which
is good for code cache locality.
When used, this causes a minor performance degradation,
it's been kept as simple as possible for ease of review.
A follow-up patch gets us back the performance by adding batching.
To simplify benchmarking, I kept the old code around so one can switch
back and forth between old and new code. This will go away in the final
submission.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Eugenio Pérez
Link: https://lore.kernel.org/r/20200401183118.8334-2-epere...@redhat.com
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.c | 297 +-
drivers/vhost/vhost.h | 16 +++
2 files changed, 312 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d450e16c5c25..56593ba6decc 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -298,6 +298,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
struct vhost_virtqueue *vq)
{
vq->num = 1;
+ vq->ndescs = 0;
vq->desc = NULL;
vq->avail = NULL;
vq->used = NULL;
@@ -368,6 +369,9 @@ static int vhost_worker(void *data)
static void vhost_vq_free_iovecs(struct vhost_virtqueue *vq)
{
+ kfree(vq->descs);
+ vq->descs = NULL;
+ vq->max_descs = 0;
kfree(vq->indirect);
vq->indirect = NULL;
kfree(vq->log);
@@ -384,6 +388,10 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
for (i = 0; i < dev->nvqs; ++i) {
vq = dev->vqs[i];
+ vq->max_descs = dev->iov_limit;
+ vq->descs = kmalloc_array(vq->max_descs,
+ sizeof(*vq->descs),
+ GFP_KERNEL);
vq->indirect = kmalloc_array(UIO_MAXIOV,
sizeof(*vq->indirect),
GFP_KERNEL);
@@ -391,7 +399,7 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
GFP_KERNEL);
vq->heads = kmalloc_array(dev->iov_limit, sizeof(*vq->heads),
GFP_KERNEL);
- if (!vq->indirect || !vq->log || !vq->heads)
+ if (!vq->indirect || !vq->log || !vq->heads || !vq->descs)
goto err_nomem;
}
return 0;
@@ -2277,6 +2285,293 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
}
EXPORT_SYMBOL_GPL(vhost_get_vq_desc);
+static struct vhost_desc *peek_split_desc(struct vhost_virtqueue *vq)
+{
+ BUG_ON(!vq->ndescs);
+ return >descs[vq->ndescs - 1];
+}
+
+static void pop_split_desc(struct vhost_virtqueue *vq)
+{
+ BUG_ON(!vq->ndescs);
+ --vq->ndescs;
+}
+
+#define VHOST_DESC_FLAGS (VRING_DESC_F_INDIRECT | VRING_DESC_F_WRITE | \
+ VRING_DESC_F_NEXT)
+static int push_split_desc(struct vhost_virtqueue *vq, struct vring_desc
*desc, u16 id)
+{
+ struct vhost_desc *h;
+
+ if (unlikely(vq->ndescs >= vq->max_descs))
+ return -EINVAL;
+ h = >descs[vq->ndescs++];
+ h->addr = vhost64_to_cpu(vq, desc->addr);
+ h->len = vhost32_to_cpu(vq, desc->len);
+ h->flags = vhost16_to_cpu(vq, desc->flags) & VHOST_DESC_FLAGS;
+ h->id = id;
+
+ return 0;
+}
+
+static int fetch_indirect_descs(struct vhost_virtqueue *vq,
+ struct vhost_desc *indirect,
+ u16 head)
+{
+ struct vring_desc desc;
+ unsigned int i = 0, count, found = 0;
+ u32 len = indirect->len;
+ struct iov_iter from;
+ int ret;
+
+ /* Sanity check */
+ if (unlikely(len % sizeof desc)) {
+ vq_err(vq, "Invalid length in indirect descriptor: "
+ "len 0x%llx not multiple of 0x%zx\n",
+ (unsigned long long)len,
+ sizeof desc);
+ return -EINVAL;
+ }
+
+ ret = translate_desc(vq, indirect->addr, len, vq->indirect,
+UIO_MAXIOV, VHOST_ACCESS_RO);
+ if (unlikely(ret < 0)) {
+ if (ret != -EAGAIN)
+ vq_err(vq, "Translation failure %d in indirect.\n",
ret);
+ return ret;
+ }
+ iov_iter_init(, READ, vq->indirect, ret, len);
+
+ /* We will use the result as an address to read from, so most
+* architectures only need a compiler barrier here. */
+ read_barrier_depends();
+
+ count = len / sizeof desc;
+ /* Buffers are chained via a 16 bit next field, so
+* we can have at most 2^16 of these. */
+ if (unlikely(count > USHRT_MAX + 1)) {
+ vq_err(vq,
[PATCH v7 19/19] vhost: batching fetches
With this patch applied, new and old code perform identically.
Lots of extra optimizations are now possible, e.g.
we can fetch multiple heads with copy_from/to_user now.
We can get rid of maintaining the log array. Etc etc.
Signed-off-by: Michael S. Tsirkin
Signed-off-by: Eugenio Pérez
Link: https://lore.kernel.org/r/20200401183118.8334-4-epere...@redhat.com
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/test.c | 2 +-
drivers/vhost/vhost.c | 47 ++-
drivers/vhost/vhost.h | 5 -
3 files changed, 47 insertions(+), 7 deletions(-)
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index b06680833f03..251ca723ac3f 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -119,7 +119,7 @@ static int vhost_test_open(struct inode *inode, struct file
*f)
dev = >dev;
vqs[VHOST_TEST_VQ] = >vqs[VHOST_TEST_VQ];
n->vqs[VHOST_TEST_VQ].handle_kick = handle_vq_kick;
- vhost_dev_init(dev, vqs, VHOST_TEST_VQ_MAX, UIO_MAXIOV,
+ vhost_dev_init(dev, vqs, VHOST_TEST_VQ_MAX, UIO_MAXIOV + 64,
VHOST_TEST_PKT_WEIGHT, VHOST_TEST_WEIGHT, NULL);
f->private_data = n;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 6ca658c21e15..0395229486a9 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -299,6 +299,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
{
vq->num = 1;
vq->ndescs = 0;
+ vq->first_desc = 0;
vq->desc = NULL;
vq->avail = NULL;
vq->used = NULL;
@@ -367,6 +368,11 @@ static int vhost_worker(void *data)
return 0;
}
+static int vhost_vq_num_batch_descs(struct vhost_virtqueue *vq)
+{
+ return vq->max_descs - UIO_MAXIOV;
+}
+
static void vhost_vq_free_iovecs(struct vhost_virtqueue *vq)
{
kfree(vq->descs);
@@ -389,6 +395,9 @@ static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
for (i = 0; i < dev->nvqs; ++i) {
vq = dev->vqs[i];
vq->max_descs = dev->iov_limit;
+ if (vhost_vq_num_batch_descs(vq) < 0) {
+ return -EINVAL;
+ }
vq->descs = kmalloc_array(vq->max_descs,
sizeof(*vq->descs),
GFP_KERNEL);
@@ -1570,6 +1579,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int
ioctl, void __user *arg
vq->last_avail_idx = s.num;
/* Forget the cached index value. */
vq->avail_idx = vq->last_avail_idx;
+ vq->ndescs = vq->first_desc = 0;
break;
case VHOST_GET_VRING_BASE:
s.index = idx;
@@ -2136,7 +2146,7 @@ static int fetch_indirect_descs(struct vhost_virtqueue
*vq,
return 0;
}
-static int fetch_descs(struct vhost_virtqueue *vq)
+static int fetch_buf(struct vhost_virtqueue *vq)
{
unsigned int i, head, found = 0;
struct vhost_desc *last;
@@ -2149,7 +2159,11 @@ static int fetch_descs(struct vhost_virtqueue *vq)
/* Check it isn't doing very strange things with descriptor numbers. */
last_avail_idx = vq->last_avail_idx;
- if (vq->avail_idx == vq->last_avail_idx) {
+ if (unlikely(vq->avail_idx == vq->last_avail_idx)) {
+ /* If we already have work to do, don't bother re-checking. */
+ if (likely(vq->ndescs))
+ return vq->num;
+
if (unlikely(vhost_get_avail_idx(vq, _idx))) {
vq_err(vq, "Failed to access avail idx at %p\n",
>avail->idx);
@@ -2240,6 +2254,24 @@ static int fetch_descs(struct vhost_virtqueue *vq)
return 0;
}
+static int fetch_descs(struct vhost_virtqueue *vq)
+{
+ int ret = 0;
+
+ if (unlikely(vq->first_desc >= vq->ndescs)) {
+ vq->first_desc = 0;
+ vq->ndescs = 0;
+ }
+
+ if (vq->ndescs)
+ return 0;
+
+ while (!ret && vq->ndescs <= vhost_vq_num_batch_descs(vq))
+ ret = fetch_buf(vq);
+
+ return vq->ndescs ? 0 : ret;
+}
+
/* This looks in the virtqueue and for the first available buffer, and converts
* it to an iovec for convenient access. Since descriptors consist of some
* number of output then some number of input descriptors, it's actually two
@@ -2265,7 +2297,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
if (unlikely(log))
*log_num = 0;
- for (i = 0; i < vq->ndescs; ++i) {
+ for (i = vq->first_desc; i < vq->ndescs; ++i) {
unsigned iov_count = *in_num + *out_num;
struct vhost_desc *desc = >descs[i];
int access;
@@ -2311,14 +2343,19 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
}
ret = desc->id;
+
+ if (!(desc->flags & VRING_DESC_F_NEXT))
+
[PATCH v7 10/19] vhost: force spec specified alignment on types
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.h | 6 +++---
include/linux/virtio_ring.h | 24 +---
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index f8403bd46b85..60cab4c78229 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -67,9 +67,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct file *kick;
struct eventfd_ctx *call_ctx;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 11680e74761a..c3f9ca054250 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,14 +60,32 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __aligned(VRING_DESC_ALIGN_SIZE) vring_desc_t;
+typedef struct vring_avail __aligned(VRING_AVAIL_ALIGN_SIZE) vring_avail_t;
+typedef struct vring_used __aligned(VRING_USED_ALIGN_SIZE) vring_used_t;
+
struct vring {
unsigned int num;
- struct vring_desc *desc;
+ vring_desc_t *desc;
- struct vring_avail *avail;
+ vring_avail_t *avail;
- struct vring_used *used;
+ vring_used_t *used;
};
/*
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 13/19] tools/virtio: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs that make this
explicit.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/ringtest/virtio_ring_0_9.c | 6 +++---
tools/virtio/virtio_test.c | 6 +++---
tools/virtio/vringh_test.c | 18 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/tools/virtio/ringtest/virtio_ring_0_9.c
b/tools/virtio/ringtest/virtio_ring_0_9.c
index 13a035a390e9..e2ab6ac53966 100644
--- a/tools/virtio/ringtest/virtio_ring_0_9.c
+++ b/tools/virtio/ringtest/virtio_ring_0_9.c
@@ -67,13 +67,13 @@ void alloc_ring(void)
int i;
void *p;
- ret = posix_memalign(, 0x1000, vring_size(ring_size, 0x1000));
+ ret = posix_memalign(, 0x1000, vring_legacy_size(ring_size, 0x1000));
if (ret) {
perror("Unable to allocate ring buffer.\n");
exit(3);
}
- memset(p, 0, vring_size(ring_size, 0x1000));
- vring_init(, ring_size, p, 0x1000);
+ memset(p, 0, vring_legacy_size(ring_size, 0x1000));
+ vring_legacy_init(, ring_size, p, 0x1000);
guest.avail_idx = 0;
guest.kicked_avail_idx = -1;
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 93d81cd64ba0..25be607d8711 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -102,10 +102,10 @@ static void vq_info_add(struct vdev_info *dev, int num)
info->idx = dev->nvqs;
info->kick = eventfd(0, EFD_NONBLOCK);
info->call = eventfd(0, EFD_NONBLOCK);
- r = posix_memalign(>ring, 4096, vring_size(num, 4096));
+ r = posix_memalign(>ring, 4096, vring_legacy_size(num, 4096));
assert(r >= 0);
- memset(info->ring, 0, vring_size(num, 4096));
- vring_init(>vring, num, info->ring, 4096);
+ memset(info->ring, 0, vring_legacy_size(num, 4096));
+ vring_legacy_init(>vring, num, info->ring, 4096);
info->vq = vring_new_virtqueue(info->idx,
info->vring.num, 4096, >vdev,
true, false, info->ring,
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 293653463303..8ee2c9a6ad46 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -151,7 +151,7 @@ static int parallel_test(u64 features,
err(1, "Opening /tmp/vringh_test-file");
/* Extra room at the end for some data, and indirects */
- mapsize = vring_size(RINGSIZE, ALIGN)
+ mapsize = vring_legacy_size(RINGSIZE, ALIGN)
+ RINGSIZE * 2 * sizeof(int)
+ RINGSIZE * 6 * sizeof(struct vring_desc);
mapsize = (mapsize + getpagesize() - 1) & ~(getpagesize() - 1);
@@ -185,7 +185,7 @@ static int parallel_test(u64 features,
close(to_guest[0]);
close(to_host[1]);
- vring_init(, RINGSIZE, host_map, ALIGN);
+ vring_legacy_init(, RINGSIZE, host_map, ALIGN);
vringh_init_user(, features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail,
vrh.vring.used);
CPU_SET(first_cpu, _set);
@@ -297,7 +297,7 @@ static int parallel_test(u64 features,
unsigned int finished = 0;
/* We pass sg[]s pointing into here, but we need RINGSIZE+1 */
- data = guest_map + vring_size(RINGSIZE, ALIGN);
+ data = guest_map + vring_legacy_size(RINGSIZE, ALIGN);
indirects = (void *)data + (RINGSIZE + 1) * 2 * sizeof(int);
/* We are the guest. */
@@ -478,7 +478,7 @@ int main(int argc, char *argv[])
if (posix_memalign(&__user_addr_min, PAGE_SIZE, USER_MEM) != 0)
abort();
__user_addr_max = __user_addr_min + USER_MEM;
- memset(__user_addr_min, 0, vring_size(RINGSIZE, ALIGN));
+ memset(__user_addr_min, 0, vring_legacy_size(RINGSIZE, ALIGN));
/* Set up guest side. */
vq = vring_new_virtqueue(0, RINGSIZE, ALIGN, , true, false,
@@ -487,7 +487,7 @@ int main(int argc, char *argv[])
"guest vq");
/* Set up host side. */
- vring_init(, RINGSIZE, __user_addr_min, ALIGN);
+ vring_legacy_init(, RINGSIZE, __user_addr_min, ALIGN);
vringh_init_user(, vdev.features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
@@ -506,7 +506,7 @@ int main(int argc, char *argv[])
sgs[1] = _sg[1];
/* May allocate an indirect, so force it to allocate user addr */
- __kmalloc_fake = __user_addr_min + vring_size(RINGSIZE, ALIGN);
+ __kmalloc_fake = __user_addr_min + vring_legacy_size(RINGSIZE, ALIGN);
err = virtqueue_add_sgs(vq, sgs, 1, 1, , GFP_KERNEL);
if (err)
errx(1, "virtqueue_add_sgs: %i", err);
@@ -556,7 +556,7 @@ int main(int argc, char *argv[])
errx(1,
[PATCH v7 11/19] virtio: add legacy init/size APIs
vring_init/vring_size (in the uapi directory) are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only vring_legacy_init/vring_legacy_size.
Signed-off-by: Michael S. Tsirkin
---
include/linux/virtio_ring.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index c3f9ca054250..766d70bca492 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -140,4 +140,22 @@ void vring_del_virtqueue(struct virtqueue *vq);
void vring_transport_features(struct virtio_device *vdev);
irqreturn_t vring_interrupt(int irq, void *_vq);
+
+static inline void vring_legacy_init(struct vring *vr, unsigned int num, void
*p,
+unsigned long align)
+{
+ vr->num = num;
+ vr->desc = p;
+ vr->avail = (struct vring_avail *)((char *)p + num * sizeof(struct
vring_desc));
+ vr->used = (void *)(((uintptr_t)>avail->ring[num] +
sizeof(__virtio16)
+ + align-1) & ~(align - 1));
+}
+
+static inline unsigned vring_legacy_size(unsigned int num, unsigned long align)
+{
+ return ((sizeof(struct vring_desc) * num + sizeof(__virtio16) * (3 +
num)
++ align - 1) & ~(align - 1))
+ + sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
+}
+
#endif /* _LINUX_VIRTIO_RING_H */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 03/19] tools/virtio: define __KERNEL__
Even though we are building in userspace, we are emulating kernel bits.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index b587b9a7a124..ea6674307507 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -4,7 +4,7 @@ test: virtio_test vringh_test
virtio_test: virtio_ring.o virtio_test.o
vringh_test: vringh_test.o vringh.o virtio_ring.o
-CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h
+CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h -D__KERNEL__
vpath %.c ../../drivers/virtio ../../drivers/vhost
mod:
${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test V=${V}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 09/19] virtio: stop using legacy struct vring in kernel
struct vring (in the uapi directory) and supporting APIs are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only struct vring and
switch everyone to use that.
Signed-off-by: Michael S. Tsirkin
---
drivers/block/virtio_blk.c | 1 +
include/linux/virtio.h | 1 -
include/linux/virtio_ring.h | 10 ++
include/linux/vringh.h | 1 +
include/uapi/linux/virtio_ring.h | 26 --
5 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 0736248999b0..dd5732dc4b07 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#define PART_BITS 4
#define VQ_NAME_LEN 16
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 15f906e4a748..a493eac08393 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -9,7 +9,6 @@
#include
#include
#include
-#include
/**
* virtqueue - a queue to register buffers for sending or receiving.
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..11680e74761a 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,6 +60,16 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
/*
* Creates a virtqueue and allocates the descriptor ring. If
* may_reduce_num is set, then this may allocate a smaller ring than
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index 9e2763d7c159..d71b3710f58e 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -11,6 +11,7 @@
#ifndef _LINUX_VRINGH_H
#define _LINUX_VRINGH_H
#include
+#include
#include
#include
#include
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 9223c3a5c46a..8961a4adda5c 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -166,6 +156,21 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+#ifndef __KERNEL__
+/*
+ * The following definitions have been put in the UAPI header by mistake. We
+ * keep them around to avoid breaking old userspace builds.
+ */
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
@@ -182,6 +187,7 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ align - 1) & ~(align - 1))
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif
#endif /* VIRTIO_RING_NO_LEGACY */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 01/19] tools/virtio: define aligned attribute
Signed-off-by: Michael S. Tsirkin --- tools/virtio/linux/compiler.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h index 903dc9c4bd11..2c51bccb97bb 100644 --- a/tools/virtio/linux/compiler.h +++ b/tools/virtio/linux/compiler.h @@ -7,4 +7,5 @@ #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var +#define __aligned(x) __attribute((__aligned__(x))) #endif -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 02/19] tools/virtio: make asm/barrier.h self contained
We are using abort() so let's include stdlib.h
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/asm/barrier.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
index d0351f83aebe..04d563fc9b95 100644
--- a/tools/virtio/asm/barrier.h
+++ b/tools/virtio/asm/barrier.h
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include
#if defined(__i386__) || defined(__x86_64__)
#define barrier() asm volatile("" ::: "memory")
#define virt_mb() __sync_synchronize()
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 12/19] virtio_ring: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs
that make this explicit.
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_ring.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 58b96baa8d48..2a2b12449bac 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -867,8 +867,10 @@ static struct virtqueue *vring_create_virtqueue_split(
}
/* TODO: allocate each queue chunk individually */
- for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ for (; num && vring_legacy_size(num, vring_align) > PAGE_SIZE;
+num /= 2) {
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr,
GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
if (queue)
@@ -882,14 +884,15 @@ static struct virtqueue *vring_create_virtqueue_split(
if (!queue) {
/* Try to get a single page. You are my only hope! */
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr, GFP_KERNEL|__GFP_ZERO);
}
if (!queue)
return NULL;
- queue_size_in_bytes = vring_size(num, vring_align);
- vring_init(, num, queue, vring_align);
+ queue_size_in_bytes = vring_legacy_size(num, vring_align);
+ vring_legacy_init(, num, queue, vring_align);
vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
@@ -2169,7 +2172,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
return NULL;
- vring_init(, num, pages, vring_align);
+ vring_legacy_init(, num, pages, vring_align);
return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 05/19] virtgpu: pull in uaccess.h
In preparation to virtio header changes, include uaccess.h directly as this file is using copy to/from user. Signed-off-by: Michael S. Tsirkin --- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 205ec4abae2b..2342a8baa5f8 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -27,6 +27,7 @@ #include #include +#include #include #include -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v7 08/19] virtio_input: pull in slab.h
In preparation to virtio header changes, include slab.h directly as this module is using it. Signed-off-by: Michael S. Tsirkin --- drivers/virtio/virtio_input.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/virtio/virtio_input.c b/drivers/virtio/virtio_input.c index 5ae529671b3d..efaf65b0f42d 100644 --- a/drivers/virtio/virtio_input.c +++ b/drivers/virtio/virtio_input.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[vhost:vhost 32/44] drivers/remoteproc/remoteproc_sysfs.c:55:2: error: implicit declaration of function 'kfree'; did you mean 'vfree'?
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: 4e6ebec0de18aaea5f5f814b25bfcae3751c6369
commit: 013a472de94693ba05696d59e7df3224c20a22e6 [32/44] virtio: stop using
legacy struct vring in kernel
config: x86_64-randconfig-s1-20200407 (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
git checkout 013a472de94693ba05696d59e7df3224c20a22e6
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All error/warnings (new ones prefixed by >>):
drivers/remoteproc/remoteproc_sysfs.c: In function 'firmware_store':
>> drivers/remoteproc/remoteproc_sysfs.c:55:2: error: implicit declaration of
>> function 'kfree'; did you mean 'vfree'?
>> [-Werror=implicit-function-declaration]
kfree(rproc->firmware);
^
vfree
cc1: some warnings being treated as errors
--
drivers/char/hw_random/virtio-rng.c: In function 'probe_common':
>> drivers/char/hw_random/virtio-rng.c:92:7: error: implicit declaration of
>> function 'kzalloc'; did you mean 'vzalloc'?
>> [-Werror=implicit-function-declaration]
vi = kzalloc(sizeof(struct virtrng_info), GFP_KERNEL);
^~~
vzalloc
>> drivers/char/hw_random/virtio-rng.c:92:5: warning: assignment makes pointer
>> from integer without a cast [-Wint-conversion]
vi = kzalloc(sizeof(struct virtrng_info), GFP_KERNEL);
^
>> drivers/char/hw_random/virtio-rng.c:125:2: error: implicit declaration of
>> function 'kfree'; did you mean 'vfree'?
>> [-Werror=implicit-function-declaration]
kfree(vi);
^
vfree
cc1: some warnings being treated as errors
vim +55 drivers/remoteproc/remoteproc_sysfs.c
2aefbef0414981 Matt Redfearn 2016-10-19 20
2aefbef0414981 Matt Redfearn 2016-10-19 21 /* Change firmware name via sysfs
*/
2aefbef0414981 Matt Redfearn 2016-10-19 22 static ssize_t
firmware_store(struct device *dev,
2aefbef0414981 Matt Redfearn 2016-10-19 23 struct
device_attribute *attr,
2aefbef0414981 Matt Redfearn 2016-10-19 24 const
char *buf, size_t count)
2aefbef0414981 Matt Redfearn 2016-10-19 25 {
2aefbef0414981 Matt Redfearn 2016-10-19 26 struct rproc *rproc =
to_rproc(dev);
2aefbef0414981 Matt Redfearn 2016-10-19 27 char *p;
2aefbef0414981 Matt Redfearn 2016-10-19 28 int err, len = count;
2aefbef0414981 Matt Redfearn 2016-10-19 29
2aefbef0414981 Matt Redfearn 2016-10-19 30 err =
mutex_lock_interruptible(>lock);
2aefbef0414981 Matt Redfearn 2016-10-19 31 if (err) {
2aefbef0414981 Matt Redfearn 2016-10-19 32 dev_err(dev, "can't
lock rproc %s: %d\n", rproc->name, err);
2aefbef0414981 Matt Redfearn 2016-10-19 33 return -EINVAL;
2aefbef0414981 Matt Redfearn 2016-10-19 34 }
2aefbef0414981 Matt Redfearn 2016-10-19 35
2aefbef0414981 Matt Redfearn 2016-10-19 36 if (rproc->state !=
RPROC_OFFLINE) {
2aefbef0414981 Matt Redfearn 2016-10-19 37 dev_err(dev, "can't
change firmware while running\n");
2aefbef0414981 Matt Redfearn 2016-10-19 38 err = -EBUSY;
2aefbef0414981 Matt Redfearn 2016-10-19 39 goto out;
2aefbef0414981 Matt Redfearn 2016-10-19 40 }
2aefbef0414981 Matt Redfearn 2016-10-19 41
2aefbef0414981 Matt Redfearn 2016-10-19 42 len = strcspn(buf, "\n");
faeadbb6409475 Suman Anna2018-09-14 43 if (!len) {
faeadbb6409475 Suman Anna2018-09-14 44 dev_err(dev, "can't
provide a NULL firmware\n");
faeadbb6409475 Suman Anna2018-09-14 45 err = -EINVAL;
faeadbb6409475 Suman Anna2018-09-14 46 goto out;
faeadbb6409475 Suman Anna2018-09-14 47 }
2aefbef0414981 Matt Redfearn 2016-10-19 48
2aefbef0414981 Matt Redfearn 2016-10-19 49 p = kstrndup(buf, len,
GFP_KERNEL);
2aefbef0414981 Matt Redfearn 2016-10-19 50 if (!p) {
2aefbef0414981 Matt Redfearn 2016-10-19 51 err = -ENOMEM;
2aefbef0414981 Matt Redfearn 2016-10-19 52 goto out;
2aefbef0414981 Matt Redfearn 2016-10-19 53 }
2aefbef0414981 Matt Redfearn 2016-10-19 54
2aefbef0414981 Matt Redfearn 2016-10-19 @55 kfree(rproc->firmware);
2aefbef0414981 Matt Redfearn 2016-10-19 56 rproc->firmware = p;
2aefbef0414981 Matt Redfearn 2016-10-19 57 out:
2aefbef0414981 Matt Redfearn 2016-10-19 58 mutex_unlock(>lock);
2aefbef0414981 Matt Redfearn 2016-10-19 59
2aefbef0414981 Matt Redfearn 2016-10-19 60 return err ? err : count;
2aefbef0414981 Matt Redfearn 2016-10-19 61 }
2aefbef0414981 Matt Redfearn 2016-10-19 62 static DEVICE_ATTR_RW(firmware);
2aefbef0414981 Matt Redfearn 2016-10-19 63
:: The code at line 55 was first introduced by commit
:: 2aefbef041498182ce1d186ed2300298b7a7101a remoteproc: Add a sysfs
interface for firmware and state
:: TO: Matt Redfearn
[vhost:vhost 32/44] drivers/gpu/drm/virtio/virtgpu_ioctl.c:113:7: error: implicit declaration of function 'copy_from_user'; did you mean 'sg_copy_from_buffer'?
tree: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git vhost
head: 4e6ebec0de18aaea5f5f814b25bfcae3751c6369
commit: 013a472de94693ba05696d59e7df3224c20a22e6 [32/44] virtio: stop using
legacy struct vring in kernel
config: m68k-allmodconfig (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 013a472de94693ba05696d59e7df3224c20a22e6
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=m68k
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
drivers/gpu/drm/virtio/virtgpu_ioctl.c: In function
'virtio_gpu_execbuffer_ioctl':
>> drivers/gpu/drm/virtio/virtgpu_ioctl.c:113:7: error: implicit declaration of
>> function 'copy_from_user'; did you mean 'sg_copy_from_buffer'?
>> [-Werror=implicit-function-declaration]
113 | if (copy_from_user(bo_handles, user_bo_handles,
| ^~
| sg_copy_from_buffer
drivers/gpu/drm/virtio/virtgpu_ioctl.c: In function
'virtio_gpu_getparam_ioctl':
>> drivers/gpu/drm/virtio/virtgpu_ioctl.c:196:6: error: implicit declaration of
>> function 'copy_to_user' [-Werror=implicit-function-declaration]
196 | if (copy_to_user(u64_to_user_ptr(param->value), ,
sizeof(int)))
| ^~~~
cc1: some warnings being treated as errors
vim +113 drivers/gpu/drm/virtio/virtgpu_ioctl.c
62fb7a5e10962a Gerd Hoffmann 2014-10-28 46
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 47 /*
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 48 * Usage of execbuffer:
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 49 * Relocations need to take
into account the full VIRTIO_GPUDrawable size.
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 50 * However, the command as
passed from user space must *not* contain the initial
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 51 * VIRTIO_GPUReleaseInfo struct
(first XXX bytes)
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 52 */
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 53 static int
virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
62fb7a5e10962a Gerd Hoffmann 2014-10-28 54
struct drm_file *drm_file)
62fb7a5e10962a Gerd Hoffmann 2014-10-28 55 {
5c32c3dd8501b0 Gustavo Padovan 2016-08-31 56 struct
drm_virtgpu_execbuffer *exbuf = data;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 57 struct
virtio_gpu_device *vgdev = dev->dev_private;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 58 struct virtio_gpu_fpriv
*vfpriv = drm_file->driver_priv;
2cd7b6f08bc4cf Robert Foss 2018-11-12 59 struct virtio_gpu_fence
*out_fence;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 60 int ret;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 61 uint32_t *bo_handles =
NULL;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 62 void __user
*user_bo_handles = NULL;
da758d51968a88 Gerd Hoffmann 2019-08-29 63 struct
virtio_gpu_object_array *buflist = NULL;
2cd7b6f08bc4cf Robert Foss 2018-11-12 64 struct sync_file
*sync_file;
2cd7b6f08bc4cf Robert Foss 2018-11-12 65 int in_fence_fd =
exbuf->fence_fd;
2cd7b6f08bc4cf Robert Foss 2018-11-12 66 int out_fence_fd = -1;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 67 void *buf;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 68
62fb7a5e10962a Gerd Hoffmann 2014-10-28 69 if (vgdev->has_virgl_3d
== false)
62fb7a5e10962a Gerd Hoffmann 2014-10-28 70 return -ENOSYS;
62fb7a5e10962a Gerd Hoffmann 2014-10-28 71
a56f9c868ccf56 Robert Foss 2018-11-12 72 if ((exbuf->flags &
~VIRTGPU_EXECBUF_FLAGS))
a56f9c868ccf56 Robert Foss 2018-11-12 73 return -EINVAL;
a56f9c868ccf56 Robert Foss 2018-11-12 74
a56f9c868ccf56 Robert Foss 2018-11-12 75 exbuf->fence_fd = -1;
a56f9c868ccf56 Robert Foss 2018-11-12 76
2cd7b6f08bc4cf Robert Foss 2018-11-12 77 if (exbuf->flags &
VIRTGPU_EXECBUF_FENCE_FD_IN) {
2cd7b6f08bc4cf Robert Foss 2018-11-12 78 struct
dma_fence *in_fence;
2cd7b6f08bc4cf Robert Foss 2018-11-12 79
2cd7b6f08bc4cf Robert Foss 2018-11-12 80 in_fence =
sync_file_get_fence(in_fence_fd);
2cd7b6f08bc4cf Robert Foss 2018-11-12 81
2cd7b6f08bc4cf Robert Foss 2018-11-12 82 if (!in_fence)
2cd7b6f08bc4cf Robert Foss 2018-11-12 83 return
-EINVAL;
2cd7b6f08bc4cf Robert Foss 2018-11-12 84
2cd7b6f08bc4cf Robert Foss 2018-11-12 85 /*
2cd7b6f08bc4cf Robert Foss 2018-11-12 86 * Wait if the
fence is from a foreign context,
[PATCH v6 02/12] tools/virtio: make asm/barrier.h self contained
We are using abort() so let's include stdlib.h
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/asm/barrier.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
index d0351f83aebe..04d563fc9b95 100644
--- a/tools/virtio/asm/barrier.h
+++ b/tools/virtio/asm/barrier.h
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include
#if defined(__i386__) || defined(__x86_64__)
#define barrier() asm volatile("" ::: "memory")
#define virt_mb() __sync_synchronize()
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v6 01/12] tools/virtio: define aligned attribute
Signed-off-by: Michael S. Tsirkin --- tools/virtio/linux/compiler.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h index 903dc9c4bd11..2c51bccb97bb 100644 --- a/tools/virtio/linux/compiler.h +++ b/tools/virtio/linux/compiler.h @@ -7,4 +7,5 @@ #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var +#define __aligned(x) __attribute((__aligned__(x))) #endif -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v6 09/12] tools/virtio: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs that make this
explicit.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/ringtest/virtio_ring_0_9.c | 6 +++---
tools/virtio/virtio_test.c | 6 +++---
tools/virtio/vringh_test.c | 18 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/tools/virtio/ringtest/virtio_ring_0_9.c
b/tools/virtio/ringtest/virtio_ring_0_9.c
index 13a035a390e9..e2ab6ac53966 100644
--- a/tools/virtio/ringtest/virtio_ring_0_9.c
+++ b/tools/virtio/ringtest/virtio_ring_0_9.c
@@ -67,13 +67,13 @@ void alloc_ring(void)
int i;
void *p;
- ret = posix_memalign(, 0x1000, vring_size(ring_size, 0x1000));
+ ret = posix_memalign(, 0x1000, vring_legacy_size(ring_size, 0x1000));
if (ret) {
perror("Unable to allocate ring buffer.\n");
exit(3);
}
- memset(p, 0, vring_size(ring_size, 0x1000));
- vring_init(, ring_size, p, 0x1000);
+ memset(p, 0, vring_legacy_size(ring_size, 0x1000));
+ vring_legacy_init(, ring_size, p, 0x1000);
guest.avail_idx = 0;
guest.kicked_avail_idx = -1;
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 93d81cd64ba0..25be607d8711 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -102,10 +102,10 @@ static void vq_info_add(struct vdev_info *dev, int num)
info->idx = dev->nvqs;
info->kick = eventfd(0, EFD_NONBLOCK);
info->call = eventfd(0, EFD_NONBLOCK);
- r = posix_memalign(>ring, 4096, vring_size(num, 4096));
+ r = posix_memalign(>ring, 4096, vring_legacy_size(num, 4096));
assert(r >= 0);
- memset(info->ring, 0, vring_size(num, 4096));
- vring_init(>vring, num, info->ring, 4096);
+ memset(info->ring, 0, vring_legacy_size(num, 4096));
+ vring_legacy_init(>vring, num, info->ring, 4096);
info->vq = vring_new_virtqueue(info->idx,
info->vring.num, 4096, >vdev,
true, false, info->ring,
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 293653463303..8ee2c9a6ad46 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -151,7 +151,7 @@ static int parallel_test(u64 features,
err(1, "Opening /tmp/vringh_test-file");
/* Extra room at the end for some data, and indirects */
- mapsize = vring_size(RINGSIZE, ALIGN)
+ mapsize = vring_legacy_size(RINGSIZE, ALIGN)
+ RINGSIZE * 2 * sizeof(int)
+ RINGSIZE * 6 * sizeof(struct vring_desc);
mapsize = (mapsize + getpagesize() - 1) & ~(getpagesize() - 1);
@@ -185,7 +185,7 @@ static int parallel_test(u64 features,
close(to_guest[0]);
close(to_host[1]);
- vring_init(, RINGSIZE, host_map, ALIGN);
+ vring_legacy_init(, RINGSIZE, host_map, ALIGN);
vringh_init_user(, features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail,
vrh.vring.used);
CPU_SET(first_cpu, _set);
@@ -297,7 +297,7 @@ static int parallel_test(u64 features,
unsigned int finished = 0;
/* We pass sg[]s pointing into here, but we need RINGSIZE+1 */
- data = guest_map + vring_size(RINGSIZE, ALIGN);
+ data = guest_map + vring_legacy_size(RINGSIZE, ALIGN);
indirects = (void *)data + (RINGSIZE + 1) * 2 * sizeof(int);
/* We are the guest. */
@@ -478,7 +478,7 @@ int main(int argc, char *argv[])
if (posix_memalign(&__user_addr_min, PAGE_SIZE, USER_MEM) != 0)
abort();
__user_addr_max = __user_addr_min + USER_MEM;
- memset(__user_addr_min, 0, vring_size(RINGSIZE, ALIGN));
+ memset(__user_addr_min, 0, vring_legacy_size(RINGSIZE, ALIGN));
/* Set up guest side. */
vq = vring_new_virtqueue(0, RINGSIZE, ALIGN, , true, false,
@@ -487,7 +487,7 @@ int main(int argc, char *argv[])
"guest vq");
/* Set up host side. */
- vring_init(, RINGSIZE, __user_addr_min, ALIGN);
+ vring_legacy_init(, RINGSIZE, __user_addr_min, ALIGN);
vringh_init_user(, vdev.features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
@@ -506,7 +506,7 @@ int main(int argc, char *argv[])
sgs[1] = _sg[1];
/* May allocate an indirect, so force it to allocate user addr */
- __kmalloc_fake = __user_addr_min + vring_size(RINGSIZE, ALIGN);
+ __kmalloc_fake = __user_addr_min + vring_legacy_size(RINGSIZE, ALIGN);
err = virtqueue_add_sgs(vq, sgs, 1, 1, , GFP_KERNEL);
if (err)
errx(1, "virtqueue_add_sgs: %i", err);
@@ -556,7 +556,7 @@ int main(int argc, char *argv[])
errx(1,
[PATCH v6 04/12] virtio: add VIRTIO_RING_NO_LEGACY
From: Matej Genci
Add macro to disable legacy vring functions.
Signed-off-by: Matej Genci
Link: https://lore.kernel.org/r/20190911124942.243713-1-matej.ge...@nutanix.com
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_pci_modern.c | 1 +
include/uapi/linux/virtio_ring.h | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 7abcc50838b8..db93cedd262f 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -16,6 +16,7 @@
#include
#define VIRTIO_PCI_NO_LEGACY
+#define VIRTIO_RING_NO_LEGACY
#include "virtio_pci_common.h"
/*
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..9223c3a5c46a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -135,6 +135,8 @@ struct vring {
#define VRING_USED_ALIGN_SIZE 4
#define VRING_DESC_ALIGN_SIZE 16
+#ifndef VIRTIO_RING_NO_LEGACY
+
/* The standard layout for the ring is a continuous chunk of memory which looks
* like this. We assume num is a power of 2.
*
@@ -181,6 +183,8 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif /* VIRTIO_RING_NO_LEGACY */
+
/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
/* Assuming a given event_idx value from the other side, if
* we have just incremented index from old to new_idx,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v6 05/12] virtio: stop using legacy struct vring in kernel
struct vring (in the uapi directory) and supporting APIs are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only struct vring and
switch everyone to use that.
Signed-off-by: Michael S. Tsirkin
---
drivers/block/virtio_blk.c | 1 +
include/linux/virtio.h | 1 -
include/linux/virtio_ring.h | 10 ++
include/linux/vringh.h | 1 +
include/uapi/linux/virtio_ring.h | 26 --
5 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 0736248999b0..dd5732dc4b07 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#define PART_BITS 4
#define VQ_NAME_LEN 16
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 15f906e4a748..a493eac08393 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -9,7 +9,6 @@
#include
#include
#include
-#include
/**
* virtqueue - a queue to register buffers for sending or receiving.
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..11680e74761a 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,6 +60,16 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
/*
* Creates a virtqueue and allocates the descriptor ring. If
* may_reduce_num is set, then this may allocate a smaller ring than
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index 9e2763d7c159..d71b3710f58e 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -11,6 +11,7 @@
#ifndef _LINUX_VRINGH_H
#define _LINUX_VRINGH_H
#include
+#include
#include
#include
#include
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 9223c3a5c46a..8961a4adda5c 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -166,6 +156,21 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+#ifndef __KERNEL__
+/*
+ * The following definitions have been put in the UAPI header by mistake. We
+ * keep them around to avoid breaking old userspace builds.
+ */
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
@@ -182,6 +187,7 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ align - 1) & ~(align - 1))
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif
#endif /* VIRTIO_RING_NO_LEGACY */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v6 08/12] virtio_ring: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs
that make this explicit.
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_ring.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 58b96baa8d48..2a2b12449bac 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -867,8 +867,10 @@ static struct virtqueue *vring_create_virtqueue_split(
}
/* TODO: allocate each queue chunk individually */
- for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ for (; num && vring_legacy_size(num, vring_align) > PAGE_SIZE;
+num /= 2) {
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr,
GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
if (queue)
@@ -882,14 +884,15 @@ static struct virtqueue *vring_create_virtqueue_split(
if (!queue) {
/* Try to get a single page. You are my only hope! */
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr, GFP_KERNEL|__GFP_ZERO);
}
if (!queue)
return NULL;
- queue_size_in_bytes = vring_size(num, vring_align);
- vring_init(, num, queue, vring_align);
+ queue_size_in_bytes = vring_legacy_size(num, vring_align);
+ vring_legacy_init(, num, queue, vring_align);
vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
@@ -2169,7 +2172,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
return NULL;
- vring_init(, num, pages, vring_align);
+ vring_legacy_init(, num, pages, vring_align);
return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v6 03/12] tools/virtio: define __KERNEL__
Even though we are building in userspace, we are emulating kernel bits.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index b587b9a7a124..ea6674307507 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -4,7 +4,7 @@ test: virtio_test vringh_test
virtio_test: virtio_ring.o virtio_test.o
vringh_test: vringh_test.o vringh.o virtio_ring.o
-CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h
+CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h -D__KERNEL__
vpath %.c ../../drivers/virtio ../../drivers/vhost
mod:
${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test V=${V}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 6/6] kernel: set USER_DS in kthread_use_mm
On Sat, Apr 04, 2020 at 11:41:01AM +0200, Christoph Hellwig wrote:
> Some architectures like arm64 and s390 require USER_DS to be set for
> kernel threads to access user address space, which is the whole purpose
> of kthread_use_mm, but other like x86 don't. That has lead to a huge
> mess where some callers are fixed up once they are tested on said
> architectures, while others linger around and yet other like io_uring
> try to do "clever" optimizations for what usually is just a trivial
> asignment to a member in the thread_struct for most architectures.
>
> Make kthread_use_mm set USER_DS, and kthread_unuse_mm restore to the
> previous value instead.
>
> Signed-off-by: Christoph Hellwig
I'm ok with vhost bits:
Acked-by: Michael S. Tsirkin
> ---
> drivers/usb/gadget/function/f_fs.c | 4
> drivers/vhost/vhost.c | 3 ---
> fs/io-wq.c | 8 ++--
> fs/io_uring.c | 4
> kernel/kthread.c | 6 ++
> 5 files changed, 8 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index d9e48bd7c692..a1198f4c527c 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -824,13 +824,9 @@ static void ffs_user_copy_worker(struct work_struct
> *work)
> bool kiocb_has_eventfd = io_data->kiocb->ki_flags & IOCB_EVENTFD;
>
> if (io_data->read && ret > 0) {
> - mm_segment_t oldfs = get_fs();
> -
> - set_fs(USER_DS);
> kthread_use_mm(io_data->mm);
> ret = ffs_copy_to_iter(io_data->buf, ret, _data->data);
> kthread_unuse_mm(io_data->mm);
> - set_fs(oldfs);
> }
>
> io_data->kiocb->ki_complete(io_data->kiocb, ret, ret);
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 1787d426a956..b5229ae01d3b 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -333,9 +333,7 @@ static int vhost_worker(void *data)
> struct vhost_dev *dev = data;
> struct vhost_work *work, *work_next;
> struct llist_node *node;
> - mm_segment_t oldfs = get_fs();
>
> - set_fs(USER_DS);
> kthread_use_mm(dev->mm);
>
> for (;;) {
> @@ -365,7 +363,6 @@ static int vhost_worker(void *data)
> }
> }
> kthread_unuse_mm(dev->mm);
> - set_fs(oldfs);
> return 0;
> }
>
> diff --git a/fs/io-wq.c b/fs/io-wq.c
> index 83c2868eff2a..75cc2f31816d 100644
> --- a/fs/io-wq.c
> +++ b/fs/io-wq.c
> @@ -168,7 +168,6 @@ static bool __io_worker_unuse(struct io_wqe *wqe, struct
> io_worker *worker)
> dropped_lock = true;
> }
> __set_current_state(TASK_RUNNING);
> - set_fs(KERNEL_DS);
> kthread_unuse_mm(worker->mm);
> mmput(worker->mm);
> worker->mm = NULL;
> @@ -420,14 +419,11 @@ static void io_wq_switch_mm(struct io_worker *worker,
> struct io_wq_work *work)
> mmput(worker->mm);
> worker->mm = NULL;
> }
> - if (!work->mm) {
> - set_fs(KERNEL_DS);
> + if (!work->mm)
> return;
> - }
> +
> if (mmget_not_zero(work->mm)) {
> kthread_use_mm(work->mm);
> - if (!worker->mm)
> - set_fs(USER_DS);
> worker->mm = work->mm;
> /* hang on to this mm */
> work->mm = NULL;
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 367406381044..c332a34e8b34 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5871,15 +5871,12 @@ static int io_sq_thread(void *data)
> struct io_ring_ctx *ctx = data;
> struct mm_struct *cur_mm = NULL;
> const struct cred *old_cred;
> - mm_segment_t old_fs;
> DEFINE_WAIT(wait);
> unsigned long timeout;
> int ret = 0;
>
> complete(>completions[1]);
>
> - old_fs = get_fs();
> - set_fs(USER_DS);
> old_cred = override_creds(ctx->creds);
>
> timeout = jiffies + ctx->sq_thread_idle;
> @@ -5985,7 +5982,6 @@ static int io_sq_thread(void *data)
> if (current->task_works)
> task_work_run();
>
> - set_fs(old_fs);
> if (cur_mm) {
> kthread_unuse_mm(cur_mm);
> mmput(cur_mm);
> diff --git a/kernel/kthread.c b/kernel/kthread.c
> index 316db17f6b4f..9e27d01b6d78 100644
> --- a/kernel/kthread.c
> +++ b/kernel/kthread.c
> @@ -52,6 +52,7 @@ struct kthread {
> unsigned long flags;
> unsigned int cpu;
> void *data;
> + mm_segment_t oldfs;
> struct completion parked;
> struct completion exited;
> #ifdef CONFIG_BLK_CGROUP
> @@ -1235,6 +1236,9 @@ void kthread_use_mm(struct mm_struct *mm)
>
> if (active_mm != mm)
> mmdrop(active_mm);
> +
> + to_kthread(tsk)->oldfs = get_fs();
> + set_fs(USER_DS);
> }
>
[PATCH v5 07/12] virtio: add legacy init/size APIs
vring_init/vring_size (in the uapi directory) are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only vring_legacy_init/vring_legacy_size.
Signed-off-by: Michael S. Tsirkin
---
include/linux/virtio_ring.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index c3f9ca054250..766d70bca492 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -140,4 +140,22 @@ void vring_del_virtqueue(struct virtqueue *vq);
void vring_transport_features(struct virtio_device *vdev);
irqreturn_t vring_interrupt(int irq, void *_vq);
+
+static inline void vring_legacy_init(struct vring *vr, unsigned int num, void
*p,
+unsigned long align)
+{
+ vr->num = num;
+ vr->desc = p;
+ vr->avail = (struct vring_avail *)((char *)p + num * sizeof(struct
vring_desc));
+ vr->used = (void *)(((uintptr_t)>avail->ring[num] +
sizeof(__virtio16)
+ + align-1) & ~(align - 1));
+}
+
+static inline unsigned vring_legacy_size(unsigned int num, unsigned long align)
+{
+ return ((sizeof(struct vring_desc) * num + sizeof(__virtio16) * (3 +
num)
++ align - 1) & ~(align - 1))
+ + sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
+}
+
#endif /* _LINUX_VIRTIO_RING_H */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v5 02/12] tools/virtio: make asm/barrier.h self contained
We are using abort() so let's include stdlib.h
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/asm/barrier.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
index d0351f83aebe..04d563fc9b95 100644
--- a/tools/virtio/asm/barrier.h
+++ b/tools/virtio/asm/barrier.h
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include
#if defined(__i386__) || defined(__x86_64__)
#define barrier() asm volatile("" ::: "memory")
#define virt_mb() __sync_synchronize()
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v5 03/12] tools/virtio: define __KERNEL__
Even though we are building in userspace, we are emulating kernel bits.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index b587b9a7a124..ea6674307507 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -4,7 +4,7 @@ test: virtio_test vringh_test
virtio_test: virtio_ring.o virtio_test.o
vringh_test: vringh_test.o vringh.o virtio_ring.o
-CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h
+CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h -D__KERNEL__
vpath %.c ../../drivers/virtio ../../drivers/vhost
mod:
${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test V=${V}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v5 04/12] virtio: add VIRTIO_RING_NO_LEGACY
From: Matej Genci
Add macro to disable legacy vring functions.
Signed-off-by: Matej Genci
Link: https://lore.kernel.org/r/20190911124942.243713-1-matej.ge...@nutanix.com
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_pci_modern.c | 1 +
include/uapi/linux/virtio_ring.h | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 7abcc50838b8..db93cedd262f 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -16,6 +16,7 @@
#include
#define VIRTIO_PCI_NO_LEGACY
+#define VIRTIO_RING_NO_LEGACY
#include "virtio_pci_common.h"
/*
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..9223c3a5c46a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -135,6 +135,8 @@ struct vring {
#define VRING_USED_ALIGN_SIZE 4
#define VRING_DESC_ALIGN_SIZE 16
+#ifndef VIRTIO_RING_NO_LEGACY
+
/* The standard layout for the ring is a continuous chunk of memory which looks
* like this. We assume num is a power of 2.
*
@@ -181,6 +183,8 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif /* VIRTIO_RING_NO_LEGACY */
+
/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
/* Assuming a given event_idx value from the other side, if
* we have just incremented index from old to new_idx,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v5 08/12] virtio_ring: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs
that make this explicit.
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_ring.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 58b96baa8d48..2a2b12449bac 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -867,8 +867,10 @@ static struct virtqueue *vring_create_virtqueue_split(
}
/* TODO: allocate each queue chunk individually */
- for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ for (; num && vring_legacy_size(num, vring_align) > PAGE_SIZE;
+num /= 2) {
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr,
GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
if (queue)
@@ -882,14 +884,15 @@ static struct virtqueue *vring_create_virtqueue_split(
if (!queue) {
/* Try to get a single page. You are my only hope! */
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr, GFP_KERNEL|__GFP_ZERO);
}
if (!queue)
return NULL;
- queue_size_in_bytes = vring_size(num, vring_align);
- vring_init(, num, queue, vring_align);
+ queue_size_in_bytes = vring_legacy_size(num, vring_align);
+ vring_legacy_init(, num, queue, vring_align);
vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
@@ -2169,7 +2172,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
return NULL;
- vring_init(, num, pages, vring_align);
+ vring_legacy_init(, num, pages, vring_align);
return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v5 09/12] tools/virtio: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs that make this
explicit.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/ringtest/virtio_ring_0_9.c | 6 +++---
tools/virtio/virtio_test.c | 6 +++---
tools/virtio/vringh_test.c | 18 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/tools/virtio/ringtest/virtio_ring_0_9.c
b/tools/virtio/ringtest/virtio_ring_0_9.c
index 13a035a390e9..e2ab6ac53966 100644
--- a/tools/virtio/ringtest/virtio_ring_0_9.c
+++ b/tools/virtio/ringtest/virtio_ring_0_9.c
@@ -67,13 +67,13 @@ void alloc_ring(void)
int i;
void *p;
- ret = posix_memalign(, 0x1000, vring_size(ring_size, 0x1000));
+ ret = posix_memalign(, 0x1000, vring_legacy_size(ring_size, 0x1000));
if (ret) {
perror("Unable to allocate ring buffer.\n");
exit(3);
}
- memset(p, 0, vring_size(ring_size, 0x1000));
- vring_init(, ring_size, p, 0x1000);
+ memset(p, 0, vring_legacy_size(ring_size, 0x1000));
+ vring_legacy_init(, ring_size, p, 0x1000);
guest.avail_idx = 0;
guest.kicked_avail_idx = -1;
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 93d81cd64ba0..25be607d8711 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -102,10 +102,10 @@ static void vq_info_add(struct vdev_info *dev, int num)
info->idx = dev->nvqs;
info->kick = eventfd(0, EFD_NONBLOCK);
info->call = eventfd(0, EFD_NONBLOCK);
- r = posix_memalign(>ring, 4096, vring_size(num, 4096));
+ r = posix_memalign(>ring, 4096, vring_legacy_size(num, 4096));
assert(r >= 0);
- memset(info->ring, 0, vring_size(num, 4096));
- vring_init(>vring, num, info->ring, 4096);
+ memset(info->ring, 0, vring_legacy_size(num, 4096));
+ vring_legacy_init(>vring, num, info->ring, 4096);
info->vq = vring_new_virtqueue(info->idx,
info->vring.num, 4096, >vdev,
true, false, info->ring,
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 293653463303..8ee2c9a6ad46 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -151,7 +151,7 @@ static int parallel_test(u64 features,
err(1, "Opening /tmp/vringh_test-file");
/* Extra room at the end for some data, and indirects */
- mapsize = vring_size(RINGSIZE, ALIGN)
+ mapsize = vring_legacy_size(RINGSIZE, ALIGN)
+ RINGSIZE * 2 * sizeof(int)
+ RINGSIZE * 6 * sizeof(struct vring_desc);
mapsize = (mapsize + getpagesize() - 1) & ~(getpagesize() - 1);
@@ -185,7 +185,7 @@ static int parallel_test(u64 features,
close(to_guest[0]);
close(to_host[1]);
- vring_init(, RINGSIZE, host_map, ALIGN);
+ vring_legacy_init(, RINGSIZE, host_map, ALIGN);
vringh_init_user(, features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail,
vrh.vring.used);
CPU_SET(first_cpu, _set);
@@ -297,7 +297,7 @@ static int parallel_test(u64 features,
unsigned int finished = 0;
/* We pass sg[]s pointing into here, but we need RINGSIZE+1 */
- data = guest_map + vring_size(RINGSIZE, ALIGN);
+ data = guest_map + vring_legacy_size(RINGSIZE, ALIGN);
indirects = (void *)data + (RINGSIZE + 1) * 2 * sizeof(int);
/* We are the guest. */
@@ -478,7 +478,7 @@ int main(int argc, char *argv[])
if (posix_memalign(&__user_addr_min, PAGE_SIZE, USER_MEM) != 0)
abort();
__user_addr_max = __user_addr_min + USER_MEM;
- memset(__user_addr_min, 0, vring_size(RINGSIZE, ALIGN));
+ memset(__user_addr_min, 0, vring_legacy_size(RINGSIZE, ALIGN));
/* Set up guest side. */
vq = vring_new_virtqueue(0, RINGSIZE, ALIGN, , true, false,
@@ -487,7 +487,7 @@ int main(int argc, char *argv[])
"guest vq");
/* Set up host side. */
- vring_init(, RINGSIZE, __user_addr_min, ALIGN);
+ vring_legacy_init(, RINGSIZE, __user_addr_min, ALIGN);
vringh_init_user(, vdev.features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
@@ -506,7 +506,7 @@ int main(int argc, char *argv[])
sgs[1] = _sg[1];
/* May allocate an indirect, so force it to allocate user addr */
- __kmalloc_fake = __user_addr_min + vring_size(RINGSIZE, ALIGN);
+ __kmalloc_fake = __user_addr_min + vring_legacy_size(RINGSIZE, ALIGN);
err = virtqueue_add_sgs(vq, sgs, 1, 1, , GFP_KERNEL);
if (err)
errx(1, "virtqueue_add_sgs: %i", err);
@@ -556,7 +556,7 @@ int main(int argc, char *argv[])
errx(1,
[PATCH v5 05/12] virtio: stop using legacy struct vring in kernel
struct vring (in the uapi directory) and supporting APIs are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only struct vring and
switch everyone to use that.
Signed-off-by: Michael S. Tsirkin
---
drivers/block/virtio_blk.c | 1 +
include/linux/virtio.h | 1 -
include/linux/virtio_ring.h | 10 ++
include/linux/vringh.h | 1 +
include/uapi/linux/virtio_ring.h | 26 --
5 files changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 0736248999b0..dd5732dc4b07 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#define PART_BITS 4
#define VQ_NAME_LEN 16
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 15f906e4a748..a493eac08393 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -9,7 +9,6 @@
#include
#include
#include
-#include
/**
* virtqueue - a queue to register buffers for sending or receiving.
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..11680e74761a 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,6 +60,16 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
/*
* Creates a virtqueue and allocates the descriptor ring. If
* may_reduce_num is set, then this may allocate a smaller ring than
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index 9e2763d7c159..d71b3710f58e 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -11,6 +11,7 @@
#ifndef _LINUX_VRINGH_H
#define _LINUX_VRINGH_H
#include
+#include
#include
#include
#include
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 9223c3a5c46a..8961a4adda5c 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -166,6 +156,21 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+#ifndef __KERNEL__
+/*
+ * The following definitions have been put in the UAPI header by mistake. We
+ * keep them around to avoid breaking old userspace builds.
+ */
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
@@ -182,6 +187,7 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ align - 1) & ~(align - 1))
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif
#endif /* VIRTIO_RING_NO_LEGACY */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v5 01/12] tools/virtio: define aligned attribute
Signed-off-by: Michael S. Tsirkin --- tools/virtio/linux/compiler.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h index 903dc9c4bd11..2c51bccb97bb 100644 --- a/tools/virtio/linux/compiler.h +++ b/tools/virtio/linux/compiler.h @@ -7,4 +7,5 @@ #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var +#define __aligned(x) __attribute((__aligned__(x))) #endif -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v5 06/12] vhost: force spec specified alignment on types
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.h | 6 +++---
include/linux/virtio_ring.h | 24 +---
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index f8403bd46b85..60cab4c78229 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -67,9 +67,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct file *kick;
struct eventfd_ctx *call_ctx;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 11680e74761a..c3f9ca054250 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,14 +60,32 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __aligned(VRING_DESC_ALIGN_SIZE) vring_desc_t;
+typedef struct vring_avail __aligned(VRING_AVAIL_ALIGN_SIZE) vring_avail_t;
+typedef struct vring_used __aligned(VRING_USED_ALIGN_SIZE) vring_used_t;
+
struct vring {
unsigned int num;
- struct vring_desc *desc;
+ vring_desc_t *desc;
- struct vring_avail *avail;
+ vring_avail_t *avail;
- struct vring_used *used;
+ vring_used_t *used;
};
/*
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[GIT PULL] vhost: fixes, vdpa
Now that many more architectures build vhost, a couple of these (um, and arm with deprecated oabi) have reported build failures with randconfig, however fixes for that need a bit more discussion/testing and will be merged separately. Not a regression - these previously simply didn't have vhost at all. Also, there's some DMA API code in the vdpa simulator is hacky - if no solution surfaces soon we can always disable it before release: it's not a big deal either way as it's just test code. The following changes since commit 16fbf79b0f83bc752cee8589279f1ebfe57b3b6e: Linux 5.6-rc7 (2020-03-22 18:31:56 -0700) are available in the Git repository at: https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus for you to fetch changes up to c9b9f5f8c0f3cdb893cb86c168cdaa3aa5ed7278: vdpa: move to drivers/vdpa (2020-04-02 10:41:40 -0400) virtio: fixes, vdpa Some bug fixes. Balloon reverted to use the OOM handler again. The new vdpa subsystem with two first drivers. Signed-off-by: Michael S. Tsirkin David Hildenbrand (1): virtio-balloon: Switch back to OOM handler for VIRTIO_BALLOON_F_DEFLATE_ON_OOM Jason Wang (7): vhost: refine vhost and vringh kconfig vhost: allow per device message handler vhost: factor out IOTLB vringh: IOTLB support vDPA: introduce vDPA bus virtio: introduce a vDPA based transport vdpasim: vDPA device simulator Michael S. Tsirkin (2): tools/virtio: option to build an out of tree module vdpa: move to drivers/vdpa Tiwei Bie (1): vhost: introduce vDPA-based backend Yuri Benditovich (3): virtio-net: Introduce extended RSC feature virtio-net: Introduce RSS receive steering feature virtio-net: Introduce hash report feature Zhu Lingshan (1): virtio: Intel IFC VF driver for VDPA MAINTAINERS | 3 + arch/arm/kvm/Kconfig | 2 - arch/arm64/kvm/Kconfig | 2 - arch/mips/kvm/Kconfig| 2 - arch/powerpc/kvm/Kconfig | 2 - arch/s390/kvm/Kconfig| 4 - arch/x86/kvm/Kconfig | 4 - drivers/Kconfig | 4 + drivers/Makefile | 1 + drivers/misc/mic/Kconfig | 4 - drivers/net/caif/Kconfig | 4 - drivers/vdpa/Kconfig | 37 ++ drivers/vdpa/Makefile| 4 + drivers/vdpa/ifcvf/Makefile | 3 + drivers/vdpa/ifcvf/ifcvf_base.c | 389 + drivers/vdpa/ifcvf/ifcvf_base.h | 118 ++ drivers/vdpa/ifcvf/ifcvf_main.c | 435 +++ drivers/vdpa/vdpa.c | 180 drivers/vdpa/vdpa_sim/Makefile | 2 + drivers/vdpa/vdpa_sim/vdpa_sim.c | 629 drivers/vhost/Kconfig| 45 +- drivers/vhost/Kconfig.vringh | 6 - drivers/vhost/Makefile | 6 + drivers/vhost/iotlb.c| 177 drivers/vhost/net.c | 5 +- drivers/vhost/scsi.c | 2 +- drivers/vhost/vdpa.c | 883 +++ drivers/vhost/vhost.c| 233 --- drivers/vhost/vhost.h| 45 +- drivers/vhost/vringh.c | 421 ++- drivers/vhost/vsock.c| 2 +- drivers/virtio/Kconfig | 13 + drivers/virtio/Makefile | 1 + drivers/virtio/virtio_balloon.c | 107 ++--- drivers/virtio/virtio_vdpa.c | 396 ++ include/linux/vdpa.h | 253 +++ include/linux/vhost_iotlb.h | 47 +++ include/linux/vringh.h | 36 ++ include/uapi/linux/vhost.h | 24 ++ include/uapi/linux/vhost_types.h | 8 + include/uapi/linux/virtio_net.h | 102 - tools/virtio/Makefile| 27 +- 42 files changed, 4354 insertions(+), 314 deletions(-) create mode 100644 drivers/vdpa/Kconfig create mode 100644 drivers/vdpa/Makefile create mode 100644 drivers/vdpa/ifcvf/Makefile create mode 100644 drivers/vdpa/ifcvf/ifcvf_base.c create mode 100644 drivers/vdpa/ifcvf/ifcvf_base.h create mode 100644 drivers/vdpa/ifcvf/ifcvf_main.c create mode 100644 drivers/vdpa/vdpa.c create mode 100644 drivers/vdpa/vdpa_sim/Makefile create mode 100644 drivers/vdpa/vdpa_sim/vdpa_sim.c delete mode 100644 drivers/vhost/Kconfig.vringh create mode 100644 drivers/vhost/iotlb.c create mode 100644 drivers/vhost/vdpa.c create mode 100644 drivers/virtio/virtio_vdpa.c create mode 100644 include/linux/vdpa.h create mode 100644 include/linux/vhost_iotlb.h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 09/12] tools/virtio: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs that make this
explicit.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/ringtest/virtio_ring_0_9.c | 6 +++---
tools/virtio/virtio_test.c | 6 +++---
tools/virtio/vringh_test.c | 18 +-
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/tools/virtio/ringtest/virtio_ring_0_9.c
b/tools/virtio/ringtest/virtio_ring_0_9.c
index 13a035a390e9..e2ab6ac53966 100644
--- a/tools/virtio/ringtest/virtio_ring_0_9.c
+++ b/tools/virtio/ringtest/virtio_ring_0_9.c
@@ -67,13 +67,13 @@ void alloc_ring(void)
int i;
void *p;
- ret = posix_memalign(, 0x1000, vring_size(ring_size, 0x1000));
+ ret = posix_memalign(, 0x1000, vring_legacy_size(ring_size, 0x1000));
if (ret) {
perror("Unable to allocate ring buffer.\n");
exit(3);
}
- memset(p, 0, vring_size(ring_size, 0x1000));
- vring_init(, ring_size, p, 0x1000);
+ memset(p, 0, vring_legacy_size(ring_size, 0x1000));
+ vring_legacy_init(, ring_size, p, 0x1000);
guest.avail_idx = 0;
guest.kicked_avail_idx = -1;
diff --git a/tools/virtio/virtio_test.c b/tools/virtio/virtio_test.c
index 93d81cd64ba0..25be607d8711 100644
--- a/tools/virtio/virtio_test.c
+++ b/tools/virtio/virtio_test.c
@@ -102,10 +102,10 @@ static void vq_info_add(struct vdev_info *dev, int num)
info->idx = dev->nvqs;
info->kick = eventfd(0, EFD_NONBLOCK);
info->call = eventfd(0, EFD_NONBLOCK);
- r = posix_memalign(>ring, 4096, vring_size(num, 4096));
+ r = posix_memalign(>ring, 4096, vring_legacy_size(num, 4096));
assert(r >= 0);
- memset(info->ring, 0, vring_size(num, 4096));
- vring_init(>vring, num, info->ring, 4096);
+ memset(info->ring, 0, vring_legacy_size(num, 4096));
+ vring_legacy_init(>vring, num, info->ring, 4096);
info->vq = vring_new_virtqueue(info->idx,
info->vring.num, 4096, >vdev,
true, false, info->ring,
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 293653463303..8ee2c9a6ad46 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -151,7 +151,7 @@ static int parallel_test(u64 features,
err(1, "Opening /tmp/vringh_test-file");
/* Extra room at the end for some data, and indirects */
- mapsize = vring_size(RINGSIZE, ALIGN)
+ mapsize = vring_legacy_size(RINGSIZE, ALIGN)
+ RINGSIZE * 2 * sizeof(int)
+ RINGSIZE * 6 * sizeof(struct vring_desc);
mapsize = (mapsize + getpagesize() - 1) & ~(getpagesize() - 1);
@@ -185,7 +185,7 @@ static int parallel_test(u64 features,
close(to_guest[0]);
close(to_host[1]);
- vring_init(, RINGSIZE, host_map, ALIGN);
+ vring_legacy_init(, RINGSIZE, host_map, ALIGN);
vringh_init_user(, features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail,
vrh.vring.used);
CPU_SET(first_cpu, _set);
@@ -297,7 +297,7 @@ static int parallel_test(u64 features,
unsigned int finished = 0;
/* We pass sg[]s pointing into here, but we need RINGSIZE+1 */
- data = guest_map + vring_size(RINGSIZE, ALIGN);
+ data = guest_map + vring_legacy_size(RINGSIZE, ALIGN);
indirects = (void *)data + (RINGSIZE + 1) * 2 * sizeof(int);
/* We are the guest. */
@@ -478,7 +478,7 @@ int main(int argc, char *argv[])
if (posix_memalign(&__user_addr_min, PAGE_SIZE, USER_MEM) != 0)
abort();
__user_addr_max = __user_addr_min + USER_MEM;
- memset(__user_addr_min, 0, vring_size(RINGSIZE, ALIGN));
+ memset(__user_addr_min, 0, vring_legacy_size(RINGSIZE, ALIGN));
/* Set up guest side. */
vq = vring_new_virtqueue(0, RINGSIZE, ALIGN, , true, false,
@@ -487,7 +487,7 @@ int main(int argc, char *argv[])
"guest vq");
/* Set up host side. */
- vring_init(, RINGSIZE, __user_addr_min, ALIGN);
+ vring_legacy_init(, RINGSIZE, __user_addr_min, ALIGN);
vringh_init_user(, vdev.features, RINGSIZE, true,
vrh.vring.desc, vrh.vring.avail, vrh.vring.used);
@@ -506,7 +506,7 @@ int main(int argc, char *argv[])
sgs[1] = _sg[1];
/* May allocate an indirect, so force it to allocate user addr */
- __kmalloc_fake = __user_addr_min + vring_size(RINGSIZE, ALIGN);
+ __kmalloc_fake = __user_addr_min + vring_legacy_size(RINGSIZE, ALIGN);
err = virtqueue_add_sgs(vq, sgs, 1, 1, , GFP_KERNEL);
if (err)
errx(1, "virtqueue_add_sgs: %i", err);
@@ -556,7 +556,7 @@ int main(int argc, char *argv[])
errx(1,
[PATCH v4 02/12] tools/virtio: make asm/barrier.h self contained
We are using abort() so let's include stdlib.h
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/asm/barrier.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/virtio/asm/barrier.h b/tools/virtio/asm/barrier.h
index d0351f83aebe..04d563fc9b95 100644
--- a/tools/virtio/asm/barrier.h
+++ b/tools/virtio/asm/barrier.h
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#include
#if defined(__i386__) || defined(__x86_64__)
#define barrier() asm volatile("" ::: "memory")
#define virt_mb() __sync_synchronize()
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 06/12] vhost: force spec specified alignment on types
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.h | 6 +++---
include/linux/virtio_ring.h | 24 +---
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index f8403bd46b85..60cab4c78229 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -67,9 +67,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct file *kick;
struct eventfd_ctx *call_ctx;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 11680e74761a..c3f9ca054250 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,14 +60,32 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __aligned(VRING_DESC_ALIGN_SIZE) vring_desc_t;
+typedef struct vring_avail __aligned(VRING_AVAIL_ALIGN_SIZE) vring_avail_t;
+typedef struct vring_used __aligned(VRING_USED_ALIGN_SIZE) vring_used_t;
+
struct vring {
unsigned int num;
- struct vring_desc *desc;
+ vring_desc_t *desc;
- struct vring_avail *avail;
+ vring_avail_t *avail;
- struct vring_used *used;
+ vring_used_t *used;
};
/*
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 08/12] virtio_ring: switch to virtio_legacy_init/size
These are used for legacy ring format, switch to APIs
that make this explicit.
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_ring.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 58b96baa8d48..2a2b12449bac 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -867,8 +867,10 @@ static struct virtqueue *vring_create_virtqueue_split(
}
/* TODO: allocate each queue chunk individually */
- for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ for (; num && vring_legacy_size(num, vring_align) > PAGE_SIZE;
+num /= 2) {
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr,
GFP_KERNEL|__GFP_NOWARN|__GFP_ZERO);
if (queue)
@@ -882,14 +884,15 @@ static struct virtqueue *vring_create_virtqueue_split(
if (!queue) {
/* Try to get a single page. You are my only hope! */
- queue = vring_alloc_queue(vdev, vring_size(num, vring_align),
+ queue = vring_alloc_queue(vdev,
+ vring_legacy_size(num, vring_align),
_addr, GFP_KERNEL|__GFP_ZERO);
}
if (!queue)
return NULL;
- queue_size_in_bytes = vring_size(num, vring_align);
- vring_init(, num, queue, vring_align);
+ queue_size_in_bytes = vring_legacy_size(num, vring_align);
+ vring_legacy_init(, num, queue, vring_align);
vq = __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
@@ -2169,7 +2172,7 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
if (virtio_has_feature(vdev, VIRTIO_F_RING_PACKED))
return NULL;
- vring_init(, num, pages, vring_align);
+ vring_legacy_init(, num, pages, vring_align);
return __vring_new_virtqueue(index, vring, vdev, weak_barriers, context,
notify, callback, name);
}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 03/12] tools/virtio: define __KERNEL__
Even though we are building in userspace, we are emulating kernel bits.
Signed-off-by: Michael S. Tsirkin
---
tools/virtio/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtio/Makefile b/tools/virtio/Makefile
index b587b9a7a124..ea6674307507 100644
--- a/tools/virtio/Makefile
+++ b/tools/virtio/Makefile
@@ -4,7 +4,7 @@ test: virtio_test vringh_test
virtio_test: virtio_ring.o virtio_test.o
vringh_test: vringh_test.o vringh.o virtio_ring.o
-CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h
+CFLAGS += -g -O2 -Werror -Wall -I. -I../include/ -I ../../usr/include/
-Wno-pointer-sign -fno-strict-overflow -fno-strict-aliasing -fno-common -MMD
-U_FORTIFY_SOURCE -include ../../include/linux/kconfig.h -D__KERNEL__
vpath %.c ../../drivers/virtio ../../drivers/vhost
mod:
${MAKE} -C `pwd`/../.. M=`pwd`/vhost_test V=${V}
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 07/12] virtio: add legacy init/size APIs
vring_init/vring_size (in the uapi directory) are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only vring_legacy_init/vring_legacy_size.
Signed-off-by: Michael S. Tsirkin
---
include/linux/virtio_ring.h | 18 ++
1 file changed, 18 insertions(+)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index c3f9ca054250..766d70bca492 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -140,4 +140,22 @@ void vring_del_virtqueue(struct virtqueue *vq);
void vring_transport_features(struct virtio_device *vdev);
irqreturn_t vring_interrupt(int irq, void *_vq);
+
+static inline void vring_legacy_init(struct vring *vr, unsigned int num, void
*p,
+unsigned long align)
+{
+ vr->num = num;
+ vr->desc = p;
+ vr->avail = (struct vring_avail *)((char *)p + num * sizeof(struct
vring_desc));
+ vr->used = (void *)(((uintptr_t)>avail->ring[num] +
sizeof(__virtio16)
+ + align-1) & ~(align - 1));
+}
+
+static inline unsigned vring_legacy_size(unsigned int num, unsigned long align)
+{
+ return ((sizeof(struct vring_desc) * num + sizeof(__virtio16) * (3 +
num)
++ align - 1) & ~(align - 1))
+ + sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
+}
+
#endif /* _LINUX_VIRTIO_RING_H */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 01/12] tools/virtio: define aligned attribute
Signed-off-by: Michael S. Tsirkin --- tools/virtio/linux/compiler.h | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virtio/linux/compiler.h b/tools/virtio/linux/compiler.h index 903dc9c4bd11..2c51bccb97bb 100644 --- a/tools/virtio/linux/compiler.h +++ b/tools/virtio/linux/compiler.h @@ -7,4 +7,5 @@ #define READ_ONCE(var) (*((volatile typeof(var) *)(&(var +#define __aligned(x) __attribute((__aligned__(x))) #endif -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 05/12] virtio: stop using legacy struct vring in kernel
struct vring (in the uapi directory) and supporting APIs are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only struct vring and
switch everyone to use that.
Signed-off-by: Michael S. Tsirkin
---
include/linux/virtio_ring.h | 10 ++
include/linux/vringh.h | 1 +
include/uapi/linux/virtio_ring.h | 26 --
3 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..11680e74761a 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -60,6 +60,16 @@ static inline void virtio_store_mb(bool weak_barriers,
struct virtio_device;
struct virtqueue;
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
/*
* Creates a virtqueue and allocates the descriptor ring. If
* may_reduce_num is set, then this may allocate a smaller ring than
diff --git a/include/linux/vringh.h b/include/linux/vringh.h
index 9e2763d7c159..d71b3710f58e 100644
--- a/include/linux/vringh.h
+++ b/include/linux/vringh.h
@@ -11,6 +11,7 @@
#ifndef _LINUX_VRINGH_H
#define _LINUX_VRINGH_H
#include
+#include
#include
#include
#include
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 9223c3a5c46a..8961a4adda5c 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -166,6 +156,21 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+#ifndef __KERNEL__
+/*
+ * The following definitions have been put in the UAPI header by mistake. We
+ * keep them around to avoid breaking old userspace builds.
+ */
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
@@ -182,6 +187,7 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ align - 1) & ~(align - 1))
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif
#endif /* VIRTIO_RING_NO_LEGACY */
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v4 04/12] virtio: add VIRTIO_RING_NO_LEGACY
From: Matej Genci
Add macro to disable legacy vring functions.
Signed-off-by: Matej Genci
Link: https://lore.kernel.org/r/20190911124942.243713-1-matej.ge...@nutanix.com
Signed-off-by: Michael S. Tsirkin
---
drivers/virtio/virtio_pci_modern.c | 1 +
include/uapi/linux/virtio_ring.h | 4
2 files changed, 5 insertions(+)
diff --git a/drivers/virtio/virtio_pci_modern.c
b/drivers/virtio/virtio_pci_modern.c
index 7abcc50838b8..db93cedd262f 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -16,6 +16,7 @@
#include
#define VIRTIO_PCI_NO_LEGACY
+#define VIRTIO_RING_NO_LEGACY
#include "virtio_pci_common.h"
/*
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..9223c3a5c46a 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -135,6 +135,8 @@ struct vring {
#define VRING_USED_ALIGN_SIZE 4
#define VRING_DESC_ALIGN_SIZE 16
+#ifndef VIRTIO_RING_NO_LEGACY
+
/* The standard layout for the ring is a continuous chunk of memory which looks
* like this. We assume num is a power of 2.
*
@@ -181,6 +183,8 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif /* VIRTIO_RING_NO_LEGACY */
+
/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
/* Assuming a given event_idx value from the other side, if
* we have just incremented index from old to new_idx,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 1/2] virtio: stop using legacy struct vring in kernel
Hi "Michael,
I love your patch! Yet something to improve:
[auto build test ERROR on next-20200406]
[also build test ERROR on v5.6]
[cannot apply to vhost/linux-next linus/master linux/master v5.6 v5.6-rc7
v5.6-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Michael-S-Tsirkin/virtio-alignment-issues/20200407-025651
base:b2e2a818a01717ba15c74fd355f76822b81a95f6
config: nds32-defconfig (attached as .config)
compiler: nds32le-linux-gcc (GCC) 9.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=9.3.0 make.cross ARCH=nds32
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
In file included from include/linux/virtio.h:12,
from include/linux/virtio_config.h:7,
from include/uapi/linux/virtio_net.h:30,
from include/linux/virtio_net.h:6,
from net//packet/af_packet.c:82:
>> include/linux/vringh.h:42:15: error: field 'vring' has incomplete type
42 | struct vring vring;
| ^
vim +/vring +42 include/linux/vringh.h
f87d0fbb579818 Rusty Russell 2013-03-20 20
f87d0fbb579818 Rusty Russell 2013-03-20 21 /* virtio_ring with
information needed for host access. */
f87d0fbb579818 Rusty Russell 2013-03-20 22 struct vringh {
b9f7ac8c72894c Michael S. Tsirkin 2014-12-12 23/* Everything is little
endian */
b9f7ac8c72894c Michael S. Tsirkin 2014-12-12 24bool little_endian;
b9f7ac8c72894c Michael S. Tsirkin 2014-12-12 25
f87d0fbb579818 Rusty Russell 2013-03-20 26/* Guest publishes used
event idx (note: we always do). */
f87d0fbb579818 Rusty Russell 2013-03-20 27bool event_indices;
f87d0fbb579818 Rusty Russell 2013-03-20 28
f87d0fbb579818 Rusty Russell 2013-03-20 29/* Can we get away with
weak barriers? */
f87d0fbb579818 Rusty Russell 2013-03-20 30bool weak_barriers;
f87d0fbb579818 Rusty Russell 2013-03-20 31
f87d0fbb579818 Rusty Russell 2013-03-20 32/* Last available index
we saw (ie. where we're up to). */
f87d0fbb579818 Rusty Russell 2013-03-20 33u16 last_avail_idx;
f87d0fbb579818 Rusty Russell 2013-03-20 34
f87d0fbb579818 Rusty Russell 2013-03-20 35/* Last index we used.
*/
f87d0fbb579818 Rusty Russell 2013-03-20 36u16 last_used_idx;
f87d0fbb579818 Rusty Russell 2013-03-20 37
f87d0fbb579818 Rusty Russell 2013-03-20 38/* How many descriptors
we've completed since last need_notify(). */
f87d0fbb579818 Rusty Russell 2013-03-20 39u32 completed;
f87d0fbb579818 Rusty Russell 2013-03-20 40
f87d0fbb579818 Rusty Russell 2013-03-20 41/* The vring (note: it
may contain user pointers!) */
f87d0fbb579818 Rusty Russell 2013-03-20 @42struct vring vring;
3beee86a4b9374 Sjur Brændeland2013-03-20 43
9ad9c49cfe970b Jason Wang 2020-03-26 44/* IOTLB for this vring
*/
9ad9c49cfe970b Jason Wang 2020-03-26 45struct vhost_iotlb
*iotlb;
9ad9c49cfe970b Jason Wang 2020-03-26 46
3beee86a4b9374 Sjur Brændeland2013-03-20 47/* The function to call
to notify the guest about added buffers */
3beee86a4b9374 Sjur Brændeland2013-03-20 48void (*notify)(struct
vringh *);
3beee86a4b9374 Sjur Brændeland2013-03-20 49 };
3beee86a4b9374 Sjur Brændeland2013-03-20 50
:: The code at line 42 was first introduced by commit
:: f87d0fbb579818fed3eeb0923cc253163ab93039 vringh: host-side
implementation of virtio rings.
:: TO: Rusty Russell
:: CC: Rusty Russell
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 1/6] amdgpu: a NULL ->mm does not mean a thread is a kthread
Am 2020-04-04 um 5:40 a.m. schrieb Christoph Hellwig:
> Use the proper API instead.
>
> Fixes: 70539bd795002 ("drm/amd: Update MEC HQD loading code for KFD")
> Signed-off-by: Christoph Hellwig
Reviewed-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 13feb313e9b3..4db143c19dcc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -190,7 +190,7 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev
> *dst, struct kgd_dev *s
> pagefault_disable();\
> if ((mmptr) == current->mm) { \
> valid = !get_user((dst), (wptr)); \
> - } else if (current->mm == NULL) { \
> + } else if (current->flags & PF_KTHREAD) { \
> use_mm(mmptr); \
> valid = !get_user((dst), (wptr)); \
> unuse_mm(mmptr);\
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 40/44] drm/cirrus: Don't use drm_device->dev_private
Am 03.04.20 um 15:58 schrieb Daniel Vetter:
> Upcasting using a container_of macro is more typesafe, faster and
> easier for the compiler to optimize.
>
> Signed-off-by: Daniel Vetter
> Cc: Dave Airlie
> Cc: Gerd Hoffmann
> Cc: Daniel Vetter
> Cc: "Noralf Trønnes"
> Cc: Sam Ravnborg
> Cc: Eric Anholt
> Cc: Thomas Zimmermann
> Cc: virtualization@lists.linux-foundation.org
> ---
> drivers/gpu/drm/cirrus/cirrus.c | 9 +
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> index 4b65637147ba..744a8e337e41 100644
> --- a/drivers/gpu/drm/cirrus/cirrus.c
> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> @@ -59,6 +59,8 @@ struct cirrus_device {
> void __iomem *mmio;
> };
>
> +#define to_cirrus(_dev) container_of(_dev, struct cirrus_device, dev)
> +
Maybe to_cirrus_device() ? I had the same comment for vbox and I think
it applies to all patches.
Best regards
Thomas
> /* -- */
> /*
> * The meat of this driver. The core passes us a mode and we have to program
> @@ -311,7 +313,7 @@ static int cirrus_mode_set(struct cirrus_device *cirrus,
> static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
> struct drm_rect *rect)
> {
> - struct cirrus_device *cirrus = fb->dev->dev_private;
> + struct cirrus_device *cirrus = to_cirrus(fb->dev);
> void *vmap;
> int idx, ret;
>
> @@ -436,7 +438,7 @@ static void cirrus_pipe_enable(struct
> drm_simple_display_pipe *pipe,
> struct drm_crtc_state *crtc_state,
> struct drm_plane_state *plane_state)
> {
> - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
> + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
>
> cirrus_mode_set(cirrus, _state->mode, plane_state->fb);
> cirrus_fb_blit_fullscreen(plane_state->fb);
> @@ -445,7 +447,7 @@ static void cirrus_pipe_enable(struct
> drm_simple_display_pipe *pipe,
> static void cirrus_pipe_update(struct drm_simple_display_pipe *pipe,
> struct drm_plane_state *old_state)
> {
> - struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
> + struct cirrus_device *cirrus = to_cirrus(pipe->crtc.dev);
> struct drm_plane_state *state = pipe->plane.state;
> struct drm_crtc *crtc = >crtc;
> struct drm_rect rect;
> @@ -573,7 +575,6 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
> return PTR_ERR(cirrus);
>
> dev = >dev;
> - dev->dev_private = cirrus;
>
> cirrus->vram = devm_ioremap(>dev, pci_resource_start(pdev, 0),
> pci_resource_len(pdev, 0));
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 30/44] drm/qxl: Use devm_drm_dev_alloc
Am 03.04.20 um 15:58 schrieb Daniel Vetter:
> Also need to remove the drm_dev_put from the remove hook.
>
> Signed-off-by: Daniel Vetter
> Cc: Dave Airlie
> Cc: Gerd Hoffmann
> Cc: virtualization@lists.linux-foundation.org
> Cc: spice-de...@lists.freedesktop.org
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
> drivers/gpu/drm/qxl/qxl_drv.h | 3 +--
> drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
> 3 files changed, 10 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 09102e2efabc..6b4ae4c5fb76 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> return -EINVAL; /* TODO: ENODEV ? */
> }
>
> - qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> - if (!qdev)
> + qdev = devm_drm_dev_alloc(>dev, _driver,
> + struct qxl_device, ddev);
> + if (IS_ERR(qdev)) {
> + pr_err("Unable to init drm dev");
> return -ENOMEM;
> + }
My feeling is that it is too early to allocate. Wouldn't it be better to
first do the pdev and conflicting-fb stuff and allocate right before
qxl_device_init() ?
Best regards
Thomas
>
> ret = pci_enable_device(pdev);
> if (ret)
> - goto free_dev;
> + return ret;
>
> ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
> if (ret)
> @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> }
> }
>
> - ret = qxl_device_init(qdev, _driver, pdev);
> + ret = qxl_device_init(qdev, pdev);
> if (ret)
> goto put_vga;
>
> @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> disable_pci:
> pci_disable_device(pdev);
> -free_dev:
> - kfree(qdev);
> +
> return ret;
> }
>
> @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> drm_atomic_helper_shutdown(dev);
> if (is_vga(pdev))
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> - drm_dev_put(dev);
> }
>
> DEFINE_DRM_GEM_FOPS(qxl_fops);
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 435126facc9b..86ac191d9205 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -276,8 +276,7 @@ struct qxl_device {
> extern const struct drm_ioctl_desc qxl_ioctls[];
> extern int qxl_max_ioctl;
>
> -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> - struct pci_dev *pdev);
> +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
> void qxl_device_fini(struct qxl_device *qdev);
>
> int qxl_modeset_init(struct qxl_device *qdev);
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 9eed1a375f24..91a34dd835d7 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
> }
>
> int qxl_device_init(struct qxl_device *qdev,
> - struct drm_driver *drv,
> struct pci_dev *pdev)
> {
> int r, sb;
>
> - r = drm_dev_init(>ddev, drv, >dev);
> - if (r) {
> - pr_err("Unable to init drm dev");
> - goto error;
> - }
> -
> qdev->ddev.pdev = pdev;
> pci_set_drvdata(pdev, >ddev);
> qdev->ddev.dev_private = qdev;
> - drmm_add_final_kfree(>ddev, qdev);
>
> mutex_init(>gem.mutex);
> mutex_init(>update_area_mutex);
> @@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
> qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base,
> pci_resource_len(pdev, 0));
> if (!qdev->vram_mapping) {
> pr_err("Unable to create vram_mapping");
> - r = -ENOMEM;
> - goto error;
> + return -ENOMEM;
> }
>
> if (pci_resource_len(pdev, 4) > 0) {
> @@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
> io_mapping_free(qdev->surface_mapping);
> vram_mapping_free:
> io_mapping_free(qdev->vram_mapping);
> -error:
> return r;
> }
>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 5/6] kernel: better document the use_mm/unuse_mm API contract
Am 2020-04-04 um 5:41 a.m. schrieb Christoph Hellwig:
> Switch the function documentation to kerneldoc comments, and add
> WARN_ON_ONCE asserts that the calling thread is a kernel thread and
> does not have ->mm set (or has ->mm set in the case of unuse_mm).
>
> Also give the functions a kthread_ prefix to better document the
> use case.
>
> Signed-off-by: Christoph Hellwig
Acked-by: Felix Kuehling
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 4 +--
> drivers/gpu/drm/i915/gvt/kvmgt.c | 4 +--
> drivers/usb/gadget/function/f_fs.c | 4 +--
> drivers/usb/gadget/legacy/inode.c | 4 +--
> drivers/vhost/vhost.c | 4 +--
> fs/io-wq.c | 6 ++--
> fs/io_uring.c | 6 ++--
> include/linux/kthread.h| 4 +--
> kernel/kthread.c | 33 +++---
> mm/oom_kill.c | 6 ++--
> mm/vmacache.c | 4 +--
> 11 files changed, 39 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index bce5e93fefc8..63db84e09408 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -192,9 +192,9 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct kgd_dev
> *dst, struct kgd_dev *s
> if ((mmptr) == current->mm) { \
> valid = !get_user((dst), (wptr)); \
> } else if (current->flags & PF_KTHREAD) { \
> - use_mm(mmptr); \
> + kthread_use_mm(mmptr); \
> valid = !get_user((dst), (wptr)); \
> - unuse_mm(mmptr);\
> + kthread_unuse_mm(mmptr);\
> } \
> pagefault_enable(); \
> } \
> diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c
> b/drivers/gpu/drm/i915/gvt/kvmgt.c
> index dee01c371bf5..92e9b340dbc2 100644
> --- a/drivers/gpu/drm/i915/gvt/kvmgt.c
> +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
> @@ -2048,7 +2048,7 @@ static int kvmgt_rw_gpa(unsigned long handle, unsigned
> long gpa,
> if (kthread) {
> if (!mmget_not_zero(kvm->mm))
> return -EFAULT;
> - use_mm(kvm->mm);
> + kthread_use_mm(kvm->mm);
> }
>
> idx = srcu_read_lock(>srcu);
> @@ -2057,7 +2057,7 @@ static int kvmgt_rw_gpa(unsigned long handle, unsigned
> long gpa,
> srcu_read_unlock(>srcu, idx);
>
> if (kthread) {
> - unuse_mm(kvm->mm);
> + kthread_unuse_mm(kvm->mm);
> mmput(kvm->mm);
> }
>
> diff --git a/drivers/usb/gadget/function/f_fs.c
> b/drivers/usb/gadget/function/f_fs.c
> index c57b1b2507c6..d9e48bd7c692 100644
> --- a/drivers/usb/gadget/function/f_fs.c
> +++ b/drivers/usb/gadget/function/f_fs.c
> @@ -827,9 +827,9 @@ static void ffs_user_copy_worker(struct work_struct *work)
> mm_segment_t oldfs = get_fs();
>
> set_fs(USER_DS);
> - use_mm(io_data->mm);
> + kthread_use_mm(io_data->mm);
> ret = ffs_copy_to_iter(io_data->buf, ret, _data->data);
> - unuse_mm(io_data->mm);
> + kthread_unuse_mm(io_data->mm);
> set_fs(oldfs);
> }
>
> diff --git a/drivers/usb/gadget/legacy/inode.c
> b/drivers/usb/gadget/legacy/inode.c
> index 8b5233888bf8..a05552bc2ff8 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -462,9 +462,9 @@ static void ep_user_copy_worker(struct work_struct *work)
> struct kiocb *iocb = priv->iocb;
> size_t ret;
>
> - use_mm(mm);
> + kthread_use_mm(mm);
> ret = copy_to_iter(priv->buf, priv->actual, >to);
> - unuse_mm(mm);
> + kthread_unuse_mm(mm);
> if (!ret)
> ret = -EFAULT;
>
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 4e9ce54869af..1787d426a956 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -336,7 +336,7 @@ static int vhost_worker(void *data)
> mm_segment_t oldfs = get_fs();
>
> set_fs(USER_DS);
> - use_mm(dev->mm);
> + kthread_use_mm(dev->mm);
>
> for (;;) {
> /* mb paired w/ kthread_stop */
> @@ -364,7 +364,7 @@ static int vhost_worker(void *data)
> schedule();
> }
> }
> - unuse_mm(dev->mm);
> + kthread_unuse_mm(dev->mm);
> set_fs(oldfs);
> return 0;
> }
> diff --git
Re: [PATCH 4/6] kernel: move use_mm/unuse_mm to kthread.c
Am 2020-04-04 um 5:40 a.m. schrieb Christoph Hellwig: > These helpers are only for use with kernel threads, and I will tie them > more into the kthread infrastructure going forward. Also move the > prototypes to kthread.h - mmu_context.h was a little weird to start with > as it otherwise contains very low-level MM bits. > > Signed-off-by: Christoph Hellwig Acked-by: Felix Kuehling Thanks for cleaning up the unnecessary includes in amdgpu. Regards, Felix > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 1 + > .../drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c | 1 - > .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c| 1 - > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 2 - > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 2 - > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 2 - > drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +- > drivers/usb/gadget/function/f_fs.c| 2 +- > drivers/usb/gadget/legacy/inode.c | 2 +- > drivers/vhost/vhost.c | 1 - > fs/aio.c | 1 - > fs/io-wq.c| 1 - > fs/io_uring.c | 1 - > include/linux/kthread.h | 5 ++ > include/linux/mmu_context.h | 5 -- > kernel/kthread.c | 56 > mm/Makefile | 2 +- > mm/mmu_context.c | 64 --- > 18 files changed, 66 insertions(+), 85 deletions(-) > delete mode 100644 mm/mmu_context.c > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > index 4db143c19dcc..bce5e93fefc8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > @@ -27,6 +27,7 @@ > > #include > #include > +#include > #include > #include > #include > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c > index 6529caca88fe..35d4a5ab0228 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_arcturus.c > @@ -22,7 +22,6 @@ > #include > #include > #include > -#include > #include > #include "amdgpu.h" > #include "amdgpu_amdkfd.h" > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > index 4ec6d0c03201..b1655054b919 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > @@ -19,7 +19,6 @@ > * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > * OTHER DEALINGS IN THE SOFTWARE. > */ > -#include > #include "amdgpu.h" > #include "amdgpu_amdkfd.h" > #include "gc/gc_10_1_0_offset.h" > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c > index 0b7e78748540..7d01420c0c85 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c > @@ -20,8 +20,6 @@ > * OTHER DEALINGS IN THE SOFTWARE. > */ > > -#include > - > #include "amdgpu.h" > #include "amdgpu_amdkfd.h" > #include "cikd.h" > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c > index ccd635b812b5..635cd1a26bed 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c > @@ -20,8 +20,6 @@ > * OTHER DEALINGS IN THE SOFTWARE. > */ > > -#include > - > #include "amdgpu.h" > #include "amdgpu_amdkfd.h" > #include "gfx_v8_0.h" > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c > index df841c2ac5e7..c7fd0c47b254 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c > @@ -19,8 +19,6 @@ > * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > * OTHER DEALINGS IN THE SOFTWARE. > */ > -#include > - > #include "amdgpu.h" > #include "amdgpu_amdkfd.h" > #include "gc/gc_9_0_offset.h" > diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c > b/drivers/gpu/drm/i915/gvt/kvmgt.c > index 5848400620b4..dee01c371bf5 100644 > --- a/drivers/gpu/drm/i915/gvt/kvmgt.c > +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c > @@ -31,7 +31,7 @@ > #include > #include > #include > -#include > +#include > #include > #include > #include > diff --git a/drivers/usb/gadget/function/f_fs.c > b/drivers/usb/gadget/function/f_fs.c > index c81023b195c3..c57b1b2507c6 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -32,7 +32,7 @@ > #include > > #include > -#include > +#include > #include > #include > > diff --git
[PATCH v3 1/2] virtio: stop using legacy struct vring in kernel
struct vring (in the uapi directory) and supporting APIs are kept
around to solely avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add an internal kernel-only struct vring, add supporting legacy APIs and
switch everyone to use that.
Signed-off-by: Michael S. Tsirkin
---
include/linux/virtio_ring.h | 28 +
include/uapi/linux/virtio_ring.h| 26 ++-
tools/virtio/ringtest/virtio_ring_0_9.c | 6 +++---
tools/virtio/vringh_test.c | 20 +-
4 files changed, 57 insertions(+), 23 deletions(-)
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 3dc70adfe5f5..b6a31b3cf87c 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -112,4 +112,32 @@ void vring_del_virtqueue(struct virtqueue *vq);
void vring_transport_features(struct virtio_device *vdev);
irqreturn_t vring_interrupt(int irq, void *_vq);
+
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
+static inline void vring_legacy_init(struct vring *vr, unsigned int num, void
*p,
+unsigned long align)
+{
+ vr->num = num;
+ vr->desc = p;
+ vr->avail = (struct vring_avail *)((char *)p + num * sizeof(struct
vring_desc));
+ vr->used = (void *)(((uintptr_t)>avail->ring[num] +
sizeof(__virtio16)
+ + align-1) & ~(align - 1));
+}
+
+static inline unsigned vring_legacy_size(unsigned int num, unsigned long align)
+{
+ return ((sizeof(struct vring_desc) * num + sizeof(__virtio16) * (3 +
num)
++ align - 1) & ~(align - 1))
+ + sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
+}
+
#endif /* _LINUX_VIRTIO_RING_H */
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..59939ba30b06 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -164,6 +154,21 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+#ifndef __KERNEL__
+/*
+ * The following definitions have been put in the UAPI header by mistake. We
+ * keep them around to avoid breaking old userspace builds.
+ */
+struct vring {
+ unsigned int num;
+
+ struct vring_desc *desc;
+
+ struct vring_avail *avail;
+
+ struct vring_used *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
@@ -180,6 +185,7 @@ static inline unsigned vring_size(unsigned int num,
unsigned long align)
+ align - 1) & ~(align - 1))
+ sizeof(__virtio16) * 3 + sizeof(struct vring_used_elem) * num;
}
+#endif
/* The following is used with USED_EVENT_IDX and AVAIL_EVENT_IDX */
/* Assuming a given event_idx value from the other side, if
diff --git a/tools/virtio/ringtest/virtio_ring_0_9.c
b/tools/virtio/ringtest/virtio_ring_0_9.c
index 13a035a390e9..e2ab6ac53966 100644
--- a/tools/virtio/ringtest/virtio_ring_0_9.c
+++ b/tools/virtio/ringtest/virtio_ring_0_9.c
@@ -67,13 +67,13 @@ void alloc_ring(void)
int i;
void *p;
- ret = posix_memalign(, 0x1000, vring_size(ring_size, 0x1000));
+ ret = posix_memalign(, 0x1000, vring_legacy_size(ring_size, 0x1000));
if (ret) {
perror("Unable to allocate ring buffer.\n");
exit(3);
}
- memset(p, 0, vring_size(ring_size, 0x1000));
- vring_init(, ring_size, p, 0x1000);
+ memset(p, 0, vring_legacy_size(ring_size, 0x1000));
+ vring_legacy_init(, ring_size, p, 0x1000);
guest.avail_idx = 0;
guest.kicked_avail_idx = -1;
diff --git a/tools/virtio/vringh_test.c b/tools/virtio/vringh_test.c
index 293653463303..d26dc6530bd4 100644
--- a/tools/virtio/vringh_test.c
+++ b/tools/virtio/vringh_test.c
@@ -151,7 +151,7 @@ static int parallel_test(u64 features,
err(1, "Opening /tmp/vringh_test-file");
/* Extra room at the end for some data, and indirects */
- mapsize = vring_size(RINGSIZE, ALIGN)
+ mapsize = vring_legacy_size(RINGSIZE, ALIGN)
+ RINGSIZE * 2 * sizeof(int)
+ RINGSIZE * 6 *
[PATCH v3 2/2] vhost: force spec specified alignment on types
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.h | 6 +++---
include/linux/virtio_ring.h | 24 +---
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 181382185bbc..3ceaafecc1fb 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -67,9 +67,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct file *kick;
struct eventfd_ctx *call_ctx;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index b6a31b3cf87c..dfb58eff7a7f 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -113,14 +113,32 @@ void vring_transport_features(struct virtio_device *vdev);
irqreturn_t vring_interrupt(int irq, void *_vq);
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __aligned(VRING_DESC_ALIGN_SIZE) vring_desc_t;
+typedef struct vring_avail __aligned(VRING_AVAIL_ALIGN_SIZE) vring_avail_t;
+typedef struct vring_used __aligned(VRING_USED_ALIGN_SIZE) vring_used_t;
+
struct vring {
unsigned int num;
- struct vring_desc *desc;
+ vring_desc_t *desc;
- struct vring_avail *avail;
+ vring_avail_t *avail;
- struct vring_used *used;
+ vring_used_t *used;
};
static inline void vring_legacy_init(struct vring *vr, unsigned int num, void
*p,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/2] virtio: stop using legacy struct vring
On Mon, Apr 06, 2020 at 05:56:02PM +0200, Greg Kroah-Hartman wrote: > On Mon, Apr 06, 2020 at 11:35:23AM -0400, Michael S. Tsirkin wrote: > > struct vring (in the uapi directory) and supporting APIs are kept > > around to avoid breaking old userspace builds. > > It's not actually part of the UAPI - it was kept in the UAPI > > header by mistake, and using it in kernel isn't necessary > > and prevents us from making changes safely. > > In particular, the APIs actually assume the legacy layout. > > > > Add struct vring_s (identical ATM) and supporting > > legacy APIs and switch everyone to use that. > > How are we going to know that "struct vring_s" is what we need/want to > use? What does "_s" mean? > > "struct vring_kernel"? > > naming is hard... > > greg k-h Hmm. I guess I can just add an ifdef so kernel doesn't see the UAPI version anymore ... -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 1/2] virtio: stop using legacy struct vring
On Mon, Apr 06, 2020 at 11:35:23AM -0400, Michael S. Tsirkin wrote: > struct vring (in the uapi directory) and supporting APIs are kept > around to avoid breaking old userspace builds. > It's not actually part of the UAPI - it was kept in the UAPI > header by mistake, and using it in kernel isn't necessary > and prevents us from making changes safely. > In particular, the APIs actually assume the legacy layout. > > Add struct vring_s (identical ATM) and supporting > legacy APIs and switch everyone to use that. How are we going to know that "struct vring_s" is what we need/want to use? What does "_s" mean? "struct vring_kernel"? naming is hard... greg k-h ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa-sim: depend on HAS_DMA
On Mon, Apr 06, 2020 at 10:12:56PM +0800, Jason Wang wrote: > > On 2020/4/6 下午10:02, Michael S. Tsirkin wrote: > > On Mon, Apr 06, 2020 at 06:09:27AM -0700, Christoph Hellwig wrote: > > > Pleae just drop the code - we should not add new drivers with custom > > > DMA ops. > > I'm not sure there's another way to do this: this not a driver, it's a > > software emulator that pretends to be an actual device. We can't > > have the platform supply DMA ops here since the platform is a regular > > x86 or whatever. > > > Yes, actually vdpa-sim does not depends on HAS_DMA. Maybe we can do shortcut > like dev->dma_ops = vdpasim_dma_ops; > > Thanks > We can do that - does it them actually work - and not just compile? -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 2/2] vhost: force spec specified alignment on types
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
drivers/vhost/vhost.h | 6 +++---
include/linux/virtio_ring.h | 24 +---
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index cc82918158d2..a67bda9792ec 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -74,9 +74,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct vhost_desc *descs;
diff --git a/include/linux/virtio_ring.h b/include/linux/virtio_ring.h
index 09fc6164b523..efcb9fde1dbf 100644
--- a/include/linux/virtio_ring.h
+++ b/include/linux/virtio_ring.h
@@ -113,14 +113,32 @@ void vring_transport_features(struct virtio_device *vdev);
irqreturn_t vring_interrupt(int irq, void *_vq);
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __aligned(VRING_DESC_ALIGN_SIZE) vring_desc_t;
+typedef struct vring_avail __aligned(VRING_AVAIL_ALIGN_SIZE) vring_avail_t;
+typedef struct vring_used __aligned(VRING_USED_ALIGN_SIZE) vring_used_t;
+
struct vring_s {
unsigned int num;
- struct vring_desc *desc;
+ vring_desc_t *desc;
- struct vring_avail *avail;
+ vring_avail_t *avail;
- struct vring_used *used;
+ vring_used_t *used;
};
static inline void vring_legacy_init(struct vring_s *vr, unsigned int num,
void *p,
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 1/2] virtio: stop using legacy struct vring
struct vring (in the uapi directory) and supporting APIs are kept
around to avoid breaking old userspace builds.
It's not actually part of the UAPI - it was kept in the UAPI
header by mistake, and using it in kernel isn't necessary
and prevents us from making changes safely.
In particular, the APIs actually assume the legacy layout.
Add struct vring_s (identical ATM) and supporting
legacy APIs and switch everyone to use that.
Signed-off-by: Michael S. Tsirkin
---
drivers/misc/mic/vop/vop_main.c | 2 +-
drivers/platform/mellanox/mlxbf-tmfifo.c | 10 -
drivers/virtio/virtio_ring.c | 10 -
include/linux/virtio.h | 2 +-
include/linux/virtio_ring.h | 28
include/linux/vringh.h | 2 +-
tools/virtio/ringtest/virtio_ring_0_9.c | 6 ++---
tools/virtio/virtio_test.c | 2 +-
tools/virtio/vringh_test.c | 20 -
9 files changed, 55 insertions(+), 27 deletions(-)
diff --git a/drivers/misc/mic/vop/vop_main.c b/drivers/misc/mic/vop/vop_main.c
index 85942f6717c5..817abf0b5cd1 100644
--- a/drivers/misc/mic/vop/vop_main.c
+++ b/drivers/misc/mic/vop/vop_main.c
@@ -281,7 +281,7 @@ static struct virtqueue *vop_new_virtqueue(unsigned int
index,
void *used)
{
bool weak_barriers = false;
- struct vring vring;
+ struct vring_s vring;
vring_init(, num, pages, MIC_VIRTIO_RING_ALIGN);
vring.used = used;
diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c
b/drivers/platform/mellanox/mlxbf-tmfifo.c
index 5739a9669b29..eddbb1a0c742 100644
--- a/drivers/platform/mellanox/mlxbf-tmfifo.c
+++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
@@ -287,7 +287,7 @@ static irqreturn_t mlxbf_tmfifo_irq_handler(int irq, void
*arg)
static struct vring_desc *
mlxbf_tmfifo_get_next_desc(struct mlxbf_tmfifo_vring *vring)
{
- const struct vring *vr = virtqueue_get_vring(vring->vq);
+ const struct vring_s *vr = virtqueue_get_vring(vring->vq);
struct virtio_device *vdev = vring->vq->vdev;
unsigned int idx, head;
@@ -308,7 +308,7 @@ mlxbf_tmfifo_get_next_desc(struct mlxbf_tmfifo_vring *vring)
static void mlxbf_tmfifo_release_desc(struct mlxbf_tmfifo_vring *vring,
struct vring_desc *desc, u32 len)
{
- const struct vring *vr = virtqueue_get_vring(vring->vq);
+ const struct vring_s *vr = virtqueue_get_vring(vring->vq);
struct virtio_device *vdev = vring->vq->vdev;
u16 idx, vr_idx;
@@ -330,7 +330,7 @@ static void mlxbf_tmfifo_release_desc(struct
mlxbf_tmfifo_vring *vring,
static u32 mlxbf_tmfifo_get_pkt_len(struct mlxbf_tmfifo_vring *vring,
struct vring_desc *desc)
{
- const struct vring *vr = virtqueue_get_vring(vring->vq);
+ const struct vring_s *vr = virtqueue_get_vring(vring->vq);
struct virtio_device *vdev = vring->vq->vdev;
u32 len = 0, idx;
@@ -412,7 +412,7 @@ static void mlxbf_tmfifo_console_output_one(struct
mlxbf_tmfifo_vdev *cons,
struct mlxbf_tmfifo_vring *vring,
struct vring_desc *desc)
{
- const struct vring *vr = virtqueue_get_vring(vring->vq);
+ const struct vring_s *vr = virtqueue_get_vring(vring->vq);
struct virtio_device *vdev = >vdev;
u32 len, idx, seg;
void *addr;
@@ -671,7 +671,7 @@ static void mlxbf_tmfifo_rxtx_header(struct
mlxbf_tmfifo_vring *vring,
static bool mlxbf_tmfifo_rxtx_one_desc(struct mlxbf_tmfifo_vring *vring,
bool is_rx, int *avail)
{
- const struct vring *vr = virtqueue_get_vring(vring->vq);
+ const struct vring_s *vr = virtqueue_get_vring(vring->vq);
struct mlxbf_tmfifo *fifo = vring->fifo;
struct virtio_device *vdev;
bool vring_change = false;
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 58b96baa8d48..d62d926eb9a4 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -117,7 +117,7 @@ struct vring_virtqueue {
/* Available for split ring */
struct {
/* Actual memory layout for this queue. */
- struct vring vring;
+ struct vring_s vring;
/* Last written value to avail->flags */
u16 avail_flags_shadow;
@@ -858,7 +858,7 @@ static struct virtqueue *vring_create_virtqueue_split(
void *queue = NULL;
dma_addr_t dma_addr;
size_t queue_size_in_bytes;
- struct vring vring;
+ struct vring_s vring;
/* We assume num is a power of 2. */
if (num & (num - 1)) {
@@ -2051,7 +2051,7 @@ EXPORT_SYMBOL_GPL(vring_interrupt);
/* Only available for split ring */
struct virtqueue
[PATCH 2/2] vdpa: Fix pointer math bug in vdpasim_get_config()
If "offset" is non-zero then we end up copying from beyond the end of
the config because of pointer math. We can fix this by casting the
struct to a u8 pointer.
Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Signed-off-by: Dan Carpenter
---
Is it really worth letting people specify the offset?
drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index b3c800653056..e03c25765513 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -509,7 +509,7 @@ static void vdpasim_get_config(struct vdpa_device *vdpa,
unsigned int offset,
struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
if (offset + len < sizeof(struct virtio_net_config))
- memcpy(buf, >config + offset, len);
+ memcpy(buf, (u8 *)>config + offset, len);
}
static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH 1/2] vdpa: Signedness bugs in vdpasim_work()
The "read" and "write" variables need to be signed for the error
handling to work.
Fixes: 2c53d0f64c06 ("vdpasim: vDPA device simulator")
Signed-off-by: Dan Carpenter
---
drivers/vdpa/vdpa_sim/vdpa_sim.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 6e8a0cf2fdeb..b3c800653056 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -132,7 +132,8 @@ static void vdpasim_work(struct work_struct *work)
vdpasim, work);
struct vdpasim_virtqueue *txq = >vqs[1];
struct vdpasim_virtqueue *rxq = >vqs[0];
- size_t read, write, total_write;
+ ssize_t read, write;
+ size_t total_write;
int err;
int pkts = 0;
--
2.25.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost: force spec specified alignment on types
On Mon, Apr 06, 2020 at 10:09:27PM +0800, Jason Wang wrote:
>
> On 2020/4/6 下午9:55, Michael S. Tsirkin wrote:
> > On Mon, Apr 06, 2020 at 09:34:00PM +0800, Jason Wang wrote:
> > > On 2020/4/6 下午8:50, Michael S. Tsirkin wrote:
> > > > The ring element addresses are passed between components with different
> > > > alignments assumptions. Thus, if guest/userspace selects a pointer and
> > > > host then gets and dereferences it, we might need to decrease the
> > > > compiler-selected alignment to prevent compiler on the host from
> > > > assuming pointer is aligned.
> > > >
> > > > This actually triggers on ARM with -mabi=apcs-gnu - which is a
> > > > deprecated configuration, but it seems safer to handle this
> > > > generally.
> > > >
> > > > I verified that the produced binary is exactly identical on x86.
> > > >
> > > > Signed-off-by: Michael S. Tsirkin
> > > > ---
> > > >
> > > > This is my preferred way to handle the ARM incompatibility issues
> > > > (in preference to kconfig hacks).
> > > > I will push this into next now.
> > > > Comments?
> > >
> > > I'm not sure if it's too late to fix. It would still be still problematic
> > > for the userspace that is using old uapi headers?
> > >
> > > Thanks
> > It's not a problem in userspace. The problem is when
> > userspace/guest uses 2 byte alignment and passes it to kernel
> > assuming 8 byte alignment. The fix is for host not to
> > make these assumptions.
>
>
> Yes, but I meant when userspace is complied with apcs-gnu, then it still
> assumes 8 byte alignment?
>
> Thanks
That's not a problem since with vhost userspace is doing the allocation.
So it can increase alignment with no bad effect.
I agree it's probably safest not to touch struct vring at all though.
>
> >
> > > >drivers/vhost/vhost.h| 6 ++---
> > > >include/uapi/linux/virtio_ring.h | 41
> > > >
> > > >2 files changed, 34 insertions(+), 13 deletions(-)
> > > >
> > > > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > > > index cc82918158d2..a67bda9792ec 100644
> > > > --- a/drivers/vhost/vhost.h
> > > > +++ b/drivers/vhost/vhost.h
> > > > @@ -74,9 +74,9 @@ struct vhost_virtqueue {
> > > > /* The actual ring of buffers. */
> > > > struct mutex mutex;
> > > > unsigned int num;
> > > > - struct vring_desc __user *desc;
> > > > - struct vring_avail __user *avail;
> > > > - struct vring_used __user *used;
> > > > + vring_desc_t __user *desc;
> > > > + vring_avail_t __user *avail;
> > > > + vring_used_t __user *used;
> > > > const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
> > > > struct vhost_desc *descs;
> > > > diff --git a/include/uapi/linux/virtio_ring.h
> > > > b/include/uapi/linux/virtio_ring.h
> > > > index 559f42e73315..cd6e0b2eaf2f 100644
> > > > --- a/include/uapi/linux/virtio_ring.h
> > > > +++ b/include/uapi/linux/virtio_ring.h
> > > > @@ -118,16 +118,6 @@ struct vring_used {
> > > > struct vring_used_elem ring[];
> > > >};
> > > > -struct vring {
> > > > - unsigned int num;
> > > > -
> > > > - struct vring_desc *desc;
> > > > -
> > > > - struct vring_avail *avail;
> > > > -
> > > > - struct vring_used *used;
> > > > -};
> > > > -
> > > >/* Alignment requirements for vring elements.
> > > > * When using pre-virtio 1.0 layout, these fall out naturally.
> > > > */
> > > > @@ -164,6 +154,37 @@ struct vring {
> > > >#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
> > > >#define vring_avail_event(vr) (*(__virtio16
> > > > *)&(vr)->used->ring[(vr)->num])
> > > > +/*
> > > > + * The ring element addresses are passed between components with
> > > > different
> > > > + * alignments assumptions. Thus, we might need to decrease the
> > > > compiler-selected
> > > > + * alignment, and so must use a typedef to make sure the __aligned
> > > > attribute
> > > > + * actually takes hold:
> > > > + *
> > > > + *
> > > > https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
> > > > + *
> > > > + * When used on a struct, or struct member, the aligned attribute can
> > > > only
> > > > + * increase the alignment; in order to decrease it, the packed
> > > > attribute must
> > > > + * be specified as well. When used as part of a typedef, the aligned
> > > > attribute
> > > > + * can both increase and decrease alignment, and specifying the packed
> > > > + * attribute generates a warning.
> > > > + */
> > > > +typedef struct vring_desc
> > > > __attribute__((aligned(VRING_DESC_ALIGN_SIZE)))
> > > > + vring_desc_t;
> > > > +typedef struct vring_avail
> > > > __attribute__((aligned(VRING_AVAIL_ALIGN_SIZE)))
> > > > + vring_avail_t;
> > > > +typedef struct vring_used
> > > > __attribute__((aligned(VRING_USED_ALIGN_SIZE)))
> > > > + vring_used_t;
> > > > +
> > > > +struct vring {
> > > > + unsigned int num;
Re: [PATCH] vdpa-sim: depend on HAS_DMA
On 2020/4/6 下午10:02, Michael S. Tsirkin wrote: On Mon, Apr 06, 2020 at 06:09:27AM -0700, Christoph Hellwig wrote: Pleae just drop the code - we should not add new drivers with custom DMA ops. I'm not sure there's another way to do this: this not a driver, it's a software emulator that pretends to be an actual device. We can't have the platform supply DMA ops here since the platform is a regular x86 or whatever. Yes, actually vdpa-sim does not depends on HAS_DMA. Maybe we can do shortcut like dev->dma_ops = vdpasim_dma_ops; Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost: force spec specified alignment on types
On 2020/4/6 下午9:55, Michael S. Tsirkin wrote:
On Mon, Apr 06, 2020 at 09:34:00PM +0800, Jason Wang wrote:
On 2020/4/6 下午8:50, Michael S. Tsirkin wrote:
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
This is my preferred way to handle the ARM incompatibility issues
(in preference to kconfig hacks).
I will push this into next now.
Comments?
I'm not sure if it's too late to fix. It would still be still problematic
for the userspace that is using old uapi headers?
Thanks
It's not a problem in userspace. The problem is when
userspace/guest uses 2 byte alignment and passes it to kernel
assuming 8 byte alignment. The fix is for host not to
make these assumptions.
Yes, but I meant when userspace is complied with apcs-gnu, then it still
assumes 8 byte alignment?
Thanks
drivers/vhost/vhost.h| 6 ++---
include/uapi/linux/virtio_ring.h | 41
2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index cc82918158d2..a67bda9792ec 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -74,9 +74,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct vhost_desc *descs;
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..cd6e0b2eaf2f 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -164,6 +154,37 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __attribute__((aligned(VRING_DESC_ALIGN_SIZE)))
+ vring_desc_t;
+typedef struct vring_avail __attribute__((aligned(VRING_AVAIL_ALIGN_SIZE)))
+ vring_avail_t;
+typedef struct vring_used __attribute__((aligned(VRING_USED_ALIGN_SIZE)))
+ vring_used_t;
+
+struct vring {
+ unsigned int num;
+
+ vring_desc_t *desc;
+
+ vring_avail_t *avail;
+
+ vring_used_t *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa-sim: depend on HAS_DMA
On Mon, Apr 06, 2020 at 06:09:27AM -0700, Christoph Hellwig wrote: > Pleae just drop the code - we should not add new drivers with custom > DMA ops. I'm not sure there's another way to do this: this not a driver, it's a software emulator that pretends to be an actual device. We can't have the platform supply DMA ops here since the platform is a regular x86 or whatever. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost: force spec specified alignment on types
On Mon, Apr 06, 2020 at 09:34:00PM +0800, Jason Wang wrote:
>
> On 2020/4/6 下午8:50, Michael S. Tsirkin wrote:
> > The ring element addresses are passed between components with different
> > alignments assumptions. Thus, if guest/userspace selects a pointer and
> > host then gets and dereferences it, we might need to decrease the
> > compiler-selected alignment to prevent compiler on the host from
> > assuming pointer is aligned.
> >
> > This actually triggers on ARM with -mabi=apcs-gnu - which is a
> > deprecated configuration, but it seems safer to handle this
> > generally.
> >
> > I verified that the produced binary is exactly identical on x86.
> >
> > Signed-off-by: Michael S. Tsirkin
> > ---
> >
> > This is my preferred way to handle the ARM incompatibility issues
> > (in preference to kconfig hacks).
> > I will push this into next now.
> > Comments?
>
>
> I'm not sure if it's too late to fix. It would still be still problematic
> for the userspace that is using old uapi headers?
>
> Thanks
It's not a problem in userspace. The problem is when
userspace/guest uses 2 byte alignment and passes it to kernel
assuming 8 byte alignment. The fix is for host not to
make these assumptions.
>
> >
> > drivers/vhost/vhost.h| 6 ++---
> > include/uapi/linux/virtio_ring.h | 41
> > 2 files changed, 34 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> > index cc82918158d2..a67bda9792ec 100644
> > --- a/drivers/vhost/vhost.h
> > +++ b/drivers/vhost/vhost.h
> > @@ -74,9 +74,9 @@ struct vhost_virtqueue {
> > /* The actual ring of buffers. */
> > struct mutex mutex;
> > unsigned int num;
> > - struct vring_desc __user *desc;
> > - struct vring_avail __user *avail;
> > - struct vring_used __user *used;
> > + vring_desc_t __user *desc;
> > + vring_avail_t __user *avail;
> > + vring_used_t __user *used;
> > const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
> > struct vhost_desc *descs;
> > diff --git a/include/uapi/linux/virtio_ring.h
> > b/include/uapi/linux/virtio_ring.h
> > index 559f42e73315..cd6e0b2eaf2f 100644
> > --- a/include/uapi/linux/virtio_ring.h
> > +++ b/include/uapi/linux/virtio_ring.h
> > @@ -118,16 +118,6 @@ struct vring_used {
> > struct vring_used_elem ring[];
> > };
> > -struct vring {
> > - unsigned int num;
> > -
> > - struct vring_desc *desc;
> > -
> > - struct vring_avail *avail;
> > -
> > - struct vring_used *used;
> > -};
> > -
> > /* Alignment requirements for vring elements.
> >* When using pre-virtio 1.0 layout, these fall out naturally.
> >*/
> > @@ -164,6 +154,37 @@ struct vring {
> > #define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
> > #define vring_avail_event(vr) (*(__virtio16
> > *)&(vr)->used->ring[(vr)->num])
> > +/*
> > + * The ring element addresses are passed between components with different
> > + * alignments assumptions. Thus, we might need to decrease the
> > compiler-selected
> > + * alignment, and so must use a typedef to make sure the __aligned
> > attribute
> > + * actually takes hold:
> > + *
> > + *
> > https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
> > + *
> > + * When used on a struct, or struct member, the aligned attribute can only
> > + * increase the alignment; in order to decrease it, the packed attribute
> > must
> > + * be specified as well. When used as part of a typedef, the aligned
> > attribute
> > + * can both increase and decrease alignment, and specifying the packed
> > + * attribute generates a warning.
> > + */
> > +typedef struct vring_desc __attribute__((aligned(VRING_DESC_ALIGN_SIZE)))
> > + vring_desc_t;
> > +typedef struct vring_avail __attribute__((aligned(VRING_AVAIL_ALIGN_SIZE)))
> > + vring_avail_t;
> > +typedef struct vring_used __attribute__((aligned(VRING_USED_ALIGN_SIZE)))
> > + vring_used_t;
> > +
> > +struct vring {
> > + unsigned int num;
> > +
> > + vring_desc_t *desc;
> > +
> > + vring_avail_t *avail;
> > +
> > + vring_used_t *used;
> > +};
> > +
> > static inline void vring_init(struct vring *vr, unsigned int num, void *p,
> > unsigned long align)
> > {
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost: force spec specified alignment on types
On 2020/4/6 下午8:50, Michael S. Tsirkin wrote:
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
This is my preferred way to handle the ARM incompatibility issues
(in preference to kconfig hacks).
I will push this into next now.
Comments?
I'm not sure if it's too late to fix. It would still be still
problematic for the userspace that is using old uapi headers?
Thanks
drivers/vhost/vhost.h| 6 ++---
include/uapi/linux/virtio_ring.h | 41
2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index cc82918158d2..a67bda9792ec 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -74,9 +74,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct vhost_desc *descs;
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..cd6e0b2eaf2f 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -164,6 +154,37 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __attribute__((aligned(VRING_DESC_ALIGN_SIZE)))
+ vring_desc_t;
+typedef struct vring_avail __attribute__((aligned(VRING_AVAIL_ALIGN_SIZE)))
+ vring_avail_t;
+typedef struct vring_used __attribute__((aligned(VRING_USED_ALIGN_SIZE)))
+ vring_used_t;
+
+struct vring {
+ unsigned int num;
+
+ vring_desc_t *desc;
+
+ vring_avail_t *avail;
+
+ vring_used_t *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/2] vhost: disable for OABI
On Mon, Apr 6, 2020 at 3:02 PM Michael S. Tsirkin wrote: > > On Mon, Apr 06, 2020 at 02:50:32PM +0200, Arnd Bergmann wrote: > > On Mon, Apr 6, 2020 at 2:12 PM Michael S. Tsirkin wrote: > > > > > > > > +config VHOST_DPN > > > + bool "VHOST dependencies" > > > + depends on !ARM || AEABI > > > + default y > > > + help > > > + Anything selecting VHOST or VHOST_RING must depend on VHOST_DPN. > > > + This excludes the deprecated ARM ABI since that forces a 4 byte > > > + alignment on all structs - incompatible with virtio spec > > > requirements. > > > + > > > > This should not be a user-visible option, so just make this 'def_bool > > !ARM || AEABI' > > > > I like keeping some kind of hint around for when one tries to understand > why is a specific symbol visible. I meant you should remove the "VHOST dependencies" prompt, not the help text, which is certainly useful here. You can also use the three lines bool depends on !ARM || AEABI default y in front of the help text, but those are equivalent to the one-line version I suggested. Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/2] vhost: disable for OABI
On Mon, Apr 06, 2020 at 03:15:20PM +0200, Arnd Bergmann wrote: > On Mon, Apr 6, 2020 at 3:02 PM Michael S. Tsirkin wrote: > > > > On Mon, Apr 06, 2020 at 02:50:32PM +0200, Arnd Bergmann wrote: > > > On Mon, Apr 6, 2020 at 2:12 PM Michael S. Tsirkin wrote: > > > > > > > > > > > +config VHOST_DPN > > > > + bool "VHOST dependencies" > > > > + depends on !ARM || AEABI > > > > + default y > > > > + help > > > > + Anything selecting VHOST or VHOST_RING must depend on > > > > VHOST_DPN. > > > > + This excludes the deprecated ARM ABI since that forces a 4 > > > > byte > > > > + alignment on all structs - incompatible with virtio spec > > > > requirements. > > > > + > > > > > > This should not be a user-visible option, so just make this 'def_bool > > > !ARM || AEABI' > > > > > > > I like keeping some kind of hint around for when one tries to understand > > why is a specific symbol visible. > > I meant you should remove the "VHOST dependencies" prompt, not the > help text, which is certainly useful here. You can also use the three lines > > bool > depends on !ARM || AEABI > default y > > in front of the help text, but those are equivalent to the one-line version > I suggested. > > Arnd Oh right. Good point. Thanks! -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: upstream boot error: KASAN: slab-out-of-bounds Write in virtio_gpu_object_create
Hi,
> > > +drivers/gpu/drm/virtio/virtgpu_object.c maintainers
> > > Now we have both mainline and linux-next boot broken (linux-next is
> > > broken for the past 40 days).
> > > No testing of new code happens.
> > >
> > > > virtio_gpu_object_shmem_init
> > > > drivers/gpu/drm/virtio/virtgpu_object.c:151 [inline]
> > > > virtio_gpu_object_create+0x9f3/0xaa0
> > > > drivers/gpu/drm/virtio/virtgpu_object.c:230
> >
> > Ah, that one.
> >
> > broken patch: f651c8b05542 ("drm/virtio: factor out the sg_table from
> > virtio_gpu_object")
> > fixed by: 0666a8d7f6a4 ("drm/virtio: fix OOB in virtio_gpu_object_create")
> >
> > Both are in drm-misc-next. I suspect the fix was added after
> > drm-misc-next was closed for the 5.7 merge window and thus should
> > have been submitted to drm-misc-next-fixes instead.
> >
> > So, what to do now? Should I cherry-pick 0666a8d7f6a4 into
> > drm-misc-next-fixes? Or should it go into drm-misc-fixes instead?
>
> Yup cherry-pick it over, with -x, to drm-misc-next-fixes.
> -Daniel
Done. So the next linux-next build should be green again. mainline
should get the fix with the next drm pull (may take a few days).
take care,
Gerd
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa-sim: depend on HAS_DMA
Pleae just drop the code - we should not add new drivers with custom DMA ops. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/2] vhost: disable for OABI
On Mon, Apr 06, 2020 at 02:50:32PM +0200, Arnd Bergmann wrote: > On Mon, Apr 6, 2020 at 2:12 PM Michael S. Tsirkin wrote: > > > > > +config VHOST_DPN > > + bool "VHOST dependencies" > > + depends on !ARM || AEABI > > + default y > > + help > > + Anything selecting VHOST or VHOST_RING must depend on VHOST_DPN. > > + This excludes the deprecated ARM ABI since that forces a 4 byte > > + alignment on all structs - incompatible with virtio spec > > requirements. > > + > > This should not be a user-visible option, so just make this 'def_bool > !ARM || AEABI' > > Arnd I like keeping some kind of hint around for when one tries to understand why is a specific symbol visible. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/2] vhost: disable for OABI
On Mon, Apr 06, 2020 at 02:45:13PM +0200, Ard Biesheuvel wrote: > On Mon, 6 Apr 2020 at 14:12, Michael S. Tsirkin wrote: > > > > vhost is currently broken on the default ARM config. > > > > Where did you get this idea? The report from the robot was using a > randconfig build, and in general, AEABI is required to run anything on > any modern ARM system . Oh - I forgot it's randconfig. This part is wrong, sorry. I decided to just force 2-byte alignment instead (seems more robust) but I'll take this into account if we do decide to add this dependency. > > > The reason is that that uses apcs-gnu which is the ancient OABI that is been > > deprecated for a long time. > > > > Given that virtio support on such ancient systems is not needed in the > > first place, let's just add something along the lines of > > > > depends on !ARM || AEABI > > > > to the virtio Kconfig declaration, and add a comment that it has to do > > with struct member alignment. > > > > Note: we can't make VHOST and VHOST_RING themselves have > > a dependency since these are selected. Add a new symbol for that. > > > > Suggested-by: Ard Biesheuvel > > Siggested-by: Richard Earnshaw > > typo ^^^ Thanks! > > > Signed-off-by: Michael S. Tsirkin > > --- > > drivers/misc/mic/Kconfig | 2 +- > > drivers/net/caif/Kconfig | 2 +- > > drivers/vdpa/Kconfig | 2 +- > > drivers/vhost/Kconfig| 17 + > > 4 files changed, 16 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/misc/mic/Kconfig b/drivers/misc/mic/Kconfig > > index 8f201d019f5a..3bfe72c59864 100644 > > --- a/drivers/misc/mic/Kconfig > > +++ b/drivers/misc/mic/Kconfig > > @@ -116,7 +116,7 @@ config MIC_COSM > > > > config VOP > > tristate "VOP Driver" > > - depends on VOP_BUS > > + depends on VOP_BUS && VHOST_DPN > > select VHOST_RING > > select VIRTIO > > help > > diff --git a/drivers/net/caif/Kconfig b/drivers/net/caif/Kconfig > > index 9db0570c5beb..661c25eb1c46 100644 > > --- a/drivers/net/caif/Kconfig > > +++ b/drivers/net/caif/Kconfig > > @@ -50,7 +50,7 @@ config CAIF_HSI > > > > config CAIF_VIRTIO > > tristate "CAIF virtio transport driver" > > - depends on CAIF && HAS_DMA > > + depends on CAIF && HAS_DMA && VHOST_DPN > > select VHOST_RING > > select VIRTIO > > select GENERIC_ALLOCATOR > > diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig > > index d0cb0e583a5d..aee28def466b 100644 > > --- a/drivers/vdpa/Kconfig > > +++ b/drivers/vdpa/Kconfig > > @@ -14,7 +14,7 @@ if VDPA_MENU > > > > config VDPA_SIM > > tristate "vDPA device simulator" > > - depends on RUNTIME_TESTING_MENU && HAS_DMA > > + depends on RUNTIME_TESTING_MENU && HAS_DMA && VHOST_DPN > > select VDPA > > select VHOST_RING > > select VHOST_IOTLB > > diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig > > index cb6b17323eb2..b3486e218f62 100644 > > --- a/drivers/vhost/Kconfig > > +++ b/drivers/vhost/Kconfig > > @@ -12,6 +12,15 @@ config VHOST_RING > > This option is selected by any driver which needs to access > > the host side of a virtio ring. > > > > +config VHOST_DPN > > + bool "VHOST dependencies" > > + depends on !ARM || AEABI > > + default y > > + help > > + Anything selecting VHOST or VHOST_RING must depend on VHOST_DPN. > > + This excludes the deprecated ARM ABI since that forces a 4 byte > > + alignment on all structs - incompatible with virtio spec > > requirements. > > + > > config VHOST > > tristate > > select VHOST_IOTLB > > @@ -27,7 +36,7 @@ if VHOST_MENU > > > > config VHOST_NET > > tristate "Host kernel accelerator for virtio net" > > - depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP) > > + depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP) && > > VHOST_DPN > > select VHOST > > ---help--- > > This kernel module can be loaded in host kernel to accelerate > > @@ -39,7 +48,7 @@ config VHOST_NET > > > > config VHOST_SCSI > > tristate "VHOST_SCSI TCM fabric driver" > > - depends on TARGET_CORE && EVENTFD > > + depends on TARGET_CORE && EVENTFD && VHOST_DPN > > select VHOST > > default n > > ---help--- > > @@ -48,7 +57,7 @@ config VHOST_SCSI > > > > config VHOST_VSOCK > > tristate "vhost virtio-vsock driver" > > - depends on VSOCKETS && EVENTFD > > + depends on VSOCKETS && EVENTFD && VHOST_DPN > > select VHOST > > select VIRTIO_VSOCKETS_COMMON > > default n > > @@ -62,7 +71,7 @@ config VHOST_VSOCK > > > > config VHOST_VDPA > > tristate "Vhost driver for vDPA-based backend" > > - depends on EVENTFD > > + depends on EVENTFD && VHOST_DPN > > select VHOST > > select VDPA > > help > > -- > > MST > >
[PATCH] vhost: force spec specified alignment on types
The ring element addresses are passed between components with different
alignments assumptions. Thus, if guest/userspace selects a pointer and
host then gets and dereferences it, we might need to decrease the
compiler-selected alignment to prevent compiler on the host from
assuming pointer is aligned.
This actually triggers on ARM with -mabi=apcs-gnu - which is a
deprecated configuration, but it seems safer to handle this
generally.
I verified that the produced binary is exactly identical on x86.
Signed-off-by: Michael S. Tsirkin
---
This is my preferred way to handle the ARM incompatibility issues
(in preference to kconfig hacks).
I will push this into next now.
Comments?
drivers/vhost/vhost.h| 6 ++---
include/uapi/linux/virtio_ring.h | 41
2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index cc82918158d2..a67bda9792ec 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -74,9 +74,9 @@ struct vhost_virtqueue {
/* The actual ring of buffers. */
struct mutex mutex;
unsigned int num;
- struct vring_desc __user *desc;
- struct vring_avail __user *avail;
- struct vring_used __user *used;
+ vring_desc_t __user *desc;
+ vring_avail_t __user *avail;
+ vring_used_t __user *used;
const struct vhost_iotlb_map *meta_iotlb[VHOST_NUM_ADDRS];
struct vhost_desc *descs;
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index 559f42e73315..cd6e0b2eaf2f 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -118,16 +118,6 @@ struct vring_used {
struct vring_used_elem ring[];
};
-struct vring {
- unsigned int num;
-
- struct vring_desc *desc;
-
- struct vring_avail *avail;
-
- struct vring_used *used;
-};
-
/* Alignment requirements for vring elements.
* When using pre-virtio 1.0 layout, these fall out naturally.
*/
@@ -164,6 +154,37 @@ struct vring {
#define vring_used_event(vr) ((vr)->avail->ring[(vr)->num])
#define vring_avail_event(vr) (*(__virtio16 *)&(vr)->used->ring[(vr)->num])
+/*
+ * The ring element addresses are passed between components with different
+ * alignments assumptions. Thus, we might need to decrease the
compiler-selected
+ * alignment, and so must use a typedef to make sure the __aligned attribute
+ * actually takes hold:
+ *
+ *
https://gcc.gnu.org/onlinedocs//gcc/Common-Type-Attributes.html#Common-Type-Attributes
+ *
+ * When used on a struct, or struct member, the aligned attribute can only
+ * increase the alignment; in order to decrease it, the packed attribute must
+ * be specified as well. When used as part of a typedef, the aligned attribute
+ * can both increase and decrease alignment, and specifying the packed
+ * attribute generates a warning.
+ */
+typedef struct vring_desc __attribute__((aligned(VRING_DESC_ALIGN_SIZE)))
+ vring_desc_t;
+typedef struct vring_avail __attribute__((aligned(VRING_AVAIL_ALIGN_SIZE)))
+ vring_avail_t;
+typedef struct vring_used __attribute__((aligned(VRING_USED_ALIGN_SIZE)))
+ vring_used_t;
+
+struct vring {
+ unsigned int num;
+
+ vring_desc_t *desc;
+
+ vring_avail_t *avail;
+
+ vring_used_t *used;
+};
+
static inline void vring_init(struct vring *vr, unsigned int num, void *p,
unsigned long align)
{
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 2/2] vhost: disable for OABI
On Mon, Apr 6, 2020 at 2:12 PM Michael S. Tsirkin wrote: > > +config VHOST_DPN > + bool "VHOST dependencies" > + depends on !ARM || AEABI > + default y > + help > + Anything selecting VHOST or VHOST_RING must depend on VHOST_DPN. > + This excludes the deprecated ARM ABI since that forces a 4 byte > + alignment on all structs - incompatible with virtio spec > requirements. > + This should not be a user-visible option, so just make this 'def_bool !ARM || AEABI' Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: upstream boot error: KASAN: slab-out-of-bounds Write in virtio_gpu_object_create
On Mon, Apr 6, 2020 at 10:06 AM Gerd Hoffmann wrote:
>
> On Mon, Apr 06, 2020 at 09:07:44AM +0200, Dmitry Vyukov wrote:
> > On Mon, Apr 6, 2020 at 8:46 AM syzbot
> > wrote:
> > >
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit:ffc1c20c Merge tag 'for-5.7/dm-changes' of
> > > git://git.kerne..
> > > git tree: upstream
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=1690471fe0
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=d6a1e2f9a9986236
> > > dashboard link:
> > > https://syzkaller.appspot.com/bug?extid=d3a7951ed361037407db
> > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > >
> > > Unfortunately, I don't have any reproducer for this crash yet.
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+d3a7951ed36103740...@syzkaller.appspotmail.com
> >
> >
> > +drivers/gpu/drm/virtio/virtgpu_object.c maintainers
> > Now we have both mainline and linux-next boot broken (linux-next is
> > broken for the past 40 days).
> > No testing of new code happens.
> >
> > > virtio_gpu_object_shmem_init drivers/gpu/drm/virtio/virtgpu_object.c:151
> > > [inline]
> > > virtio_gpu_object_create+0x9f3/0xaa0
> > > drivers/gpu/drm/virtio/virtgpu_object.c:230
>
> Ah, that one.
>
> broken patch: f651c8b05542 ("drm/virtio: factor out the sg_table from
> virtio_gpu_object")
> fixed by: 0666a8d7f6a4 ("drm/virtio: fix OOB in virtio_gpu_object_create")
>
> Both are in drm-misc-next. I suspect the fix was added after
> drm-misc-next was closed for the 5.7 merge window and thus should
> have been submitted to drm-misc-next-fixes instead.
>
> So, what to do now? Should I cherry-pick 0666a8d7f6a4 into
> drm-misc-next-fixes? Or should it go into drm-misc-fixes instead?
Yup cherry-pick it over, with -x, to drm-misc-next-fixes.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 18/70] x86/boot/compressed/64: Add stage1 #VC handler
On Thu, Mar 19, 2020 at 10:13:15AM +0100, Joerg Roedel wrote: > diff --git a/arch/x86/boot/compressed/idt_handlers_64.S > b/arch/x86/boot/compressed/idt_handlers_64.S > index bfb3fc5aa144..67ddafab2943 100644 > --- a/arch/x86/boot/compressed/idt_handlers_64.S > +++ b/arch/x86/boot/compressed/idt_handlers_64.S > @@ -75,3 +75,7 @@ SYM_FUNC_END(\name) > .code64 > > EXCEPTION_HANDLERboot_pf_handler do_boot_page_fault error_code=1 > + > +#ifdef CONFIG_AMD_MEM_ENCRYPT > +EXCEPTION_HANDLERboot_stage1_vc_handler vc_no_ghcb_handler error_code=1 Like the others boot_stage1_vc do_boot_stage1_vc ... -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 2/2] vhost: disable for OABI
vhost is currently broken on the default ARM config. The reason is that that uses apcs-gnu which is the ancient OABI that is been deprecated for a long time. Given that virtio support on such ancient systems is not needed in the first place, let's just add something along the lines of depends on !ARM || AEABI to the virtio Kconfig declaration, and add a comment that it has to do with struct member alignment. Note: we can't make VHOST and VHOST_RING themselves have a dependency since these are selected. Add a new symbol for that. Suggested-by: Ard Biesheuvel Siggested-by: Richard Earnshaw Signed-off-by: Michael S. Tsirkin --- drivers/misc/mic/Kconfig | 2 +- drivers/net/caif/Kconfig | 2 +- drivers/vdpa/Kconfig | 2 +- drivers/vhost/Kconfig| 17 + 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/misc/mic/Kconfig b/drivers/misc/mic/Kconfig index 8f201d019f5a..3bfe72c59864 100644 --- a/drivers/misc/mic/Kconfig +++ b/drivers/misc/mic/Kconfig @@ -116,7 +116,7 @@ config MIC_COSM config VOP tristate "VOP Driver" - depends on VOP_BUS + depends on VOP_BUS && VHOST_DPN select VHOST_RING select VIRTIO help diff --git a/drivers/net/caif/Kconfig b/drivers/net/caif/Kconfig index 9db0570c5beb..661c25eb1c46 100644 --- a/drivers/net/caif/Kconfig +++ b/drivers/net/caif/Kconfig @@ -50,7 +50,7 @@ config CAIF_HSI config CAIF_VIRTIO tristate "CAIF virtio transport driver" - depends on CAIF && HAS_DMA + depends on CAIF && HAS_DMA && VHOST_DPN select VHOST_RING select VIRTIO select GENERIC_ALLOCATOR diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig index d0cb0e583a5d..aee28def466b 100644 --- a/drivers/vdpa/Kconfig +++ b/drivers/vdpa/Kconfig @@ -14,7 +14,7 @@ if VDPA_MENU config VDPA_SIM tristate "vDPA device simulator" - depends on RUNTIME_TESTING_MENU && HAS_DMA + depends on RUNTIME_TESTING_MENU && HAS_DMA && VHOST_DPN select VDPA select VHOST_RING select VHOST_IOTLB diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig index cb6b17323eb2..b3486e218f62 100644 --- a/drivers/vhost/Kconfig +++ b/drivers/vhost/Kconfig @@ -12,6 +12,15 @@ config VHOST_RING This option is selected by any driver which needs to access the host side of a virtio ring. +config VHOST_DPN + bool "VHOST dependencies" + depends on !ARM || AEABI + default y + help + Anything selecting VHOST or VHOST_RING must depend on VHOST_DPN. + This excludes the deprecated ARM ABI since that forces a 4 byte + alignment on all structs - incompatible with virtio spec requirements. + config VHOST tristate select VHOST_IOTLB @@ -27,7 +36,7 @@ if VHOST_MENU config VHOST_NET tristate "Host kernel accelerator for virtio net" - depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP) + depends on NET && EVENTFD && (TUN || !TUN) && (TAP || !TAP) && VHOST_DPN select VHOST ---help--- This kernel module can be loaded in host kernel to accelerate @@ -39,7 +48,7 @@ config VHOST_NET config VHOST_SCSI tristate "VHOST_SCSI TCM fabric driver" - depends on TARGET_CORE && EVENTFD + depends on TARGET_CORE && EVENTFD && VHOST_DPN select VHOST default n ---help--- @@ -48,7 +57,7 @@ config VHOST_SCSI config VHOST_VSOCK tristate "vhost virtio-vsock driver" - depends on VSOCKETS && EVENTFD + depends on VSOCKETS && EVENTFD && VHOST_DPN select VHOST select VIRTIO_VSOCKETS_COMMON default n @@ -62,7 +71,7 @@ config VHOST_VSOCK config VHOST_VDPA tristate "Vhost driver for vDPA-based backend" - depends on EVENTFD + depends on EVENTFD && VHOST_DPN select VHOST select VDPA help -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 1/2] vdpa-sim: depend on HAS_DMA
set_dma_ops isn't available on all architectures: make ARCH=um ... drivers/vdpa/vdpa_sim/vdpa_sim.c: In function 'vdpasim_create': >> drivers/vdpa/vdpa_sim/vdpa_sim.c:324:2: error: implicit declaration of >> function 'set_dma_ops'; did you mean 'set_groups'? +[-Werror=implicit-function-declaration] set_dma_ops(dev, _dma_ops); ^~~ set_groups Signed-off-by: Michael S. Tsirkin --- drivers/vdpa/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig index 08b615f2da39..d0cb0e583a5d 100644 --- a/drivers/vdpa/Kconfig +++ b/drivers/vdpa/Kconfig @@ -14,7 +14,7 @@ if VDPA_MENU config VDPA_SIM tristate "vDPA device simulator" - depends on RUNTIME_TESTING_MENU + depends on RUNTIME_TESTING_MENU && HAS_DMA select VDPA select VHOST_RING select VHOST_IOTLB -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 15/70] x86/boot/compressed/64: Always switch to own page-table
On Thu, Mar 19, 2020 at 10:13:12AM +0100, Joerg Roedel wrote: > From: Joerg Roedel > > When booted through startup_64 the kernel keeps running on the EFI > page-table until the KASLR code sets up its own page-table. Without > KASLR the pre-decompression boot code never switches off the EFI > page-table. Change that by unconditionally switching to our own > page-table once the kernel is relocated. > > This makes sure we can make changes to the mapping when necessary, for Pls use passive voice in your commit message: no "we" or "I", etc, and describe your changes in imperative mood. -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH RFC v2 02/24] scsi: allocate separate queue for reserved commands
On 3/11/20 7:22 AM, Christoph Hellwig wrote: > On Tue, Mar 10, 2020 at 09:08:56PM +, John Garry wrote: >> On 10/03/2020 18:32, Christoph Hellwig wrote: >>> On Wed, Mar 11, 2020 at 12:25:28AM +0800, John Garry wrote: From: Hannes Reinecke Allocate a separate 'reserved_cmd_q' for sending reserved commands. >>> >>> Why? Reserved command specifically are not in any way tied to queues. >>> . >>> >> >> So the v1 series used a combination of the sdev queue and the per-host >> reserved_cmd_q. Back then you questioned using the sdev queue for virtio >> scsi, and the unconfirmed conclusion was to use a common per-host q. This is >> the best link I can find now: >> >> https://www.mail-archive.com/linux-scsi@vger.kernel.org/msg83177.html > > That was just a question on why virtio uses the per-device tags, which > didn't look like it made any sense. What I'm worried about here is > mixing up the concept of reserved tags in the tagset, and queues to use > them. Note that we already have the scsi_get_host_dev to allocate > a scsi_device and thus a request_queue for the host itself. That seems > like the better interface to use a tag for a host wide command vs > introducing a parallel path. > Thinking about it some more, I don't think that scsi_get_host_dev() is the best way of handling it. Problem is that it'll create a new scsi_device with , which will then show up via eg 'lsscsi'. This would be okay if 'this_id' would have been defined by the driver; sadly, most drivers which are affected here do set 'this_id' to -1. So we wouldn't have a nice target ID to allocate the device from, let alone the problem that we would have to emulate a complete scsi device with all required minimal command support etc. And I'm not quite sure how well that would play with the exising SCSI host template; the device we'll be allocating would have basically nothing in common with the 'normal' SCSI devices. What we could do, though, is to try it the other way round: Lift the request queue from scsi_get_host_dev() into the scsi host itself, so that scsi_get_host_dev() can use that queue, but we also would be able to use it without a SCSI device attached. Cheers, Hannes -- Dr. Hannes ReineckeKernel Storage Architect h...@suse.de +49 911 74053 688 SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg HRB 36809 (AG Nürnberg), GF: Felix Imendörffer ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] Fix: buffer overflow during hvc_alloc().
> > Description of problem: > Guest get 'Call Trace' when loading module "virtio_console" and unloading > it frequently > > > Version-Release number of selected component (if applicable): > Guest > kernel-4.18.0-167.el8.x86_64 > seabios-bin-1.11.1-4.module+el8.1.0+4066+0f1aadab.noarch > # modinfo virtio_console >filename: /lib/modules/4.18.0- >167.el8.x86_64/kernel/drivers/char/virtio_console.ko.xz >license:GPL >description:Virtio console driver >rhelversion:8.2 >srcversion: 55224090DD07750FAD75C9C >alias: virtio:d0003v* >depends: >intree: Y >name: virtio_console >vermagic: 4.18.0-167.el8.x86_64 SMP mod_unload modversions > Host: > qemu-kvm-4.2.0-2.scrmod+el8.2.0+5159+d8aa4d83.x86_64 > kernel-4.18.0-165.el8.x86_64 > seabios-bin-1.12.0-5.scrmod+el8.2.0+5159+d8aa4d83.noarch > > > > How reproducible: 100% > > > Steps to Reproduce: > > 1. boot guest with command [1] > 2. load and unload virtio_console inside guest with loop.sh ># cat loop.sh > while [ 1 ] > do > modprobe virtio_console > lsmod | grep virt > modprobe -r virtio_console > lsmod | grep virt > done > > > > Actual results: > Guest reboot and can get vmcore-dmesg.txt file > > > Expected results: > Guest works well without error > > > Additional info: > The whole log will attach to the attachments. > > Call Trace: > [ 22.974500] fuse: init (API version 7.31) > [ 81.498208] [ cut here ] > [ 81.499263] pvqspinlock: lock 0x92080020 has corrupted value > 0xc0774ca0! > [ 81.501000] WARNING: CPU: 0 PID: 785 at > kernel/locking/qspinlock_paravirt.h:500 > __pv_queued_spin_unlock_slowpath+0xc0/0xd0 > [ 81.503173] Modules linked in: virtio_console fuse xt_CHECKSUM > ipt_MASQUERADE xt_conntrack ipt_REJECT nft_counter nf_nat_tftp nft_objref > nf_conntrack_tftp tun bridge stp llc nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 > nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct > nf_tables_set nft_chain_nat_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 > nf_nat_ipv6 nft_chain_route_ipv6 nft_chain_nat_ipv4 nf_conntrack_ipv4 > nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack nft_chain_route_ipv4 > ip6_tables nft_compat ip_set nf_tables nfnetlink sunrpc bochs_drm > drm_vram_helper ttm drm_kms_helper syscopyarea sysfillrect sysimgblt > fb_sys_fops drm i2c_piix4 pcspkr crct10dif_pclmul crc32_pclmul joydev > ghash_clmulni_intel ip_tables xfs libcrc32c sd_mod sg ata_generic ata_piix > virtio_net libata crc32c_intel net_failover failover serio_raw virtio_scsi > dm_mirror dm_region_hash dm_log dm_mod [last unloaded: virtio_console] > [ 81.517019] CPU: 0 PID: 785 Comm: kworker/0:2 Kdump: loaded Not tainted > 4.18.0-167.el8.x86_64 #1 > [ 81.518639] Hardware name: Red Hat KVM, BIOS > 1.12.0-5.scrmod+el8.2.0+5159+d8aa4d83 04/01/2014 > [ 81.520205] Workqueue: events control_work_handler [virtio_console] > [ 81.521354] RIP: 0010:__pv_queued_spin_unlock_slowpath+0xc0/0xd0 > [ 81.522450] Code: 07 00 48 63 7a 10 e8 bf 64 f5 ff 66 90 c3 8b 05 e6 cf > d6 01 85 c0 74 01 c3 8b 17 48 89 fe 48 c7 c7 38 4b 29 91 e8 3a 6c fa ff > <0f> 0b c3 0f 0b 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 48 > [ 81.525830] RSP: 0018:b51a01ffbd70 EFLAGS: 00010282 > [ 81.526798] RAX: RBX: 0010 RCX: > > [ 81.528110] RDX: 9e66f1826480 RSI: 9e66f1816a08 RDI: > 9e66f1816a08 > [ 81.529437] RBP: 9153ff10 R08: 026c R09: > 0053 > [ 81.530732] R10: R11: b51a01ffbc18 R12: > 9e66cd682200 > [ 81.532133] R13: 9153ff10 R14: 9e6685569500 R15: > 9e66cd682000 > [ 81.533442] FS: () GS:9e66f180() > knlGS: > [ 81.534914] CS: 0010 DS: ES: CR0: 80050033 > [ 81.535971] CR2: 5624c55b14d0 CR3: 0003a023c000 CR4: > 003406f0 > [ 81.537283] Call Trace: > [ 81.537763] > __raw_callee_save___pv_queued_spin_unlock_slowpath+0x11/0x20 > [ 81.539011] .slowpath+0x9/0xe > [ 81.539585] hvc_alloc+0x25e/0x300 > [ 81.540237] init_port_console+0x28/0x100 [virtio_console] > [ 81.541251] handle_control_message.constprop.27+0x1c4/0x310 > [virtio_console] > [ 81.542546] control_work_handler+0x70/0x10c [virtio_console] > [ 81.543601] process_one_work+0x1a7/0x3b0 > [ 81.544356] worker_thread+0x30/0x390 > [ 81.545025] ? create_worker+0x1a0/0x1a0 > [ 81.545749] kthread+0x112/0x130 > [ 81.546358] ? kthread_flush_work_fn+0x10/0x10 > [ 81.547183] ret_from_fork+0x22/0x40 > [ 81.547842] ---[ end trace aa97649bd16c8655 ]--- > [ 83.546539] general protection fault: [#1] SMP NOPTI > [ 83.547422] CPU: 5 PID: 3225 Comm: modprobe Kdump: loaded Tainted: G > W- - - 4.18.0-167.el8.x86_64 #1 > [ 83.549191]
Re: upstream boot error: KASAN: slab-out-of-bounds Write in virtio_gpu_object_create
On Mon, Apr 06, 2020 at 09:07:44AM +0200, Dmitry Vyukov wrote:
> On Mon, Apr 6, 2020 at 8:46 AM syzbot
> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit:ffc1c20c Merge tag 'for-5.7/dm-changes' of git://git.kerne..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=1690471fe0
> > kernel config: https://syzkaller.appspot.com/x/.config?x=d6a1e2f9a9986236
> > dashboard link: https://syzkaller.appspot.com/bug?extid=d3a7951ed361037407db
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> >
> > Unfortunately, I don't have any reproducer for this crash yet.
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+d3a7951ed36103740...@syzkaller.appspotmail.com
>
>
> +drivers/gpu/drm/virtio/virtgpu_object.c maintainers
> Now we have both mainline and linux-next boot broken (linux-next is
> broken for the past 40 days).
> No testing of new code happens.
>
> > virtio_gpu_object_shmem_init drivers/gpu/drm/virtio/virtgpu_object.c:151
> > [inline]
> > virtio_gpu_object_create+0x9f3/0xaa0
> > drivers/gpu/drm/virtio/virtgpu_object.c:230
Ah, that one.
broken patch: f651c8b05542 ("drm/virtio: factor out the sg_table from
virtio_gpu_object")
fixed by: 0666a8d7f6a4 ("drm/virtio: fix OOB in virtio_gpu_object_create")
Both are in drm-misc-next. I suspect the fix was added after
drm-misc-next was closed for the 5.7 merge window and thus should
have been submitted to drm-misc-next-fixes instead.
So, what to do now? Should I cherry-pick 0666a8d7f6a4 into
drm-misc-next-fixes? Or should it go into drm-misc-fixes instead?
thanks,
Gerd
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: upstream boot error: KASAN: slab-out-of-bounds Write in virtio_gpu_object_create
On Mon, Apr 6, 2020 at 8:46 AM syzbot wrote: > > Hello, > > syzbot found the following crash on: > > HEAD commit:ffc1c20c Merge tag 'for-5.7/dm-changes' of git://git.kerne.. > git tree: upstream > console output: https://syzkaller.appspot.com/x/log.txt?x=1690471fe0 > kernel config: https://syzkaller.appspot.com/x/.config?x=d6a1e2f9a9986236 > dashboard link: https://syzkaller.appspot.com/bug?extid=d3a7951ed361037407db > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > Unfortunately, I don't have any reproducer for this crash yet. > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+d3a7951ed36103740...@syzkaller.appspotmail.com +drivers/gpu/drm/virtio/virtgpu_object.c maintainers Now we have both mainline and linux-next boot broken (linux-next is broken for the past 40 days). No testing of new code happens. > NET: Registered protocol family 38 > async_tx: api initialized (async) > Key type asymmetric registered > Asymmetric key parser 'x509' registered > Asymmetric key parser 'pkcs8' registered > Key type pkcs7_test registered > Asymmetric key parser 'tpm_parser' registered > Block layer SCSI generic (bsg) driver version 0.4 loaded (major 243) > io scheduler mq-deadline registered > io scheduler kyber registered > io scheduler bfq registered > hgafb: HGA card not detected. > hgafb: probe of hgafb.0 failed with error -22 > usbcore: registered new interface driver udlfb > uvesafb: failed to execute /sbin/v86d > uvesafb: make sure that the v86d helper is installed and executable > uvesafb: Getting VBE info block failed (eax=0x4f00, err=-2) > uvesafb: vbe_init() failed with -22 > uvesafb: probe of uvesafb.0 failed with error -22 > vga16fb: mapped to 0xab381a8e > Console: switching to colour frame buffer device 80x30 > fb0: VGA16 VGA frame buffer device > input: Power Button as /devices/LNXSYSTM:00/LNXPWRBN:00/input/input0 > ACPI: Power Button [PWRF] > ioatdma: Intel(R) QuickData Technology Driver 5.00 > PCI Interrupt Link [GSIF] enabled at IRQ 21 > PCI Interrupt Link [GSIH] enabled at IRQ 23 > N_HDLC line discipline registered with maxframe=4096 > Serial: 8250/16550 driver, 4 ports, IRQ sharing enabled > 00:05: ttyS0 at I/O 0x3f8 (irq = 4, base_baud = 115200) is a 16550A > Cyclades driver 2.6 > Initializing Nozomi driver 2.1d > RocketPort device driver module, version 2.09, 12-June-2003 > No rocketport ports found; unloading driver > Non-volatile memory driver v1.3 > Linux agpgart interface v0.103 > [drm] Initialized vgem 1.0.0 20120112 for vgem on minor 0 > [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [drm] Initialized vkms 1.0.0 20180514 for vkms on minor 1 > usbcore: registered new interface driver udl > [drm] pci: virtio-vga detected at :00:01.0 > fb0: switching to virtiodrmfb from VGA16 VGA > Console: switching to colour VGA+ 80x25 > virtio-pci :00:01.0: vgaarb: deactivate vga console > Console: switching to colour dummy device 80x25 > [drm] features: -virgl +edid > [drm] number of scanouts: 1 > [drm] number of cap sets: 0 > [drm] Initialized virtio_gpu 0.1.0 0 for virtio0 on minor 2 > == > BUG: KASAN: slab-out-of-bounds in virtio_gpu_object_shmem_init > drivers/gpu/drm/virtio/virtgpu_object.c:151 [inline] > BUG: KASAN: slab-out-of-bounds in virtio_gpu_object_create+0x9f3/0xaa0 > drivers/gpu/drm/virtio/virtgpu_object.c:230 > Write of size 8 at addr 888027f7a388 by task swapper/0/1 > > CPU: 0 PID: 1 Comm: swapper/0 Not tainted 5.6.0-syzkaller #0 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 > Call Trace: > __dump_stack lib/dump_stack.c:77 [inline] > dump_stack+0x188/0x20d lib/dump_stack.c:118 > print_address_description.constprop.0.cold+0xd3/0x315 mm/kasan/report.c:374 > __kasan_report.cold+0x1a/0x32 mm/kasan/report.c:506 > kasan_report+0xe/0x20 mm/kasan/common.c:641 > virtio_gpu_object_shmem_init drivers/gpu/drm/virtio/virtgpu_object.c:151 > [inline] > virtio_gpu_object_create+0x9f3/0xaa0 > drivers/gpu/drm/virtio/virtgpu_object.c:230 > virtio_gpu_gem_create+0xaa/0x1d0 drivers/gpu/drm/virtio/virtgpu_gem.c:42 > virtio_gpu_mode_dumb_create+0x21e/0x360 > drivers/gpu/drm/virtio/virtgpu_gem.c:82 > drm_mode_create_dumb+0x27c/0x300 drivers/gpu/drm/drm_dumb_buffers.c:94 > drm_client_buffer_create drivers/gpu/drm/drm_client.c:267 [inline] > drm_client_framebuffer_create+0x1b7/0x770 drivers/gpu/drm/drm_client.c:412 > drm_fb_helper_generic_probe+0x1e4/0x810 drivers/gpu/drm/drm_fb_helper.c:2039 > drm_fb_helper_single_fb_probe drivers/gpu/drm/drm_fb_helper.c:1588 [inline] > __drm_fb_helper_initial_config_and_unlock+0xb56/0x11e0 > drivers/gpu/drm/drm_fb_helper.c:1746 > drm_fb_helper_initial_config drivers/gpu/drm/drm_fb_helper.c:1841 [inline] > drm_fb_helper_initial_config drivers/gpu/drm/drm_fb_helper.c:1833 [inline] >
Re: [PATCH 31/44] drm/qxl: Don't use drm_device->dev_private
On Fri, Apr 03, 2020 at 03:58:15PM +0200, Daniel Vetter wrote: > Upcasting using a container_of macro is more typesafe, faster and > easier for the compiler to optimize. > > Signed-off-by: Daniel Vetter > Cc: Dave Airlie > Cc: Gerd Hoffmann > Cc: virtualization@lists.linux-foundation.org > Cc: spice-de...@lists.freedesktop.org Acked-by: Gerd Hoffmann ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 30/44] drm/qxl: Use devm_drm_dev_alloc
On Fri, Apr 03, 2020 at 03:58:14PM +0200, Daniel Vetter wrote: > Also need to remove the drm_dev_put from the remove hook. > > Signed-off-by: Daniel Vetter > Cc: Dave Airlie > Cc: Gerd Hoffmann > Cc: virtualization@lists.linux-foundation.org > Cc: spice-de...@lists.freedesktop.org Acked-by: Gerd Hoffmann ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
