Re: [Xen-devel] [PATCH] Fix a panic in SPEC_CTRL_ENTRY_FROM_INTR_IST

2018-02-14 Thread Zhenzhong Duan

在 2018/2/14 15:56, Jan Beulich 写道:

On 14.02.18 at 05:03,  wrote:

On on 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.

Reviewed-by: Boris Ostrovsky 
Tested-by: Boris Ostrovsky 
Signed-off-by: Zhenzhong Duan 

Two formal things: Please put tags in sequential (in time) order:
reporter, author(s), reviewers/testers. And please follow patch
submission rules - send them _to_ the list, with maintainers (and
other interested parties) on _cc_.

Got it.



--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -269,16 +269,16 @@
   * 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
  
  xor %edx, %edx

@@ -291,6 +291,7 @@
   * 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

This is wrong now, and the comment very clearly states why. I think
rather than switching %eax to %edx it would be better to preserve
%eax (e.g. by saving into %edx) around DO_OVERWRITE_RSB. I'll
send an updated patch in a few minutes.

Right, I missed this part, thanks for point.
Your v2 patch looks good for me.

--
thanks
zduan


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

Re: [Xen-devel] [PATCH] Fix a panic in SPEC_CTRL_ENTRY_FROM_INTR_IST

2018-02-13 Thread Jan Beulich
>>> On 14.02.18 at 05:03,  wrote:
> On on 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.
> 
> Reviewed-by: Boris Ostrovsky 
> Tested-by: Boris Ostrovsky 
> Signed-off-by: Zhenzhong Duan 

Two formal things: Please put tags in sequential (in time) order:
reporter, author(s), reviewers/testers. And please follow patch
submission rules - send them _to_ the list, with maintainers (and
other interested parties) on _cc_.

> --- a/xen/include/asm-x86/spec_ctrl_asm.h
> +++ b/xen/include/asm-x86/spec_ctrl_asm.h
> @@ -269,16 +269,16 @@
>   * 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
>  
>  xor %edx, %edx
> @@ -291,6 +291,7 @@
>   * 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

This is wrong now, and the comment very clearly states why. I think
rather than switching %eax to %edx it would be better to preserve
%eax (e.g. by saving into %edx) around DO_OVERWRITE_RSB. I'll
send an updated patch in a few minutes.

Jan


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

[Xen-devel] [PATCH] Fix a panic in SPEC_CTRL_ENTRY_FROM_INTR_IST

2018-02-13 Thread Zhenzhong Duan
On on 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.

Reviewed-by: Boris Ostrovsky 
Tested-by: Boris Ostrovsky 
Signed-off-by: Zhenzhong Duan 
---
 xen/include/asm-x86/spec_ctrl_asm.h |7 ---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/xen/include/asm-x86/spec_ctrl_asm.h 
b/xen/include/asm-x86/spec_ctrl_asm.h
index 814f53d..55d87c0 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -269,16 +269,16 @@
  * 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
 
 xor %edx, %edx
@@ -291,6 +291,7 @@
  * 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