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] 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
Dispatcher.c.patch
Description: Dispatcher.c.patch
------------------------------------------------------------------------------ Put Bad Developers to Shame Dominate Development with Jenkins Continuous Integration Continuously Automate Build, Test & Deployment Start a new project now. Try Jenkins in the cloud. http://p.sf.net/sfu/13600_Cloudbees
_______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
