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

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, April 28, 2014 5:26 PM
> To: Wu, Feng
> Cc: andrew.cooper3@xxxxxxxxxx; ian.campbell@xxxxxxxxxx; Dong, Eddie;
> Nakajima, Jun; Tian, Kevin; xen-devel@xxxxxxxxxxxxx
> 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@xxxxxxxxx> 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

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel