* [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
@ 2016-12-27 1:51 Zhang, Chao B
2016-12-27 2:13 ` Yao, Jiewen
2016-12-27 3:18 ` Zeng, Star
0 siblings, 2 replies; 6+ messages in thread
From: Zhang, Chao B @ 2016-12-27 1:51 UTC (permalink / raw)
To: edk2-devel; +Cc: jiewen.yao, star.zeng, Chao Zhang
Update TPM2 HID using vendor ManufacturerID & FirmwareVersion1.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
---
.../Library/Tpm2CommandLib/Tpm2Capability.c | 4 +-
SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 102 +++++++++++++++++++++
SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h | 10 +-
SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf | 2 +-
SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 10 +-
5 files changed, 122 insertions(+), 6 deletions(-)
diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
index 9aab17f..79e80fb 100644
--- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
+++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
@@ -1,7 +1,7 @@
/** @file
Implement TPM2 Capability related command.
-Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
+Copyright (c) 2013 - 2016, 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
@@ -189,7 +189,7 @@ Tpm2GetCapabilityManufactureID (
if (EFI_ERROR (Status)) {
return Status;
}
- *ManufactureId = SwapBytes32 (TpmCap.data.tpmProperties.tpmProperty->value);
+ *ManufactureId = TpmCap.data.tpmProperties.tpmProperty->value;
return EFI_SUCCESS;
}
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index d02123d..addb302 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -303,6 +303,103 @@ UpdatePPVersion (
}
/**
+ Patch TPM2 device HID string. The initial string tag in TPM2 ACPI table is "NNN0000".
+
+ @param[in, out] Table The TPM2 SSDT ACPI table.
+
+ @return HID Update status.
+
+**/
+EFI_STATUS
+UpdateHID (
+ EFI_ACPI_DESCRIPTION_HEADER *Table
+ )
+{
+ EFI_STATUS Status;
+ UINT8 *DataPtr;
+ CHAR8 HID[TPM_HID_ACPI_SIZE];
+ UINT32 ManufacturerID;
+ UINT32 FirmwareVersion1;
+ UINT32 FirmwareVersion2;
+ BOOLEAN PnpHID;
+
+ PnpHID = TRUE;
+
+ //
+ // Initialize HID with Default PNP string
+ //
+ ZeroMem(HID, TPM_HID_ACPI_SIZE);
+ CopyMem(HID, TPM_HID_TAG, TPM_HID_PNP_SIZE);
+
+ //
+ // Get Manufacturer ID
+ //
+ Status = Tpm2GetCapabilityManufactureID(&ManufacturerID);
+ if (!EFI_ERROR(Status)) {
+ DEBUG((EFI_D_INFO, "TPM_PT_VENDOR_STRING_1 0x%08x\n", ManufacturerID));
+ //
+ // ManfacturerID defined in TCG Vendor ID Registry
+ // may tailed with 0x00 or 0x20
+ //
+ if ((ManufacturerID >> 24) == 0x00 || ((ManufacturerID >> 24) == 0x20)) {
+ //
+ // HID containing PNP ID "NNN####"
+ // NNN is uppercase letter for Vendor ID specified by manufacturer
+ //
+ CopyMem(HID, &ManufacturerID, 3);
+ } else {
+ //
+ // HID containing ACP ID "NNNN####"
+ // NNNN is uppercase letter for Vendor ID specified by manufacturer
+ //
+ CopyMem(HID, &ManufacturerID, 4);
+ PnpHID = FALSE;
+ }
+ } else {
+ DEBUG ((EFI_D_ERROR, "Get TPM_PT_VENDOR_STRING_1 failed %x!\n", Status));
+ }
+
+ Status = Tpm2GetCapabilityFirmwareVersion(&FirmwareVersion1, &FirmwareVersion2);
+ if (!EFI_ERROR(Status)) {
+ DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_1 0x%x\n", FirmwareVersion1));
+ DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_2 0x%x\n", FirmwareVersion2));
+ //
+ // #### is Firmware Version 1
+ //
+ if (PnpHID) {
+ AsciiSPrint(HID + 3, TPM_HID_PNP_SIZE - 3, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
+ } else {
+ AsciiSPrint(HID + 4, TPM_HID_ACPI_SIZE - 4, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
+ }
+
+ } else {
+ DEBUG ((EFI_D_ERROR, "Get TPM_PT_FIRMWARE_VERSION_X failed %x!\n", Status));
+ }
+
+ //
+ // Patch HID in ASL code before loading the SSDT.
+ //
+ for (DataPtr = (UINT8 *)(Table + 1);
+ DataPtr <= (UINT8 *) ((UINT8 *) Table + Table->Length - TPM_HID_PNP_SIZE);
+ DataPtr += 1) {
+ if (AsciiStrCmp((CHAR8 *)DataPtr, TPM_HID_TAG) == 0) {
+ if (PnpHID) {
+ CopyMem(DataPtr, HID, TPM_HID_PNP_SIZE);
+ } else {
+ //
+ // NOOP will be patched to '\0'
+ //
+ CopyMem(DataPtr, HID, TPM_HID_ACPI_SIZE);
+ }
+ DEBUG((EFI_D_INFO, "TPM2 ACPI _HID updated to %a\n", HID));
+ return Status;
+ }
+ }
+
+ return EFI_NOT_FOUND;
+}
+
+/**
Initialize and publish TPM items in ACPI table.
@retval EFI_SUCCESS The TCG ACPI table is published successfully.
@@ -336,6 +433,11 @@ PublishAcpiTable (
ASSERT_EFI_ERROR (Status);
//
+ // Update Table version before measuring it to PCR
+ //
+ Status = UpdateHID(Table);
+
+ //
// Measure to PCR[0] with event EV_POST_CODE ACPI DATA
//
TpmMeasureAndLogData(
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
index 0b09032..18e8bfc 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
@@ -35,9 +35,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include <Library/UefiBootServicesTableLib.h>
#include <Library/DxeServicesLib.h>
#include <Library/TpmMeasurementLib.h>
-#include <Library/Tpm2DeviceLib.h>
+#include <Library/Tpm2CommandLib.h>
#include <Library/Tcg2PhysicalPresenceLib.h>
#include <Library/IoLib.h>
+#include <Library/PrintLib.h>
#include <IndustryStandard/TpmPtp.h>
@@ -94,4 +95,11 @@ typedef struct {
#define PHYSICAL_PRESENCE_VERSION_TAG "$PV"
#define PHYSICAL_PRESENCE_VERSION_SIZE 4
+//
+// PNP _HID for TPM2 device
+//
+#define TPM_HID_TAG "NNN0000"
+#define TPM_HID_PNP_SIZE 8
+#define TPM_HID_ACPI_SIZE 9
+
#endif // __TCG_SMM_H__
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
index 0de4fce..8c823d6 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
@@ -48,7 +48,7 @@
DebugLib
DxeServicesLib
TpmMeasurementLib
- Tpm2DeviceLib
+ Tpm2CommandLib
Tcg2PhysicalPresenceLib
IoLib
diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
index 2083a3e..4128684 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
@@ -30,8 +30,14 @@ DefinitionBlock (
//
// TCG2
//
- Name (_HID, "MSFT0101")
-
+ Name (_HID, "NNN0000")
+ //
+ // Reserve 1 more byte for ACPI HID
+ //
+ Noop
+
+ Name (_CID, "MSFT0101")
+
//
// Readable name of this device, don't know if this way is correct yet
//
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
2016-12-27 1:51 [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID Zhang, Chao B
@ 2016-12-27 2:13 ` Yao, Jiewen
2016-12-27 3:18 ` Zeng, Star
1 sibling, 0 replies; 6+ messages in thread
From: Yao, Jiewen @ 2016-12-27 2:13 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Zeng, Star
Reviewed-by: jiewen.yao@Intel.com
> -----Original Message-----
> From: Zhang, Chao B
> Sent: Tuesday, December 27, 2016 9:51 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] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
>
> Update TPM2 HID using vendor ManufacturerID & FirmwareVersion1.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
> .../Library/Tpm2CommandLib/Tpm2Capability.c | 4 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 102
> +++++++++++++++++++++
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h | 10 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf | 2 +-
> SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 10 +-
> 5 files changed, 122 insertions(+), 6 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> index 9aab17f..79e80fb 100644
> --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> @@ -1,7 +1,7 @@
> /** @file
> Implement TPM2 Capability related command.
>
> -Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2016, 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
> @@ -189,7 +189,7 @@ Tpm2GetCapabilityManufactureID (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - *ManufactureId = SwapBytes32
> (TpmCap.data.tpmProperties.tpmProperty->value);
> + *ManufactureId = TpmCap.data.tpmProperties.tpmProperty->value;
>
> return EFI_SUCCESS;
> }
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index d02123d..addb302 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -303,6 +303,103 @@ UpdatePPVersion (
> }
>
> /**
> + Patch TPM2 device HID string. The initial string tag in TPM2 ACPI table is
> "NNN0000".
> +
> + @param[in, out] Table The TPM2 SSDT ACPI table.
> +
> + @return HID Update status.
> +
> +**/
> +EFI_STATUS
> +UpdateHID (
> + EFI_ACPI_DESCRIPTION_HEADER *Table
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 *DataPtr;
> + CHAR8 HID[TPM_HID_ACPI_SIZE];
> + UINT32 ManufacturerID;
> + UINT32 FirmwareVersion1;
> + UINT32 FirmwareVersion2;
> + BOOLEAN PnpHID;
> +
> + PnpHID = TRUE;
> +
> + //
> + // Initialize HID with Default PNP string
> + //
> + ZeroMem(HID, TPM_HID_ACPI_SIZE);
> + CopyMem(HID, TPM_HID_TAG, TPM_HID_PNP_SIZE);
> +
> + //
> + // Get Manufacturer ID
> + //
> + Status = Tpm2GetCapabilityManufactureID(&ManufacturerID);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_VENDOR_STRING_1 0x%08x\n",
> ManufacturerID));
> + //
> + // ManfacturerID defined in TCG Vendor ID Registry
> + // may tailed with 0x00 or 0x20
> + //
> + if ((ManufacturerID >> 24) == 0x00 || ((ManufacturerID >> 24) == 0x20)) {
> + //
> + // HID containing PNP ID "NNN####"
> + // NNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 3);
> + } else {
> + //
> + // HID containing ACP ID "NNNN####"
> + // NNNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 4);
> + PnpHID = FALSE;
> + }
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_VENDOR_STRING_1 failed %x!\n",
> Status));
> + }
> +
> + Status = Tpm2GetCapabilityFirmwareVersion(&FirmwareVersion1,
> &FirmwareVersion2);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_1 0x%x\n",
> FirmwareVersion1));
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_2 0x%x\n",
> FirmwareVersion2));
> + //
> + // #### is Firmware Version 1
> + //
> + if (PnpHID) {
> + AsciiSPrint(HID + 3, TPM_HID_PNP_SIZE - 3, "%02d%02d",
> ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + } else {
> + AsciiSPrint(HID + 4, TPM_HID_ACPI_SIZE - 4, "%02d%02d",
> ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + }
> +
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_FIRMWARE_VERSION_X
> failed %x!\n", Status));
> + }
> +
> + //
> + // Patch HID in ASL code before loading the SSDT.
> + //
> + for (DataPtr = (UINT8 *)(Table + 1);
> + DataPtr <= (UINT8 *) ((UINT8 *) Table + Table->Length -
> TPM_HID_PNP_SIZE);
> + DataPtr += 1) {
> + if (AsciiStrCmp((CHAR8 *)DataPtr, TPM_HID_TAG) == 0) {
> + if (PnpHID) {
> + CopyMem(DataPtr, HID, TPM_HID_PNP_SIZE);
> + } else {
> + //
> + // NOOP will be patched to '\0'
> + //
> + CopyMem(DataPtr, HID, TPM_HID_ACPI_SIZE);
> + }
> + DEBUG((EFI_D_INFO, "TPM2 ACPI _HID updated to %a\n", HID));
> + return Status;
> + }
> + }
> +
> + return EFI_NOT_FOUND;
> +}
> +
> +/**
> Initialize and publish TPM items in ACPI table.
>
> @retval EFI_SUCCESS The TCG ACPI table is published successfully.
> @@ -336,6 +433,11 @@ PublishAcpiTable (
> ASSERT_EFI_ERROR (Status);
>
> //
> + // Update Table version before measuring it to PCR
> + //
> + Status = UpdateHID(Table);
> +
> + //
> // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
> //
> TpmMeasureAndLogData(
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> index 0b09032..18e8bfc 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> @@ -35,9 +35,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/DxeServicesLib.h>
> #include <Library/TpmMeasurementLib.h>
> -#include <Library/Tpm2DeviceLib.h>
> +#include <Library/Tpm2CommandLib.h>
> #include <Library/Tcg2PhysicalPresenceLib.h>
> #include <Library/IoLib.h>
> +#include <Library/PrintLib.h>
>
> #include <IndustryStandard/TpmPtp.h>
>
> @@ -94,4 +95,11 @@ typedef struct {
> #define PHYSICAL_PRESENCE_VERSION_TAG
> "$PV"
> #define PHYSICAL_PRESENCE_VERSION_SIZE 4
>
> +//
> +// PNP _HID for TPM2 device
> +//
> +#define TPM_HID_TAG
> "NNN0000"
> +#define TPM_HID_PNP_SIZE 8
> +#define TPM_HID_ACPI_SIZE 9
> +
> #endif // __TCG_SMM_H__
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> index 0de4fce..8c823d6 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> @@ -48,7 +48,7 @@
> DebugLib
> DxeServicesLib
> TpmMeasurementLib
> - Tpm2DeviceLib
> + Tpm2CommandLib
> Tcg2PhysicalPresenceLib
> IoLib
>
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> index 2083a3e..4128684 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> @@ -30,8 +30,14 @@ DefinitionBlock (
> //
> // TCG2
> //
> - Name (_HID, "MSFT0101")
> -
> + Name (_HID, "NNN0000")
> + //
> + // Reserve 1 more byte for ACPI HID
> + //
> + Noop
> +
> + Name (_CID, "MSFT0101")
> +
> //
> // Readable name of this device, don't know if this way is correct yet
> //
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
2016-12-27 1:51 [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID Zhang, Chao B
2016-12-27 2:13 ` Yao, Jiewen
@ 2016-12-27 3:18 ` Zeng, Star
2016-12-27 3:30 ` Zhang, Chao B
1 sibling, 1 reply; 6+ messages in thread
From: Zeng, Star @ 2016-12-27 3:18 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel; +Cc: jiewen.yao, star.zeng
Chao,
Minor comments inline.
On 2016/12/27 9:51, Zhang, Chao B wrote:
> Update TPM2 HID using vendor ManufacturerID & FirmwareVersion1.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
> ---
> .../Library/Tpm2CommandLib/Tpm2Capability.c | 4 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 102 +++++++++++++++++++++
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h | 10 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf | 2 +-
> SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 10 +-
> 5 files changed, 122 insertions(+), 6 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> index 9aab17f..79e80fb 100644
> --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> @@ -1,7 +1,7 @@
> /** @file
> Implement TPM2 Capability related command.
>
> -Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2016, 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
> @@ -189,7 +189,7 @@ Tpm2GetCapabilityManufactureID (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - *ManufactureId = SwapBytes32 (TpmCap.data.tpmProperties.tpmProperty->value);
> + *ManufactureId = TpmCap.data.tpmProperties.tpmProperty->value;
>
> return EFI_SUCCESS;
> }
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index d02123d..addb302 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -303,6 +303,103 @@ UpdatePPVersion (
> }
>
> /**
> + Patch TPM2 device HID string. The initial string tag in TPM2 ACPI table is "NNN0000".
> +
> + @param[in, out] Table The TPM2 SSDT ACPI table.
> +
> + @return HID Update status.
> +
> +**/
> +EFI_STATUS
> +UpdateHID (
> + EFI_ACPI_DESCRIPTION_HEADER *Table
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 *DataPtr;
> + CHAR8 HID[TPM_HID_ACPI_SIZE];
> + UINT32 ManufacturerID;
> + UINT32 FirmwareVersion1;
> + UINT32 FirmwareVersion2;
> + BOOLEAN PnpHID;
> +
> + PnpHID = TRUE;
> +
> + //
> + // Initialize HID with Default PNP string
> + //
> + ZeroMem(HID, TPM_HID_ACPI_SIZE);
> + CopyMem(HID, TPM_HID_TAG, TPM_HID_PNP_SIZE);
> +
> + //
> + // Get Manufacturer ID
> + //
> + Status = Tpm2GetCapabilityManufactureID(&ManufacturerID);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_VENDOR_STRING_1 0x%08x\n", ManufacturerID));
> + //
> + // ManfacturerID defined in TCG Vendor ID Registry
> + // may tailed with 0x00 or 0x20
> + //
> + if ((ManufacturerID >> 24) == 0x00 || ((ManufacturerID >> 24) == 0x20)) {
> + //
> + // HID containing PNP ID "NNN####"
> + // NNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 3);
> + } else {
> + //
> + // HID containing ACP ID "NNNN####"
> + // NNNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 4);
> + PnpHID = FALSE;
> + }
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_VENDOR_STRING_1 failed %x!\n", Status));
Should just return failure here?
> + }
> +
> + Status = Tpm2GetCapabilityFirmwareVersion(&FirmwareVersion1, &FirmwareVersion2);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_1 0x%x\n", FirmwareVersion1));
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_2 0x%x\n", FirmwareVersion2));
> + //
> + // #### is Firmware Version 1
> + //
> + if (PnpHID) {
> + AsciiSPrint(HID + 3, TPM_HID_PNP_SIZE - 3, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + } else {
> + AsciiSPrint(HID + 4, TPM_HID_ACPI_SIZE - 4, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + }
> +
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_FIRMWARE_VERSION_X failed %x!\n", Status));
Should just return failure here?
> + }
> +
> + //
> + // Patch HID in ASL code before loading the SSDT.
> + //
> + for (DataPtr = (UINT8 *)(Table + 1);
> + DataPtr <= (UINT8 *) ((UINT8 *) Table + Table->Length - TPM_HID_PNP_SIZE);
> + DataPtr += 1) {
> + if (AsciiStrCmp((CHAR8 *)DataPtr, TPM_HID_TAG) == 0) {
> + if (PnpHID) {
> + CopyMem(DataPtr, HID, TPM_HID_PNP_SIZE);
> + } else {
> + //
> + // NOOP will be patched to '\0'
> + //
> + CopyMem(DataPtr, HID, TPM_HID_ACPI_SIZE);
> + }
> + DEBUG((EFI_D_INFO, "TPM2 ACPI _HID updated to %a\n", HID));
> + return Status;
> + }
> + }
> +
> + return EFI_NOT_FOUND;
Since the for loop above can find out TPM_HID_TAG definitly,
can the code run to here according to current code logic?
> +}
> +
> +/**
> Initialize and publish TPM items in ACPI table.
>
> @retval EFI_SUCCESS The TCG ACPI table is published successfully.
> @@ -336,6 +433,11 @@ PublishAcpiTable (
> ASSERT_EFI_ERROR (Status);
>
> //
> + // Update Table version before measuring it to PCR
Does the comment here need to be updated?
Thanks,
Star
> + //
> + Status = UpdateHID(Table);
> +
> + //
> // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
> //
> TpmMeasureAndLogData(
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> index 0b09032..18e8bfc 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> @@ -35,9 +35,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/DxeServicesLib.h>
> #include <Library/TpmMeasurementLib.h>
> -#include <Library/Tpm2DeviceLib.h>
> +#include <Library/Tpm2CommandLib.h>
> #include <Library/Tcg2PhysicalPresenceLib.h>
> #include <Library/IoLib.h>
> +#include <Library/PrintLib.h>
>
> #include <IndustryStandard/TpmPtp.h>
>
> @@ -94,4 +95,11 @@ typedef struct {
> #define PHYSICAL_PRESENCE_VERSION_TAG "$PV"
> #define PHYSICAL_PRESENCE_VERSION_SIZE 4
>
> +//
> +// PNP _HID for TPM2 device
> +//
> +#define TPM_HID_TAG "NNN0000"
> +#define TPM_HID_PNP_SIZE 8
> +#define TPM_HID_ACPI_SIZE 9
> +
> #endif // __TCG_SMM_H__
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> index 0de4fce..8c823d6 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> @@ -48,7 +48,7 @@
> DebugLib
> DxeServicesLib
> TpmMeasurementLib
> - Tpm2DeviceLib
> + Tpm2CommandLib
> Tcg2PhysicalPresenceLib
> IoLib
>
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> index 2083a3e..4128684 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> @@ -30,8 +30,14 @@ DefinitionBlock (
> //
> // TCG2
> //
> - Name (_HID, "MSFT0101")
> -
> + Name (_HID, "NNN0000")
> + //
> + // Reserve 1 more byte for ACPI HID
> + //
> + Noop
> +
> + Name (_CID, "MSFT0101")
> +
> //
> // Readable name of this device, don't know if this way is correct yet
> //
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
2016-12-27 3:18 ` Zeng, Star
@ 2016-12-27 3:30 ` Zhang, Chao B
2016-12-27 3:35 ` Yao, Jiewen
2016-12-27 4:38 ` Zeng, Star
0 siblings, 2 replies; 6+ messages in thread
From: Zhang, Chao B @ 2016-12-27 3:30 UTC (permalink / raw)
To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Yao, Jiewen
Star:
Thanks for your comments.
1. For error handling, ManufacturerID & Version failure is not considered to cause fatal break. We can still use default "NNN0000" value.
2. I will update the comments for UpdateHID
3. EFI_NOT_FOUND is not expected here, but I think it is reasonable error handling if user override TPM.asl.
Thanks & Best regards
Chao Zhang
From: Zeng, Star
Sent: Tuesday, December 27, 2016 11:19 AM
To: Zhang, Chao B; edk2-devel@lists.01.org
Cc: Yao, Jiewen; Zeng, Star
Subject: Re: [edk2] [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
Chao,
Minor comments inline.
On 2016/12/27 9:51, Zhang, Chao B wrote:
> Update TPM2 HID using vendor ManufacturerID & FirmwareVersion1.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> ---
> .../Library/Tpm2CommandLib/Tpm2Capability.c | 4 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 102 +++++++++++++++++++++
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h | 10 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf | 2 +-
> SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 10 +-
> 5 files changed, 122 insertions(+), 6 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> index 9aab17f..79e80fb 100644
> --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> @@ -1,7 +1,7 @@
> /** @file
> Implement TPM2 Capability related command.
>
> -Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2016, 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
> @@ -189,7 +189,7 @@ Tpm2GetCapabilityManufactureID (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - *ManufactureId = SwapBytes32 (TpmCap.data.tpmProperties.tpmProperty->value);
> + *ManufactureId = TpmCap.data.tpmProperties.tpmProperty->value;
>
> return EFI_SUCCESS;
> }
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index d02123d..addb302 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -303,6 +303,103 @@ UpdatePPVersion (
> }
>
> /**
> + Patch TPM2 device HID string. The initial string tag in TPM2 ACPI table is "NNN0000".
> +
> + @param[in, out] Table The TPM2 SSDT ACPI table.
> +
> + @return HID Update status.
> +
> +**/
> +EFI_STATUS
> +UpdateHID (
> + EFI_ACPI_DESCRIPTION_HEADER *Table
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 *DataPtr;
> + CHAR8 HID[TPM_HID_ACPI_SIZE];
> + UINT32 ManufacturerID;
> + UINT32 FirmwareVersion1;
> + UINT32 FirmwareVersion2;
> + BOOLEAN PnpHID;
> +
> + PnpHID = TRUE;
> +
> + //
> + // Initialize HID with Default PNP string
> + //
> + ZeroMem(HID, TPM_HID_ACPI_SIZE);
> + CopyMem(HID, TPM_HID_TAG, TPM_HID_PNP_SIZE);
> +
> + //
> + // Get Manufacturer ID
> + //
> + Status = Tpm2GetCapabilityManufactureID(&ManufacturerID);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_VENDOR_STRING_1 0x%08x\n", ManufacturerID));
> + //
> + // ManfacturerID defined in TCG Vendor ID Registry
> + // may tailed with 0x00 or 0x20
> + //
> + if ((ManufacturerID >> 24) == 0x00 || ((ManufacturerID >> 24) == 0x20)) {
> + //
> + // HID containing PNP ID "NNN####"
> + // NNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 3);
> + } else {
> + //
> + // HID containing ACP ID "NNNN####"
> + // NNNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 4);
> + PnpHID = FALSE;
> + }
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_VENDOR_STRING_1 failed %x!\n", Status));
Should just return failure here?
> + }
> +
> + Status = Tpm2GetCapabilityFirmwareVersion(&FirmwareVersion1, &FirmwareVersion2);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_1 0x%x\n", FirmwareVersion1));
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_2 0x%x\n", FirmwareVersion2));
> + //
> + // #### is Firmware Version 1
> + //
> + if (PnpHID) {
> + AsciiSPrint(HID + 3, TPM_HID_PNP_SIZE - 3, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + } else {
> + AsciiSPrint(HID + 4, TPM_HID_ACPI_SIZE - 4, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + }
> +
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_FIRMWARE_VERSION_X failed %x!\n", Status));
Should just return failure here?
> + }
> +
> + //
> + // Patch HID in ASL code before loading the SSDT.
> + //
> + for (DataPtr = (UINT8 *)(Table + 1);
> + DataPtr <= (UINT8 *) ((UINT8 *) Table + Table->Length - TPM_HID_PNP_SIZE);
> + DataPtr += 1) {
> + if (AsciiStrCmp((CHAR8 *)DataPtr, TPM_HID_TAG) == 0) {
> + if (PnpHID) {
> + CopyMem(DataPtr, HID, TPM_HID_PNP_SIZE);
> + } else {
> + //
> + // NOOP will be patched to '\0'
> + //
> + CopyMem(DataPtr, HID, TPM_HID_ACPI_SIZE);
> + }
> + DEBUG((EFI_D_INFO, "TPM2 ACPI _HID updated to %a\n", HID));
> + return Status;
> + }
> + }
> +
> + return EFI_NOT_FOUND;
Since the for loop above can find out TPM_HID_TAG definitly,
can the code run to here according to current code logic?
> +}
> +
> +/**
> Initialize and publish TPM items in ACPI table.
>
> @retval EFI_SUCCESS The TCG ACPI table is published successfully.
> @@ -336,6 +433,11 @@ PublishAcpiTable (
> ASSERT_EFI_ERROR (Status);
>
> //
> + // Update Table version before measuring it to PCR
Does the comment here need to be updated?
Thanks,
Star
> + //
> + Status = UpdateHID(Table);
> +
> + //
> // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
> //
> TpmMeasureAndLogData(
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> index 0b09032..18e8bfc 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> @@ -35,9 +35,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/DxeServicesLib.h>
> #include <Library/TpmMeasurementLib.h>
> -#include <Library/Tpm2DeviceLib.h>
> +#include <Library/Tpm2CommandLib.h>
> #include <Library/Tcg2PhysicalPresenceLib.h>
> #include <Library/IoLib.h>
> +#include <Library/PrintLib.h>
>
> #include <IndustryStandard/TpmPtp.h>
>
> @@ -94,4 +95,11 @@ typedef struct {
> #define PHYSICAL_PRESENCE_VERSION_TAG "$PV"
> #define PHYSICAL_PRESENCE_VERSION_SIZE 4
>
> +//
> +// PNP _HID for TPM2 device
> +//
> +#define TPM_HID_TAG "NNN0000"
> +#define TPM_HID_PNP_SIZE 8
> +#define TPM_HID_ACPI_SIZE 9
> +
> #endif // __TCG_SMM_H__
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> index 0de4fce..8c823d6 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> @@ -48,7 +48,7 @@
> DebugLib
> DxeServicesLib
> TpmMeasurementLib
> - Tpm2DeviceLib
> + Tpm2CommandLib
> Tcg2PhysicalPresenceLib
> IoLib
>
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> index 2083a3e..4128684 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> @@ -30,8 +30,14 @@ DefinitionBlock (
> //
> // TCG2
> //
> - Name (_HID, "MSFT0101")
> -
> + Name (_HID, "NNN0000")
> + //
> + // Reserve 1 more byte for ACPI HID
> + //
> + Noop
> +
> + Name (_CID, "MSFT0101")
> +
> //
> // Readable name of this device, don't know if this way is correct yet
> //
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
2016-12-27 3:30 ` Zhang, Chao B
@ 2016-12-27 3:35 ` Yao, Jiewen
2016-12-27 4:38 ` Zeng, Star
1 sibling, 0 replies; 6+ messages in thread
From: Yao, Jiewen @ 2016-12-27 3:35 UTC (permalink / raw)
To: Zhang, Chao B, Zeng, Star, edk2-devel@lists.01.org
Maybe still use "MSFT0101" as default value, if we fail to get ManufacturerID?
From: Zhang, Chao B
Sent: Tuesday, December 27, 2016 11:31 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [edk2] [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
Star:
Thanks for your comments.
1. For error handling, ManufacturerID & Version failure is not considered to cause fatal break. We can still use default "NNN0000" value.
2. I will update the comments for UpdateHID
3. EFI_NOT_FOUND is not expected here, but I think it is reasonable error handling if user override TPM.asl.
Thanks & Best regards
Chao Zhang
From: Zeng, Star
Sent: Tuesday, December 27, 2016 11:19 AM
To: Zhang, Chao B; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Yao, Jiewen; Zeng, Star
Subject: Re: [edk2] [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
Chao,
Minor comments inline.
On 2016/12/27 9:51, Zhang, Chao B wrote:
> Update TPM2 HID using vendor ManufacturerID & FirmwareVersion1.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
> ---
> .../Library/Tpm2CommandLib/Tpm2Capability.c | 4 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 102 +++++++++++++++++++++
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h | 10 +-
> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf | 2 +-
> SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 10 +-
> 5 files changed, 122 insertions(+), 6 deletions(-)
>
> diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> index 9aab17f..79e80fb 100644
> --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
> @@ -1,7 +1,7 @@
> /** @file
> Implement TPM2 Capability related command.
>
> -Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
> +Copyright (c) 2013 - 2016, 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
> @@ -189,7 +189,7 @@ Tpm2GetCapabilityManufactureID (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - *ManufactureId = SwapBytes32 (TpmCap.data.tpmProperties.tpmProperty->value);
> + *ManufactureId = TpmCap.data.tpmProperties.tpmProperty->value;
>
> return EFI_SUCCESS;
> }
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index d02123d..addb302 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -303,6 +303,103 @@ UpdatePPVersion (
> }
>
> /**
> + Patch TPM2 device HID string. The initial string tag in TPM2 ACPI table is "NNN0000".
> +
> + @param[in, out] Table The TPM2 SSDT ACPI table.
> +
> + @return HID Update status.
> +
> +**/
> +EFI_STATUS
> +UpdateHID (
> + EFI_ACPI_DESCRIPTION_HEADER *Table
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 *DataPtr;
> + CHAR8 HID[TPM_HID_ACPI_SIZE];
> + UINT32 ManufacturerID;
> + UINT32 FirmwareVersion1;
> + UINT32 FirmwareVersion2;
> + BOOLEAN PnpHID;
> +
> + PnpHID = TRUE;
> +
> + //
> + // Initialize HID with Default PNP string
> + //
> + ZeroMem(HID, TPM_HID_ACPI_SIZE);
> + CopyMem(HID, TPM_HID_TAG, TPM_HID_PNP_SIZE);
> +
> + //
> + // Get Manufacturer ID
> + //
> + Status = Tpm2GetCapabilityManufactureID(&ManufacturerID);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_VENDOR_STRING_1 0x%08x\n", ManufacturerID));
> + //
> + // ManfacturerID defined in TCG Vendor ID Registry
> + // may tailed with 0x00 or 0x20
> + //
> + if ((ManufacturerID >> 24) == 0x00 || ((ManufacturerID >> 24) == 0x20)) {
> + //
> + // HID containing PNP ID "NNN####"
> + // NNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 3);
> + } else {
> + //
> + // HID containing ACP ID "NNNN####"
> + // NNNN is uppercase letter for Vendor ID specified by manufacturer
> + //
> + CopyMem(HID, &ManufacturerID, 4);
> + PnpHID = FALSE;
> + }
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_VENDOR_STRING_1 failed %x!\n", Status));
Should just return failure here?
> + }
> +
> + Status = Tpm2GetCapabilityFirmwareVersion(&FirmwareVersion1, &FirmwareVersion2);
> + if (!EFI_ERROR(Status)) {
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_1 0x%x\n", FirmwareVersion1));
> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_2 0x%x\n", FirmwareVersion2));
> + //
> + // #### is Firmware Version 1
> + //
> + if (PnpHID) {
> + AsciiSPrint(HID + 3, TPM_HID_PNP_SIZE - 3, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + } else {
> + AsciiSPrint(HID + 4, TPM_HID_ACPI_SIZE - 4, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
> + }
> +
> + } else {
> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_FIRMWARE_VERSION_X failed %x!\n", Status));
Should just return failure here?
> + }
> +
> + //
> + // Patch HID in ASL code before loading the SSDT.
> + //
> + for (DataPtr = (UINT8 *)(Table + 1);
> + DataPtr <= (UINT8 *) ((UINT8 *) Table + Table->Length - TPM_HID_PNP_SIZE);
> + DataPtr += 1) {
> + if (AsciiStrCmp((CHAR8 *)DataPtr, TPM_HID_TAG) == 0) {
> + if (PnpHID) {
> + CopyMem(DataPtr, HID, TPM_HID_PNP_SIZE);
> + } else {
> + //
> + // NOOP will be patched to '\0'
> + //
> + CopyMem(DataPtr, HID, TPM_HID_ACPI_SIZE);
> + }
> + DEBUG((EFI_D_INFO, "TPM2 ACPI _HID updated to %a\n", HID));
> + return Status;
> + }
> + }
> +
> + return EFI_NOT_FOUND;
Since the for loop above can find out TPM_HID_TAG definitly,
can the code run to here according to current code logic?
> +}
> +
> +/**
> Initialize and publish TPM items in ACPI table.
>
> @retval EFI_SUCCESS The TCG ACPI table is published successfully.
> @@ -336,6 +433,11 @@ PublishAcpiTable (
> ASSERT_EFI_ERROR (Status);
>
> //
> + // Update Table version before measuring it to PCR
Does the comment here need to be updated?
Thanks,
Star
> + //
> + Status = UpdateHID(Table);
> +
> + //
> // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
> //
> TpmMeasureAndLogData(
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> index 0b09032..18e8bfc 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
> @@ -35,9 +35,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/DxeServicesLib.h>
> #include <Library/TpmMeasurementLib.h>
> -#include <Library/Tpm2DeviceLib.h>
> +#include <Library/Tpm2CommandLib.h>
> #include <Library/Tcg2PhysicalPresenceLib.h>
> #include <Library/IoLib.h>
> +#include <Library/PrintLib.h>
>
> #include <IndustryStandard/TpmPtp.h>
>
> @@ -94,4 +95,11 @@ typedef struct {
> #define PHYSICAL_PRESENCE_VERSION_TAG "$PV"
> #define PHYSICAL_PRESENCE_VERSION_SIZE 4
>
> +//
> +// PNP _HID for TPM2 device
> +//
> +#define TPM_HID_TAG "NNN0000"
> +#define TPM_HID_PNP_SIZE 8
> +#define TPM_HID_ACPI_SIZE 9
> +
> #endif // __TCG_SMM_H__
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> index 0de4fce..8c823d6 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
> @@ -48,7 +48,7 @@
> DebugLib
> DxeServicesLib
> TpmMeasurementLib
> - Tpm2DeviceLib
> + Tpm2CommandLib
> Tcg2PhysicalPresenceLib
> IoLib
>
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> index 2083a3e..4128684 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
> @@ -30,8 +30,14 @@ DefinitionBlock (
> //
> // TCG2
> //
> - Name (_HID, "MSFT0101")
> -
> + Name (_HID, "NNN0000")
> + //
> + // Reserve 1 more byte for ACPI HID
> + //
> + Noop
> +
> + Name (_CID, "MSFT0101")
> +
> //
> // Readable name of this device, don't know if this way is correct yet
> //
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
2016-12-27 3:30 ` Zhang, Chao B
2016-12-27 3:35 ` Yao, Jiewen
@ 2016-12-27 4:38 ` Zeng, Star
1 sibling, 0 replies; 6+ messages in thread
From: Zeng, Star @ 2016-12-27 4:38 UTC (permalink / raw)
To: Zhang, Chao B, edk2-devel@lists.01.org; +Cc: Yao, Jiewen, star.zeng
On 2016/12/27 11:30, Zhang, Chao B wrote:
> Star:
> Thanks for your comments.
>
> 1. For error handling, ManufacturerID & Version failure is not considered to cause fatal break. We can still use default "NNN0000" value.
I mean why still to update the default _HID value in TPM.asl if getting
ManufacturerID or Version is failed?
>
> 2. I will update the comments for UpdateHID
>
> 3. EFI_NOT_FOUND is not expected here, but I think it is reasonable error handling if user override TPM.asl.
If user override TPM.asl, for example the default _HID value in TPM.asl,
shouldn't the value of TPM_HID_TAG be updated to match the default _HID
value in TPM.asl? If not, the UpdateHID() function couldn't work as the
for loop couldn't find the tag. If yes, how does the code run to "return
EFI_NOT_FOUND"?
Anyway, why does the UpdateHID() function need return status?
Thanks,
Star
>
>
>
>
> Thanks & Best regards
> Chao Zhang
>
> From: Zeng, Star
> Sent: Tuesday, December 27, 2016 11:19 AM
> To: Zhang, Chao B; edk2-devel@lists.01.org
> Cc: Yao, Jiewen; Zeng, Star
> Subject: Re: [edk2] [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID
>
> Chao,
>
> Minor comments inline.
>
> On 2016/12/27 9:51, Zhang, Chao B wrote:
>> Update TPM2 HID using vendor ManufacturerID & FirmwareVersion1.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Chao Zhang <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
>> ---
>> .../Library/Tpm2CommandLib/Tpm2Capability.c | 4 +-
>> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 102 +++++++++++++++++++++
>> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h | 10 +-
>> SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf | 2 +-
>> SecurityPkg/Tcg/Tcg2Smm/Tpm.asl | 10 +-
>> 5 files changed, 122 insertions(+), 6 deletions(-)
>>
>> diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
>> index 9aab17f..79e80fb 100644
>> --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
>> +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2Capability.c
>> @@ -1,7 +1,7 @@
>> /** @file
>> Implement TPM2 Capability related command.
>>
>> -Copyright (c) 2013, Intel Corporation. All rights reserved. <BR>
>> +Copyright (c) 2013 - 2016, 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
>> @@ -189,7 +189,7 @@ Tpm2GetCapabilityManufactureID (
>> if (EFI_ERROR (Status)) {
>> return Status;
>> }
>> - *ManufactureId = SwapBytes32 (TpmCap.data.tpmProperties.tpmProperty->value);
>> + *ManufactureId = TpmCap.data.tpmProperties.tpmProperty->value;
>>
>> return EFI_SUCCESS;
>> }
>> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
>> index d02123d..addb302 100644
>> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
>> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
>> @@ -303,6 +303,103 @@ UpdatePPVersion (
>> }
>>
>> /**
>> + Patch TPM2 device HID string. The initial string tag in TPM2 ACPI table is "NNN0000".
>> +
>> + @param[in, out] Table The TPM2 SSDT ACPI table.
>> +
>> + @return HID Update status.
>> +
>> +**/
>> +EFI_STATUS
>> +UpdateHID (
>> + EFI_ACPI_DESCRIPTION_HEADER *Table
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UINT8 *DataPtr;
>> + CHAR8 HID[TPM_HID_ACPI_SIZE];
>> + UINT32 ManufacturerID;
>> + UINT32 FirmwareVersion1;
>> + UINT32 FirmwareVersion2;
>> + BOOLEAN PnpHID;
>> +
>> + PnpHID = TRUE;
>> +
>> + //
>> + // Initialize HID with Default PNP string
>> + //
>> + ZeroMem(HID, TPM_HID_ACPI_SIZE);
>> + CopyMem(HID, TPM_HID_TAG, TPM_HID_PNP_SIZE);
>> +
>> + //
>> + // Get Manufacturer ID
>> + //
>> + Status = Tpm2GetCapabilityManufactureID(&ManufacturerID);
>> + if (!EFI_ERROR(Status)) {
>> + DEBUG((EFI_D_INFO, "TPM_PT_VENDOR_STRING_1 0x%08x\n", ManufacturerID));
>> + //
>> + // ManfacturerID defined in TCG Vendor ID Registry
>> + // may tailed with 0x00 or 0x20
>> + //
>> + if ((ManufacturerID >> 24) == 0x00 || ((ManufacturerID >> 24) == 0x20)) {
>> + //
>> + // HID containing PNP ID "NNN####"
>> + // NNN is uppercase letter for Vendor ID specified by manufacturer
>> + //
>> + CopyMem(HID, &ManufacturerID, 3);
>> + } else {
>> + //
>> + // HID containing ACP ID "NNNN####"
>> + // NNNN is uppercase letter for Vendor ID specified by manufacturer
>> + //
>> + CopyMem(HID, &ManufacturerID, 4);
>> + PnpHID = FALSE;
>> + }
>> + } else {
>> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_VENDOR_STRING_1 failed %x!\n", Status));
>
> Should just return failure here?
>
>> + }
>> +
>> + Status = Tpm2GetCapabilityFirmwareVersion(&FirmwareVersion1, &FirmwareVersion2);
>> + if (!EFI_ERROR(Status)) {
>> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_1 0x%x\n", FirmwareVersion1));
>> + DEBUG((EFI_D_INFO, "TPM_PT_FIRMWARE_VERSION_2 0x%x\n", FirmwareVersion2));
>> + //
>> + // #### is Firmware Version 1
>> + //
>> + if (PnpHID) {
>> + AsciiSPrint(HID + 3, TPM_HID_PNP_SIZE - 3, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
>> + } else {
>> + AsciiSPrint(HID + 4, TPM_HID_ACPI_SIZE - 4, "%02d%02d", ((FirmwareVersion1 & 0xFFFF0000) >> 16), (FirmwareVersion1 && 0x0000FFFF));
>> + }
>> +
>> + } else {
>> + DEBUG ((EFI_D_ERROR, "Get TPM_PT_FIRMWARE_VERSION_X failed %x!\n", Status));
>
> Should just return failure here?
>
>> + }
>> +
>> + //
>> + // Patch HID in ASL code before loading the SSDT.
>> + //
>> + for (DataPtr = (UINT8 *)(Table + 1);
>> + DataPtr <= (UINT8 *) ((UINT8 *) Table + Table->Length - TPM_HID_PNP_SIZE);
>> + DataPtr += 1) {
>> + if (AsciiStrCmp((CHAR8 *)DataPtr, TPM_HID_TAG) == 0) {
>> + if (PnpHID) {
>> + CopyMem(DataPtr, HID, TPM_HID_PNP_SIZE);
>> + } else {
>> + //
>> + // NOOP will be patched to '\0'
>> + //
>> + CopyMem(DataPtr, HID, TPM_HID_ACPI_SIZE);
>> + }
>> + DEBUG((EFI_D_INFO, "TPM2 ACPI _HID updated to %a\n", HID));
>> + return Status;
>> + }
>> + }
>> +
>> + return EFI_NOT_FOUND;
>
> Since the for loop above can find out TPM_HID_TAG definitly,
> can the code run to here according to current code logic?
>
>> +}
>> +
>> +/**
>> Initialize and publish TPM items in ACPI table.
>>
>> @retval EFI_SUCCESS The TCG ACPI table is published successfully.
>> @@ -336,6 +433,11 @@ PublishAcpiTable (
>> ASSERT_EFI_ERROR (Status);
>>
>> //
>> + // Update Table version before measuring it to PCR
>
> Does the comment here need to be updated?
>
> Thanks,
> Star
>
>> + //
>> + Status = UpdateHID(Table);
>> +
>> + //
>> // Measure to PCR[0] with event EV_POST_CODE ACPI DATA
>> //
>> TpmMeasureAndLogData(
>> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
>> index 0b09032..18e8bfc 100644
>> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
>> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.h
>> @@ -35,9 +35,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> #include <Library/UefiBootServicesTableLib.h>
>> #include <Library/DxeServicesLib.h>
>> #include <Library/TpmMeasurementLib.h>
>> -#include <Library/Tpm2DeviceLib.h>
>> +#include <Library/Tpm2CommandLib.h>
>> #include <Library/Tcg2PhysicalPresenceLib.h>
>> #include <Library/IoLib.h>
>> +#include <Library/PrintLib.h>
>>
>> #include <IndustryStandard/TpmPtp.h>
>>
>> @@ -94,4 +95,11 @@ typedef struct {
>> #define PHYSICAL_PRESENCE_VERSION_TAG "$PV"
>> #define PHYSICAL_PRESENCE_VERSION_SIZE 4
>>
>> +//
>> +// PNP _HID for TPM2 device
>> +//
>> +#define TPM_HID_TAG "NNN0000"
>> +#define TPM_HID_PNP_SIZE 8
>> +#define TPM_HID_ACPI_SIZE 9
>> +
>> #endif // __TCG_SMM_H__
>> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
>> index 0de4fce..8c823d6 100644
>> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
>> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf
>> @@ -48,7 +48,7 @@
>> DebugLib
>> DxeServicesLib
>> TpmMeasurementLib
>> - Tpm2DeviceLib
>> + Tpm2CommandLib
>> Tcg2PhysicalPresenceLib
>> IoLib
>>
>> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
>> index 2083a3e..4128684 100644
>> --- a/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
>> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tpm.asl
>> @@ -30,8 +30,14 @@ DefinitionBlock (
>> //
>> // TCG2
>> //
>> - Name (_HID, "MSFT0101")
>> -
>> + Name (_HID, "NNN0000")
>> + //
>> + // Reserve 1 more byte for ACPI HID
>> + //
>> + Noop
>> +
>> + Name (_CID, "MSFT0101")
>> +
>> //
>> // Readable name of this device, don't know if this way is correct yet
>> //
>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-27 4:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-27 1:51 [PATCH] SecurityPkg: Tcg2Smm: TPM2 Vendor specific HID Zhang, Chao B
2016-12-27 2:13 ` Yao, Jiewen
2016-12-27 3:18 ` Zeng, Star
2016-12-27 3:30 ` Zhang, Chao B
2016-12-27 3:35 ` Yao, Jiewen
2016-12-27 4:38 ` Zeng, Star
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox