[PATCH] sbi: Prevent modification of struct sbi_platform from affecting SBI_PLATFORM_xxx_OFFSET

[Anup Patel]

On Wed, Mar 16, 2022 at 7:15 AM Xiang W <wxjstz at 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 at 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 at 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