Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait

2018-03-18 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Monday, March 19, 2018 1:54 PM
> To: Wu, Hao A ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Yao, Jiewen ;
> Laszlo Ersek ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at
> ExitBootServices AP Mwait
> 
> On 3/19/2018 1:31 PM, Hao Wu wrote:
> > Within function ApWakeupFunction():
> >
> > When source level debugger is enabled, AP interrupts will be enabled by
> > EnableDebugAgent(). Then the AP function will be execeuted by:
> >
> > Procedure (Parameter);
> >
> > After the AP function returns, AP interrupts will be disabled when the
> > APs are placed in loop mode (both HltLoop and MwaiLoop).
> >
> > However, at ExitBootServices, ApWakeupFunction() is called with
> > 'Procedure' equals to RelocateApLoop().
> >
> > (ExitBootServices callback registered within InitMpGlobalData())
> >
> > RelocateApLoop() never retuns, so it has to disable the AP interrupts by
> > itself. However, we find that interrupts are only disabled for the
> > HltLoop case, but not for the MwaitLoop case (within file MpFuncs.nasm).
> >
> > This commit adds the missing disabling of AP interrupts for MwaitLoop.
> >
> > Also, for X64, this commit will disable the interrupts before switching to
> > 32-bit mode.
> >
> > Cc: Laszlo Ersek 
> > Cc: Jeff Fan 
> > Cc: Ruiyu Ni 
> > Cc: Jiewen Yao 
> > Cc: Jian J Wang 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu 
> > ---
> >   UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 3 ++-
> >   UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 4 +++-
> >   2 files changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> > index bd79be0f5e..59e88d3f8f 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> > @@ -1,5 +1,5 @@
> >   
> > ;--
> >  ;
> > -; Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> > +; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> >   ; This program and the accompanying materials
> >   ; are licensed and made available under the terms and conditions of the 
> > BSD
> License
> >   ; which accompanies this distribution.  The full text of the license may 
> > be
> found at
> > @@ -239,6 +239,7 @@ AsmRelocateApLoopStart:
> >   cmpcl,  1  ; Check mwait-monitor support
> >   jnzHltLoop
> >   MwaitLoop:
> > +cli
> >   moveax, esp
> >   xorecx, ecx
> >   xoredx, edx
> > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > index 759594..76f8c078ab 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > @@ -1,5 +1,5 @@
> >   
> > ;--
> >  ;
> > -; Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
> > +; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
> >   ; This program and the accompanying materials
> >   ; are licensed and made available under the terms and conditions of the 
> > BSD
> License
> >   ; which accompanies this distribution.  The full text of the license may 
> > be
> found at
> > @@ -253,6 +253,7 @@ RendezvousFunnelProcEnd:
> >   global ASM_PFX(AsmRelocateApLoop)
> >   ASM_PFX(AsmRelocateApLoop):
> >   AsmRelocateApLoopStart:
> > +cli  ; Disable interrupt before switching to
> 32-bit mode
> >   movrax, [rsp + 40]   ; CountTofinish
> >   lock dec   dword [rax]   ; (*CountTofinish)--
> >   movrsp, r9
> > @@ -288,6 +289,7 @@ PmEntry:
> >   jnzHltLoop
> >   movebx, edx   ; Save C-State to ebx
> >   MwaitLoop:
> > +cli
> >   moveax, esp   ; Set Monitor Address
> >   xorecx, ecx   ; ecx = 0
> >   xoredx, edx   ; edx = 0
> >
> Reviewed-by: Ruiyu Ni 
> 
> --
> Thanks,
> Ray
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait

2018-03-18 Thread Ni, Ruiyu

On 3/19/2018 1:31 PM, Hao Wu wrote:

Within function ApWakeupFunction():

When source level debugger is enabled, AP interrupts will be enabled by
EnableDebugAgent(). Then the AP function will be execeuted by:

Procedure (Parameter);

After the AP function returns, AP interrupts will be disabled when the
APs are placed in loop mode (both HltLoop and MwaiLoop).

However, at ExitBootServices, ApWakeupFunction() is called with
'Procedure' equals to RelocateApLoop().

(ExitBootServices callback registered within InitMpGlobalData())

RelocateApLoop() never retuns, so it has to disable the AP interrupts by
itself. However, we find that interrupts are only disabled for the
HltLoop case, but not for the MwaitLoop case (within file MpFuncs.nasm).

This commit adds the missing disabling of AP interrupts for MwaitLoop.

Also, for X64, this commit will disable the interrupts before switching to
32-bit mode.

Cc: Laszlo Ersek 
Cc: Jeff Fan 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
  UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 3 ++-
  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 4 +++-
  2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm 
b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index bd79be0f5e..59e88d3f8f 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -1,5 +1,5 @@
  
;-- 
;
-; Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
  ; This program and the accompanying materials
  ; are licensed and made available under the terms and conditions of the BSD 
License
  ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -239,6 +239,7 @@ AsmRelocateApLoopStart:
  cmpcl,  1  ; Check mwait-monitor support
  jnzHltLoop
  MwaitLoop:
+cli
  moveax, esp
  xorecx, ecx
  xoredx, edx
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm 
b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 759594..76f8c078ab 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -1,5 +1,5 @@
  
;-- 
;
-; Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
  ; This program and the accompanying materials
  ; are licensed and made available under the terms and conditions of the BSD 
License
  ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -253,6 +253,7 @@ RendezvousFunnelProcEnd:
  global ASM_PFX(AsmRelocateApLoop)
  ASM_PFX(AsmRelocateApLoop):
  AsmRelocateApLoopStart:
+cli  ; Disable interrupt before switching to 
32-bit mode
  movrax, [rsp + 40]   ; CountTofinish
  lock dec   dword [rax]   ; (*CountTofinish)--
  movrsp, r9
@@ -288,6 +289,7 @@ PmEntry:
  jnzHltLoop
  movebx, edx   ; Save C-State to ebx
  MwaitLoop:
+cli
  moveax, esp   ; Set Monitor Address
  xorecx, ecx   ; ecx = 0
  xoredx, edx   ; edx = 0


Reviewed-by: Ruiyu Ni 

--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait

2018-03-18 Thread Hao Wu
Within function ApWakeupFunction():

When source level debugger is enabled, AP interrupts will be enabled by
EnableDebugAgent(). Then the AP function will be execeuted by:

Procedure (Parameter);

After the AP function returns, AP interrupts will be disabled when the
APs are placed in loop mode (both HltLoop and MwaiLoop).

However, at ExitBootServices, ApWakeupFunction() is called with
'Procedure' equals to RelocateApLoop().

(ExitBootServices callback registered within InitMpGlobalData())

RelocateApLoop() never retuns, so it has to disable the AP interrupts by
itself. However, we find that interrupts are only disabled for the
HltLoop case, but not for the MwaitLoop case (within file MpFuncs.nasm).

This commit adds the missing disabling of AP interrupts for MwaitLoop.

Also, for X64, this commit will disable the interrupts before switching to
32-bit mode.

Cc: Laszlo Ersek 
Cc: Jeff Fan 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 3 ++-
 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm 
b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index bd79be0f5e..59e88d3f8f 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -1,5 +1,5 @@
 
;-- 
;
-; Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -239,6 +239,7 @@ AsmRelocateApLoopStart:
 cmpcl,  1  ; Check mwait-monitor support
 jnzHltLoop
 MwaitLoop:
+cli
 moveax, esp
 xorecx, ecx
 xoredx, edx
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm 
b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 759594..76f8c078ab 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -1,5 +1,5 @@
 
;-- 
;
-; Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -253,6 +253,7 @@ RendezvousFunnelProcEnd:
 global ASM_PFX(AsmRelocateApLoop)
 ASM_PFX(AsmRelocateApLoop):
 AsmRelocateApLoopStart:
+cli  ; Disable interrupt before switching to 
32-bit mode
 movrax, [rsp + 40]   ; CountTofinish
 lock dec   dword [rax]   ; (*CountTofinish)--
 movrsp, r9
@@ -288,6 +289,7 @@ PmEntry:
 jnzHltLoop
 movebx, edx   ; Save C-State to ebx
 MwaitLoop:
+cli
 moveax, esp   ; Set Monitor Address
 xorecx, ecx   ; ecx = 0
 xoredx, edx   ; edx = 0
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel