[PATCH v2 0/5] vduse: Add support for reconnection

[PATCH v2 0/5] vduse: Add support for reconnection

[Cindy Lu]

Here is the reconnect support in vduse,

kernel will allocted pages for reconnection
userspace need use ioctl VDUSE_GET_RECONNECT_INFO to
get the mmap related information and then mapping these pages
to userspace.
kernel and userspace will use these pages to sync
the reconnect information
kernel will use VDUSE_VQ_GET_INFO to sync the information      
userspace App  will call during the "user_app_dev_start()".

change in V2  
1. Address the comments from v1
2. Add the document for reconnect process

Cindy Lu (5):
  vduse: Add function to get/free the pages for reconnection
  vduse: Add file operation for mmap
  vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
  vduse: update the vq_info in ioctl
  Documentation: Add reconnect process for VDUSE

 Documentation/userspace-api/vduse.rst |  29 ++++
 drivers/vdpa/vdpa_user/vduse_dev.c    | 198 +++++++++++++++++++++++++-
 include/uapi/linux/vduse.h            |  50 +++++++
 3 files changed, 276 insertions(+), 1 deletion(-)

-- 
2.34.3

[PATCH v2 1/5] vduse: Add function to get/free the pages for reconnection

[Cindy Lu]

Add the function vduse_alloc_reconnnect_info_mem
and vduse_alloc_reconnnect_info_mem
In this 2 function, vduse will get/free (vq_num + 1)*page  
Page 0 will be used to save the reconnection information, The
Userspace App will maintain this. Page 1 ~ vq_num + 1 will save
the reconnection information for vqs.

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 80 ++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 26b7e29cb900..6209e2f00278 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -41,6 +41,16 @@
 #define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
 #define VDUSE_MSG_DEFAULT_TIMEOUT 30
 
+/* struct vdpa_reconnect_info saved the alloc pages info
+ * these pages will mmaped to userspace for reconnection
+ */
+struct vdpa_reconnect_info {
+	/* Offset (within vm_file) in PAGE_SIZE */
+	u32 index;
+	/* virtual address for this page*/
+	unsigned long vaddr;
+};
+
 struct vduse_virtqueue {
 	u16 index;
 	u16 num_max;
@@ -57,6 +67,7 @@ struct vduse_virtqueue {
 	struct vdpa_callback cb;
 	struct work_struct inject;
 	struct work_struct kick;
+	struct vdpa_reconnect_info reconnect_info;
 };
 
 struct vduse_dev;
@@ -106,6 +117,7 @@ struct vduse_dev {
 	u32 vq_align;
 	struct vduse_umem *umem;
 	struct mutex mem_lock;
+	struct vdpa_reconnect_info reconnect_info;
 };
 
 struct vduse_dev_msg {
@@ -1030,6 +1042,64 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
 	return ret;
 }
 
+static int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
+{
+	struct vdpa_reconnect_info *info;
+	struct vduse_virtqueue *vq;
+
+	for (int i = 0; i < dev->vq_num + 1; i++) {
+		if (i == 0) {
+			/*page 0 is use to save status,Userland APP will use this to
+			 *save the information needed in reconnection,
+			 *kernel don't need to maintain this
+			 */
+			info = &dev->reconnect_info;
+			info->vaddr = get_zeroed_page(GFP_KERNEL);
+			if (info->vaddr == 0)
+				return -ENOMEM;
+			/* index is vm Offset in PAGE_SIZE */
+			info->index = 0;
+		}
+
+		/*page 1~ vq_num + 1 save the reconnect info for vq*/
+		vq = &dev->vqs[i];
+		info = &vq->reconnect_info;
+		info->vaddr = get_zeroed_page(GFP_KERNEL);
+		if (info->vaddr == 0)
+			return -ENOMEM;
+
+		info->index = i + 1;
+	}
+
+	return 0;
+}
+
+static int vduse_free_reconnnect_info_mem(struct vduse_dev *dev)
+{
+	struct vdpa_reconnect_info *info;
+	struct vduse_virtqueue *vq;
+
+	for (int i = 0; i < dev->vq_num + 1; i++) {
+		if (i == 0) {
+			info = &dev->reconnect_info;
+			if (info->vaddr)
+				free_page(info->vaddr);
+			info->index = 0;
+			info->vaddr = 0;
+		}
+
+		vq = &dev->vqs[i];
+		info = &vq->reconnect_info;
+
+		if (info->vaddr)
+			free_page(info->vaddr);
+		info->vaddr = 0;
+		info->index = 0;
+	}
+
+	return 0;
+}
+
 static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 			    unsigned long arg)
 {
@@ -1390,6 +1460,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);
 
@@ -1542,9 +1614,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

[PATCH v2 2/5] vduse: Add file operation for mmap

[Cindy Lu]

Add the operation for mmap, The user space APP will
use this function to map the pages to userspace

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 79 ++++++++++++++++++++++++++++++
 1 file changed, 79 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index 6209e2f00278..ccb30e98bab5 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1376,6 +1376,83 @@ 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;
+	struct vdpa_reconnect_info *info;
+
+	/* index 0  page reserved for vduse status*/
+	if (index == 0) {
+		info = &dev->reconnect_info;
+	} else {
+		/* index 1+vq_number page reserved for vduse vqs*/
+		vq = &dev->vqs[index - 1];
+		info = &vq->reconnect_info;
+	}
+	if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
+			    PFN_DOWN(virt_to_phys((void *)info->vaddr)),
+			    PAGE_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;
+	struct vdpa_reconnect_info *info;
+	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) {
+		info = &dev->reconnect_info;
+	} else {
+		/* index 1+vq_number page reserved for vduse vqs*/
+		vq = &dev->vqs[index - 1];
+		info = &vq->reconnect_info;
+	}
+	/*check if map pages was allocated and inited by kernel */
+	if (info->vaddr == 0)
+		return -EOPNOTSUPP;
+
+	/* check if the address is page aligned, if not,
+	 * this address maybe damaged
+	 */
+	if (virt_to_phys((void *)info->vaddr) & (PAGE_SIZE - 1))
+		return -EINVAL;
+
+	/* check if Userspace App mapped the correct size
+	 * the userspace App should map one page each time
+	 */
+	if (vma->vm_end - vma->vm_start != PAGE_SIZE)
+		return -EOPNOTSUPP;
+	/* VM_IO: set as a memory-mapped I/O region,This will for vq information
+	 * VM_PFNMAP: only need  the pure PFN
+	 * VM_DONTEXPAND: do not need to use mremap() in this function
+	 * VM_DONTDUMP:Do not include in the core dump
+	 */
+	vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
+	vma->vm_ops = &vduse_vm_ops;
+
+	return 0;
+}
+
 static int vduse_dev_open(struct inode *inode, struct file *file)
 {
 	int ret;
@@ -1408,6 +1485,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

[PATCH v2 3/5] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Cindy Lu]

In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
and The number of mapping memory pages from the kernel. The userspace
App can use this information to map the pages.

Add struct vhost_reconnect_data/vhost_reconnect_vring for sync the
information in reconnection

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++
 include/uapi/linux/vduse.h         | 50 ++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index ccb30e98bab5..d0fe9a7e86ab 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1343,6 +1343,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		ret = 0;
 		break;
 	}
+	case VDUSE_GET_RECONNECT_INFO: {
+		struct vduse_reconnect_mmap_info info;
+
+		ret = -EFAULT;
+		if (copy_from_user(&info, argp, sizeof(info)))
+			break;
+
+		info.size = PAGE_SIZE;
+		info.max_index = dev->vq_num + 1;
+
+		if (copy_to_user(argp, &info, sizeof(info)))
+			break;
+		ret = 0;
+		break;
+	}
 	default:
 		ret = -ENOIOCTLCMD;
 		break;
diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
index 11bd48c72c6c..c0b7133aebfd 100644
--- a/include/uapi/linux/vduse.h
+++ b/include/uapi/linux/vduse.h
@@ -3,6 +3,7 @@
 #define _UAPI_VDUSE_H_
 
 #include <linux/types.h>
+#include <linux/virtio_net.h>
 
 #define VDUSE_BASE	0x81
 
@@ -350,4 +351,53 @@ struct vduse_dev_response {
 	};
 };
 
+/**
+ * struct vhost_reconnect_data - saved the reconnect info for device
+ * @version; version for userspace APP
+ * @reconnected: indetify if this is reconnected. userspace APP needs set this bit to 1
+ *               while reconnecting.kernel will use this bit to indetify if this is
+ *               reconnect
+ * @features; Device features negotiated in the last connect.
+ * @status; Device status in last reconnect
+ * @nr_vrings; number of active vqs in last connect
+ * @struct virtio_net_config config; the config in last connect
+ */
+
+struct vhost_reconnect_data {
+	__u32 version;
+	__u32 reconnected;
+	__u64 features;
+	__u8 status;
+	__u32 nr_vrings;
+	struct virtio_net_config config;
+};
+
+/**
+ * struct vhost_reconnect_vring -saved the reconnect info for vqs
+ * when use split mode will only use last_avail_idx
+ * when use packed mode will use both last_avail_idx and avail_wrap_counter
+ * userspace App
+ * @last_avail_idx: device last available index
+ * @avail_wrap_counter: Driver ring wrap counter
+ */
+struct vhost_reconnect_vring {
+	__u16 last_avail_idx;
+	__u16 avail_wrap_counter;
+};
+
+/**
+ * struct vduse_reconnect_mmap_info
+ * @size: mapping memory size, here we use page_size
+ * @max_index: the number of pages allocated in kernel,just
+ * use for sanity check
+ */
+
+struct vduse_reconnect_mmap_info {
+	__u32 size;
+	__u32 max_index;
+};
+
+#define VDUSE_GET_RECONNECT_INFO \
+	_IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
+
 #endif /* _UAPI_VDUSE_H_ */
-- 
2.34.3

[PATCH v2 4/5] vduse: update the vq_info in ioctl

[Cindy Lu]

In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
with reconnect info, After mapping the reconnect pages to userspace
The userspace App will update the reconnect_time in
struct vhost_reconnect_vring, If this is not 0 then it means this
vq is reconnected and will update the last_avail_idx

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 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 d0fe9a7e86ab..6bc5fc2b88cc 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1209,6 +1209,9 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		struct vduse_vq_info vq_info;
 		struct vduse_virtqueue *vq;
 		u32 index;
+		struct vdpa_reconnect_info *area;
+		struct vhost_reconnect_vring *vq_reconnect;
+		struct vhost_reconnect_data *dev_reconnect;
 
 		ret = -EFAULT;
 		if (copy_from_user(&vq_info, argp, sizeof(vq_info)))
@@ -1225,6 +1228,12 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
 		vq_info.device_addr = vq->device_addr;
 		vq_info.num = vq->num;
 
+		area = &dev->reconnect_info;
+		dev_reconnect = (struct vhost_reconnect_data *)area->vaddr;
+
+		area = &vq->reconnect_info;
+		vq_reconnect = (struct vhost_reconnect_vring *)area->vaddr;
+
 		if (dev->driver_features & BIT_ULL(VIRTIO_F_RING_PACKED)) {
 			vq_info.packed.last_avail_counter =
 				vq->state.packed.last_avail_counter;
@@ -1234,9 +1243,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 last_avail_idx*/
+			if (dev_reconnect->reconnected != 0) {
+				vq_info.packed.last_avail_idx =
+					vq_reconnect->last_avail_idx;
+				vq_info.packed.last_avail_counter =
+					vq_reconnect->avail_wrap_counter;
+			}
+		} else {
 			vq_info.split.avail_index =
 				vq->state.split.avail_index;
+			/*check if the vq is reconnect, if yes then update the last_avail_idx*/
+			if (dev_reconnect->reconnected != 0) {
+				vq_info.split.avail_index =
+					vq_reconnect->last_avail_idx;
+			}
+		}
 
 		vq_info.ready = vq->ready;
 
-- 
2.34.3

[PATCH v2 5/5] Documentation: Add reconnect process for VDUSE

[Cindy Lu]

Add the document to explained how the reconnect process
include how the Userspace App need to do and how it works
with kernel

Signed-off-by: Cindy Lu <lulu@redhat.com>
---
 Documentation/userspace-api/vduse.rst | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/Documentation/userspace-api/vduse.rst b/Documentation/userspace-api/vduse.rst
index bdb880e01132..6e01c21d94df 100644
--- a/Documentation/userspace-api/vduse.rst
+++ b/Documentation/userspace-api/vduse.rst
@@ -231,3 +231,32 @@ 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 not create
+   the device. If yes means this is reconnect, goto 3
+
+1. Create a new VDUSE instance with ioctl(VDUSE_CREATE_DEV) on
+   /dev/vduse/control.
+
+2. In ioctl(VDUSE_CREATE_DEV), the kernel alloc the memory to sync the reconnect
+   information.
+
+3. Userspace App will mmap the pages to userspace
+
+   If this first time to connect, userspace App need save the reconnect
+   information (struct vhost_reconnect_data) in mapped pages
+
+   If this is reconnect, userspace App need to check if the saved information
+   OK to reconnect, Also userspace App MUST set the bit reconnected in
+   struct vhost_reconnect_data to 1. (kernel will use this bit to identify if the
+   userAPP is reconnected )
+
+4. Successfully start the userspace App.
+   userspace APP need to call ioctl VDUSE_VQ_GET_INFO to sync the vq information. also
+   APP need to save the reconnect information (struct vhost_reconnect_vring)
+   while running
+
+5. When the Userspace App exit, Userspace App need to unmap all the reconnect
+   pages
-- 
2.34.3

Re: [PATCH v2 2/5] vduse: Add file operation for mmap

[Jason Wang]

On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
>
> Add the operation for mmap, The user space APP will
> use this function to map the pages to userspace
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 79 ++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 6209e2f00278..ccb30e98bab5 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1376,6 +1376,83 @@ 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;
> +       struct vdpa_reconnect_info *info;
> +
> +       /* index 0  page reserved for vduse status*/
> +       if (index == 0) {
> +               info = &dev->reconnect_info;
> +       } else {
> +               /* index 1+vq_number page reserved for vduse vqs*/
> +               vq = &dev->vqs[index - 1];
> +               info = &vq->reconnect_info;
> +       }
> +       if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
> +                           PFN_DOWN(virt_to_phys((void *)info->vaddr)),
> +                           PAGE_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;
> +       struct vdpa_reconnect_info *info;
> +       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) {
> +               info = &dev->reconnect_info;
> +       } else {
> +               /* index 1+vq_number page reserved for vduse vqs*/
> +               vq = &dev->vqs[index - 1];
> +               info = &vq->reconnect_info;
> +       }
> +       /*check if map pages was allocated and inited by kernel */
> +       if (info->vaddr == 0)
> +               return -EOPNOTSUPP;
> +
> +       /* check if the address is page aligned, if not,
> +        * this address maybe damaged
> +        */
> +       if (virt_to_phys((void *)info->vaddr) & (PAGE_SIZE - 1))
> +               return -EINVAL;
> +
> +       /* check if Userspace App mapped the correct size
> +        * the userspace App should map one page each time
> +        */
> +       if (vma->vm_end - vma->vm_start != PAGE_SIZE)
> +               return -EOPNOTSUPP;
> +       /* VM_IO: set as a memory-mapped I/O region,This will for vq information
> +        * VM_PFNMAP: only need  the pure PFN
> +        * VM_DONTEXPAND: do not need to use mremap() in this function
> +        * VM_DONTDUMP:Do not include in the core dump
> +        */

Instead of duplicating the semantic, you need to explain why it is needed.

For example I don't see how the following is useful:

VM_IO: the page is not backed by any I/O page
VM_PFNMAP: we need map it to usespace for sure, for it's not a pure PFN
VM_DONTDUMP: including this in core dump may help to debug the
userspace process for sure

Thanks

> +       vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
> +       vma->vm_ops = &vduse_vm_ops;
> +
> +       return 0;
> +}
> +
>  static int vduse_dev_open(struct inode *inode, struct file *file)
>  {
>         int ret;
> @@ -1408,6 +1485,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
>

Re: [PATCH v2 0/5] vduse: Add support for reconnection

[Jason Wang]

On Tue, Nov 21, 2023 at 3:30 PM Cindy Lu <lulu@redhat.com> wrote:
>
> Here is the reconnect support in vduse,
>
> kernel will allocted pages for reconnection
> userspace need use ioctl VDUSE_GET_RECONNECT_INFO to
> get the mmap related information and then mapping these pages
> to userspace.
> kernel and userspace will use these pages to sync
> the reconnect information
> kernel will use VDUSE_VQ_GET_INFO to sync the information
> userspace App  will call during the "user_app_dev_start()".

It would be better to describe the uAPI instead of duplicating the
logic of the codes.

>
> change in V2
> 1. Address the comments from v1

It's better to be more verbose here, people can easily forget the
comments since V1.

Thanks

> 2. Add the document for reconnect process
>
> Cindy Lu (5):
>   vduse: Add function to get/free the pages for reconnection
>   vduse: Add file operation for mmap
>   vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
>   vduse: update the vq_info in ioctl
>   Documentation: Add reconnect process for VDUSE
>
>  Documentation/userspace-api/vduse.rst |  29 ++++
>  drivers/vdpa/vdpa_user/vduse_dev.c    | 198 +++++++++++++++++++++++++-
>  include/uapi/linux/vduse.h            |  50 +++++++
>  3 files changed, 276 insertions(+), 1 deletion(-)
>
> --
> 2.34.3
>

Re: [PATCH v2 2/5] vduse: Add file operation for mmap

[Jason Wang]

On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
>
> Add the operation for mmap, The user space APP will
> use this function to map the pages to userspace
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 79 ++++++++++++++++++++++++++++++
>  1 file changed, 79 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 6209e2f00278..ccb30e98bab5 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1376,6 +1376,83 @@ 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;
> +       struct vdpa_reconnect_info *info;
> +
> +       /* index 0  page reserved for vduse status*/
> +       if (index == 0) {

I'd avoid using magic numbers but a well defined uAPI for 0.

> +               info = &dev->reconnect_info;
> +       } else {
> +               /* index 1+vq_number page reserved for vduse vqs*/
> +               vq = &dev->vqs[index - 1];
> +               info = &vq->reconnect_info;
> +       }
> +       if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
> +                           PFN_DOWN(virt_to_phys((void *)info->vaddr)),
> +                           PAGE_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;
> +       struct vdpa_reconnect_info *info;
> +       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 themselves, such comment is not necessary.

> +       if (index > dev->vq_num + 1)
> +               return -EINVAL;
> +
> +       /* index 0  page reserved for vduse status*/
> +       if (index == 0) {
> +               info = &dev->reconnect_info;
> +       } else {
> +               /* index 1+vq_number page reserved for vduse vqs*/
> +               vq = &dev->vqs[index - 1];
> +               info = &vq->reconnect_info;
> +       }
> +       /*check if map pages was allocated and inited by kernel */

Typo above.

> +       if (info->vaddr == 0)
> +               return -EOPNOTSUPP;

Under which condition can we reach here?

> +
> +       /* check if the address is page aligned, if not,
> +        * this address maybe damaged
> +        */
> +       if (virt_to_phys((void *)info->vaddr) & (PAGE_SIZE - 1))
> +               return -EINVAL;

And here?

Thanks

> +
> +       /* check if Userspace App mapped the correct size
> +        * the userspace App should map one page each time
> +        */
> +       if (vma->vm_end - vma->vm_start != PAGE_SIZE)
> +               return -EOPNOTSUPP;
> +       /* VM_IO: set as a memory-mapped I/O region,This will for vq information
> +        * VM_PFNMAP: only need  the pure PFN
> +        * VM_DONTEXPAND: do not need to use mremap() in this function
> +        * VM_DONTDUMP:Do not include in the core dump
> +        */
> +       vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
> +       vma->vm_ops = &vduse_vm_ops;
> +
> +       return 0;
> +}
> +
>  static int vduse_dev_open(struct inode *inode, struct file *file)
>  {
>         int ret;
> @@ -1408,6 +1485,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
>

Re: [PATCH v2 1/5] vduse: Add function to get/free the pages for reconnection

[Jason Wang]

On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
>
> Add the function vduse_alloc_reconnnect_info_mem
> and vduse_alloc_reconnnect_info_mem
> In this 2 function, vduse will get/free (vq_num + 1)*page
> Page 0 will be used to save the reconnection information, The
> Userspace App will maintain this. Page 1 ~ vq_num + 1 will save
> the reconnection information for vqs.
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 80 ++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index 26b7e29cb900..6209e2f00278 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -41,6 +41,16 @@
>  #define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
>  #define VDUSE_MSG_DEFAULT_TIMEOUT 30
>
> +/* struct vdpa_reconnect_info saved the alloc pages info
> + * these pages will mmaped to userspace for reconnection
> + */
> +struct vdpa_reconnect_info {
> +       /* Offset (within vm_file) in PAGE_SIZE */
> +       u32 index;

Is index ever used in this series?

> +       /* virtual address for this page*/
> +       unsigned long vaddr;
> +};
> +
>  struct vduse_virtqueue {
>         u16 index;
>         u16 num_max;
> @@ -57,6 +67,7 @@ struct vduse_virtqueue {
>         struct vdpa_callback cb;
>         struct work_struct inject;
>         struct work_struct kick;
> +       struct vdpa_reconnect_info reconnect_info;
>  };
>
>  struct vduse_dev;
> @@ -106,6 +117,7 @@ struct vduse_dev {
>         u32 vq_align;
>         struct vduse_umem *umem;
>         struct mutex mem_lock;
> +       struct vdpa_reconnect_info reconnect_info;
>  };
>
>  struct vduse_dev_msg {
> @@ -1030,6 +1042,64 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
>         return ret;
>  }
>
> +static int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
> +{
> +       struct vdpa_reconnect_info *info;
> +       struct vduse_virtqueue *vq;
> +
> +       for (int i = 0; i < dev->vq_num + 1; i++) {
> +               if (i == 0) {
> +                       /*page 0 is use to save status,Userland APP will use this to
> +                        *save the information needed in reconnection,
> +                        *kernel don't need to maintain this

Please tweak the case here and to make sure the series can pass checkpatch.

> +                        */
> +                       info = &dev->reconnect_info;
> +                       info->vaddr = get_zeroed_page(GFP_KERNEL);
> +                       if (info->vaddr == 0)
> +                               return -ENOMEM;
> +                       /* index is vm Offset in PAGE_SIZE */

The case seems odd but anyhow this has been explained in the uAPI file?

Thanks

Re: [PATCH v2 3/5] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Jason Wang]

On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
>
> In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> and The number of mapping memory pages from the kernel. The userspace
> App can use this information to map the pages.
>
> Add struct vhost_reconnect_data/vhost_reconnect_vring for sync the
> information in reconnection
>
> Signed-off-by: Cindy Lu <lulu@redhat.com>
> ---
>  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++
>  include/uapi/linux/vduse.h         | 50 ++++++++++++++++++++++++++++++
>  2 files changed, 65 insertions(+)
>
> diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> index ccb30e98bab5..d0fe9a7e86ab 100644
> --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> @@ -1343,6 +1343,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
>                 ret = 0;
>                 break;
>         }
> +       case VDUSE_GET_RECONNECT_INFO: {
> +               struct vduse_reconnect_mmap_info info;
> +
> +               ret = -EFAULT;
> +               if (copy_from_user(&info, argp, sizeof(info)))
> +                       break;
> +
> +               info.size = PAGE_SIZE;
> +               info.max_index = dev->vq_num + 1;

It looks to be both PAGE_SIZE and vq_num is the well knowledge that
doesn't require a query?

> +
> +               if (copy_to_user(argp, &info, sizeof(info)))
> +                       break;
> +               ret = 0;
> +               break;
> +       }
>         default:
>                 ret = -ENOIOCTLCMD;
>                 break;
> diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> index 11bd48c72c6c..c0b7133aebfd 100644
> --- a/include/uapi/linux/vduse.h
> +++ b/include/uapi/linux/vduse.h
> @@ -3,6 +3,7 @@
>  #define _UAPI_VDUSE_H_
>
>  #include <linux/types.h>
> +#include <linux/virtio_net.h>
>
>  #define VDUSE_BASE     0x81
>
> @@ -350,4 +351,53 @@ struct vduse_dev_response {
>         };
>  };
>
> +/**
> + * struct vhost_reconnect_data - saved the reconnect info for device
> + * @version; version for userspace APP
> + * @reconnected: indetify if this is reconnected. userspace APP needs set this bit to 1
> + *               while reconnecting.kernel will use this bit to indetify if this is
> + *               reconnect

Typos.

> + * @features; Device features negotiated in the last connect.
> + * @status; Device status in last reconnect
> + * @nr_vrings; number of active vqs in last connect

What's the meaning of "active vqs"? Is it the #active_queue_pairs? If
yes, couldn't we get it from the virtio_net_config?

> + * @struct virtio_net_config config; the config in last connect
> + */
> +
> +struct vhost_reconnect_data {
> +       __u32 version;
> +       __u32 reconnected;
> +       __u64 features;
> +       __u8 status;
> +       __u32 nr_vrings;
> +       struct virtio_net_config config;

This seems like a layer violation. The fields here needs to be type
agnostic or we should introduce a new device specific area with a
union.

Or can we simply invent VDUSE_DEV_GET_CONFIG() to do this?

> +};
> +
> +/**
> + * struct vhost_reconnect_vring -saved the reconnect info for vqs
> + * when use split mode will only use last_avail_idx
> + * when use packed mode will use both last_avail_idx and avail_wrap_counter

How about last_used_idx and last_used_wrap_counter.

Btw, vDPA or vhost-vDPA has a good uAPI for this, can we reuse that?

Thanks

> + * userspace App
> + * @last_avail_idx: device last available index
> + * @avail_wrap_counter: Driver ring wrap counter
> + */
> +struct vhost_reconnect_vring {
> +       __u16 last_avail_idx;
> +       __u16 avail_wrap_counter;
> +};
> +
> +/**
> + * struct vduse_reconnect_mmap_info
> + * @size: mapping memory size, here we use page_size
> + * @max_index: the number of pages allocated in kernel,just
> + * use for sanity check
> + */
> +
> +struct vduse_reconnect_mmap_info {
> +       __u32 size;
> +       __u32 max_index;
> +};
> +
> +#define VDUSE_GET_RECONNECT_INFO \
> +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> +
>  #endif /* _UAPI_VDUSE_H_ */
> --
> 2.34.3
>

Re: [PATCH v2 4/5] vduse: update the vq_info in ioctl

[Jason Wang]

On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
>
> In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx

For driver, did you mean virtio driver?

> with reconnect info, After mapping the reconnect pages to userspace
> The userspace App will update the reconnect_time in
> struct vhost_reconnect_vring, If this is not 0 then it means this
> vq is reconnected and will update the last_avail_idx

I have a hard time understanding the above.

Thanks

Re: [PATCH v2 4/5] vduse: update the vq_info in ioctl

[Cindy Lu]

On Wed, Nov 22, 2023 at 3:12 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
> >
> > In VDUSE_VQ_GET_INFO, the driver will sync the last_avail_idx
>
> For driver, did you mean virtio driver?
>
> > with reconnect info, After mapping the reconnect pages to userspace
> > The userspace App will update the reconnect_time in
> > struct vhost_reconnect_vring, If this is not 0 then it means this
> > vq is reconnected and will update the last_avail_idx
>
> I have a hard time understanding the above.
>
sure, I will re-write this part
thanks
cindy
> Thanks
>

Re: [PATCH v2 2/5] vduse: Add file operation for mmap

[Cindy Lu]

On Wed, Nov 22, 2023 at 2:14 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
> >
> > Add the operation for mmap, The user space APP will
> > use this function to map the pages to userspace
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 79 ++++++++++++++++++++++++++++++
> >  1 file changed, 79 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 6209e2f00278..ccb30e98bab5 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1376,6 +1376,83 @@ 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;
> > +       struct vdpa_reconnect_info *info;
> > +
> > +       /* index 0  page reserved for vduse status*/
> > +       if (index == 0) {
>
> I'd avoid using magic numbers but a well defined uAPI for 0.
>
sure will Add this

> > +               info = &dev->reconnect_info;
> > +       } else {
> > +               /* index 1+vq_number page reserved for vduse vqs*/
> > +               vq = &dev->vqs[index - 1];
> > +               info = &vq->reconnect_info;
> > +       }
> > +       if (remap_pfn_range(vma, vmf->address & PAGE_MASK,
> > +                           PFN_DOWN(virt_to_phys((void *)info->vaddr)),
> > +                           PAGE_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;
> > +       struct vdpa_reconnect_info *info;
> > +       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 themselves, such comment is not necessary.
>
sure will remove this
> > +       if (index > dev->vq_num + 1)
> > +               return -EINVAL;
> > +
> > +       /* index 0  page reserved for vduse status*/
> > +       if (index == 0) {
> > +               info = &dev->reconnect_info;
> > +       } else {
> > +               /* index 1+vq_number page reserved for vduse vqs*/
> > +               vq = &dev->vqs[index - 1];
> > +               info = &vq->reconnect_info;
> > +       }
> > +       /*check if map pages was allocated and inited by kernel */
>
> Typo above.
>
> > +       if (info->vaddr == 0)
> > +               return -EOPNOTSUPP;
>
> Under which condition can we reach here?
>
if the userApp call this mmap without call the dev_create,may
cause this problem, So Add the check here
thanks
Cindy
> > +
> > +       /* check if the address is page aligned, if not,
> > +        * this address maybe damaged
> > +        */
> > +       if (virt_to_phys((void *)info->vaddr) & (PAGE_SIZE - 1))
> > +               return -EINVAL;
>
> And here?
>
if the address is not page-aligned, this information may be damaged
So add the check here
> Thanks
>
> > +
> > +       /* check if Userspace App mapped the correct size
> > +        * the userspace App should map one page each time
> > +        */
> > +       if (vma->vm_end - vma->vm_start != PAGE_SIZE)
> > +               return -EOPNOTSUPP;
> > +       /* VM_IO: set as a memory-mapped I/O region,This will for vq information
> > +        * VM_PFNMAP: only need  the pure PFN
> > +        * VM_DONTEXPAND: do not need to use mremap() in this function
> > +        * VM_DONTDUMP:Do not include in the core dump
> > +        */
> > +       vm_flags_set(vma, VM_IO | VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP);
> > +       vma->vm_ops = &vduse_vm_ops;
> > +
> > +       return 0;
> > +}
> > +
> >  static int vduse_dev_open(struct inode *inode, struct file *file)
> >  {
> >         int ret;
> > @@ -1408,6 +1485,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
> >
>

Re: [PATCH v2 3/5] vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO

[Cindy Lu]

On Wed, Nov 22, 2023 at 2:29 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
> >
> > In VDUSE_GET_RECONNECT_INFO, the Userspace App can get the map size
> > and The number of mapping memory pages from the kernel. The userspace
> > App can use this information to map the pages.
> >
> > Add struct vhost_reconnect_data/vhost_reconnect_vring for sync the
> > information in reconnection
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 15 +++++++++
> >  include/uapi/linux/vduse.h         | 50 ++++++++++++++++++++++++++++++
> >  2 files changed, 65 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index ccb30e98bab5..d0fe9a7e86ab 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -1343,6 +1343,21 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,
> >                 ret = 0;
> >                 break;
> >         }
> > +       case VDUSE_GET_RECONNECT_INFO: {
> > +               struct vduse_reconnect_mmap_info info;
> > +
> > +               ret = -EFAULT;
> > +               if (copy_from_user(&info, argp, sizeof(info)))
> > +                       break;
> > +
> > +               info.size = PAGE_SIZE;
> > +               info.max_index = dev->vq_num + 1;
>
> It looks to be both PAGE_SIZE and vq_num is the well knowledge that
> doesn't require a query?
sure we can remove this IOCTL
>
> > +
> > +               if (copy_to_user(argp, &info, sizeof(info)))
> > +                       break;
> > +               ret = 0;
> > +               break;
> > +       }
> >         default:
> >                 ret = -ENOIOCTLCMD;
> >                 break;
> > diff --git a/include/uapi/linux/vduse.h b/include/uapi/linux/vduse.h
> > index 11bd48c72c6c..c0b7133aebfd 100644
> > --- a/include/uapi/linux/vduse.h
> > +++ b/include/uapi/linux/vduse.h
> > @@ -3,6 +3,7 @@
> >  #define _UAPI_VDUSE_H_
> >
> >  #include <linux/types.h>
> > +#include <linux/virtio_net.h>
> >
> >  #define VDUSE_BASE     0x81
> >
> > @@ -350,4 +351,53 @@ struct vduse_dev_response {
> >         };
> >  };
> >
> > +/**
> > + * struct vhost_reconnect_data - saved the reconnect info for device
> > + * @version; version for userspace APP
> > + * @reconnected: indetify if this is reconnected. userspace APP needs set this bit to 1
> > + *               while reconnecting.kernel will use this bit to indetify if this is
> > + *               reconnect
>
> Typos.
>
will fix this
> > + * @features; Device features negotiated in the last connect.
> > + * @status; Device status in last reconnect
> > + * @nr_vrings; number of active vqs in last connect
>
> What's the meaning of "active vqs"? Is it the #active_queue_pairs? If
> yes, couldn't we get it from the virtio_net_config?
>

> > + * @struct virtio_net_config config; the config in last connect
> > + */
> > +
> > +struct vhost_reconnect_data {
> > +       __u32 version;
> > +       __u32 reconnected;
> > +       __u64 features;
> > +       __u8 status;
> > +       __u32 nr_vrings;
> > +       struct virtio_net_config config;
>
> This seems like a layer violation. The fields here needs to be type
> agnostic or we should introduce a new device specific area with a
> union.
>
> Or can we simply invent VDUSE_DEV_GET_CONFIG() to do this?
>
will remove virtio_net_config here
> her
> > +
> > +/**
> > + * struct vhost_reconnect_vring -saved the reconnect info for vqs
> > + * when use split mode will only use last_avail_idx
> > + * when use packed mode will use both last_avail_idx and avail_wrap_counter
>
> How about last_used_idx and last_used_wrap_counter.
>
> Btw, vDPA or vhost-vDPA has a good uAPI for this, can we reuse that?
>
I didn't get here, Do you mean set_vq_state/get_vq_state ?
Thanks
Cindy
> Thanks
>
> > + * userspace App
> > + * @last_avail_idx: device last available index
> > + * @avail_wrap_counter: Driver ring wrap counter
> > + */
> > +struct vhost_reconnect_vring {
> > +       __u16 last_avail_idx;
> > +       __u16 avail_wrap_counter;
> > +};
> > +
> > +/**
> > + * struct vduse_reconnect_mmap_info
> > + * @size: mapping memory size, here we use page_size
> > + * @max_index: the number of pages allocated in kernel,just
> > + * use for sanity check
> > + */
> > +
> > +struct vduse_reconnect_mmap_info {
> > +       __u32 size;
> > +       __u32 max_index;
> > +};
> > +
> > +#define VDUSE_GET_RECONNECT_INFO \
> > +       _IOWR(VDUSE_BASE, 0x1b, struct vduse_reconnect_mmap_info)
> > +
> >  #endif /* _UAPI_VDUSE_H_ */
> > --
> > 2.34.3
> >
>

Re: [PATCH v2 1/5] vduse: Add function to get/free the pages for reconnection

[Cindy Lu]

On Wed, Nov 22, 2023 at 2:23 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <lulu@redhat.com> wrote:
> >
> > Add the function vduse_alloc_reconnnect_info_mem
> > and vduse_alloc_reconnnect_info_mem
> > In this 2 function, vduse will get/free (vq_num + 1)*page
> > Page 0 will be used to save the reconnection information, The
> > Userspace App will maintain this. Page 1 ~ vq_num + 1 will save
> > the reconnection information for vqs.
> >
> > Signed-off-by: Cindy Lu <lulu@redhat.com>
> > ---
> >  drivers/vdpa/vdpa_user/vduse_dev.c | 80 ++++++++++++++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> >
> > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > index 26b7e29cb900..6209e2f00278 100644
> > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > @@ -41,6 +41,16 @@
> >  #define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
> >  #define VDUSE_MSG_DEFAULT_TIMEOUT 30
> >
> > +/* struct vdpa_reconnect_info saved the alloc pages info
> > + * these pages will mmaped to userspace for reconnection
> > + */
> > +struct vdpa_reconnect_info {
> > +       /* Offset (within vm_file) in PAGE_SIZE */
> > +       u32 index;
>
> Is index ever used in this series?
>
this index was use for sanity check in mmap function

> > +       /* virtual address for this page*/
> > +       unsigned long vaddr;
> > +};
> > +
> >  struct vduse_virtqueue {
> >         u16 index;
> >         u16 num_max;
> > @@ -57,6 +67,7 @@ struct vduse_virtqueue {
> >         struct vdpa_callback cb;
> >         struct work_struct inject;
> >         struct work_struct kick;
> > +       struct vdpa_reconnect_info reconnect_info;
> >  };
> >
> >  struct vduse_dev;
> > @@ -106,6 +117,7 @@ struct vduse_dev {
> >         u32 vq_align;
> >         struct vduse_umem *umem;
> >         struct mutex mem_lock;
> > +       struct vdpa_reconnect_info reconnect_info;
> >  };
> >
> >  struct vduse_dev_msg {
> > @@ -1030,6 +1042,64 @@ static int vduse_dev_reg_umem(struct vduse_dev *dev,
> >         return ret;
> >  }
> >
> > +static int vduse_alloc_reconnnect_info_mem(struct vduse_dev *dev)
> > +{
> > +       struct vdpa_reconnect_info *info;
> > +       struct vduse_virtqueue *vq;
> > +
> > +       for (int i = 0; i < dev->vq_num + 1; i++) {
> > +               if (i == 0) {
> > +                       /*page 0 is use to save status,Userland APP will use this to
> > +                        *save the information needed in reconnection,
> > +                        *kernel don't need to maintain this
>
> Please tweak the case here and to make sure the series can pass checkpatch.
>
sure will do
> > +                        */
> > +                       info = &dev->reconnect_info;
> > +                       info->vaddr = get_zeroed_page(GFP_KERNEL);
> > +                       if (info->vaddr == 0)
> > +                               return -ENOMEM;
> > +                       /* index is vm Offset in PAGE_SIZE */
>
> The case seems odd but anyhow this has been explained in the uAPI file?
>
sure will fix this
thanks
Cindy
> Thanks
>

Re: [PATCH v2 0/5] vduse: Add support for reconnection

[Cindy Lu]

On Wed, Nov 22, 2023 at 2:11 PM Jason Wang <jasowang@redhat.com> wrote:
>
> On Tue, Nov 21, 2023 at 3:30 PM Cindy Lu <lulu@redhat.com> wrote:
> >
> > Here is the reconnect support in vduse,
> >
> > kernel will allocted pages for reconnection
> > userspace need use ioctl VDUSE_GET_RECONNECT_INFO to
> > get the mmap related information and then mapping these pages
> > to userspace.
> > kernel and userspace will use these pages to sync
> > the reconnect information
> > kernel will use VDUSE_VQ_GET_INFO to sync the information
> > userspace App  will call during the "user_app_dev_start()".
>
> It would be better to describe the uAPI instead of duplicating the
> logic of the codes.
>
sure will rewrite this part
> >
> > change in V2
> > 1. Address the comments from v1
>
> It's better to be more verbose here, people can easily forget the
> comments since V1.
>
will fix this problem

> Thanks
>
> > 2. Add the document for reconnect process
> >
> > Cindy Lu (5):
> >   vduse: Add function to get/free the pages for reconnection
> >   vduse: Add file operation for mmap
> >   vduse: Add new ioctl VDUSE_GET_RECONNECT_INFO
> >   vduse: update the vq_info in ioctl
> >   Documentation: Add reconnect process for VDUSE
> >
> >  Documentation/userspace-api/vduse.rst |  29 ++++
> >  drivers/vdpa/vdpa_user/vduse_dev.c    | 198 +++++++++++++++++++++++++-
> >  include/uapi/linux/vduse.h            |  50 +++++++
> >  3 files changed, 276 insertions(+), 1 deletion(-)
> >
> > --
> > 2.34.3
> >
>