Hi Chasel
Good idea to reserve bootloader IDT.
I have a little concern on the code below. It unconditionally limits the boot
loader IDT to our IdtTableInStack.
#define SEC_IDT_ENTRY_COUNT 34
typedef struct _SEC_IDT_TABLE {
EFI_PEI_SERVICES *PeiService;
UINT64 IdtTable[SEC_IDT_ENTRY_COUNT];
} SEC_IDT_TABLE;
Is that right assumption that 34 entries is enough?
Can we enlarge the IdtTableInStack to hold or bootloader IDT?
Or can we enlarge IdtTableInStack to hold all 0x100 entries, and use ASSERT for
that?
Silence failure seems not the best choice.
> + // Get minimum size
> + if (IdtDescriptor.Limit + 1 > sizeof (IdtTableInStack.IdtTable)) {
> + IdtSize = sizeof (IdtTableInStack.IdtTable);
> + } else {
Thank you
Yao Jiewen
> -----Original Message-----
> From: Chiu, Chasel
> Sent: Friday, October 19, 2018 5:43 PM
> To: [email protected]
> Cc: Yao, Jiewen <[email protected]>; Desimone, Nathaniel L
> <[email protected]>; Chiu, Chasel <[email protected]>
> Subject: [PATCH] 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.
>
> Test: Verified on internal platform and boots successfully.
>
> Cc: Jiewen Yao <[email protected]>
> Cc: Desimone Nathaniel L <[email protected]>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chasel Chiu <[email protected]>
> ---
> IntelFsp2Pkg/FspSecCore/SecMain.c | 22 +++++++++++++++++-----
> 1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
> b/IntelFsp2Pkg/FspSecCore/SecMain.c
> index 37fd4dfdeb..5912221204 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,24 @@ 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 < SEC_IDT_ENTRY_COUNT; Index ++) {
> + CopyMem ((VOID*)&IdtTableInStack.IdtTable[Index],
> (VOID*)&ExceptionHandler, sizeof (UINT64));
> + }
> + IdtSize = sizeof (IdtTableInStack.IdtTable);
> + } else {
> + // Get minimum size
> + if (IdtDescriptor.Limit + 1 > sizeof (IdtTableInStack.IdtTable)) {
> + IdtSize = sizeof (IdtTableInStack.IdtTable);
> + } 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);
>
> --
> 2.13.3.windows.1
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
