Re: [PATCH 2/2] MdeModulePkg/PiSmmCpuDxeSmm: Check RegisterCpuInterruptHandler status

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Jeff,

Shouldn't this ASSERT() be moved into the implementation
of SmmRegisterExceptionHandler()?  This will make sure
the PiSmmCpuDxeSmm driver uses the right lib instance 
for exceptions that module registers and for exceptions
other SMM modules register through the EFI_SMM_CPU_SERVICE_PROTOCOL.

Mike

> -----Original Message-----
> From: Fan, Jeff
> Sent: Wednesday, November 16, 2016 6:34 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [PATCH 2/2] MdeModulePkg/PiSmmCpuDxeSmm: Check RegisterCpuInterruptHandler
> status
> 
> Once platform selects the incorrect instance, the caller could know it from
> return status and ASSERT().
> 
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan <jeff.fan@intel.com>
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c   | 5 ++++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c  | 4 +++-
>  3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index a871bef..f2e2ceb 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> @@ -27,6 +27,7 @@ SmmInitPageTable (
>  {
>    UINTN                             PageFaultHandlerHookAddress;
>    IA32_IDT_GATE_DESCRIPTOR          *IdtEntry;
> +  EFI_STATUS                        Status;
> 
>    //
>    // Initialize spin lock
> @@ -49,7 +50,8 @@ SmmInitPageTable (
>      //
>      // Register SMM Page Fault Handler
>      //
> -    SmmRegisterExceptionHandler (&mSmmCpuService, EXCEPT_IA32_PAGE_FAULT,
> SmiPFHandler);
> +    Status = SmmRegisterExceptionHandler (&mSmmCpuService, EXCEPT_IA32_PAGE_FAULT,
> SmiPFHandler);
> +    ASSERT_EFI_ERROR (Status);
>    }
> 
>    //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> index 329574e..350a6a2 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
> @@ -1384,5 +1384,8 @@ InitIdtr (
>    VOID
>    )
>  {
> -  SmmRegisterExceptionHandler (&mSmmCpuService, EXCEPT_IA32_DEBUG,
> DebugExceptionHandler);
> +  EFI_STATUS                        Status;
> +
> +  Status = SmmRegisterExceptionHandler (&mSmmCpuService, EXCEPT_IA32_DEBUG,
> DebugExceptionHandler);
> +  ASSERT_EFI_ERROR (Status);
>  }
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 9cee784..c49fb26 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -102,6 +102,7 @@ SmmInitPageTable (
>    UINTN                             Index;
>    UINTN                             PageFaultHandlerHookAddress;
>    IA32_IDT_GATE_DESCRIPTOR          *IdtEntry;
> +  EFI_STATUS                        Status;
> 
>    //
>    // Initialize spin lock
> @@ -160,7 +161,8 @@ SmmInitPageTable (
>      //
>      // Register Smm Page Fault Handler
>      //
> -    SmmRegisterExceptionHandler (&mSmmCpuService, EXCEPT_IA32_PAGE_FAULT,
> SmiPFHandler);
> +    Status = SmmRegisterExceptionHandler (&mSmmCpuService, EXCEPT_IA32_PAGE_FAULT,
> SmiPFHandler);
> +    ASSERT_EFI_ERROR (Status);
>    }
> 
>    //
> --
> 2.9.3.windows.2