From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH v5 04/27] x86/fpu/xstate: Add XSAVES system states for shadow stack Date: Thu, 8 Nov 2018 14:01:42 -0800 Message-ID: References: <20181011151523.27101-1-yu-cheng.yu@intel.com> <20181011151523.27101-5-yu-cheng.yu@intel.com> <4295b8f786c10c469870a6d9725749ce75dcdaa2.camel@intel.com> <20181108213126.GD13195@uranus.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Return-path: In-Reply-To: <20181108213126.GD13195@uranus.lan> Sender: linux-kernel-owner@vger.kernel.org To: Cyrill Gorcunov Cc: Yu-cheng Yu , X86 ML , "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , LKML , "open list:DOCUMENTATION" , Linux-MM , linux-arch , Linux API , Arnd Bergmann , Balbir Singh , Dave Hansen , Eugene Syromiatnikov , Florian Weimer , "H. J. Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit List-Id: linux-arch.vger.kernel.org On Thu, Nov 8, 2018 at 1:31 PM Cyrill Gorcunov wrote: > > On Thu, Nov 08, 2018 at 01:22:54PM -0800, Andy Lutomirski wrote: > > > > > > > > Why are these __packed? It seems like it'll generate bad code for no > > > > obvious purpose. > > > > > > That prevents any possibility that the compiler will insert padding, although in > > > 64-bit kernel this should not happen to either struct. Also all xstate > > > components here are packed. > > > > > > > They both seem like bugs, perhaps. As I understand it, __packed > > removes padding, but it also forces the compiler to expect the fields > > to be unaligned even if they are actually aligned. > > How is that? Andy, mind to point where you get that this > attribute forces compiler to make such assumption? It's from memory. But gcc seems to agree with me I compiled this: struct foo { int x; } __attribute__((packed)); int read_foo(struct foo *f) { return f->x; } int align_of_foo_x(struct foo *f) { return __alignof__(f->x); } Compiling with -O2 gives: .globl read_foo .type read_foo, @function read_foo: movl (%rdi), %eax ret .size read_foo, .-read_foo .p2align 4,,15 .globl align_of_foo_x .type align_of_foo_x, @function align_of_foo_x: movl $1, %eax ret .size align_of_foo_x, .-align_of_foo_x So gcc thinks that the x field is one-byte-aligned, but the code is okay (at least in this instance) on x86. Building for armv5 gives: .type read_foo, %function read_foo: @ args = 0, pretend = 0, frame = 0 @ frame_needed = 0, uses_anonymous_args = 0 @ link register save eliminated. ldrb r3, [r0] @ zero_extendqisi2 ldrb r1, [r0, #1] @ zero_extendqisi2 ldrb r2, [r0, #2] @ zero_extendqisi2 orr r3, r3, r1, lsl #8 ldrb r0, [r0, #3] @ zero_extendqisi2 orr r3, r3, r2, lsl #16 orr r0, r3, r0, lsl #24 bx lr .size read_foo, .-read_foo .align 2 .global align_of_foo_x .syntax unified .arm .fpu vfpv3-d16 .type align_of_foo_x, %function So I'm pretty sure I'm right. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f68.google.com ([209.85.221.68]:37870 "EHLO mail-wr1-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728072AbeKIHj0 (ORCPT ); Fri, 9 Nov 2018 02:39:26 -0500 Received: by mail-wr1-f68.google.com with SMTP id o15-v6so19231024wrv.4 for ; Thu, 08 Nov 2018 14:01:57 -0800 (PST) MIME-Version: 1.0 References: <20181011151523.27101-1-yu-cheng.yu@intel.com> <20181011151523.27101-5-yu-cheng.yu@intel.com> <4295b8f786c10c469870a6d9725749ce75dcdaa2.camel@intel.com> <20181108213126.GD13195@uranus.lan> In-Reply-To: <20181108213126.GD13195@uranus.lan> From: Andy Lutomirski Date: Thu, 8 Nov 2018 14:01:42 -0800 Message-ID: Subject: Re: [PATCH v5 04/27] x86/fpu/xstate: Add XSAVES system states for shadow stack Content-Type: text/plain; charset="UTF-8" Sender: linux-arch-owner@vger.kernel.org List-ID: To: Cyrill Gorcunov Cc: Yu-cheng Yu , X86 ML , "H. Peter Anvin" , Thomas Gleixner , Ingo Molnar , LKML , "open list:DOCUMENTATION" , Linux-MM , linux-arch , Linux API , Arnd Bergmann , Balbir Singh , Dave Hansen , Eugene Syromiatnikov , Florian Weimer , "H. J. Lu" , Jann Horn , Jonathan Corbet , Kees Cook , Mike Kravetz , Nadav Amit , Oleg Nesterov , Pavel Machek , Peter Zijlstra , Randy Dunlap , "Ravi V. Shankar" , "Shanbhogue, Vedvyas" Message-ID: <20181108220142.Pkh4d0kYaYZf1drgoOktFAKUc6SyH44Aqfo-4dcfW_4@z> On Thu, Nov 8, 2018 at 1:31 PM Cyrill Gorcunov wrote: > > On Thu, Nov 08, 2018 at 01:22:54PM -0800, Andy Lutomirski wrote: > > > > > > > > Why are these __packed? It seems like it'll generate bad code for no > > > > obvious purpose. > > > > > > That prevents any possibility that the compiler will insert padding, although in > > > 64-bit kernel this should not happen to either struct. Also all xstate > > > components here are packed. > > > > > > > They both seem like bugs, perhaps. As I understand it, __packed > > removes padding, but it also forces the compiler to expect the fields > > to be unaligned even if they are actually aligned. > > How is that? Andy, mind to point where you get that this > attribute forces compiler to make such assumption? It's from memory. But gcc seems to agree with me I compiled this: struct foo { int x; } __attribute__((packed)); int read_foo(struct foo *f) { return f->x; } int align_of_foo_x(struct foo *f) { return __alignof__(f->x); } Compiling with -O2 gives: .globl read_foo .type read_foo, @function read_foo: movl (%rdi), %eax ret .size read_foo, .-read_foo .p2align 4,,15 .globl align_of_foo_x .type align_of_foo_x, @function align_of_foo_x: movl $1, %eax ret .size align_of_foo_x, .-align_of_foo_x So gcc thinks that the x field is one-byte-aligned, but the code is okay (at least in this instance) on x86. Building for armv5 gives: .type read_foo, %function read_foo: @ args = 0, pretend = 0, frame = 0 @ frame_needed = 0, uses_anonymous_args = 0 @ link register save eliminated. ldrb r3, [r0] @ zero_extendqisi2 ldrb r1, [r0, #1] @ zero_extendqisi2 ldrb r2, [r0, #2] @ zero_extendqisi2 orr r3, r3, r1, lsl #8 ldrb r0, [r0, #3] @ zero_extendqisi2 orr r3, r3, r2, lsl #16 orr r0, r3, r0, lsl #24 bx lr .size read_foo, .-read_foo .align 2 .global align_of_foo_x .syntax unified .arm .fpu vfpv3-d16 .type align_of_foo_x, %function So I'm pretty sure I'm right.