diff mbox series

[1/8] powerpc/rtas: ensure 8-byte alignment for struct rtas_args

Message ID 20230220-rtas-queue-for-6-4-v1-1-010e4416f13f@linux.ibm.com (mailing list archive)
State Accepted
Commit f40b0f6c5c27de167fdd10e541e0a4b5f2bc772b
Headers show
Series RTAS changes for 6.4 | expand

Commit Message

Nathan Lynch via B4 Relay March 6, 2023, 9:33 p.m. UTC 
From: Nathan Lynch <nathanl@linux.ibm.com>

CHRP and PAPR agree: "In order to make an RTAS call, the operating
system must construct an argument call buffer aligned on an eight byte
boundary in physically contiguous real memory [...]." (7.2.7 Calling
Mechanism and Conventions).

struct rtas_args is the type used for this argument call buffer. The
unarchitected 'rets' member happens to produce 8-byte alignment for
the struct on 64-bit targets in practice. But without an alignment
directive the structure will have only 4-byte alignment on 32-bit
targets:

  $ nm b/{before,after}/chrp32/vmlinux | grep rtas_args
  c096881c b rtas_args
  c0968820 b rtas_args

Add an alignment directive to the struct rtas_args declaration so all
instances have the alignment required by the specs. rtas-types.h no
longer refers to any spinlock types, so drop the spinlock_types.h
inclusion while we're here.

Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
---
 arch/powerpc/include/asm/rtas-types.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Andrew Donnellan March 23, 2023, 4 a.m. UTC | #1 
On Mon, 2023-03-06 at 15:33 -0600, Nathan Lynch via B4 Relay wrote:
> > From: Nathan Lynch <nathanl@linux.ibm.com>
> > 
> > CHRP and PAPR agree: "In order to make an RTAS call, the operating
> > system must construct an argument call buffer aligned on an eight
> > byte
> > boundary in physically contiguous real memory [...]." (7.2.7
> > Calling
> > Mechanism and Conventions).
> > 
> > struct rtas_args is the type used for this argument call buffer.
> > The
> > unarchitected 'rets' member happens to produce 8-byte alignment for
> > the struct on 64-bit targets in practice. But without an alignment
> > directive the structure will have only 4-byte alignment on 32-bit
> > targets:
> > 
> >   $ nm b/{before,after}/chrp32/vmlinux | grep rtas_args
> >   c096881c b rtas_args
> >   c0968820 b rtas_args
> > 
> > Add an alignment directive to the struct rtas_args declaration so
> > all
> > instances have the alignment required by the specs. rtas-types.h no
> > longer refers to any spinlock types, so drop the spinlock_types.h
> > inclusion while we're here.
> > 
> > Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

> > ---
> >  arch/powerpc/include/asm/rtas-types.h | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/powerpc/include/asm/rtas-types.h
> > b/arch/powerpc/include/asm/rtas-types.h
> > index f2ad4a96cbc5..861145c8a021 100644
> > --- a/arch/powerpc/include/asm/rtas-types.h
> > +++ b/arch/powerpc/include/asm/rtas-types.h
> > @@ -2,7 +2,8 @@
> >  #ifndef _ASM_POWERPC_RTAS_TYPES_H
> >  #define _ASM_POWERPC_RTAS_TYPES_H
> >  
> > -#include <linux/spinlock_types.h>
> > +#include <linux/compiler_attributes.h>
> > +#include <linux/sizes.h>
> >  
> >  typedef __be32 rtas_arg_t;
> >  
> > @@ -12,7 +13,7 @@ struct rtas_args {
> >         __be32 nret;
> >         rtas_arg_t args[16];
> >         rtas_arg_t *rets;     /* Pointer to return values in
> > args[].
> > */
> > -};
> > +} __aligned(SZ_8);

Nowhere else in the kernel uses __aligned(SZ_8) over just __aligned(8),
which I suppose would also save an include, but I don't care either
way.

> >  
> >  struct rtas_t {
> >         unsigned long entry;            /* physical address pointer
> > */
> >
diff mbox series

Patch

diff --git a/arch/powerpc/include/asm/rtas-types.h b/arch/powerpc/include/asm/rtas-types.h
index f2ad4a96cbc5..861145c8a021 100644
--- a/arch/powerpc/include/asm/rtas-types.h
+++ b/arch/powerpc/include/asm/rtas-types.h
@@ -2,7 +2,8 @@ 
 #ifndef _ASM_POWERPC_RTAS_TYPES_H
 #define _ASM_POWERPC_RTAS_TYPES_H
 
-#include <linux/spinlock_types.h>
+#include <linux/compiler_attributes.h>
+#include <linux/sizes.h>
 
 typedef __be32 rtas_arg_t;
 
@@ -12,7 +13,7 @@  struct rtas_args {
 	__be32 nret;
 	rtas_arg_t args[16];
 	rtas_arg_t *rets;     /* Pointer to return values in args[]. */
-};
+} __aligned(SZ_8);
 
 struct rtas_t {
 	unsigned long entry;		/* physical address pointer */