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]> wrote:

> 
> On Mar 30, 2014, at 8:19 PM, Gao, Liming <[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]
>> 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]
>> https://lists.sourceforge.net/lists/listinfo/edk2-devel
> 

------------------------------------------------------------------------------
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to