public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version
@ 2017-01-10  2:24 Zhang, Chao B
  2017-01-10  2:24 ` [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option Zhang, Chao B
  2017-01-10  5:11 ` [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version Yao, Jiewen
  0 siblings, 2 replies; 6+ messages in thread
From: Zhang, Chao B @ 2017-01-10  2:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: jiewen.yao, star.zeng, Chao Zhang

Update TPM2 ACPI Table revision to 4. New version & data structure is
defined in TCG ACPI Spec 00.37

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
 MdePkg/Include/IndustryStandard/Tpm2Acpi.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/Tpm2Acpi.h b/MdePkg/Include/IndustryStandard/Tpm2Acpi.h
index 73ef561..3500739 100644
--- a/MdePkg/Include/IndustryStandard/Tpm2Acpi.h
+++ b/MdePkg/Include/IndustryStandard/Tpm2Acpi.h
@@ -1,7 +1,7 @@
 /** @file
   TPM2 ACPI table definition.
 
-Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2013 - 2017, 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
@@ -19,11 +19,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #pragma pack (1)
 
-#define EFI_TPM2_ACPI_TABLE_REVISION  3
+#define EFI_TPM2_ACPI_TABLE_REVISION  4
 
 typedef struct {
   EFI_ACPI_DESCRIPTION_HEADER Header;
-  UINT32                      Flags;
+  UINT16                      PlatformClass;
+  UINT16                      Reserved;
   UINT64                      AddressOfControlArea;
   UINT32                      StartMethod;
 //UINT8                       PlatformSpecificParameters[];
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option
  2017-01-10  2:24 [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version Zhang, Chao B
@ 2017-01-10  2:24 ` Zhang, Chao B
  2017-01-10  5:04   ` Zeng, Star
  2017-01-10  5:12   ` Yao, Jiewen
  2017-01-10  5:11 ` [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version Yao, Jiewen
  1 sibling, 2 replies; 6+ messages in thread
From: Zhang, Chao B @ 2017-01-10  2:24 UTC (permalink / raw)
  To: edk2-devel; +Cc: jiewen.yao, star.zeng, Chao Zhang

Add TPM2 ACPI Table Rev Option in Tcg2Config UI. Rev 4 is defined in
TCG ACPI Specification 00.37

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
 SecurityPkg/SecurityPkg.dec                      |  7 ++++++
 SecurityPkg/SecurityPkg.dsc                      |  1 +
 SecurityPkg/SecurityPkg.uni                      |  8 ++++++-
 SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr        | 16 +++++++++++++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c    | 29 ++++++++++++++++++++++++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf     |  1 +
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c      | 22 +++++++++++++++++-
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h    |  7 +++++-
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni | 12 ++++++++++
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c                |  6 ++++-
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf              |  3 ++-
 11 files changed, 107 insertions(+), 5 deletions(-)

diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index feeaf60..0c64d25 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -429,6 +429,13 @@
   # @Prompt A physical presence user status
   gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|FALSE|BOOLEAN|0x00010019
 
+  ## Indicate the TPM2 ACPI table revision. Rev 4 is defined in TCG ACPI Specification Rev 00.37.<BR><BR>
+  # To support configuring from setup page, this PCD can be DynamicHii type and map to a setup option.<BR>
+  # For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by Tcg2ConfigDxe driver.<BR>
+  # gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>
+  # @Prompt Revision of TPM2 ACPI table.
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|3|UINT8|0x0001001A
+
   ## This PCD defines initial setting of TCG2 Persistent Firmware Management Flags
   # PCD can be configured for different settings in different scenarios
   # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 0d39741..dee9241 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -149,6 +149,7 @@
 
 [PcdsDynamicHii.common.DEFAULT]
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
 
 [Components]
   SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
index 815bf0b..17d36c0 100644
--- a/SecurityPkg/SecurityPkg.uni
+++ b/SecurityPkg/SecurityPkg.uni
@@ -227,4 +227,10 @@
 #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_PROMPT  #language en-US " Initial setting of TCG2 Persistent Firmware Management Flags"
 
 #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_HELP  #language en-US "This PCD defines initial setting of TCG2 Persistent Firmware Management Flags\n"
-                                                                                   "PCD can be configured for different settings in different scenarios."
\ No newline at end of file
+
+#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_PROMPT  #language en-US "The revision of TPM2 ACPI table"
+
+#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP  #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
+                                                                                    "To support configuring from setup page, this PCD can be DynamicHii type and map to a setup option.<BR>\n"
+                                                                                    "For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by Tcg2ConfigDxe driver.<BR>\n"
+                                                                                    "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
\ No newline at end of file
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
index a116713..1d44c99 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
@@ -67,6 +67,22 @@ formset
         text   = STRING_TOKEN(STR_TPM2_ACPI_HID_CONTENT);
 
     text
+      help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_HELP),
+      text   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_PROMPT),
+        text   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_CONTENT);
+
+    oneof varid  = TCG2_VERSION.Tpm2AcpiTableRev,
+          questionid = KEY_TPM2_ACPI_REVISION,
+          prompt = STRING_TOKEN(STR_TPM2_ACPI_REVISION_PROMPT),
+          help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_HELP),
+          flags  = INTERACTIVE,
+            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_3),     value = TPM2_ACPI_REVISION_3,     flags = RESET_REQUIRED;
+            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_4),     value = TPM2_ACPI_REVISION_4,     flags = DEFAULT | MANUFACTURING | RESET_REQUIRED;
+    endoneof;
+
+    subtitle text = STRING_TOKEN(STR_NULL);
+
+    text
       help   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_HELP),
       text   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT),
         text   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT);
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
index 050e43a..1b0db4c 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
@@ -82,6 +82,7 @@ InitializeTcg2VersionInfo (
   TCG2_VERSION                  Tcg2Version;
   UINTN                         DataSize;
   UINT64                        PcdTcg2PpiVersion;
+  UINT8                         PcdTpm2AcpiTableRev;
 
   //
   // Get the PCD value before initializing efi varstore configuration data.
@@ -93,6 +94,8 @@ InitializeTcg2VersionInfo (
     AsciiStrSize ((CHAR8 *) PcdGetPtr (PcdTcgPhysicalPresenceInterfaceVer))
     );
 
+  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
+
   //
   // Initialize efi varstore configuration data.
   //
@@ -174,6 +177,9 @@ InitializeTcg2VersionInfo (
       if (PcdTcg2PpiVersion != Tcg2Version.PpiVersion) {
         DEBUG ((DEBUG_WARN, "WARNING: PcdTcgPhysicalPresenceInterfaceVer default value is not same with the default value in VFR\n"));
         DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be chosen\n"));
+      } else if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
+        DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev default value is not same with the default value in VFR\n"));
+        DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be chosen\n"));
       }
     }
   }
@@ -206,6 +212,29 @@ InitializeTcg2VersionInfo (
       ASSERT (FALSE);
       break;
   }
+
+  //
+  // Get the PcdTpm2AcpiTableRev value again.
+  // If the PCD value is not equal to the value in variable,
+  // the PCD is not DynamicHii type and does not map to TCG2_VERSION Variable.
+  //
+  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
+  if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
+    DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev is not DynamicHii type and does not map to TCG2_VERSION.Tpm2AcpiTableRev\n"));
+    DEBUG ((DEBUG_WARN, "WARNING: The Tpm2 ACPI Revision configuring from setup page will not work\n"));
+  }
+
+  switch (PcdTpm2AcpiTableRev) {
+    case TPM2_ACPI_REVISION_3:
+      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 3", NULL);
+      break;
+    case TPM2_ACPI_REVISION_4:
+      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 4", NULL);
+      break;
+    default:
+      ASSERT (FALSE);
+      break;
+  }
 }
 
 /**
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
index 9f21aab..38fa331 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
@@ -78,6 +78,7 @@
   gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap    ## CONSUMES
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress             ## CONSUMES
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ## CONSUMES
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev           ## CONSUMES
 
 [Depex]
   gEfiTcg2ProtocolGuid              AND
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
index f4a07c6..a83000f 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
@@ -481,6 +481,7 @@ Tcg2VersionInfoCallback (
 {
   EFI_INPUT_KEY                 Key;
   UINT64                        PcdTcg2PpiVersion;
+  UINT8                         PcdTpm2AcpiTableRev;
 
   ASSERT (Action == EFI_BROWSER_ACTION_SUBMITTED);
 
@@ -506,6 +507,24 @@ Tcg2VersionInfoCallback (
         NULL
         );
     }
+  } else if (QuestionId == KEY_TPM2_ACPI_REVISION){
+    //
+    // Get the PCD value after EFI_BROWSER_ACTION_SUBMITTED,
+    // the SetVariable to TCG2_VERSION_NAME should have been done.
+    // If the PCD value is not equal to the value set to variable,
+    // the PCD is not DynamicHii type and does not map to the setup option.
+    //
+    PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
+
+    if (PcdTpm2AcpiTableRev != Value->u8) {
+      CreatePopUp (
+        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+        &Key,
+        L"WARNING: PcdTpm2AcpiTableRev is not DynamicHii type and does not map to this option!",
+        L"The Revision configuring by this setup option will not work!",
+        NULL
+        );
+    }
   }
 
   return EFI_SUCCESS;
@@ -607,7 +626,7 @@ Tcg2Callback (
   }
 
   if (Action == EFI_BROWSER_ACTION_SUBMITTED) {
-    if (QuestionId == KEY_TCG2_PPI_VERSION) {
+    if (QuestionId == KEY_TCG2_PPI_VERSION || QuestionId == KEY_TPM2_ACPI_REVISION) {
       return Tcg2VersionInfoCallback (Action, QuestionId, Type, Value);
     }
   }
@@ -971,6 +990,7 @@ InstallTcg2ConfigForm (
   if (EFI_ERROR (Status)) {
     DEBUG ((EFI_D_ERROR, "Tcg2ConfigDriver: Fail to set TCG2_STORAGE_INFO_NAME\n"));
   }
+
   return EFI_SUCCESS;  
 }
 
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
index 7868c21..5960446 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
@@ -29,7 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define EFI_TCG2_EVENT_LOG_FORMAT_ALL           (EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 | EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
 
 #define TCG2_CONFIGURATION_VARSTORE_ID  0x0001
-#define TCG2_CONFIGURATION_INFO_VARSTORE_ID  0x0002
+#define TCG2_CONFIGURATION_INFO_VARSTORE_ID     0x0002
 #define TCG2_VERSION_VARSTORE_ID        0x0003
 #define TCG2_CONFIGURATION_FORM_ID      0x0001
 
@@ -43,6 +43,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define KEY_TPM2_PCR_BANKS_REQUEST_4            0x2007
 #define KEY_TPM_DEVICE_INTERFACE                       0x2008
 #define KEY_TCG2_PPI_VERSION                    0x2009
+#define KEY_TPM2_ACPI_REVISION                  0x200A
 
 #define TPM_DEVICE_NULL           0
 #define TPM_DEVICE_1_2            1
@@ -51,6 +52,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define TPM_DEVICE_MAX            TPM_DEVICE_2_0_DTPM
 #define TPM_DEVICE_DEFAULT        TPM_DEVICE_1_2
 
+#define TPM2_ACPI_REVISION_3       3
+#define TPM2_ACPI_REVISION_4       4
+
 #define TPM_DEVICE_INTERFACE_TIS       0
 #define TPM_DEVICE_INTERFACE_PTP_FIFO  1
 #define TPM_DEVICE_INTERFACE_PTP_CRB   2
@@ -72,6 +76,7 @@ typedef struct {
 
 typedef struct {
   UINT64  PpiVersion;
+  UINT8   Tpm2AcpiTableRev;
 } TCG2_VERSION;
 
 typedef struct {
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
index 414dcec..a7d62bc 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
@@ -38,6 +38,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #string STR_TPM2_ACPI_HID_HELP                    #language en-US "HID from TPM2 ACPI Table: ManufacturerID + FirmwareVersion_1"
 #string STR_TPM2_ACPI_HID_CONTENT                 #language en-US ""
 
+#string STR_TPM2_ACPI_REVISION_STATE_PROMPT   #language en-US "Current Rev of TPM2 ACPI Table"
+#string STR_TPM2_ACPI_REVISION_STATE_HELP     #language en-US "Current Rev of TPM2 ACPI Table: Rev 3 or Rev 4"
+#string STR_TPM2_ACPI_REVISION_STATE_CONTENT  #language en-US ""
+
+#string STR_TPM2_ACPI_REVISION_PROMPT          #language en-US "Attempt Rev of TPM2 ACPI Table"
+#string STR_TPM2_ACPI_REVISION_HELP            #language en-US "Rev 3 or Rev 4 (Rev 4 is defined in TCG ACPI Spec 00.37)"
+                                                               "PcdTpm2AcpiTableRev needs to be DynamicHii type and map to this option\n"
+                                                               "Otherwise the version configuring by this setup option will not work"
+
 #string STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT         #language en-US "Current TPM Device Interface"
 #string STR_TCG2_DEVICE_INTERFACE_STATE_HELP           #language en-US "Current TPM Device Interface: TIS, PTP FIFO, PTP CRB"
 #string STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT        #language en-US ""
@@ -74,6 +83,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #string STR_TCG2_TPM_1_2                   #language en-US "TPM 1.2"
 #string STR_TCG2_TPM_2_0_DTPM              #language en-US "TPM 2.0"
 
+#string STR_TPM2_ACPI_REVISION_3           #language en-US "Rev 3"
+#string STR_TPM2_ACPI_REVISION_4           #language en-US "Rev 4"
+
 #string STR_TCG2_PPI_VERSION_1_2           #language en-US "1.2"
 #string STR_TCG2_PPI_VERSION_1_3           #language en-US "1.3"
 
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index 7557e29..927de15 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -83,7 +83,8 @@ EFI_TPM2_ACPI_TABLE  mTpm2AcpiTemplate = {
     // These fields should be filled in in production
     //
   },
-  0, // Flags
+  0, // 16-bit PlatformClass
+  0, // 16-bit Reserved
   0, // Control Area
   EFI_TPM2_ACPI_TABLE_START_METHOD_TIS, // StartMethod
 };
@@ -508,6 +509,9 @@ PublishTpm2 (
   EFI_TPM2_ACPI_CONTROL_AREA     *ControlArea;
   PTP_INTERFACE_TYPE             InterfaceType;
 
+  mTpm2AcpiTemplate.Header.Revision = PcdGet8(PcdTpm2AcpiTableRev);
+  DEBUG((DEBUG_INFO, "Tpm2 ACPI table revision is %d\n", mTpm2AcpiTemplate.Header.Revision));
+
   //
   // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
   //
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
index 8c823d6..2793242 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
@@ -9,7 +9,7 @@
 #  This driver will have external input - variable and ACPINvs data in SMM mode.
 #  This external input must be validated carefully to avoid security issue.
 #
-# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2017, 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
@@ -73,6 +73,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision  ## SOMETIMES_CONSUMES
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress               ## CONSUMES
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ## CONSUMES
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev                 ## CONSUMES
 
 [Depex]
   gEfiAcpiTableProtocolGuid AND
-- 
1.9.5.msysgit.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option
  2017-01-10  2:24 ` [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option Zhang, Chao B
@ 2017-01-10  5:04   ` Zeng, Star
  2017-01-10  5:12   ` Yao, Jiewen
  1 sibling, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2017-01-10  5:04 UTC (permalink / raw)
  To: Zhang, Chao B, edk2-devel; +Cc: jiewen.yao, star.zeng

Chao,

Add minor comments at below, others are good to me.
Reviewed-by: Star Zeng <star.zeng@intel.com>

On 2017/1/10 10:24, Zhang, Chao B wrote:
> Add TPM2 ACPI Table Rev Option in Tcg2Config UI. Rev 4 is defined in
> TCG ACPI Specification 00.37
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
>  SecurityPkg/SecurityPkg.dec                      |  7 ++++++
>  SecurityPkg/SecurityPkg.dsc                      |  1 +
>  SecurityPkg/SecurityPkg.uni                      |  8 ++++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr        | 16 +++++++++++++
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c    | 29 ++++++++++++++++++++++++
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf     |  1 +
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c      | 22 +++++++++++++++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h    |  7 +++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni | 12 ++++++++++
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c                |  6 ++++-
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf              |  3 ++-
>  11 files changed, 107 insertions(+), 5 deletions(-)
>
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index feeaf60..0c64d25 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -429,6 +429,13 @@
>    # @Prompt A physical presence user status
>    gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|FALSE|BOOLEAN|0x00010019
>
> +  ## Indicate the TPM2 ACPI table revision. Rev 4 is defined in TCG ACPI Specification Rev 00.37.<BR><BR>
> +  # To support configuring from setup page, this PCD can be DynamicHii type and map to a setup option.<BR>
> +  # For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by Tcg2ConfigDxe driver.<BR>
> +  # gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>
> +  # @Prompt Revision of TPM2 ACPI table.
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|3|UINT8|0x0001001A
> +
>    ## This PCD defines initial setting of TCG2 Persistent Firmware Management Flags
>    # PCD can be configured for different settings in different scenarios
>    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT | TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
> index 0d39741..dee9241 100644
> --- a/SecurityPkg/SecurityPkg.dsc
> +++ b/SecurityPkg/SecurityPkg.dsc
> @@ -149,6 +149,7 @@
>
>  [PcdsDynamicHii.common.DEFAULT]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
>
>  [Components]
>    SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index 815bf0b..17d36c0 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -227,4 +227,10 @@
>  #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_PROMPT  #language en-US " Initial setting of TCG2 Persistent Firmware Management Flags"
>
>  #string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_HELP  #language en-US "This PCD defines initial setting of TCG2 Persistent Firmware Management Flags\n"
> -                                                                                   "PCD can be configured for different settings in different scenarios."
> \ No newline at end of file
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_PROMPT  #language en-US "The revision of TPM2 ACPI table"
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP  #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
> +                                                                                    "To support configuring from setup page, this PCD can be DynamicHii type and map to a setup option.<BR>\n"
> +                                                                                    "For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by Tcg2ConfigDxe driver.<BR>\n"
> +                                                                                    "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
> \ No newline at end of file
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> index a116713..1d44c99 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> @@ -67,6 +67,22 @@ formset
>          text   = STRING_TOKEN(STR_TPM2_ACPI_HID_CONTENT);
>
>      text
> +      help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_HELP),
> +      text   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_PROMPT),
> +        text   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_CONTENT);
> +
> +    oneof varid  = TCG2_VERSION.Tpm2AcpiTableRev,
> +          questionid = KEY_TPM2_ACPI_REVISION,
> +          prompt = STRING_TOKEN(STR_TPM2_ACPI_REVISION_PROMPT),
> +          help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_HELP),
> +          flags  = INTERACTIVE,
> +            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_3),     value = TPM2_ACPI_REVISION_3,     flags = RESET_REQUIRED;
> +            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_4),     value = TPM2_ACPI_REVISION_4,     flags = DEFAULT | MANUFACTURING | RESET_REQUIRED;
> +    endoneof;
> +
> +    subtitle text = STRING_TOKEN(STR_NULL);
> +
> +    text
>        help   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_HELP),
>        text   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT),
>          text   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT);
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> index 050e43a..1b0db4c 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> @@ -82,6 +82,7 @@ InitializeTcg2VersionInfo (
>    TCG2_VERSION                  Tcg2Version;
>    UINTN                         DataSize;
>    UINT64                        PcdTcg2PpiVersion;
> +  UINT8                         PcdTpm2AcpiTableRev;
>
>    //
>    // Get the PCD value before initializing efi varstore configuration data.
> @@ -93,6 +94,8 @@ InitializeTcg2VersionInfo (
>      AsciiStrSize ((CHAR8 *) PcdGetPtr (PcdTcgPhysicalPresenceInterfaceVer))
>      );
>
> +  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +
>    //
>    // Initialize efi varstore configuration data.
>    //
> @@ -174,6 +177,9 @@ InitializeTcg2VersionInfo (
>        if (PcdTcg2PpiVersion != Tcg2Version.PpiVersion) {
>          DEBUG ((DEBUG_WARN, "WARNING: PcdTcgPhysicalPresenceInterfaceVer default value is not same with the default value in VFR\n"));
>          DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be chosen\n"));
> +      } else if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
> +        DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev default value is not same with the default value in VFR\n"));
> +        DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be chosen\n"));

The two if conditions can happen at same boot. Could we just use 
separated if instead of the "else if"?

>        }
>      }
>    }
> @@ -206,6 +212,29 @@ InitializeTcg2VersionInfo (
>        ASSERT (FALSE);
>        break;
>    }
> +
> +  //
> +  // Get the PcdTpm2AcpiTableRev value again.
> +  // If the PCD value is not equal to the value in variable,
> +  // the PCD is not DynamicHii type and does not map to TCG2_VERSION Variable.
> +  //
> +  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +  if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
> +    DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev is not DynamicHii type and does not map to TCG2_VERSION.Tpm2AcpiTableRev\n"));
> +    DEBUG ((DEBUG_WARN, "WARNING: The Tpm2 ACPI Revision configuring from setup page will not work\n"));
> +  }
> +
> +  switch (PcdTpm2AcpiTableRev) {
> +    case TPM2_ACPI_REVISION_3:
> +      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 3", NULL);
> +      break;
> +    case TPM2_ACPI_REVISION_4:
> +      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 4", NULL);
> +      break;
> +    default:
> +      ASSERT (FALSE);
> +      break;
> +  }
>  }
>
>  /**
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> index 9f21aab..38fa331 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> @@ -78,6 +78,7 @@
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap    ## CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress             ## CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ## CONSUMES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev           ## CONSUMES
>
>  [Depex]
>    gEfiTcg2ProtocolGuid              AND
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> index f4a07c6..a83000f 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> @@ -481,6 +481,7 @@ Tcg2VersionInfoCallback (
>  {
>    EFI_INPUT_KEY                 Key;
>    UINT64                        PcdTcg2PpiVersion;
> +  UINT8                         PcdTpm2AcpiTableRev;
>
>    ASSERT (Action == EFI_BROWSER_ACTION_SUBMITTED);
>
> @@ -506,6 +507,24 @@ Tcg2VersionInfoCallback (
>          NULL
>          );
>      }
> +  } else if (QuestionId == KEY_TPM2_ACPI_REVISION){
> +    //
> +    // Get the PCD value after EFI_BROWSER_ACTION_SUBMITTED,
> +    // the SetVariable to TCG2_VERSION_NAME should have been done.
> +    // If the PCD value is not equal to the value set to variable,
> +    // the PCD is not DynamicHii type and does not map to the setup option.
> +    //
> +    PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +
> +    if (PcdTpm2AcpiTableRev != Value->u8) {
> +      CreatePopUp (
> +        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
> +        &Key,
> +        L"WARNING: PcdTpm2AcpiTableRev is not DynamicHii type and does not map to this option!",
> +        L"The Revision configuring by this setup option will not work!",
> +        NULL
> +        );
> +    }
>    }
>
>    return EFI_SUCCESS;
> @@ -607,7 +626,7 @@ Tcg2Callback (
>    }
>
>    if (Action == EFI_BROWSER_ACTION_SUBMITTED) {
> -    if (QuestionId == KEY_TCG2_PPI_VERSION) {
> +    if (QuestionId == KEY_TCG2_PPI_VERSION || QuestionId == KEY_TPM2_ACPI_REVISION) {
>        return Tcg2VersionInfoCallback (Action, QuestionId, Type, Value);
>      }
>    }
> @@ -971,6 +990,7 @@ InstallTcg2ConfigForm (
>    if (EFI_ERROR (Status)) {
>      DEBUG ((EFI_D_ERROR, "Tcg2ConfigDriver: Fail to set TCG2_STORAGE_INFO_NAME\n"));
>    }
> +
>    return EFI_SUCCESS;
>  }
>
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> index 7868c21..5960446 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> @@ -29,7 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #define EFI_TCG2_EVENT_LOG_FORMAT_ALL           (EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 | EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
>
>  #define TCG2_CONFIGURATION_VARSTORE_ID  0x0001
> -#define TCG2_CONFIGURATION_INFO_VARSTORE_ID  0x0002
> +#define TCG2_CONFIGURATION_INFO_VARSTORE_ID     0x0002
>  #define TCG2_VERSION_VARSTORE_ID        0x0003
>  #define TCG2_CONFIGURATION_FORM_ID      0x0001
>
> @@ -43,6 +43,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #define KEY_TPM2_PCR_BANKS_REQUEST_4            0x2007
>  #define KEY_TPM_DEVICE_INTERFACE                       0x2008
>  #define KEY_TCG2_PPI_VERSION                    0x2009
> +#define KEY_TPM2_ACPI_REVISION                  0x200A
>
>  #define TPM_DEVICE_NULL           0
>  #define TPM_DEVICE_1_2            1
> @@ -51,6 +52,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #define TPM_DEVICE_MAX            TPM_DEVICE_2_0_DTPM
>  #define TPM_DEVICE_DEFAULT        TPM_DEVICE_1_2
>
> +#define TPM2_ACPI_REVISION_3       3
> +#define TPM2_ACPI_REVISION_4       4
> +
>  #define TPM_DEVICE_INTERFACE_TIS       0
>  #define TPM_DEVICE_INTERFACE_PTP_FIFO  1
>  #define TPM_DEVICE_INTERFACE_PTP_CRB   2
> @@ -72,6 +76,7 @@ typedef struct {
>
>  typedef struct {
>    UINT64  PpiVersion;
> +  UINT8   Tpm2AcpiTableRev;
>  } TCG2_VERSION;
>
>  typedef struct {
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> index 414dcec..a7d62bc 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> @@ -38,6 +38,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #string STR_TPM2_ACPI_HID_HELP                    #language en-US "HID from TPM2 ACPI Table: ManufacturerID + FirmwareVersion_1"
>  #string STR_TPM2_ACPI_HID_CONTENT                 #language en-US ""
>
> +#string STR_TPM2_ACPI_REVISION_STATE_PROMPT   #language en-US "Current Rev of TPM2 ACPI Table"
> +#string STR_TPM2_ACPI_REVISION_STATE_HELP     #language en-US "Current Rev of TPM2 ACPI Table: Rev 3 or Rev 4"
> +#string STR_TPM2_ACPI_REVISION_STATE_CONTENT  #language en-US ""
> +
> +#string STR_TPM2_ACPI_REVISION_PROMPT          #language en-US "Attempt Rev of TPM2 ACPI Table"
> +#string STR_TPM2_ACPI_REVISION_HELP            #language en-US "Rev 3 or Rev 4 (Rev 4 is defined in TCG ACPI Spec 00.37)"
> +                                                               "PcdTpm2AcpiTableRev needs to be DynamicHii type and map to this option\n"
> +                                                               "Otherwise the version configuring by this setup option will not work"

I see you are using revision in other places, could we also use revision 
instead version here?

Thanks,
Star

> +
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT         #language en-US "Current TPM Device Interface"
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_HELP           #language en-US "Current TPM Device Interface: TIS, PTP FIFO, PTP CRB"
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT        #language en-US ""
> @@ -74,6 +83,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #string STR_TCG2_TPM_1_2                   #language en-US "TPM 1.2"
>  #string STR_TCG2_TPM_2_0_DTPM              #language en-US "TPM 2.0"
>
> +#string STR_TPM2_ACPI_REVISION_3           #language en-US "Rev 3"
> +#string STR_TPM2_ACPI_REVISION_4           #language en-US "Rev 4"
> +
>  #string STR_TCG2_PPI_VERSION_1_2           #language en-US "1.2"
>  #string STR_TCG2_PPI_VERSION_1_3           #language en-US "1.3"
>
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index 7557e29..927de15 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -83,7 +83,8 @@ EFI_TPM2_ACPI_TABLE  mTpm2AcpiTemplate = {
>      // These fields should be filled in in production
>      //
>    },
> -  0, // Flags
> +  0, // 16-bit PlatformClass
> +  0, // 16-bit Reserved
>    0, // Control Area
>    EFI_TPM2_ACPI_TABLE_START_METHOD_TIS, // StartMethod
>  };
> @@ -508,6 +509,9 @@ PublishTpm2 (
>    EFI_TPM2_ACPI_CONTROL_AREA     *ControlArea;
>    PTP_INTERFACE_TYPE             InterfaceType;
>
> +  mTpm2AcpiTemplate.Header.Revision = PcdGet8(PcdTpm2AcpiTableRev);
> +  DEBUG((DEBUG_INFO, "Tpm2 ACPI table revision is %d\n", mTpm2AcpiTemplate.Header.Revision));
> +
>    //
>    // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
>    //
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> index 8c823d6..2793242 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> @@ -9,7 +9,7 @@
>  #  This driver will have external input - variable and ACPINvs data in SMM mode.
>  #  This external input must be validated carefully to avoid security issue.
>  #
> -# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2017, 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
> @@ -73,6 +73,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision  ## SOMETIMES_CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress               ## CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ## CONSUMES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev                 ## CONSUMES
>
>  [Depex]
>    gEfiAcpiTableProtocolGuid AND
>



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version
  2017-01-10  2:24 [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version Zhang, Chao B
  2017-01-10  2:24 ` [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option Zhang, Chao B
@ 2017-01-10  5:11 ` Yao, Jiewen
  1 sibling, 0 replies; 6+ messages in thread
From: Yao, Jiewen @ 2017-01-10  5:11 UTC (permalink / raw)
  To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Zhang, Chao B, Zeng, Star

Hi
Can we keep the old definition?

-#define EFI_TPM2_ACPI_TABLE_REVISION  3
+#define EFI_TPM2_ACPI_TABLE_REVISION  4

E.g.
#define EFI_TPM2_ACPI_TABLE_REVISION_3  3
#define EFI_TPM2_ACPI_TABLE_REVISION_4  4
#define EFI_TPM2_ACPI_TABLE_REVISION    EFI_TPM2_ACPI_TABLE_REVISION_4

So that it can be used in other code.

Thank you
Yao Jiewen


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Zhang,
> Chao B
> Sent: Tuesday, January 10, 2017 10:25 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table
> version
> 
> Update TPM2 ACPI Table revision to 4. New version & data structure is
> defined in TCG ACPI Spec 00.37
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
>  MdePkg/Include/IndustryStandard/Tpm2Acpi.h | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Include/IndustryStandard/Tpm2Acpi.h
> b/MdePkg/Include/IndustryStandard/Tpm2Acpi.h
> index 73ef561..3500739 100644
> --- a/MdePkg/Include/IndustryStandard/Tpm2Acpi.h
> +++ b/MdePkg/Include/IndustryStandard/Tpm2Acpi.h
> @@ -1,7 +1,7 @@
>  /** @file
>    TPM2 ACPI table definition.
> 
> -Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2017, 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
> @@ -19,11 +19,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #pragma pack (1)
> 
> -#define EFI_TPM2_ACPI_TABLE_REVISION  3
> +#define EFI_TPM2_ACPI_TABLE_REVISION  4
> 
>  typedef struct {
>    EFI_ACPI_DESCRIPTION_HEADER Header;
> -  UINT32                      Flags;
> +  UINT16                      PlatformClass;
> +  UINT16                      Reserved;
>    UINT64                      AddressOfControlArea;
>    UINT32                      StartMethod;
>  //UINT8                       PlatformSpecificParameters[];
> --
> 1.9.5.msysgit.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option
  2017-01-10  2:24 ` [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option Zhang, Chao B
  2017-01-10  5:04   ` Zeng, Star
@ 2017-01-10  5:12   ` Yao, Jiewen
  2017-01-10  5:21     ` Zhang, Chao B
  1 sibling, 1 reply; 6+ messages in thread
From: Yao, Jiewen @ 2017-01-10  5:12 UTC (permalink / raw)
  To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Zeng, Star

Hi
Can we use the definition in Acpi table?
I do not think we need redefine them here.

+#define TPM2_ACPI_REVISION_3       3
+#define TPM2_ACPI_REVISION_4       4

Thank you
Yao Jiewen

> -----Original Message-----
> From: Zhang, Chao B
> Sent: Tuesday, January 10, 2017 10:25 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Zhang, Chao B <chao.b.zhang@intel.com>
> Subject: [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option
>
> Add TPM2 ACPI Table Rev Option in Tcg2Config UI. Rev 4 is defined in
> TCG ACPI Specification 00.37
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
>  SecurityPkg/SecurityPkg.dec                      |  7 ++++++
>  SecurityPkg/SecurityPkg.dsc                      |  1 +
>  SecurityPkg/SecurityPkg.uni                      |  8 ++++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr        | 16 +++++++++++++
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c    | 29
> ++++++++++++++++++++++++
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf     |  1 +
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c      | 22 +++++++++++++++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h    |  7 +++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni | 12 ++++++++++
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c                |  6 ++++-
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf              |  3 ++-
>  11 files changed, 107 insertions(+), 5 deletions(-)
>
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index feeaf60..0c64d25 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -429,6 +429,13 @@
>    # @Prompt A physical presence user status
>
> gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|FALSE|BOOLEAN|0x0
> 0010019
>
> +  ## Indicate the TPM2 ACPI table revision. Rev 4 is defined in TCG ACPI
> Specification Rev 00.37.<BR><BR>
> +  # To support configuring from setup page, this PCD can be DynamicHii type
> and map to a setup option.<BR>
> +  # For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured
> by Tcg2ConfigDxe driver.<BR>
> +  #
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2
> ConfigFormSetGuid|0x8|3|NV,BS<BR>
> +  # @Prompt Revision of TPM2 ACPI table.
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|3|UINT8|0x0001001A
> +
>    ## This PCD defines initial setting of TCG2 Persistent Firmware Management
> Flags
>    # PCD can be configured for different settings in different scenarios
>    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
> index 0d39741..dee9241 100644
> --- a/SecurityPkg/SecurityPkg.dsc
> +++ b/SecurityPkg/SecurityPkg.dsc
> @@ -149,6 +149,7 @@
>
>  [PcdsDynamicHii.common.DEFAULT]
>
> gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_V
> ERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> +
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2
> ConfigFormSetGuid|0x8|3|NV,BS
>
>  [Components]
>    SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index 815bf0b..17d36c0 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -227,4 +227,10 @@
>  #string
> STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_PROMPT
> #language en-US " Initial setting of TCG2 Persistent Firmware Management
> Flags"
>
>  #string
> STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_HELP
> #language en-US "This PCD defines initial setting of TCG2 Persistent Firmware
> Management Flags\n"
> -
> "PCD can be configured for different settings in different scenarios."
> \ No newline at end of file
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_PROMPT
> #language en-US "The revision of TPM2 ACPI table"
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP
> #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
> +
> "To support configuring from setup page, this PCD can be DynamicHii type and
> map to a setup option.<BR>\n"
> +
> "For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by
> Tcg2ConfigDxe driver.<BR>\n"
> +
> "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gT
> cg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
> \ No newline at end of file
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> index a116713..1d44c99 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> @@ -67,6 +67,22 @@ formset
>          text   = STRING_TOKEN(STR_TPM2_ACPI_HID_CONTENT);
>
>      text
> +      help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_HELP),
> +      text   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_PROMPT),
> +        text   =
> STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_CONTENT);
> +
> +    oneof varid  = TCG2_VERSION.Tpm2AcpiTableRev,
> +          questionid = KEY_TPM2_ACPI_REVISION,
> +          prompt = STRING_TOKEN(STR_TPM2_ACPI_REVISION_PROMPT),
> +          help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_HELP),
> +          flags  = INTERACTIVE,
> +            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_3),
> value = TPM2_ACPI_REVISION_3,     flags = RESET_REQUIRED;
> +            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_4),
> value = TPM2_ACPI_REVISION_4,     flags = DEFAULT | MANUFACTURING |
> RESET_REQUIRED;
> +    endoneof;
> +
> +    subtitle text = STRING_TOKEN(STR_NULL);
> +
> +    text
>        help   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_HELP),
>        text   =
> STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT),
>          text   =
> STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT);
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> index 050e43a..1b0db4c 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> @@ -82,6 +82,7 @@ InitializeTcg2VersionInfo (
>    TCG2_VERSION                  Tcg2Version;
>    UINTN                         DataSize;
>    UINT64                        PcdTcg2PpiVersion;
> +  UINT8                         PcdTpm2AcpiTableRev;
>
>    //
>    // Get the PCD value before initializing efi varstore configuration data.
> @@ -93,6 +94,8 @@ InitializeTcg2VersionInfo (
>      AsciiStrSize ((CHAR8 *) PcdGetPtr (PcdTcgPhysicalPresenceInterfaceVer))
>      );
>
> +  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +
>    //
>    // Initialize efi varstore configuration data.
>    //
> @@ -174,6 +177,9 @@ InitializeTcg2VersionInfo (
>        if (PcdTcg2PpiVersion != Tcg2Version.PpiVersion) {
>          DEBUG ((DEBUG_WARN, "WARNING:
> PcdTcgPhysicalPresenceInterfaceVer default value is not same with the default
> value in VFR\n"));
>          DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be
> chosen\n"));
> +      } else if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
> +        DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev default
> value is not same with the default value in VFR\n"));
> +        DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be
> chosen\n"));
>        }
>      }
>    }
> @@ -206,6 +212,29 @@ InitializeTcg2VersionInfo (
>        ASSERT (FALSE);
>        break;
>    }
> +
> +  //
> +  // Get the PcdTpm2AcpiTableRev value again.
> +  // If the PCD value is not equal to the value in variable,
> +  // the PCD is not DynamicHii type and does not map to TCG2_VERSION
> Variable.
> +  //
> +  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +  if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
> +    DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev is not
> DynamicHii type and does not map to TCG2_VERSION.Tpm2AcpiTableRev\n"));
> +    DEBUG ((DEBUG_WARN, "WARNING: The Tpm2 ACPI Revision configuring
> from setup page will not work\n"));
> +  }
> +
> +  switch (PcdTpm2AcpiTableRev) {
> +    case TPM2_ACPI_REVISION_3:
> +      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN
> (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 3", NULL);
> +      break;
> +    case TPM2_ACPI_REVISION_4:
> +      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN
> (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 4", NULL);
> +      break;
> +    default:
> +      ASSERT (FALSE);
> +      break;
> +  }
>  }
>
>  /**
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> index 9f21aab..38fa331 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> @@ -78,6 +78,7 @@
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap    ##
> CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress             ##
> CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ##
> CONSUMES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev           ##
> CONSUMES
>
>  [Depex]
>    gEfiTcg2ProtocolGuid              AND
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> index f4a07c6..a83000f 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> @@ -481,6 +481,7 @@ Tcg2VersionInfoCallback (
>  {
>    EFI_INPUT_KEY                 Key;
>    UINT64                        PcdTcg2PpiVersion;
> +  UINT8                         PcdTpm2AcpiTableRev;
>
>    ASSERT (Action == EFI_BROWSER_ACTION_SUBMITTED);
>
> @@ -506,6 +507,24 @@ Tcg2VersionInfoCallback (
>          NULL
>          );
>      }
> +  } else if (QuestionId == KEY_TPM2_ACPI_REVISION){
> +    //
> +    // Get the PCD value after EFI_BROWSER_ACTION_SUBMITTED,
> +    // the SetVariable to TCG2_VERSION_NAME should have been done.
> +    // If the PCD value is not equal to the value set to variable,
> +    // the PCD is not DynamicHii type and does not map to the setup option.
> +    //
> +    PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +
> +    if (PcdTpm2AcpiTableRev != Value->u8) {
> +      CreatePopUp (
> +        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
> +        &Key,
> +        L"WARNING: PcdTpm2AcpiTableRev is not DynamicHii type and does
> not map to this option!",
> +        L"The Revision configuring by this setup option will not work!",
> +        NULL
> +        );
> +    }
>    }
>
>    return EFI_SUCCESS;
> @@ -607,7 +626,7 @@ Tcg2Callback (
>    }
>
>    if (Action == EFI_BROWSER_ACTION_SUBMITTED) {
> -    if (QuestionId == KEY_TCG2_PPI_VERSION) {
> +    if (QuestionId == KEY_TCG2_PPI_VERSION || QuestionId ==
> KEY_TPM2_ACPI_REVISION) {
>        return Tcg2VersionInfoCallback (Action, QuestionId, Type, Value);
>      }
>    }
> @@ -971,6 +990,7 @@ InstallTcg2ConfigForm (
>    if (EFI_ERROR (Status)) {
>      DEBUG ((EFI_D_ERROR, "Tcg2ConfigDriver: Fail to set
> TCG2_STORAGE_INFO_NAME\n"));
>    }
> +
>    return EFI_SUCCESS;
>  }
>
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> index 7868c21..5960446 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> @@ -29,7 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #define EFI_TCG2_EVENT_LOG_FORMAT_ALL
> (EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 |
> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
>
>  #define TCG2_CONFIGURATION_VARSTORE_ID  0x0001
> -#define TCG2_CONFIGURATION_INFO_VARSTORE_ID  0x0002
> +#define TCG2_CONFIGURATION_INFO_VARSTORE_ID     0x0002
>  #define TCG2_VERSION_VARSTORE_ID        0x0003
>  #define TCG2_CONFIGURATION_FORM_ID      0x0001
>
> @@ -43,6 +43,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #define KEY_TPM2_PCR_BANKS_REQUEST_4            0x2007
>  #define KEY_TPM_DEVICE_INTERFACE                       0x2008
>  #define KEY_TCG2_PPI_VERSION                    0x2009
> +#define KEY_TPM2_ACPI_REVISION                  0x200A
>
>  #define TPM_DEVICE_NULL           0
>  #define TPM_DEVICE_1_2            1
> @@ -51,6 +52,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #define TPM_DEVICE_MAX            TPM_DEVICE_2_0_DTPM
>  #define TPM_DEVICE_DEFAULT        TPM_DEVICE_1_2
>
> +#define TPM2_ACPI_REVISION_3       3
> +#define TPM2_ACPI_REVISION_4       4
> +
>  #define TPM_DEVICE_INTERFACE_TIS       0
>  #define TPM_DEVICE_INTERFACE_PTP_FIFO  1
>  #define TPM_DEVICE_INTERFACE_PTP_CRB   2
> @@ -72,6 +76,7 @@ typedef struct {
>
>  typedef struct {
>    UINT64  PpiVersion;
> +  UINT8   Tpm2AcpiTableRev;
>  } TCG2_VERSION;
>
>  typedef struct {
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> index 414dcec..a7d62bc 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> @@ -38,6 +38,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #string STR_TPM2_ACPI_HID_HELP                    #language en-US
> "HID from TPM2 ACPI Table: ManufacturerID + FirmwareVersion_1"
>  #string STR_TPM2_ACPI_HID_CONTENT                 #language en-US ""
>
> +#string STR_TPM2_ACPI_REVISION_STATE_PROMPT   #language en-US
> "Current Rev of TPM2 ACPI Table"
> +#string STR_TPM2_ACPI_REVISION_STATE_HELP     #language en-US
> "Current Rev of TPM2 ACPI Table: Rev 3 or Rev 4"
> +#string STR_TPM2_ACPI_REVISION_STATE_CONTENT  #language en-US ""
> +
> +#string STR_TPM2_ACPI_REVISION_PROMPT          #language en-US
> "Attempt Rev of TPM2 ACPI Table"
> +#string STR_TPM2_ACPI_REVISION_HELP            #language en-US "Rev 3
> or Rev 4 (Rev 4 is defined in TCG ACPI Spec 00.37)"
> +
> "PcdTpm2AcpiTableRev needs to be DynamicHii type and map to this option\n"
> +
> "Otherwise the version configuring by this setup option will not work"
> +
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT         #language
> en-US "Current TPM Device Interface"
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_HELP           #language
> en-US "Current TPM Device Interface: TIS, PTP FIFO, PTP CRB"
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT        #language
> en-US ""
> @@ -74,6 +83,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #string STR_TCG2_TPM_1_2                   #language en-US "TPM 1.2"
>  #string STR_TCG2_TPM_2_0_DTPM              #language en-US "TPM 2.0"
>
> +#string STR_TPM2_ACPI_REVISION_3           #language en-US "Rev 3"
> +#string STR_TPM2_ACPI_REVISION_4           #language en-US "Rev 4"
> +
>  #string STR_TCG2_PPI_VERSION_1_2           #language en-US "1.2"
>  #string STR_TCG2_PPI_VERSION_1_3           #language en-US "1.3"
>
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index 7557e29..927de15 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -83,7 +83,8 @@ EFI_TPM2_ACPI_TABLE  mTpm2AcpiTemplate = {
>      // These fields should be filled in in production
>      //
>    },
> -  0, // Flags
> +  0, // 16-bit PlatformClass
> +  0, // 16-bit Reserved
>    0, // Control Area
>    EFI_TPM2_ACPI_TABLE_START_METHOD_TIS, // StartMethod
>  };
> @@ -508,6 +509,9 @@ PublishTpm2 (
>    EFI_TPM2_ACPI_CONTROL_AREA     *ControlArea;
>    PTP_INTERFACE_TYPE             InterfaceType;
>
> +  mTpm2AcpiTemplate.Header.Revision = PcdGet8(PcdTpm2AcpiTableRev);
> +  DEBUG((DEBUG_INFO, "Tpm2 ACPI table revision is %d\n",
> mTpm2AcpiTemplate.Header.Revision));
> +
>    //
>    // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
>    //
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> index 8c823d6..2793242 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> @@ -9,7 +9,7 @@
>  #  This driver will have external input - variable and ACPINvs data in SMM
> mode.
>  #  This external input must be validated carefully to avoid security issue.
>  #
> -# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2017, 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
> @@ -73,6 +73,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision  ##
> SOMETIMES_CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress               ##
> CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ##
> CONSUMES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev                 ##
> CONSUMES
>
>  [Depex]
>    gEfiAcpiTableProtocolGuid AND
> --
> 1.9.5.msysgit.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option
  2017-01-10  5:12   ` Yao, Jiewen
@ 2017-01-10  5:21     ` Zhang, Chao B
  0 siblings, 0 replies; 6+ messages in thread
From: Zhang, Chao B @ 2017-01-10  5:21 UTC (permalink / raw)
  To: Yao, Jiewen, edk2-devel@lists.01.org; +Cc: Zeng, Star

Thanks for your comments. I will follow up to fix these 2 issues.

From: Yao, Jiewen
Sent: Tuesday, January 10, 2017 1:12 PM
To: Zhang, Chao B <chao.b.zhang@intel.com>; edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option


Hi
Can we use the definition in Acpi table?
I do not think we need redefine them here.

+#define TPM2_ACPI_REVISION_3       3
+#define TPM2_ACPI_REVISION_4       4
Thank you
Yao Jiewen

> -----Original Message-----
> From: Zhang, Chao B
> Sent: Tuesday, January 10, 2017 10:25 AM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>;
> Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> Subject: [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option
>
> Add TPM2 ACPI Table Rev Option in Tcg2Config UI. Rev 4 is defined in
> TCG ACPI Specification 00.37
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> ---
>  SecurityPkg/SecurityPkg.dec                      |  7 ++++++
>  SecurityPkg/SecurityPkg.dsc                      |  1 +
>  SecurityPkg/SecurityPkg.uni                      |  8 ++++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr        | 16 +++++++++++++
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c    | 29
> ++++++++++++++++++++++++
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf     |  1 +
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c      | 22 +++++++++++++++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h    |  7 +++++-
>  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni | 12 ++++++++++
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c                |  6 ++++-
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf              |  3 ++-
>  11 files changed, 107 insertions(+), 5 deletions(-)
>
> diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
> index feeaf60..0c64d25 100644
> --- a/SecurityPkg/SecurityPkg.dec
> +++ b/SecurityPkg/SecurityPkg.dec
> @@ -429,6 +429,13 @@
>    # @Prompt A physical presence user status
>
> gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|FALSE|BOOLEAN|0x0
> 0010019
>
> +  ## Indicate the TPM2 ACPI table revision. Rev 4 is defined in TCG ACPI
> Specification Rev 00.37.<BR><BR>
> +  # To support configuring from setup page, this PCD can be DynamicHii type
> and map to a setup option.<BR>
> +  # For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured
> by Tcg2ConfigDxe driver.<BR>
> +  #
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2
> ConfigFormSetGuid|0x8|3|NV,BS<BR>
> +  # @Prompt Revision of TPM2 ACPI table.
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|3|UINT8|0x0001001A
> +
>    ## This PCD defines initial setting of TCG2 Persistent Firmware Management
> Flags
>    # PCD can be configured for different settings in different scenarios
>    # Default setting is TCG2_BIOS_TPM_MANAGEMENT_FLAG_DEFAULT |
> TCG2_BIOS_STORAGE_MANAGEMENT_FLAG_DEFAULT
> diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
> index 0d39741..dee9241 100644
> --- a/SecurityPkg/SecurityPkg.dsc
> +++ b/SecurityPkg/SecurityPkg.dsc
> @@ -149,6 +149,7 @@
>
>  [PcdsDynamicHii.common.DEFAULT]
>
> gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_V
> ERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> +
> gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2
> ConfigFormSetGuid|0x8|3|NV,BS
>
>  [Components]
>    SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> diff --git a/SecurityPkg/SecurityPkg.uni b/SecurityPkg/SecurityPkg.uni
> index 815bf0b..17d36c0 100644
> --- a/SecurityPkg/SecurityPkg.uni
> +++ b/SecurityPkg/SecurityPkg.uni
> @@ -227,4 +227,10 @@
>  #string
> STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_PROMPT
> #language en-US " Initial setting of TCG2 Persistent Firmware Management
> Flags"
>
>  #string
> STR_gEfiSecurityPkgTokenSpaceGuid_PcdTcg2PhysicalPresenceFlags_HELP
> #language en-US "This PCD defines initial setting of TCG2 Persistent Firmware
> Management Flags\n"
> -
> "PCD can be configured for different settings in different scenarios."
> \ No newline at end of file
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_PROMPT
> #language en-US "The revision of TPM2 ACPI table"
> +
> +#string STR_gEfiSecurityPkgTokenSpaceGuid_PcdTpm2AcpiTableRev_HELP
> #language en-US "This PCD defines initial revision of TPM2 ACPI table\n"
> +
> "To support configuring from setup page, this PCD can be DynamicHii type and
> map to a setup option.<BR>\n"
> +
> "For example, map to TCG2_VERSION.Tpm2AcpiTableRev to be configured by
> Tcg2ConfigDxe driver.<BR>\n"
> +
> "gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L\"TCG2_VERSION\"|gT
> cg2ConfigFormSetGuid|0x8|3|NV,BS<BR>"
> \ No newline at end of file
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> index a116713..1d44c99 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr
> @@ -67,6 +67,22 @@ formset
>          text   = STRING_TOKEN(STR_TPM2_ACPI_HID_CONTENT);
>
>      text
> +      help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_HELP),
> +      text   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_PROMPT),
> +        text   =
> STRING_TOKEN(STR_TPM2_ACPI_REVISION_STATE_CONTENT);
> +
> +    oneof varid  = TCG2_VERSION.Tpm2AcpiTableRev,
> +          questionid = KEY_TPM2_ACPI_REVISION,
> +          prompt = STRING_TOKEN(STR_TPM2_ACPI_REVISION_PROMPT),
> +          help   = STRING_TOKEN(STR_TPM2_ACPI_REVISION_HELP),
> +          flags  = INTERACTIVE,
> +            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_3),
> value = TPM2_ACPI_REVISION_3,     flags = RESET_REQUIRED;
> +            option text = STRING_TOKEN(STR_TPM2_ACPI_REVISION_4),
> value = TPM2_ACPI_REVISION_4,     flags = DEFAULT | MANUFACTURING |
> RESET_REQUIRED;
> +    endoneof;
> +
> +    subtitle text = STRING_TOKEN(STR_NULL);
> +
> +    text
>        help   = STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_HELP),
>        text   =
> STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT),
>          text   =
> STRING_TOKEN(STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT);
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> index 050e43a..1b0db4c 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDriver.c
> @@ -82,6 +82,7 @@ InitializeTcg2VersionInfo (
>    TCG2_VERSION                  Tcg2Version;
>    UINTN                         DataSize;
>    UINT64                        PcdTcg2PpiVersion;
> +  UINT8                         PcdTpm2AcpiTableRev;
>
>    //
>    // Get the PCD value before initializing efi varstore configuration data.
> @@ -93,6 +94,8 @@ InitializeTcg2VersionInfo (
>      AsciiStrSize ((CHAR8 *) PcdGetPtr (PcdTcgPhysicalPresenceInterfaceVer))
>      );
>
> +  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +
>    //
>    // Initialize efi varstore configuration data.
>    //
> @@ -174,6 +177,9 @@ InitializeTcg2VersionInfo (
>        if (PcdTcg2PpiVersion != Tcg2Version.PpiVersion) {
>          DEBUG ((DEBUG_WARN, "WARNING:
> PcdTcgPhysicalPresenceInterfaceVer default value is not same with the default
> value in VFR\n"));
>          DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be
> chosen\n"));
> +      } else if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
> +        DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev default
> value is not same with the default value in VFR\n"));
> +        DEBUG ((DEBUG_WARN, "WARNING: The default value in VFR has be
> chosen\n"));
>        }
>      }
>    }
> @@ -206,6 +212,29 @@ InitializeTcg2VersionInfo (
>        ASSERT (FALSE);
>        break;
>    }
> +
> +  //
> +  // Get the PcdTpm2AcpiTableRev value again.
> +  // If the PCD value is not equal to the value in variable,
> +  // the PCD is not DynamicHii type and does not map to TCG2_VERSION
> Variable.
> +  //
> +  PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +  if (PcdTpm2AcpiTableRev != Tcg2Version.Tpm2AcpiTableRev) {
> +    DEBUG ((DEBUG_WARN, "WARNING: PcdTpm2AcpiTableRev is not
> DynamicHii type and does not map to TCG2_VERSION.Tpm2AcpiTableRev\n"));
> +    DEBUG ((DEBUG_WARN, "WARNING: The Tpm2 ACPI Revision configuring
> from setup page will not work\n"));
> +  }
> +
> +  switch (PcdTpm2AcpiTableRev) {
> +    case TPM2_ACPI_REVISION_3:
> +      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN
> (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 3", NULL);
> +      break;
> +    case TPM2_ACPI_REVISION_4:
> +      HiiSetString (PrivateData->HiiHandle, STRING_TOKEN
> (STR_TPM2_ACPI_REVISION_STATE_CONTENT), L"Rev 4", NULL);
> +      break;
> +    default:
> +      ASSERT (FALSE);
> +      break;
> +  }
>  }
>
>  /**
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> index 9f21aab..38fa331 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> @@ -78,6 +78,7 @@
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcg2HashAlgorithmBitmap    ##
> CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress             ##
> CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ##
> CONSUMES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev           ##
> CONSUMES
>
>  [Depex]
>    gEfiTcg2ProtocolGuid              AND
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> index f4a07c6..a83000f 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
> @@ -481,6 +481,7 @@ Tcg2VersionInfoCallback (
>  {
>    EFI_INPUT_KEY                 Key;
>    UINT64                        PcdTcg2PpiVersion;
> +  UINT8                         PcdTpm2AcpiTableRev;
>
>    ASSERT (Action == EFI_BROWSER_ACTION_SUBMITTED);
>
> @@ -506,6 +507,24 @@ Tcg2VersionInfoCallback (
>          NULL
>          );
>      }
> +  } else if (QuestionId == KEY_TPM2_ACPI_REVISION){
> +    //
> +    // Get the PCD value after EFI_BROWSER_ACTION_SUBMITTED,
> +    // the SetVariable to TCG2_VERSION_NAME should have been done.
> +    // If the PCD value is not equal to the value set to variable,
> +    // the PCD is not DynamicHii type and does not map to the setup option.
> +    //
> +    PcdTpm2AcpiTableRev = PcdGet8 (PcdTpm2AcpiTableRev);
> +
> +    if (PcdTpm2AcpiTableRev != Value->u8) {
> +      CreatePopUp (
> +        EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
> +        &Key,
> +        L"WARNING: PcdTpm2AcpiTableRev is not DynamicHii type and does
> not map to this option!",
> +        L"The Revision configuring by this setup option will not work!",
> +        NULL
> +        );
> +    }
>    }
>
>    return EFI_SUCCESS;
> @@ -607,7 +626,7 @@ Tcg2Callback (
>    }
>
>    if (Action == EFI_BROWSER_ACTION_SUBMITTED) {
> -    if (QuestionId == KEY_TCG2_PPI_VERSION) {
> +    if (QuestionId == KEY_TCG2_PPI_VERSION || QuestionId ==
> KEY_TPM2_ACPI_REVISION) {
>        return Tcg2VersionInfoCallback (Action, QuestionId, Type, Value);
>      }
>    }
> @@ -971,6 +990,7 @@ InstallTcg2ConfigForm (
>    if (EFI_ERROR (Status)) {
>      DEBUG ((EFI_D_ERROR, "Tcg2ConfigDriver: Fail to set
> TCG2_STORAGE_INFO_NAME\n"));
>    }
> +
>    return EFI_SUCCESS;
>  }
>
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> index 7868c21..5960446 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
> @@ -29,7 +29,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #define EFI_TCG2_EVENT_LOG_FORMAT_ALL
> (EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 |
> EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
>
>  #define TCG2_CONFIGURATION_VARSTORE_ID  0x0001
> -#define TCG2_CONFIGURATION_INFO_VARSTORE_ID  0x0002
> +#define TCG2_CONFIGURATION_INFO_VARSTORE_ID     0x0002
>  #define TCG2_VERSION_VARSTORE_ID        0x0003
>  #define TCG2_CONFIGURATION_FORM_ID      0x0001
>
> @@ -43,6 +43,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #define KEY_TPM2_PCR_BANKS_REQUEST_4            0x2007
>  #define KEY_TPM_DEVICE_INTERFACE                       0x2008
>  #define KEY_TCG2_PPI_VERSION                    0x2009
> +#define KEY_TPM2_ACPI_REVISION                  0x200A
>
>  #define TPM_DEVICE_NULL           0
>  #define TPM_DEVICE_1_2            1
> @@ -51,6 +52,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #define TPM_DEVICE_MAX            TPM_DEVICE_2_0_DTPM
>  #define TPM_DEVICE_DEFAULT        TPM_DEVICE_1_2
>
> +#define TPM2_ACPI_REVISION_3       3
> +#define TPM2_ACPI_REVISION_4       4
> +
>  #define TPM_DEVICE_INTERFACE_TIS       0
>  #define TPM_DEVICE_INTERFACE_PTP_FIFO  1
>  #define TPM_DEVICE_INTERFACE_PTP_CRB   2
> @@ -72,6 +76,7 @@ typedef struct {
>
>  typedef struct {
>    UINT64  PpiVersion;
> +  UINT8   Tpm2AcpiTableRev;
>  } TCG2_VERSION;
>
>  typedef struct {
> diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> index 414dcec..a7d62bc 100644
> --- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> +++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigStrings.uni
> @@ -38,6 +38,15 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #string STR_TPM2_ACPI_HID_HELP                    #language en-US
> "HID from TPM2 ACPI Table: ManufacturerID + FirmwareVersion_1"
>  #string STR_TPM2_ACPI_HID_CONTENT                 #language en-US ""
>
> +#string STR_TPM2_ACPI_REVISION_STATE_PROMPT   #language en-US
> "Current Rev of TPM2 ACPI Table"
> +#string STR_TPM2_ACPI_REVISION_STATE_HELP     #language en-US
> "Current Rev of TPM2 ACPI Table: Rev 3 or Rev 4"
> +#string STR_TPM2_ACPI_REVISION_STATE_CONTENT  #language en-US ""
> +
> +#string STR_TPM2_ACPI_REVISION_PROMPT          #language en-US
> "Attempt Rev of TPM2 ACPI Table"
> +#string STR_TPM2_ACPI_REVISION_HELP            #language en-US "Rev 3
> or Rev 4 (Rev 4 is defined in TCG ACPI Spec 00.37)"
> +
> "PcdTpm2AcpiTableRev needs to be DynamicHii type and map to this option\n"
> +
> "Otherwise the version configuring by this setup option will not work"
> +
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_PROMPT         #language
> en-US "Current TPM Device Interface"
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_HELP           #language
> en-US "Current TPM Device Interface: TIS, PTP FIFO, PTP CRB"
>  #string STR_TCG2_DEVICE_INTERFACE_STATE_CONTENT        #language
> en-US ""
> @@ -74,6 +83,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
>  #string STR_TCG2_TPM_1_2                   #language en-US "TPM 1.2"
>  #string STR_TCG2_TPM_2_0_DTPM              #language en-US "TPM 2.0"
>
> +#string STR_TPM2_ACPI_REVISION_3           #language en-US "Rev 3"
> +#string STR_TPM2_ACPI_REVISION_4           #language en-US "Rev 4"
> +
>  #string STR_TCG2_PPI_VERSION_1_2           #language en-US "1.2"
>  #string STR_TCG2_PPI_VERSION_1_3           #language en-US "1.3"
>
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index 7557e29..927de15 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -83,7 +83,8 @@ EFI_TPM2_ACPI_TABLE  mTpm2AcpiTemplate = {
>      // These fields should be filled in in production
>      //
>    },
> -  0, // Flags
> +  0, // 16-bit PlatformClass
> +  0, // 16-bit Reserved
>    0, // Control Area
>    EFI_TPM2_ACPI_TABLE_START_METHOD_TIS, // StartMethod
>  };
> @@ -508,6 +509,9 @@ PublishTpm2 (
>    EFI_TPM2_ACPI_CONTROL_AREA     *ControlArea;
>    PTP_INTERFACE_TYPE             InterfaceType;
>
> +  mTpm2AcpiTemplate.Header.Revision = PcdGet8(PcdTpm2AcpiTableRev);
> +  DEBUG((DEBUG_INFO, "Tpm2 ACPI table revision is %d\n",
> mTpm2AcpiTemplate.Header.Revision));
> +
>    //
>    // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
>    //
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> index 8c823d6..2793242 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> @@ -9,7 +9,7 @@
>  #  This driver will have external input - variable and ACPINvs data in SMM
> mode.
>  #  This external input must be validated carefully to avoid security issue.
>  #
> -# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015 - 2017, 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
> @@ -73,6 +73,7 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision  ##
> SOMETIMES_CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress               ##
> CONSUMES
>    gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer  ##
> CONSUMES
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev                 ##
> CONSUMES
>
>  [Depex]
>    gEfiAcpiTableProtocolGuid AND
> --
> 1.9.5.msysgit.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-01-10  5:21 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-10  2:24 [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version Zhang, Chao B
2017-01-10  2:24 ` [PATCH 2/2] SecurityPkg: Tcg2Config: TPM2 ACPI Table Rev Option Zhang, Chao B
2017-01-10  5:04   ` Zeng, Star
2017-01-10  5:12   ` Yao, Jiewen
2017-01-10  5:21     ` Zhang, Chao B
2017-01-10  5:11 ` [PATCH 1/2] MdePkg: Tpm2Acpi.h: Update TPM2 ACPI table version Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox