[PATCH 0/2] DSPBRIDGE: 128 bytes alignment check

[PATCH 0/2] DSPBRIDGE: 128 bytes alignment check

[Omar Ramirez Luna]

Technical info:
https://omapzoom.org/gf/download/docmanfileversion/52/985/DSP_cache.pdf

This set of patches introduces the 128 byte alignment check,
needed to avoid corruption if the dsp is meant to write to
boundary portions of an unaligned chunk of memory.

The second patch uses a field composed of 2 bits to distinguish
if the mapped chunk is readable/writeable, so the check can be
performed on w/rw chunks of memory. 

Omar Ramirez Luna (2):
  DSPBRIDGE: add checking 128 byte alignment for dsp cache line size
  DSPBRIDGE: Distinguish between read or write buffers

 arch/arm/plat-omap/include/dspbridge/dbdefs.h |    7 ++++++-
 drivers/dsp/bridge/Kconfig                    |   14 ++++++++++++++
 drivers/dsp/bridge/rmgr/proc.c                |   19 +++++++++++++++++++
 drivers/dsp/bridge/wmd/tiomap3430.c           |    4 ++--
 4 files changed, 41 insertions(+), 3 deletions(-)

[PATCH 1/2] DSPBRIDGE: add checking 128 byte alignment for dsp cache line size

[Omar Ramirez Luna]

A buffer shared with MPU and DSP has to be aligned on both cache line
size to avoid memory corrupton with some DSP cache operations. Since
there's no way for dspbridge to know how the shared buffer will be
used like: "read-only", "write-only", "rw" through its life span, any
shared buffer passed to DSP should be on this alignment. This patch
adds checking those shared buffer alignement in bridgedriver cache
operations and prevents userland applications from causing the above
memory corruption.

Please refer to:
https://omapzoom.org/gf/download/docmanfileversion/52/985/DSP_cache.pdf

Signed-off-by: Hiroshi DOYU <hiroshi.doyu@nokia.com>
[orl: check into PROC_Map, created Kconfig option]
Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 drivers/dsp/bridge/Kconfig     |   14 ++++++++++++++
 drivers/dsp/bridge/rmgr/proc.c |   11 +++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/dsp/bridge/Kconfig b/drivers/dsp/bridge/Kconfig
index a3251c3..a973695 100644
--- a/drivers/dsp/bridge/Kconfig
+++ b/drivers/dsp/bridge/Kconfig
@@ -38,6 +38,20 @@ config BRIDGE_DEBUG
 	help
 	  Say Y to enable Bridge debugging capabilities
 
+config BRIDGE_CACHE_LINE_CHECK
+	bool "Check buffers to be 128 byte aligned"
+	depends on MPU_BRIDGE
+	default n
+	help
+	  When the DSP processes data, the DSP cache controller loads 128-Byte
+	  chunks (lines) from SDRAM and writes the data back in 128-Byte chunks.
+	  If a DMM buffer does not start and end on a 128-Byte boundary, the data
+	  preceding the start address (SA) from the 128-Byte boundary to the SA
+	  and the data at addresses trailing the end address (EA) from the EA to
+	  the next 128-Byte boundary will be loaded and written back as well.
+	  This can lead to heap corruption. Say Y, to enforce the check for 128
+	  byte alignment, buffers failing this check will be rejected.
+
 comment "Bridge Notifications"
 	depends on MPU_BRIDGE
 
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index e100d31..ac141a8 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -68,6 +68,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 */
@@ -1067,6 +1069,15 @@ dsp_status proc_map(void *hprocessor, void *pmpu_addr, u32 ul_size,
 	struct proc_object *p_proc_object = (struct proc_object *)hprocessor;
 	struct dmm_map_object *map_obj;
 
+#ifdef CONFIG_BRIDGE_CACHE_LINE_CHECK
+	if (!IS_ALIGNED((u32)pmpu_addr, DSP_CACHE_LINE) ||
+	    !IS_ALIGNED(ul_size, DSP_CACHE_LINE)) {
+		pr_err("%s: not aligned: 0x%x (%d)\n", __func__,
+						(u32)pmpu_addr, ul_size);
+		return -EFAULT;
+	}
+#endif
+
 	/* Calculate the page-aligned PA, VA and size */
 	va_align = PG_ALIGN_LOW((u32) req_addr, PG_SIZE4K);
 	pa_align = PG_ALIGN_LOW((u32) pmpu_addr, PG_SIZE4K);
-- 
1.6.2.4

[PATCH 2/2] DSPBRIDGE: Distinguish between read or write buffers

[Omar Ramirez Luna]

This patch introduces the check to differentiate the buffers
coming to the dsp through bridgedriver. So far they can be
input (read) or output (write) or rw (which are treated the
same way as an output buffer), this distinctions are made from
dsp perspective.

Since this needs to be checked on map function, unused
bits (15, 14) of flags were used to check for this argument.

As 128 byte alignment limitation doesn't affect input buffers
only writable buffers are checked. Default value for read buffers
is set to be 1, this will enforce that users of bridge will fill
the flags with significant values otherwise (if enabled) check
will reject buffers not aligned to 128 bytes (even if they fall in
the input category).

Signed-off-by: Omar Ramirez Luna <omar.ramirez@ti.com>
---
 arch/arm/plat-omap/include/dspbridge/dbdefs.h |    7 ++++++-
 drivers/dsp/bridge/rmgr/proc.c                |   16 ++++++++++++----
 drivers/dsp/bridge/wmd/tiomap3430.c           |    4 ++--
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/arm/plat-omap/include/dspbridge/dbdefs.h b/arch/arm/plat-omap/include/dspbridge/dbdefs.h
index 7fcc4aa..d01d9ae 100644
--- a/arch/arm/plat-omap/include/dspbridge/dbdefs.h
+++ b/arch/arm/plat-omap/include/dspbridge/dbdefs.h
@@ -490,7 +490,10 @@ bit 3 - MMU element size = 8bit (valid only for non mixed page entries)
 bit 4 - MMU element size = 16bit (valid only for non mixed page entries)
 bit 5 - MMU element size = 32bit (valid only for non mixed page entries)
 bit 6 - MMU element size = 64bit (valid only for non mixed page entries)
- */
+
+bit 14 - Input (read only) buffer
+bit 15 - Output (writeable) buffer
+*/
 
 /* Types of mapping attributes */
 
@@ -518,6 +521,8 @@ bit 6 - MMU element size = 64bit (valid only for non mixed page entries)
 
 #define DSP_MAPDONOTLOCK	   0x00000100
 
+#define DSP_MAP_DIR_MASK		0x3FFF
+
 #define GEM_CACHE_LINE_SIZE     128
 #define GEM_L1P_PREFETCH_SIZE   128
 
diff --git a/drivers/dsp/bridge/rmgr/proc.c b/drivers/dsp/bridge/rmgr/proc.c
index ac141a8..64659ec 100644
--- a/drivers/dsp/bridge/rmgr/proc.c
+++ b/drivers/dsp/bridge/rmgr/proc.c
@@ -70,6 +70,12 @@
 
 #define DSP_CACHE_LINE 128
 
+#define BUFMODE_MASK	(3 << 14)
+
+/* Buffer modes from DSP perspective */
+#define RBUF		0x4000		/* Input buffer */
+#define WBUF		0x8000		/* Output Buffer */
+
 extern char *iva_img;
 
 /*  ----------------------------------- Globals */
@@ -1070,11 +1076,13 @@ dsp_status proc_map(void *hprocessor, void *pmpu_addr, u32 ul_size,
 	struct dmm_map_object *map_obj;
 
 #ifdef CONFIG_BRIDGE_CACHE_LINE_CHECK
-	if (!IS_ALIGNED((u32)pmpu_addr, DSP_CACHE_LINE) ||
-	    !IS_ALIGNED(ul_size, DSP_CACHE_LINE)) {
-		pr_err("%s: not aligned: 0x%x (%d)\n", __func__,
+	if ((ul_map_attr & BUFMODE_MASK) != RBUF) {
+		if (!IS_ALIGNED((u32)pmpu_addr, DSP_CACHE_LINE) ||
+		    !IS_ALIGNED(ul_size, DSP_CACHE_LINE)) {
+			pr_err("%s: not aligned: 0x%x (%d)\n", __func__,
 						(u32)pmpu_addr, ul_size);
-		return -EFAULT;
+			return -EFAULT;
+		}
 	}
 #endif
 
diff --git a/drivers/dsp/bridge/wmd/tiomap3430.c b/drivers/dsp/bridge/wmd/tiomap3430.c
index a972ea2..cbf3203 100644
--- a/drivers/dsp/bridge/wmd/tiomap3430.c
+++ b/drivers/dsp/bridge/wmd/tiomap3430.c
@@ -1308,11 +1308,11 @@ static dsp_status bridge_brd_mem_map(struct wmd_dev_context *hDevContext,
 	if (ul_num_bytes == 0)
 		return DSP_EINVALIDARG;
 
-	if (ul_map_attr != 0) {
+	if (ul_map_attr & DSP_MAP_DIR_MASK) {
 		attrs = ul_map_attr;
 	} else {
 		/* Assign default attributes */
-		attrs = DSP_MAPVIRTUALADDR | DSP_MAPELEMSIZE16;
+		attrs = ul_map_attr | (DSP_MAPVIRTUALADDR | DSP_MAPELEMSIZE16);
 	}
 	/* Take mapping properties */
 	if (attrs & DSP_MAPBIGENDIAN)
-- 
1.6.2.4

Re: [PATCH 0/2] DSPBRIDGE: 128 bytes alignment check

[Omar Ramirez Luna]

On 3/26/2010 4:02 PM, Ramirez Luna, Omar wrote:
> Technical info:
> https://omapzoom.org/gf/download/docmanfileversion/52/985/DSP_cache.pdf
>
> This set of patches introduces the 128 byte alignment check,
> needed to avoid corruption if the dsp is meant to write to
> boundary portions of an unaligned chunk of memory.
>
> The second patch uses a field composed of 2 bits to distinguish
> if the mapped chunk is readable/writeable, so the check can be
> performed on w/rw chunks of memory.
>
> Omar Ramirez Luna (2):
>    DSPBRIDGE: add checking 128 byte alignment for dsp cache line size
>    DSPBRIDGE: Distinguish between read or write buffers
>
>   arch/arm/plat-omap/include/dspbridge/dbdefs.h |    7 ++++++-
>   drivers/dsp/bridge/Kconfig                    |   14 ++++++++++++++
>   drivers/dsp/bridge/rmgr/proc.c                |   19 +++++++++++++++++++
>   drivers/dsp/bridge/wmd/tiomap3430.c           |    4 ++--
>   4 files changed, 41 insertions(+), 3 deletions(-)
>

Pushed to dspbridge.

- omar