Skip to content

subsys: ipc: icbmsg: Reduce block alignment to 32-bits #79604

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
nashif merged 1 commit into from
Nov 22, 2024
Merged

subsys: ipc: icbmsg: Reduce block alignment to 32-bits #79604

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
19 changes: 12 additions & 7 deletions subsys/ipc/ipc_service/backends/ipc_icbmsg.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1295,6 +1295,11 @@
.release_rx_buffer = release_rx_buffer,
};

/**
* Required block alignment.
*/
#define BLOCK_ALIGNMENT sizeof(uint32_t)

/**
* Number of bytes per each ICMsg message. It is used to calculate size of ICMsg area.
*/
Expand All @@ -1308,29 +1313,29 @@
(PBUF_HEADER_OVERHEAD(GET_CACHE_ALIGNMENT(i)) + 2 * BYTES_PER_ICMSG_MESSAGE)

/**
* Returns required block alignment for instance "i".
* Returns required data cache alignment for instance "i".
*/
#define GET_CACHE_ALIGNMENT(i) \
MAX(sizeof(uint32_t), DT_INST_PROP_OR(i, dcache_alignment, 0))
MAX(BLOCK_ALIGNMENT, DT_INST_PROP_OR(i, dcache_alignment, 0))

Check notice on line 1320 in subsys/ipc/ipc_service/backends/ipc_icbmsg.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

subsys/ipc/ipc_service/backends/ipc_icbmsg.c:1320 
-#define GET_CACHE_ALIGNMENT(i) \
-	MAX(BLOCK_ALIGNMENT, DT_INST_PROP_OR(i, dcache_alignment, 0))
+#define GET_CACHE_ALIGNMENT(i) MAX(BLOCK_ALIGNMENT, DT_INST_PROP_OR(i, dcache_alignment, 0))

Check notice on line 1320 in subsys/ipc/ipc_service/backends/ipc_icbmsg.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

subsys/ipc/ipc_service/backends/ipc_icbmsg.c:1320 
-#define GET_CACHE_ALIGNMENT(i) \
-	MAX(BLOCK_ALIGNMENT, DT_INST_PROP_OR(i, dcache_alignment, 0))
+#define GET_CACHE_ALIGNMENT(i) MAX(BLOCK_ALIGNMENT, DT_INST_PROP_OR(i, dcache_alignment, 0))
/**
* Calculates minimum size required for ICMsg region for specific number of local
* and remote blocks. The minimum size ensures that ICMsg queue is will never overflow
* because it can hold data message for each local block and release message
* for each remote block.
*/
#define GET_ICMSG_MIN_SIZE(i, local_blocks, remote_blocks) \
#define GET_ICMSG_MIN_SIZE(i, local_blocks, remote_blocks) ROUND_UP( \
(ICMSG_BUFFER_OVERHEAD(i) + BYTES_PER_ICMSG_MESSAGE * \
(local_blocks + remote_blocks))
(local_blocks + remote_blocks)), GET_CACHE_ALIGNMENT(i))

/**
* Calculate aligned block size by evenly dividing remaining space after removing
* the space for ICMsg.
*/
#define GET_BLOCK_SIZE(i, total_size, local_blocks, remote_blocks) ROUND_DOWN( \
((total_size) - GET_ICMSG_MIN_SIZE(i, (local_blocks), (remote_blocks))) / \
(local_blocks), GET_CACHE_ALIGNMENT(i))
(local_blocks), BLOCK_ALIGNMENT)

Check notice on line 1338 in subsys/ipc/ipc_service/backends/ipc_icbmsg.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

subsys/ipc/ipc_service/backends/ipc_icbmsg.c:1338 
-#define GET_ICMSG_MIN_SIZE(i, local_blocks, remote_blocks) ROUND_UP(			\
-	(ICMSG_BUFFER_OVERHEAD(i) + BYTES_PER_ICMSG_MESSAGE *				\
-	 (local_blocks + remote_blocks)), GET_CACHE_ALIGNMENT(i))
+#define GET_ICMSG_MIN_SIZE(i, local_blocks, remote_blocks)                                         \
+	ROUND_UP((ICMSG_BUFFER_OVERHEAD(i) +                                                       \
+		  BYTES_PER_ICMSG_MESSAGE * (local_blocks + remote_blocks)),                       \
+		 GET_CACHE_ALIGNMENT(i))
 
 /**
  * Calculate aligned block size by evenly dividing remaining space after removing
  * the space for ICMsg.
  */
-#define GET_BLOCK_SIZE(i, total_size, local_blocks, remote_blocks) ROUND_DOWN(		\
-	((total_size) - GET_ICMSG_MIN_SIZE(i, (local_blocks), (remote_blocks))) /	\
-	(local_blocks), BLOCK_ALIGNMENT)
+#define GET_BLOCK_SIZE(i, total_size, local_blocks, remote_blocks)                                 \
+	ROUND_DOWN(((total_size) - GET_ICMSG_MIN_SIZE(i, (local_blocks), (remote_blocks))) /       \
+			   (local_blocks),                                                         \
+		   BLOCK_ALIGNMENT)

Check notice on line 1338 in subsys/ipc/ipc_service/backends/ipc_icbmsg.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

subsys/ipc/ipc_service/backends/ipc_icbmsg.c:1338 
-#define GET_ICMSG_MIN_SIZE(i, local_blocks, remote_blocks) ROUND_UP(			\
-	(ICMSG_BUFFER_OVERHEAD(i) + BYTES_PER_ICMSG_MESSAGE *				\
-	 (local_blocks + remote_blocks)), GET_CACHE_ALIGNMENT(i))
+#define GET_ICMSG_MIN_SIZE(i, local_blocks, remote_blocks)                                         \
+	ROUND_UP((ICMSG_BUFFER_OVERHEAD(i) +                                                       \
+		  BYTES_PER_ICMSG_MESSAGE * (local_blocks + remote_blocks)),                       \
+		 GET_CACHE_ALIGNMENT(i))
 
 /**
  * Calculate aligned block size by evenly dividing remaining space after removing
  * the space for ICMsg.
  */
-#define GET_BLOCK_SIZE(i, total_size, local_blocks, remote_blocks) ROUND_DOWN(		\
-	((total_size) - GET_ICMSG_MIN_SIZE(i, (local_blocks), (remote_blocks))) /	\
-	(local_blocks), BLOCK_ALIGNMENT)
+#define GET_BLOCK_SIZE(i, total_size, local_blocks, remote_blocks)                                 \
+	ROUND_DOWN(((total_size) - GET_ICMSG_MIN_SIZE(i, (local_blocks), (remote_blocks))) /       \
+			   (local_blocks),                                                         \
+		   BLOCK_ALIGNMENT)
/**
* Calculate offset where area for blocks starts which is just after the ICMsg.
*/
Expand Down Expand Up @@ -1434,11 +1439,11 @@
}; \
BUILD_ASSERT(IS_POWER_OF_TWO(GET_CACHE_ALIGNMENT(i)), \
"This module supports only power of two cache alignment"); \
BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, tx, rx) >= GET_CACHE_ALIGNMENT(i)) && \
BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, tx, rx) >= BLOCK_ALIGNMENT) && \
(GET_BLOCK_SIZE_INST(i, tx, rx) < \
GET_MEM_SIZE_INST(i, tx)), \
"TX region is too small for provided number of blocks"); \
BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, rx, tx) >= GET_CACHE_ALIGNMENT(i)) && \
BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, rx, tx) >= BLOCK_ALIGNMENT) && \
(GET_BLOCK_SIZE_INST(i, rx, tx) < \
GET_MEM_SIZE_INST(i, rx)), \
"RX region is too small for provided number of blocks"); \
Expand All @@ -1452,5 +1457,5 @@
POST_KERNEL, \
CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY, \
&backend_ops);

Check notice on line 1460 in subsys/ipc/ipc_service/backends/ipc_icbmsg.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

subsys/ipc/ipc_service/backends/ipc_icbmsg.c:1460 
-#define DEFINE_BACKEND_DEVICE(i)							\
-	SYS_BITARRAY_DEFINE_STATIC(tx_usage_bitmap_##i, DT_INST_PROP(i, tx_blocks));	\
-	SYS_BITARRAY_DEFINE_STATIC(rx_hold_bitmap_##i, DT_INST_PROP(i, rx_blocks));	\
-	PBUF_DEFINE(tx_icbmsg_pb_##i,							\
-			GET_MEM_ADDR_INST(i, tx),					\
-			GET_ICMSG_SIZE_INST(i, tx, rx),					\
-			GET_CACHE_ALIGNMENT(i));					\
-	PBUF_DEFINE(rx_icbmsg_pb_##i,							\
-			GET_MEM_ADDR_INST(i, rx),					\
-			GET_ICMSG_SIZE_INST(i, rx, tx),					\
-			GET_CACHE_ALIGNMENT(i));					\
-	static struct backend_data backend_data_##i = {					\
-		.control_data = {							\
-			.tx_pb = &tx_icbmsg_pb_##i,					\
-			.rx_pb = &rx_icbmsg_pb_##i,					\
-		}									\
-	};										\
-	static MAYBE_CONST struct icbmsg_config backend_config_##i =			\
-	{										\
-		.control_config = {							\
-			.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),			\
-			.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx),			\
-		},									\
-		.tx = {									\
-			.blocks_ptr = (uint8_t *)GET_BLOCKS_ADDR_INST(i, tx, rx),	\
-			.block_count = DT_INST_PROP(i, tx_blocks),			\
-			.block_size = GET_BLOCK_SIZE_INST(i, tx, rx),			\
-		},									\
-		.rx = {									\
-			.blocks_ptr = (uint8_t *)GET_BLOCKS_ADDR_INST(i, rx, tx),	\
-			.block_count = DT_INST_PROP(i, rx_blocks),			\
-			.block_size = GET_BLOCK_SIZE_INST(i, rx, tx),			\
-		},									\
-		.tx_usage_bitmap = &tx_usage_bitmap_##i,				\
-		.rx_hold_bitmap = &rx_hold_bitmap_##i,					\
-	};										\
-	BUILD_ASSERT(IS_POWER_OF_TWO(GET_CACHE_ALIGNMENT(i)),				\
-		     "This module supports only power of two cache alignment");		\
-	BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, tx, rx) >= BLOCK_ALIGNMENT) &&		\
-		     (GET_BLOCK_SIZE_INST(i, tx, rx) <					\
-		      GET_MEM_SIZE_INST(i, tx)),					\
-		     "TX region is too small for provided number of blocks");		\
-	BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, rx, tx) >= BLOCK_ALIGNMENT) &&		\
-		     (GET_BLOCK_SIZE_INST(i, rx, tx) <					\
-		      GET_MEM_SIZE_INST(i, rx)),					\
-		     "RX region is too small for provided number of blocks");		\
-	BUILD_ASSERT(DT_INST_PROP(i, rx_blocks) <= 256, "Too many RX blocks");		\
-	BUILD_ASSERT(DT_INST_PROP(i, tx_blocks) <= 256, "Too many TX blocks");		\
-	DEVICE_DT_INST_DEFINE(i,							\
-			      &backend_init,						\
-			      NULL,							\
-			      &backend_data_##i,					\
-			      &backend_config_##i,					\
-			      POST_KERNEL,						\
-			      CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY,			\
-			      &backend_ops);
+#define DEFINE_BACKEND_DEVICE(i)                                                                   \
+	SYS_BITARRAY_DEFINE_STATIC(tx_usage_bitmap_##i, DT_INST_PROP(i, tx_blocks));               \
+	SYS_BITARRAY_DEFINE_STATIC(rx_hold_bitmap_##i, DT_INST_PROP(i, rx_blocks));                \
+	PBUF_DEFINE(tx_icbmsg_pb_##i, GET_MEM_ADDR_INST(i, tx), GET_ICMSG_SIZE_INST(i, tx, rx),    \
+		    GET_CACHE_ALIGNMENT(i));                                                       \
+	PBUF_DEFINE(rx_icbmsg_pb_##i, GET_MEM_ADDR_INST(i, rx), GET_ICMSG_SIZE_INST(i, rx, tx),    \
+		    GET_CACHE_ALIGNMENT(i));                                                       \
+	static struct backend_data backend_data_##i = {.control_data = {                           \
+							       .tx_pb = &tx_icbmsg_pb_##i,         \
+							       .rx_pb = &rx_icbmsg_pb_##i,         \
+						       }};                                         \
+	static MAYBE_CONST struct icbmsg_config backend_config_##i = {                             \
+		.control_config =                                                                  \
+			{                                                                          \
+				.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),                           \
+				.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx),                           \
+			},                                                                         \
+		.tx =                                                                              \
+			{

Check notice on line 1460 in subsys/ipc/ipc_service/backends/ipc_icbmsg.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change 

subsys/ipc/ipc_service/backends/ipc_icbmsg.c:1460 
-#define DEFINE_BACKEND_DEVICE(i)							\
-	SYS_BITARRAY_DEFINE_STATIC(tx_usage_bitmap_##i, DT_INST_PROP(i, tx_blocks));	\
-	SYS_BITARRAY_DEFINE_STATIC(rx_hold_bitmap_##i, DT_INST_PROP(i, rx_blocks));	\
-	PBUF_DEFINE(tx_icbmsg_pb_##i,							\
-			GET_MEM_ADDR_INST(i, tx),					\
-			GET_ICMSG_SIZE_INST(i, tx, rx),					\
-			GET_CACHE_ALIGNMENT(i));					\
-	PBUF_DEFINE(rx_icbmsg_pb_##i,							\
-			GET_MEM_ADDR_INST(i, rx),					\
-			GET_ICMSG_SIZE_INST(i, rx, tx),					\
-			GET_CACHE_ALIGNMENT(i));					\
-	static struct backend_data backend_data_##i = {					\
-		.control_data = {							\
-			.tx_pb = &tx_icbmsg_pb_##i,					\
-			.rx_pb = &rx_icbmsg_pb_##i,					\
-		}									\
-	};										\
-	static MAYBE_CONST struct icbmsg_config backend_config_##i =			\
-	{										\
-		.control_config = {							\
-			.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),			\
-			.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx),			\
-		},									\
-		.tx = {									\
-			.blocks_ptr = (uint8_t *)GET_BLOCKS_ADDR_INST(i, tx, rx),	\
-			.block_count = DT_INST_PROP(i, tx_blocks),			\
-			.block_size = GET_BLOCK_SIZE_INST(i, tx, rx),			\
-		},									\
-		.rx = {									\
-			.blocks_ptr = (uint8_t *)GET_BLOCKS_ADDR_INST(i, rx, tx),	\
-			.block_count = DT_INST_PROP(i, rx_blocks),			\
-			.block_size = GET_BLOCK_SIZE_INST(i, rx, tx),			\
-		},									\
-		.tx_usage_bitmap = &tx_usage_bitmap_##i,				\
-		.rx_hold_bitmap = &rx_hold_bitmap_##i,					\
-	};										\
-	BUILD_ASSERT(IS_POWER_OF_TWO(GET_CACHE_ALIGNMENT(i)),				\
-		     "This module supports only power of two cache alignment");		\
-	BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, tx, rx) >= BLOCK_ALIGNMENT) &&		\
-		     (GET_BLOCK_SIZE_INST(i, tx, rx) <					\
-		      GET_MEM_SIZE_INST(i, tx)),					\
-		     "TX region is too small for provided number of blocks");		\
-	BUILD_ASSERT((GET_BLOCK_SIZE_INST(i, rx, tx) >= BLOCK_ALIGNMENT) &&		\
-		     (GET_BLOCK_SIZE_INST(i, rx, tx) <					\
-		      GET_MEM_SIZE_INST(i, rx)),					\
-		     "RX region is too small for provided number of blocks");		\
-	BUILD_ASSERT(DT_INST_PROP(i, rx_blocks) <= 256, "Too many RX blocks");		\
-	BUILD_ASSERT(DT_INST_PROP(i, tx_blocks) <= 256, "Too many TX blocks");		\
-	DEVICE_DT_INST_DEFINE(i,							\
-			      &backend_init,						\
-			      NULL,							\
-			      &backend_data_##i,					\
-			      &backend_config_##i,					\
-			      POST_KERNEL,						\
-			      CONFIG_IPC_SERVICE_REG_BACKEND_PRIORITY,			\
-			      &backend_ops);
+#define DEFINE_BACKEND_DEVICE(i)                                                                   \
+	SYS_BITARRAY_DEFINE_STATIC(tx_usage_bitmap_##i, DT_INST_PROP(i, tx_blocks));               \
+	SYS_BITARRAY_DEFINE_STATIC(rx_hold_bitmap_##i, DT_INST_PROP(i, rx_blocks));                \
+	PBUF_DEFINE(tx_icbmsg_pb_##i, GET_MEM_ADDR_INST(i, tx), GET_ICMSG_SIZE_INST(i, tx, rx),    \
+		    GET_CACHE_ALIGNMENT(i));                                                       \
+	PBUF_DEFINE(rx_icbmsg_pb_##i, GET_MEM_ADDR_INST(i, rx), GET_ICMSG_SIZE_INST(i, rx, tx),    \
+		    GET_CACHE_ALIGNMENT(i));                                                       \
+	static struct backend_data backend_data_##i = {.control_data = {                           \
+							       .tx_pb = &tx_icbmsg_pb_##i,         \
+							       .rx_pb = &rx_icbmsg_pb_##i,         \
+						       }};                                         \
+	static MAYBE_CONST struct icbmsg_config backend_config_##i = {                             \
+		.control_config =                                                                  \
+			{                                                                          \
+				.mbox_tx = MBOX_DT_SPEC_INST_GET(i, tx),                           \
+				.mbox_rx = MBOX_DT_SPEC_INST_GET(i, rx),                           \
+			},                                                                         \
+		.tx =                                                                              \
+			{
DT_INST_FOREACH_STATUS_OKAY(DEFINE_BACKEND_DEVICE)
Loading