DSPBRIDGE: 128 bytes alignment check
Subsystems (2)
cover DSPBRIDGE: 128 bytes alignment check
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
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
--- drivers/dsp/bridge/Kconfig | 14 ++++++++++++++ drivers/dsp/bridge/rmgr/proc.c | 11 +++++++++++ 2 files changed, 25 insertions(+), 0 deletions(-)
drivers/dsp/bridge/Kconfig +14 -0
@@ -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
drivers/dsp/bridge/rmgr/proc.c +11 -0
@@ -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 );
Signature · 1 line
1.6.2.4
PATCH 2/2 DSPBRIDGE: Distinguish between read or write buffers
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).
--- 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(-)
arch/arm/plat-omap/include/dspbridge/dbdefs.h +6 -1
@@ -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
drivers/dsp/bridge/rmgr/proc.c +12 -4
@@ -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
drivers/dsp/bridge/wmd/tiomap3430.c +2 -2
@@ -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 )
Signature · 1 line
1.6.2.4
Omar Ramirez Luna
AppliedQuoted 15 lines · Omar
Technical info:
https://omapzoom.org/gf/download/docmanfileversion/52/985/DSP_cache.pdfThis 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
Quoted 6 lines
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