public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait
@ 2018-03-19  5:31 Hao Wu
  2018-03-19  5:54 ` Ni, Ruiyu
  2018-03-19 13:31 ` 答复: " Fan Jeff
  0 siblings, 2 replies; 4+ messages in thread
From: Hao Wu @ 2018-03-19  5:31 UTC (permalink / raw)
  To: edk2-devel
  Cc: Hao Wu, Laszlo Ersek, Jeff Fan, Ruiyu Ni, Jiewen Yao, Jian J Wang,
	Star Zeng, Eric Dong

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 <lersek@redhat.com>
Cc: Jeff Fan <vanjeff_919@hotmail.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 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.<BR>
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 ; 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:
     cmp        cl,  1              ; Check mwait-monitor support
     jnz        HltLoop
 MwaitLoop:
+    cli
     mov        eax, esp
     xor        ecx, ecx
     xor        edx, edx
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 7595988884..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.<BR>
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 ; 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
     mov        rax, [rsp + 40]   ; CountTofinish
     lock dec   dword [rax]       ; (*CountTofinish)--
     mov        rsp, r9
@@ -288,6 +289,7 @@ PmEntry:
     jnz        HltLoop
     mov        ebx, edx           ; Save C-State to ebx
 MwaitLoop:
+    cli
     mov        eax, esp           ; Set Monitor Address
     xor        ecx, ecx           ; ecx = 0
     xor        edx, edx           ; edx = 0
-- 
2.12.0.windows.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait
  2018-03-19  5:31 [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait Hao Wu
@ 2018-03-19  5:54 ` Ni, Ruiyu
  2018-03-19  6:13   ` Yao, Jiewen
  2018-03-19 13:31 ` 答复: " Fan Jeff
  1 sibling, 1 reply; 4+ messages in thread
From: Ni, Ruiyu @ 2018-03-19  5:54 UTC (permalink / raw)
  To: Hao Wu, edk2-devel
  Cc: Laszlo Ersek, Jeff Fan, Jiewen Yao, Jian J Wang, Star Zeng,
	Eric Dong

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 <lersek@redhat.com>
> Cc: Jeff Fan <vanjeff_919@hotmail.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>   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.<BR>
> +; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
>   ; 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:
>       cmp        cl,  1              ; Check mwait-monitor support
>       jnz        HltLoop
>   MwaitLoop:
> +    cli
>       mov        eax, esp
>       xor        ecx, ecx
>       xor        edx, edx
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index 7595988884..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.<BR>
> +; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
>   ; 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
>       mov        rax, [rsp + 40]   ; CountTofinish
>       lock dec   dword [rax]       ; (*CountTofinish)--
>       mov        rsp, r9
> @@ -288,6 +289,7 @@ PmEntry:
>       jnz        HltLoop
>       mov        ebx, edx           ; Save C-State to ebx
>   MwaitLoop:
> +    cli
>       mov        eax, esp           ; Set Monitor Address
>       xor        ecx, ecx           ; ecx = 0
>       xor        edx, edx           ; edx = 0
> 
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>

-- 
Thanks,
Ray


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait
  2018-03-19  5:54 ` Ni, Ruiyu
@ 2018-03-19  6:13   ` Yao, Jiewen
  0 siblings, 0 replies; 4+ messages in thread
From: Yao, Jiewen @ 2018-03-19  6:13 UTC (permalink / raw)
  To: Ni, Ruiyu, Wu, Hao A, edk2-devel@lists.01.org
  Cc: Dong, Eric, Laszlo Ersek, Zeng, Star

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Monday, March 19, 2018 1:54 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Zeng, Star <star.zeng@intel.com>
> 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 <lersek@redhat.com>
> > Cc: Jeff Fan <vanjeff_919@hotmail.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> > ---
> >   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.<BR>
> > +; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> >   ; 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:
> >       cmp        cl,  1              ; Check mwait-monitor support
> >       jnz        HltLoop
> >   MwaitLoop:
> > +    cli
> >       mov        eax, esp
> >       xor        ecx, ecx
> >       xor        edx, edx
> > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> > index 7595988884..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.<BR>
> > +; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
> >   ; 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
> >       mov        rax, [rsp + 40]   ; CountTofinish
> >       lock dec   dword [rax]       ; (*CountTofinish)--
> >       mov        rsp, r9
> > @@ -288,6 +289,7 @@ PmEntry:
> >       jnz        HltLoop
> >       mov        ebx, edx           ; Save C-State to ebx
> >   MwaitLoop:
> > +    cli
> >       mov        eax, esp           ; Set Monitor Address
> >       xor        ecx, ecx           ; ecx = 0
> >       xor        edx, edx           ; edx = 0
> >
> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
> 
> --
> Thanks,
> Ray
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* 答复: [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait
  2018-03-19  5:31 [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait Hao Wu
  2018-03-19  5:54 ` Ni, Ruiyu
@ 2018-03-19 13:31 ` Fan Jeff
  1 sibling, 0 replies; 4+ messages in thread
From: Fan Jeff @ 2018-03-19 13:31 UTC (permalink / raw)
  To: Hao Wu, edk2-devel@lists.01.org
  Cc: Hao Wu, Laszlo Ersek, Ruiyu Ni, Jiewen Yao, Jian J Wang,
	Star Zeng, Eric Dong

Thanks!

Reviewed-by:  Jeff Fan <vanjeff_919@hotmail.com>

发件人: Hao Wu<mailto:hao.a.wu@intel.com>
发送时间: 2018年3月19日 13:31
收件人: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
抄送: Hao Wu<mailto:hao.a.wu@intel.com>; Laszlo Ersek<mailto:lersek@redhat.com>; Jeff Fan<mailto:vanjeff_919@hotmail.com>; Ruiyu Ni<mailto:ruiyu.ni@intel.com>; Jiewen Yao<mailto:jiewen.yao@intel.com>; Jian J Wang<mailto:jian.j.wang@intel.com>; Star Zeng<mailto:star.zeng@intel.com>; Eric Dong<mailto:eric.dong@intel.com>
主题: [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait

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 <lersek@redhat.com>
Cc: Jeff Fan <vanjeff_919@hotmail.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
 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.<BR>
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 ; 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:
     cmp        cl,  1              ; Check mwait-monitor support
     jnz        HltLoop
 MwaitLoop:
+    cli
     mov        eax, esp
     xor        ecx, ecx
     xor        edx, edx
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index 7595988884..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.<BR>
+; Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
 ; 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
     mov        rax, [rsp + 40]   ; CountTofinish
     lock dec   dword [rax]       ; (*CountTofinish)--
     mov        rsp, r9
@@ -288,6 +289,7 @@ PmEntry:
     jnz        HltLoop
     mov        ebx, edx           ; Save C-State to ebx
 MwaitLoop:
+    cli
     mov        eax, esp           ; Set Monitor Address
     xor        ecx, ecx           ; ecx = 0
     xor        edx, edx           ; edx = 0
--
2.12.0.windows.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-03-19 13:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-03-19  5:31 [PATCH] UefiCpuPkg/MpInitLib: Disable interrupt at ExitBootServices AP Mwait Hao Wu
2018-03-19  5:54 ` Ni, Ruiyu
2018-03-19  6:13   ` Yao, Jiewen
2018-03-19 13:31 ` 答复: " Fan Jeff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox