diff mbox series

sbi: Prevent modification of struct sbi_platform from affecting SBI_PLATFORM_xxx_OFFSET

Message ID 20220316014525.57326-1-wxjstz@126.com
State Accepted
Headers show
Series sbi: Prevent modification of struct sbi_platform from affecting SBI_PLATFORM_xxx_OFFSET | expand

Commit Message

Xiang W March 16, 2022, 1:45 a.m. UTC
Add static detection to prevent the modification of struct sbi_platform
from forgetting the modification of SBI_PLATFORM_xxx_OFFSET

Signed-off-by: Xiang W <wxjstz@126.com>
---
 include/sbi/sbi_platform.h | 48 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

Comments

Anup Patel March 27, 2022, 4:27 a.m. UTC | #1
On Wed, Mar 16, 2022 at 7:15 AM Xiang W <wxjstz@126.com> wrote:
>
> Add static detection to prevent the modification of struct sbi_platform
> from forgetting the modification of SBI_PLATFORM_xxx_OFFSET
>
> Signed-off-by: Xiang W <wxjstz@126.com>

Please use a shorter patch subject and "include: " prefix.

> ---
>  include/sbi/sbi_platform.h | 48 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index 0b5ae4b..87b582e 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -175,6 +175,54 @@ struct sbi_platform {
>         const u32 *hart_index2id;
>  };
>
> +/** Prevent modification of struct sbi_platform from affecting
> + * SBI_PLATFORM_xxx_OFFSET */

This comment block should follow multi-line comment block style
/**
 *  __ xyz __
 */

> +_Static_assert(
> +       offsetof(struct sbi_platform, opensbi_version)
> +               == SBI_PLATFORM_OPENSBI_VERSION_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_OPENSBI_VERSION_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, platform_version)
> +               == SBI_PLATFORM_VERSION_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_VERSION_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, name)
> +               == SBI_PLATFORM_NAME_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_NAME_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, features)
> +               == SBI_PLATFORM_FEATURES_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_FEATURES_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, hart_count)
> +               == SBI_PLATFORM_HART_COUNT_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_HART_COUNT_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, hart_stack_size)
> +               == SBI_PLATFORM_HART_STACK_SIZE_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_HART_STACK_SIZE_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, platform_ops_addr)
> +               == SBI_PLATFORM_OPS_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_OPS_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, firmware_context)
> +               == SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET");
> +_Static_assert(
> +       offsetof(struct sbi_platform, hart_index2id)
> +               == SBI_PLATFORM_HART_INDEX2ID_OFFSET,
> +       "struct sbi_platform definition has changed, please redefine "
> +       "SBI_PLATFORM_HART_INDEX2ID_OFFSET");
> +
>  /** Get pointer to sbi_platform for sbi_scratch pointer */
>  #define sbi_platform_ptr(__s) \
>         ((const struct sbi_platform *)((__s)->platform_addr))
> --
> 2.30.2
>

Otherwise, this looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

I have taken care of the above comments at the time of merging
this patch.

Applied this patch to the riscv/opensbi repo.

Thanks,
Anup
diff mbox series

Patch

diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index 0b5ae4b..87b582e 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -175,6 +175,54 @@  struct sbi_platform {
 	const u32 *hart_index2id;
 };
 
+/** Prevent modification of struct sbi_platform from affecting
+ * SBI_PLATFORM_xxx_OFFSET */
+_Static_assert(
+	offsetof(struct sbi_platform, opensbi_version)
+		== SBI_PLATFORM_OPENSBI_VERSION_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_OPENSBI_VERSION_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, platform_version)
+		== SBI_PLATFORM_VERSION_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_VERSION_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, name)
+		== SBI_PLATFORM_NAME_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_NAME_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, features)
+		== SBI_PLATFORM_FEATURES_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_FEATURES_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, hart_count)
+		== SBI_PLATFORM_HART_COUNT_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_HART_COUNT_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, hart_stack_size)
+		== SBI_PLATFORM_HART_STACK_SIZE_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_HART_STACK_SIZE_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, platform_ops_addr)
+		== SBI_PLATFORM_OPS_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_OPS_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, firmware_context)
+		== SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_FIRMWARE_CONTEXT_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_platform, hart_index2id)
+		== SBI_PLATFORM_HART_INDEX2ID_OFFSET,
+	"struct sbi_platform definition has changed, please redefine "
+	"SBI_PLATFORM_HART_INDEX2ID_OFFSET");
+
 /** Get pointer to sbi_platform for sbi_scratch pointer */
 #define sbi_platform_ptr(__s) \
 	((const struct sbi_platform *)((__s)->platform_addr))