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
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 <[email protected]>
---
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
Add the operation for mmap, The user space APP will
use this function to map the pages to userspace
Signed-off-by: Cindy Lu <[email protected]>
---
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
On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <[email protected]> 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 <[email protected]>
> ---
> 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
>
On Tue, Nov 21, 2023 at 3:30 PM Cindy Lu <[email protected]> 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
>
On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <[email protected]> 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 <[email protected]>
> ---
> 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
>
On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <[email protected]> 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 <[email protected]>
> ---
> 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
>
On Wed, Nov 22, 2023 at 2:14 PM Jason Wang <[email protected]> wrote:
>
> On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >
>
On Wed, Nov 22, 2023 at 2:29 PM Jason Wang <[email protected]> wrote:
>
> On Tue, Nov 21, 2023 at 3:31 PM Cindy Lu <[email protected]> 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 <[email protected]>
> > ---
> > 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
> >
>
On Wed, Nov 22, 2023 at 2:11 PM Jason Wang <[email protected]> wrote:
>
> On Tue, Nov 21, 2023 at 3:30 PM Cindy Lu <[email protected]> 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
> >
>