* [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0 in SecMain
@ 2019-07-25 15:50 Chiu, Chasel
2019-07-26 0:24 ` [edk2-devel] " Nate DeSimone
2019-07-26 1:28 ` Zeng, Star
0 siblings, 2 replies; 3+ messages in thread
From: Chiu, Chasel @ 2019-07-25 15:50 UTC (permalink / raw)
To: devel; +Cc: Nate DeSimone, Star Zeng
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2018
Current FSP SEC_IDT_TABLE structure is not natural aligned:
typedef struct _SEC_IDT_TABLE {
EFI_PEI_SERVICES *PeiService;
UINT64 IdtTable[];
} SEC_IDT_TABLE;
Compiler will insert DWORD padding between 2 elements and
GetPeiServicesTablePointer() in early phase then always
returns padding data from stack, which was not rest to 0
in SecMain.
Solution is to align FSP SEC_IDT_TABLE structure to UefiCpuPkg
to have UINT64 as PeiService field and rest it to 0.
Test: Verified on internal platform and booting successfully
with FSP API mode.
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
IntelFsp2Pkg/FspSecCore/SecMain.c | 2 +-
IntelFsp2Pkg/FspSecCore/SecMain.h | 10 ++++++++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c b/IntelFsp2Pkg/FspSecCore/SecMain.c
index a63d1336e4..7169afc6c7 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.c
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
@@ -114,7 +114,7 @@ SecStartup (
// | |
// | |
// |-------------------|----> TempRamBase
- IdtTableInStack.PeiService = NULL;
+ IdtTableInStack.PeiService = 0;
AsmReadIdtr (&IdtDescriptor);
if (IdtDescriptor.Base == 0) {
ExceptionHandler = FspGetExceptionHandler(mIdtEntryTemplate);
diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.h b/IntelFsp2Pkg/FspSecCore/SecMain.h
index 6fb16febab..af7f387960 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.h
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.h
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -29,7 +29,13 @@ typedef VOID (*PEI_CORE_ENTRY) ( \
);
typedef struct _SEC_IDT_TABLE {
- EFI_PEI_SERVICES *PeiService;
+ //
+ // Reserved 8 bytes preceding IDT to store EFI_PEI_SERVICES**, since IDT base
+ // address should be 8-byte alignment.
+ // Note: For IA32, only the 4 bytes immediately preceding IDT is used to store
+ // EFI_PEI_SERVICES**
+ //
+ UINT64 PeiService;
UINT64 IdtTable[FixedPcdGet8 (PcdFspMaxInterruptSupported)];
} SEC_IDT_TABLE;
--
2.13.3.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-devel] [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0 in SecMain
2019-07-25 15:50 [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0 in SecMain Chiu, Chasel
@ 2019-07-26 0:24 ` Nate DeSimone
2019-07-26 1:28 ` Zeng, Star
1 sibling, 0 replies; 3+ messages in thread
From: Nate DeSimone @ 2019-07-26 0:24 UTC (permalink / raw)
To: devel@edk2.groups.io, Chiu, Chasel; +Cc: Zeng, Star
Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chiu, Chasel
Sent: Thursday, July 25, 2019 8:51 AM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2-devel] [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0 in SecMain
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2018
Current FSP SEC_IDT_TABLE structure is not natural aligned:
typedef struct _SEC_IDT_TABLE {
EFI_PEI_SERVICES *PeiService;
UINT64 IdtTable[];
} SEC_IDT_TABLE;
Compiler will insert DWORD padding between 2 elements and
GetPeiServicesTablePointer() in early phase then always returns padding data from stack, which was not rest to 0 in SecMain.
Solution is to align FSP SEC_IDT_TABLE structure to UefiCpuPkg to have UINT64 as PeiService field and rest it to 0.
Test: Verified on internal platform and booting successfully
with FSP API mode.
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
---
IntelFsp2Pkg/FspSecCore/SecMain.c | 2 +- IntelFsp2Pkg/FspSecCore/SecMain.h | 10 ++++++++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c b/IntelFsp2Pkg/FspSecCore/SecMain.c
index a63d1336e4..7169afc6c7 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.c
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
@@ -114,7 +114,7 @@ SecStartup (
// | |
// | |
// |-------------------|----> TempRamBase
- IdtTableInStack.PeiService = NULL;
+ IdtTableInStack.PeiService = 0;
AsmReadIdtr (&IdtDescriptor);
if (IdtDescriptor.Base == 0) {
ExceptionHandler = FspGetExceptionHandler(mIdtEntryTemplate);
diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.h b/IntelFsp2Pkg/FspSecCore/SecMain.h
index 6fb16febab..af7f387960 100644
--- a/IntelFsp2Pkg/FspSecCore/SecMain.h
+++ b/IntelFsp2Pkg/FspSecCore/SecMain.h
@@ -1,6 +1,6 @@
/** @file
- Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2014 - 2019, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
@@ -29,7 +29,13 @@ typedef VOID (*PEI_CORE_ENTRY) ( \ );
typedef struct _SEC_IDT_TABLE {
- EFI_PEI_SERVICES *PeiService;
+ //
+ // Reserved 8 bytes preceding IDT to store EFI_PEI_SERVICES**, since
+ IDT base // address should be 8-byte alignment.
+ // Note: For IA32, only the 4 bytes immediately preceding IDT is used
+ to store // EFI_PEI_SERVICES** //
+ UINT64 PeiService;
UINT64 IdtTable[FixedPcdGet8 (PcdFspMaxInterruptSupported)];
} SEC_IDT_TABLE;
--
2.13.3.windows.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-devel] [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0 in SecMain
2019-07-25 15:50 [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0 in SecMain Chiu, Chasel
2019-07-26 0:24 ` [edk2-devel] " Nate DeSimone
@ 2019-07-26 1:28 ` Zeng, Star
1 sibling, 0 replies; 3+ messages in thread
From: Zeng, Star @ 2019-07-26 1:28 UTC (permalink / raw)
To: devel@edk2.groups.io, Chiu, Chasel; +Cc: Desimone, Nathaniel L, Zeng, Star
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Chiu, Chasel
> Sent: Thursday, July 25, 2019 11:51 PM
> To: devel@edk2.groups.io
> Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: [edk2-devel] [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0
> in SecMain
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2018
>
> Current FSP SEC_IDT_TABLE structure is not natural aligned:
> typedef struct _SEC_IDT_TABLE {
> EFI_PEI_SERVICES *PeiService;
> UINT64 IdtTable[];
> } SEC_IDT_TABLE;
> Compiler will insert DWORD padding between 2 elements and
> GetPeiServicesTablePointer() in early phase then always returns padding
> data from stack, which was not rest to 0 in SecMain.
'rest' -> 'reset'
>
> Solution is to align FSP SEC_IDT_TABLE structure to UefiCpuPkg to have
> UINT64 as PeiService field and rest it to 0.
'rest' to 'reset'
Please correct the typo 'rest' to 'reset' in the commit message.
After that, Reviewed-by: Star Zeng <star.zeng@intel.com>.
Thanks,
Star
>
> Test: Verified on internal platform and booting successfully
> with FSP API mode.
>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Chasel Chiu <chasel.chiu@intel.com>
> ---
> IntelFsp2Pkg/FspSecCore/SecMain.c | 2 +-
> IntelFsp2Pkg/FspSecCore/SecMain.h | 10 ++++++++--
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.c
> b/IntelFsp2Pkg/FspSecCore/SecMain.c
> index a63d1336e4..7169afc6c7 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.c
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.c
> @@ -114,7 +114,7 @@ SecStartup (
> // | |
> // | |
> // |-------------------|----> TempRamBase
> - IdtTableInStack.PeiService = NULL;
> + IdtTableInStack.PeiService = 0;
> AsmReadIdtr (&IdtDescriptor);
> if (IdtDescriptor.Base == 0) {
> ExceptionHandler = FspGetExceptionHandler(mIdtEntryTemplate);
> diff --git a/IntelFsp2Pkg/FspSecCore/SecMain.h
> b/IntelFsp2Pkg/FspSecCore/SecMain.h
> index 6fb16febab..af7f387960 100644
> --- a/IntelFsp2Pkg/FspSecCore/SecMain.h
> +++ b/IntelFsp2Pkg/FspSecCore/SecMain.h
> @@ -1,6 +1,6 @@
> /** @file
>
> - Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2014 - 2019, Intel Corporation. All rights
> + reserved.<BR>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> **/
> @@ -29,7 +29,13 @@ typedef VOID (*PEI_CORE_ENTRY) ( \ );
>
> typedef struct _SEC_IDT_TABLE {
> - EFI_PEI_SERVICES *PeiService;
> + //
> + // Reserved 8 bytes preceding IDT to store EFI_PEI_SERVICES**, since
> + IDT base // address should be 8-byte alignment.
> + // Note: For IA32, only the 4 bytes immediately preceding IDT is used
> + to store // EFI_PEI_SERVICES** //
> + UINT64 PeiService;
> UINT64 IdtTable[FixedPcdGet8 (PcdFspMaxInterruptSupported)];
> } SEC_IDT_TABLE;
>
> --
> 2.13.3.windows.1
>
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-07-26 1:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-07-25 15:50 [PATCH] IntelFsp2Pkg: PeiService pointer not reset to 0 in SecMain Chiu, Chasel
2019-07-26 0:24 ` [edk2-devel] " Nate DeSimone
2019-07-26 1:28 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox