diff mbox

[2/4] x86: adjust xsave structure attributes

Message ID 56AB4CAA02000078000CC53F@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich Jan. 29, 2016, 10:27 a.m. UTC 
The packed attribute was pointlessly used here - there are no
misaligned fields, and hence even if the attribute took effect, it
would at best lead to the compiler generating worse code.

At the same time specify the required alignment of the fpu_sse sub-
structure, such that the various typeof() uses on that field obtain
pointers to properly aligned memory (knowledge which a compiler may
want to make use of).

Also add suitable build-time checks.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
x86: adjust xsave structure attributes

The packed attribute was pointlessly used here - there are no
misaligned fields, and hence even if the attribute took effect, it
would at best lead to the compiler generating worse code.

At the same time specify the required alignment of the fpu_sse sub-
structure, such that the various typeof() uses on that field obtain
pointers to properly aligned memory (knowledge which a compiler may
want to make use of).

Also add suitable build-time checks.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -277,7 +277,9 @@ int vcpu_init_fpu(struct vcpu *v)
     }
     else
     {
-        v->arch.fpu_ctxt = _xzalloc(sizeof(v->arch.xsave_area->fpu_sse), 16);
+        BUILD_BUG_ON(__alignof(v->arch.xsave_area->fpu_sse) < 16);
+        v->arch.fpu_ctxt = _xzalloc(sizeof(v->arch.xsave_area->fpu_sse),
+                                    __alignof(v->arch.xsave_area->fpu_sse));
         if ( v->arch.fpu_ctxt )
         {
             typeof(v->arch.xsave_area->fpu_sse) *fpu_sse = v->arch.fpu_ctxt;
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -414,7 +414,8 @@ int xstate_alloc_save_area(struct vcpu *
     BUG_ON(xsave_cntxt_size < XSTATE_AREA_MIN_SIZE);
 
     /* XSAVE/XRSTOR requires the save area be 64-byte-boundary aligned. */
-    save_area = _xzalloc(xsave_cntxt_size, 64);
+    BUILD_BUG_ON(__alignof(*save_area) < 64);
+    save_area = _xzalloc(xsave_cntxt_size, __alignof(*save_area));
     if ( save_area == NULL )
         return -ENOMEM;
 
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -48,9 +48,9 @@ extern u64 xfeature_mask;
 extern unsigned int *xstate_sizes;
 
 /* extended state save area */
-struct __packed __attribute__((aligned (64))) xsave_struct
+struct __attribute__((aligned (64))) xsave_struct
 {
-    union {                                  /* FPU/MMX, SSE */
+    union __attribute__((aligned(16))) {     /* FPU/MMX, SSE */
         char x[512];
         struct {
             uint16_t fcw;

Comments

Andrew Cooper Jan. 29, 2016, 3:07 p.m. UTC | #1 
On 29/01/16 10:27, Jan Beulich wrote:
> The packed attribute was pointlessly used here - there are no
> misaligned fields, and hence even if the attribute took effect, it
> would at best lead to the compiler generating worse code.
>
> At the same time specify the required alignment of the fpu_sse sub-
> structure, such that the various typeof() uses on that field obtain
> pointers to properly aligned memory (knowledge which a compiler may
> want to make use of).
>
> Also add suitable build-time checks.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
diff mbox

Patch

--- a/xen/arch/x86/i387.c
+++ b/xen/arch/x86/i387.c
@@ -277,7 +277,9 @@  int vcpu_init_fpu(struct vcpu *v)
     }
     else
     {
-        v->arch.fpu_ctxt = _xzalloc(sizeof(v->arch.xsave_area->fpu_sse), 16);
+        BUILD_BUG_ON(__alignof(v->arch.xsave_area->fpu_sse) < 16);
+        v->arch.fpu_ctxt = _xzalloc(sizeof(v->arch.xsave_area->fpu_sse),
+                                    __alignof(v->arch.xsave_area->fpu_sse));
         if ( v->arch.fpu_ctxt )
         {
             typeof(v->arch.xsave_area->fpu_sse) *fpu_sse = v->arch.fpu_ctxt;
--- a/xen/arch/x86/xstate.c
+++ b/xen/arch/x86/xstate.c
@@ -414,7 +414,8 @@  int xstate_alloc_save_area(struct vcpu *
     BUG_ON(xsave_cntxt_size < XSTATE_AREA_MIN_SIZE);
 
     /* XSAVE/XRSTOR requires the save area be 64-byte-boundary aligned. */
-    save_area = _xzalloc(xsave_cntxt_size, 64);
+    BUILD_BUG_ON(__alignof(*save_area) < 64);
+    save_area = _xzalloc(xsave_cntxt_size, __alignof(*save_area));
     if ( save_area == NULL )
         return -ENOMEM;
 
--- a/xen/include/asm-x86/xstate.h
+++ b/xen/include/asm-x86/xstate.h
@@ -48,9 +48,9 @@  extern u64 xfeature_mask;
 extern unsigned int *xstate_sizes;
 
 /* extended state save area */
-struct __packed __attribute__((aligned (64))) xsave_struct
+struct __attribute__((aligned (64))) xsave_struct
 {
-    union {                                  /* FPU/MMX, SSE */
+    union __attribute__((aligned(16))) {     /* FPU/MMX, SSE */
         char x[512];
         struct {
             uint16_t fcw;