[Sound-open-firmware] [PATCH 0/3] Long overdue minor fixes
one patch to make the scripts more useful for people creating releases (i don't think we should use always-obsolete BKMs but working scripts) and two patches to fix coverity (was on the back burner since January...)
Pierre-Louis Bossart (3): sof: scripts: force PATH to compile xtensa without environment dependencies sof: solve circular dependencies, move rmemcpy/arch_memcpy definitions mailbox: move multi-line macros to static inlines
scripts/xtensa-build-all.sh | 1 + src/arch/xtensa/include/arch/reef.h | 5 --- src/arch/xtensa/include/arch/string.h | 40 ++++++++++++++++++++++++ src/include/reef/mailbox.h | 59 ++++++++++++++++++++++------------- src/include/reef/string.h | 47 ++++++++++++++++++++++++++++ 5 files changed, 125 insertions(+), 27 deletions(-) create mode 100644 src/arch/xtensa/include/arch/string.h create mode 100644 src/include/reef/string.h
The ROOT is already assumed to be as $pwd/../xtensa-root/$ROOT, set the PATH as well
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- scripts/xtensa-build-all.sh | 1 + 1 file changed, 1 insertion(+)
diff --git a/scripts/xtensa-build-all.sh b/scripts/xtensa-build-all.sh index 5111383..227254f 100755 --- a/scripts/xtensa-build-all.sh +++ b/scripts/xtensa-build-all.sh @@ -54,6 +54,7 @@ do PLATFORM="cannonlake" ROOT="xtensa-cnl-elf" fi + PATH=$pwd/../xtensa-root/$ROOT/bin:$PATH ./configure --with-arch=xtensa --with-platform=$PLATFORM --with-root-dir=$pwd/../xtensa-root/$ROOT --host=$ROOT make clean make
The preprocessor doesn't seem to care about circular dependencies, the compiler does. So before we replace multi-line macros with static inlines, move rmemcpy/arch_memcpy to a reef/string.h and arch/string.h header file.
This is mostly useful for the mailbox functions, which depend on rmemcpy but are also called from arch/reef.h dump/panic utilities
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/arch/xtensa/include/arch/reef.h | 5 ---- src/arch/xtensa/include/arch/string.h | 40 +++++++++++++++++++++++++++++ src/include/reef/mailbox.h | 1 + src/include/reef/string.h | 47 +++++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 5 deletions(-) create mode 100644 src/arch/xtensa/include/arch/string.h create mode 100644 src/include/reef/string.h
diff --git a/src/arch/xtensa/include/arch/reef.h b/src/arch/xtensa/include/arch/reef.h index 592221e..7f9b7db 100644 --- a/src/arch/xtensa/include/arch/reef.h +++ b/src/arch/xtensa/include/arch/reef.h @@ -40,11 +40,6 @@ /* architecture specific stack frames to dump */ #define ARCH_STACK_DUMP_FRAMES 32
-void *xthal_memcpy(void *dst, const void *src, size_t len); - -#define arch_memcpy(dest, src, size) \ - xthal_memcpy(dest, src, size) - static inline void *arch_get_stack_ptr(void) { void *ptr; diff --git a/src/arch/xtensa/include/arch/string.h b/src/arch/xtensa/include/arch/string.h new file mode 100644 index 0000000..73f78bc --- /dev/null +++ b/src/arch/xtensa/include/arch/string.h @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2018, Intel Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the Intel Corporation nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * Author: Liam Girdwood liam.r.girdwood@linux.intel.com + * + */ + +#ifndef __INCLUDE_ARCH_STRING_REEF__ +#define __INCLUDE_ARCH_STRING_REEF__ + +void *xthal_memcpy(void *dst, const void *src, size_t len); + +#define arch_memcpy(dest, src, size) \ + xthal_memcpy(dest, src, size) + +#endif diff --git a/src/include/reef/mailbox.h b/src/include/reef/mailbox.h index f134548..cedfbb7 100644 --- a/src/include/reef/mailbox.h +++ b/src/include/reef/mailbox.h @@ -34,6 +34,7 @@ #include <platform/mailbox.h> #include <arch/cache.h> #include <stdint.h> +#include <reef/string.h>
/* 4k should be enough for everyone ..... */ #define IPC_MAX_MAILBOX_BYTES 0x1000 diff --git a/src/include/reef/string.h b/src/include/reef/string.h new file mode 100644 index 0000000..42f47c9 --- /dev/null +++ b/src/include/reef/string.h @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2018, Intel Corporation + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are met: + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * * Neither the name of the Intel Corporation nor the + * names of its contributors may be used to endorse or promote products + * derived from this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * Author: Liam Girdwood liam.r.girdwood@linux.intel.com + */ + +#ifndef __INCLUDE_STRING_REEF__ +#define __INCLUDE_STRING_REEF__ + +#include <arch/string.h> + +/* C memcpy for arch that don't have arch_memcpy() */ +void cmemcpy(void *dest, void *src, size_t size); + +#if defined(arch_memcpy) +#define rmemcpy(dest, src, size) \ + arch_memcpy(dest, src, size) +#else +#define rmemcpy(dest, src, size) \ + cmemcpy(dest, src, size) +#endif + +#endif
Detected with Coverity:
In do_notify: The indentation of this code suggests it is nested when it is not. (CWE-483)
multi_stmt_macro: The macro on this line expands into multiple statements, only the first of which is nested within the preceding parent while the rest are not.
if (msg->rx_size && msg->rx_size < SOF_IPC_MSG_MAX_SIZE) mailbox_dspbox_read(msg->rx_data, 0, msg->rx_size);
Move mailbox macros to inline functions to remove the issue, keep indentation the same and add typecasts as needed
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- src/include/reef/mailbox.h | 58 ++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 22 deletions(-)
diff --git a/src/include/reef/mailbox.h b/src/include/reef/mailbox.h index cedfbb7..b237e44 100644 --- a/src/include/reef/mailbox.h +++ b/src/include/reef/mailbox.h @@ -63,27 +63,41 @@ #define mailbox_get_debug_size() \ MAILBOX_DEBUG_SIZE
-#define mailbox_dspbox_write(dest, src, bytes) \ - rmemcpy((void*)(MAILBOX_DSPBOX_BASE + dest), src, bytes); \ - dcache_writeback_region((void*)(MAILBOX_DSPBOX_BASE + dest), bytes); - -#define mailbox_dspbox_read(dest, src, bytes) \ - dcache_invalidate_region((void*)(MAILBOX_DSPBOX_BASE + src), bytes); \ - rmemcpy(dest, (void*)(MAILBOX_DSPBOX_BASE + src), bytes); - -#define mailbox_hostbox_write(dest, src, bytes) \ - rmemcpy((void*)(MAILBOX_HOSTBOX_BASE + dest), src, bytes); \ - dcache_writeback_region((void*)(MAILBOX_HOSTBOX_BASE + dest), bytes); - -#define mailbox_hostbox_read(dest, src, bytes) \ - dcache_invalidate_region((void*)(MAILBOX_HOSTBOX_BASE + src), bytes); \ - rmemcpy(dest, (void*)(MAILBOX_HOSTBOX_BASE + src), bytes); - -#define mailbox_stream_write(dest, src, bytes) \ - do { \ - rmemcpy((void *)(MAILBOX_STREAM_BASE + dest), src, bytes); \ - dcache_writeback_region((void *)(MAILBOX_STREAM_BASE + dest), \ - bytes); \ - } while (0) +static inline +void mailbox_dspbox_write(size_t offset, const void *src, size_t bytes) +{ + rmemcpy((void *)(MAILBOX_DSPBOX_BASE + offset), src, bytes); + dcache_writeback_region((void *)(MAILBOX_DSPBOX_BASE + offset), bytes); +} + +static inline +void mailbox_dspbox_read(void *dest, size_t offset, size_t bytes) +{ + dcache_invalidate_region((void *)(MAILBOX_DSPBOX_BASE + offset), bytes); + rmemcpy(dest, (void *)(MAILBOX_DSPBOX_BASE + offset), bytes); +} + +static inline +void mailbox_hostbox_write(size_t offset, const void *src, size_t bytes) +{ + rmemcpy((void *)(MAILBOX_HOSTBOX_BASE + offset), src, bytes); + dcache_writeback_region((void *)(MAILBOX_HOSTBOX_BASE + offset), bytes); +} + +static inline +void mailbox_hostbox_read(void *dest, size_t offset, size_t bytes) +{ + dcache_invalidate_region((void *)(MAILBOX_HOSTBOX_BASE + offset), + bytes); + rmemcpy(dest, (void *)(MAILBOX_HOSTBOX_BASE + offset), bytes); +} + +static inline +void mailbox_stream_write(size_t offset, const void *src, size_t bytes) +{ + rmemcpy((void *)(MAILBOX_STREAM_BASE + offset), src, bytes); + dcache_writeback_region((void *)(MAILBOX_STREAM_BASE + offset), + bytes); +}
#endif
On Tue, 2018-04-03 at 08:57 -0500, Pierre-Louis Bossart wrote:
one patch to make the scripts more useful for people creating releases (i don't think we should use always-obsolete BKMs but working scripts) and two patches to fix coverity (was on the back burner since January...)
Pierre-Louis Bossart (3): sof: scripts: force PATH to compile xtensa without environment dependencies sof: solve circular dependencies, move rmemcpy/arch_memcpy definitions mailbox: move multi-line macros to static inlines
All applied.
Thanks
Liam
participants (2)
-
Liam Girdwood -
Pierre-Louis Bossart