Here is the reconnect support in vduse,
Kernel will allocate pages for reconnection.
Userspace needs to use mmap to map the memory to userspace.
The kernel and userspace will use these pages to synchronize the
reconnect information.
test passd in vduse+dpdk-testpmd
change in V2
1. Address the comments from v1
2. Add the document for reconnect process
change in V3
1. Move the vdpa_vq_state to the uAPI. vduse will use this to synchronize the vq info between the kernel and userspace app.
2. Add a new ioctl VDUSE_DEV_GET_CONFIG. userspace app use this to get config space
3. Rewrite the commit message.
4. Only save the address for the page address and remove the index.
5. remove the ioctl VDUSE_GET_RECONNECT_INFO, userspace app will use uAPI VDUSE_RECONNCT_MMAP_SIZE to mmap
6. Rewrite the document for the reconnect process to make it clearer.
Signed-off-by: Cindy Lu <[email protected]>
Cindy Lu (7):
vdpa: Move struct vdpa_vq_state to uAPI
vduse: Add new uAPI for vduse reconnection
vduse: Add new ioctl VDUSE_DEV_GET_CONFIG
vduse: Add function to get/free the pages for reconnection
vduse: Add file operation for mmap
vduse: Update the vq_info in ioctl VDUSE_VQ_GET_INFO
Documentation: Add reconnect process for VDUSE
Documentation/userspace-api/vduse.rst | 30 +++++
drivers/vdpa/vdpa_user/vduse_dev.c | 184 +++++++++++++++++++++++++-
include/linux/vdpa.h | 29 +---
include/uapi/linux/vdpa.h | 31 +++++
include/uapi/linux/vduse.h | 25 ++++
5 files changed, 270 insertions(+), 29 deletions(-)
--
2.34.3
Move struct vdpa_vq_state to uAPI for vduse reconnect.
Userspace and kernel will use this structure to synchronize the vq status
Signed-off-by: Cindy Lu <[email protected]>
---
include/linux/vdpa.h | 29 +----------------------------
include/uapi/linux/vdpa.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 43f59ef10cc9..62ce97bce934 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -8,6 +8,7 @@
#include <linux/vhost_iotlb.h>
#include <linux/virtio_net.h>
#include <linux/if_ether.h>
+#include <uapi/linux/vdpa.h>
/**
* struct vdpa_calllback - vDPA callback definition.
@@ -29,34 +30,6 @@ struct vdpa_notification_area {
resource_size_t size;
};
-/**
- * struct vdpa_vq_state_split - vDPA split virtqueue state
- * @avail_index: available index
- */
-struct vdpa_vq_state_split {
- u16 avail_index;
-};
-
-/**
- * struct vdpa_vq_state_packed - vDPA packed virtqueue state
- * @last_avail_counter: last driver ring wrap counter observed by device
- * @last_avail_idx: device available index
- * @last_used_counter: device ring wrap counter
- * @last_used_idx: used index
- */
-struct vdpa_vq_state_packed {
- u16 last_avail_counter:1;
- u16 last_avail_idx:15;
- u16 last_used_counter:1;
- u16 last_used_idx:15;
-};
-
-struct vdpa_vq_state {
- union {
- struct vdpa_vq_state_split split;
- struct vdpa_vq_state_packed packed;
- };
-};
struct vdpa_mgmt_dev;
diff --git a/include/uapi/linux/vdpa.h b/include/uapi/linux/vdpa.h
index 54b649ab0f22..f4bc3589685e 100644
--- a/include/uapi/linux/vdpa.h
+++ b/include/uapi/linux/vdpa.h
@@ -7,6 +7,8 @@
#ifndef _UAPI_LINUX_VDPA_H_
#define _UAPI_LINUX_VDPA_H_
+#include <linux/types.h>
+
#define VDPA_GENL_NAME "vdpa"
#define VDPA_GENL_VERSION 0x1
@@ -60,4 +62,33 @@ enum vdpa_attr {
VDPA_ATTR_MAX,
};
+/**
+ * struct vdpa_vq_state_split - vDPA split virtqueue state
+ * @avail_index: available index
+ */
+struct vdpa_vq_state_split {
+ __u16 avail_index;
+};
+
+/**
+ * struct vdpa_vq_state_packed - vDPA packed virtqueue state
+ * @last_avail_counter: last driver ring wrap counter observed by device
+ * @last_avail_idx: device available index
+ * @last_used_counter: device ring wrap counter
+ * @last_used_idx: used index
+ */
+struct vdpa_vq_state_packed {
+ __u16 last_avail_counter : 1;
+ __u16 last_avail_idx : 15;
+ __u16 last_used_counter : 1;
+ __u16 last_used_idx : 15;
+};
+
+struct vdpa_vq_state {
+ union {
+ struct vdpa_vq_state_split split;
+ struct vdpa_vq_state_packed packed;
+ };
+};
+
#endif
--
2.34.3
To synchronize the information for reconnection, add a new structure
struct vduse_dev_reconnect_data to save the device-related information,
Add the VDUSE_RECONNCT_MMAP_SIZE for the size of mapped memory for each vq
and device status.
Signed-off-by: Cindy Lu <[email protected]>
---
include/uapi/linux/vduse.h | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index 11bd48c72c6c..c22838247814 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -350,4 +350,26 @@ struct vduse_dev_response {
};
};
+/**
+ * struct vduse_dev_reconnect_data - saved the reconnect info for device
+ * @version; version for userspace APP
+ * @reconnected: indetify if this is reconnected.userspace APP needs set this
+ * to VDUSE_RECONNECT, while reconnecting.kernel will use this
+ * to indetify if this is reconnect
+ * @features; Device features negotiated in the last connect.
+ * @status; Device status in last reconnect
+ */
+
+struct vduse_dev_reconnect_data {
+ __u32 version;
+#define VDUSE_RECONNECT 1
+#define VDUSE_NOT_RECONNECT 0
+ __u32 reconnected;
+ __u64 features;
+ __u8 status;
+};
+
+/* the reconnection mmap size for each VQ and dev status */
+#define VDUSE_RECONNCT_MMAP_SIZE PAGE_SIZE
+
#endif /* _UAPI_VDUSE_H_ */
--
2.34.3
The ioctl VDUSE_GET_RECONNECT_INFO is used by the Userspace App
to get the device configuration space.
Signed-off-by: Cindy Lu <[email protected]>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 21 +++++++++++++++++++++
include/uapi/linux/vduse.h | 3 +++
2 files changed, 24 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 26b7e29cb900..dd074a7b4bc7 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1273,6 +1273,27 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
ret = 0;
break;
}
+ case VDUSE_DEV_GET_CONFIG: {
+ struct vduse_config_data config;
+ unsigned long size = offsetof(struct vduse_config_data, buffer);
+
+ ret = -EFAULT;
+ if (copy_from_user(&config, argp, size))
+ break;
+
+ ret = -EINVAL;
+ if (config.offset > dev->config_size || config.length == 0 ||
+ config.length > dev->config_size - config.offset)
+ break;
+
+ if (copy_to_user(argp + size, dev->config + config.offset,
+ config.length)) {
+ ret = -EFAULT;
+ break;
+ }
+ ret = 0;
+ break;
+ }
default:
ret = -ENOIOCTLCMD;
break;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index c22838247814..5280042ced23 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -372,4 +372,7 @@ struct vduse_dev_reconnect_data {
/* the reconnection mmap size for each VQ and dev status */
#define VDUSE_RECONNCT_MMAP_SIZE PAGE_SIZE
+/* get device configuration space */
+#define VDUSE_DEV_GET_CONFIG _IOR(VDUSE_BASE, 0x1b, struct vduse_config_data)
+
#endif /* _UAPI_VDUSE_H_ */
--
2.34.3
Add the function vduse_alloc_reconnnect_info_mem
and vduse_alloc_reconnnect_info_mem
These functions allow vduse to allocate and free memory for reconnection
information. The amount of memory allocated is (vq_num + 1) pages.
Page 0 is reserved for saving the dev reconnection information, which
will be maintained by the Userspace App. Pages 1 to vq_num + 1 will be
used to save the reconnection information for vqs.
Signed-off-by: Cindy Lu <[email protected]>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 64 ++++++++++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index dd074a7b4bc7..52ccde636406 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -41,6 +41,7 @@
#define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
#define VDUSE_MSG_DEFAULT_TIMEOUT 30
+
struct vduse_virtqueue {
u16 index;
u16 num_max;
@@ -57,6 +58,7 @@ struct vduse_virtqueue {
struct vdpa_callback cb;
struct work_struct inject;
struct work_struct kick;
+ unsigned long vdpa_reconnect_vaddr;
};
struct vduse_dev;
@@ -106,6 +108,7 @@ struct vduse_dev {
u32 vq_align;
struct vduse_umem *umem;
struct mutex mem_lock;
+ unsigned long vdpa_reconnect_vaddr;
};
struct vduse_dev_msg {
@@ -1030,6 +1033,57 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
return ret;
}
+static int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
+{
+ unsigned long vaddr = 0;
+ struct vduse_virtqueue *vq;
+
+ for (int i = 0; i < dev->vq_num + 1; i++) {
+ if (i == 0) {
+ vaddr = __get_free_pages(
+ GFP_KERNEL | __GFP_ZERO,
+ get_order(VDUSE_RECONNCT_MMAP_SIZE));
+ if (vaddr == 0)
+ return -ENOMEM;
+ dev->vdpa_reconnect_vaddr = vaddr;
+ }
+
+ /*page 1~ vq_num + 1 save the reconnect info for vq*/
+ vq = &dev->vqs[i];
+ vaddr = __get_free_pages(GFP_KERNEL | __GFP_ZERO,
+ get_order(VDUSE_RECONNCT_MMAP_SIZE));
+ if (vaddr == 0)
+ return -ENOMEM;
+
+ vq->vdpa_reconnect_vaddr = vaddr;
+ }
+
+ return 0;
+}
+
+static int vduse_free_reconnnect_info_mem(struct vduse_dev *dev)
+{
+ struct vduse_virtqueue *vq;
+
+ for (int i = 0; i < dev->vq_num + 1; i++) {
+ if (i == 0) {
+ if (dev->vdpa_reconnect_vaddr)
+ free_pages(dev->vdpa_reconnect_vaddr,
+ get_order(VDUSE_RECONNCT_MMAP_SIZE));
+ dev->vdpa_reconnect_vaddr = 0;
+ }
+
+ vq = &dev->vqs[i];
+
+ if (vq->vdpa_reconnect_vaddr)
+ free_pages(vq->vdpa_reconnect_vaddr,
+ get_order(VDUSE_RECONNCT_MMAP_SIZE));
+ vq->vdpa_reconnect_vaddr = 0;
+ }
+
+ return 0;
+}
+
static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -1411,6 +1465,8 @@ static int vduse_destroy_dev(char *name)
mutex_unlock(&dev->lock);
return -EBUSY;
}
+ vduse_free_reconnnect_info_mem(dev);
+
dev->connected = true;
mutex_unlock(&dev->lock);
@@ -1563,9 +1619,17 @@ static int vduse_create_dev(struct vduse_dev_config *config,
ret = PTR_ERR(dev->dev);
goto err_dev;
}
+
+ ret = vduse_alloc_reconnnect_info_mem(dev);
+ if (ret < 0)
+ goto err_mem;
+
__module_get(THIS_MODULE);
return 0;
+
+err_mem:
+ vduse_free_reconnnect_info_mem(dev);
err_dev:
idr_remove(&vduse_idr, dev->minor);
err_idr:
--
2.34.3
Add a document explaining the reconnect process, including what the
Userspace App needs to do and how it works with the kernel.
Signed-off-by: Cindy Lu <[email protected]>
---
Documentation/userspace-api/vduse.rst | 30 +++++++++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/Documentation/userspace-api/vduse.rst b/Documentation/userspace-api/vduse.rst
index bdb880e01132..ec66e573f6da 100644
--- a/Documentation/userspace-api/vduse.rst
+++ b/Documentation/userspace-api/vduse.rst
@@ -231,3 +231,33 @@ able to start the dataplane processing as follows:
after the used ring is filled.
For more details on the uAPI, please see include/uapi/linux/vduse.h.
+
+HOW VDUSE devices reconnectoin works
+----------------
+0. Userspace APP checks if the device /dev/vduse/vduse_name exists.
+ If it does not exist, need to create the instance.
+ If it does exist, it means this is a reconnect and the program proceeds to step 3.
+
+1. Create a new VDUSE instance with ioctl(VDUSE_CREATE_DEV) on
+ /dev/vduse/control.
+
+2. When the ioctl(VDUSE_CREATE_DEV) function is called, the kernel allocates memory
+ to synchronize the reconnect information.
+
+3. Userspace App need to mmap the pages to userspace
+
+ When connecting for the first time, the userspace app must save the reconnect
+ information (struct vhost_reconnect_data) in mapped pages
+
+ If this is reconnect,If the purpose is to reconnect, the userspace application
+ must verify if the saved information is suitable for reconnection. Additionally,
+ the userspace application is required to set the "reconnected" bit in the
+ "struct vhost_reconnect_data" to "VDUSE_RECONNECT". The kernel will use this bit
+ to identify if the userspace application is reconnect.
+
+4. Start the userspace App.
+ userspace APP will call ioctl VDUSE_VQ_GET_INFO to sync the vq information.
+ Additionally, the APP must save the vq reconnect information (struct vdpa_vq_state)
+ in mapped pages while running.
+
+5. When the Userspace App exits, it is necessary to unmap all the reconnect pages.
--
2.34.3
Add the operation for mmap, This function will be used by the user space
application to map the pages to the user space.
Signed-off-by: Cindy Lu <[email protected]>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 75 ++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 52ccde636406..f55f415629de 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1381,6 +1381,79 @@ static struct vduse_dev *vduse_dev_get_from_minor(int minor)
return dev;
}
+static vm_fault_t vduse_vm_fault(struct vm_fault *vmf)
+{
+ struct vduse_dev *dev = vmf->vma->vm_file->private_data;
+ struct vm_area_struct *vma = vmf->vma;
+ u16 index = vma->vm_pgoff;
+ struct vduse_virtqueue *vq;
+ unsigned long vaddr;
+
+ /* index 0 page reserved for vduse status*/
+ if (index == 0) {
+ vaddr = dev->vdpa_reconnect_vaddr;
+ } else {
+ /* index 1+vq_number page reserved for vduse vqs*/
+ vq = &dev->vqs[index - 1];
+ vaddr = vq->vdpa_reconnect_vaddr;
+ }
+ if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
+ PFN_DOWN(virt_to_phys((void *)vaddr)),
+ VDUSE_RECONNCT_MMAP_SIZE, vma->vm_page_prot))
+ return VM_FAULT_SIGBUS;
+ return VM_FAULT_NOPAGE;
+}
+
+static const struct vm_operations_struct vduse_vm_ops = {
+ .fault = vduse_vm_fault,
+};
+
+static int vduse_dev_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ struct vduse_dev *dev = file->private_data;
+ unsigned long vaddr = 0;
+ unsigned long index = vma->vm_pgoff;
+ struct vduse_virtqueue *vq;
+
+ if (vma->vm_end - vma->vm_start != PAGE_SIZE)
+ return -EINVAL;
+ if ((vma->vm_flags & VM_SHARED) == 0)
+ return -EINVAL;
+
+ /*check if Userspace App map the page number larger than kernel allocated*/
+ if (index > dev->vq_num + 1)
+ return -EINVAL;
+
+ /* index 0 page reserved for vduse status*/
+ if (index == 0) {
+ vaddr = dev->vdpa_reconnect_vaddr;
+ } else {
+ /* index 1+vq_number page reserved for vduse vqs*/
+ vq = &dev->vqs[index - 1];
+ vaddr = vq->vdpa_reconnect_vaddr;
+ }
+ /* Check whether the memory for the mmap was allocated by the kernel.
+ * If not, this device may not have been created/destroyed correctly.
+ */
+ if (vaddr == 0)
+ return -EOPNOTSUPP;
+
+ /* check if the address is page aligned, if not,
+ * this address maybe damaged
+ */
+ if (virt_to_phys((void *)vaddr) & (PAGE_SIZE - 1))
+ return -EINVAL;
+
+ /* Check if the Userspace App has mapped the correct size */
+ if (vma->vm_end - vma->vm_start != VDUSE_RECONNCT_MMAP_SIZE)
+ return -EOPNOTSUPP;
+
+ vm_flags_set(vma, VM_DONTEXPAND);
+ vma->vm_ops = &vduse_vm_ops;
+
+ return 0;
+}
+
static int vduse_dev_open(struct inode *inode, struct file *file)
{
int ret;
@@ -1413,6 +1486,8 @@ static const struct file_operations vduse_dev_fops = {
.unlocked_ioctl = vduse_dev_ioctl,
.compat_ioctl = compat_ptr_ioctl,
.llseek = noop_llseek,
+ .mmap = vduse_dev_mmap,
+
};
static struct vduse_dev *vduse_dev_create(void)
--
2.34.3
Once the reconnect memory pages are mapped to userspace, the userspace
application will update the "reconnected" bit in the
"struct vduse_dev_reconnect_data".
The kernel will then check this bit. If it is not set to
"VDUSE_NOT_RECONNECT", it means that the application has been
reconnected, and the kernel will synchronize the vq information.
Signed-off-by: Cindy Lu <[email protected]>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index f55f415629de..422f1aedebac 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1193,6 +1193,9 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
struct vduse_vq_info vq_info;
struct vduse_virtqueue *vq;
u32 index;
+ unsigned long vaddr;
+ struct vduse_vq_state *vq_reconnect;
+ struct vduse_dev_reconnect_data *dev_reconnect;
ret = -EFAULT;
if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
@@ -1209,6 +1212,12 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
vq_info.device_addr = vq->device_addr;
vq_info.num = vq->num;
+ vaddr = dev->vdpa_reconnect_vaddr;
+ dev_reconnect = (struct vduse_dev_reconnect_data *)vaddr;
+
+ vaddr = vq->vdpa_reconnect_vaddr;
+ vq_reconnect = (struct vduse_vq_state *)vaddr;
+
if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
vq_info.packed.last_avail_counter =
vq->state.packed.last_avail_counter;
@@ -1218,9 +1227,22 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
vq->state.packed.last_used_counter;
vq_info.packed.last_used_idx =
vq->state.packed.last_used_idx;
- } else
+ /*check if the vq is reconnect, if yes then update the info*/
+ if (dev_reconnect->reconnected != VDUSE_NOT_RECONNECT) {
+ vq_info.packed.last_avail_idx =
+ vq_reconnect->packed.last_avail_idx;
+ vq_info.packed.last_avail_counter =
+ vq_reconnect->packed.last_avail_counter;
+ }
+ } else {
vq_info.split.avail_index =
vq->state.split.avail_index;
+ /*check if the vq is reconnect, if yes then update the info*/
+ if (dev_reconnect->reconnected != VDUSE_NOT_RECONNECT) {
+ vq_info.split.avail_index =
+ vq_reconnect->split.avail_index;
+ }
+ }
vq_info.ready = vq->ready;
--
2.34.3
On Tue, Dec 5, 2023 at 4:35 PM Cindy Lu <[email protected]> wrote:
>
> Add the function vduse_alloc_reconnnect_info_mem
> and vduse_alloc_reconnnect_info_mem
> These functions allow vduse to allocate and free memory for reconnection
> information. The amount of memory allocated is (vq_num + 1) pages.
>
> Page 0 is reserved for saving the dev reconnection information, which
> will be maintained by the Userspace App. Pages 1 to vq_num + 1 will be
> used to save the reconnection information for vqs.
>
> Signed-off-by: Cindy Lu <[email protected]>
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 64 ++++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index dd074a7b4bc7..52ccde636406 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -41,6 +41,7 @@
> #define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
> #define VDUSE_MSG_DEFAULT_TIMEOUT 30
>
> +
Unnecessary changes.
> struct vduse_virtqueue {
> u16 index;
> u16 num_max;
> @@ -57,6 +58,7 @@ struct vduse_virtqueue {
> struct vdpa_callback cb;
> struct work_struct inject;
> struct work_struct kick;
> + unsigned long vdpa_reconnect_vaddr;
> };
>
> struct vduse_dev;
> @@ -106,6 +108,7 @@ struct vduse_dev {
> u32 vq_align;
> struct vduse_umem *umem;
> struct mutex mem_lock;
> + unsigned long vdpa_reconnect_vaddr;
> };
>
> struct vduse_dev_msg {
> @@ -1030,6 +1033,57 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
> return ret;
> }
>
> +static int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
> +{
> + unsigned long vaddr = 0;
> + struct vduse_virtqueue *vq;
> +
> + for (int i = 0; i < dev->vq_num + 1; i++) {
> + if (i == 0) {
> + vaddr = __get_free_pages(
> + GFP_KERNEL | __GFP_ZERO,
> + get_order(VDUSE_RECONNCT_MMAP_SIZE));
order 0 should be sufficient here or anything I missed?
> + if (vaddr == 0)
> + return -ENOMEM;
> + dev->vdpa_reconnect_vaddr = vaddr;
> + }
> +
> + /*page 1~ vq_num + 1 save the reconnect info for vq*/
> + vq = &dev->vqs[i];
> + vaddr = __get_free_pages(GFP_KERNEL | __GFP_ZERO,
> + get_order(VDUSE_RECONNCT_MMAP_SIZE));
> + if (vaddr == 0)
> + return -ENOMEM;
> +
> + vq->vdpa_reconnect_vaddr = vaddr;
> + }
> +
> + return 0;
> +}
> +
> +static int vduse_free_reconnnect_info_mem(struct vduse_dev *dev)
> +{
> + struct vduse_virtqueue *vq;
> +
> + for (int i = 0; i < dev->vq_num + 1; i++) {
> + if (i == 0) {
> + if (dev->vdpa_reconnect_vaddr)
> + free_pages(dev->vdpa_reconnect_vaddr,
> + get_order(VDUSE_RECONNCT_MMAP_SIZE));
> + dev->vdpa_reconnect_vaddr = 0;
Let's use NULL here.
> + }
> +
> + vq = &dev->vqs[i];
> +
> + if (vq->vdpa_reconnect_vaddr)
> + free_pages(vq->vdpa_reconnect_vaddr,
> + get_order(VDUSE_RECONNCT_MMAP_SIZE));
> + vq->vdpa_reconnect_vaddr = 0;
> + }
> +
> + return 0;
> +}
> +
> static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> {
> @@ -1411,6 +1465,8 @@ static int vduse_destroy_dev(char *name)
> mutex_unlock(&dev->lock);
> return -EBUSY;
> }
> + vduse_free_reconnnect_info_mem(dev);
> +
> dev->connected = true;
> mutex_unlock(&dev->lock);
>
> @@ -1563,9 +1619,17 @@ static int vduse_create_dev(struct vduse_dev_config *config,
> ret = PTR_ERR(dev->dev);
> goto err_dev;
> }
> +
> + ret = vduse_alloc_reconnnect_info_mem(dev);
> + if (ret < 0)
> + goto err_mem;
> +
> __module_get(THIS_MODULE);
>
> return 0;
> +
> +err_mem:
> + vduse_free_reconnnect_info_mem(dev);
> err_dev:
> idr_remove(&vduse_idr, dev->minor);
> err_idr:
> --
> 2.34.3
>
Thanks
On Tue, Dec 5, 2023 at 4:35 PM Cindy Lu <[email protected]> wrote:
>
> Once the reconnect memory pages are mapped to userspace, the userspace
> application will update the "reconnected" bit in the
> "struct vduse_dev_reconnect_data".
> The kernel will then check this bit. If it is not set to
> "VDUSE_NOT_RECONNECT", it means that the application has been
> reconnected, and the kernel will synchronize the vq information.
Can you explain why we need such a flag?
>
> Signed-off-by: Cindy Lu <[email protected]>
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 24 +++++++++++++++++++++++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index f55f415629de..422f1aedebac 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1193,6 +1193,9 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> struct vduse_vq_info vq_info;
> struct vduse_virtqueue *vq;
> u32 index;
> + unsigned long vaddr;
> + struct vduse_vq_state *vq_reconnect;
> + struct vduse_dev_reconnect_data *dev_reconnect;
>
> ret = -EFAULT;
> if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
> @@ -1209,6 +1212,12 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> vq_info.device_addr = vq->device_addr;
> vq_info.num = vq->num;
>
> + vaddr = dev->vdpa_reconnect_vaddr;
> + dev_reconnect = (struct vduse_dev_reconnect_data *)vaddr;
> +
> + vaddr = vq->vdpa_reconnect_vaddr;
> + vq_reconnect = (struct vduse_vq_state *)vaddr;
> +
> if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
> vq_info.packed.last_avail_counter =
> vq->state.packed.last_avail_counter;
> @@ -1218,9 +1227,22 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> vq->state.packed.last_used_counter;
> vq_info.packed.last_used_idx =
> vq->state.packed.last_used_idx;
> - } else
> + /*check if the vq is reconnect, if yes then update the info*/
> + if (dev_reconnect->reconnected != VDUSE_NOT_RECONNECT) {
So the dev_reconnect is shared, how to synchronize between them?
> + vq_info.packed.last_avail_idx =
> + vq_reconnect->packed.last_avail_idx;
> + vq_info.packed.last_avail_counter =
> + vq_reconnect->packed.last_avail_counter;
> + }
> + } else {
> vq_info.split.avail_index =
> vq->state.split.avail_index;
> + /*check if the vq is reconnect, if yes then update the info*/
> + if (dev_reconnect->reconnected != VDUSE_NOT_RECONNECT) {
If this only check is !=, I'd define it as VDUSE_RECONNECT to ease the
code logic.
> + vq_info.split.avail_index =
> + vq_reconnect->split.avail_index;
> + }
> + }
It looks like I miss something here
If I read the code correctly, vq_reconnect is written by userspace, so
userspace query it again via the get_vq_info()? If this is true,
userspace can just ignore the index via GET_VQ_INFO() or is it just
for consistency?
Thanks
>
> vq_info.ready = vq->ready;
>
> --
> 2.34.3
>
On Tue, Dec 5, 2023 at 4:35 PM Cindy Lu <[email protected]> wrote:
>
> Add the operation for mmap, This function will be used by the user space
> application to map the pages to the user space.
>
> Signed-off-by: Cindy Lu <[email protected]>
> ---
> drivers/vdpa/vdpa_user/vduse_dev.c | 75 ++++++++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 52ccde636406..f55f415629de 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1381,6 +1381,79 @@ static struct vduse_dev *vduse_dev_get_from_minor(int minor)
> return dev;
> }
>
> +static vm_fault_t vduse_vm_fault(struct vm_fault *vmf)
> +{
> + struct vduse_dev *dev = vmf->vma->vm_file->private_data;
> + struct vm_area_struct *vma = vmf->vma;
> + u16 index = vma->vm_pgoff;
> + struct vduse_virtqueue *vq;
> + unsigned long vaddr;
> +
> + /* index 0 page reserved for vduse status*/
I'd suggest to tweak it as "reserved for device state"
> + if (index == 0) {
> + vaddr = dev->vdpa_reconnect_vaddr;
> + } else {
> + /* index 1+vq_number page reserved for vduse vqs*/
And "reserved for virtqueue state"
> + vq = &dev->vqs[index - 1];
> + vaddr = vq->vdpa_reconnect_vaddr;
> + }
> + if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
> + PFN_DOWN(virt_to_phys((void *)vaddr)),
> + VDUSE_RECONNCT_MMAP_SIZE, vma->vm_page_prot))
If there's no chance for VDUSE_RECONNCT_MMAP_SIZE to differ from
PAGE_SIZE. Let's just use PAGE_SIZE here.
> + return VM_FAULT_SIGBUS;
> + return VM_FAULT_NOPAGE;
> +}
> +
> +static const struct vm_operations_struct vduse_vm_ops = {
> + .fault = vduse_vm_fault,
> +};
> +
> +static int vduse_dev_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> + struct vduse_dev *dev = file->private_data;
> + unsigned long vaddr = 0;
> + unsigned long index = vma->vm_pgoff;
> + struct vduse_virtqueue *vq;
> +
> + if (vma->vm_end - vma->vm_start != PAGE_SIZE)
> + return -EINVAL;
> + if ((vma->vm_flags & VM_SHARED) == 0)
> + return -EINVAL;
> +
> + /*check if Userspace App map the page number larger than kernel allocated*/
Code explains itself, let's just drop the comment please.
> + if (index > dev->vq_num + 1)
> + return -EINVAL;
> +
> + /* index 0 page reserved for vduse status*/
This comment duplicates the one in fault. I'd suggest to move it to uAPI.
> + if (index == 0) {
> + vaddr = dev->vdpa_reconnect_vaddr;
> + } else {
> + /* index 1+vq_number page reserved for vduse vqs*/
> + vq = &dev->vqs[index - 1];
> + vaddr = vq->vdpa_reconnect_vaddr;
> + }
> + /* Check whether the memory for the mmap was allocated by the kernel.
> + * If not, this device may not have been created/destroyed correctly.
> + */
Code explains, so let's drop the comment.
> + if (vaddr == 0)
> + return -EOPNOTSUPP;
> +
> + /* check if the address is page aligned, if not,
> + * this address maybe damaged
> + */
And this comment.
Thanks
On Tue, Dec 5, 2023 at 4:35 PM Cindy Lu <[email protected]> wrote:
>
> To synchronize the information for reconnection, add a new structure
> struct vduse_dev_reconnect_data to save the device-related information,
> Add the VDUSE_RECONNCT_MMAP_SIZE for the size of mapped memory for each vq
> and device status.
>
> Signed-off-by: Cindy Lu <[email protected]>
> ---
> include/uapi/linux/vduse.h | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> index 11bd48c72c6c..c22838247814 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -350,4 +350,26 @@ struct vduse_dev_response {
> };
> };
>
> +/**
> + * struct vduse_dev_reconnect_data - saved the reconnect info for device
> + * @version; version for userspace APP
I guess it should be the version of the reconnection protocol not the
userspace application.
> + * @reconnected: indetify if this is reconnected.userspace APP needs set this
> + * to VDUSE_RECONNECT, while reconnecting.kernel will use this
> + * to indetify if this is reconnect
Typos, I think checkpatch may help or you can tweak your editor to
enable spell checkings.
> + * @features; Device features negotiated in the last connect.
This seems to cause confusion, let's use driver_features instead.
But can't we just get driver_features via VDUSE_DEV_GET_FEATURES?
> + * @status; Device status in last reconnect
> + */
> +
> +struct vduse_dev_reconnect_data {
> + __u32 version;
> +#define VDUSE_RECONNECT 1
> +#define VDUSE_NOT_RECONNECT 0
> + __u32 reconnected;
> + __u64 features;
> + __u8 status;
> +};
For status, VDUSE currently forwards the request to the userspace:
static int vduse_dev_set_status(struct vduse_dev *dev, u8 status)
{
struct vduse_dev_msg msg = { 0 };
msg.req.type = VDUSE_SET_STATUS;
msg.req.s.status = status;
return vduse_dev_msg_sync(dev, &msg);
}
Do we need to handle the case where the user space crashes when
dealing with this message? For example, driver set DRIVER_OK but VDUSE
application crashes when dealing with DRIVER_OK.
At the uAPI level, it probably requires to fetch inflight VDUSE
requests. It may help for the case where dealing with crash at feature
negotiation and configuration space access.
> +
> +/* the reconnection mmap size for each VQ and dev status */
> +#define VDUSE_RECONNCT_MMAP_SIZE PAGE_SIZE
PAGE_SIZE should not belong to uAPI. Userspace can query it via e.g sysconf etc.
Btw, what is the virtqueue part for this? I'd expect there should be
at least the part of inflight descriptors?
> +
> #endif /* _UAPI_VDUSE_H_ */
> --
> 2.34.3
>
On Tue, Dec 5, 2023 at 4:35 PM Cindy Lu <[email protected]> wrote:
>
> Add a document explaining the reconnect process, including what the
> Userspace App needs to do and how it works with the kernel.
>
> Signed-off-by: Cindy Lu <[email protected]>
> ---
> Documentation/userspace-api/vduse.rst | 30 +++++++++++++++++++++++++++
> 1 file changed, 30 insertions(+)
>
> diff --git a/Documentation/userspace-api/vduse.rst b/Documentation/userspace-api/vduse.rst
> index bdb880e01132..ec66e573f6da 100644
> --- a/Documentation/userspace-api/vduse.rst
> +++ b/Documentation/userspace-api/vduse.rst
> @@ -231,3 +231,33 @@ able to start the dataplane processing as follows:
> after the used ring is filled.
>
> For more details on the uAPI, please see include/uapi/linux/vduse.h.
> +
> +HOW VDUSE devices reconnectoin works
> +----------------
> +0. Userspace APP checks if the device /dev/vduse/vduse_name exists.
> + If it does not exist, need to create the instance.
> + If it does exist, it means this is a reconnect and the program proceeds to step 3.
> +
> +1. Create a new VDUSE instance with ioctl(VDUSE_CREATE_DEV) on
> + /dev/vduse/control.
> +
> +2. When the ioctl(VDUSE_CREATE_DEV) function is called, the kernel allocates memory
> + to synchronize the reconnect information.
This is the implementation details and I guess this doc only focuses
on the uAPI. So there's no need to describe it here.
> +
> +3. Userspace App need to mmap the pages to userspace
> +
> + When connecting for the first time, the userspace app must save the reconnect
> + information (struct vhost_reconnect_data) in mapped pages
This looks vague. We need to describe what kind of information that
needs to be saved and when to save.
Thanks
> +
> + If this is reconnect,If the purpose is to reconnect, the userspace application
> + must verify if the saved information is suitable for reconnection. Additionally,
> + the userspace application is required to set the "reconnected" bit in the
> + "struct vhost_reconnect_data" to "VDUSE_RECONNECT". The kernel will use this bit
> + to identify if the userspace application is reconnect.
> +
> +4. Start the userspace App.
> + userspace APP will call ioctl VDUSE_VQ_GET_INFO to sync the vq information.
> + Additionally, the APP must save the vq reconnect information (struct vdpa_vq_state)
> + in mapped pages while running.
> +
> +5. When the Userspace App exits, it is necessary to unmap all the reconnect pages.
> --
> 2.34.3
>