diff mbox series

[3/3] include: sbi: Don't pack struct sbi_platform and sbi_platform_operations

Message ID 20210113100657.809754-4-anup.patel@wdc.com
State Accepted
Headers show
Series Avoid using packed structures | expand

Commit Message

Anup Patel Jan. 13, 2021, 10:06 a.m. UTC
We don't need to pack struct sbi_platform and sbi_platform_operations
because GCC ensures member offsets match member data type irrespective
to the target system (RV32 or RV64). This also allows GCC to generate
more optimized instruction sequence when accessing members of struct
sbi_platform and struct sbi_platform_operations.

Reported-by: Paul Campbell <taniwha@gmail.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
---
 include/sbi/sbi_platform.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alistair Francis Jan. 13, 2021, 7:44 p.m. UTC | #1
On Wed, 2021-01-13 at 15:36 +0530, Anup Patel wrote:
> We don't need to pack struct sbi_platform and sbi_platform_operations
> because GCC ensures member offsets match member data type
> irrespective
> to the target system (RV32 or RV64). This also allows GCC to generate
> more optimized instruction sequence when accessing members of struct
> sbi_platform and struct sbi_platform_operations.
> 
> Reported-by: Paul Campbell <taniwha@gmail.com>
> Signed-off-by: Anup Patel <anup.patel@wdc.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  include/sbi/sbi_platform.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index c252628..cc7e3ff 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -152,7 +152,7 @@ struct sbi_platform_operations {
>                                    const struct sbi_trap_regs *regs,
>                                    unsigned long *out_value,
>                                    struct sbi_trap_info *out_trap);
> -} __packed;
> +};
>  
>  /** Platform default per-HART stack size for exception/interrupt
> handling */
>  #define SBI_PLATFORM_DEFAULT_HART_STACK_SIZE   8192
> @@ -199,7 +199,7 @@ struct sbi_platform {
>          * 2. HART id < SBI_HARTMASK_MAX_BITS
>          */
>         const u32 *hart_index2id;
> -} __packed;
> +};
>  
>  /** Get pointer to sbi_platform for sbi_scratch pointer */
>  #define sbi_platform_ptr(__s) \
Anup Patel Jan. 15, 2021, 5:16 a.m. UTC | #2
> -----Original Message-----
> From: Alistair Francis <Alistair.Francis@wdc.com>
> Sent: 14 January 2021 01:15
> To: Atish Patra <Atish.Patra@wdc.com>; Anup Patel
> <Anup.Patel@wdc.com>
> Cc: anup@brainfault.org; opensbi@lists.infradead.org; taniwha@gmail.com
> Subject: Re: [PATCH 3/3] include: sbi: Don't pack struct sbi_platform and
> sbi_platform_operations
> 
> On Wed, 2021-01-13 at 15:36 +0530, Anup Patel wrote:
> > We don't need to pack struct sbi_platform and sbi_platform_operations
> > because GCC ensures member offsets match member data type
> irrespective
> > to the target system (RV32 or RV64). This also allows GCC to generate
> > more optimized instruction sequence when accessing members of struct
> > sbi_platform and struct sbi_platform_operations.
> >
> > Reported-by: Paul Campbell <taniwha@gmail.com>
> > Signed-off-by: Anup Patel <anup.patel@wdc.com>
> 
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> 
> Alistair
> 
> > ---
> >  include/sbi/sbi_platform.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> > index c252628..cc7e3ff 100644
> > --- a/include/sbi/sbi_platform.h
> > +++ b/include/sbi/sbi_platform.h
> > @@ -152,7 +152,7 @@ struct sbi_platform_operations {
> >                                    const struct sbi_trap_regs *regs,
> >                                    unsigned long *out_value,
> >                                    struct sbi_trap_info *out_trap); -}
> > __packed;
> > +};
> >
> >  /** Platform default per-HART stack size for exception/interrupt
> > handling */
> >  #define SBI_PLATFORM_DEFAULT_HART_STACK_SIZE   8192 @@ -199,7
> +199,7
> > @@ struct sbi_platform {
> >          * 2. HART id < SBI_HARTMASK_MAX_BITS
> >          */
> >         const u32 *hart_index2id;
> > -} __packed;
> > +};
> >
> >  /** Get pointer to sbi_platform for sbi_scratch pointer */
> >  #define sbi_platform_ptr(__s) \

Applied this patch to the riscv/opensbi repo

Regards,
Anup
diff mbox series

Patch

diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index c252628..cc7e3ff 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -152,7 +152,7 @@  struct sbi_platform_operations {
 				   const struct sbi_trap_regs *regs,
 				   unsigned long *out_value,
 				   struct sbi_trap_info *out_trap);
-} __packed;
+};
 
 /** Platform default per-HART stack size for exception/interrupt handling */
 #define SBI_PLATFORM_DEFAULT_HART_STACK_SIZE	8192
@@ -199,7 +199,7 @@  struct sbi_platform {
 	 * 2. HART id < SBI_HARTMASK_MAX_BITS
 	 */
 	const u32 *hart_index2id;
-} __packed;
+};
 
 /** Get pointer to sbi_platform for sbi_scratch pointer */
 #define sbi_platform_ptr(__s) \