Re: [PATCH v2] IntelFsp2Pkg: FSP should not override IDT

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Chiu, Chasel
> Sent: Tuesday, October 23, 2018 5:34 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
> Subject: [PATCH v2] IntelFsp2Pkg: FSP should not override IDT
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1265
> 
> FSP should not override IDT table when it is initialized
> by boot loader. IDT should be re-initialized in FSP only
> when it is invalid.
> To mitigate temporary memory usage a PCD
> PcdFspMaxInterruptSupported created for platform to decide
> how many interrupts the FSP IDT table can support.
> 
> Test: Verified on internal platform and boots successfully.
> 
> Cc: Jiewen Yao <Jiewen.yao@intel.com>
> Cc: Desimone Nathaniel L <nathaniel.l.desimone@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
>  IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf |  1 +
>  IntelFsp2Pkg/FspSecCore/SecMain.c       | 24
> +++++++++++++++++++-----
>  IntelFsp2Pkg/FspSecCore/SecMain.h       |  6 ++----
>  IntelFsp2Pkg/IntelFsp2Pkg.dec           |  4 ++++
>  4 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> index c61af10b8a..dafe6f5993 100644
> --- a/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> +++ b/IntelFsp2Pkg/FspSecCore/FspSecCoreM.inf
> @@ -62,6 +62,7 @@
>    gIntelFsp2PkgTokenSpaceGuid.PcdTemporaryRamSize              ##
> CONSUMES
>    gIntelFsp2PkgTokenSpaceGuid.PcdFspTemporaryRamSize           ##
> CONSUMES
>    gIntelFsp2PkgTokenSpaceGuid.PcdFspHeapSizePercentage         ##
> CONSUMES
> +  gIntelFsp2PkgTokenSpaceGuid.PcdFspMaxInterruptSupported      ##
> CONSUMES
> 
>  [Ppis]
>    gEfiTemporaryRamSupportPpiGuid
> ## PRODUCES
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
> b/IntelFsp2Pkg/FspSecCore/SecMain.c
> index 37fd4dfdeb..ddbfc4fcdf 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
> @@ -70,6 +70,7 @@ SecStartup (
>    UINT32                      Index;
>    FSP_GLOBAL_DATA             PeiFspData;
>    UINT64                      ExceptionHandler;
> +  UINTN                       IdtSize;
> 
>    //
>    // Process all libraries constructor function linked to SecCore.
> @@ -98,13 +99,26 @@ SecStartup (
>    // |                   |
>    // |-------------------|---->  TempRamBase
>    IdtTableInStack.PeiService  = NULL;
> -  ExceptionHandler = FspGetExceptionHandler(mIdtEntryTemplate);
> -  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
> -    CopyMem ((VOID*)&IdtTableInStack.IdtTable[Index],
> (VOID*)&ExceptionHandler, sizeof (UINT64));
> +  AsmReadIdtr (&IdtDescriptor);
> +  if ((IdtDescriptor.Base == 0) && (IdtDescriptor.Limit == 0xFFFF)) {
> +    ExceptionHandler = FspGetExceptionHandler(mIdtEntryTemplate);
> +    for (Index = 0; Index < FixedPcdGet8(PcdFspMaxInterruptSupported);
> Index ++) {
> +      CopyMem ((VOID*)&IdtTableInStack.IdtTable[Index],
> (VOID*)&ExceptionHandler, sizeof (UINT64));
> +    }
> +    IdtSize = sizeof (IdtTableInStack.IdtTable);
> +  } else {
> +    if (IdtDescriptor.Limit + 1 > sizeof (IdtTableInStack.IdtTable)) {
> +      //
> +      // ERROR: IDT table size from boot loader is larger than FSP can
> support, DeadLoop here!
> +      //
> +      CpuDeadLoop();
> +    } else {
> +      IdtSize = IdtDescriptor.Limit + 1;
> +    }
> +    CopyMem ((VOID *) (UINTN) &IdtTableInStack.IdtTable, (VOID *)
> IdtDescriptor.Base, IdtSize);
>    }
> -
>    IdtDescriptor.Base  = (UINTN) &IdtTableInStack.IdtTable;
> -  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
> +  IdtDescriptor.Limit = (UINT16)(IdtSize - 1);
> 
>    AsmWriteIdtr (&IdtDescriptor);
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.h
> b/IntelFsp2Pkg/FspSecCore/SecMain.h
> index 291bc5ca5c..19ac2fbfc1 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.h
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.h
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -  Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.<BR>
> +  Copyright (c) 2014 - 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
> @@ -29,8 +29,6 @@
>  #include <Library/FspCommonLib.h>
>  #include <FspEas.h>
> 
> -#define SEC_IDT_ENTRY_COUNT    34
> -
>  typedef VOID (*PEI_CORE_ENTRY) ( \
>    IN CONST  EFI_SEC_PEI_HAND_OFF    *SecCoreData, \
>    IN CONST  EFI_PEI_PPI_DESCRIPTOR  *PpiList \
> @@ -38,7 +36,7 @@ typedef VOID (*PEI_CORE_ENTRY) ( \
> 
>  typedef struct _SEC_IDT_TABLE {
>    EFI_PEI_SERVICES  *PeiService;
> -  UINT64            IdtTable[SEC_IDT_ENTRY_COUNT];
> +  UINT64            IdtTable[FixedPcdGet8
> (PcdFspMaxInterruptSupported)];
>  } SEC_IDT_TABLE;
> 
>  /**
> diff --git a/IntelFsp2Pkg/IntelFsp2Pkg.dec b/IntelFsp2Pkg/IntelFsp2Pkg.dec
> index 5b037d65e2..50496241da 100644
> --- a/IntelFsp2Pkg/IntelFsp2Pkg.dec
> +++ b/IntelFsp2Pkg/IntelFsp2Pkg.dec
> @@ -86,6 +86,10 @@
>    # x % of FSP temporary memory will be used for heap
>    # (100 - x) % of FSP temporary memory will be used for stack
>    gIntelFsp2PkgTokenSpaceGuid.PcdFspHeapSizePercentage    |
> 50| UINT8|0x10000004
> +  #
> +  # Maximal Interrupt supported in IDT table.
> +  #
> +  gIntelFsp2PkgTokenSpaceGuid.PcdFspMaxInterruptSupported |
> 34| UINT8|0x10000005
> 
>  [PcdsFixedAtBuild,PcdsDynamic,PcdsDynamicEx]
>    gIntelFsp2PkgTokenSpaceGuid.PcdFspReservedMemoryLength
> |0x00100000|UINT32|0x46530000
> --
> 2.13.3.windows.1