[PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Feng Wu]

Clear AC bit in RFLAGS at the beginning of exception, interrupt, hypercall,
so Xen itself can be protected by SMAP mechanism.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 xen/arch/x86/x86_64/compat/entry.S | 1 +
 xen/arch/x86/x86_64/entry.S        | 8 ++++++++
 xen/arch/x86/x86_64/traps.c        | 4 ++--
 3 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index 32b3bcc..ac594c9 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -13,6 +13,7 @@
 #include <irq_vectors.h>
 
 ENTRY(compat_hypercall)
+        ASM_CLAC
         pushq $0
         SAVE_VOLATILE type=TRAP_syscall compat=1
 
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 1c81852..5f46803 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -273,6 +273,7 @@ ENTRY(sysenter_entry)
         pushq $0
         pushfq
 GLOBAL(sysenter_eflags_saved)
+        ASM_CLAC
         pushq $3 /* ring 3 null cs */
         pushq $0 /* null rip */
         pushq $0
@@ -309,6 +310,7 @@ UNLIKELY_END(sysenter_gpf)
         jmp   .Lbounce_exception
 
 ENTRY(int80_direct_trap)
+        ASM_CLAC
         pushq $0
         SAVE_VOLATILE 0x80
 
@@ -466,6 +468,7 @@ ENTRY(dom_crash_sync_extable)
         jmp   asm_domain_crash_synchronous /* Does not return */
 
 ENTRY(common_interrupt)
+        ASM_CLAC
         SAVE_ALL
         movq %rsp,%rdi
         callq do_IRQ
@@ -485,6 +488,7 @@ ENTRY(page_fault)
         movl  $TRAP_page_fault,4(%rsp)
 /* No special register assumptions. */
 GLOBAL(handle_exception)
+        ASM_CLAC
         SAVE_ALL
 handle_exception_saved:
         testb $X86_EFLAGS_IF>>8,UREGS_eflags+1(%rsp)
@@ -613,6 +617,7 @@ ENTRY(spurious_interrupt_bug)
         jmp   handle_exception
 
 ENTRY(double_fault)
+        ASM_CLAC
         movl  $TRAP_double_fault,4(%rsp)
         SAVE_ALL
         movq  %rsp,%rdi
@@ -628,6 +633,7 @@ ENTRY(early_page_fault)
         .popsection
 
 ENTRY(nmi)
+        ASM_CLAC
         pushq $0
         movl  $TRAP_nmi,4(%rsp)
 handle_ist_exception:
@@ -665,6 +671,7 @@ handle_ist_exception:
         jmp   compat_restore_all_guest
 
 ENTRY(nmi_crash)
+        ASM_CLAC
         pushq $0
         movl $TRAP_nmi,4(%rsp)
         SAVE_ALL
@@ -673,6 +680,7 @@ ENTRY(nmi_crash)
         ud2
 
 ENTRY(machine_check)
+        ASM_CLAC
         pushq $0
         movl  $TRAP_machine_check,4(%rsp)
         jmp   handle_ist_exception
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 90072c1..466b2f2 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -437,8 +437,8 @@ void __devinit subarch_percpu_traps_init(void)
     /* Common SYSCALL parameters. */
     wrmsr(MSR_STAR, 0, (FLAT_RING3_CS32<<16) | __HYPERVISOR_CS);
     wrmsr(MSR_SYSCALL_MASK,
-          X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|
-          X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_TF,
+          X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|X86_EFLAGS_DF|
+          X86_EFLAGS_IF|X86_EFLAGS_TF|X86_EFLAGS_AC,
           0U);
 }
 
-- 
1.8.3.1

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Jan Beulich]

>>> On 28.04.14 at 05:15, <feng.wu@intel.com> wrote:
> @@ -466,6 +468,7 @@ ENTRY(dom_crash_sync_extable)
>          jmp   asm_domain_crash_synchronous /* Does not return */
>  
>  ENTRY(common_interrupt)
> +        ASM_CLAC
>          SAVE_ALL
>          movq %rsp,%rdi
>          callq do_IRQ
> @@ -485,6 +488,7 @@ ENTRY(page_fault)
>          movl  $TRAP_page_fault,4(%rsp)
>  /* No special register assumptions. */
>  GLOBAL(handle_exception)
> +        ASM_CLAC
>          SAVE_ALL

Did you check whether the addition wouldn't better go right into
SAVE_ALL?

> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -437,8 +437,8 @@ void __devinit subarch_percpu_traps_init(void)
>      /* Common SYSCALL parameters. */
>      wrmsr(MSR_STAR, 0, (FLAT_RING3_CS32<<16) | __HYPERVISOR_CS);
>      wrmsr(MSR_SYSCALL_MASK,
> -          X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|
> -          X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_TF,
> +          X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|X86_EFLAGS_DF|
> +          X86_EFLAGS_IF|X86_EFLAGS_TF|X86_EFLAGS_AC,

Please pay attention to ordering of existing accumulations like this:
Here, the operands are ordered by bit position, so the addition
should go at the beginning of the expression rather than the end.

Jan

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Wu, Feng]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Monday, April 28, 2014 5:26 PM
> To: Wu, Feng
> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> Subject: Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
> 
> >>> On 28.04.14 at 05:15, <feng.wu@intel.com> wrote:
> > @@ -466,6 +468,7 @@ ENTRY(dom_crash_sync_extable)
> >          jmp   asm_domain_crash_synchronous /* Does not return */
> >
> >  ENTRY(common_interrupt)
> > +        ASM_CLAC
> >          SAVE_ALL
> >          movq %rsp,%rdi
> >          callq do_IRQ
> > @@ -485,6 +488,7 @@ ENTRY(page_fault)
> >          movl  $TRAP_page_fault,4(%rsp)
> >  /* No special register assumptions. */
> >  GLOBAL(handle_exception)
> > +        ASM_CLAC
> >          SAVE_ALL
> 
> Did you check whether the addition wouldn't better go right into
> SAVE_ALL?

Most of them can be moved into SAVE_ALL obviously, however, there are two exceptions:

1. SAVE_ALL is not executed in the beginning of some exception handlers, such as, double_fault, nmi, etc. 
2. We don't need CLAC in .fixup section where SAVE_ALL is used.

For case 1, considering the handler of page fault, " movl  $TRAP_page_fault,4(%rsp) " is executed before CLAC, moving CLAC into SAVE_ALL seems acceptable.
For case 2, using CLAC there is not harmful, and can reduce the code redundancy,

So, I will follow this suggestion.

> 
> > --- a/xen/arch/x86/x86_64/traps.c
> > +++ b/xen/arch/x86/x86_64/traps.c
> > @@ -437,8 +437,8 @@ void __devinit subarch_percpu_traps_init(void)
> >      /* Common SYSCALL parameters. */
> >      wrmsr(MSR_STAR, 0, (FLAT_RING3_CS32<<16) | __HYPERVISOR_CS);
> >      wrmsr(MSR_SYSCALL_MASK,
> > -          X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|
> > -          X86_EFLAGS_DF|X86_EFLAGS_IF|X86_EFLAGS_TF,
> > +
> X86_EFLAGS_VM|X86_EFLAGS_RF|X86_EFLAGS_NT|X86_EFLAGS_DF|
> > +          X86_EFLAGS_IF|X86_EFLAGS_TF|X86_EFLAGS_AC,
> 
> Please pay attention to ordering of existing accumulations like this:
> Here, the operands are ordered by bit position, so the addition
> should go at the beginning of the expression rather than the end.
> 
> Jan

Thanks,
Feng

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Jan Beulich]

>>> On 28.04.14 at 11:43, <feng.wu@intel.com> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Monday, April 28, 2014 5:26 PM
>> To: Wu, Feng
>> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
>> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org 
>> Subject: Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by 
> SMAP
>> 
>> >>> On 28.04.14 at 05:15, <feng.wu@intel.com> wrote:
>> > @@ -466,6 +468,7 @@ ENTRY(dom_crash_sync_extable)
>> >          jmp   asm_domain_crash_synchronous /* Does not return */
>> >
>> >  ENTRY(common_interrupt)
>> > +        ASM_CLAC
>> >          SAVE_ALL
>> >          movq %rsp,%rdi
>> >          callq do_IRQ
>> > @@ -485,6 +488,7 @@ ENTRY(page_fault)
>> >          movl  $TRAP_page_fault,4(%rsp)
>> >  /* No special register assumptions. */
>> >  GLOBAL(handle_exception)
>> > +        ASM_CLAC
>> >          SAVE_ALL
>> 
>> Did you check whether the addition wouldn't better go right into
>> SAVE_ALL?
> 
> Most of them can be moved into SAVE_ALL obviously, however, there are two 
> exceptions:
> 
> 1. SAVE_ALL is not executed in the beginning of some exception handlers, 
> such as, double_fault, nmi, etc. 

But that's orthogonal - I didn't say that would automatically cover
_all_ cases.

> 2. We don't need CLAC in .fixup section where SAVE_ALL is used.

Do we not?

Jan

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Wu, Feng]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Monday, April 28, 2014 5:58 PM
> To: Wu, Feng
> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
> 
> >>> On 28.04.14 at 11:43, <feng.wu@intel.com> wrote:
> 
> >
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: Monday, April 28, 2014 5:26 PM
> >> To: Wu, Feng
> >> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> >> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> >> Subject: Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by
> > SMAP
> >>
> >> >>> On 28.04.14 at 05:15, <feng.wu@intel.com> wrote:
> >> > @@ -466,6 +468,7 @@ ENTRY(dom_crash_sync_extable)
> >> >          jmp   asm_domain_crash_synchronous /* Does not return */
> >> >
> >> >  ENTRY(common_interrupt)
> >> > +        ASM_CLAC
> >> >          SAVE_ALL
> >> >          movq %rsp,%rdi
> >> >          callq do_IRQ
> >> > @@ -485,6 +488,7 @@ ENTRY(page_fault)
> >> >          movl  $TRAP_page_fault,4(%rsp)
> >> >  /* No special register assumptions. */
> >> >  GLOBAL(handle_exception)
> >> > +        ASM_CLAC
> >> >          SAVE_ALL
> >>
> >> Did you check whether the addition wouldn't better go right into
> >> SAVE_ALL?
> >
> > Most of them can be moved into SAVE_ALL obviously, however, there are two
> > exceptions:
> >
> > 1. SAVE_ALL is not executed in the beginning of some exception handlers,
> > such as, double_fault, nmi, etc.
> 
> But that's orthogonal - I didn't say that would automatically cover
> _all_ cases.

So, do you think we can move CLAC into SAVE_ALL in these cases? Thanks!

> 
> > 2. We don't need CLAC in .fixup section where SAVE_ALL is used.
> 
> Do we not?

I think the AC bit is clear when running SAVE_ALL in the .fixup section.

> 
> Jan
> 
Thanks,
Feng

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Jan Beulich]

>>> On 29.04.14 at 05:06, <feng.wu@intel.com> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Monday, April 28, 2014 5:58 PM
>> To: Wu, Feng
>> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
>> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org 
>> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by 
> SMAP
>> 
>> >>> On 28.04.14 at 11:43, <feng.wu@intel.com> wrote:
>> 
>> >
>> >> -----Original Message-----
>> >> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >> Sent: Monday, April 28, 2014 5:26 PM
>> >> To: Wu, Feng
>> >> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
>> >> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org 
>> >> Subject: Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by
>> > SMAP
>> >>
>> >> >>> On 28.04.14 at 05:15, <feng.wu@intel.com> wrote:
>> >> > @@ -466,6 +468,7 @@ ENTRY(dom_crash_sync_extable)
>> >> >          jmp   asm_domain_crash_synchronous /* Does not return */
>> >> >
>> >> >  ENTRY(common_interrupt)
>> >> > +        ASM_CLAC
>> >> >          SAVE_ALL
>> >> >          movq %rsp,%rdi
>> >> >          callq do_IRQ
>> >> > @@ -485,6 +488,7 @@ ENTRY(page_fault)
>> >> >          movl  $TRAP_page_fault,4(%rsp)
>> >> >  /* No special register assumptions. */
>> >> >  GLOBAL(handle_exception)
>> >> > +        ASM_CLAC
>> >> >          SAVE_ALL
>> >>
>> >> Did you check whether the addition wouldn't better go right into
>> >> SAVE_ALL?
>> >
>> > Most of them can be moved into SAVE_ALL obviously, however, there are two
>> > exceptions:
>> >
>> > 1. SAVE_ALL is not executed in the beginning of some exception handlers,
>> > such as, double_fault, nmi, etc.
>> 
>> But that's orthogonal - I didn't say that would automatically cover
>> _all_ cases.
> 
> So, do you think we can move CLAC into SAVE_ALL in these cases? Thanks!

That was actually meant for you to figure out. But since you ask back,
yes, I don't see any case where it would be outright wrong, but there
are cases where it unnecessary, i.e. parameterizing SAVE_ALL might
nevertheless be desirable.

>> > 2. We don't need CLAC in .fixup section where SAVE_ALL is used.
>> 
>> Do we not?
> 
> I think the AC bit is clear when running SAVE_ALL in the .fixup section.

Correct, but it would once again be harmless to do it here too.

Jan

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Wu, Feng]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, April 29, 2014 3:21 PM
> To: Wu, Feng
> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
> 
> >>> On 29.04.14 at 05:06, <feng.wu@intel.com> wrote:
> 
> >
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: Monday, April 28, 2014 5:58 PM
> >> To: Wu, Feng
> >> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> >> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> >> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by
> > SMAP
> >>
> >> >>> On 28.04.14 at 11:43, <feng.wu@intel.com> wrote:
> >>
> >> >
> >> >> -----Original Message-----
> >> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> >> Sent: Monday, April 28, 2014 5:26 PM
> >> >> To: Wu, Feng
> >> >> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> >> >> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> >> >> Subject: Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by
> >> > SMAP
> >> >>
> >> >> >>> On 28.04.14 at 05:15, <feng.wu@intel.com> wrote:
> >> >> > @@ -466,6 +468,7 @@ ENTRY(dom_crash_sync_extable)
> >> >> >          jmp   asm_domain_crash_synchronous /* Does not
> return */
> >> >> >
> >> >> >  ENTRY(common_interrupt)
> >> >> > +        ASM_CLAC
> >> >> >          SAVE_ALL
> >> >> >          movq %rsp,%rdi
> >> >> >          callq do_IRQ
> >> >> > @@ -485,6 +488,7 @@ ENTRY(page_fault)
> >> >> >          movl  $TRAP_page_fault,4(%rsp)
> >> >> >  /* No special register assumptions. */
> >> >> >  GLOBAL(handle_exception)
> >> >> > +        ASM_CLAC
> >> >> >          SAVE_ALL
> >> >>
> >> >> Did you check whether the addition wouldn't better go right into
> >> >> SAVE_ALL?
> >> >
> >> > Most of them can be moved into SAVE_ALL obviously, however, there are
> two
> >> > exceptions:
> >> >
> >> > 1. SAVE_ALL is not executed in the beginning of some exception handlers,
> >> > such as, double_fault, nmi, etc.
> >>
> >> But that's orthogonal - I didn't say that would automatically cover
> >> _all_ cases.
> >
> > So, do you think we can move CLAC into SAVE_ALL in these cases? Thanks!
> 
> That was actually meant for you to figure out. But since you ask back,
> yes, I don't see any case where it would be outright wrong, but there
> are cases where it unnecessary, i.e. parameterizing SAVE_ALL might
> nevertheless be desirable.

Thanks for the suggestion. BTW, can we move SAVE_ALL into #ifdef __ASSEMBLY__ ...... #endif,
since it is only used in assembly code?

> 
> >> > 2. We don't need CLAC in .fixup section where SAVE_ALL is used.
> >>
> >> Do we not?
> >
> > I think the AC bit is clear when running SAVE_ALL in the .fixup section.
> 
> Correct, but it would once again be harmless to do it here too.
> 
> Jan

Thanks,
Feng

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Jan Beulich]

>>> On 29.04.14 at 10:19, <feng.wu@intel.com> wrote:
> BTW, can we move SAVE_ALL into #ifdef 
> __ASSEMBLY__ ...... #endif,
> since it is only used in assembly code?

Sure - that could be part of your first patch eliminating the last C user.

Jan

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Wu, Feng]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, April 29, 2014 4:59 PM
> To: Wu, Feng
> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
> 
> >>> On 29.04.14 at 10:19, <feng.wu@intel.com> wrote:
> > BTW, can we move SAVE_ALL into #ifdef
> > __ASSEMBLY__ ...... #endif,
> > since it is only used in assembly code?
> 
> Sure - that could be part of your first patch eliminating the last C user.

Thanks!

May I move the definition of SAVE_ALL (or more related code, such as, SAVE_VOLATILE, etc.) from " xen/include/asm-x86/x86_64/asm_defns.h " to " xen/include/asm-x86/asm_defns.h ", since I got an build error after adding a parameter to marco SAVE_ALL like this, which makes me think it should be defined after ASM_CLAC?

#ifdef __ASSEMBLY__
.macro SAVE_ALL clear_ac=1
.if \clear_ac
        ASM_CLAC
.endif
        addq  $-(UREGS_error_code-UREGS_r15), %rsp
        cld
        movq  %rdi,UREGS_rdi(%rsp)
        movq  %rsi,UREGS_rsi(%rsp)
        movq  %rdx,UREGS_rdx(%rsp)
        movq  %rcx,UREGS_rcx(%rsp)
        movq  %rax,UREGS_rax(%rsp)
        movq  %r8,UREGS_r8(%rsp)
       movq  %r9,UREGS_r9(%rsp)
        movq  %r10,UREGS_r10(%rsp)
        movq  %r11,UREGS_r11(%rsp)
        movq  %rbx,UREGS_rbx(%rsp)
        movq  %rbp,UREGS_rbp(%rsp)
        SETUP_EXCEPTION_FRAME_POINTER(UREGS_rbp)
        movq  %r12,UREGS_r12(%rsp)
        movq  %r13,UREGS_r13(%rsp)
        movq  %r14,UREGS_r14(%rsp)
        movq  %r15,UREGS_r15(%rsp)
.endm


> 
> Jan

Thanks,
Feng

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Jan Beulich]

>>> On 29.04.14 at 11:27, <feng.wu@intel.com> wrote:
> May I move the definition of SAVE_ALL (or more related code, such as, 
> SAVE_VOLATILE, etc.) from " xen/include/asm-x86/x86_64/asm_defns.h " to " 
> xen/include/asm-x86/asm_defns.h ", since I got an build error after adding a 
> parameter to marco SAVE_ALL like this, which makes me think it should be 
> defined after ASM_CLAC?

No problem (and in fact I wouldn't mind you merging all of
x86_64/asm_defns.h into the other file - that due at some point
anyway), but I'm afraid we'd want this to be yet another separate
prerequisite patch - not sure if you're up to that, or if you wouldn't
find it simpler to keep SAVE_ALL being a C macro then.

Jan

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Wu, Feng]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, April 29, 2014 5:48 PM
> To: Wu, Feng
> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
> 
> >>> On 29.04.14 at 11:27, <feng.wu@intel.com> wrote:
> > May I move the definition of SAVE_ALL (or more related code, such as,
> > SAVE_VOLATILE, etc.) from " xen/include/asm-x86/x86_64/asm_defns.h " to "
> > xen/include/asm-x86/asm_defns.h ", since I got an build error after adding a
> > parameter to marco SAVE_ALL like this, which makes me think it should be
> > defined after ASM_CLAC?
> 
> No problem (and in fact I wouldn't mind you merging all of
> x86_64/asm_defns.h into the other file - that due at some point
> anyway), but I'm afraid we'd want this to be yet another separate
> prerequisite patch - not sure if you're up to that, or if you wouldn't
> find it simpler to keep SAVE_ALL being a C macro then.

I am fine with the merging task. But Is there anything hard to handle while merging x86_64/asm_defns.h to asm_defns.h, I am wondering
why x86_64/asm_defns.h is there for a long time.

> 
> Jan

Thanks,
Feng

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Andrew Cooper]

On 29/04/14 11:29, Wu, Feng wrote:
>
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Tuesday, April 29, 2014 5:48 PM
>> To: Wu, Feng
>> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
>> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
>> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
>>
>>>>> On 29.04.14 at 11:27, <feng.wu@intel.com> wrote:
>>> May I move the definition of SAVE_ALL (or more related code, such as,
>>> SAVE_VOLATILE, etc.) from " xen/include/asm-x86/x86_64/asm_defns.h " to "
>>> xen/include/asm-x86/asm_defns.h ", since I got an build error after adding a
>>> parameter to marco SAVE_ALL like this, which makes me think it should be
>>> defined after ASM_CLAC?
>> No problem (and in fact I wouldn't mind you merging all of
>> x86_64/asm_defns.h into the other file - that due at some point
>> anyway), but I'm afraid we'd want this to be yet another separate
>> prerequisite patch - not sure if you're up to that, or if you wouldn't
>> find it simpler to keep SAVE_ALL being a C macro then.
> I am fine with the merging task. But Is there anything hard to handle while merging x86_64/asm_defns.h to asm_defns.h, I am wondering
> why x86_64/asm_defns.h is there for a long time.

Because x86_32/asm_defns.h used to exist when we supported building Xen
as 32bit.  I wouldn't have thought there is anything problematic to be
encountered while merging the files.

~Andrew

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Wu, Feng]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Tuesday, April 29, 2014 6:32 PM
> To: Wu, Feng
> Cc: Jan Beulich; ian.campbell@citrix.com; Dong, Eddie; Nakajima, Jun; Tian,
> Kevin; xen-devel@lists.xen.org
> Subject: Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
> 
> On 29/04/14 11:29, Wu, Feng wrote:
> >
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@suse.com]
> >> Sent: Tuesday, April 29, 2014 5:48 PM
> >> To: Wu, Feng
> >> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> >> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> >> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by
> SMAP
> >>
> >>>>> On 29.04.14 at 11:27, <feng.wu@intel.com> wrote:
> >>> May I move the definition of SAVE_ALL (or more related code, such as,
> >>> SAVE_VOLATILE, etc.) from " xen/include/asm-x86/x86_64/asm_defns.h "
> to "
> >>> xen/include/asm-x86/asm_defns.h ", since I got an build error after adding
> a
> >>> parameter to marco SAVE_ALL like this, which makes me think it should be
> >>> defined after ASM_CLAC?
> >> No problem (and in fact I wouldn't mind you merging all of
> >> x86_64/asm_defns.h into the other file - that due at some point
> >> anyway), but I'm afraid we'd want this to be yet another separate
> >> prerequisite patch - not sure if you're up to that, or if you wouldn't
> >> find it simpler to keep SAVE_ALL being a C macro then.
> > I am fine with the merging task. But Is there anything hard to handle while
> merging x86_64/asm_defns.h to asm_defns.h, I am wondering
> > why x86_64/asm_defns.h is there for a long time.
> 
> Because x86_32/asm_defns.h used to exist when we supported building Xen
> as 32bit.  I wouldn't have thought there is anything problematic to be
> encountered while merging the files.
> 
> ~Andrew

Okay, I got it. Thanks!

Thanks,
Feng

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Jan Beulich]

>>> On 29.04.14 at 12:29, <feng.wu@intel.com> wrote:
> I am fine with the merging task. But Is there anything hard to handle while 
> merging x86_64/asm_defns.h to asm_defns.h, I am wondering
> why x86_64/asm_defns.h is there for a long time.

Because there was a (different) x86_32/asm_defns.h until 32-bit
support got ripped out.

Jan

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Wu, Feng]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Tuesday, April 29, 2014 6:42 PM
> To: Wu, Feng
> Cc: andrew.cooper3@citrix.com; ian.campbell@citrix.com; Dong, Eddie;
> Nakajima, Jun; Tian, Kevin; xen-devel@lists.xen.org
> Subject: RE: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP
> 
> >>> On 29.04.14 at 12:29, <feng.wu@intel.com> wrote:
> > I am fine with the merging task. But Is there anything hard to handle while
> > merging x86_64/asm_defns.h to asm_defns.h, I am wondering
> > why x86_64/asm_defns.h is there for a long time.
> 
> Because there was a (different) x86_32/asm_defns.h until 32-bit
> support got ripped out.
> 

Thanks, I knew this. I am curious about whey this file wasn't merged when the 32-bit version was removed.:)
Anyway, that is not a problem, I will merge it.

> Jan

Thanks,
Feng

Re: [PATCH v3 4/9] Clear AC bit in RFLAGS to protect Xen itself by SMAP

[Jan Beulich]

>>> On 29.04.14 at 12:49, <feng.wu@intel.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> >>> On 29.04.14 at 12:29, <feng.wu@intel.com> wrote:
>> > I am fine with the merging task. But Is there anything hard to handle while
>> > merging x86_64/asm_defns.h to asm_defns.h, I am wondering
>> > why x86_64/asm_defns.h is there for a long time.
>> 
>> Because there was a (different) x86_32/asm_defns.h until 32-bit
>> support got ripped out.
> 
> Thanks, I knew this. I am curious about whey this file wasn't merged when 
> the 32-bit version was removed.:)

Because no-one took the time so far, and no-one was forced to do it.

Jan