On 28 October 2015 at 10:33, Star Zeng <[email protected]> wrote: > This patch does below things. > 1. Smbios3Table used as SmbiosTable wrongly after SmbiosTable got from > configuration table. > 2. Correct the return comments of entrypoint function. > 3. Add parameters' comments for MeasureSmbiosTable(). > 4. Remove the tailing space from SmbiosMeasurementDxe.c. > 5. Correct the Protocols and Guids usage comments in SmbiosMeasurementDxe.inf. > 6. Add (VOID **) typecast in LocateProtocol call for GCC build failure. > 7. Use EFI_D_VERBOSE instead of EFI_D_INFO in InternalDumpData() and > InternalDumpHex(). > 8. Use correct VendorGuid and VendorTable to measure. >
If you need a numbered list to summarize the changes you are making in a patch, that is usually a strong hint that you should split it into several patches. You have a bug fixes (1,8), comment fixes (2,3,5), a functional change (7), a build fix (6) and a whitespace fix (4), so you should split this into 5 or 6 patches at least. -- Ard. > Cc:Jiewen Yao <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Star Zeng <[email protected]> > --- > .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.c | 75 > +++++++++++++--------- > .../SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf | 6 +- > 2 files changed, 46 insertions(+), 35 deletions(-) > > diff --git > a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c > b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c > index 2152827..c10e9f7 100644 > --- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c > +++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.c > @@ -1,14 +1,14 @@ > /** @file > This driver measures SMBIOS table to TPM. > - > + > Copyright (c) 2015, Intel Corporation. All rights reserved.<BR> > -This program and the accompanying materials > -are licensed and made available under the terms and conditions of the BSD > License > -which accompanies this distribution. The full text of the license may be > found at > -http://opensource.org/licenses/bsd-license.php > - > -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +This program and the accompanying materials > +are licensed and made available under the terms and conditions of the BSD > License > +which accompanies this distribution. The full text of the license may be > found at > +http://opensource.org/licenses/bsd-license.php > + > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > > **/ > > @@ -125,7 +125,7 @@ InternalDumpData ( > { > UINTN Index; > for (Index = 0; Index < Size; Index++) { > - DEBUG ((EFI_D_INFO, "%02x", (UINTN)Data[Index])); > + DEBUG ((EFI_D_VERBOSE, "%02x", (UINTN)Data[Index])); > } > } > > @@ -152,15 +152,15 @@ InternalDumpHex ( > Count = Size / COLUME_SIZE; > Left = Size % COLUME_SIZE; > for (Index = 0; Index < Count; Index++) { > - DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE)); > + DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE)); > InternalDumpData (Data + Index * COLUME_SIZE, COLUME_SIZE); > - DEBUG ((EFI_D_INFO, "\n")); > + DEBUG ((EFI_D_VERBOSE, "\n")); > } > > if (Left != 0) { > - DEBUG ((EFI_D_INFO, "%04x: ", Index * COLUME_SIZE)); > + DEBUG ((EFI_D_VERBOSE, "%04x: ", Index * COLUME_SIZE)); > InternalDumpData (Data + Index * COLUME_SIZE, Left); > - DEBUG ((EFI_D_INFO, "\n")); > + DEBUG ((EFI_D_VERBOSE, "\n")); > } > } > > @@ -217,7 +217,7 @@ GetSmbiosStringById ( > // look for the two consecutive zeros, check the string limit by the way. > // > String = NULL; > - while (*CharInStr != 0 || *(CharInStr+1) != 0) { > + while (*CharInStr != 0 || *(CharInStr+1) != 0) { > if (*CharInStr == 0) { > Size += 1; > CharInStr++; > @@ -271,7 +271,7 @@ FilterSmbiosEntry ( > UINTN StringLen; > > DEBUG ((EFI_D_INFO, "Smbios Table (Type - %d):\n", ((SMBIOS_STRUCTURE > *)TableEntry)->Type)); > - InternalDumpHex (TableEntry, TableEntrySize); > + DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize);); > > FilterStruct = GetFilterStructByType (((SMBIOS_STRUCTURE > *)TableEntry)->Type); > if (FilterStruct != NULL) { > @@ -297,7 +297,7 @@ FilterSmbiosEntry ( > } > > DEBUG ((EFI_D_INFO, "Filter Smbios Table (Type - %d):\n", > ((SMBIOS_STRUCTURE *)TableEntry)->Type)); > - InternalDumpHex (TableEntry, TableEntrySize); > + DEBUG_CODE (InternalDumpHex (TableEntry, TableEntrySize);); > } > > /** > @@ -306,7 +306,7 @@ FilterSmbiosEntry ( > > @param Head Pointer to the beginning of SMBIOS structure. > @param NumberOfStrings The returned number of optional strings that > follow the formatted structure. > - > + > @return Size The returned size. > **/ > UINTN > @@ -327,7 +327,7 @@ GetSmbiosStructureSize ( > // > // look for the two consecutive zeros, check the string limit by the way. > // > - while (*CharInStr != 0 || *(CharInStr+1) != 0) { > + while (*CharInStr != 0 || *(CharInStr+1) != 0) { > if (*CharInStr == 0) { > Size += 1; > CharInStr++; > @@ -432,7 +432,11 @@ FilterSmbiosTable ( > } > > /** > - Measure SMBIOS with EV_EFI_HANDOFF_TABLES to PCR[1] > + Measure SMBIOS with EV_EFI_HANDOFF_TABLES to PCR[1]. > + > + @param[in] Event Event whose notification function is being invoked. > + @param[in] Context Pointer to the notification function's context. > + > **/ > VOID > EFIAPI > @@ -478,6 +482,8 @@ MeasureSmbiosTable ( > DEBUG ((EFI_D_INFO, " TableAddress - 0x%016lx\n", > Smbios3Table->TableAddress)); > } > } > + > + if (Smbios3Table == NULL) { > Status = EfiGetSystemConfigurationTable ( > &gEfiSmbiosTableGuid, > (VOID **) &SmbiosTable > @@ -485,10 +491,10 @@ MeasureSmbiosTable ( > if (!EFI_ERROR (Status)) { > DEBUG ((EFI_D_INFO, "SmbiosTable:\n")); > DEBUG ((EFI_D_INFO, " AnchorString - '%c%c%c%c'\n", > - Smbios3Table->AnchorString[0], > - Smbios3Table->AnchorString[1], > - Smbios3Table->AnchorString[2], > - Smbios3Table->AnchorString[3] > + SmbiosTable->AnchorString[0], > + SmbiosTable->AnchorString[1], > + SmbiosTable->AnchorString[2], > + SmbiosTable->AnchorString[3] > )); > DEBUG ((EFI_D_INFO, " EntryPointStructureChecksum - 0x%02x\n", > SmbiosTable->EntryPointStructureChecksum)); > DEBUG ((EFI_D_INFO, " EntryPointLength - 0x%02x\n", > SmbiosTable->EntryPointLength)); > @@ -516,6 +522,7 @@ MeasureSmbiosTable ( > DEBUG ((EFI_D_INFO, " NumberOfSmbiosStructures - 0x%04x\n", > SmbiosTable->NumberOfSmbiosStructures)); > DEBUG ((EFI_D_INFO, " SmbiosBcdRevision - 0x%02x\n", > SmbiosTable->SmbiosBcdRevision)); > } > + } > > if (Smbios3Table != NULL) { > SmbiosTableAddress = (VOID *)(UINTN)Smbios3Table->TableAddress; > @@ -528,7 +535,7 @@ MeasureSmbiosTable ( > if (SmbiosTableAddress != NULL) { > DEBUG ((DEBUG_INFO, "The Smbios Table starts at: 0x%x\n", > SmbiosTableAddress)); > DEBUG ((DEBUG_INFO, "The Smbios Table size: 0x%x\n", TableLength)); > - InternalDumpHex ((UINT8 *)(UINTN)SmbiosTableAddress, TableLength); > + DEBUG_CODE (InternalDumpHex ((UINT8 *)(UINTN)SmbiosTableAddress, > TableLength);); > > TableAddress = AllocateCopyPool ((UINTN)TableLength, (VOID > *)(UINTN)SmbiosTableAddress); > if (TableAddress == NULL) { > @@ -539,11 +546,16 @@ MeasureSmbiosTable ( > > DEBUG ((DEBUG_INFO, "The final Smbios Table starts at: 0x%x\n", > TableAddress)); > DEBUG ((DEBUG_INFO, "The final Smbios Table size: 0x%x\n", TableLength)); > - InternalDumpHex (TableAddress, TableLength); > + DEBUG_CODE (InternalDumpHex (TableAddress, TableLength);); > > HandoffTables.NumberOfTables = 1; > - HandoffTables.TableEntry[0].VendorGuid = gEfiSmbiosTableGuid; > - HandoffTables.TableEntry[0].VendorTable = SmbiosTable; > + if (Smbios3Table != NULL) { > + HandoffTables.TableEntry[0].VendorGuid = gEfiSmbios3TableGuid; > + HandoffTables.TableEntry[0].VendorTable = Smbios3Table; > + } else { > + HandoffTables.TableEntry[0].VendorGuid = gEfiSmbiosTableGuid; > + HandoffTables.TableEntry[0].VendorTable = SmbiosTable; > + } > Status = TpmMeasureAndLogData ( > 1, // PCRIndex > EV_EFI_HANDOFF_TABLES, // EventType > @@ -562,13 +574,12 @@ MeasureSmbiosTable ( > > /** > > - Driver to produce Smbios measurement. > + Driver to produce Smbios measurement. > > @param ImageHandle Module's image handle > @param SystemTable Pointer of EFI_SYSTEM_TABLE > > - @retval EFI_SUCCESS Smbios protocol installed > - @retval Other No protocol installed, unload driver. > + @return Status returned from EfiCreateEventReadyToBootEx(). > > **/ > EFI_STATUS > @@ -581,10 +592,10 @@ SmbiosMeasurementDriverEntryPoint ( > EFI_STATUS Status; > EFI_EVENT Event; > > - Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, &mSmbios); > + Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, (VOID **) > &mSmbios); > ASSERT_EFI_ERROR (Status); > DEBUG ((DEBUG_INFO, "The Smbios Table Version: %x.%x\n", > mSmbios->MajorVersion, mSmbios->MinorVersion)); > - > + > if (mSmbios->MajorVersion < 2 || (mSmbios->MajorVersion == 2 && > mSmbios->MinorVersion < 7)){ > mMaxLen = SMBIOS_STRING_MAX_LENGTH; > } else if (mSmbios->MajorVersion < 3) { > diff --git > a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf > b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf > index 6f89447..771259e 100644 > --- a/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf > +++ b/MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf > @@ -55,11 +55,11 @@ [LibraryClasses] > TpmMeasurementLib > > [Protocols] > - gEfiSmbiosProtocolGuid ## PRODUCES > + gEfiSmbiosProtocolGuid ## CONSUMES > > [Guids] > - gEfiSmbiosTableGuid ## SOMETIMES_PRODUCES ## > SystemTable > - gEfiSmbios3TableGuid ## SOMETIMES_PRODUCES ## > SystemTable > + gEfiSmbiosTableGuid ## SOMETIMES_CONSUMES ## > SystemTable > + gEfiSmbios3TableGuid ## SOMETIMES_CONSUMES ## > SystemTable > > [Depex] > gEfiSmbiosProtocolGuid > -- > 1.9.5.msysgit.0 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
