Re: [PATCH] evl: fix the gdb accessing control buffer issue.

[Philippe Gerum <rpm@xenomai.org>]

Qichen Qiu <ruiqurm@gmail.com> writes:

> Currently, EVL maps the control buffer using remap_pfn_range, tagging 
> the memory with VM_IO. However, this prevents access by gdb.
>
> This patch introduces the control_mmap_access function for the control 
> VMA, enabling gdb access when CONFIG_HAVE_IOREMAP_PROT is supported on 
> the target architecture.
>
> Signed-off-by: Qichen Qiu <ruiqurm@gmail.com>
> ---
>  kernel/evl/control.c | 53 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/evl/control.c b/kernel/evl/control.c
> index 2d1b44f3b0dc..33ec47745740 100644
> --- a/kernel/evl/control.c
> +++ b/kernel/evl/control.c
> @@ -8,6 +8,7 @@
>  #include <linux/mm.h>
>  #include <linux/sched/isolation.h>
>  #include <linux/bitmap.h>
> +#include <linux/highmem.h>
>  #include <evl/memory.h>
>  #include <evl/factory.h>
>  #include <evl/tick.h>
> @@ -335,8 +336,52 @@ static long control_ioctl(struct file *filp, unsigned int cmd,
>  	return ret;
>  }
>  
> +static int control_mmap_access(struct vm_area_struct *vma, unsigned long addr,
> +			       void *buf, int len, int write)
> +{
> +	resource_size_t phys_addr;
> +	pte_t *ptep, pte;
> +	void *virt_addr;
> +	spinlock_t *ptl;
> +	struct page *page;
> +	int offset = offset_in_page(addr);
> +	int ret = -EINVAL;
> +
> +	if (follow_pte(vma, addr, &ptep, &ptl))

follow_pte() was deprecated in v6.12. We need to use the new
follow_pfnmap*() API from that point on.

> +		return -EINVAL;
> +
> +	pte = ptep_get(ptep);
> +	pte_unmap_unlock(ptep, ptl);
> +
> +	phys_addr = (resource_size_t)pte_pfn(pte) << PAGE_SHIFT;
> +	page = pfn_to_page(PFN_DOWN(phys_addr));
> +
> +	virt_addr = kmap(page) + offset;
> +	if (!virt_addr) {

Mm, not sure about the test above.

> +		ret = -ENOMEM;
> +		goto unmap;
> +	}
> +
> +	if (write) {
> +		memcpy(virt_addr, buf, len);
> +	} else {
> +		memcpy(buf, virt_addr, len);
> +	}
> +	ret = len;
> +
> +unmap:
> +	kunmap(page);
> +
> +	return ret;
> +}
> +
> +static const struct vm_operations_struct control_mmap_ops = {
> +	.access = control_mmap_access
> +};
> +
>  static int control_mmap(struct file *filp, struct vm_area_struct *vma)
>  {
> +	int err;
>  	void *p = evl_get_heap_base(&evl_shared_heap);
>  	unsigned long pfn = __pa(p) >> PAGE_SHIFT;
>  	size_t len = vma->vm_end - vma->vm_start;
> @@ -344,7 +389,13 @@ static int control_mmap(struct file *filp, struct vm_area_struct *vma)
>  	if (len != evl_shm_size)
>  		return -EINVAL;
>  
> -	return remap_pfn_range(vma, vma->vm_start, pfn, len, PAGE_SHARED);
> +	err = remap_pfn_range(vma, vma->vm_start, pfn, len, PAGE_SHARED);
> +	if (err < 0){
> +		return err;
> +	}
> +
> +	vma->vm_ops = &control_mmap_ops;
> +	return 0;
>  }
>  
>  static const struct file_operations control_fops = {


There may be another (version-agnostic) way: simply obtain the heap
memory from the vmalloc space instead of kmem/logical, then use
vm_insert_page() to populate the mapping, which would drop the
requirement for an access trampoline to pfn ranges. Dovetail deals with
vmalloc memory pinning appropriately.

-- 
Philippe.