Thanks.
Can we use CopyGuid() for below assignment?
HandoffTables.TableEntry[0].VendorGuid  = gEfiSmbiosTableGuid;

Reviewed-by: jiewen....@intel.com


-----Original Message-----
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star Zeng
Sent: Wednesday, October 28, 2015 9:33 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH] MdeModulePkg SmbiosMeasurementDxe: Smbios3Table used as 
SmbiosTable wrongly

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.

Cc:Jiewen Yao <jiewen....@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.z...@intel.com>
---
 .../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.i
+++ nf
@@ -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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to