Re: [PATCH 0/2] Put AP into safe hlt-loop code on S3 path

["Fan, Jeff" <jeff.fan@intel.com>]

Paolo,

I added patch #3 in v2 to do InterlockedDecrement (&mNumberToFinish) in the safe code. 
This is very good comment to eliminate this gap.

Thanks!
Jeff

-----Original Message-----
From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
Sent: Thursday, November 10, 2016 5:59 PM
To: Fan, Jeff; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 0/2] Put AP into safe hlt-loop code on S3 path



On 10/11/2016 07:07, Jeff Fan wrote:
> On S3 path, we will wake up APs to restore CPU context in 
> PiSmmCpuDxeSmm driver. In case, one NMI or SMI happens, APs may exit 
> from hlt state and execute the instruction after HLT instruction.
> 
> But APs are not running on safe code, it leads OVMF S3 boot unstable.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=216
> 
> I tested real platform with 64bit DXE.
> 
> Jeff Fan (2):
>   UefiCpuPkg/PiSmmCpuDxeSmm: Put AP into safe hlt-loop code on S3 path
>   UefiCpuPkg/PiSmmCpuDxeSmm: Place AP to 32bit protected mode on S3 
> path
> 
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c             | 31 ++++++++++++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 25 ++++++++++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h    | 13 ++++++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  | 59 
> +++++++++++++++++++++++++++
>  4 files changed, 128 insertions(+)
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

It would be slightly more robust to do the "InterlockedDecrement (&mNumberToFinish);" while in safe state, but the race window is really really small.

Paolo