Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1444089 > unrolled thread

[PATCH 0/8] Cache-coherent DMA access using UIO

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/8] Cache-coherent DMA access using UIO Anup Patel <anup.patel@broadcom.com> - 2016-07-15 11:10 +0200
    [PATCH 2/8] uio: Add new UIO_MEM_PHYS_CACHE type for mem regions Anup Patel <anup.patel@broadcom.com> - 2016-07-15 11:10 +0200
    [PATCH 5/8] uio: fix dmem_region_start computation Anup Patel <anup.patel@broadcom.com> - 2016-07-15 11:10 +0200
      Re: [PATCH 5/8] uio: fix dmem_region_start computation Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2016-07-15 13:40 +0200
        Re: [PATCH 5/8] uio: fix dmem_region_start computation Jan Viktorin <viktorin@rehivetech.com> - 2016-07-15 14:10 +0200
    [PATCH 1/8] uio: code style cleanup Anup Patel <anup.patel@broadcom.com> - 2016-07-15 11:10 +0200
    [PATCH 3/8] uio: Add new UIO_MEM_DEVICE type for mem regions Anup Patel <anup.patel@broadcom.com> - 2016-07-15 11:10 +0200

#1444089 — [PATCH 0/8] Cache-coherent DMA access using UIO

The goal of this patchset is to improve UIO framework and UIO dmem
driver to allow cache-coherent DMA accesses from user-space.

This patchset is based on two previous patchsets:
1) [PATCH v5 0/6] UIO driver for APM X-Gene QMTM
(Refer, http://www.spinics.net/lists/devicetree/msg58244.html)
2) [PATCH 0/4] Fix and extend uio_dmem_genirq
(Refer, https://lkml.org/lkml/2016/5/17/141)

We have adopted only patch0-3 of patchset1 which was abandoned
long time back. We have taken care of last few unaddressed comments
on these patches.

The patchset2 is quite recent has been adopted entirely. We have
taken care review comments on these patches too.

This patchset is based on v4.7-rc7 tag and it is available in uio-v1
branch of https://github.com/Broadcom/arm64-linux.git

Ankit Jindal (3):
  uio: code style cleanup
  uio: Add new UIO_MEM_PHYS_CACHE type for mem regions
  Documentation: Update documentation for UIO_MEM_PHYS_CACHE and
    UIO_MEM_DEVICE

Anup Patel (2):
  uio: Add new UIO_MEM_DEVICE type for mem regions
  uio: Use new memtypes in uio_dmem_genirq

Jan Viktorin (3):
  uio: fix dmem_region_start computation
  uio: UIO_IRQ_NONE is a valid option for uioinfo->irq
  uio: bind uio_dmem_genirq via OF

 Documentation/DocBook/uio-howto.tmpl |   6 +-
 drivers/uio/uio.c                    |  32 +++++---
 drivers/uio/uio_dmem_genirq.c        | 141 +++++++++++++++++++++++++----------
 include/linux/uio_driver.h           |  10 ++-
 4 files changed, 136 insertions(+), 53 deletions(-)

-- 
1.9.1

[toc] | [next] | [standalone]

#1444090 — [PATCH 2/8] uio: Add new UIO_MEM_PHYS_CACHE type for mem regions

From: Ankit Jindal <thatsjindal@gmail.com>

Currently, three types of mem regions are supported: UIO_MEM_PHYS,
UIO_MEM_LOGICAL and UIO_MEM_VIRTUAL. Among these UIO_MEM_PHYS helps
UIO driver export physcial memory to user space as non-cacheable
user memory. Typcially memory-mapped registers of a device are exported
to user space as UIO_MEM_PHYS type mem region. The UIO_MEM_PHYS type
is not efficient if dma-capable devices are capable of maintaining
coherency with CPU caches.

This patch adds new type UIO_MEM_PHYS_CACHE for mem regions to enable
cacheable access to physical memory from user space.

Signed-off-by: Ankit Jindal <thatsjindal@gmail.com>
Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/uio/uio.c          | 16 +++++++++++++---
 include/linux/uio_driver.h |  9 +++++----
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index f2729b7..0e53076 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -641,7 +641,7 @@ static const struct vm_operations_struct uio_physical_vm_ops = {
 #endif
 };
 
-static int uio_mmap_physical(struct vm_area_struct *vma)
+static int uio_mmap_physical(struct vm_area_struct *vma, int memtype)
 {
 	struct uio_device *idev = vma->vm_private_data;
 	int mi = uio_find_mem_index(vma);
@@ -656,7 +656,16 @@ static int uio_mmap_physical(struct vm_area_struct *vma)
 		return -EINVAL;
 
 	vma->vm_ops = &uio_physical_vm_ops;
-	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	switch (memtype) {
+	case UIO_MEM_PHYS:
+		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+		break;
+	case UIO_MEM_PHYS_CACHE:
+		/* Do nothing. */
+		break;
+	default:
+		return -EINVAL;
+	}
 
 	/*
 	 * We cannot use the vm_iomap_memory() helper here,
@@ -704,7 +713,8 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
 
 	switch (idev->info->mem[mi].memtype) {
 	case UIO_MEM_PHYS:
-		return uio_mmap_physical(vma);
+	case UIO_MEM_PHYS_CACHE:
+		return uio_mmap_physical(vma, idev->info->mem[mi].memtype);
 	case UIO_MEM_LOGICAL:
 	case UIO_MEM_VIRTUAL:
 		return uio_mmap_logical(vma);
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index 32c0e83..31359aee 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -124,10 +124,11 @@ extern void uio_event_notify(struct uio_info *info);
 #define UIO_IRQ_NONE	0
 
 /* defines for uio_mem->memtype */
-#define UIO_MEM_NONE	0
-#define UIO_MEM_PHYS	1
-#define UIO_MEM_LOGICAL	2
-#define UIO_MEM_VIRTUAL 3
+#define UIO_MEM_NONE		0
+#define UIO_MEM_PHYS		1
+#define UIO_MEM_LOGICAL		2
+#define UIO_MEM_VIRTUAL		3
+#define UIO_MEM_PHYS_CACHE	4
 
 /* defines for uio_port->porttype */
 #define UIO_PORT_NONE	0
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1444092 — [PATCH 5/8] uio: fix dmem_region_start computation

From: Jan Viktorin <viktorin@rehivetech.com>

The variable i contains a total number of resources (including
IORESOURCE_IRQ). However, we want the dmem_region_start to point
after the last resource of type IORESOURCE_MEM. The original behaviour
leads (very likely) to skipping several UIO mapping regions and makes
them useless. Fix this by computing dmem_region_start from the uiomem
which points to the last used UIO mapping.

Fixes: 0a0c3b5a24bd ("Add new uio device for dynamic memory allocation")

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
---
 drivers/uio/uio_dmem_genirq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
index 915facb..e1134a4 100644
--- a/drivers/uio/uio_dmem_genirq.c
+++ b/drivers/uio/uio_dmem_genirq.c
@@ -229,7 +229,7 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev)
 		++uiomem;
 	}
 
-	priv->dmem_region_start = i;
+	priv->dmem_region_start = uiomem - &uioinfo->mem[0];
 	priv->num_dmem_regions = pdata->num_dynamic_regions;
 
 	for (i = 0; i < pdata->num_dynamic_regions; ++i) {
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1444206 — Re: [PATCH 5/8] uio: fix dmem_region_start computation

On Fri, Jul 15, 2016 at 02:34:00PM +0530, Anup Patel wrote:
> From: Jan Viktorin <viktorin@rehivetech.com>
> 
> The variable i contains a total number of resources (including
> IORESOURCE_IRQ). However, we want the dmem_region_start to point
> after the last resource of type IORESOURCE_MEM. The original behaviour
> leads (very likely) to skipping several UIO mapping regions and makes
> them useless. Fix this by computing dmem_region_start from the uiomem
> which points to the last used UIO mapping.
> 
> Fixes: 0a0c3b5a24bd ("Add new uio device for dynamic memory allocation")
> 
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> ---
>  drivers/uio/uio_dmem_genirq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Why isn't this patch first in the series, with a stable marking, and
your signed off on it (you are forwarding it on to me, so you need to
add your mark to it as well, I can't take it otherwise.)

> diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
> index 915facb..e1134a4 100644
> --- a/drivers/uio/uio_dmem_genirq.c
> +++ b/drivers/uio/uio_dmem_genirq.c
> @@ -229,7 +229,7 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev)
>  		++uiomem;
>  	}
>  
> -	priv->dmem_region_start = i;
> +	priv->dmem_region_start = uiomem - &uioinfo->mem[0];

Are you sure about this?  It doesn't look correct at first glance, I'm
loath to take this without a bunch of testing.  Were you able to test
this out to verify it doesn't break working hardware?

thanks,

greg k-h

[toc] | [prev] | [next] | [standalone]

#1444226 — Re: [PATCH 5/8] uio: fix dmem_region_start computation

On Fri, 15 Jul 2016 20:32:24 +0900
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Fri, Jul 15, 2016 at 02:34:00PM +0530, Anup Patel wrote:
> > From: Jan Viktorin <viktorin@rehivetech.com>
> > 
> > The variable i contains a total number of resources (including
> > IORESOURCE_IRQ). However, we want the dmem_region_start to point
> > after the last resource of type IORESOURCE_MEM. The original behaviour
> > leads (very likely) to skipping several UIO mapping regions and makes
> > them useless. Fix this by computing dmem_region_start from the uiomem
> > which points to the last used UIO mapping.
> > 
> > Fixes: 0a0c3b5a24bd ("Add new uio device for dynamic memory allocation")
> > 
> > Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> > ---
> >  drivers/uio/uio_dmem_genirq.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)  
> 
> Why isn't this patch first in the series, with a stable marking, and
> your signed off on it (you are forwarding it on to me, so you need to
> add your mark to it as well, I can't take it otherwise.)
> 
> > diff --git a/drivers/uio/uio_dmem_genirq.c b/drivers/uio/uio_dmem_genirq.c
> > index 915facb..e1134a4 100644
> > --- a/drivers/uio/uio_dmem_genirq.c
> > +++ b/drivers/uio/uio_dmem_genirq.c
> > @@ -229,7 +229,7 @@ static int uio_dmem_genirq_probe(struct platform_device *pdev)
> >  		++uiomem;
> >  	}
> >  
> > -	priv->dmem_region_start = i;
> > +	priv->dmem_region_start = uiomem - &uioinfo->mem[0];  
> 
> Are you sure about this?  It doesn't look correct at first glance, I'm

The loop over resources counts all resources in _i_ and mem resources in _uiomem_
pointer. Thus, the dmem_region_start cannot be derived from _i_ but from _uiomem_.

> loath to take this without a bunch of testing.  Were you able to test
> this out to verify it doesn't break working hardware?

This is a good point, however, what is the working hardware? I could not find
any application of the uio_dmem_genirq anywhere online. Any example, nothing.

I am afraid that nobody is using more then 1 memory resource with this driver
and so nobody could discover this to be a bug. I was working with more then 2
resources.

I'd be glad if somebody else can test it on any other working setup.

Regards
Jan

> 
> thanks,
> 
> greg k-h

[toc] | [prev] | [next] | [standalone]

#1444093 — [PATCH 1/8] uio: code style cleanup

From: Ankit Jindal <thatsjindal@gmail.com>

This patch fixes the indentation of switch-case block in uio driver.

Signed-off-by: Ankit Jindal <thatsjindal@gmail.com>
Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/uio/uio.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index fba021f..f2729b7 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -703,13 +703,13 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
 	}
 
 	switch (idev->info->mem[mi].memtype) {
-		case UIO_MEM_PHYS:
-			return uio_mmap_physical(vma);
-		case UIO_MEM_LOGICAL:
-		case UIO_MEM_VIRTUAL:
-			return uio_mmap_logical(vma);
-		default:
-			return -EINVAL;
+	case UIO_MEM_PHYS:
+		return uio_mmap_physical(vma);
+	case UIO_MEM_LOGICAL:
+	case UIO_MEM_VIRTUAL:
+		return uio_mmap_logical(vma);
+	default:
+		return -EINVAL;
 	}
 }
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1444095 — [PATCH 3/8] uio: Add new UIO_MEM_DEVICE type for mem regions

On ARM64, the MMU supports special memory attributes for device
memory/registers. Due to this we have pgprot_device() provided
by asm/pgtable.h of arch/arm64.

On architectures that do not have special MMU attribute for device
memory/registers, the asm-generic/pgtable.h maps pgprot_device()
to pgprot_noncached().

This patch introduces a new UIO mem region type UIO_MEM_DEVICE to
represent device registers/memory. The UIO device drivers should
prefer this new UIO mem region type for memory mapped device registers.

Signed-off-by: Anup Patel <anup.patel@broadcom.com>
---
 drivers/uio/uio.c          | 4 ++++
 include/linux/uio_driver.h | 1 +
 2 files changed, 5 insertions(+)

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index 0e53076..a00990c 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -663,6 +663,9 @@ static int uio_mmap_physical(struct vm_area_struct *vma, int memtype)
 	case UIO_MEM_PHYS_CACHE:
 		/* Do nothing. */
 		break;
+	case UIO_MEM_DEVICE:
+		vma->vm_page_prot = pgprot_device(vma->vm_page_prot);
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -714,6 +717,7 @@ static int uio_mmap(struct file *filep, struct vm_area_struct *vma)
 	switch (idev->info->mem[mi].memtype) {
 	case UIO_MEM_PHYS:
 	case UIO_MEM_PHYS_CACHE:
+	case UIO_MEM_DEVICE:
 		return uio_mmap_physical(vma, idev->info->mem[mi].memtype);
 	case UIO_MEM_LOGICAL:
 	case UIO_MEM_VIRTUAL:
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index 31359aee..7349f95 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -129,6 +129,7 @@ extern void uio_event_notify(struct uio_info *info);
 #define UIO_MEM_LOGICAL		2
 #define UIO_MEM_VIRTUAL		3
 #define UIO_MEM_PHYS_CACHE	4
+#define UIO_MEM_DEVICE		5
 
 /* defines for uio_port->porttype */
 #define UIO_PORT_NONE	0
-- 
1.9.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web