Skip to content

Commit 39266ca

Browse files
lyakhlgirdwood
authored andcommitted
core: assure alignment is only done on power of 2 values
Alignment should always be done on powers of 2. Enforcing this rule also allows the use of binary AND for alignment instead of much slower division. Also add compile-time checks where possible. At the moment no non-power-of-2 alignment cases are known in SOF, but a complete verification is difficult, therefore we add compile- and runtime checks, enabled by default for now. After a period of time (one or two releases) this verification option can be converted to an off by default configuration parameter. Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
1 parent b50d496 commit 39266ca

File tree

8 files changed

+83
-23
lines changed

8 files changed

+83
-23
lines changed

src/include/sof/common.h

Lines changed: 64 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,70 @@
1313

1414
/* Align the number to the nearest alignment value */
1515
#define IS_ALIGNED(size, alignment) ((size) % (alignment) == 0)
16-
#define ALIGN_UP(size, alignment) \
17-
((size) + (alignment) - 1 - ((size) + (alignment) - 1) % (alignment))
18-
#define ALIGN_DOWN(size, alignment) \
19-
((size) - ((size) % (alignment)))
16+
17+
#define is_non0_power_of_2(x) ((x) && !((x) & ((x) - 1)))
18+
19+
#define compile_fail_zero_or_true(x) (sizeof(struct{int _f : 1 - !(x); }))
20+
#define compile_fail_zero_or_false_fn(x) ({int _f[(x) - 1]; 0 & _f[0]; })
21+
22+
#define ALIGN_UP_INTERNAL(val, align) (((val) + (align) - 1) & ~((align) - 1))
23+
24+
#define VERIFY_ALIGN
25+
#ifdef VERIFY_ALIGN
26+
27+
/* For data initializers etc. */
28+
#define ALIGN_UP_COMPILE(size, alignment) \
29+
(compile_fail_zero_or_true(is_non0_power_of_2(alignment)) ? \
30+
ALIGN_UP_INTERNAL(size, alignment) : 0)
31+
32+
#ifdef __XCC__
33+
34+
/*
35+
* xcc doesn't support __builtin_constant_p() so we can only do run-time
36+
* verification
37+
*/
38+
39+
#define ALIGN_UP(size, alignment) ({ \
40+
if (!is_non0_power_of_2(alignment)) \
41+
panic(SOF_IPC_PANIC_ASSERT); \
42+
ALIGN_UP_INTERNAL(size, alignment); \
43+
})
44+
45+
#define ALIGN_DOWN(size, alignment) ({ \
46+
if (!is_non0_power_of_2(alignment)) \
47+
panic(SOF_IPC_PANIC_ASSERT); \
48+
(size) & ~((alignment) - 1); \
49+
})
50+
51+
#else
52+
53+
#define COMPILE_TIME_ALIGNED(align) (!__builtin_constant_p(align) || \
54+
is_non0_power_of_2(align))
55+
56+
#define ALIGN_UP(size, alignment) ({ \
57+
if (compile_fail_zero_or_false_fn(COMPILE_TIME_ALIGNED(alignment)) || \
58+
!is_non0_power_of_2(alignment)) \
59+
panic(SOF_IPC_PANIC_ASSERT); \
60+
ALIGN_UP_INTERNAL(size, alignment); \
61+
})
62+
63+
#define ALIGN_DOWN(size, alignment) ({ \
64+
if (compile_fail_zero_or_false_fn(COMPILE_TIME_ALIGNED(alignment)) || \
65+
!is_non0_power_of_2(alignment)) \
66+
panic(SOF_IPC_PANIC_ASSERT); \
67+
(size) & ~((alignment) - 1); \
68+
})
69+
70+
#endif
71+
72+
#else
73+
74+
#define ALIGN_UP(size, alignment) ALIGN_UP_INTERNAL(size, alignment)
75+
#define ALIGN_UP_COMPILE ALIGN_UP
76+
#define ALIGN_DOWN(size, alignment) ((size) & ~((alignment) - 1))
77+
78+
#endif
79+
2080
#define ALIGN ALIGN_UP
2181
#define DIV_ROUND_UP(val, div) (((val) + (div) - 1) / (div))
2282

src/init/ext_manifest.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
const struct ext_man_fw_version ext_man_fw_ver
1818
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
1919
.hdr.type = EXT_MAN_ELEM_FW_VERSION,
20-
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_fw_version),
21-
EXT_MAN_ALIGN),
20+
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_fw_version),
21+
EXT_MAN_ALIGN),
2222
.version = {
2323
.hdr.size = sizeof(struct sof_ipc_fw_version),
2424
.micro = SOF_MICRO,
@@ -40,8 +40,8 @@ const struct ext_man_fw_version ext_man_fw_ver
4040
const struct ext_man_cc_version ext_man_cc_ver
4141
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
4242
.hdr.type = EXT_MAN_ELEM_CC_VERSION,
43-
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_cc_version),
44-
EXT_MAN_ALIGN),
43+
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_cc_version),
44+
EXT_MAN_ALIGN),
4545
.cc_version = {
4646
.ext_hdr.hdr.size = sizeof(struct sof_ipc_cc_version),
4747
.ext_hdr.hdr.cmd = SOF_IPC_FW_READY,
@@ -59,8 +59,8 @@ const struct ext_man_cc_version ext_man_cc_ver
5959
const struct ext_man_probe_support ext_man_probe
6060
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
6161
.hdr.type = EXT_MAN_ELEM_PROBE_INFO,
62-
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_probe_support),
63-
EXT_MAN_ALIGN),
62+
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_probe_support),
63+
EXT_MAN_ALIGN),
6464
.probe = {
6565
.ext_hdr.hdr.size = sizeof(struct sof_ipc_probe_support),
6666
.ext_hdr.hdr.cmd = SOF_IPC_FW_READY,
@@ -75,8 +75,8 @@ const struct ext_man_probe_support ext_man_probe
7575
const struct ext_man_dbg_abi ext_man_dbg_info
7676
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
7777
.hdr.type = EXT_MAN_ELEM_DBG_ABI,
78-
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_dbg_abi),
79-
EXT_MAN_ALIGN),
78+
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_dbg_abi),
79+
EXT_MAN_ALIGN),
8080
.dbg_abi = {
8181
.ext_hdr.hdr.size = sizeof(struct sof_ipc_user_abi_version),
8282
.ext_hdr.hdr.cmd = SOF_IPC_FW_READY,
@@ -91,9 +91,9 @@ const struct ext_man_dbg_abi ext_man_dbg_info
9191
const struct ext_man_config_data ext_man_config
9292
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
9393
.hdr.type = EXT_MAN_ELEM_CONFIG_DATA,
94-
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_config_data) +
95-
sizeof(struct config_elem) * CONFIG_ELEM_CNT,
96-
EXT_MAN_ALIGN),
94+
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_config_data) +
95+
sizeof(struct config_elem) * CONFIG_ELEM_CNT,
96+
EXT_MAN_ALIGN),
9797
.elems = {
9898
{EXT_MAN_CONFIG_IPC_MSG_SIZE, SOF_IPC_MSG_MAX_SIZE},
9999
{EXT_MAN_CONFIG_MEMORY_USAGE_SCAN, IS_ENABLED(CONFIG_DEBUG_MEMORY_USAGE_SCAN)},

src/platform/baytrail/platform.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ const struct ext_man_windows xsram_window
8383
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
8484
.hdr = {
8585
.type = EXT_MAN_ELEM_WINDOW,
86-
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
86+
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
8787
},
8888
.window = {
8989
.ext_hdr = {

src/platform/haswell/platform.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ const struct ext_man_windows xsram_window
6969
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
7070
.hdr = {
7171
.type = EXT_MAN_ELEM_WINDOW,
72-
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
72+
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
7373
},
7474
.window = {
7575
.ext_hdr = {

src/platform/imx8/platform.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ const struct ext_man_windows xsram_window
6868
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
6969
.hdr = {
7070
.type = EXT_MAN_ELEM_WINDOW,
71-
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
71+
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
7272
},
7373
.window = {
7474
.ext_hdr = {

src/platform/imx8m/platform.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ const struct ext_man_windows xsram_window
6767
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
6868
.hdr = {
6969
.type = EXT_MAN_ELEM_WINDOW,
70-
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
70+
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
7171
},
7272
.window = {
7373
.ext_hdr = {

src/platform/intel/cavs/ext_manifest.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@
1616
const struct ext_man_cavs_config_data ext_man_cavs_config
1717
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
1818
.hdr.type = EXT_MAN_ELEM_PLATFORM_CONFIG_DATA,
19-
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_cavs_config_data) +
20-
sizeof(struct config_elem) * CAVS_CONFIG_ELEM_CNT,
21-
EXT_MAN_ALIGN),
19+
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_cavs_config_data) +
20+
sizeof(struct config_elem) * CAVS_CONFIG_ELEM_CNT,
21+
EXT_MAN_ALIGN),
2222
.elems = {
2323
{EXT_MAN_CAVS_CONFIG_LPRO, IS_ENABLED(CONFIG_CAVS_LPRO_ONLY)},
2424
{EXT_MAN_CAVS_CONFIG_OUTBOX_SIZE, SRAM_OUTBOX_SIZE},

src/platform/intel/cavs/platform.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ const struct ext_man_windows xsram_window
8282
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
8383
.hdr = {
8484
.type = EXT_MAN_ELEM_WINDOW,
85-
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
85+
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
8686
},
8787
.window = {
8888
.ext_hdr = {

0 commit comments

Comments
 (0)