Hi, Andrew and All: After further evaluation, I think we can remove the unused internal structure PEIM_FILE_HANDLE_EXTENDED_DATA from PeiCore to reduce the confuse. The patch is attached. Please help review it.
Thanks Liming From: Gao, Liming [mailto:[email protected]] Sent: Wednesday, April 09, 2014 11:21 AM To: [email protected] Subject: Re: [edk2] MdeModulePkg maintainer, I'm confused by code in the PEI Core dispatcher? It looks wrong? Yes. I attach my patch. Please help review it. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Gao, Liming [email protected]<mailto:[email protected]> Thanks Liming From: Andrew Fish [mailto:[email protected]] Sent: Monday, April 07, 2014 2:53 AM To: [email protected]<mailto:[email protected]> Subject: Re: [edk2] MdeModulePkg maintainer, I'm confused by code in the PEI Core dispatcher? It looks wrong? Liming, Do you want to submit add some comments to the code to document this issue? Thanks, Andrew Fish On Mar 31, 2014, at 7:58 AM, Andrew Fish <[email protected]<mailto:[email protected]>> wrote: On Mar 30, 2014, at 8:19 PM, Gao, Liming <[email protected]<mailto:[email protected]>> wrote: Andrew: PeiCore extended report data includes two fields: DataHeader and Handle. But, it only uses PEIM File Handle. I agree DataHeader field is redundant. No one uses it. It should not cause any issue. So, I think it is safe to keep it as it. To reduce confuse, we can add some comments for the unused DataHeader. Aside from the uninitialized data the use of a local structure also creates alignment issues. This data structure is not the same for 32-bit vs. 64-bit builds. The only way to pull the data of on the consumer side is to re-define the data structure in the logging code. I was trying to avoid having to redefine internal PEI Core types in the logging code. Thanks, Andrew Fish Thanks Liming From: Andrew Fish [mailto:[email protected]] Sent: Saturday, March 29, 2014 11:33 AM To: [email protected]<mailto:[email protected]> Subject: [edk2] MdeModulePkg maintainer, I'm confused by code in the PEI Core dispatcher? It looks wrong? It looks like the PEI Core report status code calls for (EFI_SOFTWARE_PEI_CORE | EFI_SW_PC_INIT_BEGIN) and (EFI_SOFTWARE_PEI_CORE | EFI_SW_PC_INIT_END) both pass in random, unitialized data? I don't understand the point of PEIM_FILE_HANDLE_EXTENDED_DATA. The DataHeader is not filled in by the call, as you can see in ReportStatusCodeEx(), the header data is allocated locally and is not passed in? It looks to me like the DXE Core just passed in data directly? Why is the PEI Core defining PEIM_FILE_HANDLE_EXTENDED_DATA, and doing this? Maybe I'm missing something? Thanks Andrew Fish https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c typedef struct { EFI_STATUS_CODE_DATA DataHeader; EFI_HANDLE Handle; } PEIM_FILE_HANDLE_EXTENDED_DATA; ExtendedData.Handle = (EFI_HANDLE)PeimFileHandle; REPORT_STATUS_CODE_WITH_EXTENDED_DATA ( EFI_PROGRESS_CODE, (EFI_SOFTWARE_PEI_CORE | EFI_SW_PC_INIT_BEGIN), (VOID *)(&ExtendedData), sizeof (ExtendedData) ); https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Library/PeiReportStatusCodeLib/ReportStatusCodeLib.c EFI_STATUS EFIAPI ReportStatusCodeEx ( IN EFI_STATUS_CODE_TYPE Type, IN EFI_STATUS_CODE_VALUE Value, IN UINT32 Instance, IN CONST EFI_GUID *CallerId OPTIONAL, IN CONST EFI_GUID *ExtendedDataGuid OPTIONAL, IN CONST VOID *ExtendedData OPTIONAL, IN UINTN ExtendedDataSize ) { EFI_STATUS_CODE_DATA *StatusCodeData; UINT64 Buffer[(MAX_EXTENDED_DATA_SIZE / sizeof (UINT64)) + 1]; // // If ExtendedData is NULL and ExtendedDataSize is not zero, then ASSERT(). // ASSERT (!((ExtendedData == NULL) && (ExtendedDataSize != 0))); // // If ExtendedData is not NULL and ExtendedDataSize is zero, then ASSERT(). // ASSERT (!((ExtendedData != NULL) && (ExtendedDataSize == 0))); if (ExtendedDataSize > (MAX_EXTENDED_DATA_SIZE - sizeof (EFI_STATUS_CODE_DATA))) { // // The local variable Buffer not large enough to hold the extended data associated // with the status code being reported. // DEBUG ((EFI_D_ERROR, "Status code extended data is too large to be reported!\n")); return EFI_OUT_OF_RESOURCES; } StatusCodeData = (EFI_STATUS_CODE_DATA *) Buffer; StatusCodeData->HeaderSize = (UINT16) sizeof (EFI_STATUS_CODE_DATA); StatusCodeData->Size = (UINT16) ExtendedDataSize; if (ExtendedDataGuid == NULL) { ExtendedDataGuid = &gEfiStatusCodeSpecificDataGuid; } CopyGuid (&StatusCodeData->Type, ExtendedDataGuid); if (ExtendedData != NULL) { CopyMem (StatusCodeData + 1, ExtendedData, ExtendedDataSize); } if (CallerId == NULL) { CallerId = &gEfiCallerIdGuid; } return InternalReportStatusCode (Type, Value, Instance, CallerId, StatusCodeData); } https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdeModulePkg/Core/Dxe/Dispatcher/Dispatcher.c REPORT_STATUS_CODE_WITH_EXTENDED_DATA ( EFI_PROGRESS_CODE, (EFI_SOFTWARE_DXE_CORE | EFI_SW_PC_INIT_BEGIN), &DriverEntry->ImageHandle, sizeof (DriverEntry->ImageHandle) ); ASSERT (DriverEntry->ImageHandle != NULL); Status = CoreStartImage (DriverEntry->ImageHandle, NULL, NULL); ------------------------------------------------------------------------------ _______________________________________________ edk2-devel mailing list [email protected]<mailto:[email protected]> https://lists.sourceforge.net/lists/listinfo/edk2-devel
0001-MdeModulePkg-Remove-unused-internal-structure-in-Pei.patch
Description: 0001-MdeModulePkg-Remove-unused-internal-structure-in-Pei.patch
------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/
_______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
