Messages in this thread | | | From | Jason Wang <> | Date | Thu, 7 Dec 2023 16:46:12 +0800 | Subject | Re: [PATCH v3 5/7] vduse: Add file operation for mmap |
| |
On Tue, Dec 5, 2023 at 4:35 PM Cindy Lu <lulu@redhat.com> 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 <lulu@redhat.com> > --- > 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
| |