[PATCH vhost 10/10] virtio_ring: introduce virtqueue_reset()
Introduce virtqueue_reset() to release all buffer inside vq.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 50
include/linux/virtio.h | 2 ++
2 files changed, 52 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 2ba60a14f557..2750a365439a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2930,6 +2930,56 @@ int virtqueue_resize(struct virtqueue *_vq, u32 num,
}
EXPORT_SYMBOL_GPL(virtqueue_resize);
+/**
+ * virtqueue_reset - detach and recycle all unused buffers
+ * @_vq: the struct virtqueue we're talking about.
+ * @recycle: callback to recycle unused buffers
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * Returns zero or a negative error.
+ * 0: success.
+ * -EBUSY: Failed to sync with device, vq may not work properly
+ * -ENOENT: Transport or device not supported
+ * -EPERM: Operation not permitted
+ */
+int virtqueue_reset(struct virtqueue *_vq,
+ void (*recycle)(struct virtqueue *vq, void *buf))
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ struct virtio_device *vdev = vq->vq.vdev;
+ void *buf;
+ int err;
+
+ if (!vq->we_own_ring)
+ return -EPERM;
+
+ if (!vdev->config->disable_vq_and_reset)
+ return -ENOENT;
+
+ if (!vdev->config->enable_vq_after_reset)
+ return -ENOENT;
+
+ err = vdev->config->disable_vq_and_reset(_vq);
+ if (err)
+ return err;
+
+ while ((buf = virtqueue_detach_unused_buf(_vq)) != NULL)
+ recycle(_vq, buf);
+
+ if (vq->packed_ring)
+ virtqueue_reinit_packed(vq);
+ else
+ virtqueue_reinit_split(vq);
+
+ if (vdev->config->enable_vq_after_reset(_vq))
+ return -EBUSY;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtqueue_reset);
+
/* Only available for split ring */
struct virtqueue *vring_new_virtqueue(unsigned int index,
unsigned int num,
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d0e707d744a0..cf4c157e4e75 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -106,6 +106,8 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue *vq);
int virtqueue_resize(struct virtqueue *vq, u32 num,
void (*recycle)(struct virtqueue *vq, void *buf));
+int virtqueue_reset(struct virtqueue *vq,
+ void (*recycle)(struct virtqueue *vq, void *buf));
/**
* struct virtio_device - representation of a device using virtio
--
2.32.0.3.g01195cf9f
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH vhost 09/10] virtio_ring: correct the expression of the description of virtqueue_resize()
Modify the "useless" to a more accurate "unused". Signed-off-by: Xuan Zhuo --- drivers/virtio/virtio_ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c index 84129b8c3e2a..2ba60a14f557 100644 --- a/drivers/virtio/virtio_ring.c +++ b/drivers/virtio/virtio_ring.c @@ -2865,7 +2865,7 @@ EXPORT_SYMBOL_GPL(vring_create_virtqueue_dma); * virtqueue_resize - resize the vring of vq * @_vq: the struct virtqueue we're talking about. * @num: new ring num - * @recycle: callback for recycle the useless buffer + * @recycle: callback to recycle unused buffers * * When it is really necessary to create a new vring, it will set the current vq * into the reset state. Then call the passed callback to recycle the buffer -- 2.32.0.3.g01195cf9f ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH vhost 04/10] virtio_ring: split: introduce virtqueue_add_split_premapped()
virtqueue_add_split() only supports virtual addresses, dma is completed
in virtqueue_add_split().
In some scenarios (such as the AF_XDP scenario), the memory is allocated
and DMA is completed in advance, so it is necessary for us to support
passing the DMA address to virtio core.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 100 +--
include/linux/virtio.h | 5 ++
2 files changed, 100 insertions(+), 5 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 47b6f9152f9f..a31155abe101 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -70,6 +70,7 @@
struct vring_desc_state_split {
void *data; /* Data for callback. */
struct vring_desc *indir_desc; /* Indirect descriptor, if any. */
+ bool premapped;
};
struct vring_desc_state_packed {
@@ -440,7 +441,7 @@ static void vring_unmap_one_split_indirect(const struct
vring_virtqueue *vq,
}
static unsigned int vring_unmap_one_split(const struct vring_virtqueue *vq,
- unsigned int i)
+ unsigned int i, bool premapped)
{
struct vring_desc_extra *extra = vq->split.desc_extra;
u16 flags;
@@ -457,6 +458,9 @@ static unsigned int vring_unmap_one_split(const struct
vring_virtqueue *vq,
(flags & VRING_DESC_F_WRITE) ?
DMA_FROM_DEVICE : DMA_TO_DEVICE);
} else {
+ if (premapped)
+ goto out;
+
dma_unmap_page(vring_dma_dev(vq),
extra[i].addr,
extra[i].len,
@@ -788,6 +792,47 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
return err;
}
+static inline int virtqueue_add_split_premapped(struct virtqueue *_vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs,
+ void *data,
+ void *ctx,
+ gfp_t gfp)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ struct vring_desc *desc;
+ int head;
+ int err;
+
+ START_USE(vq);
+
+ /* check vq state and try to alloc desc for indirect. */
+ err = virtqueue_add_split_prepare(vq, total_sg, out_sgs, data, ctx,
gfp, );
+ if (err)
+ goto end;
+
+ head = vq->free_head;
+ err = virtqueue_add_split_vring(vq, sgs, total_sg, out_sgs, in_sgs,
desc);
+ if (err)
+ goto err;
+
+ /* Store token and indirect buffer state. */
+ vq->split.desc_state[head].data = data;
+ vq->split.desc_state[head].indir_desc = desc ? desc : ctx;
+ vq->split.desc_state[head].premapped = true;
+
+ goto end;
+
+err:
+ kfree(desc);
+
+end:
+ END_USE(vq);
+ return err;
+}
+
static bool virtqueue_kick_prepare_split(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
@@ -824,20 +869,23 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
{
unsigned int i, j;
__virtio16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT);
+ bool premapped;
/* Clear data ptr. */
vq->split.desc_state[head].data = NULL;
+ premapped = vq->split.desc_state[head].premapped;
+
/* Put back on free list: unmap first-level descriptors and find end */
i = head;
while (vq->split.vring.desc[i].flags & nextflag) {
- vring_unmap_one_split(vq, i);
+ vring_unmap_one_split(vq, i, premapped);
i = vq->split.desc_extra[i].next;
vq->vq.num_free++;
}
- vring_unmap_one_split(vq, i);
+ vring_unmap_one_split(vq, i, premapped);
vq->split.desc_extra[i].next = vq->free_head;
vq->free_head = head;
@@ -859,8 +907,10 @@ static void detach_buf_split(struct vring_virtqueue *vq,
unsigned int head,
VRING_DESC_F_INDIRECT));
BUG_ON(len == 0 || len % sizeof(struct vring_desc));
- for (j = 0; j < len / sizeof(struct vring_desc); j++)
- vring_unmap_one_split_indirect(vq, _desc[j]);
+ if (!premapped) {
+ for (j = 0; j < len / sizeof(struct vring_desc); j++)
+ vring_unmap_one_split_indirect(vq,
_desc[j]);
+ }
kfree(indir_desc);
vq->split.desc_state[head].indir_desc = NULL;
@@ -2204,6 +2254,21 @@ static inline int virtqueue_add(struct
[PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma
Added virtio_dma_map() to map DMA addresses for virtual memory in
advance. The purpose is to keep memory mapped across multiple add/get
buf operations.
Added virtio_dma_unmap() for unmap DMA address.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 92
include/linux/virtio.h | 9
2 files changed, 101 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd9364eb2345..855338609c7f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct virtqueue
*vq)
}
EXPORT_SYMBOL_GPL(virtqueue_get_vring);
+/**
+ * virtio_dma_map_page - get the DMA addr of the memory for virtio device
+ * @dev: virtio device
+ * @page: the page of the memory to DMA
+ * @offset: the offset of the memory inside page
+ * @length: memory length
+ * @dir: DMA direction
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ *
+ * Returns the DMA addr. DMA_MAPPING_ERROR means error.
+ */
+dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, size_t
offset,
+ unsigned int length, enum dma_data_direction dir)
+{
+ struct virtio_device *vdev = dev_to_virtio(dev);
+
+ if (!vring_use_dma_api(vdev))
+ return page_to_phys(page) + offset;
+
+ return dma_map_page(vdev->dev.parent, page, offset, length, dir);
+}
+
+/**
+ * virtio_dma_map - get the DMA addr of the memory for virtio device
+ * @dev: virtio device
+ * @addr: the addr to DMA
+ * @length: memory length
+ * @dir: DMA direction
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ *
+ * Returns the DMA addr.
+ */
+dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int length,
+ enum dma_data_direction dir)
+{
+ struct page *page;
+ size_t offset;
+
+ page = virt_to_page(addr);
+ offset = offset_in_page(addr);
+
+ return virtio_dma_map_page(dev, page, offset, length, dir);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_map);
+
+/**
+ * virtio_dma_mapping_error - check dma address
+ * @dev: virtio device
+ * @addr: DMA address
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ *
+ * Returns 0 means dma valid. Other means invalid dma address.
+ */
+int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr)
+{
+ struct virtio_device *vdev = dev_to_virtio(dev);
+
+ if (!vring_use_dma_api(vdev))
+ return 0;
+
+ return dma_mapping_error(vdev->dev.parent, addr);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_mapping_error);
+
+/**
+ * virtio_dma_unmap - unmap DMA addr
+ * @dev: virtio device
+ * @dma: DMA address
+ * @length: memory length
+ * @dir: DMA direction
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ */
+void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int length,
+ enum dma_data_direction dir)
+{
+ struct virtio_device *vdev = dev_to_virtio(dev);
+
+ if (!vring_use_dma_api(vdev))
+ return;
+
+ dma_unmap_page(vdev->dev.parent, dma, length, dir);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_unmap);
+
MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 3ebb346ebb7c..b5fa71476737 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -9,6 +9,7 @@
#include
#include
#include
+#include
/**
* struct virtqueue - a queue to register buffers for sending or receiving.
@@ -216,4 +217,12 @@ void unregister_virtio_driver(struct virtio_driver *drv);
#define module_virtio_driver(__virtio_driver) \
module_driver(__virtio_driver, register_virtio_driver, \
unregister_virtio_driver)
+
+dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, size_t
offset,
+ unsigned int length, enum dma_data_direction
dir);
+dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int length,
+ enum dma_data_direction dir);
+int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr);
+void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int length,
+ enum dma_data_direction dir);
#endif /* _LINUX_VIRTIO_H */
--
2.32.0.3.g01195cf9f
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH vhost 08/10] virtio_ring: introduce dma sync api for virtio
These API has been introduced:
* virtio_dma_need_sync
* virtio_dma_sync_single_range_for_cpu
* virtio_dma_sync_single_range_for_device
These APIs can be used together with the premapped mechanism to sync the
DMA address.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 70
include/linux/virtio.h | 8 +
2 files changed, 78 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 855338609c7f..84129b8c3e2a 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -3264,4 +3264,74 @@ void virtio_dma_unmap(struct device *dev, dma_addr_t
dma, unsigned int length,
}
EXPORT_SYMBOL_GPL(virtio_dma_unmap);
+/**
+ * virtio_dma_need_sync - check a dma address needs sync
+ * @dev: virtio device
+ * @addr: DMA address
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ */
+bool virtio_dma_need_sync(struct device *dev, dma_addr_t addr)
+{
+ struct virtio_device *vdev = dev_to_virtio(dev);
+
+ if (!vring_use_dma_api(vdev))
+ return 0;
+
+ return dma_need_sync(vdev->dev.parent, addr);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_need_sync);
+
+/**
+ * virtio_dma_sync_single_range_for_cpu - dma sync for cpu
+ * @dev: virtio device
+ * @addr: DMA address
+ * @offset: DMA address offset
+ * @size: mem size for sync
+ * @dir: DMA direction
+ *
+ * Before calling this function, use virtio_dma_need_sync() to confirm that the
+ * DMA address really needs to be synchronized
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ */
+void virtio_dma_sync_single_range_for_cpu(struct device *dev, dma_addr_t addr,
+ unsigned long offset, size_t size,
+ enum dma_data_direction dir)
+{
+ struct virtio_device *vdev = dev_to_virtio(dev);
+
+ dma_sync_single_range_for_cpu(vdev->dev.parent, addr, offset,
+ size, DMA_BIDIRECTIONAL);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_sync_single_range_for_cpu);
+
+/**
+ * virtio_dma_sync_single_range_for_device - dma sync for device
+ * @dev: virtio device
+ * @addr: DMA address
+ * @offset: DMA address offset
+ * @size: mem size for sync
+ * @dir: DMA direction
+ *
+ * Before calling this function, use virtio_dma_need_sync() to confirm that the
+ * DMA address really needs to be synchronized
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ */
+void virtio_dma_sync_single_range_for_device(struct device *dev,
+dma_addr_t addr,
+unsigned long offset, size_t size,
+enum dma_data_direction dir)
+{
+ struct virtio_device *vdev = dev_to_virtio(dev);
+
+ dma_sync_single_range_for_device(vdev->dev.parent, addr, offset,
+size, DMA_BIDIRECTIONAL);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_sync_single_range_for_device);
+
MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b5fa71476737..d0e707d744a0 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -225,4 +225,12 @@ dma_addr_t virtio_dma_map(struct device *dev, void *addr,
unsigned int length,
int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr);
void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int length,
enum dma_data_direction dir);
+bool virtio_dma_need_sync(struct device *dev, dma_addr_t addr);
+void virtio_dma_sync_single_range_for_cpu(struct device *dev, dma_addr_t addr,
+ unsigned long offset, size_t size,
+ enum dma_data_direction dir);
+void virtio_dma_sync_single_range_for_device(struct device *dev,
+dma_addr_t addr,
+unsigned long offset, size_t size,
+enum dma_data_direction dir);
#endif /* _LINUX_VIRTIO_H */
--
2.32.0.3.g01195cf9f
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH vhost 05/10] virtio_ring: packed: introduce virtqueue_add_packed_premapped()
virtqueue_add_packed() only supports virtual addresses, dma is completed
in virtqueue_add_packed().
In some scenarios (such as the AF_XDP scenario), the memory is allocated
and DMA is completed in advance, so it is necessary for us to support
passing the DMA address to virtio core.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 67
1 file changed, 61 insertions(+), 6 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index a31155abe101..79244ccbae9e 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -78,6 +78,7 @@ struct vring_desc_state_packed {
struct vring_packed_desc *indir_desc; /* Indirect descriptor, if any. */
u16 num;/* Descriptor list length. */
u16 last; /* The last desc state in a list. */
+ bool premapped;
};
struct vring_desc_extra {
@@ -1318,7 +1319,8 @@ static inline u16 packed_last_used(u16 last_used_idx)
}
static void vring_unmap_extra_packed(const struct vring_virtqueue *vq,
-struct vring_desc_extra *extra)
+struct vring_desc_extra *extra,
+bool premapped)
{
u16 flags;
@@ -1333,6 +1335,9 @@ static void vring_unmap_extra_packed(const struct
vring_virtqueue *vq,
(flags & VRING_DESC_F_WRITE) ?
DMA_FROM_DEVICE : DMA_TO_DEVICE);
} else {
+ if (premapped)
+ return;
+
dma_unmap_page(vring_dma_dev(vq),
extra->addr, extra->len,
(flags & VRING_DESC_F_WRITE) ?
@@ -1382,7 +1387,7 @@ static int virtqueue_add_indirect_packed(struct
vring_virtqueue *vq,
struct vring_packed_desc *desc)
{
struct scatterlist *sg;
- unsigned int i, n, err_idx;
+ unsigned int i, n;
u16 head, id;
dma_addr_t addr;
@@ -1640,6 +1645,51 @@ static inline int virtqueue_add_packed(struct virtqueue
*_vq,
return err;
}
+static inline int virtqueue_add_packed_premapped(struct virtqueue *_vq,
+struct scatterlist *sgs[],
+unsigned int total_sg,
+unsigned int out_sgs,
+unsigned int in_sgs,
+void *data,
+void *ctx,
+gfp_t gfp)
+{
+ struct vring_virtqueue *vq = to_vvq(_vq);
+ struct vring_packed_desc *desc;
+ u16 id;
+ int err;
+
+ START_USE(vq);
+
+ /* check vq state and try to alloc desc for indirect. */
+ err = virtqueue_add_packed_prepare(vq, total_sg, data, ctx, , gfp);
+ if (err)
+ goto end;
+
+ id = vq->free_head;
+
+ if (desc) {
+ err = virtqueue_add_indirect_packed(vq, sgs, total_sg, out_sgs,
in_sgs, desc);
+ if (err)
+ goto err;
+ } else {
+ virtqueue_add_packed_vring(vq, sgs, total_sg, out_sgs, in_sgs);
+ vq->packed.desc_state[id].indir_desc = ctx;
+ }
+
+ vq->packed.desc_state[id].data = data;
+ vq->packed.desc_state[id].premapped = true;
+
+ goto end;
+
+err:
+ kfree(desc);
+
+end:
+ END_USE(vq);
+ return err;
+}
+
static bool virtqueue_kick_prepare_packed(struct virtqueue *_vq)
{
struct vring_virtqueue *vq = to_vvq(_vq);
@@ -1695,8 +1745,10 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
struct vring_desc_state_packed *state = NULL;
struct vring_packed_desc *desc;
unsigned int i, curr;
+ bool premapped;
state = >packed.desc_state[id];
+ premapped = state->premapped;
/* Clear data ptr. */
state->data = NULL;
@@ -1709,7 +1761,8 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
curr = id;
for (i = 0; i < state->num; i++) {
vring_unmap_extra_packed(vq,
->packed.desc_extra[curr]);
+>packed.desc_extra[curr],
+premapped);
curr = vq->packed.desc_extra[curr].next;
}
}
@@ -1722,7 +1775,7 @@ static void detach_buf_packed(struct vring_virtqueue *vq,
if (!desc)
return;
- if (vq->use_dma_api) {
+ if (vq->use_dma_api && !premapped) {
len = vq->packed.desc_extra[id].len;
[PATCH vhost 01/10] virtio_ring: split: refactor virtqueue_add_split() for premapped
DMA-related logic is separated from the virtqueue_add_split to prepare
for subsequent support for premapped.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 219 ---
1 file changed, 152 insertions(+), 67 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 41144b5246a8..560ee30d942c 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -520,29 +520,83 @@ static inline unsigned int
virtqueue_add_desc_split(struct virtqueue *vq,
return next;
}
-static inline int virtqueue_add_split(struct virtqueue *_vq,
- struct scatterlist *sgs[],
- unsigned int total_sg,
- unsigned int out_sgs,
- unsigned int in_sgs,
- void *data,
- void *ctx,
- gfp_t gfp)
+static int virtqueue_map_sgs(struct vring_virtqueue *vq,
+struct scatterlist *sgs[],
+unsigned int total_sg,
+unsigned int out_sgs,
+unsigned int in_sgs)
{
- struct vring_virtqueue *vq = to_vvq(_vq);
struct scatterlist *sg;
- struct vring_desc *desc;
- unsigned int i, n, avail, descs_used, prev, err_idx;
- int head;
- bool indirect;
+ unsigned int n;
- START_USE(vq);
+ for (n = 0; n < out_sgs; n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ dma_addr_t addr = vring_map_one_sg(vq, sg,
DMA_TO_DEVICE);
+
+ if (vring_mapping_error(vq, addr))
+ return -ENOMEM;
+
+ sg->dma_address = addr;
+ }
+ }
+ for (; n < (out_sgs + in_sgs); n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ dma_addr_t addr = vring_map_one_sg(vq, sg,
DMA_FROM_DEVICE);
+
+ if (vring_mapping_error(vq, addr))
+ return -ENOMEM;
+
+ sg->dma_address = addr;
+ }
+ }
+
+ return 0;
+}
+
+static void virtqueue_unmap_sgs(struct vring_virtqueue *vq,
+ struct scatterlist *sgs[],
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ unsigned int in_sgs)
+{
+ struct scatterlist *sg;
+ unsigned int n;
+
+ for (n = 0; n < out_sgs; n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ if (!sg->dma_address)
+ return;
+
+ dma_unmap_single(vring_dma_dev(vq), sg->dma_address,
+sg->length, DMA_TO_DEVICE);
+ }
+ }
+ for (; n < (out_sgs + in_sgs); n++) {
+ for (sg = sgs[n]; sg; sg = sg_next(sg)) {
+ if (!sg->dma_address)
+ return;
+
+ dma_unmap_single(vring_dma_dev(vq), sg->dma_address,
+sg->length, DMA_FROM_DEVICE);
+ }
+ }
+}
+
+static inline int virtqueue_add_split_prepare(struct vring_virtqueue *vq,
+ unsigned int total_sg,
+ unsigned int out_sgs,
+ void *data,
+ void *ctx,
+ gfp_t gfp,
+ struct vring_desc **pdesc)
+{
+ struct vring_desc *desc;
+ unsigned int descs_used;
BUG_ON(data == NULL);
BUG_ON(ctx && vq->indirect);
if (unlikely(vq->broken)) {
- END_USE(vq);
return -EIO;
}
@@ -550,27 +604,17 @@ static inline int virtqueue_add_split(struct virtqueue
*_vq,
BUG_ON(total_sg == 0);
- head = vq->free_head;
-
if (virtqueue_use_indirect(vq, total_sg))
- desc = alloc_indirect_split(_vq, total_sg, gfp);
+ desc = alloc_indirect_split(>vq, total_sg, gfp);
else {
desc = NULL;
WARN_ON_ONCE(total_sg > vq->split.vring.num && !vq->indirect);
}
- if (desc) {
- /* Use a single buffer which doesn't continue */
- indirect = true;
- /* Set up rest to use this indirect table. */
- i = 0;
+ if (desc)
descs_used = 1;
- } else {
- indirect = false;
- desc = vq->split.vring.desc;
- i = head;
+ else
descs_used =
[PATCH vhost 06/10] virtio_ring: introduce virtqueue_add_inbuf_premapped()
Introduce virtqueue_add_inbuf_premapped() to submit premapped sgs.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 25 +
include/linux/virtio.h | 5 +
2 files changed, 30 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 79244ccbae9e..cd9364eb2345 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -2452,6 +2452,31 @@ int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
}
EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_ctx);
+/**
+ * virtqueue_add_inbuf_premapped - expose input buffers to other end
+ * @vq: the struct virtqueue we're talking about.
+ * @sg: scatterlist (must be well-formed and terminated!)
+ * @num: the number of entries in @sg writable by other side
+ * @data: the token identifying the buffer.
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Caller must ensure we don't call this with other virtqueue operations
+ * at the same time (except where noted).
+ *
+ * It is required that all addrs have completed DMA operations. And use
+ * sg->dma_address, sg->length to pass addr and length.
+ *
+ * Returns zero or a negative error (ie. ENOSPC, ENOMEM, EIO).
+ */
+int virtqueue_add_inbuf_premapped(struct virtqueue *vq,
+ struct scatterlist *sg, unsigned int num,
+ void *data,
+ gfp_t gfp)
+{
+ return virtqueue_add_premapped(vq, , num, 0, 1, data, NULL, gfp);
+}
+EXPORT_SYMBOL_GPL(virtqueue_add_inbuf_premapped);
+
/**
* virtqueue_kick_prepare - first half of split virtqueue_kick call.
* @_vq: the struct virtqueue
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index d8b472a7dcae..3ebb346ebb7c 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -59,6 +59,11 @@ int virtqueue_add_inbuf_ctx(struct virtqueue *vq,
void *ctx,
gfp_t gfp);
+int virtqueue_add_inbuf_premapped(struct virtqueue *vq,
+ struct scatterlist *sg, unsigned int num,
+ void *data,
+ gfp_t gfp);
+
int virtqueue_add_sgs(struct virtqueue *vq,
struct scatterlist *sgs[],
unsigned int out_sgs,
--
2.32.0.3.g01195cf9f
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH vhost 03/10] virtio_ring: packed: refactor virtqueue_add_packed() for premapped
DMA-related logic is separated from virtqueue_add_packed to prepare for
the subsequent support for premapped.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 150 ++-
1 file changed, 78 insertions(+), 72 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 42b1ff87518e..47b6f9152f9f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1329,7 +1329,6 @@ static int virtqueue_add_indirect_packed(struct
vring_virtqueue *vq,
unsigned int total_sg,
unsigned int out_sgs,
unsigned int in_sgs,
-void *data,
struct vring_packed_desc *desc)
{
struct scatterlist *sg;
@@ -1345,14 +1344,9 @@ static int virtqueue_add_indirect_packed(struct
vring_virtqueue *vq,
for (n = 0; n < out_sgs + in_sgs; n++) {
for (sg = sgs[n]; sg; sg = sg_next(sg)) {
- addr = vring_map_one_sg(vq, sg, n < out_sgs ?
- DMA_TO_DEVICE : DMA_FROM_DEVICE);
- if (vring_mapping_error(vq, addr))
- goto unmap_release;
-
desc[i].flags = cpu_to_le16(n < out_sgs ?
0 : VRING_DESC_F_WRITE);
- desc[i].addr = cpu_to_le64(addr);
+ desc[i].addr = cpu_to_le64(sg->dma_address);
desc[i].len = cpu_to_le32(sg->length);
i++;
}
@@ -1363,7 +1357,7 @@ static int virtqueue_add_indirect_packed(struct
vring_virtqueue *vq,
total_sg * sizeof(struct vring_packed_desc),
DMA_TO_DEVICE);
if (vring_mapping_error(vq, addr))
- goto unmap_release;
+ return -ENOMEM;
vq->packed.vring.desc[head].addr = cpu_to_le64(addr);
vq->packed.vring.desc[head].len = cpu_to_le32(total_sg *
@@ -1404,53 +1398,30 @@ static int virtqueue_add_indirect_packed(struct
vring_virtqueue *vq,
/* Store token and indirect buffer state. */
vq->packed.desc_state[id].num = 1;
- vq->packed.desc_state[id].data = data;
vq->packed.desc_state[id].indir_desc = desc;
vq->packed.desc_state[id].last = id;
vq->num_added += 1;
pr_debug("Added buffer head %i to %p\n", head, vq);
- END_USE(vq);
return 0;
-
-unmap_release:
- err_idx = i;
-
- for (i = 0; i < err_idx; i++)
- vring_unmap_desc_packed(vq, [i]);
-
- kfree(desc);
-
- END_USE(vq);
- return -ENOMEM;
}
-static inline int virtqueue_add_packed(struct virtqueue *_vq,
- struct scatterlist *sgs[],
- unsigned int total_sg,
- unsigned int out_sgs,
- unsigned int in_sgs,
- void *data,
- void *ctx,
- gfp_t gfp)
+static inline int virtqueue_add_packed_prepare(struct vring_virtqueue *vq,
+ unsigned int total_sg,
+ void *data,
+ void *ctx,
+ struct vring_packed_desc **pdesc,
+ gfp_t gfp)
{
- struct vring_virtqueue *vq = to_vvq(_vq);
struct vring_packed_desc *desc;
- struct scatterlist *sg;
- unsigned int i, n, c, descs_used, err_idx;
- __le16 head_flags, flags;
- u16 head, id, prev, curr, avail_used_flags;
- int err;
-
- START_USE(vq);
+ unsigned int descs_used;
BUG_ON(data == NULL);
BUG_ON(ctx && vq->indirect);
if (unlikely(vq->broken)) {
- END_USE(vq);
return -EIO;
}
@@ -1458,39 +1429,55 @@ static inline int virtqueue_add_packed(struct virtqueue
*_vq,
BUG_ON(total_sg == 0);
+ desc = NULL;
+
if (virtqueue_use_indirect(vq, total_sg)) {
desc = alloc_indirect_packed(total_sg, gfp);
if (desc) {
if (unlikely(vq->vq.num_free < 1)) {
pr_debug("Can't add buf len 1 - avail = 0\n");
kfree(desc);
- END_USE(vq);
return -ENOSPC;
}
- return virtqueue_add_indirect_packed(vq, sgs, total_sg,
out_sgs,
-in_sgs, data,
desc);
+
[PATCH vhost 00/10] virtio core prepares for AF_XDP
XDP socket(AF_XDP) is an excellent bypass kernel network framework. The zero copy feature of xsk (XDP socket) needs to be supported by the driver. The performance of zero copy is very good. ENV: Qemu with vhost. vhost cpu | Guest APP CPU |Guest Softirq CPU | PPS -|---|--| xmit by sockperf: 90%| 100%| | 318967 xmit by xsk: 100% | 30% | 33%| 1192064 recv by sockperf: 100% | 68% | 100% | 692288 recv by xsk: 100% | 33% | 43%| 771670 Before achieving the function of Virtio-Net, we also have to let virtio core support these features: 1. virtio core support premapped 2. virtio core support reset per-queue 3. introduce DMA APIs to virtio core Please review. Thanks. Xuan Zhuo (10): virtio_ring: split: refactor virtqueue_add_split() for premapped virtio_ring: packed: separate prepare code from virtuque_add_indirect_packed() virtio_ring: packed: refactor virtqueue_add_packed() for premapped virtio_ring: split: introduce virtqueue_add_split_premapped() virtio_ring: packed: introduce virtqueue_add_packed_premapped() virtio_ring: introduce virtqueue_add_inbuf_premapped() virtio_ring: add api virtio_dma_map() for advance dma virtio_ring: introduce dma sync api for virtio virtio_ring: correct the expression of the description of virtqueue_resize() virtio_ring: introduce virtqueue_reset() drivers/virtio/virtio_ring.c | 792 --- include/linux/virtio.h | 29 ++ 2 files changed, 659 insertions(+), 162 deletions(-) -- 2.32.0.3.g01195cf9f ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH vhost 02/10] virtio_ring: packed: separate prepare code from virtuque_add_indirect_packed()
Separating the logic of allocating indirect desc and checking queue
status to the upper layer function.
The proposal of this is convenient to refactor virtqueue_add_packed()
for premapped.
Signed-off-by: Xuan Zhuo
---
drivers/virtio/virtio_ring.c | 29 -
1 file changed, 12 insertions(+), 17 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 560ee30d942c..42b1ff87518e 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -1330,25 +1330,14 @@ static int virtqueue_add_indirect_packed(struct
vring_virtqueue *vq,
unsigned int out_sgs,
unsigned int in_sgs,
void *data,
-gfp_t gfp)
+struct vring_packed_desc *desc)
{
- struct vring_packed_desc *desc;
struct scatterlist *sg;
unsigned int i, n, err_idx;
u16 head, id;
dma_addr_t addr;
head = vq->packed.next_avail_idx;
- desc = alloc_indirect_packed(total_sg, gfp);
- if (!desc)
- return -ENOMEM;
-
- if (unlikely(vq->vq.num_free < 1)) {
- pr_debug("Can't add buf len 1 - avail = 0\n");
- kfree(desc);
- END_USE(vq);
- return -ENOSPC;
- }
i = 0;
id = vq->free_head;
@@ -1470,11 +1459,17 @@ static inline int virtqueue_add_packed(struct virtqueue
*_vq,
BUG_ON(total_sg == 0);
if (virtqueue_use_indirect(vq, total_sg)) {
- err = virtqueue_add_indirect_packed(vq, sgs, total_sg, out_sgs,
- in_sgs, data, gfp);
- if (err != -ENOMEM) {
- END_USE(vq);
- return err;
+ desc = alloc_indirect_packed(total_sg, gfp);
+ if (desc) {
+ if (unlikely(vq->vq.num_free < 1)) {
+ pr_debug("Can't add buf len 1 - avail = 0\n");
+ kfree(desc);
+ END_USE(vq);
+ return -ENOSPC;
+ }
+
+ return virtqueue_add_indirect_packed(vq, sgs, total_sg,
out_sgs,
+in_sgs, data,
desc);
}
/* fall back on direct */
--
2.32.0.3.g01195cf9f
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vp_vdpa: fix the crash in hot unplug with vp_vdpa
On Tue, Feb 14, 2023 at 2:17 PM Cindy Lu wrote:
>
> While unplugging the vp_vdpa device, the kernel will crash
> The root cause is the function vp_modern_get_status() called following the
> vp_modern_remove().
This needs some tweaking, maybe it's better to say
vdpa_mgmtdev_unregister() will access modern devices which will cause
a use after free.
>So need to change the sequence in vp_vdpa_remove
>
> [ 195.016001] Call Trace:
Let's paste the full log with the reason for the calltrace (e.g
general protection fault or whatever else).
> [ 195.016233]
> [ 195.016434] vp_modern_get_status+0x12/0x20
> [ 195.016823] vp_vdpa_reset+0x1b/0x50 [vp_vdpa]
> [ 195.017238] virtio_vdpa_reset+0x3c/0x48 [virtio_vdpa]
> [ 195.017709] remove_vq_common+0x1f/0x3a0 [virtio_net]
> [ 195.018178] virtnet_remove+0x5d/0x70 [virtio_net]
> [ 195.018618] virtio_dev_remove+0x3d/0x90
> [ 195.018986] device_release_driver_internal+0x1aa/0x230
> [ 195.019466] bus_remove_device+0xd8/0x150
> [ 195.019841] device_del+0x18b/0x3f0
> [ 195.020167] ? kernfs_find_ns+0x35/0xd0
> [ 195.020526] device_unregister+0x13/0x60
> [ 195.020894] unregister_virtio_device+0x11/0x20
> [ 195.021311] device_release_driver_internal+0x1aa/0x230
> [ 195.021790] bus_remove_device+0xd8/0x150
> [ 195.022162] device_del+0x18b/0x3f0
> [ 195.022487] device_unregister+0x13/0x60
> [ 195.022852] ? vdpa_dev_remove+0x30/0x30 [vdpa]
> [ 195.023270] vp_vdpa_dev_del+0x12/0x20 [vp_vdpa]
> [ 195.023694] vdpa_match_remove+0x2b/0x40 [vdpa]
> [ 195.024115] bus_for_each_dev+0x78/0xc0
> [ 195.024471] vdpa_mgmtdev_unregister+0x65/0x80 [vdpa]
> [ 195.024937] vp_vdpa_remove+0x23/0x40 [vp_vdpa]
> [ 195.025353] pci_device_remove+0x36/0xa0
> [ 195.025719] device_release_driver_internal+0x1aa/0x230
> [ 195.026201] pci_stop_bus_device+0x6c/0x90
> [ 195.026580] pci_stop_and_remove_bus_device+0xe/0x20
> [ 195.027039] disable_slot+0x49/0x90
> [ 195.027366] acpiphp_disable_and_eject_slot+0x15/0x90
> [ 195.027832] hotplug_event+0xea/0x210
> [ 195.028171] ? hotplug_event+0x210/0x210
> [ 195.028535] acpiphp_hotplug_notify+0x22/0x80
> [ 195.028942] ? hotplug_event+0x210/0x210
> [ 195.029303] acpi_device_hotplug+0x8a/0x1d0
> [ 195.029690] acpi_hotplug_work_fn+0x1a/0x30
> [ 195.030077] process_one_work+0x1e8/0x3c0
> [ 195.030451] worker_thread+0x50/0x3b0
> [ 195.030791] ? rescuer_thread+0x3a0/0x3a0
> [ 195.031165] kthread+0xd9/0x100
> [ 195.031459] ? kthread_complete_and_exit+0x20/0x20
> [ 195.031899] ret_from_fork+0x22/0x30
> [ 195.032233]
>
> Fixes: ffbda8e9df10 ("vdpa/vp_vdpa : add vdpa tool support in vp_vdpa")
> Tested-by: Lei Yang
> Cc: sta...@vger.kernel.org
> Signed-off-by: Cindy Lu
Other than above,
Acked-by: Jason Wang
Thanks
> ---
> drivers/vdpa/virtio_pci/vp_vdpa.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/virtio_pci/vp_vdpa.c
> b/drivers/vdpa/virtio_pci/vp_vdpa.c
> index 8fe267ca3e76..281287fae89f 100644
> --- a/drivers/vdpa/virtio_pci/vp_vdpa.c
> +++ b/drivers/vdpa/virtio_pci/vp_vdpa.c
> @@ -645,8 +645,8 @@ static void vp_vdpa_remove(struct pci_dev *pdev)
> struct virtio_pci_modern_device *mdev = NULL;
>
> mdev = vp_vdpa_mgtdev->mdev;
> - vp_modern_remove(mdev);
> vdpa_mgmtdev_unregister(_vdpa_mgtdev->mgtdev);
> + vp_modern_remove(mdev);
> kfree(vp_vdpa_mgtdev->mgtdev.id_table);
> kfree(mdev);
> kfree(vp_vdpa_mgtdev);
> --
> 2.34.3
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vdpa/mlx5: should not activate virtq object when suspended
On Tue, Feb 14, 2023 at 6:48 AM Si-Wei Liu wrote:
>
> Otherwise the virtqueue object to instate could point to invalid address
> that was unmapped from the MTT:
>
> mlx5_core :41:04.2: mlx5_cmd_out_err:782:(pid 8321):
> CREATE_GENERAL_OBJECT(0xa00) op_mod(0xd) failed, status
> bad parameter(0x3), syndrome (0x5fa1c), err(-22)
>
> Fixes: cae15c2ed8e6 ("vdpa/mlx5: Implement susupend virtqueue callback")
> Cc: Eli Cohen
> Signed-off-by: Si-Wei Liu
>
> ---
> v2: removed the change for improving warning message
> ---
> drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 3a6dbbc6..d7e8ca0 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -165,6 +165,7 @@ struct mlx5_vdpa_net {
> u32 cur_num_vqs;
> u32 rqt_size;
> bool nb_registered;
> + bool suspended;
Any reason we make this net specific? (or is it better to use
mlx5_vdpa_dev structure?)
> struct notifier_block nb;
> struct vdpa_callback config_cb;
> struct mlx5_vdpa_wq_ent cvq_ent;
> @@ -2411,7 +2412,7 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev
> *mvdev,
> if (err)
> goto err_mr;
>
> - if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK))
> + if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK) || ndev->suspended)
> goto err_mr;
>
> restore_channels_info(ndev);
> @@ -2580,6 +2581,7 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
> mlx5_vdpa_destroy_mr(>mvdev);
> ndev->mvdev.status = 0;
> ndev->cur_num_vqs = 0;
> + ndev->suspended = false;
> ndev->mvdev.cvq.received_desc = 0;
> ndev->mvdev.cvq.completed_desc = 0;
> memset(ndev->event_cbs, 0, sizeof(*ndev->event_cbs) * (mvdev->max_vqs
> + 1));
> @@ -2815,6 +2817,8 @@ static int mlx5_vdpa_suspend(struct vdpa_device *vdev)
> struct mlx5_vdpa_virtqueue *mvq;
> int i;
>
> + mlx5_vdpa_info(mvdev, "suspending device\n");
> +
Is this better to show the info after the device has been suspended?
Thanks
> down_write(>reslock);
> ndev->nb_registered = false;
> mlx5_notifier_unregister(mvdev->mdev, >nb);
> @@ -2824,6 +2828,7 @@ static int mlx5_vdpa_suspend(struct vdpa_device *vdev)
> suspend_vq(ndev, mvq);
> }
> mlx5_vdpa_cvq_suspend(mvdev);
> + ndev->suspended = true;
> up_write(>reslock);
> return 0;
> }
> --
> 1.8.3.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost-vdpa: cleanup memory maps when closing vdpa fds
在 2023/1/31 22:53, Longpeng(Mike) 写道:
From: Longpeng
We must cleanup all memory maps when closing the vdpa fds, otherwise
some critical resources (e.g. memory, iommu map) will leaked if the
userspace exits unexpectedly (e.g. kill -9).
Sounds like a bug of the kernel, should we fix there?
Thanks
Signed-off-by: Longpeng
---
drivers/vhost/vdpa.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index a527eeeac637..37477cffa5aa 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -823,6 +823,18 @@ static void vhost_vdpa_unmap(struct vhost_vdpa *v,
vhost_vdpa_remove_as(v, asid);
}
+static void vhost_vdpa_clean_map(struct vhost_vdpa *v)
+{
+ struct vhost_vdpa_as *as;
+ u32 asid;
+
+ for (asid = 0; asid < v->vdpa->nas; asid++) {
+ as = asid_to_as(v, asid);
+ if (as)
+ vhost_vdpa_unmap(v, >iotlb, 0ULL, 0ULL - 1);
+ }
+}
+
static int vhost_vdpa_va_map(struct vhost_vdpa *v,
struct vhost_iotlb *iotlb,
u64 iova, u64 size, u64 uaddr, u32 perm)
@@ -1247,6 +1259,7 @@ static int vhost_vdpa_release(struct inode *inode, struct
file *filep)
vhost_vdpa_clean_irq(v);
vhost_vdpa_reset(v);
vhost_dev_stop(>vdev);
+ vhost_vdpa_clean_map(v);
vhost_vdpa_free_domain(v);
vhost_vdpa_config_put(v);
vhost_vdpa_cleanup(v);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [EXT] [RFC PATCH 0/4] PCI: endpoint: Introduce a virtio-net EP function
On 2023/02/08 1:02, Frank Li wrote: We project extending this module to support RDMA. The plan is based on virtio-rdma[1]. It extends the virtio-net and we are plan to implement the proposed spec based on this patch. [1] virtio-rdma - proposal: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k ernel.org%2Fall%2F20220511095900.343-1- xieyongji%40bytedance.com%2FT%2F=05%7C01%7Cfrank.li%40nxp.co m%7C0ef2bd62eda945c413be08db08f62ba3%7C686ea1d3bc2b4c6fa92cd99c5 c301635%7C0%7C0%7C638113625610341574%7CUnknown%7CTWFpbGZsb3d 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0% 3D%7C3000%7C%7C%7C=HyhpRTG8MNx%2BtfmWn6x3srmdBjHcZAo 2qbxL9USph9o%3D=0 - presentation on kvm forum: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fyout u.be%2FQrhv6hC_YK4=05%7C01%7Cfrank.li%40nxp.com%7C0ef2bd62 eda945c413be08db08f62ba3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% 7C0%7C638113625610341574%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7 C%7C%7C=ucOsGR1letTjxf0gKN6uls5y951CXaIspZtLGnASEC8%3D erved=0 Sorry for our outlook client always change link. This previous discussion. https://lore.kernel.org/imx/d098a631-9930-26d3-48f3-8f95386c8...@ti.com/T/#t Look like Endpoint maintainer Kishon like endpoint side work as vhost. Previous Haotian Wang submit similar patches, which just not use eDMA, just use memcpy. But overall idea is the same. I think your and haotian's method is more reasonable for PCI-RC EP connection. Kishon is not active recently. Maybe need Lorenzo Pieralisi and Bjorn helgass's comments for overall directions. I think so too. Thank you for your summarization. I've commented on the e-mail. Frank Li Please feel free to comment and suggest. Frank Li To realize the function, this patchset has few changes and introduces a new APIs to PCI EP framework related to virtio. Furthermore, it device depends on the some patchtes that is discussing. Those depended patchset are following: - [PATCH 1/2] dmaengine: dw-edma: Fix to change for continuous transfer link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k %2F=05%7C01%7Cfrank.li%40nxp.com%7C0ef2bd62eda945c413be08db 08f62ba3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381136256 10341574%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=d VZMaheX3eR1xA2wQtecmT857h2%2BFtUbhDSHXwgvsEY%3D=0 ernel.org%2Fdmaengine%2F20221223022608.550697-1- mie%40igel.co.jp%2F=05%7C01%7CFrank.Li%40nxp.com%7Cac57a62d4 10b458a5ba408db05ce0a4e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% 7C0%7C638110154722945380%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7 C%7C%7C=tIn0MHzEvrdxaC4KKTvTRvYXBzQ6MyrFa2GXpa3ePv0%3D& reserved=0 - [RFC PATCH 0/3] Deal with alignment restriction on EP side link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k %2F=05%7C01%7Cfrank.li%40nxp.com%7C0ef2bd62eda945c413be08db 08f62ba3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381136256 10341574%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=d VZMaheX3eR1xA2wQtecmT857h2%2BFtUbhDSHXwgvsEY%3D=0 ernel.org%2Flinux-pci%2F20230113090350.1103494-1- mie%40igel.co.jp%2F=05%7C01%7CFrank.Li%40nxp.com%7Cac57a62d4 10b458a5ba408db05ce0a4e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% 7C0%7C638110154722945380%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7 C%7C%7C=RLpnDiLwfqQd5QMXdiQyPVCkfOj8q2AyVeZOwWHvlsM%3 D=0 - [RFC PATCH v2 0/7] Introduce a vringh accessor for IO memory link: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k %2F=05%7C01%7Cfrank.li%40nxp.com%7C0ef2bd62eda945c413be08db 08f62ba3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6381136256 10341574%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoi V2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=d VZMaheX3eR1xA2wQtecmT857h2%2BFtUbhDSHXwgvsEY%3D=0 ernel.org%2Fvirtualization%2F20230202090934.549556-1- mie%40igel.co.jp%2F=05%7C01%7CFrank.Li%40nxp.com%7Cac57a62d4 10b458a5ba408db05ce0a4e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% 7C0%7C638110154722945380%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4 wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7 C%7C%7C=6jgY76BMSbvamb%2Fl3Urjt4Gcizeqon%2BZE5nPssc2kDA% 3D=0 About this patchset has 4 patches. The first of two patch is little changes to virtio. The third patch add APIs to easily access virtio data structure on PCIe Host side memory. The last one introduce a virtio-net EP device function. Details are in commit respectively. Currently those network devices are testd using ping only. I'll add a result of performance evaluation using iperf and etc to the future version of this patchset. Shunsuke Mie (4): virtio_pci: add a definition of queue flag in ISR virtio_ring: remove const from
Re: PCIe RC\EP virtio rdma solution discussion.
Thanks for organizing the discussion. On 2023/02/08 4:45, Frank Li wrote: From: Frank Li Recently more and more people are interested in PCI RC and EP connection, especially network usage cases. I upstreamed a vntb solution last year. But the transfer speed is not good enough. I initialized a discussion athttps://lore.kernel.org/imx/d098a631-9930-26d3-48f3-8f95386c8...@ti.com/T/#t I've investigated the vntb + ntbnet device that uses ntb transfer. Is it difficult to adapt the eDMA to the ntb transfer? It is likely one of the solutions for the performance problem. ┌─┐ ┌──┐ │ │ │ │ │ │ │ │ │ VirtQueue RX │ │ VirtQueue │ │ TX ┌──┐ │ │TX│ │ ┌─┐ │ │ │ │ ┌─┐ │ │ │ SRC LEN ├─┐ ┌──┤ │◄┼───┼─┤ SRC LEN │ │ │ ├─┤ │ │ │ │ │ │ ├─┤ │ │ │ │ │ │ │ │ │ │ │ │ │ │ ├─┤ │ │ │ │ │ │ ├─┤ │ │ │ │ │ │ │ │ │ │ │ │ │ │ └─┘ │ │ └──┘ │ │ └─┘ │ │ │ │ │ │ │ │ RX ┌───┼──┘ TX │ │RX│ │ ┌─┐ │ │ ┌──┐ │ │ ┌─┐ │ │ │ │◄┘ └►│ ├─┼───┼─┤ │ │ │ ├─┤ │ │ │ │ ├─┤ │ │ │ │ │ │ │ │ │ │ │ │ ├─┤ │ │ │ │ ├─┤ │ │ │ │ │ │ │ │ │ │ │ │ └─┘ │ │ │ │ └─┘ │ │ virtio_net └──┘ │ │ virtio_net │ │ Virtual PCI BUS EDMA Queue │ │ │ ├─┤ │ │ │ PCI EP Controller with eDMA│ │ PCI Host│ └─┘ └──┘ Basic idea is 1. Both EP and host probe virtio_net driver 2. There are two queues, one is the EP side(EQ), the other is the Host side. 3. EP side epf driver map Host side’s queue into EP’s space. Called HQ. 4. One working thread 5. pick one TX from EQ and RX from HQ, combine and generate EDMA requests, and put into the DMA TX queue. 6. Pick one RX from EQ and TX from HQ, combine and generate EDMA requests, and put into the DMA RX queue. 7. EDMA done irq will mark related item in EP and HQ finished. The whole transfer is zero copied and uses a DMA queue. The Shunsuke Mie implemented the above idea. https://lore.kernel.org/linux-pci/CANXvt5q_qgLuAfF7dxxrqUirT_Ld4B=pocq8jcb9uprvcgd...@mail.gmail.com/T/#t Similar solution posted at 2019, except use memcpy from/to PCI EP map windows. Using DMA should be simpler because EDMA can access the whole HOST\EP side memory space. https://lore.kernel.org/linux-pci/9f8e596f-b601-7f97-a98a-111763f96...@ti.com/T/ Solution 1 (Based on shunsuke): Both EP and Host side use virtio. Using EDMA to simplify data transfer and improve transfer speed. RDMA implement based on RoCE - proposal:https://lore.kernel.org/all/20220511095900.343-1-xieyon...@bytedance.com/T/ - presentation on kvm forum:https://youtu.be/Qrhv6hC_YK4 Solution 2(2020, Kishon) Previoushttps://lore.kernel.org/linux-pci/20200702082143.25259-1-kis...@ti.com/ EP side use vhost, RC side use virtio. I don’t think anyone works on this thread now. If using eDMA, it needs both sides to have a transfer queue. I don't know how to easily implement it on the vhost side. We had implemented this solution at the design stage of our proposal. This solution has to prepare a network device and register to the kernel from scratch for the endpoint. There is a lot of duplicated code, so we think the solution 1 is better, as Frank said. Solution 3(I am working on) Implement infiniband rdma driver at both EP and RC side. EP side build EDMA hardware queue based on EP/RC side’s send and receive queue and when eDMA finished, write status to complete queue for both EP/RC side. Use ipoib implement network transfer. The new InfiniBand device has to implement an InfiniBand network layer. I think it is overengineered for this peer-to-peer communication. In addition, the driver of the InfiniBand device should be implemented or emulate the existing InfiniBand device to use the upstream driver. We want to reduce the cost of implementation and maintenance. The whole upstream effort is quite huge for these. I don’t want to waste time and efforts because direction is wrong. I think Solution 1 is an easy path. Best, Shunsuke. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org
Re: [PATCH 06/33] virtio_ring: introduce virtqueue_reset()
On Mon, 13 Feb 2023 07:15:02 -0500, "Michael S. Tsirkin"
wrote:
> On Fri, Feb 03, 2023 at 05:09:12PM +0800, Xuan Zhuo wrote:
> > On Fri, 3 Feb 2023 04:05:38 -0500, "Michael S. Tsirkin"
> > wrote:
> > > On Thu, Feb 02, 2023 at 07:00:31PM +0800, Xuan Zhuo wrote:
> > > > Introduce virtqueue_reset() to release all buffer inside vq.
> > > >
> > > > Signed-off-by: Xuan Zhuo
> > > > ---
> > > > drivers/virtio/virtio_ring.c | 50
> > > > include/linux/virtio.h | 2 ++
> > > > 2 files changed, 52 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index e32046fd15a5..7dfce7001f9f 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -2735,6 +2735,56 @@ int virtqueue_resize(struct virtqueue *_vq, u32
> > > > num,
> > > > }
> > > > EXPORT_SYMBOL_GPL(virtqueue_resize);
> > > >
> > > > +/**
> > > > + * virtqueue_reset - reset the vring of vq
> > >
> > > ..., detach and recycle all unused buffers
> > >
> > > after all this is why we are doing this reset, right?
> > >
> > > > + * @_vq: the struct virtqueue we're talking about.
> > > > + * @recycle: callback for recycle the useless buffer
> > >
> > > not useless :) unused:
> > >
> > > callback to recycle unused buffers
> >
> >
> > I agree. Will fix.
> >
> > Thanks.
>
> Probably too late for this merge cycle then. Oh well.
It's ok for next.
I plan to push the virtio ring code to the vhost branch firstly.
Thanks.
>
>
> > >
> > > I know we have the same confusion in virtqueue_resize, I will fix
> > > that.
> > >
> > > > + *
> > > > + * Caller must ensure we don't call this with other virtqueue
> > > > operations
> > > > + * at the same time (except where noted).
> > > > + *
> > > > + * Returns zero or a negative error.
> > > > + * 0: success.
> > > > + * -EBUSY: Failed to sync with device, vq may not work properly
> > > > + * -ENOENT: Transport or device not supported
> > > > + * -EPERM: Operation not permitted
> > > > + */
> > > > +int virtqueue_reset(struct virtqueue *_vq,
> > > > + void (*recycle)(struct virtqueue *vq, void *buf))
> > > > +{
> > > > + struct vring_virtqueue *vq = to_vvq(_vq);
> > > > + struct virtio_device *vdev = vq->vq.vdev;
> > > > + void *buf;
> > > > + int err;
> > > > +
> > > > + if (!vq->we_own_ring)
> > > > + return -EPERM;
> > > > +
> > > > + if (!vdev->config->disable_vq_and_reset)
> > > > + return -ENOENT;
> > > > +
> > > > + if (!vdev->config->enable_vq_after_reset)
> > > > + return -ENOENT;
> > > > +
> > > > + err = vdev->config->disable_vq_and_reset(_vq);
> > > > + if (err)
> > > > + return err;
> > > > +
> > > > + while ((buf = virtqueue_detach_unused_buf(_vq)) != NULL)
> > > > + recycle(_vq, buf);
> > > > +
> > > > + if (vq->packed_ring)
> > > > + virtqueue_reinit_packed(vq);
> > > > + else
> > > > + virtqueue_reinit_split(vq);
> > > > +
> > > > + if (vdev->config->enable_vq_after_reset(_vq))
> > > > + return -EBUSY;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(virtqueue_reset);
> > > > +
> > > > /* Only available for split ring */
> > > > struct virtqueue *vring_new_virtqueue(unsigned int index,
> > > > unsigned int num,
> > > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> > > > index 3ebb346ebb7c..3ca2edb1aef3 100644
> > > > --- a/include/linux/virtio.h
> > > > +++ b/include/linux/virtio.h
> > > > @@ -105,6 +105,8 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue
> > > > *vq);
> > > >
> > > > int virtqueue_resize(struct virtqueue *vq, u32 num,
> > > > void (*recycle)(struct virtqueue *vq, void *buf));
> > > > +int virtqueue_reset(struct virtqueue *vq,
> > > > + void (*recycle)(struct virtqueue *vq, void *buf));
> > > >
> > > > /**
> > > > * struct virtio_device - representation of a device using virtio
> > > > --
> > > > 2.32.0.3.g01195cf9f
> > >
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 01/13] vdpa net: move iova tree creation from init to start
On 2/13/2023 3:14 AM, Eugenio Perez Martin wrote:
On Mon, Feb 13, 2023 at 7:51 AM Si-Wei Liu wrote:
On 2/8/2023 1:42 AM, Eugenio Pérez wrote:
Only create iova_tree if and when it is needed.
The cleanup keeps being responsible of last VQ but this change allows it
to merge both cleanup functions.
Signed-off-by: Eugenio Pérez
Acked-by: Jason Wang
---
net/vhost-vdpa.c | 99 ++--
1 file changed, 71 insertions(+), 28 deletions(-)
diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
index de5ed8ff22..a9e6c8f28e 100644
--- a/net/vhost-vdpa.c
+++ b/net/vhost-vdpa.c
@@ -178,13 +178,9 @@ err_init:
static void vhost_vdpa_cleanup(NetClientState *nc)
{
VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
-struct vhost_dev *dev = >vhost_net->dev;
qemu_vfree(s->cvq_cmd_out_buffer);
qemu_vfree(s->status);
-if (dev->vq_index + dev->nvqs == dev->vq_index_end) {
-g_clear_pointer(>vhost_vdpa.iova_tree, vhost_iova_tree_delete);
-}
if (s->vhost_net) {
vhost_net_cleanup(s->vhost_net);
g_free(s->vhost_net);
@@ -234,10 +230,64 @@ static ssize_t vhost_vdpa_receive(NetClientState *nc,
const uint8_t *buf,
return size;
}
+/** From any vdpa net client, get the netclient of first queue pair */
+static VhostVDPAState *vhost_vdpa_net_first_nc_vdpa(VhostVDPAState *s)
+{
+NICState *nic = qemu_get_nic(s->nc.peer);
+NetClientState *nc0 = qemu_get_peer(nic->ncs, 0);
+
+return DO_UPCAST(VhostVDPAState, nc, nc0);
+}
+
+static void vhost_vdpa_net_data_start_first(VhostVDPAState *s)
+{
+struct vhost_vdpa *v = >vhost_vdpa;
+
+if (v->shadow_vqs_enabled) {
+v->iova_tree = vhost_iova_tree_new(v->iova_range.first,
+ v->iova_range.last);
+}
+}
+
+static int vhost_vdpa_net_data_start(NetClientState *nc)
+{
+VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
+struct vhost_vdpa *v = >vhost_vdpa;
+
+assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
+
+if (v->index == 0) {
+vhost_vdpa_net_data_start_first(s);
+return 0;
+}
+
+if (v->shadow_vqs_enabled) {
+VhostVDPAState *s0 = vhost_vdpa_net_first_nc_vdpa(s);
+v->iova_tree = s0->vhost_vdpa.iova_tree;
+}
+
+return 0;
+}
+
+static void vhost_vdpa_net_client_stop(NetClientState *nc)
+{
+VhostVDPAState *s = DO_UPCAST(VhostVDPAState, nc, nc);
+struct vhost_dev *dev;
+
+assert(nc->info->type == NET_CLIENT_DRIVER_VHOST_VDPA);
+
+dev = s->vhost_vdpa.dev;
+if (dev->vq_index + dev->nvqs == dev->vq_index_end) {
+g_clear_pointer(>vhost_vdpa.iova_tree, vhost_iova_tree_delete);
+}
+}
+
static NetClientInfo net_vhost_vdpa_info = {
.type = NET_CLIENT_DRIVER_VHOST_VDPA,
.size = sizeof(VhostVDPAState),
.receive = vhost_vdpa_receive,
+.start = vhost_vdpa_net_data_start,
+.stop = vhost_vdpa_net_client_stop,
.cleanup = vhost_vdpa_cleanup,
.has_vnet_hdr = vhost_vdpa_has_vnet_hdr,
.has_ufo = vhost_vdpa_has_ufo,
@@ -351,7 +401,7 @@ dma_map_err:
static int vhost_vdpa_net_cvq_start(NetClientState *nc)
{
-VhostVDPAState *s;
+VhostVDPAState *s, *s0;
struct vhost_vdpa *v;
uint64_t backend_features;
int64_t cvq_group;
@@ -425,6 +475,15 @@ out:
return 0;
}
+s0 = vhost_vdpa_net_first_nc_vdpa(s);
+if (s0->vhost_vdpa.iova_tree) {
+/* SVQ is already configured for all virtqueues */
+v->iova_tree = s0->vhost_vdpa.iova_tree;
+} else {
+v->iova_tree = vhost_iova_tree_new(v->iova_range.first,
+ v->iova_range.last);
I wonder how this case could happen, vhost_vdpa_net_data_start_first()
should've allocated an iova tree on the first data vq. Is zero data vq
ever possible on net vhost-vdpa?
It's the case of the current qemu master when only CVQ is being
shadowed. It's not that "there are no data vq": If that case were
possible, CVQ vhost-vdpa state would be s0.
The case is that since only CVQ vhost-vdpa is the one being migrated,
only CVQ has an iova tree.
OK, so this corresponds to the case where live migration is not started
and CVQ starts in its own address space of VHOST_VDPA_NET_CVQ_ASID.
Thanks for explaining it!
With this series applied and with no migration running, the case is
the same as before: only SVQ gets shadowed. When migration starts, all
vqs are migrated, and share iova tree.
I wonder what is the reason to share the iova tree when migration
starts, I think CVQ may stay on its own VHOST_VDPA_NET_CVQ_ASID still?
Actually there's discrepancy in vhost_vdpa_net_log_global_enable(), I
don't see explicit code to switch from VHOST_VDPA_NET_CVQ_ASID to
VHOST_VDPA_GUEST_PA_ASID for the CVQ. This is the address space I
collision I mentioned earlier:
[PATCH v2] vdpa/mlx5: should not activate virtq object when suspended
Otherwise the virtqueue object to instate could point to invalid address
that was unmapped from the MTT:
mlx5_core :41:04.2: mlx5_cmd_out_err:782:(pid 8321):
CREATE_GENERAL_OBJECT(0xa00) op_mod(0xd) failed, status
bad parameter(0x3), syndrome (0x5fa1c), err(-22)
Fixes: cae15c2ed8e6 ("vdpa/mlx5: Implement susupend virtqueue callback")
Cc: Eli Cohen
Signed-off-by: Si-Wei Liu
---
v2: removed the change for improving warning message
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 3a6dbbc6..d7e8ca0 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -165,6 +165,7 @@ struct mlx5_vdpa_net {
u32 cur_num_vqs;
u32 rqt_size;
bool nb_registered;
+ bool suspended;
struct notifier_block nb;
struct vdpa_callback config_cb;
struct mlx5_vdpa_wq_ent cvq_ent;
@@ -2411,7 +2412,7 @@ static int mlx5_vdpa_change_map(struct mlx5_vdpa_dev
*mvdev,
if (err)
goto err_mr;
- if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK))
+ if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK) || ndev->suspended)
goto err_mr;
restore_channels_info(ndev);
@@ -2580,6 +2581,7 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
mlx5_vdpa_destroy_mr(>mvdev);
ndev->mvdev.status = 0;
ndev->cur_num_vqs = 0;
+ ndev->suspended = false;
ndev->mvdev.cvq.received_desc = 0;
ndev->mvdev.cvq.completed_desc = 0;
memset(ndev->event_cbs, 0, sizeof(*ndev->event_cbs) * (mvdev->max_vqs +
1));
@@ -2815,6 +2817,8 @@ static int mlx5_vdpa_suspend(struct vdpa_device *vdev)
struct mlx5_vdpa_virtqueue *mvq;
int i;
+ mlx5_vdpa_info(mvdev, "suspending device\n");
+
down_write(>reslock);
ndev->nb_registered = false;
mlx5_notifier_unregister(mvdev->mdev, >nb);
@@ -2824,6 +2828,7 @@ static int mlx5_vdpa_suspend(struct vdpa_device *vdev)
suspend_vq(ndev, mvq);
}
mlx5_vdpa_cvq_suspend(mvdev);
+ ndev->suspended = true;
up_write(>reslock);
return 0;
}
--
1.8.3.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vdpa/mlx5: should not activate virtq object when suspended
+ Eugenio
On 2/11/2023 10:55 PM, Eli Cohen wrote:
On 11/02/2023 8:08, Si-Wei Liu wrote:
Otherwise the virtqueue object to instate could point to invalid address
that was unmapped from the MTT:
mlx5_core :41:04.2: mlx5_cmd_out_err:782:(pid 8321):
CREATE_GENERAL_OBJECT(0xa00) op_mod(0xd) failed, status
bad parameter(0x3), syndrome (0x5fa1c), err(-22)
I am familiar with this issue but I think it should be handled at the
qemu level. This logic just hides the real problem. qemu provides the
iova and the VQs' addresses so it should not shrink the map while such
that VQ addresses are out of the iova range.
Here the whole device is already suspended, which should guarantee no
further DMA memory access will be initiated by the device. What is the
problem the mapping can't shrink in this case? Noted the code already
allows shrinking for the reset case (~VIRTIO_CONFIG_S_DRIVER_OK status
check), I see no essential difference the same couldn't be applied to
the suspend case. On the other hand, map shrinking works well with
platform IOMMU iommu_map/unmap() calls while vdpa device is suspended.
If your concern is that the device won't work with the shrunk map once
the device is to be RESUME'd, I think I agree that's what QEMU has to
guarantee, by either adding back the required mapping entries as needed,
or change VQ addresses during suspend that points to mapped IOVA
addresses. Or both can be done at the same time during suspend, with
that we can build a fast path to switch svq mode with on-chip IOMMU e.g.
mlx5_vdpa, rather than go through full device reset cycle.
While at it, add warning message to tell apart which object is
responsible for the CREATE_GENERAL_OBJECT command failure.
Fixes: cae15c2ed8e6 ("vdpa/mlx5: Implement susupend virtqueue callback")
Cc: Eli Cohen
Signed-off-by: Si-Wei Liu
---
drivers/vdpa/mlx5/net/mlx5_vnet.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 3a6dbbc6..c05c7f6 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -165,6 +165,7 @@ struct mlx5_vdpa_net {
u32 cur_num_vqs;
u32 rqt_size;
bool nb_registered;
+ bool suspended;
struct notifier_block nb;
struct vdpa_callback config_cb;
struct mlx5_vdpa_wq_ent cvq_ent;
@@ -1245,12 +1246,18 @@ static int setup_vq(struct mlx5_vdpa_net
*ndev, struct mlx5_vdpa_virtqueue *mvq)
goto err_connect;
err = counter_set_alloc(ndev, mvq);
- if (err)
+ if (err) {
+ mlx5_vdpa_warn(>mvdev, "failed to alloc counter on vq
idx %d(%d)\n",
+ idx, err);
Although you mention in the commit log, maybe it's better to have a
separate patch for improving warning messages.
Alright, I will move the improved warning message out of this patch.
This issue effectively blocks live migrating mlx5_vdpa device that has
to get merged asap. While others can post patches to improve warnings on
top.
Thanks,
-Siwei
goto err_counter;
+ }
err = create_virtqueue(ndev, mvq);
- if (err)
+ if (err) {
+ mlx5_vdpa_warn(>mvdev, "failed to create virtqueue idx
%d(%d)\n",
+ idx, err);
goto err_connect;
+ }
if (mvq->ready) {
err = modify_virtqueue(ndev, mvq,
MLX5_VIRTIO_NET_Q_OBJECT_STATE_RDY);
@@ -2411,7 +2418,7 @@ static int mlx5_vdpa_change_map(struct
mlx5_vdpa_dev *mvdev,
if (err)
goto err_mr;
- if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK))
+ if (!(mvdev->status & VIRTIO_CONFIG_S_DRIVER_OK) ||
ndev->suspended)
goto err_mr;
restore_channels_info(ndev);
@@ -2580,6 +2587,7 @@ static int mlx5_vdpa_reset(struct vdpa_device
*vdev)
mlx5_vdpa_destroy_mr(>mvdev);
ndev->mvdev.status = 0;
ndev->cur_num_vqs = 0;
+ ndev->suspended = false;
ndev->mvdev.cvq.received_desc = 0;
ndev->mvdev.cvq.completed_desc = 0;
memset(ndev->event_cbs, 0, sizeof(*ndev->event_cbs) *
(mvdev->max_vqs + 1));
@@ -2815,6 +2823,8 @@ static int mlx5_vdpa_suspend(struct vdpa_device
*vdev)
struct mlx5_vdpa_virtqueue *mvq;
int i;
+ mlx5_vdpa_info(mvdev, "suspending device\n");
+
down_write(>reslock);
ndev->nb_registered = false;
mlx5_notifier_unregister(mvdev->mdev, >nb);
@@ -2824,6 +2834,7 @@ static int mlx5_vdpa_suspend(struct vdpa_device
*vdev)
suspend_vq(ndev, mvq);
}
mlx5_vdpa_cvq_suspend(mvdev);
+ ndev->suspended = true;
up_write(>reslock);
return 0;
}
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 01/11] genirq/affinity:: Export irq_create_affinity_masks()
On Mon, Feb 13 2023 at 22:50, Yongji Xie wrote: > On Mon, Feb 13, 2023 at 8:00 PM Michael S. Tsirkin wrote: > I can try to split irq_create_affinity_masks() into a common part and > an irq specific part, and move the common part to a common dir such as > /lib and export it. Then we can use the common part to build a new API > for usage. https://lore.kernel.org/all/20221227022905.352674-1-ming@redhat.com/ Thanks, tglx ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v6 2/5] ACPI: bus: Add stub acpi_sleep_state_supported() in non-ACPI cases
On Fri, Feb 10, 2023 at 10:34 AM Saurabh Sengar
wrote:
>
> acpi_sleep_state_supported() is defined only when CONFIG_ACPI=y. The
> function is in acpi_bus.h, and acpi_bus.h can only be used in
> CONFIG_ACPI=y cases. Add the stub function to linux/acpi.h to make
> compilation successful for !CONFIG_ACPI cases.
>
> Signed-off-by: Saurabh Sengar
Acked-by: Rafael J. Wysocki
and please feel free to toute this patch whichever way is convenient.
Thanks!
> ---
> include/linux/acpi.h | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index efff750f326d..d331f76b0c19 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1075,6 +1075,11 @@ static inline u32 acpi_osc_ctx_get_cxl_control(struct
> acpi_osc_context *context)
> return 0;
> }
>
> +static inline bool acpi_sleep_state_supported(u8 sleep_state)
> +{
> + return false;
> +}
> +
> #endif /* !CONFIG_ACPI */
>
> #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC
> --
> 2.34.1
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 2/2] vdpa/mlx5: Initialize CVQ iotlb spinlock
On Mon, Feb 13, 2023 at 02:24:40PM +0200, Eli Cohen wrote:
>
> On 13/02/2023 14:19, Michael S. Tsirkin wrote:
> > On Mon, Feb 06, 2023 at 02:20:16PM +0200, Eli Cohen wrote:
> > > Initialize itolb spinlock.
> > >
> > > Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC
> > > setting")
> > > Signed-off-by: Eli Cohen
> > threading was broken here but whatevs.
> What exactly do you mean by "threading was broken". Is it because I sent two
> unrelated fixes together appearing as patch 1/2 and 2/2?
exactly. if I see 2/2 I expect a thread so I can find 1/2
and preferably a cover letter explaining what unifies all
these patches.
If they are unrelated no need to number them.
> >
> > > ---
> > > drivers/vdpa/mlx5/core/resources.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/vdpa/mlx5/core/resources.c
> > > b/drivers/vdpa/mlx5/core/resources.c
> > > index 45ad41287a31..d5a59c9035fb 100644
> > > --- a/drivers/vdpa/mlx5/core/resources.c
> > > +++ b/drivers/vdpa/mlx5/core/resources.c
> > > @@ -233,6 +233,7 @@ static int init_ctrl_vq(struct mlx5_vdpa_dev *mvdev)
> > > if (!mvdev->cvq.iotlb)
> > > return -ENOMEM;
> > > + spin_lock_init(>cvq.iommu_lock);
> > > vringh_set_iotlb(>cvq.vring, mvdev->cvq.iotlb,
> > > >cvq.iommu_lock);
> > > return 0;
> > > --
> > > 2.38.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] vhost/vdpa: Add MSI translation tables to iommu for software-managed MSI
On Tue, Feb 07, 2023 at 08:08:43PM +0800, Nanyong Sun wrote:
> From: Rong Wang
>
> Once enable iommu domain for one device, the MSI
> translation tables have to be there for software-managed MSI.
> Otherwise, platform with software-managed MSI without an
> irq bypass function, can not get a correct memory write event
> from pcie, will not get irqs.
> The solution is to obtain the MSI phy base address from
> iommu reserved region, and set it to iommu MSI cookie,
> then translation tables will be created while request irq.
>
> Change log
> --
>
> v1->v2:
> - add resv iotlb to avoid overlap mapping.
>
> Signed-off-by: Rong Wang
> Signed-off-by: Nanyong Sun
Could I get an ACK from IOMMU maintainers on exporting this pls?
> ---
> drivers/iommu/iommu.c | 1 +
> drivers/vhost/vdpa.c | 59 ---
> 2 files changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 5f6a85aea501..af9c064ad8b2 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2623,6 +2623,7 @@ void iommu_get_resv_regions(struct device *dev, struct
> list_head *list)
> if (ops->get_resv_regions)
> ops->get_resv_regions(dev, list);
> }
> +EXPORT_SYMBOL(iommu_get_resv_regions);
>
> /**
> * iommu_put_resv_regions - release resered regions
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index ec32f785dfde..a58979da8acd 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -49,6 +49,7 @@ struct vhost_vdpa {
> struct completion completion;
> struct vdpa_device *vdpa;
> struct hlist_head as[VHOST_VDPA_IOTLB_BUCKETS];
> + struct vhost_iotlb resv_iotlb;
> struct device dev;
> struct cdev cdev;
> atomic_t opened;
> @@ -216,6 +217,8 @@ static int vhost_vdpa_reset(struct vhost_vdpa *v)
>
> v->in_batch = 0;
>
> + vhost_iotlb_reset(>resv_iotlb);
> +
> return vdpa_reset(vdpa);
> }
>
> @@ -1013,6 +1016,10 @@ static int vhost_vdpa_process_iotlb_update(struct
> vhost_vdpa *v,
> msg->iova + msg->size - 1 > v->range.last)
> return -EINVAL;
>
> + if (vhost_iotlb_itree_first(>resv_iotlb, msg->iova,
> + msg->iova + msg->size - 1))
> + return -EINVAL;
> +
> if (vhost_iotlb_itree_first(iotlb, msg->iova,
> msg->iova + msg->size - 1))
> return -EEXIST;
> @@ -1103,6 +1110,45 @@ static ssize_t vhost_vdpa_chr_write_iter(struct kiocb
> *iocb,
> return vhost_chr_write_iter(dev, from);
> }
>
> +static int vhost_vdpa_resv_iommu_region(struct iommu_domain *domain, struct
> device *dma_dev,
> + struct vhost_iotlb *resv_iotlb)
> +{
> + struct list_head dev_resv_regions;
> + phys_addr_t resv_msi_base = 0;
> + struct iommu_resv_region *region;
> + int ret = 0;
> + bool with_sw_msi = false;
> + bool with_hw_msi = false;
> +
> + INIT_LIST_HEAD(_resv_regions);
> + iommu_get_resv_regions(dma_dev, _resv_regions);
> +
> + list_for_each_entry(region, _resv_regions, list) {
> + ret = vhost_iotlb_add_range_ctx(resv_iotlb, region->start,
> + region->start + region->length - 1,
> + 0, 0, NULL);
> + if (ret) {
> + vhost_iotlb_reset(resv_iotlb);
> + break;
> + }
> +
> + if (region->type == IOMMU_RESV_MSI)
> + with_hw_msi = true;
> +
> + if (region->type == IOMMU_RESV_SW_MSI) {
> + resv_msi_base = region->start;
> + with_sw_msi = true;
> + }
> + }
> +
> + if (!ret && !with_hw_msi && with_sw_msi)
> + ret = iommu_get_msi_cookie(domain, resv_msi_base);
> +
> + iommu_put_resv_regions(dma_dev, _resv_regions);
> +
> + return ret;
> +}
> +
> static int vhost_vdpa_alloc_domain(struct vhost_vdpa *v)
> {
> struct vdpa_device *vdpa = v->vdpa;
> @@ -1128,11 +1174,16 @@ static int vhost_vdpa_alloc_domain(struct vhost_vdpa
> *v)
>
> ret = iommu_attach_device(v->domain, dma_dev);
> if (ret)
> - goto err_attach;
> + goto err_alloc_domain;
>
> - return 0;
> + ret = vhost_vdpa_resv_iommu_region(v->domain, dma_dev, >resv_iotlb);
> + if (ret)
> + goto err_attach_device;
>
> -err_attach:
> + return 0;
> +err_attach_device:
> + iommu_detach_device(v->domain, dma_dev);
> +err_alloc_domain:
> iommu_domain_free(v->domain);
> return ret;
> }
> @@ -1385,6 +1436,8 @@ static int vhost_vdpa_probe(struct vdpa_device *vdpa)
> goto err;
> }
>
> + vhost_iotlb_init(>resv_iotlb, 0, 0);
> +
> r = dev_set_name(>dev, "vhost-vdpa-%u", minor);
> if (r)
> goto err;
Jason any feedback on vdpa change here?
> --
>
Re: [PATCH 2/2] vdpa/mlx5: Initialize CVQ iotlb spinlock
On Mon, Feb 06, 2023 at 02:20:16PM +0200, Eli Cohen wrote:
> Initialize itolb spinlock.
>
> Fixes: 5262912ef3cf ("vdpa/mlx5: Add support for control VQ and MAC setting")
> Signed-off-by: Eli Cohen
threading was broken here but whatevs.
> ---
> drivers/vdpa/mlx5/core/resources.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/vdpa/mlx5/core/resources.c
> b/drivers/vdpa/mlx5/core/resources.c
> index 45ad41287a31..d5a59c9035fb 100644
> --- a/drivers/vdpa/mlx5/core/resources.c
> +++ b/drivers/vdpa/mlx5/core/resources.c
> @@ -233,6 +233,7 @@ static int init_ctrl_vq(struct mlx5_vdpa_dev *mvdev)
> if (!mvdev->cvq.iotlb)
> return -ENOMEM;
>
> + spin_lock_init(>cvq.iommu_lock);
> vringh_set_iotlb(>cvq.vring, mvdev->cvq.iotlb,
> >cvq.iommu_lock);
>
> return 0;
> --
> 2.38.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 06/33] virtio_ring: introduce virtqueue_reset()
On Fri, Feb 03, 2023 at 05:09:12PM +0800, Xuan Zhuo wrote:
> On Fri, 3 Feb 2023 04:05:38 -0500, "Michael S. Tsirkin"
> wrote:
> > On Thu, Feb 02, 2023 at 07:00:31PM +0800, Xuan Zhuo wrote:
> > > Introduce virtqueue_reset() to release all buffer inside vq.
> > >
> > > Signed-off-by: Xuan Zhuo
> > > ---
> > > drivers/virtio/virtio_ring.c | 50
> > > include/linux/virtio.h | 2 ++
> > > 2 files changed, 52 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index e32046fd15a5..7dfce7001f9f 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -2735,6 +2735,56 @@ int virtqueue_resize(struct virtqueue *_vq, u32
> > > num,
> > > }
> > > EXPORT_SYMBOL_GPL(virtqueue_resize);
> > >
> > > +/**
> > > + * virtqueue_reset - reset the vring of vq
> >
> > ..., detach and recycle all unused buffers
> >
> > after all this is why we are doing this reset, right?
> >
> > > + * @_vq: the struct virtqueue we're talking about.
> > > + * @recycle: callback for recycle the useless buffer
> >
> > not useless :) unused:
> >
> > callback to recycle unused buffers
>
>
> I agree. Will fix.
>
> Thanks.
Probably too late for this merge cycle then. Oh well.
> >
> > I know we have the same confusion in virtqueue_resize, I will fix
> > that.
> >
> > > + *
> > > + * Caller must ensure we don't call this with other virtqueue operations
> > > + * at the same time (except where noted).
> > > + *
> > > + * Returns zero or a negative error.
> > > + * 0: success.
> > > + * -EBUSY: Failed to sync with device, vq may not work properly
> > > + * -ENOENT: Transport or device not supported
> > > + * -EPERM: Operation not permitted
> > > + */
> > > +int virtqueue_reset(struct virtqueue *_vq,
> > > + void (*recycle)(struct virtqueue *vq, void *buf))
> > > +{
> > > + struct vring_virtqueue *vq = to_vvq(_vq);
> > > + struct virtio_device *vdev = vq->vq.vdev;
> > > + void *buf;
> > > + int err;
> > > +
> > > + if (!vq->we_own_ring)
> > > + return -EPERM;
> > > +
> > > + if (!vdev->config->disable_vq_and_reset)
> > > + return -ENOENT;
> > > +
> > > + if (!vdev->config->enable_vq_after_reset)
> > > + return -ENOENT;
> > > +
> > > + err = vdev->config->disable_vq_and_reset(_vq);
> > > + if (err)
> > > + return err;
> > > +
> > > + while ((buf = virtqueue_detach_unused_buf(_vq)) != NULL)
> > > + recycle(_vq, buf);
> > > +
> > > + if (vq->packed_ring)
> > > + virtqueue_reinit_packed(vq);
> > > + else
> > > + virtqueue_reinit_split(vq);
> > > +
> > > + if (vdev->config->enable_vq_after_reset(_vq))
> > > + return -EBUSY;
> > > +
> > > + return 0;
> > > +}
> > > +EXPORT_SYMBOL_GPL(virtqueue_reset);
> > > +
> > > /* Only available for split ring */
> > > struct virtqueue *vring_new_virtqueue(unsigned int index,
> > > unsigned int num,
> > > diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> > > index 3ebb346ebb7c..3ca2edb1aef3 100644
> > > --- a/include/linux/virtio.h
> > > +++ b/include/linux/virtio.h
> > > @@ -105,6 +105,8 @@ dma_addr_t virtqueue_get_used_addr(struct virtqueue
> > > *vq);
> > >
> > > int virtqueue_resize(struct virtqueue *vq, u32 num,
> > >void (*recycle)(struct virtqueue *vq, void *buf));
> > > +int virtqueue_reset(struct virtqueue *vq,
> > > + void (*recycle)(struct virtqueue *vq, void *buf));
> > >
> > > /**
> > > * struct virtio_device - representation of a device using virtio
> > > --
> > > 2.32.0.3.g01195cf9f
> >
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 00/33] virtio-net: support AF_XDP zero copy
On Mon, Feb 06, 2023 at 10:41:16AM +0800, Xuan Zhuo wrote: > On Fri, 3 Feb 2023 04:17:59 -0500, "Michael S. Tsirkin" > wrote: > > On Fri, Feb 03, 2023 at 11:33:31AM +0800, Xuan Zhuo wrote: > > > On Thu, 02 Feb 2023 15:41:44 +0100, Paolo Abeni wrote: > > > > On Thu, 2023-02-02 at 19:00 +0800, Xuan Zhuo wrote: > > > > > XDP socket(AF_XDP) is an excellent bypass kernel network framework. > > > > > The zero > > > > > copy feature of xsk (XDP socket) needs to be supported by the driver. > > > > > The > > > > > performance of zero copy is very good. mlx5 and intel ixgbe already > > > > > support > > > > > this feature, This patch set allows virtio-net to support xsk's > > > > > zerocopy xmit > > > > > feature. > > > > > > > > > > Virtio-net did not support per-queue reset, so it was impossible to > > > > > support XDP > > > > > Socket Zerocopy. At present, we have completed the work of Virtio > > > > > Spec and > > > > > Kernel in Per-Queue Reset. It is time for Virtio-Net to complete the > > > > > support for > > > > > the XDP Socket Zerocopy. > > > > > > > > > > Virtio-net can not increase the queue at will, so xsk shares the > > > > > queue with > > > > > kernel. > > > > > > > > > > On the other hand, Virtio-Net does not support generate interrupt > > > > > manually, so > > > > > when we wakeup tx xmit, we used some tips. If the CPU run by TX NAPI > > > > > last time > > > > > is other CPUs, use IPI to wake up NAPI on the remote CPU. If it is > > > > > also the > > > > > local CPU, then we wake up sofrirqd. > > > > > > > > Thank you for the large effort. > > > > > > > > Since this will likely need a few iterations, on next revision please > > > > do split the work in multiple chunks to help the reviewer efforts - > > > > from Documentation/process/maintainer-netdev.rst: > > > > > > > > - don't post large series (> 15 patches), break them up > > > > > > > > In this case I guess you can split it in 1 (or even 2) pre-req series > > > > and another one for the actual xsk zero copy support. > > > > > > > > > OK. > > > > > > I can split patch into multiple parts such as > > > > > > * virtio core > > > * xsk > > > * virtio-net prepare > > > * virtio-net support xsk zerocopy > > > > > > However, there is a problem, the virtio core part should enter the VHOST > > > branch > > > of Michael. Then, should I post follow-up patches to which branch vhost or > > > next-next? > > > > > > Thanks. > > > > > > > Here are some ideas on how to make the patchset smaller > > and easier to merge: > > - keep everything in virtio_net.c for now. We can split > > things out later, but this way your patchset will not > > conflict with every since change merged meanwhile. > > Also, split up needs to be done carefully with sane > > APIs between components, let's maybe not waste time > > on that now, do the split-up later. > > - you have patches that add APIs then other > > patches use them. as long as it's only virtio net just > > add and use in a single patch, review is actually easier this way. > > I will try to merge #16-#18 and #20-#23. don't do the code reorg thing for now either. leave this for later. > > > - we can try merging pre-requisites earlier, then patchset > > size will shrink. > > Do you mean the patches of virtio core? Should we put these > patches to vhost branch? > > Thanks. I can merge patches 1-8, yes. This patchset probably missed the merge window anyway. > > > > > > > > > > > > Thanks! > > > > > > > > Paolo > > > > > > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2] scsi: virtio_scsi: Fix poential NULL pointer dereference in virtscsi_rescan_hotunplug
On Thu, Feb 02, 2023 at 02:41:24PM +0800, Zheng Wang wrote:
> There is no check about the return value of kmalloc in
> virtscsi_rescan_hotunplug. Add the check to avoid use
> of null pointer 'inq_result' in case of the failure
> of kmalloc.
>
> Signed-off-by: Zheng Wang
> ---
I fixed a typo in subject and tweaked the patch a bit
> v2:
> - add kfree to avoid memory leak
> ---
> drivers/scsi/virtio_scsi.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index d07d24c06b54..a66d8815d738 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -330,7 +330,7 @@ static void virtscsi_handle_param_change(struct
> virtio_scsi *vscsi,
> scsi_device_put(sdev);
> }
>
> -static void virtscsi_rescan_hotunplug(struct virtio_scsi *vscsi)
> +static int virtscsi_rescan_hotunplug(struct virtio_scsi *vscsi)
> {
> struct scsi_device *sdev;
> struct Scsi_Host *shost = virtio_scsi_host(vscsi->vdev);
> @@ -338,6 +338,11 @@ static void virtscsi_rescan_hotunplug(struct virtio_scsi
> *vscsi)
> int result, inquiry_len, inq_result_len = 256;
> char *inq_result = kmalloc(inq_result_len, GFP_KERNEL);
>
> + if (!inq_result) {
> + kfree(inq_result);
> + return -ENOMEM;
> + }
> +
> shost_for_each_device(sdev, shost) {
> inquiry_len = sdev->inquiry_len ? sdev->inquiry_len : 36;
>
> @@ -366,6 +371,7 @@ static void virtscsi_rescan_hotunplug(struct virtio_scsi
> *vscsi)
> }
>
> kfree(inq_result);
> + return 0;
> }
>
> static void virtscsi_handle_event(struct work_struct *work)
> @@ -374,12 +380,15 @@ static void virtscsi_handle_event(struct work_struct
> *work)
> container_of(work, struct virtio_scsi_event_node, work);
> struct virtio_scsi *vscsi = event_node->vscsi;
> struct virtio_scsi_event *event = _node->event;
> + int ret = 0;
>
dropped = 0 here
> if (event->event &
> cpu_to_virtio32(vscsi->vdev, VIRTIO_SCSI_T_EVENTS_MISSED)) {
and moved declaration here.
> event->event &= ~cpu_to_virtio32(vscsi->vdev,
> VIRTIO_SCSI_T_EVENTS_MISSED);
> - virtscsi_rescan_hotunplug(vscsi);
> + ret = virtscsi_rescan_hotunplug(vscsi);
> + if (ret)
> + return;
> scsi_scan_host(virtio_scsi_host(vscsi->vdev));
> }
>
> --
> 2.25.1
>
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v3 2/2] vdpasim: support doorbell mapping
On Sun, Jan 29, 2023 at 02:19:51PM +0800, Jason Wang wrote:
> On Sun, Jan 29, 2023 at 10:51 AM Longpeng(Mike) wrote:
> >
> > From: Longpeng
> >
> > Support doorbell mapping for vdpasim devices, then we can test the notify
> > passthrough feature even if there's no real hardware on hand.
> >
> > Allocates a dummy page which is used to emulate the notify page of the
> > device,
> > all VQs share the same notify register that initiated to 0x. A
> > periodic
> > work will check whether there're requests need to process ( the value of the
> > notify register is 0x or not ).
> > ---
> > drivers/vdpa/vdpa_sim/vdpa_sim.c | 65
> > drivers/vdpa/vdpa_sim/vdpa_sim.h | 3 ++
> > 2 files changed, 68 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > index b071f0d842fb..4fcfeb6e2fb8 100644
> > --- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > +++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
> > @@ -39,6 +39,8 @@ MODULE_PARM_DESC(max_iotlb_entries,
> > #define VDPASIM_QUEUE_ALIGN PAGE_SIZE
> > #define VDPASIM_QUEUE_MAX 256
> > #define VDPASIM_VENDOR_ID 0
> > +#define VDPASIM_VRING_POLL_PERIOD 100 /* ms */
> > +#define VDPASIM_NOTIFY_DEFVAL 0x
> >
> > static struct vdpasim *vdpa_to_sim(struct vdpa_device *vdpa)
> > {
> > @@ -246,6 +248,28 @@ static const struct dma_map_ops vdpasim_dma_ops = {
> > static const struct vdpa_config_ops vdpasim_config_ops;
> > static const struct vdpa_config_ops vdpasim_batch_config_ops;
> >
> > +static void vdpasim_notify_work(struct work_struct *work)
> > +{
> > + struct vdpasim *vdpasim;
> > + u16 *val;
> > +
> > + vdpasim = container_of(work, struct vdpasim, notify_work.work);
> > +
> > + if (!(vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK))
> > + goto out;
> > +
> > + if (!vdpasim->running)
> > + goto out;
> > +
> > + val = (u16 *)vdpasim->notify;
> > + if (xchg(val, VDPASIM_NOTIFY_DEFVAL) != VDPASIM_NOTIFY_DEFVAL)
> > + schedule_work(>work);
> > +
> > +out:
> > + schedule_delayed_work(>notify_work,
> > + msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD));
> > +}
> > +
> > struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr,
> >const struct vdpa_dev_set_config *config)
> > {
> > @@ -287,6 +311,13 @@ struct vdpasim *vdpasim_create(struct vdpasim_dev_attr
> > *dev_attr,
> > set_dma_ops(dev, _dma_ops);
> > vdpasim->vdpa.mdev = dev_attr->mgmt_dev;
> >
> > + INIT_DELAYED_WORK(>notify_work, vdpasim_notify_work);
> > +
> > + vdpasim->notify = __get_free_page(GFP_KERNEL | __GFP_ZERO);
> > + if (!vdpasim->notify)
> > + goto err_iommu;
>
> We can simply avoid the advertising notification area in this case.
>
> > + *(u16 *)vdpasim->notify = VDPASIM_NOTIFY_DEFVAL;
>
> WRITE_ONCE()?
it is just initialization so it should not matter.
> > +
> > vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL);
> > if (!vdpasim->config)
> > goto err_iommu;
> > @@ -498,16 +529,21 @@ static u8 vdpasim_get_status(struct vdpa_device *vdpa)
> > static void vdpasim_set_status(struct vdpa_device *vdpa, u8 status)
> > {
> > struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> > + bool started = vdpasim->status & VIRTIO_CONFIG_S_DRIVER_OK;
>
> Do we need to do the check under the vdpasim->lock?
>
> >
> > spin_lock(>lock);
> > vdpasim->status = status;
> > spin_unlock(>lock);
> > + if (!started && (status & VIRTIO_CONFIG_S_DRIVER_OK))
> > + schedule_delayed_work(>notify_work,
> > +
> > msecs_to_jiffies(VDPASIM_VRING_POLL_PERIOD));
> > }
> >
> > static int vdpasim_reset(struct vdpa_device *vdpa)
> > {
> > struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> >
> > + cancel_delayed_work_sync(>notify_work);
>
> Do we need to do this after setting running to zero? Otherwise it's racy.
>
> Thanks
>
> > spin_lock(>lock);
> > vdpasim->status = 0;
> > vdpasim_do_reset(vdpasim);
> > @@ -672,11 +708,34 @@ static int vdpasim_dma_unmap(struct vdpa_device
> > *vdpa, unsigned int asid,
> > return 0;
> > }
> >
> > +static pgprot_t vdpasim_get_vq_notification_pgprot(struct vdpa_device
> > *vdpa,
> > + u16 qid, pgprot_t prot)
> > +{
> > + /*
> > +* We use normal RAM pages to emulate the vq notification area, so
> > +* just keep the pgprot as it mmaped.
> > +*/
> > + return prot;
> > +}
> > +
> > +static struct vdpa_notification_area
> > +vdpasim_get_vq_notification(struct vdpa_device *vdpa, u16 qid)
> > +{
> > + struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
> > + struct vdpa_notification_area notify;
> > +
> > + notify.addr = virt_to_phys((void
Re: [PATCH 2/2] vringh: fetch used_idx from vring at vringh_init_iotlb
On Tue, Jan 31, 2023 at 08:58:37AM +0100, Eugenio Perez Martin wrote:
> On Tue, Jan 31, 2023 at 4:16 AM Jason Wang wrote:
> >
> > On Tue, Jan 31, 2023 at 12:39 AM Eugenio Perez Martin
> > wrote:
> > >
> > > On Sun, Jan 29, 2023 at 7:01 AM Jason Wang wrote:
> > > >
> > > > On Thu, Jan 19, 2023 at 4:11 PM Eugenio Perez Martin
> > > > wrote:
> > > > >
> > > > > On Thu, Jan 19, 2023 at 4:20 AM Jason Wang
> > > > > wrote:
> > > > > >
> > > > > > On Thu, Jan 19, 2023 at 12:44 AM Eugenio Pérez
> > > > > > wrote:
> > > > > > >
> > > > > > > Starting from an used_idx different than 0 is needed in use cases
> > > > > > > like
> > > > > > > virtual machine migration. Not doing so and letting the caller
> > > > > > > set an
> > > > > > > avail idx different than 0 causes destination device to try to
> > > > > > > use old
> > > > > > > buffers that source driver already recover and are not available
> > > > > > > anymore.
> > > > > > >
> > > > > > > While callers like vdpa_sim set avail_idx directly it does not set
> > > > > > > used_idx. Instead of let the caller do the assignment, fetch it
> > > > > > > from
> > > > > > > the guest at initialization like vhost-kernel do.
> > > > > > >
> > > > > > > To perform the same at vring_kernel_init and vring_user_init is
> > > > > > > left for
> > > > > > > the future.
> > > > > > >
> > > > > > > Signed-off-by: Eugenio Pérez
> > > > > > > ---
> > > > > > > drivers/vhost/vringh.c | 25 +++--
> > > > > > > 1 file changed, 23 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > > > > > > index 33eb941fcf15..0eed825197f2 100644
> > > > > > > --- a/drivers/vhost/vringh.c
> > > > > > > +++ b/drivers/vhost/vringh.c
> > > > > > > @@ -1301,6 +1301,17 @@ static inline int putused_iotlb(const
> > > > > > > struct vringh *vrh,
> > > > > > > return 0;
> > > > > > > }
> > > > > > >
> > > > > > > +/**
> > > > > > > + * vringh_update_used_idx - fetch used idx from driver's used
> > > > > > > split vring
> > > > > > > + * @vrh: The vring.
> > > > > > > + *
> > > > > > > + * Returns -errno or 0.
> > > > > > > + */
> > > > > > > +static inline int vringh_update_used_idx(struct vringh *vrh)
> > > > > > > +{
> > > > > > > + return getu16_iotlb(vrh, >last_used_idx,
> > > > > > > >vring.used->idx);
> > > > > > > +}
> > > > > > > +
> > > > > > > /**
> > > > > > > * vringh_init_iotlb - initialize a vringh for a ring with IOTLB.
> > > > > > > * @vrh: the vringh to initialize.
> > > > > > > @@ -1319,8 +1330,18 @@ int vringh_init_iotlb(struct vringh *vrh,
> > > > > > > u64 features,
> > > > > > > struct vring_avail *avail,
> > > > > > > struct vring_used *used)
> > > > > > > {
> > > > > >
> > > > > > While at this, I wonder if it's better to have a dedicated parameter
> > > > > > for last_avail_idx?
> > > > > >
> > > > >
> > > > > I also had that thought. To directly assign last_avail_idx is not a
> > > > > specially elegant API IMO.
> > > > >
> > > > > Maybe expose a way to fetch used_idx from device vring and pass
> > > > > used_idx as parameter too?
> > > >
> > > > If I was not wrong, we can start from last_avail_idx, for used_idx it
> > > > is only needed for inflight descriptors which might require other
> > > > APIs?
> > > >
> > > > (All the current vDPA user of vringh is doing in order processing)
> > > >
> > >
> > > That was actually my first attempt and it works equally well for the
> > > moment, but it diverges from vhost-kernel behavior for little benefit.
> > >
> > > To assign both values at set_vring_base mean that if vDPA introduces
> > > an (hypothetical) VHOST_VDPA_F_INFLIGHT backend feature in the future,
> > > the initialization process would vary a lot:
> > > * Without that feature, the used_idx starts with 0, and the avail one
> > > is 0 or whatever value the user set with vring_set_base.
> > > * With that feature, the device will read guest's used_idx as
> > > vhost-kernel? We would enable a new ioctl to set it, or expand
> > > set_base to include used_idx, effectively diverting from vhost-kernel?
> >
> > Adding Longpeng who is looking at this.
> >
>
> Sorry, I'll CC longpe...@huawei.com in future series like this one.
>
> > We can leave this to the caller to decide.
> >
> > Btw, a question, at which case the device used index does not equal to
> > the used index in the vring when the device is suspended? I think the
> > correct way is to flush the pending used indexes before suspending.
> > Otherwise we need an API to get pending used indices?
> >
> > >
> > > To me the wisest option is to move this with vhost-kernel. Maybe we
> > > need to add a feature bit to know that the hypervisor can trust the
> > > device will do "the right thing" (VHOST_VDPA_F_FETCH_USED_AT_ENABLE?),
> > > but we should keep it orthogonal to inflight descriptor migration in
> > > my opinion.
> >
> > I think we need to understand if
Re: [PATCH v2 01/11] genirq/affinity:: Export irq_create_affinity_masks()
On Mon, Jan 30, 2023 at 07:53:55PM +0800, Yongji Xie wrote: > On Fri, Jan 27, 2023 at 4:22 PM Michael S. Tsirkin wrote: > > > > On Mon, Dec 19, 2022 at 05:36:02PM +0800, Yongji Xie wrote: > > > On Mon, Dec 19, 2022 at 3:33 PM Michael S. Tsirkin > > > wrote: > > > > > > > > On Mon, Dec 05, 2022 at 04:41:17PM +0800, Xie Yongji wrote: > > > > > Export irq_create_affinity_masks() so that some modules > > > > > can make use of it to implement interrupt affinity > > > > > spreading mechanism. > > > > > > > > > > Signed-off-by: Xie Yongji > > > > > > > > So this got nacked, what's the plan now? > > > > > > > > > > I‘d like to check with Christoph again first. > > > > > > Hi Christoph, > > > > > > Jason will post some patches to get rid of the DMA API for vDPA > > > simulators. And the irq affinity algorithm is independent of the IRQ > > > subsystem IIUC. So could you allow this patch so that we can reuse the > > > algorithm to select the best CPU (per-cpu affinity if possible, or at > > > least per-node) to run the virtqueue's irq callback. > > > > > > Thanks, > > > Yongji > > > > I think you need to explain why you are building low level > > affinity masks. > > In VDUSE case, we use workqueue to run the virtqueue's irq callback. > Now I want to queue the irq callback kwork to one specific CPU to get > per-cpu affinity if possible, or at least per-node. So I need to use > this function to build the low level affinity masks for each > virtqueue. > > > what's the plan now? > > > > If there is no objection, I'll post a new version. > > Thanks, > Yongji I doubt you made a convicing case here - I think Christoph was saying if it is not an irq it should not use an irq affinity API. So a new API possibly sharing implementation with irq affinity is called for then? Maybe. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH] vhost: Add uAPI for Vhost SCMI backend
On Mon, Feb 13, 2023 at 10:04:17AM +0530, Neeraj Upadhyay wrote: > Add a uAPI for starting and stopping a SCMI vhost based > backend. > > Signed-off-by: Neeraj Upadhyay > --- > > SCMI Vhost backend implementation is work in progress: > https://lore.kernel.org/linux-arm-kernel/20220609071956.5183-1-quic_neer...@quicinc.com/ Excellent, and this UAPI patch should come together with kernel code which needs this new define. Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
