Re: [Xen-devel] [PATCH v3] x86: fix a crash in SPEC_CTRL_ENTRY_FROM_INTR_IST

2018-02-14 Thread Zhenzhong Duan 

在 2018/2/14 17:58, Jan Beulich 写道:

On 14.02.18 at 10:25,  wrote:

--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -269,28 +269,29 @@
   * This is logical merge of DO_OVERWRITE_RSB and DO_SPEC_CTRL_ENTRY
   * maybexen=1, but with conditionals rather than alternatives.
   */
-movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %eax
+movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %edx
  
-testb $BTI_IST_RSB, %al

+testb $BTI_IST_RSB, %dl
  jz .L\@_skip_rsb
  
  DO_OVERWRITE_RSB
  
  .L\@_skip_rsb:
  
-testb $BTI_IST_WRMSR, %al

+testb $BTI_IST_WRMSR, %dl
  jz .L\@_skip_wrmsr
  
+mov %edx, %eax

  xor %edx, %edx
  testb $3, UREGS_cs(%rsp)
  setz %dl
  and %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
  
-.L\@_entry_from_xen:

  /*
   * Load Xen's intended value.  SPEC_CTRL_IBRS vs 0 is encoded in the
   * bottom bit of bti_ist_info, via a deliberate alias with BTI_IST_IBRS.
   */
+xor %edx, %edx
  mov $MSR_SPEC_CTRL, %ecx
  and $BTI_IST_IBRS, %eax
  wrmsr

While indeed you add one less instruction, you don't shrink overall
code size compared to v2. I also prefer v2 because of being more
explicit about the register needing to be preserved across
DO_OVERWRITE_RSB.
Then Ok, in fact my inital thought is to avoid unnecessory mov 
instructions around DO_OVERWRITE_RSB in the 'jmp _skip_wrmsr' case, so 
tried to remove them.


--
thanks
zduan


___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v3] x86: fix a crash in SPEC_CTRL_ENTRY_FROM_INTR_IST

2018-02-14 Thread Zhenzhong Duan 
In an IBRS available env, bootup panic when bti=0 like below:

(XEN) Speculative mitigation facilities:
(XEN)   Hardware features: SMEP IBRS/IBPB STIBP
(XEN) BTI mitigations: Thunk N/A, Others: IBRS- SMEP
(XEN) [ Xen-4.4.4OVM  x86_64  debug=n  Tainted:C ]
(XEN) CPU:0
(XEN) RIP:e008:[] entry.o#handle_ist_exception+0xd1/0x176
(XEN) RFLAGS: 00010046   CONTEXT: hypervisor
(XEN) rax:    rbx:    rcx: 0048
(XEN) rdx: 0001   rsi:    rdi: 
(XEN) rbp:    rsp: 82d080529f58   r8:  
(XEN) r9:     r10:    r11: 
(XEN) r12:    r13:    r14: 82d08052
(XEN) r15:    cr0: 8005003b   cr4: 001506f0
(XEN) cr3: 76fbe000   cr2: 
(XEN) ds:    es:    fs:    gs:    ss:    cs: e008
(XEN) Xen stack trace from rsp=82d080529f58:
(XEN)0018  0002 82d080528000
(XEN) 82d0802a50e0 82d08052fd98 82d08072fc00
(XEN) 0001 0400 0830
(XEN) 000a 82d0803f0fc0 0002
(XEN)82d080298876 e008 0046 82d08052fdf8
(XEN)
(XEN) Xen call trace:
(XEN)[] entry.o#handle_ist_exception+0xd1/0x176
(XEN)
(XEN)
(XEN) 
(XEN) Panic on CPU 0:
(XEN) GENERAL PROTECTION FAULT
(XEN) [error_code=]
(XEN) 

It's due to %edx isn't cleared to zero before wrmsr.

DO_OVERWRITE_RSB clobbers %eax and happend to cover the bug in certain case so
we didn't reproduce without bti=0. Change to use %edx.

Also drop an unused label per Jan.

Reported-by: Boris Ostrovsky 
Signed-off-by: Zhenzhong Duan 
Signed-off-by: Jan Beulich 
---
 xen/include/asm-x86/spec_ctrl_asm.h |9 +
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/xen/include/asm-x86/spec_ctrl_asm.h 
b/xen/include/asm-x86/spec_ctrl_asm.h
index 814f53d..7b259c4 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -269,28 +269,29 @@
  * This is logical merge of DO_OVERWRITE_RSB and DO_SPEC_CTRL_ENTRY
  * maybexen=1, but with conditionals rather than alternatives.
  */
-movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %eax
+movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %edx
 
-testb $BTI_IST_RSB, %al
+testb $BTI_IST_RSB, %dl
 jz .L\@_skip_rsb
 
 DO_OVERWRITE_RSB
 
 .L\@_skip_rsb:
 
-testb $BTI_IST_WRMSR, %al
+testb $BTI_IST_WRMSR, %dl
 jz .L\@_skip_wrmsr
 
+mov %edx, %eax
 xor %edx, %edx
 testb $3, UREGS_cs(%rsp)
 setz %dl
 and %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
 
-.L\@_entry_from_xen:
 /*
  * Load Xen's intended value.  SPEC_CTRL_IBRS vs 0 is encoded in the
  * bottom bit of bti_ist_info, via a deliberate alias with BTI_IST_IBRS.
  */
+xor %edx, %edx
 mov $MSR_SPEC_CTRL, %ecx
 and $BTI_IST_IBRS, %eax
 wrmsr
-- 
1.7.3

___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel