[PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Hiroshi DOYU]

From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>

Based on the discussion:
  http://www.spinics.net/lists/arm-kernel/msg72810.html

HACK: export "follow_page()" for dspbridge cache operation

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/rmgr/proc.c |   76 +++++++++++++++++++++++++++++++---------
 mm/memory.c                    |    1 +
 2 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index a75b64a..dc37beb 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -159,6 +159,8 @@
 #define PWR_TIMEOUT	 500	/* Sleep/wake timout in msec */
 #define EXTEND	      "_EXT_END"	/* Extmem end addr in DSP binary */
 
+#define DSP_CACHE_LINE 128
+
 extern char *iva_img;
 
 /*  ----------------------------------- Globals */
@@ -679,8 +681,48 @@ DSP_STATUS PROC_EnumNodes(DSP_HPROCESSOR hProcessor, OUT DSP_HNODE *aNodeTab,
 	return status;
 }
 
+/* Cache operation against kernel address instead of users */
+static int memory_sync_page(struct vm_area_struct *vma, unsigned long start,
+			    ssize_t len, enum DSP_FLUSHTYPE ftype)
+{
+	struct page *page;
+	void *kaddr;
+	unsigned long offset;
+	ssize_t rest;
+
+#ifdef CHECK_DSP_CACHE_LINE
+	if ((start & DSP_CACHE_LINE) || (len & DSP_CACHE_LINE))
+		pr_warning("%s: not aligned: %08lx(%d)\n", __func__,
+			   start, len);
+#endif
+	while (len) {
+		page = follow_page(vma, start, FOLL_GET);
+		if (!page) {
+			pr_err("%s: no page for %08lx\n", __func__, start);
+			return -EINVAL;
+		} else if (IS_ERR(page)) {
+			pr_err("%s: err page for %08lx(%lu)\n", __func__, start,
+			       IS_ERR(page));
+			return IS_ERR(page);
+		}
+
+		offset = start & ~PAGE_SIZE;
+		kaddr = page_address(page) + offset;
+		rest = min_t(ssize_t, PAGE_SIZE - offset, len);
+
+		MEM_FlushCache(kaddr, rest, ftype);
+
+		put_page(page);
+		len -= rest;
+		start += rest;
+	}
+
+	return 0;
+}
+
 /* Check if the given area blongs to process virtul memory address space */
-static int memory_check_vma(unsigned long start, u32 len)
+static int memory_sync_vma(unsigned long start, u32 len,
+			   enum DSP_FLUSHTYPE ftype)
 {
 	int err = 0;
 	unsigned long end;
@@ -690,14 +732,19 @@ static int memory_check_vma(unsigned long start, u32 len)
 	if (end <= start)
 		return -EINVAL;
 
-	down_read(&current->mm->mmap_sem);
-
 	while ((vma = find_vma(current->mm, start)) != NULL) {
+		ssize_t size;
 
-		if (vma->vm_start > start) {
-			err = -EINVAL;
+		if (vma->vm_flags & (VM_IO | VM_PFNMAP))
+			return -EINVAL;
+
+		if (vma->vm_start > start)
+			return -EINVAL;
+
+		size = min_t(ssize_t, vma->vm_end - start, len);
+		err = memory_sync_page(vma, start, size, ftype);
+		if (err)
 			break;
-		}
 
 		if (end <= vma->vm_end)
 			break;
@@ -708,8 +755,6 @@ static int memory_check_vma(unsigned long start, u32 len)
 	if (!vma)
 		err = -EINVAL;
 
-	up_read(&current->mm->mmap_sem);
-
 	return err;
 }
 
@@ -734,18 +779,15 @@ static DSP_STATUS proc_memory_sync(DSP_HPROCESSOR hProcessor, void *pMpuAddr,
 		goto err_out;
 	}
 
-	if (memory_check_vma((u32)pMpuAddr, ulSize)) {
-		GT_3trace(PROC_DebugMask, GT_7CLASS,
-			  "%s: InValid address parameters\n",
-			  __func__, pMpuAddr, ulSize);
+	down_read(&current->mm->mmap_sem);
+
+	if (memory_sync_vma((u32)pMpuAddr, ulSize, FlushMemType)) {
+		pr_err("%s: InValid address parameters %p %x\n",
+		       __func__, pMpuAddr, ulSize);
 		status = DSP_EHANDLE;
-		goto err_out;
 	}
 
-	(void)SYNC_EnterCS(hProcLock);
-	MEM_FlushCache(pMpuAddr, ulSize, FlushMemType);
-	(void)SYNC_LeaveCS(hProcLock);
-
+	up_read(&current->mm->mmap_sem);
 err_out:
 	GT_2trace(PROC_DebugMask, GT_ENTER,
 		  "Leaving %s [0x%x]", __func__, status);
diff --git a/mm/memory.c b/mm/memory.c
index 164951c..e9dc657 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1110,6 +1110,7 @@ no_page_table:
 	}
 	return page;
 }
+EXPORT_SYMBOL_GPL(follow_page);
 
 /* Can we do the FOLL_ANON optimization? */
 static inline int use_zero_page(struct vm_area_struct *vma)
-- 
1.6.0.4

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Hiroshi DOYU]

[-- Attachment #1: Type: Text/Plain, Size: 2132 bytes --]

From: "Doyu Hiroshi (Nokia-D/Helsinki)" <hiroshi.doyu@nokia.com>
Subject: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
Date: Fri, 6 Nov 2009 13:34:21 +0100

> From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> 
> Based on the discussion:
>   http://www.spinics.net/lists/arm-kernel/msg72810.html
> 
> HACK: export "follow_page()" for dspbridge cache operation
> 
> Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> ---
>  drivers/dsp/bridge/rmgr/proc.c |   76 +++++++++++++++++++++++++++++++---------
>  mm/memory.c                    |    1 +
>  2 files changed, 60 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
> index a75b64a..dc37beb 100644
> --- a/drivers/dsp/bridge/rmgr/proc.c
> +++ b/drivers/dsp/bridge/rmgr/proc.c
> @@ -159,6 +159,8 @@
>  #define PWR_TIMEOUT	 500	/* Sleep/wake timout in msec */
>  #define EXTEND	      "_EXT_END"	/* Extmem end addr in DSP binary */
>  
> +#define DSP_CACHE_LINE 128
> +
>  extern char *iva_img;
>  
>  /*  ----------------------------------- Globals */
> @@ -679,8 +681,48 @@ DSP_STATUS PROC_EnumNodes(DSP_HPROCESSOR hProcessor, OUT DSP_HNODE *aNodeTab,
>  	return status;
>  }
>  
> +/* Cache operation against kernel address instead of users */
> +static int memory_sync_page(struct vm_area_struct *vma, unsigned long start,
> +			    ssize_t len, enum DSP_FLUSHTYPE ftype)
> +{
> +	struct page *page;
> +	void *kaddr;
> +	unsigned long offset;
> +	ssize_t rest;
> +
> +#ifdef CHECK_DSP_CACHE_LINE
> +	if ((start & DSP_CACHE_LINE) || (len & DSP_CACHE_LINE))
> +		pr_warning("%s: not aligned: %08lx(%d)\n", __func__,
> +			   start, len);
> +#endif
> +	while (len) {
> +		page = follow_page(vma, start, FOLL_GET);
> +		if (!page) {
> +			pr_err("%s: no page for %08lx\n", __func__, start);
> +			return -EINVAL;
> +		} else if (IS_ERR(page)) {
> +			pr_err("%s: err page for %08lx(%lu)\n", __func__, start,
> +			       IS_ERR(page));
> +			return IS_ERR(page);
> +		}
> +
> +		offset = start & ~PAGE_SIZE;

should be:

 +		offset = start & ~PAGE_MASK;

The fixed version is attached.

[-- Attachment #2: 0001-DSPBRIDGE-cache-operation-against-kernel-address-in.patch --]
[-- Type: Application/Octet-Stream, Size: 4260 bytes --]

From 78d9847e7dc10c4567566e1571439b2b8e9e90ce Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Date: Thu, 15 Oct 2009 12:31:50 +0300
Subject: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

Based on the discussion:
  http://www.spinics.net/lists/arm-kernel/msg72810.html

HACK: export "follow_page()" for dspbridge cache operation

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 drivers/dsp/bridge/rmgr/proc.c |   76 +++++++++++++++++++++++++++++++---------
 mm/memory.c                    |    1 +
 2 files changed, 60 insertions(+), 17 deletions(-)

diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index a75b64a..c195b6f 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -159,6 +159,8 @@
 #define PWR_TIMEOUT	 500	/* Sleep/wake timout in msec */
 #define EXTEND	      "_EXT_END"	/* Extmem end addr in DSP binary */
 
+#define DSP_CACHE_LINE 128
+
 extern char *iva_img;
 
 /*  ----------------------------------- Globals */
@@ -679,8 +681,48 @@ DSP_STATUS PROC_EnumNodes(DSP_HPROCESSOR hProcessor, OUT DSP_HNODE *aNodeTab,
 	return status;
 }
 
+/* Cache operation against kernel address instead of users */
+static int memory_sync_page(struct vm_area_struct *vma, unsigned long start,
+			    ssize_t len, enum DSP_FLUSHTYPE ftype)
+{
+	struct page *page;
+	void *kaddr;
+	unsigned long offset;
+	ssize_t rest;
+
+#ifdef CHECK_DSP_CACHE_LINE
+	if ((start & DSP_CACHE_LINE) || (len & DSP_CACHE_LINE))
+		pr_warning("%s: not aligned: %08lx(%d)\n", __func__,
+			   start, len);
+#endif
+	while (len) {
+		page = follow_page(vma, start, FOLL_GET);
+		if (!page) {
+			pr_err("%s: no page for %08lx\n", __func__, start);
+			return -EINVAL;
+		} else if (IS_ERR(page)) {
+			pr_err("%s: err page for %08lx(%lu)\n", __func__, start,
+			       IS_ERR(page));
+			return IS_ERR(page);
+		}
+
+		offset = start & ~PAGE_MASK;
+		kaddr = page_address(page) + offset;
+		rest = min_t(ssize_t, PAGE_SIZE - offset, len);
+
+		MEM_FlushCache(kaddr, rest, ftype);
+
+		put_page(page);
+		len -= rest;
+		start += rest;
+	}
+
+	return 0;
+}
+
 /* Check if the given area blongs to process virtul memory address space */
-static int memory_check_vma(unsigned long start, u32 len)
+static int memory_sync_vma(unsigned long start, u32 len,
+			   enum DSP_FLUSHTYPE ftype)
 {
 	int err = 0;
 	unsigned long end;
@@ -690,14 +732,19 @@ static int memory_check_vma(unsigned long start, u32 len)
 	if (end <= start)
 		return -EINVAL;
 
-	down_read(&current->mm->mmap_sem);
-
 	while ((vma = find_vma(current->mm, start)) != NULL) {
+		ssize_t size;
 
-		if (vma->vm_start > start) {
-			err = -EINVAL;
+		if (vma->vm_flags & (VM_IO | VM_PFNMAP))
+			return -EINVAL;
+
+		if (vma->vm_start > start)
+			return -EINVAL;
+
+		size = min_t(ssize_t, vma->vm_end - start, len);
+		err = memory_sync_page(vma, start, size, ftype);
+		if (err)
 			break;
-		}
 
 		if (end <= vma->vm_end)
 			break;
@@ -708,8 +755,6 @@ static int memory_check_vma(unsigned long start, u32 len)
 	if (!vma)
 		err = -EINVAL;
 
-	up_read(&current->mm->mmap_sem);
-
 	return err;
 }
 
@@ -734,18 +779,15 @@ static DSP_STATUS proc_memory_sync(DSP_HPROCESSOR hProcessor, void *pMpuAddr,
 		goto err_out;
 	}
 
-	if (memory_check_vma((u32)pMpuAddr, ulSize)) {
-		GT_3trace(PROC_DebugMask, GT_7CLASS,
-			  "%s: InValid address parameters\n",
-			  __func__, pMpuAddr, ulSize);
+	down_read(&current->mm->mmap_sem);
+
+	if (memory_sync_vma((u32)pMpuAddr, ulSize, FlushMemType)) {
+		pr_err("%s: InValid address parameters %p %x\n",
+		       __func__, pMpuAddr, ulSize);
 		status = DSP_EHANDLE;
-		goto err_out;
 	}
 
-	(void)SYNC_EnterCS(hProcLock);
-	MEM_FlushCache(pMpuAddr, ulSize, FlushMemType);
-	(void)SYNC_LeaveCS(hProcLock);
-
+	up_read(&current->mm->mmap_sem);
 err_out:
 	GT_2trace(PROC_DebugMask, GT_ENTER,
 		  "Leaving %s [0x%x]", __func__, status);
diff --git a/mm/memory.c b/mm/memory.c
index 164951c..e9dc657 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1110,6 +1110,7 @@ no_page_table:
 	}
 	return page;
 }
+EXPORT_SYMBOL_GPL(follow_page);
 
 /* Can we do the FOLL_ANON optimization? */
 static inline int use_zero_page(struct vm_area_struct *vma)
-- 
1.6.0.4

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Hiroshi DOYU]

From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Subject: Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
Date: Fri, 13 Nov 2009 12:12:12 +0200 (EET)

> From: "Doyu Hiroshi (Nokia-D/Helsinki)" <hiroshi.doyu@nokia.com>
> Subject: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
> Date: Fri, 6 Nov 2009 13:34:21 +0100
> 
> > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > 
> > Based on the discussion:
> >   http://www.spinics.net/lists/arm-kernel/msg72810.html
> > 
> > HACK: export "follow_page()" for dspbridge cache operation
> > 
> > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > ---

Now there's no need for homebrewed cache function because we use
kernel address and can pass "virt_addr_valid()" check in
"dma_cache_maint()".

"dma_cache_maint()" shouldn't be used directly because of DMABOUNCE
support but this can be fixed later.

>From 949a76946359bb5dbf2b488dfe30315780b53183 Mon Sep 17 00:00:00 2001
From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
Date: Mon, 16 Nov 2009 19:00:29 +0200
Subject: [PATCH 1/1] DSPBRIDGE: replace homebrewed cache func with kernel API

From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>

Use "dma_cache_maint()" instead of "MEM_FlushCache()"

Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
---
 arch/arm/plat-omap/include/dspbridge/dbdefs.h |    1 +
 arch/arm/plat-omap/include/dspbridge/mem.h    |   16 -----------
 drivers/dsp/bridge/rmgr/proc.c                |   23 +++++++++++----
 drivers/dsp/bridge/services/mem.c             |   37 -------------------------
 4 files changed, 18 insertions(+), 59 deletions(-)

diff --git a/arch/arm/plat-omap/include/dspbridge/dbdefs.h b/arch/arm/plat-omap/include/dspbridge/dbdefs.h
index 78be880..d4035b3 100644
--- a/arch/arm/plat-omap/include/dspbridge/dbdefs.h
+++ b/arch/arm/plat-omap/include/dspbridge/dbdefs.h
@@ -305,6 +305,7 @@
 		PROC_INVALIDATE_MEM = 0,
 		PROC_WRITEBACK_MEM,
 		PROC_WRITEBACK_INVALIDATE_MEM,
+		PROC_FLUSHTYPE_MAX = PROC_WRITEBACK_INVALIDATE_MEM
 	} ;
 
 /* Memory Segment Status Values */
diff --git a/arch/arm/plat-omap/include/dspbridge/mem.h b/arch/arm/plat-omap/include/dspbridge/mem.h
index 4e43f21..0f0beca 100644
--- a/arch/arm/plat-omap/include/dspbridge/mem.h
+++ b/arch/arm/plat-omap/include/dspbridge/mem.h
@@ -27,7 +27,6 @@
  *      MEM_AllocPhysMem
  *      MEM_Calloc
  *      MEM_Exit
- *      MEM_FlushCache
  *      MEM_Free
  *      MEM_FreeObject
  *      MEM_FreePhysMem
@@ -173,21 +172,6 @@
 	extern void MEM_Exit(void);
 
 /*
- *  ======== MEM_FlushCache ========
- *  Purpose:
- *      Performs system cache sync with discard
- *  Parameters:
- *      pMemBuf:    Pointer to memory region to be flushed.
- *      pMemBuf:    Size of the memory region to be flushed.
- *  Returns:
- *  Requires:
- *      MEM is initialized.
- *  Ensures:
- *      Cache is synchronized
- */
-	extern void MEM_FlushCache(void *pMemBuf, u32 cBytes, s32 FlushType);
-
-/*
  *  ======== MEM_Free ========
  *  Purpose:
  *      Free the given block of system memory.
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index c195b6f..9345a7a 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -683,7 +683,7 @@ DSP_STATUS PROC_EnumNodes(DSP_HPROCESSOR hProcessor, OUT DSP_HNODE *aNodeTab,
 
 /* Cache operation against kernel address instead of users */
 static int memory_sync_page(struct vm_area_struct *vma, unsigned long start,
-			    ssize_t len, enum DSP_FLUSHTYPE ftype)
+			    ssize_t len, int dir)
 {
 	struct page *page;
 	void *kaddr;
@@ -710,7 +710,7 @@ static int memory_sync_page(struct vm_area_struct *vma, unsigned long start,
 		kaddr = page_address(page) + offset;
 		rest = min_t(ssize_t, PAGE_SIZE - offset, len);
 
-		MEM_FlushCache(kaddr, rest, ftype);
+		dma_cache_maint(kaddr, rest, dir);
 
 		put_page(page);
 		len -= rest;
@@ -721,8 +721,7 @@ static int memory_sync_page(struct vm_area_struct *vma, unsigned long start,
 }
 
 /* Check if the given area blongs to process virtul memory address space */
-static int memory_sync_vma(unsigned long start, u32 len,
-			   enum DSP_FLUSHTYPE ftype)
+static int memory_sync_vma(unsigned long start, u32 len, int dir)
 {
 	int err = 0;
 	unsigned long end;
@@ -742,7 +741,7 @@ static int memory_sync_vma(unsigned long start, u32 len,
 			return -EINVAL;
 
 		size = min_t(ssize_t, vma->vm_end - start, len);
-		err = memory_sync_page(vma, start, size, ftype);
+		err = memory_sync_page(vma, start, size, dir);
 		if (err)
 			break;
 
@@ -765,6 +764,7 @@ static DSP_STATUS proc_memory_sync(DSP_HPROCESSOR hProcessor, void *pMpuAddr,
 	/* Keep STATUS here for future additions to this function */
 	DSP_STATUS status = DSP_SOK;
 	struct PROC_OBJECT *pProcObject = (struct PROC_OBJECT *)hProcessor;
+	int dir;
 
 	DBC_Require(cRefs > 0);
 	GT_5trace(PROC_DebugMask, GT_ENTER,
@@ -779,9 +779,20 @@ static DSP_STATUS proc_memory_sync(DSP_HPROCESSOR hProcessor, void *pMpuAddr,
 		goto err_out;
 	}
 
+	/*
+	 * PROC_INVALIDATE_MEM		(0) : DMA_FROM_DEVICE  (2)
+	 * PROC_WRITEBACK_MEM		(1) : DMA_TO_DEVICE    (1)
+	 * PROC_WRITEBACK_INVALIDATE_MEM(2) : DMA_BIDIRECTIONAL(0)
+	 */
+	if (FlushMemType > PROC_FLUSHTYPE_MAX) {
+		status = DSP_EHANDLE;
+		goto err_out;
+	}
+	dir = (FlushMemType & 1) ? 1 : (~FlushMemType & 2);
+
 	down_read(&current->mm->mmap_sem);
 
-	if (memory_sync_vma((u32)pMpuAddr, ulSize, FlushMemType)) {
+	if (memory_sync_vma((u32)pMpuAddr, ulSize, dir)) {
 		pr_err("%s: InValid address parameters %p %x\n",
 		       __func__, pMpuAddr, ulSize);
 		status = DSP_EHANDLE;
diff --git a/drivers/dsp/bridge/services/mem.c b/drivers/dsp/bridge/services/mem.c
index 22f382b..fba14ab 100644
--- a/drivers/dsp/bridge/services/mem.c
+++ b/drivers/dsp/bridge/services/mem.c
@@ -25,7 +25,6 @@
  *      MEM_AllocPhysMem
  *      MEM_Calloc
  *      MEM_Exit
- *      MEM_FlushCache
  *      MEM_Free
  *      MEM_FreePhysMem
  *      MEM_Init
@@ -465,42 +464,6 @@ void MEM_Exit(void)
 }
 
 /*
- *  ======== MEM_FlushCache ========
- *  Purpose:
- *      Flush cache
- */
-void MEM_FlushCache(void *pMemBuf, u32 cBytes, s32 FlushType)
-{
-	DBC_Require(cRefs > 0);
-
-	switch (FlushType) {
-	/* invalidate only */
-	case PROC_INVALIDATE_MEM:
-		dmac_inv_range(pMemBuf, pMemBuf + cBytes);
-		outer_inv_range(__pa((u32)pMemBuf), __pa((u32)pMemBuf +
-				cBytes));
-	break;
-	/* writeback only */
-	case PROC_WRITEBACK_MEM:
-		dmac_clean_range(pMemBuf, pMemBuf + cBytes);
-		outer_clean_range(__pa((u32)pMemBuf), __pa((u32)pMemBuf +
-				  cBytes));
-	break;
-	/* writeback and invalidate */
-	case PROC_WRITEBACK_INVALIDATE_MEM:
-		dmac_flush_range(pMemBuf, pMemBuf + cBytes);
-		outer_flush_range(__pa((u32)pMemBuf), __pa((u32)pMemBuf +
-				  cBytes));
-	break;
-	default:
-		GT_1trace(MEM_debugMask, GT_6CLASS, "MEM_FlushCache: invalid "
-			  "FlushMemType 0x%x\n", FlushType);
-	break;
-	}
-
-}
-
-/*
  *  ======== MEM_VFree ========
  *  Purpose:
  *      Free the given block of system memory in virtual space.
-- 
1.6.0.4

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Russell King]

On Tue, Nov 17, 2009 at 08:41:08AM +0200, Hiroshi DOYU wrote:
> From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> Subject: Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
> Date: Fri, 13 Nov 2009 12:12:12 +0200 (EET)
> 
> > From: "Doyu Hiroshi (Nokia-D/Helsinki)" <hiroshi.doyu@nokia.com>
> > Subject: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
> > Date: Fri, 6 Nov 2009 13:34:21 +0100
> > 
> > > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > > 
> > > Based on the discussion:
> > >   http://www.spinics.net/lists/arm-kernel/msg72810.html
> > > 
> > > HACK: export "follow_page()" for dspbridge cache operation
> > > 
> > > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > > ---
> 
> Now there's no need for homebrewed cache function because we use
> kernel address and can pass "virt_addr_valid()" check in
> "dma_cache_maint()".

Note that with the advent of ARMv7/Cortex A9, dma_cache_maint() is going
away - and probably will be gone during the next merge window.  There is
no directly equivalent replacement for it - it is being replaced by two
sets of functions, one to be called prior to DMA and another to be called
after DMA has completed.

In the longer run, it is likely that the 'dmac_*_range' and 'outer_*_range'
will probably also be going away, to be replaced by two new per-cpu methods
along the lines of the above.

I think this may throw a spanner in the works for this patch, but it's
necessary to make Cortex A9 work.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Russell King]

PS, please can we have future technical kernel discussions via my linux@
address rather than my rmk@ address?

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Hiroshi DOYU]

Hi Russell,

From: ext Russell King <rmk@arm.linux.org.uk>
Subject: Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
Date: Sat, 21 Nov 2009 20:17:25 +0100

> On Tue, Nov 17, 2009 at 08:41:08AM +0200, Hiroshi DOYU wrote:
> > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > Subject: Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
> > Date: Fri, 13 Nov 2009 12:12:12 +0200 (EET)
> > 
> > > From: "Doyu Hiroshi (Nokia-D/Helsinki)" <hiroshi.doyu@nokia.com>
> > > Subject: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's
> > > Date: Fri, 6 Nov 2009 13:34:21 +0100
> > > 
> > > > From: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > > > 
> > > > Based on the discussion:
> > > >   http://www.spinics.net/lists/arm-kernel/msg72810.html
> > > > 
> > > > HACK: export "follow_page()" for dspbridge cache operation
> > > > 
> > > > Signed-off-by: Hiroshi DOYU <Hiroshi.DOYU@nokia.com>
> > > > ---
> > 
> > Now there's no need for homebrewed cache function because we use
> > kernel address and can pass "virt_addr_valid()" check in
> > "dma_cache_maint()".
> 
> Note that with the advent of ARMv7/Cortex A9, dma_cache_maint() is going
> away - and probably will be gone during the next merge window.  There is
> no directly equivalent replacement for it - it is being replaced by two
> sets of functions, one to be called prior to DMA and another to be called
> after DMA has completed.
> 
> In the longer run, it is likely that the 'dmac_*_range' and 'outer_*_range'
> will probably also be going away, to be replaced by two new per-cpu methods
> along the lines of the above.
> 
> I think this may throw a spanner in the works for this patch, but it's
> necessary to make Cortex A9 work.

Thank you for updating the status of cache operations.

I don't think that none of my patch in this thread can be a generic
solution at all, but they are just a hack for VIPT non-aliasing until
a generic solution comes. We need to fix some kernel crash at
v7_dma_*_range() just right now. Anyway, I am looking forward to your
new DMA APIs.

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Felipe Contreras]

Hi,

While investigating a bug in maemo[1] I found that this patch triggers
it, I think I found the reason.

It probably doesn't matter for upstream anymore.

On Fri, Nov 6, 2009 at 3:34 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> @@ -690,14 +732,19 @@ static int memory_check_vma(unsigned long start, u32 len)
>        if (end <= start)
>                return -EINVAL;
>
> -       down_read(&current->mm->mmap_sem);
> -
>        while ((vma = find_vma(current->mm, start)) != NULL) {
> +               ssize_t size;
>
> -               if (vma->vm_start > start) {
> -                       err = -EINVAL;
> +               if (vma->vm_flags & (VM_IO | VM_PFNMAP))
> +                       return -EINVAL;
> +
> +               if (vma->vm_start > start)
> +                       return -EINVAL;
> +
> +               size = min_t(ssize_t, vma->vm_end - start, len);

This 'len' is the total length, which is not what we want; in each
iteration the length should be decreased so that it's always the
remaining length. Right?

len -= size;

> +               err = memory_sync_page(vma, start, size, ftype);
> +               if (err)
>                        break;
> -               }
>
>                if (end <= vma->vm_end)
>                        break;

[1] https://bugs.maemo.org/show_bug.cgi?id=10813

-- 
Felipe Contreras

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Hiroshi DOYU]

Hi Felipe,

On Sun, 25 Jul 2010 22:10:32 +0200
ext Felipe Contreras <felipe.contreras@gmail.com> wrote:

> Hi,
> 
> While investigating a bug in maemo[1] I found that this patch triggers
> it, I think I found the reason.
> 
> It probably doesn't matter for upstream anymore.
> 
> On Fri, Nov 6, 2009 at 3:34 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> > @@ -690,14 +732,19 @@ static int memory_check_vma(unsigned long start, u32 len)
> >        if (end <= start)
> >                return -EINVAL;
> >
> > -       down_read(&current->mm->mmap_sem);
> > -
> >        while ((vma = find_vma(current->mm, start)) != NULL) {
> > +               ssize_t size;
> >
> > -               if (vma->vm_start > start) {
> > -                       err = -EINVAL;
> > +               if (vma->vm_flags & (VM_IO | VM_PFNMAP))
> > +                       return -EINVAL;
> > +
> > +               if (vma->vm_start > start)
> > +                       return -EINVAL;
> > +
> > +               size = min_t(ssize_t, vma->vm_end - start, len);
> 
> This 'len' is the total length, which is not what we want; in each
> iteration the length should be decreased so that it's always the
> remaining length. Right?
> 
> len -= size;

Great finding and I'm so sorry for this bug...

> 
> > +               err = memory_sync_page(vma, start, size, ftype);
> > +               if (err)
> >                        break;
> > -               }
> >
> >                if (end <= vma->vm_end)
> >                        break;
> 
> [1] https://bugs.maemo.org/show_bug.cgi?id=10813
> 
> -- 
> Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] DSPBRIDGE: cache operation against kernel address instead of user's

[Felipe Contreras]

On Thu, Jul 29, 2010 at 10:08 AM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
> On Sun, 25 Jul 2010 22:10:32 +0200
> ext Felipe Contreras <felipe.contreras@gmail.com> wrote:
>> On Fri, Nov 6, 2009 at 3:34 PM, Hiroshi DOYU <Hiroshi.DOYU@nokia.com> wrote:
>> > @@ -690,14 +732,19 @@ static int memory_check_vma(unsigned long start, u32 len)
>> >        if (end <= start)
>> >                return -EINVAL;
>> >
>> > -       down_read(&current->mm->mmap_sem);
>> > -
>> >        while ((vma = find_vma(current->mm, start)) != NULL) {
>> > +               ssize_t size;
>> >
>> > -               if (vma->vm_start > start) {
>> > -                       err = -EINVAL;
>> > +               if (vma->vm_flags & (VM_IO | VM_PFNMAP))
>> > +                       return -EINVAL;
>> > +
>> > +               if (vma->vm_start > start)
>> > +                       return -EINVAL;
>> > +
>> > +               size = min_t(ssize_t, vma->vm_end - start, len);
>>
>> This 'len' is the total length, which is not what we want; in each
>> iteration the length should be decreased so that it's always the
>> remaining length. Right?
>>
>> len -= size;
>
> Great finding and I'm so sorry for this bug...

All right, thanks for confirming :)

-- 
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html