Skip to content

[RFC] core: assure alignment is only done on power of 2 values #3671

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lgirdwood merged 1 commit into from
Jan 8, 2021
Merged

[RFC] core: assure alignment is only done on power of 2 values #3671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 64 additions & 4 deletions src/include/sof/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,70 @@

/* Align the number to the nearest alignment value */
#define IS_ALIGNED(size, alignment) ((size) % (alignment) == 0)
#define ALIGN_UP(size, alignment) \
((size) + (alignment) - 1 - ((size) + (alignment) - 1) % (alignment))
#define ALIGN_DOWN(size, alignment) \
((size) - ((size) % (alignment)))

#define is_non0_power_of_2(x) ((x) && !((x) & ((x) - 1)))

#define compile_fail_zero_or_true(x) (sizeof(struct{int _f : 1 - !(x); }))
#define compile_fail_zero_or_false_fn(x) ({int _f[(x) - 1]; 0 & _f[0]; })

#define ALIGN_UP_INTERNAL(val, align) (((val) + (align) - 1) & ~((align) - 1))

#define VERIFY_ALIGN
#ifdef VERIFY_ALIGN

/* For data initializers etc. */
#define ALIGN_UP_COMPILE(size, alignment) \
(compile_fail_zero_or_true(is_non0_power_of_2(alignment)) ? \
ALIGN_UP_INTERNAL(size, alignment) : 0)

#ifdef __XCC__

Copy link
Member

@lgirdwood lgirdwood Jan 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this different for XCC, need to state this delta in the comments.

Copy link
Collaborator Author

@lyakh lyakh Jan 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lgirdwood comment added, conflict resolved. However I don't like all qemu targets failing (at least before). Let's wait for the new test to finish and if failures are still there, who can help get some debugging information from those failures?

/*
* xcc doesn't support __builtin_constant_p() so we can only do run-time
* verification
*/

#define ALIGN_UP(size, alignment) ({ \
if (!is_non0_power_of_2(alignment)) \
panic(SOF_IPC_PANIC_ASSERT); \
ALIGN_UP_INTERNAL(size, alignment); \
})

#define ALIGN_DOWN(size, alignment) ({ \
if (!is_non0_power_of_2(alignment)) \
panic(SOF_IPC_PANIC_ASSERT); \
(size) & ~((alignment) - 1); \
})

#else

#define COMPILE_TIME_ALIGNED(align) (!__builtin_constant_p(align) || \
is_non0_power_of_2(align))

#define ALIGN_UP(size, alignment) ({ \
if (compile_fail_zero_or_false_fn(COMPILE_TIME_ALIGNED(alignment)) || \
!is_non0_power_of_2(alignment)) \
panic(SOF_IPC_PANIC_ASSERT); \
ALIGN_UP_INTERNAL(size, alignment); \
})

#define ALIGN_DOWN(size, alignment) ({ \
if (compile_fail_zero_or_false_fn(COMPILE_TIME_ALIGNED(alignment)) || \
!is_non0_power_of_2(alignment)) \
panic(SOF_IPC_PANIC_ASSERT); \
(size) & ~((alignment) - 1); \
})

#endif

#else

#define ALIGN_UP(size, alignment) ALIGN_UP_INTERNAL(size, alignment)
#define ALIGN_UP_COMPILE ALIGN_UP
#define ALIGN_DOWN(size, alignment) ((size) & ~((alignment) - 1))

#endif

#define ALIGN ALIGN_UP
#define DIV_ROUND_UP(val, div) (((val) + (div) - 1) / (div))

Expand Down
22 changes: 11 additions & 11 deletions src/init/ext_manifest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
const struct ext_man_fw_version ext_man_fw_ver
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
.hdr.type = EXT_MAN_ELEM_FW_VERSION,
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_fw_version),
EXT_MAN_ALIGN),
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_fw_version),
EXT_MAN_ALIGN),
.version = {
.hdr.size = sizeof(struct sof_ipc_fw_version),
.micro = SOF_MICRO,
Expand All @@ -40,8 +40,8 @@ const struct ext_man_fw_version ext_man_fw_ver
const struct ext_man_cc_version ext_man_cc_ver
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
.hdr.type = EXT_MAN_ELEM_CC_VERSION,
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_cc_version),
EXT_MAN_ALIGN),
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_cc_version),
EXT_MAN_ALIGN),
.cc_version = {
.ext_hdr.hdr.size = sizeof(struct sof_ipc_cc_version),
.ext_hdr.hdr.cmd = SOF_IPC_FW_READY,
Expand All @@ -59,8 +59,8 @@ const struct ext_man_cc_version ext_man_cc_ver
const struct ext_man_probe_support ext_man_probe
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
.hdr.type = EXT_MAN_ELEM_PROBE_INFO,
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_probe_support),
EXT_MAN_ALIGN),
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_probe_support),
EXT_MAN_ALIGN),
.probe = {
.ext_hdr.hdr.size = sizeof(struct sof_ipc_probe_support),
.ext_hdr.hdr.cmd = SOF_IPC_FW_READY,
Expand All @@ -75,8 +75,8 @@ const struct ext_man_probe_support ext_man_probe
const struct ext_man_dbg_abi ext_man_dbg_info
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
.hdr.type = EXT_MAN_ELEM_DBG_ABI,
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_dbg_abi),
EXT_MAN_ALIGN),
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_dbg_abi),
EXT_MAN_ALIGN),
.dbg_abi = {
.ext_hdr.hdr.size = sizeof(struct sof_ipc_user_abi_version),
.ext_hdr.hdr.cmd = SOF_IPC_FW_READY,
Expand All @@ -91,9 +91,9 @@ const struct ext_man_dbg_abi ext_man_dbg_info
const struct ext_man_config_data ext_man_config
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
.hdr.type = EXT_MAN_ELEM_CONFIG_DATA,
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_config_data) +
sizeof(struct config_elem) * CONFIG_ELEM_CNT,
EXT_MAN_ALIGN),
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_config_data) +
sizeof(struct config_elem) * CONFIG_ELEM_CNT,
EXT_MAN_ALIGN),
.elems = {
{EXT_MAN_CONFIG_IPC_MSG_SIZE, SOF_IPC_MSG_MAX_SIZE},
{EXT_MAN_CONFIG_MEMORY_USAGE_SCAN, IS_ENABLED(CONFIG_DEBUG_MEMORY_USAGE_SCAN)},
Expand Down
2 changes: 1 addition & 1 deletion src/platform/baytrail/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ const struct ext_man_windows xsram_window
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
.hdr = {
.type = EXT_MAN_ELEM_WINDOW,
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
},
.window = {
.ext_hdr = {
Expand Down
2 changes: 1 addition & 1 deletion src/platform/haswell/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const struct ext_man_windows xsram_window
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
.hdr = {
.type = EXT_MAN_ELEM_WINDOW,
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
},
.window = {
.ext_hdr = {
Expand Down
2 changes: 1 addition & 1 deletion src/platform/imx8/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const struct ext_man_windows xsram_window
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
.hdr = {
.type = EXT_MAN_ELEM_WINDOW,
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
},
.window = {
.ext_hdr = {
Expand Down
2 changes: 1 addition & 1 deletion src/platform/imx8m/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const struct ext_man_windows xsram_window
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
.hdr = {
.type = EXT_MAN_ELEM_WINDOW,
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
},
.window = {
.ext_hdr = {
Expand Down
6 changes: 3 additions & 3 deletions src/platform/intel/cavs/ext_manifest.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
const struct ext_man_cavs_config_data ext_man_cavs_config
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") = {
.hdr.type = EXT_MAN_ELEM_PLATFORM_CONFIG_DATA,
.hdr.elem_size = ALIGN_UP(sizeof(struct ext_man_cavs_config_data) +
sizeof(struct config_elem) * CAVS_CONFIG_ELEM_CNT,
EXT_MAN_ALIGN),
.hdr.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_cavs_config_data) +
sizeof(struct config_elem) * CAVS_CONFIG_ELEM_CNT,
EXT_MAN_ALIGN),
.elems = {
{EXT_MAN_CAVS_CONFIG_LPRO, IS_ENABLED(CONFIG_CAVS_LPRO_ONLY)},
{EXT_MAN_CAVS_CONFIG_OUTBOX_SIZE, SRAM_OUTBOX_SIZE},
Expand Down
2 changes: 1 addition & 1 deletion src/platform/intel/cavs/platform.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ const struct ext_man_windows xsram_window
__aligned(EXT_MAN_ALIGN) __section(".fw_metadata") __unused = {
.hdr = {
.type = EXT_MAN_ELEM_WINDOW,
.elem_size = ALIGN_UP(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
.elem_size = ALIGN_UP_COMPILE(sizeof(struct ext_man_windows), EXT_MAN_ALIGN),
},
.window = {
.ext_hdr = {
Expand Down