* [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support
@ 2018-03-07 15:57 marcandre.lureau
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask marcandre.lureau
` (8 more replies)
0 siblings, 9 replies; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Hi,
The following series adds basic TPM2 support for OVMF-on-QEMU (I
haven't tested TPM1, for lack of interest). It links with the modules
to initializes the device in PEI phase, and do measurements (both PEI
and DXE). The Tcg2Dxe module provides the Tcg2 protocol which allows
the guest to access the measurement log and other facilities.
DxeTpm2MeasureBootLib seems to do its job at measuring images that are
not measured in PEI phase (such as PCI PXE rom)
Tcg2ConfigDxe is not included due to its integration with edk2 own PPI
implementation which conflicts with qemu design. PPI design is still
being discussed & experimented at this point.
Linux guests seem to work fine. But windows guest generally complains
about the lack of PPI interface (most HLK tests require it, tpm.msc
admin interactions too). I haven't done "real" use-cases tests, as I
lack experience with TPM usage. Any help appreciated to test the TPM.
I build edk2 with:
$ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE -DMEM_VARSTORE_EMU_ENABLE=FALSE
I test with qemu & swtpm/libtpms (tpm2 branches, swtpm_setup.sh --tpm2 --tpm-state tpmstatedir)
$ swtpm socket --tpmstate tpmstatedir --ctrl type=unixio,path=tpmsock --tpm2 &
$ qemu .. -chardev socket,id=chrtpm,path=tpmsock -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
Thanks
Github tree:
https://github.com/elmarco/edk2/tree/tpm2 (tpm2-v2 tag)
Related bug:
https://bugzilla.tianocore.org/show_bug.cgi?id=594
v2:
- the series can now be applied to master directly, thanks to dropping
PeiReadOnlyVariable requirement
- remove the HOB list workaround, the main fix is now upstream. Add a
preliminary patch to complete it.
- removed traces of TPM1.2 support
- add own OvmfPkg Tcg2ConfigPei, which performs only TPM2 detection
- make PcdTpmInstanceGuid default all-bits-zero
- drop unneeded Pcd values
- explain why SHA1 is still nice to have (for 1.2 log format)
- drop Tcg2ConfigDxe
- more detailed commit messages, thanks to Laszlo explanations!
- rebased
TODO:
- modify Ia32 and Ia32X64 builds
Marc-André Lureau (8):
SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
MdeModulePkg: fix REGISITER -> REGISTER
ovmf: simplify SecurityStubDxe.inf inclusion
ovmf: add OvmfPkg Tcg2ConfigPei module
ovmf: link with Tcg2Pei module
ovmf: link with Tcg2Dxe module
ovmf: add DxeTpm2MeasureBootLib
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +--
MdeModulePkg/Core/Pei/Image/Image.c | 4 +-
MdeModulePkg/Core/Pei/PeiMain.h | 2 +-
MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 2 +-
OvmfPkg/OvmfPkgIa32.dsc | 6 +-
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 49 ++++++-
OvmfPkg/OvmfPkgX64.fdf | 9 ++
OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 57 ++++++++
OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 124 ++++++++++++++++++
OvmfPkg/Tcg/Tcg2Config/TpmDetection.c | 46 +++++++
.../HashLibBaseCryptoRouterPei.c | 1 +
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
13 files changed, 299 insertions(+), 26 deletions(-)
create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
create mode 100644 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
--
2.16.2.346.g9779355e34
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-08 0:35 ` Zhang, Chao B
2018-03-08 11:40 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex marcandre.lureau
` (7 subsequent siblings)
8 siblings, 2 replies; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau, Chao Zhang, Star Zeng
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Commit 4cc2b63bd829426b05bad0d8952f1855a10d6ed7 fixed an out of bounds
ZeroMem() call. However, as Laszlo Ersek pointed out, the intent was
to clear all but the Identifier (to revert the effect of
RegisterHashInterfaceLib()). For that, it should clear the
SupportedHashMask too.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
.../Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index 361a4f6508a0..bf6e1336ee76 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
@@ -426,6 +426,7 @@ HashLibBaseCryptoRouterPeiConstructor (
//
ZeroMem (&HashInterfaceHob->HashInterface, sizeof (HashInterfaceHob->HashInterface));
HashInterfaceHob->HashInterfaceCount = 0;
+ HashInterfaceHob->SupportedHashMask = 0;
}
//
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-07 16:04 ` Yao, Jiewen
` (2 more replies)
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER marcandre.lureau
` (6 subsequent siblings)
8 siblings, 3 replies; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The module doesn't use read-only variable.
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
1 file changed, 1 deletion(-)
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
index bc910c3baf97..a4aae1488ff8 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
@@ -91,7 +91,6 @@ [Pcd]
[Depex]
gEfiPeiMasterBootModePpiGuid AND
- gEfiPeiReadOnlyVariable2PpiGuid AND
gEfiTpmDeviceSelectedGuid
[UserExtensions.TianoCore."ExtraFiles"]
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask marcandre.lureau
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-08 11:59 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 4/8] ovmf: simplify SecurityStubDxe.inf inclusion marcandre.lureau
` (5 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +++++++++---------
MdeModulePkg/Core/Pei/Image/Image.c | 4 ++--
MdeModulePkg/Core/Pei/PeiMain.h | 2 +-
MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 2 +-
4 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
index 79f2e5cebcbe..027176d872c7 100644
--- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
+++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
@@ -970,7 +970,7 @@ PeiDispatcher (
if ((Private->PeiMemoryInstalled) && (Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
//
// Once real memory is available, shadow the RegisterForShadow modules. And meanwhile
- // update the modules' status from PEIM_STATE_REGISITER_FOR_SHADOW to PEIM_STATE_DONE.
+ // update the modules' status from PEIM_STATE_REGISTER_FOR_SHADOW to PEIM_STATE_DONE.
//
SaveCurrentPeimCount = Private->CurrentPeimCount;
SaveCurrentFvCount = Private->CurrentPeimFvCount;
@@ -978,7 +978,7 @@ PeiDispatcher (
for (Index1 = 0; Index1 <= SaveCurrentFvCount; Index1++) {
for (Index2 = 0; (Index2 < PcdGet32 (PcdPeiCoreMaxPeimPerFv)) && (Private->Fv[Index1].FvFileHandles[Index2] != NULL); Index2++) {
- if (Private->Fv[Index1].PeimState[Index2] == PEIM_STATE_REGISITER_FOR_SHADOW) {
+ if (Private->Fv[Index1].PeimState[Index2] == PEIM_STATE_REGISTER_FOR_SHADOW) {
PeimFileHandle = Private->Fv[Index1].FvFileHandles[Index2];
Private->CurrentFileHandle = PeimFileHandle;
Private->CurrentPeimFvCount = Index1;
@@ -986,13 +986,13 @@ PeiDispatcher (
Status = PeiLoadImage (
(CONST EFI_PEI_SERVICES **) &Private->Ps,
PeimFileHandle,
- PEIM_STATE_REGISITER_FOR_SHADOW,
+ PEIM_STATE_REGISTER_FOR_SHADOW,
&EntryPoint,
&AuthenticationState
);
if (Status == EFI_SUCCESS) {
//
- // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
+ // PEIM_STATE_REGISTER_FOR_SHADOW move to PEIM_STATE_DONE
//
Private->Fv[Index1].PeimState[Index2]++;
//
@@ -1165,7 +1165,7 @@ PeiDispatcher (
//
PeiCheckAndSwitchStack (SecCoreData, Private);
- if ((Private->PeiMemoryInstalled) && (Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISITER_FOR_SHADOW) && \
+ if ((Private->PeiMemoryInstalled) && (Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW) && \
(Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
//
// If memory is available we shadow images by default for performance reasons.
@@ -1179,7 +1179,7 @@ PeiDispatcher (
Status = PeiLoadImage (
PeiServices,
PeimFileHandle,
- PEIM_STATE_REGISITER_FOR_SHADOW,
+ PEIM_STATE_REGISTER_FOR_SHADOW,
&EntryPoint,
&AuthenticationState
);
@@ -1192,7 +1192,7 @@ PeiDispatcher (
//PERF_END (PeiServices, L"PEIM", PeimFileHandle, 0);
//
- // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
+ // PEIM_STATE_REGISTER_FOR_SHADOW move to PEIM_STATE_DONE
//
Private->Fv[FvCount].PeimState[PeimCount]++;
@@ -1356,14 +1356,14 @@ PeiRegisterForShadow (
return EFI_NOT_FOUND;
}
- if (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] >= PEIM_STATE_REGISITER_FOR_SHADOW) {
+ if (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] >= PEIM_STATE_REGISTER_FOR_SHADOW) {
//
// If the PEIM has already entered the PEIM_STATE_REGISTER_FOR_SHADOW or PEIM_STATE_DONE then it's already been started
//
return EFI_ALREADY_STARTED;
}
- Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] = PEIM_STATE_REGISITER_FOR_SHADOW;
+ Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] = PEIM_STATE_REGISTER_FOR_SHADOW;
return EFI_SUCCESS;
}
diff --git a/MdeModulePkg/Core/Pei/Image/Image.c b/MdeModulePkg/Core/Pei/Image/Image.c
index f41d3acac77e..f07f48823117 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -387,7 +387,7 @@ LoadAndRelocatePeCoffImage (
}
IsRegisterForShadow = FALSE;
if ((Private->CurrentFileHandle == FileHandle)
- && (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] == PEIM_STATE_REGISITER_FOR_SHADOW)) {
+ && (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW)) {
IsRegisterForShadow = TRUE;
}
@@ -876,7 +876,7 @@ PeiLoadImage (
//
// The shadowed PEIM must be relocatable.
//
- if (PeimState == PEIM_STATE_REGISITER_FOR_SHADOW) {
+ if (PeimState == PEIM_STATE_REGISTER_FOR_SHADOW) {
IsStrip = RelocationIsStrip ((VOID *) (UINTN) ImageAddress);
ASSERT (!IsStrip);
if (IsStrip) {
diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
index fef3753e4b3b..a1cdbc15d98a 100644
--- a/MdeModulePkg/Core/Pei/PeiMain.h
+++ b/MdeModulePkg/Core/Pei/PeiMain.h
@@ -104,7 +104,7 @@ typedef struct {
//
#define PEIM_STATE_NOT_DISPATCHED 0x00
#define PEIM_STATE_DISPATCHED 0x01
-#define PEIM_STATE_REGISITER_FOR_SHADOW 0x02
+#define PEIM_STATE_REGISTER_FOR_SHADOW 0x02
#define PEIM_STATE_DONE 0x03
typedef struct {
diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
index 3cd61906c3df..775bf18ce938 100644
--- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
+++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
@@ -104,7 +104,7 @@ ShadowPeiCore (
Status = PeiLoadImage (
GetPeiServicesTablePointer (),
*((EFI_PEI_FILE_HANDLE*)&PeiCoreFileHandle),
- PEIM_STATE_REGISITER_FOR_SHADOW,
+ PEIM_STATE_REGISTER_FOR_SHADOW,
&EntryPoint,
&AuthenticationState
);
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 4/8] ovmf: simplify SecurityStubDxe.inf inclusion
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
` (2 preceding siblings ...)
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-08 16:35 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module marcandre.lureau
` (4 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
SecurityStubDxe.inf should be included unconditionally.
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
OvmfPkg/OvmfPkgIa32.dsc | 6 ++----
OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++----
OvmfPkg/OvmfPkgX64.dsc | 6 ++----
3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index fbe0f790e431..5bd3f4f977df 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -611,14 +611,12 @@ [Components]
MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
<LibraryClasses>
+!if $(SECURE_BOOT_ENABLE) == TRUE
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
- }
-!else
- MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
!endif
+ }
MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index fb10e0b0f2e4..7dded86c4940 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -620,14 +620,12 @@ [Components.X64]
MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
<LibraryClasses>
+!if $(SECURE_BOOT_ENABLE) == TRUE
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
- }
-!else
- MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
!endif
+ }
MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a31551f5ae24..a8e89276c0b2 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -618,14 +618,12 @@ [Components]
MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-!if $(SECURE_BOOT_ENABLE) == TRUE
MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
<LibraryClasses>
+!if $(SECURE_BOOT_ENABLE) == TRUE
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
- }
-!else
- MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
!endif
+ }
MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
` (3 preceding siblings ...)
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 4/8] ovmf: simplify SecurityStubDxe.inf inclusion marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-08 17:46 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 6/8] ovmf: link with Tcg2Pei module marcandre.lureau
` (3 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The Tcg2ConfigPei module informs the firmware globally about the TPM
device type, by setting the PcdTpmInstanceGuid PCD to the appropriate
GUID value. The original module under SecurityPkg can perform device
detection, or read a cached value from a non-volatile UEFI variable.
OvmfPkg's clone of the module only performs the TPM2 hardware detection.
This is what the module does:
- Check the QEMU hardware for TPM2 availability only
- If found, set the dynamic PCD "PcdTpmInstanceGuid" to
&gEfiTpmDeviceInstanceTpm20DtpmGuid. This is what informs the rest of
the firmware about the TPM type.
- Install the gEfiTpmDeviceSelectedGuid PPI. This action permits the
PEI_CORE to dispatch the Tcg2Pei module, which consumes the above PCD.
In effect, the gEfiTpmDeviceSelectedGuid PPI serializes the setting
and the consumption of the "TPM type" PCD.
- If no TPM2 was found, install gPeiTpmInitializationDonePpiGuid.
(Normally this is performed by Tcg2Pei, but Tcg2Pei doesn't do it if
no TPM2 is available. So in that case our Tcg2ConfigPei must do it.)
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
OvmfPkg/OvmfPkgX64.dsc | 17 ++++
OvmfPkg/OvmfPkgX64.fdf | 4 +
OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 57 +++++++++++
OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 124 +++++++++++++++++++++++
OvmfPkg/Tcg/Tcg2Config/TpmDetection.c | 46 +++++++++
5 files changed, 248 insertions(+)
create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
create mode 100644 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index a8e89276c0b2..64bd6b6a9f08 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -39,6 +39,7 @@ [Defines]
DEFINE HTTP_BOOT_ENABLE = FALSE
DEFINE SMM_REQUIRE = FALSE
DEFINE TLS_ENABLE = FALSE
+ DEFINE TPM2_ENABLE = FALSE
#
# Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
@@ -208,6 +209,10 @@ [LibraryClasses]
OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
+!if $(TPM2_ENABLE) == TRUE
+ Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
+!endif
+
[LibraryClasses.common]
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -272,6 +277,10 @@ [LibraryClasses.common.PEIM]
PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
+!if $(TPM2_ENABLE)
+ Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
+!endif
+
[LibraryClasses.common.DXE_CORE]
HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
@@ -554,6 +563,10 @@ [PcdsDynamicDefault]
gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
+!if $(TPM2_ENABLE) == TRUE
+ gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
+!endif
+
################################################################################
#
# Components Section - list of all EDK II Modules needed by this Platform.
@@ -600,6 +613,10 @@ [Components]
!endif
UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+!if $(TPM2_ENABLE) == TRUE
+ OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+!endif
+
#
# DXE Phase modules
#
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 2fc17810eb23..dbafada5226b 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -165,6 +165,10 @@ [FV.PEIFV]
!endif
INF UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+!if $(TPM2_ENABLE) == TRUE
+INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+!endif
+
################################################################################
[FV.DXEFV]
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
new file mode 100644
index 000000000000..201e4f24d5f4
--- /dev/null
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -0,0 +1,57 @@
+## @file
+# Set TPM device type
+#
+# This module initializes TPM device type based on variable and detection.
+# This OvmfPkg of SecurityPkg only does TPM2 detection.
+#
+# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+# Copyright (C) 2018, Red Hat, Inc.
+#
+# This program and the accompanying materials
+# are licensed and made available under the terms and conditions of the BSD License
+# which accompanies this distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = Tcg2ConfigPei
+ FILE_GUID = BF7F2B0C-9F2F-4889-AB5C-12460022BE87
+ MODULE_TYPE = PEIM
+ VERSION_STRING = 1.0
+ ENTRY_POINT = Tcg2ConfigPeimEntryPoint
+
+[Sources]
+ Tcg2ConfigPeim.c
+ TpmDetection.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ SecurityPkg/SecurityPkg.dec
+
+[LibraryClasses]
+ PeimEntryPoint
+ DebugLib
+ Tpm2DeviceLib
+
+[Guids]
+ ## SOMETIMES_CONSUMES ## Variable:L"TCG2_CONFIGURATION"
+ ## SOMETIMES_CONSUMES ## Variable:L"TCG2_DEVICE_DETECTION"
+ gTcg2ConfigFormSetGuid
+ gEfiTpmDeviceSelectedGuid ## PRODUCES ## GUID # Used as a PPI GUID
+ gEfiTpmDeviceInstanceNoneGuid ## SOMETIMES_CONSUMES ## GUID # TPM device identifier
+
+[Ppis]
+ gPeiTpmInitializationDonePpiGuid ## SOMETIMES_PRODUCES
+
+[Pcd]
+ gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ## PRODUCES
+ gEfiSecurityPkgTokenSpaceGuid.PcdTpmAutoDetection ## CONSUMES
+ gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress ## SOMETIMES_CONSUMES
+
+[Depex]
+ gEfiPeiMasterBootModePpiGuid
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
new file mode 100644
index 000000000000..31f27968401b
--- /dev/null
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
@@ -0,0 +1,124 @@
+/** @file
+ The module entry point for Tcg2 configuration module.
+
+Copyright (c) 2018, Red Hat, Inc.
+Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
+
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution. The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+
+#include <PiPei.h>
+
+#include <Guid/TpmInstance.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PeiServicesLib.h>
+#include <Tcg/Tcg2Config/Tcg2ConfigNvData.h>
+
+TPM_INSTANCE_ID mTpmInstanceId[] = TPM_INSTANCE_ID_LIST;
+
+CONST EFI_PEI_PPI_DESCRIPTOR gTpmSelectedPpi = {
+ (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
+ &gEfiTpmDeviceSelectedGuid,
+ NULL
+};
+
+EFI_PEI_PPI_DESCRIPTOR mTpmInitializationDonePpiList = {
+ EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+ &gPeiTpmInitializationDonePpiGuid,
+ NULL
+};
+
+/**
+ This routine check both SetupVariable and real TPM device, and return final TpmDevice configuration.
+
+ @param SetupTpmDevice TpmDevice configuration in setup driver
+
+ @return TpmDevice configuration
+**/
+UINT8
+DetectTpmDevice (
+ IN UINT8 SetupTpmDevice
+ );
+
+/**
+ The entry point for Tcg2 configuration driver.
+
+ @param FileHandle Handle of the file being invoked.
+ @param PeiServices Describes the list of possible PEI Services.
+
+ @retval EFI_SUCCES Convert variable to PCD successfully.
+ @retval Others Fail to convert variable to PCD.
+**/
+EFI_STATUS
+EFIAPI
+Tcg2ConfigPeimEntryPoint (
+ IN EFI_PEI_FILE_HANDLE FileHandle,
+ IN CONST EFI_PEI_SERVICES **PeiServices
+ )
+{
+ UINTN Size;
+ EFI_STATUS Status;
+ EFI_STATUS Status2;
+ TCG2_CONFIGURATION Tcg2Configuration;
+ UINTN Index;
+ UINT8 TpmDevice;
+
+ Tcg2Configuration.TpmDevice = TPM_DEVICE_2_0_DTPM;
+
+ DEBUG ((EFI_D_INFO, "OvmfPkg Tcg2ConfigPeimEntryPoint"));
+
+ if (PcdGetBool (PcdTpmAutoDetection)) {
+ TpmDevice = DetectTpmDevice (Tcg2Configuration.TpmDevice);
+ DEBUG ((EFI_D_INFO, "TpmDevice final: %x\n", TpmDevice));
+ if (TpmDevice != TPM_DEVICE_NULL) {
+ Tcg2Configuration.TpmDevice = TpmDevice;
+ }
+ } else {
+ TpmDevice = Tcg2Configuration.TpmDevice;
+ }
+
+ //
+ // Convert variable to PCD.
+ // This is work-around because there is no gurantee DynamicHiiPcd can return correct value in DXE phase.
+ // Using DynamicPcd instead.
+ //
+ // NOTE: Tcg2Configuration variable contains the desired TpmDevice type,
+ // while PcdTpmInstanceGuid PCD contains the real detected TpmDevice type
+ //
+ for (Index = 0; Index < sizeof(mTpmInstanceId)/sizeof(mTpmInstanceId[0]); Index++) {
+ if (TpmDevice == mTpmInstanceId[Index].TpmDevice) {
+ Size = sizeof(mTpmInstanceId[Index].TpmInstanceGuid);
+ Status = PcdSetPtrS (PcdTpmInstanceGuid, &Size, &mTpmInstanceId[Index].TpmInstanceGuid);
+ ASSERT_EFI_ERROR (Status);
+ DEBUG ((EFI_D_INFO, "TpmDevice PCD: %g\n", &mTpmInstanceId[Index].TpmInstanceGuid));
+ break;
+ }
+ }
+
+ //
+ // Selection done
+ //
+ Status = PeiServicesInstallPpi (&gTpmSelectedPpi);
+ ASSERT_EFI_ERROR (Status);
+
+ //
+ // Even if no TPM is selected or detected, we still need intall TpmInitializationDonePpi.
+ // Because TcgPei or Tcg2Pei will not run, but we still need a way to notify other driver.
+ // Other driver can know TPM initialization state by TpmInitializedPpi.
+ //
+ if (CompareGuid (PcdGetPtr(PcdTpmInstanceGuid), &gEfiTpmDeviceInstanceNoneGuid)) {
+ Status2 = PeiServicesInstallPpi (&mTpmInitializationDonePpiList);
+ ASSERT_EFI_ERROR (Status2);
+ }
+
+ return Status;
+}
diff --git a/OvmfPkg/Tcg/Tcg2Config/TpmDetection.c b/OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
new file mode 100644
index 000000000000..df222cbff13d
--- /dev/null
+++ b/OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
@@ -0,0 +1,46 @@
+/** @file
+ TPM2.0 auto detection.
+
+Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
+Copyright (C) 2018, Red Hat, Inc.
+
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution. The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+
+#include <PiPei.h>
+
+#include <Library/DebugLib.h>
+#include <Library/Tpm2DeviceLib.h>
+#include <Tcg/Tcg2Config/Tcg2ConfigNvData.h>
+
+/**
+ This routine check both SetupVariable and real TPM device, and return final TpmDevice configuration.
+
+ @param SetupTpmDevice TpmDevice configuration in setup driver
+
+ @return TpmDevice configuration
+**/
+UINT8
+DetectTpmDevice (
+ IN UINT8 SetupTpmDevice
+ )
+{
+ EFI_STATUS Status;
+
+ DEBUG ((EFI_D_INFO, "DetectTpmDevice:\n"));
+
+ Status = Tpm2RequestUseTpm ();
+ if (EFI_ERROR (Status)) {
+ return TPM_DEVICE_NULL;
+ }
+
+ return TPM_DEVICE_2_0_DTPM;
+}
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 6/8] ovmf: link with Tcg2Pei module
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
` (4 preceding siblings ...)
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-08 18:20 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 7/8] ovmf: link with Tcg2Dxe module marcandre.lureau
` (2 subsequent siblings)
8 siblings, 1 reply; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This module will initialize TPM device, measure reported FVs and BIOS
version. We keep both SHA-1 and SHA-256 for the TCG 1.2 log format
compatibility, but the SHA-256 measurements and TCG 2 log format are
now recommended.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
OvmfPkg/OvmfPkgX64.dsc | 7 +++++++
OvmfPkg/OvmfPkgX64.fdf | 1 +
2 files changed, 8 insertions(+)
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 64bd6b6a9f08..3fa1a31f4c37 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -278,6 +278,8 @@ [LibraryClasses.common.PEIM]
QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
!if $(TPM2_ENABLE)
+ BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+ HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
!endif
@@ -615,6 +617,11 @@ [Components]
!if $(TPM2_ENABLE) == TRUE
OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+ SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
+ <LibraryClasses>
+ NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+ NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
+ }
!endif
#
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index dbafada5226b..c0173e7adf5f 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -167,6 +167,7 @@ [FV.PEIFV]
!if $(TPM2_ENABLE) == TRUE
INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
!endif
################################################################################
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 7/8] ovmf: link with Tcg2Dxe module
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
` (5 preceding siblings ...)
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 6/8] ovmf: link with Tcg2Pei module marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-08 19:14 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib marcandre.lureau
2018-03-08 12:31 ` [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support Shi, Steven
8 siblings, 1 reply; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
This module measures and log the boot environment. It also produces
the Tcg2 protocol, which allows for example to read the log from OS.
The linux kernel doesn't yet read the EFI_TCG2_EVENT_LOG_FORMAT_TCG_2,
which is required for crypto-agile log. In fact, only upcoming 4.16
adds support EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2:
[ 0.000000] efi: EFI v2.70 by EDK II
[ 0.000000] efi: SMBIOS=0x3fa1f000 ACPI=0x3fbb6000 ACPI 2.0=0x3fbb6014 MEMATTR=0x3e7d4318 TPMEventLog=0x3db21018
$ python chipsec_util.py tpm parse_log binary_bios_measurements
[CHIPSEC] Version 1.3.5.dev2
[CHIPSEC] API mode: using OS native API (not using CHIPSEC kernel module)
[CHIPSEC] Executing command 'tpm' with args ['parse_log', '/tmp/binary_bios_measurements']
PCR: 0 type: EV_S_CRTM_VERSION size: 0x2 digest: 1489f923c4dca729178b3e3233458550d8dddf29
+ version:
PCR: 0 type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10 digest: fd39ced7c0d2a61f6830c78c7625f94826b05bcc
+ base: 0x820000 length: 0xe0000
PCR: 0 type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10 digest: 39ebc6783b72bc1e73c7d5bcfeb5f54a3f105d4c
+ base: 0x900000 length: 0xa00000
PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x35 digest: 57cd4dc19442475aa82743484f3b1caa88e142b8
PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24 digest: 9b1387306ebb7ff8e795e7be77563666bbf4516e
PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26 digest: 9afa86c507419b8570c62167cb9486d9fc809758
PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24 digest: 5bf8faa078d40ffbd03317c93398b01229a0e1e0
PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26 digest: 734424c9fe8fc71716c42096f4b74c88733b175e
PCR: 7 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x3e digest: 252f8ebb85340290b64f4b06a001742be8e5cab6
PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x6e digest: 22a4f6ee9af6dba01d3528deb64b74b582fc182b
PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x80 digest: b7811d5bf30a7efd4e385c6179fe10d9290bb9e8
PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x84 digest: 425e502c24fc924e231e0a62327b6b7d1f704573
PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x9a digest: 0b5d2c98ac5de6148a4a1490ff9d5df69039f04e
PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0xbd digest: 20bd5f402271d57a88ea314fe35c1705956b1f74
PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x88 digest: df5d6605cb8f4366d745a8464cfb26c1efdc305c
PCR: 4 type: EV_EFI_ACTION size: 0x28 digest: cd0fdb4531a6ec41be2753ba042637d6e5f7f256
PCR: 0 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
PCR: 1 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
PCR: 2 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
PCR: 3 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
PCR: 4 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
PCR: 5 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
$ tpm2_pcrlist
sha1 :
0 : 35bd1786b6909daad610d7598b1d620352d33b8a
1 : ec0511e860206e0af13c31da2f9e943fb6ca353d
2 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
3 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
4 : 45a323382bd933f08e7f0e256bc8249e4095b1ec
5 : d16d7e629fd8d08ca256f9ad3a3a1587c9e6cc1b
6 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
7 : 518bd167271fbb64589c61e43d8c0165861431d8
8 : 0000000000000000000000000000000000000000
9 : 0000000000000000000000000000000000000000
10 : 0000000000000000000000000000000000000000
11 : 0000000000000000000000000000000000000000
12 : 0000000000000000000000000000000000000000
13 : 0000000000000000000000000000000000000000
14 : 0000000000000000000000000000000000000000
15 : 0000000000000000000000000000000000000000
16 : 0000000000000000000000000000000000000000
17 : ffffffffffffffffffffffffffffffffffffffff
18 : ffffffffffffffffffffffffffffffffffffffff
19 : ffffffffffffffffffffffffffffffffffffffff
20 : ffffffffffffffffffffffffffffffffffffffff
21 : ffffffffffffffffffffffffffffffffffffffff
22 : ffffffffffffffffffffffffffffffffffffffff
23 : 0000000000000000000000000000000000000000
sha256 :
0 : 9ae903dbae3357ac00d223660bac19ea5c021499a56201104332ab966631ce2c
1 : acc611d90245cf04e77b0ca94901f90e7fa54770f0426f53c3049b532243d1b8
2 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
3 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
4 : 7a94ffe8a7729a566d3d3c577fcb4b6b1e671f31540375f80eae6382ab785e35
5 : a5ceb755d043f32431d63e39f5161464620a3437280494b5850dc1b47cc074e0
6 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
7 : 65caf8dd1e0ea7a6347b635d2b379c93b9a1351edc2afc3ecda700e534eb3068
8 : 0000000000000000000000000000000000000000000000000000000000000000
9 : 0000000000000000000000000000000000000000000000000000000000000000
10 : 0000000000000000000000000000000000000000000000000000000000000000
11 : 0000000000000000000000000000000000000000000000000000000000000000
12 : 0000000000000000000000000000000000000000000000000000000000000000
13 : 0000000000000000000000000000000000000000000000000000000000000000
14 : 0000000000000000000000000000000000000000000000000000000000000000
15 : 0000000000000000000000000000000000000000000000000000000000000000
16 : 0000000000000000000000000000000000000000000000000000000000000000
17 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
18 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
19 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
20 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
21 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
22 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
23 : 0000000000000000000000000000000000000000000000000000000000000000
sha384 :
The PhysicalPresenceLib is required, it sets some variables, but the
firmware doesn't act on it yet.
Laszlo Ersek explained on the list why Tpm2DeviceLib has to be
resolved differently for DXE_DRIVER modules in general and for
"Tcg2Dxe.inf" specifically:
* We have a library class called Tpm2DeviceLib -- this is basically the
set of APIs declared in "SecurityPkg/Include/Library/Tpm2DeviceLib.h".
Its leading comment says "This library abstract how to access TPM2
hardware device".
There are two *sets* of APIs in "Tpm2DeviceLib.h":
(a) functions that deal with the TPM2 device:
- Tpm2RequestUseTpm(),
- Tpm2SubmitCommand()
This set of APIs is supposed to be used by clients that *consume*
the TPM2 device abstraction.
(b) the function Tpm2RegisterTpm2DeviceLib(), which is supposed to be
used by *providers* of various TPM2 device abstractions.
* Then, we have two implementations (instances) of the Tpm2DeviceLib class:
(1) SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
(2) SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
(1) The first library instance ("Tpm2DeviceLibTcg2.inf") implements the
APIs listed under (a), and it does not implement (b) -- see
EFI_UNSUPPORTED. In other words, this lib instance is strictly meant for
drivers that *consume* the TPM2 device abstraction. And, the (a) group
of APIs is implemented by forwarding the requests to the TCG2 protocol.
The idea here is that all the drivers that consume the TPM2 abstraction
do not have to be statically linked with a large TPM2 device library
instance; instead they are only linked (statically) with this "thin"
library instance, and all the actual work is delegated to whichever
driver that provides the singleton TCG2 protocol.
(2) The second library instance ("Tpm2DeviceLibRouterDxe.inf") is meant
for the driver that offers (produces) the TCG2 protocol. This lib
instance implements both (a) and (b) API groups.
* Here's how things fit together:
(i) The "SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf"
library instance (which has no lib class) is linked into "Tcg2Dxe.inf"
via NULL class resolution. This simply means that before the
"Tcg2Dxe.inf" entry point function is entered, the constructor function
of "Tpm2InstanceLibDTpm.inf" will be called.
(ii) This Tpm2InstanceLibDTpmConstructor() function calls API (b), and
registers its own actual TPM2 command implementation with the
"Tpm2DeviceLibRouter" library instance (also linked into the Tcg2Dxe
driver). This provides the back-end for the API set (a).
TCG2 protocol provider (Tcg2Dxe.inf driver) launches
|
v
NULL class: Tpm2InstanceLibDTpm instance construction
|
v
Tpm2DeviceLib class: Tpm2DeviceLibRouter instance
backend registration for API set (a)
(iii) The Tcg2Dxe driver exposes the TCG2 protocol.
(iv) A TPM2 consumer calls API set (a) via lib instance (1). Such calls
land in Tcg2Dxe, via the protocol.
(v) Tcg2Dxe serves the protocol request by forwarding it to API set (a)
from lib instance (2).
(vi) Those functions call the "backend" functions registered by
Tpm2DeviceLibDTpm in step (ii).
TPM 2 consumer driver
|
v
Tpm2DeviceLib class: Tpm2DeviceLibTcg2 instance
|
v
TCG2 protocol interface
|
v
TCG2 protocol provider: Tcg2Dxe.inf driver
|
v
Tpm2DeviceLib class: Tpm2DeviceLibRouter instance
|
v
NULL class: Tpm2InstanceLibDTpm instance
(via earlier registration)
|
v
TPM2 chip (actual hardware)
* So that is the "router" pattern in edk2. Namely,
- Consumers of an abstraction use a thin library instance.
- The thin library instance calls a firmware-global (singleton) service,
i.e. a PPI (in the PEI phase) or protocol (in the DXE phase).
- The PEIM providing the PPI, or the DXE driver providing the protocol,
don't themselves implement the actual service either. Instead they
offer a "registration" service too, and they only connect the incoming
"consumer" calls to the earlier registered back-end(s).
- The "registration service", for back-ends to use, may take various
forms.
It can be exposed globally to the rest of the firmware, as
another member function of the PPI / protocol structure. Then backends
can be provided by separate PEIMs / DXE drivers.
Or else, the registration service can be exposed as just another
library API. In this case, the backends are provided as NULL class
library instances, and a platform DSC file links them into the PEIM /
DXE driver via NULL class resolutions. The backend lib instances call
the registration service in their own respective constructor
functions.
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
OvmfPkg/OvmfPkgX64.dsc | 16 ++++++++++++++++
OvmfPkg/OvmfPkgX64.fdf | 4 ++++
2 files changed, 20 insertions(+)
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3fa1a31f4c37..7753852144fb 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -211,6 +211,8 @@ [LibraryClasses]
!if $(TPM2_ENABLE) == TRUE
Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
+ Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
+ Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
!endif
[LibraryClasses.common]
@@ -362,6 +364,10 @@ [LibraryClasses.common.DXE_DRIVER]
PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
+!if $(TPM2_ENABLE)
+ HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
+ Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
+!endif
[LibraryClasses.common.UEFI_APPLICATION]
PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
@@ -642,6 +648,16 @@ [Components]
MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
+!if $(TPM2_ENABLE) == TRUE
+ SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
+ <LibraryClasses>
+ Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
+ NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
+ NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+ NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
+ }
+!endif
+
MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
<LibraryClasses>
!if $(SECURE_BOOT_ENABLE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index c0173e7adf5f..1a46104fc63d 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -392,6 +392,10 @@ [FV.DXEFV]
INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
!endif
+!if $(TPM2_ENABLE) == TRUE
+INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
+!endif
+
################################################################################
[FV.FVMAIN_COMPACT]
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
` (6 preceding siblings ...)
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 7/8] ovmf: link with Tcg2Dxe module marcandre.lureau
@ 2018-03-07 15:57 ` marcandre.lureau
2018-03-08 19:54 ` Laszlo Ersek
2018-03-08 12:31 ` [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support Shi, Steven
8 siblings, 1 reply; 36+ messages in thread
From: marcandre.lureau @ 2018-03-07 15:57 UTC (permalink / raw)
To: edk2-devel
Cc: pjones, jiewen.yao, stefanb, lersek, qemu-devel, javierm,
Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The library registers a security management handler, to measure images
that are not measure in PEI phase.
This seems to work for example with the qemu PXE rom:
Loading driver at 0x0003E6C2000 EntryPoint=0x0003E6C9076 8086100e.efi
And the following binary_bios_measurements log entry seems to be
added:
PCR: 2 type: EV_EFI_BOOT_SERVICES_DRIVER size: 0x4e digest: 70a22475e9f18806d2ed9193b48d80d26779d9a4
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
OvmfPkg/OvmfPkgX64.dsc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 7753852144fb..9db1712e3623 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -662,6 +662,9 @@ [Components]
<LibraryClasses>
!if $(SECURE_BOOT_ENABLE) == TRUE
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+!endif
+!if $(TPM2_ENABLE) == TRUE
+ NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
!endif
}
--
2.16.2.346.g9779355e34
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex marcandre.lureau
@ 2018-03-07 16:04 ` Yao, Jiewen
2018-03-08 0:36 ` [Qemu-devel] [edk2] " Zhang, Chao B
2018-03-08 11:41 ` Laszlo Ersek
2 siblings, 0 replies; 36+ messages in thread
From: Yao, Jiewen @ 2018-03-07 16:04 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: pjones, stefanb, lersek, qemu-devel, javierm
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: marcandre.lureau@redhat.com [mailto:marcandre.lureau@redhat.com]
> Sent: Wednesday, March 7, 2018 11:58 PM
> To: edk2-devel@lists.01.org
> Cc: pjones@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>;
> stefanb@linux.vnet.ibm.com; lersek@redhat.com; qemu-devel@nongnu.org;
> javierm@redhat.com; Marc-André Lureau <marcandre.lureau@redhat.com>
> Subject: [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from
> Depex
>
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The module doesn't use read-only variable.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> index bc910c3baf97..a4aae1488ff8 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> @@ -91,7 +91,6 @@ [Pcd]
>
> [Depex]
> gEfiPeiMasterBootModePpiGuid AND
> - gEfiPeiReadOnlyVariable2PpiGuid AND
> gEfiTpmDeviceSelectedGuid
>
> [UserExtensions.TianoCore."ExtraFiles"]
> --
> 2.16.2.346.g9779355e34
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask marcandre.lureau
@ 2018-03-08 0:35 ` Zhang, Chao B
2018-03-08 0:48 ` Zeng, Star
2018-03-08 11:40 ` [Qemu-devel] [edk2] " Laszlo Ersek
1 sibling, 1 reply; 36+ messages in thread
From: Zhang, Chao B @ 2018-03-08 0:35 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel
Cc: pjones, Yao, Jiewen, stefanb, lersek, qemu-devel, javierm, Zeng, Star
Reviewed-by: Chao Zhang<chao.b.zhang@intel.com>
-----Original Message-----
From: marcandre.lureau@redhat.com [mailto:marcandre.lureau@redhat.com]
Sent: Wednesday, March 7, 2018 11:58 PM
To: edk2-devel@lists.01.org
Cc: pjones@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>; stefanb@linux.vnet.ibm.com; lersek@redhat.com; qemu-devel@nongnu.org; javierm@redhat.com; Marc-André Lureau <marcandre.lureau@redhat.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Commit 4cc2b63bd829426b05bad0d8952f1855a10d6ed7 fixed an out of bounds
ZeroMem() call. However, as Laszlo Ersek pointed out, the intent was to clear all but the Identifier (to revert the effect of RegisterHashInterfaceLib()). For that, it should clear the SupportedHashMask too.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
.../Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index 361a4f6508a0..bf6e1336ee76 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.c
@@ -426,6 +426,7 @@ HashLibBaseCryptoRouterPeiConstructor (
//
ZeroMem (&HashInterfaceHob->HashInterface, sizeof (HashInterfaceHob->HashInterface));
HashInterfaceHob->HashInterfaceCount = 0;
+ HashInterfaceHob->SupportedHashMask = 0;
}
//
--
2.16.2.346.g9779355e34
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex marcandre.lureau
2018-03-07 16:04 ` Yao, Jiewen
@ 2018-03-08 0:36 ` Zhang, Chao B
2018-03-09 13:05 ` Marc-André Lureau
2018-03-08 11:41 ` Laszlo Ersek
2 siblings, 1 reply; 36+ messages in thread
From: Zhang, Chao B @ 2018-03-08 0:36 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel
Cc: qemu-devel, javierm, pjones, Yao, Jiewen, lersek
Hi Lureau:
I think we can remove same dependency in TcgPei.
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of marcandre.lureau@redhat.com
Sent: Wednesday, March 7, 2018 11:58 PM
To: edk2-devel@lists.01.org
Cc: qemu-devel@nongnu.org; javierm@redhat.com; pjones@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>; lersek@redhat.com
Subject: [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The module doesn't use read-only variable.
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
1 file changed, 1 deletion(-)
diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
index bc910c3baf97..a4aae1488ff8 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
@@ -91,7 +91,6 @@ [Pcd]
[Depex]
gEfiPeiMasterBootModePpiGuid AND
- gEfiPeiReadOnlyVariable2PpiGuid AND
gEfiTpmDeviceSelectedGuid
[UserExtensions.TianoCore."ExtraFiles"]
--
2.16.2.346.g9779355e34
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply related [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
2018-03-08 0:35 ` Zhang, Chao B
@ 2018-03-08 0:48 ` Zeng, Star
0 siblings, 0 replies; 36+ messages in thread
From: Zeng, Star @ 2018-03-08 0:48 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel
Cc: pjones, Yao, Jiewen, stefanb, lersek, qemu-devel, javierm, Zhang,
Chao B, Zeng, Star
Reviewed-by: Star Zeng <star.zeng@intel.com>
Thanks,
Star
-----Original Message-----
From: Zhang, Chao B
Sent: Thursday, March 8, 2018 8:35 AM
To: marcandre.lureau@redhat.com; edk2-devel@lists.01.org
Cc: pjones@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>; stefanb@linux.vnet.ibm.com; lersek@redhat.com; qemu-devel@nongnu.org; javierm@redhat.com; Zeng, Star <star.zeng@intel.com>
Subject: RE: [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
Reviewed-by: Chao Zhang<chao.b.zhang@intel.com>
-----Original Message-----
From: marcandre.lureau@redhat.com [mailto:marcandre.lureau@redhat.com]
Sent: Wednesday, March 7, 2018 11:58 PM
To: edk2-devel@lists.01.org
Cc: pjones@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>; stefanb@linux.vnet.ibm.com; lersek@redhat.com; qemu-devel@nongnu.org; javierm@redhat.com; Marc-André Lureau <marcandre.lureau@redhat.com>; Zhang, Chao B <chao.b.zhang@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Commit 4cc2b63bd829426b05bad0d8952f1855a10d6ed7 fixed an out of bounds
ZeroMem() call. However, as Laszlo Ersek pointed out, the intent was to clear all but the Identifier (to revert the effect of RegisterHashInterfaceLib()). For that, it should clear the SupportedHashMask too.
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
.../Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index 361a4f6508a0..bf6e1336ee76 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.c
@@ -426,6 +426,7 @@ HashLibBaseCryptoRouterPeiConstructor (
//
ZeroMem (&HashInterfaceHob->HashInterface, sizeof (HashInterfaceHob->HashInterface));
HashInterfaceHob->HashInterfaceCount = 0;
+ HashInterfaceHob->SupportedHashMask = 0;
}
//
--
2.16.2.346.g9779355e34
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask marcandre.lureau
2018-03-08 0:35 ` Zhang, Chao B
@ 2018-03-08 11:40 ` Laszlo Ersek
1 sibling, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 11:40 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel
Cc: qemu-devel, javierm, pjones, jiewen.yao, Star Zeng, Chao Zhang
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Commit 4cc2b63bd829426b05bad0d8952f1855a10d6ed7 fixed an out of bounds
> ZeroMem() call. However, as Laszlo Ersek pointed out, the intent was
> to clear all but the Identifier (to revert the effect of
> RegisterHashInterfaceLib()). For that, it should clear the
> SupportedHashMask too.
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> .../Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> index 361a4f6508a0..bf6e1336ee76 100644
> --- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> +++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> @@ -426,6 +426,7 @@ HashLibBaseCryptoRouterPeiConstructor (
> //
> ZeroMem (&HashInterfaceHob->HashInterface, sizeof (HashInterfaceHob->HashInterface));
> HashInterfaceHob->HashInterfaceCount = 0;
> + HashInterfaceHob->SupportedHashMask = 0;
> }
>
> //
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex marcandre.lureau
2018-03-07 16:04 ` Yao, Jiewen
2018-03-08 0:36 ` [Qemu-devel] [edk2] " Zhang, Chao B
@ 2018-03-08 11:41 ` Laszlo Ersek
2 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 11:41 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: qemu-devel, javierm, pjones, jiewen.yao
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The module doesn't use read-only variable.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> index bc910c3baf97..a4aae1488ff8 100644
> --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> @@ -91,7 +91,6 @@ [Pcd]
>
> [Depex]
> gEfiPeiMasterBootModePpiGuid AND
> - gEfiPeiReadOnlyVariable2PpiGuid AND
> gEfiTpmDeviceSelectedGuid
>
> [UserExtensions.TianoCore."ExtraFiles"]
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER marcandre.lureau
@ 2018-03-08 11:59 ` Laszlo Ersek
2018-03-08 12:08 ` Zeng, Star
0 siblings, 1 reply; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 11:59 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel
Cc: qemu-devel, javierm, pjones, jiewen.yao, Star Zeng, Eric Dong
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +++++++++---------
> MdeModulePkg/Core/Pei/Image/Image.c | 4 ++--
> MdeModulePkg/Core/Pei/PeiMain.h | 2 +-
> MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 2 +-
> 4 files changed, 13 insertions(+), 13 deletions(-)
CC'ing Star & Eric (see Maintainers.txt).
I suggest changing te subject like this:
MdeModulePkg/Core/Pei: fix REGISITER -> REGISTER typo
And just so the commit message isn't empty, let's say there, "No
functional changes.".
With those updates:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
> diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> index 79f2e5cebcbe..027176d872c7 100644
> --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> @@ -970,7 +970,7 @@ PeiDispatcher (
> if ((Private->PeiMemoryInstalled) && (Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
> //
> // Once real memory is available, shadow the RegisterForShadow modules. And meanwhile
> - // update the modules' status from PEIM_STATE_REGISITER_FOR_SHADOW to PEIM_STATE_DONE.
> + // update the modules' status from PEIM_STATE_REGISTER_FOR_SHADOW to PEIM_STATE_DONE.
> //
> SaveCurrentPeimCount = Private->CurrentPeimCount;
> SaveCurrentFvCount = Private->CurrentPeimFvCount;
> @@ -978,7 +978,7 @@ PeiDispatcher (
>
> for (Index1 = 0; Index1 <= SaveCurrentFvCount; Index1++) {
> for (Index2 = 0; (Index2 < PcdGet32 (PcdPeiCoreMaxPeimPerFv)) && (Private->Fv[Index1].FvFileHandles[Index2] != NULL); Index2++) {
> - if (Private->Fv[Index1].PeimState[Index2] == PEIM_STATE_REGISITER_FOR_SHADOW) {
> + if (Private->Fv[Index1].PeimState[Index2] == PEIM_STATE_REGISTER_FOR_SHADOW) {
> PeimFileHandle = Private->Fv[Index1].FvFileHandles[Index2];
> Private->CurrentFileHandle = PeimFileHandle;
> Private->CurrentPeimFvCount = Index1;
> @@ -986,13 +986,13 @@ PeiDispatcher (
> Status = PeiLoadImage (
> (CONST EFI_PEI_SERVICES **) &Private->Ps,
> PeimFileHandle,
> - PEIM_STATE_REGISITER_FOR_SHADOW,
> + PEIM_STATE_REGISTER_FOR_SHADOW,
> &EntryPoint,
> &AuthenticationState
> );
> if (Status == EFI_SUCCESS) {
> //
> - // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
> + // PEIM_STATE_REGISTER_FOR_SHADOW move to PEIM_STATE_DONE
> //
> Private->Fv[Index1].PeimState[Index2]++;
> //
> @@ -1165,7 +1165,7 @@ PeiDispatcher (
> //
> PeiCheckAndSwitchStack (SecCoreData, Private);
>
> - if ((Private->PeiMemoryInstalled) && (Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISITER_FOR_SHADOW) && \
> + if ((Private->PeiMemoryInstalled) && (Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW) && \
> (Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
> //
> // If memory is available we shadow images by default for performance reasons.
> @@ -1179,7 +1179,7 @@ PeiDispatcher (
> Status = PeiLoadImage (
> PeiServices,
> PeimFileHandle,
> - PEIM_STATE_REGISITER_FOR_SHADOW,
> + PEIM_STATE_REGISTER_FOR_SHADOW,
> &EntryPoint,
> &AuthenticationState
> );
> @@ -1192,7 +1192,7 @@ PeiDispatcher (
> //PERF_END (PeiServices, L"PEIM", PeimFileHandle, 0);
>
> //
> - // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
> + // PEIM_STATE_REGISTER_FOR_SHADOW move to PEIM_STATE_DONE
> //
> Private->Fv[FvCount].PeimState[PeimCount]++;
>
> @@ -1356,14 +1356,14 @@ PeiRegisterForShadow (
> return EFI_NOT_FOUND;
> }
>
> - if (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] >= PEIM_STATE_REGISITER_FOR_SHADOW) {
> + if (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] >= PEIM_STATE_REGISTER_FOR_SHADOW) {
> //
> // If the PEIM has already entered the PEIM_STATE_REGISTER_FOR_SHADOW or PEIM_STATE_DONE then it's already been started
> //
> return EFI_ALREADY_STARTED;
> }
>
> - Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] = PEIM_STATE_REGISITER_FOR_SHADOW;
> + Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] = PEIM_STATE_REGISTER_FOR_SHADOW;
>
> return EFI_SUCCESS;
> }
> diff --git a/MdeModulePkg/Core/Pei/Image/Image.c b/MdeModulePkg/Core/Pei/Image/Image.c
> index f41d3acac77e..f07f48823117 100644
> --- a/MdeModulePkg/Core/Pei/Image/Image.c
> +++ b/MdeModulePkg/Core/Pei/Image/Image.c
> @@ -387,7 +387,7 @@ LoadAndRelocatePeCoffImage (
> }
> IsRegisterForShadow = FALSE;
> if ((Private->CurrentFileHandle == FileHandle)
> - && (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] == PEIM_STATE_REGISITER_FOR_SHADOW)) {
> + && (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW)) {
> IsRegisterForShadow = TRUE;
> }
>
> @@ -876,7 +876,7 @@ PeiLoadImage (
> //
> // The shadowed PEIM must be relocatable.
> //
> - if (PeimState == PEIM_STATE_REGISITER_FOR_SHADOW) {
> + if (PeimState == PEIM_STATE_REGISTER_FOR_SHADOW) {
> IsStrip = RelocationIsStrip ((VOID *) (UINTN) ImageAddress);
> ASSERT (!IsStrip);
> if (IsStrip) {
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h b/MdeModulePkg/Core/Pei/PeiMain.h
> index fef3753e4b3b..a1cdbc15d98a 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -104,7 +104,7 @@ typedef struct {
> //
> #define PEIM_STATE_NOT_DISPATCHED 0x00
> #define PEIM_STATE_DISPATCHED 0x01
> -#define PEIM_STATE_REGISITER_FOR_SHADOW 0x02
> +#define PEIM_STATE_REGISTER_FOR_SHADOW 0x02
> #define PEIM_STATE_DONE 0x03
>
> typedef struct {
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 3cd61906c3df..775bf18ce938 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -104,7 +104,7 @@ ShadowPeiCore (
> Status = PeiLoadImage (
> GetPeiServicesTablePointer (),
> *((EFI_PEI_FILE_HANDLE*)&PeiCoreFileHandle),
> - PEIM_STATE_REGISITER_FOR_SHADOW,
> + PEIM_STATE_REGISTER_FOR_SHADOW,
> &EntryPoint,
> &AuthenticationState
> );
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER
2018-03-08 11:59 ` [Qemu-devel] [edk2] " Laszlo Ersek
@ 2018-03-08 12:08 ` Zeng, Star
0 siblings, 0 replies; 36+ messages in thread
From: Zeng, Star @ 2018-03-08 12:08 UTC (permalink / raw)
To: Laszlo Ersek, marcandre.lureau, edk2-devel
Cc: qemu-devel, javierm, pjones, Yao, Jiewen, Dong, Eric, Zeng, Star
I agree with Laszlo's suggestion.
And it is good observation.
Reviewed-by: Star Zeng <star.zeng@intel.com>
Thanks,
Star
-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Thursday, March 8, 2018 7:59 PM
To: marcandre.lureau@redhat.com; edk2-devel@lists.01.org
Cc: qemu-devel@nongnu.org; javierm@redhat.com; pjones@redhat.com; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: Re: [edk2] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 18 +++++++++---------
> MdeModulePkg/Core/Pei/Image/Image.c | 4 ++--
> MdeModulePkg/Core/Pei/PeiMain.h | 2 +-
> MdeModulePkg/Core/Pei/PeiMain/PeiMain.c | 2 +-
> 4 files changed, 13 insertions(+), 13 deletions(-)
CC'ing Star & Eric (see Maintainers.txt).
I suggest changing te subject like this:
MdeModulePkg/Core/Pei: fix REGISITER -> REGISTER typo
And just so the commit message isn't empty, let's say there, "No functional changes.".
With those updates:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
> diff --git a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> index 79f2e5cebcbe..027176d872c7 100644
> --- a/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> +++ b/MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c
> @@ -970,7 +970,7 @@ PeiDispatcher (
> if ((Private->PeiMemoryInstalled) && (Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
> //
> // Once real memory is available, shadow the RegisterForShadow modules. And meanwhile
> - // update the modules' status from PEIM_STATE_REGISITER_FOR_SHADOW to PEIM_STATE_DONE.
> + // update the modules' status from PEIM_STATE_REGISTER_FOR_SHADOW to PEIM_STATE_DONE.
> //
> SaveCurrentPeimCount = Private->CurrentPeimCount;
> SaveCurrentFvCount = Private->CurrentPeimFvCount;
> @@ -978,7 +978,7 @@ PeiDispatcher (
>
> for (Index1 = 0; Index1 <= SaveCurrentFvCount; Index1++) {
> for (Index2 = 0; (Index2 < PcdGet32 (PcdPeiCoreMaxPeimPerFv)) && (Private->Fv[Index1].FvFileHandles[Index2] != NULL); Index2++) {
> - if (Private->Fv[Index1].PeimState[Index2] == PEIM_STATE_REGISITER_FOR_SHADOW) {
> + if (Private->Fv[Index1].PeimState[Index2] ==
> + PEIM_STATE_REGISTER_FOR_SHADOW) {
> PeimFileHandle = Private->Fv[Index1].FvFileHandles[Index2];
> Private->CurrentFileHandle = PeimFileHandle;
> Private->CurrentPeimFvCount = Index1; @@ -986,13 +986,13
> @@ PeiDispatcher (
> Status = PeiLoadImage (
> (CONST EFI_PEI_SERVICES **) &Private->Ps,
> PeimFileHandle,
> - PEIM_STATE_REGISITER_FOR_SHADOW,
> + PEIM_STATE_REGISTER_FOR_SHADOW,
> &EntryPoint,
> &AuthenticationState
> );
> if (Status == EFI_SUCCESS) {
> //
> - // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
> + // PEIM_STATE_REGISTER_FOR_SHADOW move to PEIM_STATE_DONE
> //
> Private->Fv[Index1].PeimState[Index2]++;
> //
> @@ -1165,7 +1165,7 @@ PeiDispatcher (
> //
> PeiCheckAndSwitchStack (SecCoreData, Private);
>
> - if ((Private->PeiMemoryInstalled) && (Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISITER_FOR_SHADOW) && \
> + if ((Private->PeiMemoryInstalled) && (Private->Fv[FvCount].PeimState[PeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW) && \
> (Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME || PcdGetBool (PcdShadowPeimOnS3Boot))) {
> //
> // If memory is available we shadow images by default for performance reasons.
> @@ -1179,7 +1179,7 @@ PeiDispatcher (
> Status = PeiLoadImage (
> PeiServices,
> PeimFileHandle,
> - PEIM_STATE_REGISITER_FOR_SHADOW,
> + PEIM_STATE_REGISTER_FOR_SHADOW,
> &EntryPoint,
> &AuthenticationState
> );
> @@ -1192,7 +1192,7 @@ PeiDispatcher (
> //PERF_END (PeiServices, L"PEIM", PeimFileHandle, 0);
>
> //
> - // PEIM_STATE_REGISITER_FOR_SHADOW move to PEIM_STATE_DONE
> + // PEIM_STATE_REGISTER_FOR_SHADOW move to
> + PEIM_STATE_DONE
> //
> Private->Fv[FvCount].PeimState[PeimCount]++;
>
> @@ -1356,14 +1356,14 @@ PeiRegisterForShadow (
> return EFI_NOT_FOUND;
> }
>
> - if
> (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPe
> imCount] >= PEIM_STATE_REGISITER_FOR_SHADOW) {
> + if
> + (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->Current
> + PeimCount] >= PEIM_STATE_REGISTER_FOR_SHADOW) {
> //
> // If the PEIM has already entered the PEIM_STATE_REGISTER_FOR_SHADOW or PEIM_STATE_DONE then it's already been started
> //
> return EFI_ALREADY_STARTED;
> }
>
> -
> Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPei
> mCount] = PEIM_STATE_REGISITER_FOR_SHADOW;
> +
> + Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentP
> + eimCount] = PEIM_STATE_REGISTER_FOR_SHADOW;
>
> return EFI_SUCCESS;
> }
> diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
> b/MdeModulePkg/Core/Pei/Image/Image.c
> index f41d3acac77e..f07f48823117 100644
> --- a/MdeModulePkg/Core/Pei/Image/Image.c
> +++ b/MdeModulePkg/Core/Pei/Image/Image.c
> @@ -387,7 +387,7 @@ LoadAndRelocatePeCoffImage (
> }
> IsRegisterForShadow = FALSE;
> if ((Private->CurrentFileHandle == FileHandle)
> - && (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->CurrentPeimCount] == PEIM_STATE_REGISITER_FOR_SHADOW)) {
> + &&
> + (Private->Fv[Private->CurrentPeimFvCount].PeimState[Private->Current
> + PeimCount] == PEIM_STATE_REGISTER_FOR_SHADOW)) {
> IsRegisterForShadow = TRUE;
> }
>
> @@ -876,7 +876,7 @@ PeiLoadImage (
> //
> // The shadowed PEIM must be relocatable.
> //
> - if (PeimState == PEIM_STATE_REGISITER_FOR_SHADOW) {
> + if (PeimState == PEIM_STATE_REGISTER_FOR_SHADOW) {
> IsStrip = RelocationIsStrip ((VOID *) (UINTN) ImageAddress);
> ASSERT (!IsStrip);
> if (IsStrip) {
> diff --git a/MdeModulePkg/Core/Pei/PeiMain.h
> b/MdeModulePkg/Core/Pei/PeiMain.h index fef3753e4b3b..a1cdbc15d98a
> 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain.h
> +++ b/MdeModulePkg/Core/Pei/PeiMain.h
> @@ -104,7 +104,7 @@ typedef struct {
> //
> #define PEIM_STATE_NOT_DISPATCHED 0x00
> #define PEIM_STATE_DISPATCHED 0x01
> -#define PEIM_STATE_REGISITER_FOR_SHADOW 0x02
> +#define PEIM_STATE_REGISTER_FOR_SHADOW 0x02
> #define PEIM_STATE_DONE 0x03
>
> typedef struct {
> diff --git a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> index 3cd61906c3df..775bf18ce938 100644
> --- a/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> +++ b/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c
> @@ -104,7 +104,7 @@ ShadowPeiCore (
> Status = PeiLoadImage (
> GetPeiServicesTablePointer (),
> *((EFI_PEI_FILE_HANDLE*)&PeiCoreFileHandle),
> - PEIM_STATE_REGISITER_FOR_SHADOW,
> + PEIM_STATE_REGISTER_FOR_SHADOW,
> &EntryPoint,
> &AuthenticationState
> );
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
` (7 preceding siblings ...)
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib marcandre.lureau
@ 2018-03-08 12:31 ` Shi, Steven
2018-03-08 13:59 ` Marc-André Lureau
8 siblings, 1 reply; 36+ messages in thread
From: Shi, Steven @ 2018-03-08 12:31 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel
Cc: qemu-devel, javierm, pjones, Yao, Jiewen, lersek
Hi Marcandre,
> I test with qemu & swtpm/libtpms (tpm2 branches, swtpm_setup.sh --tpm2 --tpm-state tpmstatedir)
> $ swtpm socket --tpmstate tpmstatedir --ctrl type=unixio,path=tpmsock --tpm2 &
Where is the swtpm_setup.sh? And could you tell how to build & install the swtpm?
Thanks
Steven Shi
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support
2018-03-08 12:31 ` [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support Shi, Steven
@ 2018-03-08 13:59 ` Marc-André Lureau
2018-03-09 3:03 ` Shi, Steven
0 siblings, 1 reply; 36+ messages in thread
From: Marc-André Lureau @ 2018-03-08 13:59 UTC (permalink / raw)
To: Shi, Steven
Cc: edk2-devel, lersek, pjones, Yao, Jiewen, qemu-devel, javierm,
Stefan Berger
Hi
On Thu, Mar 8, 2018 at 1:31 PM, Shi, Steven <steven.shi@intel.com> wrote:
> Hi Marcandre,
>> I test with qemu & swtpm/libtpms (tpm2 branches, swtpm_setup.sh --tpm2 --tpm-state tpmstatedir)
>> $ swtpm socket --tpmstate tpmstatedir --ctrl type=unixio,path=tpmsock --tpm2 &
>
> Where is the swtpm_setup.sh? And could you tell how to build & install the swtpm?
>
You need to compile & install libtpms & swtpm :
git clone -b tpm2-preview.rev146.v2 https://github.com/stefanberger/libtpms
cd libtpms
autoreconf -vfi && ./configure --with-tpm2 --with-openssl && make install
git clone -b tpm2-preview.v2 https://github.com/stefanberger/swtpm
cd swtpm
autoreconf -vfi && ./configure --with-openssl && make install
Then you can run:
mkdir tpmstatedir
swtpm_setup.sh --tpm2 --tpm-state tpmstatedir
Run the emulator:
swtpm socket --tpmstate dir=tpmstatedir --ctrl
type=unixio,path=tpmemu.sock --tpm2
Run qemu (from git) with ovmf (with this series):
qemu ... -chardev socket,id=chrtpm,path=tpmemu.sock -tpmdev
emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
-drive if=pflash,format=raw,file=OVMF_CODE.fd,readonly -drive
if=pflash,format=raw,file=OVMF_VARS.fd ..
cheers
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 4/8] ovmf: simplify SecurityStubDxe.inf inclusion
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 4/8] ovmf: simplify SecurityStubDxe.inf inclusion marcandre.lureau
@ 2018-03-08 16:35 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 16:35 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: qemu-devel, javierm, pjones, jiewen.yao
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> SecurityStubDxe.inf should be included unconditionally.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> OvmfPkg/OvmfPkgIa32.dsc | 6 ++----
> OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++----
> OvmfPkg/OvmfPkgX64.dsc | 6 ++----
> 3 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index fbe0f790e431..5bd3f4f977df 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -611,14 +611,12 @@ [Components]
>
> MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> <LibraryClasses>
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> - }
> -!else
> - MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> !endif
> + }
>
> MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
> PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index fb10e0b0f2e4..7dded86c4940 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -620,14 +620,12 @@ [Components.X64]
>
> MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> <LibraryClasses>
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> - }
> -!else
> - MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> !endif
> + }
>
> MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
> PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index a31551f5ae24..a8e89276c0b2 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -618,14 +618,12 @@ [Components]
>
> MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>
> -!if $(SECURE_BOOT_ENABLE) == TRUE
> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> <LibraryClasses>
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> - }
> -!else
> - MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> !endif
> + }
>
> MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
> PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
>
Please change the subject as follows:
OvmfPkg: simplify SecurityStubDxe.inf inclusion
With that update:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module marcandre.lureau
@ 2018-03-08 17:46 ` Laszlo Ersek
2018-03-08 18:10 ` Laszlo Ersek
0 siblings, 1 reply; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 17:46 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: qemu-devel, javierm, pjones, jiewen.yao
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The Tcg2ConfigPei module informs the firmware globally about the TPM
> device type, by setting the PcdTpmInstanceGuid PCD to the appropriate
> GUID value. The original module under SecurityPkg can perform device
> detection, or read a cached value from a non-volatile UEFI variable.
>
> OvmfPkg's clone of the module only performs the TPM2 hardware detection.
>
> This is what the module does:
>
> - Check the QEMU hardware for TPM2 availability only
>
> - If found, set the dynamic PCD "PcdTpmInstanceGuid" to
> &gEfiTpmDeviceInstanceTpm20DtpmGuid. This is what informs the rest of
> the firmware about the TPM type.
>
> - Install the gEfiTpmDeviceSelectedGuid PPI. This action permits the
> PEI_CORE to dispatch the Tcg2Pei module, which consumes the above PCD.
> In effect, the gEfiTpmDeviceSelectedGuid PPI serializes the setting
> and the consumption of the "TPM type" PCD.
>
> - If no TPM2 was found, install gPeiTpmInitializationDonePpiGuid.
> (Normally this is performed by Tcg2Pei, but Tcg2Pei doesn't do it if
> no TPM2 is available. So in that case our Tcg2ConfigPei must do it.)
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> OvmfPkg/OvmfPkgX64.dsc | 17 ++++
> OvmfPkg/OvmfPkgX64.fdf | 4 +
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 57 +++++++++++
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 124 +++++++++++++++++++++++
> OvmfPkg/Tcg/Tcg2Config/TpmDetection.c | 46 +++++++++
> 5 files changed, 248 insertions(+)
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> create mode 100644 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
(1) Please change the subject line to:
OvmfPkg: add customized Tcg2ConfigPei clone
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index a8e89276c0b2..64bd6b6a9f08 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -39,6 +39,7 @@ [Defines]
> DEFINE HTTP_BOOT_ENABLE = FALSE
> DEFINE SMM_REQUIRE = FALSE
> DEFINE TLS_ENABLE = FALSE
> + DEFINE TPM2_ENABLE = FALSE
>
> #
> # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly to
> @@ -208,6 +209,10 @@ [LibraryClasses]
> OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf
> XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
>
> +!if $(TPM2_ENABLE) == TRUE
> + Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> +!endif
> +
> [LibraryClasses.common]
> BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
>
> @@ -272,6 +277,10 @@ [LibraryClasses.common.PEIM]
> PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>
> +!if $(TPM2_ENABLE)
(2) Please be consistent with the other "!if" checks for the same
define; this should be
!if $(TPM2_ENABLE) == TRUE
> + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> +!endif
> +
> [LibraryClasses.common.DXE_CORE]
> HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
> DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
> @@ -554,6 +563,10 @@ [PcdsDynamicDefault]
>
> gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x00
>
> +!if $(TPM2_ENABLE) == TRUE
> + gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> +!endif
> +
> ################################################################################
> #
> # Components Section - list of all EDK II Modules needed by this Platform.
> @@ -600,6 +613,10 @@ [Components]
> !endif
> UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> +!if $(TPM2_ENABLE) == TRUE
> + OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +!endif
> +
> #
> # DXE Phase modules
> #
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 2fc17810eb23..dbafada5226b 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -165,6 +165,10 @@ [FV.PEIFV]
> !endif
> INF UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>
> +!if $(TPM2_ENABLE) == TRUE
> +INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +!endif
> +
> ################################################################################
>
> [FV.DXEFV]
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> new file mode 100644
> index 000000000000..201e4f24d5f4
> --- /dev/null
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -0,0 +1,57 @@
> +## @file
> +# Set TPM device type
> +#
> +# This module initializes TPM device type based on variable and detection.
> +# This OvmfPkg of SecurityPkg only does TPM2 detection.
(3) Thanks for updating this, relative to SecurityPkg -- can you please
clean it up a little? The second sentence looks malformed. I suggest
something like:
# In SecurityPkg, this module initializes the TPM device type based on
# a UEFI variable and/or hardware detection. In OvmfPkg, the module
# only performs TPM2 hardware detection.
> +#
> +# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> +# Copyright (C) 2018, Red Hat, Inc.
> +#
> +# This program and the accompanying materials
> +# are licensed and made available under the terms and conditions of the BSD License
> +# which accompanies this distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
(4) Hmm these lines are a bit too long:
$ wc -L OvmfPkg/Tcg/Tcg2Config/*
96 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
106 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
102 OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
106 total
Can you please rewrap the source files so they don't exceed 80
characters per line?
Well, actually, don't bother with that. If you do that, some of the line
breaks might force you to break function calls to multiple lines, and
then we'll likely need yet another round of review, because the edk2
"multiline function call" style is pretty idiosyncratic. It goes like
this (note the indentation!):
- variant 1:
SmartFunction (
Argument1OnASeparateLine,
Argument2OnASeparateLine,
Argument3OnASeparateLine
); // closing paren on a separate line, aligned exactly like this
- variant 2:
SmartFunction2 (Argument1, Argument2, Argument3, Argument4,
Argument5, Argument6);
So, I guess I'll leave it up to you. :) If you don't feel like messing
with this, I can rewrap the source in a separate patchset.
> +
> +[Defines]
> + INF_VERSION = 0x00010005
> + BASE_NAME = Tcg2ConfigPei
> + FILE_GUID = BF7F2B0C-9F2F-4889-AB5C-12460022BE87
> + MODULE_TYPE = PEIM
> + VERSION_STRING = 1.0
> + ENTRY_POINT = Tcg2ConfigPeimEntryPoint
> +
> +[Sources]
> + Tcg2ConfigPeim.c
> + TpmDetection.c
> +
> +[Packages]
> + MdePkg/MdePkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + SecurityPkg/SecurityPkg.dec
> +
> +[LibraryClasses]
> + PeimEntryPoint
> + DebugLib
> + Tpm2DeviceLib
> +
> +[Guids]
> + ## SOMETIMES_CONSUMES ## Variable:L"TCG2_CONFIGURATION"
> + ## SOMETIMES_CONSUMES ## Variable:L"TCG2_DEVICE_DETECTION"
> + gTcg2ConfigFormSetGuid
(5) Can you please drop these three lines? These lines are related to
the UEFI variable(s), but we don't use those.
> + gEfiTpmDeviceSelectedGuid ## PRODUCES ## GUID # Used as a PPI GUID
> + gEfiTpmDeviceInstanceNoneGuid ## SOMETIMES_CONSUMES ## GUID # TPM device identifier
(6) I'll comment more on this later; just a short request here: please
drop "gEfiTpmDeviceInstanceNoneGuid".
The dynamic default that you set in the DSC file is
gEfiTpmDeviceInstanceNoneGuid (all bits zero), and that's sufficient for us.
(7) On the other hand, please list "gEfiTpmDeviceInstanceTpm20DtpmGuid",
as SOMETIMES_CONSUMES.
> +
> +[Ppis]
> + gPeiTpmInitializationDonePpiGuid ## SOMETIMES_PRODUCES
> +
> +[Pcd]
> + gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ## PRODUCES
> + gEfiSecurityPkgTokenSpaceGuid.PcdTpmAutoDetection ## CONSUMES
(8) Please drop PcdTpmAutoDetection, we never want to set that to FALSE
in our DSCs.
> + gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress ## SOMETIMES_CONSUMES
(9) Please drop this as well. This module does not *directly* consume
this PCD. If one of the library instances pulled into the module
consumes the PCD, then it should be spelled out in the INF file of that
library instance.
> +
> +[Depex]
> + gEfiPeiMasterBootModePpiGuid
(10) Please drop the DEPEX (the entire section). This depex exists in
the SecurityPkg original because that module behaves differently (wrt.
detection / cached value) according to the boot mode, and so it has a
dependency on some other PEIM detecting and exposing the boot mode.
Higher up, you correctly removed the S3_RESUME references, and indeed
OVMF's clone of the source ignores the boot mode (also correctly). But
that implies we have no dependency on this PPI.
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> new file mode 100644
> index 000000000000..31f27968401b
> --- /dev/null
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> @@ -0,0 +1,124 @@
> +/** @file
> + The module entry point for Tcg2 configuration module.
> +
> +Copyright (c) 2018, Red Hat, Inc.
> +Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution. The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +
> +#include <PiPei.h>
> +
> +#include <Guid/TpmInstance.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PeiServicesLib.h>
(11) Please make sure that
(11a) whatever <Library/...> headers are included in any *.c or *.h file
are synched with the [LibraryClasses] section in the INF file,
(11b) ditto for <Guid/...> and [Guids] -- already covered by my remarks
(6) and (7) above
(11c) ditto for <Ppi/...> and [Ppis] -- for example, I think we need to
#include <Ppi/TpmInitialized.h> for "gPeiTpmInitializationDonePpiGuid".
> +#include <Tcg/Tcg2Config/Tcg2ConfigNvData.h>
(12) Please drop this, we don't need it.
> +
> +TPM_INSTANCE_ID mTpmInstanceId[] = TPM_INSTANCE_ID_LIST;
(13) Please drop this.
> +
> +CONST EFI_PEI_PPI_DESCRIPTOR gTpmSelectedPpi = {
> + (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST),
> + &gEfiTpmDeviceSelectedGuid,
> + NULL
> +};
(14) Can you please rename this to "mTpmSelectedPpiList"?
> +
> +EFI_PEI_PPI_DESCRIPTOR mTpmInitializationDonePpiList = {
> + EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> + &gPeiTpmInitializationDonePpiGuid,
> + NULL
> +};
(15) Please make both of these global variables STATIC CONST.
> +
> +/**
> + This routine check both SetupVariable and real TPM device, and return final TpmDevice configuration.
> +
> + @param SetupTpmDevice TpmDevice configuration in setup driver
> +
> + @return TpmDevice configuration
> +**/
> +UINT8
> +DetectTpmDevice (
> + IN UINT8 SetupTpmDevice
> + );
(16a) Please remove this function declaration,
(16b) Please remove the DetectTpmDevice() function definition, together
with the "TpmDetection.c" source file,
(16c) Please fold the contents of DetectTpmDevice() -- basically a call
to Tpm2RequestUseTpm() -- into Tcg2ConfigPeimEntryPoint().
> +
> +/**
> + The entry point for Tcg2 configuration driver.
> +
> + @param FileHandle Handle of the file being invoked.
> + @param PeiServices Describes the list of possible PEI Services.
> +
> + @retval EFI_SUCCES Convert variable to PCD successfully.
> + @retval Others Fail to convert variable to PCD.
> +**/
> +EFI_STATUS
> +EFIAPI
> +Tcg2ConfigPeimEntryPoint (
> + IN EFI_PEI_FILE_HANDLE FileHandle,
> + IN CONST EFI_PEI_SERVICES **PeiServices
> + )
> +{
> + UINTN Size;
> + EFI_STATUS Status;
> + EFI_STATUS Status2;
> + TCG2_CONFIGURATION Tcg2Configuration;
> + UINTN Index;
> + UINT8 TpmDevice;
> +
> + Tcg2Configuration.TpmDevice = TPM_DEVICE_2_0_DTPM;
> +
> + DEBUG ((EFI_D_INFO, "OvmfPkg Tcg2ConfigPeimEntryPoint"));
Just some side comments here:
(17a) Nowadays we use DEBUG_xxxx macros, not EFI_D_xxx macros. So this
should be DEBUG_INFO.
(17b) Please don't forget the terminating "\n" for the debug message.
(17c) No need to print "OvmfPkg".
(17d) The preferred method of printing the containing function name is:
DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__));
The "%s" format specifier prints UCS2-encoded wide char strings (arrays
of CHAR16 elements), while the "%a" format specifier prints ASCII
strings (CHAR8 arrays). __FUNCTION__ is expanded to the latter.
> +
> + if (PcdGetBool (PcdTpmAutoDetection)) {
> + TpmDevice = DetectTpmDevice (Tcg2Configuration.TpmDevice);
> + DEBUG ((EFI_D_INFO, "TpmDevice final: %x\n", TpmDevice));
> + if (TpmDevice != TPM_DEVICE_NULL) {
> + Tcg2Configuration.TpmDevice = TpmDevice;
> + }
> + } else {
> + TpmDevice = Tcg2Configuration.TpmDevice;
> + }
> +
> + //
> + // Convert variable to PCD.
> + // This is work-around because there is no gurantee DynamicHiiPcd can return correct value in DXE phase.
> + // Using DynamicPcd instead.
> + //
> + // NOTE: Tcg2Configuration variable contains the desired TpmDevice type,
> + // while PcdTpmInstanceGuid PCD contains the real detected TpmDevice type
> + //
> + for (Index = 0; Index < sizeof(mTpmInstanceId)/sizeof(mTpmInstanceId[0]); Index++) {
> + if (TpmDevice == mTpmInstanceId[Index].TpmDevice) {
> + Size = sizeof(mTpmInstanceId[Index].TpmInstanceGuid);
> + Status = PcdSetPtrS (PcdTpmInstanceGuid, &Size, &mTpmInstanceId[Index].TpmInstanceGuid);
> + ASSERT_EFI_ERROR (Status);
> + DEBUG ((EFI_D_INFO, "TpmDevice PCD: %g\n", &mTpmInstanceId[Index].TpmInstanceGuid));
> + break;
> + }
> + }
(18) So basically all this logic should be simplified to:
- call Tpm2RequestUseTpm()
- if Tpm2RequestUseTpm() succeeds:
- set "PcdTpmInstanceGuid" to "gEfiTpmDeviceInstanceTpm20DtpmGuid"
- install "gEfiTpmDeviceSelectedGuid" via "mTpmSelectedPpiList"
- if Tpm2RequestUseTpm() fails:
- don't touch the PCD, our dynamic default from the DSC is fine
- install "gEfiTpmDeviceSelectedGuid" via "mTpmSelectedPpiList"
- install "gPeiTpmInitializationDonePpiGuid" via
"mTpmInitializationDonePpiList"
(19) DEBUG messages are nice to keep, just please update EFI_D_xxx to
DEBUG_xxx.
(20) I think all of the above "//" comments can be dropped, the
resultant code will be simple enough, and our commit message is detailed.
> +
> + //
> + // Selection done
> + //
> + Status = PeiServicesInstallPpi (&gTpmSelectedPpi);
> + ASSERT_EFI_ERROR (Status);
> +
> + //
> + // Even if no TPM is selected or detected, we still need intall TpmInitializationDonePpi.
> + // Because TcgPei or Tcg2Pei will not run, but we still need a way to notify other driver.
> + // Other driver can know TPM initialization state by TpmInitializedPpi.
> + //
(21) This comment block is nice to keep (under the "Tpm2RequestUseTpm()
fails" branch); I suggest the following variant:
// If no TPM2 was detected, we still need to install
// TpmInitializationDonePpi. Namely, Tcg2Pei will exit early upon
// seeing the default (all-bits-zero) contents of PcdTpmInstanceGuid,
// thus we have to install the PPI in its place, in order to unblock
// any dependent PEIMs.
Thank you!
Laszlo
> + if (CompareGuid (PcdGetPtr(PcdTpmInstanceGuid), &gEfiTpmDeviceInstanceNoneGuid)) {
> + Status2 = PeiServicesInstallPpi (&mTpmInitializationDonePpiList);
> + ASSERT_EFI_ERROR (Status2);
> + }
> +
> + return Status;
> +}
> diff --git a/OvmfPkg/Tcg/Tcg2Config/TpmDetection.c b/OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
> new file mode 100644
> index 000000000000..df222cbff13d
> --- /dev/null
> +++ b/OvmfPkg/Tcg/Tcg2Config/TpmDetection.c
> @@ -0,0 +1,46 @@
> +/** @file
> + TPM2.0 auto detection.
> +
> +Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> +Copyright (C) 2018, Red Hat, Inc.
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution. The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +
> +#include <PiPei.h>
> +
> +#include <Library/DebugLib.h>
> +#include <Library/Tpm2DeviceLib.h>
> +#include <Tcg/Tcg2Config/Tcg2ConfigNvData.h>
> +
> +/**
> + This routine check both SetupVariable and real TPM device, and return final TpmDevice configuration.
> +
> + @param SetupTpmDevice TpmDevice configuration in setup driver
> +
> + @return TpmDevice configuration
> +**/
> +UINT8
> +DetectTpmDevice (
> + IN UINT8 SetupTpmDevice
> + )
> +{
> + EFI_STATUS Status;
> +
> + DEBUG ((EFI_D_INFO, "DetectTpmDevice:\n"));
> +
> + Status = Tpm2RequestUseTpm ();
> + if (EFI_ERROR (Status)) {
> + return TPM_DEVICE_NULL;
> + }
> +
> + return TPM_DEVICE_2_0_DTPM;
> +}
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module
2018-03-08 17:46 ` [Qemu-devel] [edk2] " Laszlo Ersek
@ 2018-03-08 18:10 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 18:10 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: qemu-devel, javierm, pjones, jiewen.yao
On 03/08/18 18:46, Laszlo Ersek wrote:
> (11a) whatever <Library/...> headers are included in any *.c or *.h file
> are synched with the [LibraryClasses] section in the INF file,
Small correction / clarification: the "entry point" lib classes in the
INF file, such as PeimEntryPoint and UefiDriverEntryPoint, need not be
matched with <Library/...> #includes in the source code.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 6/8] ovmf: link with Tcg2Pei module
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 6/8] ovmf: link with Tcg2Pei module marcandre.lureau
@ 2018-03-08 18:20 ` Laszlo Ersek
2018-03-08 18:33 ` Laszlo Ersek
0 siblings, 1 reply; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 18:20 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: qemu-devel, javierm, pjones, jiewen.yao
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> This module will initialize TPM device, measure reported FVs and BIOS
> version. We keep both SHA-1 and SHA-256 for the TCG 1.2 log format
> compatibility, but the SHA-256 measurements and TCG 2 log format are
> now recommended.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> OvmfPkg/OvmfPkgX64.dsc | 7 +++++++
> OvmfPkg/OvmfPkgX64.fdf | 1 +
> 2 files changed, 8 insertions(+)
(1) Please change the subject line to:
OvmfPkg: include Tcg2Pei module
>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 64bd6b6a9f08..3fa1a31f4c37 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -278,6 +278,8 @@ [LibraryClasses.common.PEIM]
> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>
> !if $(TPM2_ENABLE)
> + BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
(2) Technically this makes sense, but given the fact that we resolve
BaseCryptLib unconditionally for a bunch of other module types, I think
we should do that for PEIMs as well.
> + HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> !endif
>
> @@ -615,6 +617,11 @@ [Components]
>
> !if $(TPM2_ENABLE) == TRUE
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> + SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> + <LibraryClasses>
> + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> + NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> + }
> !endif
>
> #
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index dbafada5226b..c0173e7adf5f 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -167,6 +167,7 @@ [FV.PEIFV]
>
> !if $(TPM2_ENABLE) == TRUE
> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> !endif
>
> ################################################################################
>
Looks good. (The final version should handle the other DSC / FDF files too.)
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 6/8] ovmf: link with Tcg2Pei module
2018-03-08 18:20 ` [Qemu-devel] [edk2] " Laszlo Ersek
@ 2018-03-08 18:33 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 18:33 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: qemu-devel, javierm, pjones, jiewen.yao
On 03/08/18 19:20, Laszlo Ersek wrote:
> On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> This module will initialize TPM device, measure reported FVs and BIOS
>> version. We keep both SHA-1 and SHA-256 for the TCG 1.2 log format
>> compatibility, but the SHA-256 measurements and TCG 2 log format are
>> now recommended.
>>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>> OvmfPkg/OvmfPkgX64.dsc | 7 +++++++
>> OvmfPkg/OvmfPkgX64.fdf | 1 +
>> 2 files changed, 8 insertions(+)
>
> (1) Please change the subject line to:
>
> OvmfPkg: include Tcg2Pei module
>
>>
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index 64bd6b6a9f08..3fa1a31f4c37 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -278,6 +278,8 @@ [LibraryClasses.common.PEIM]
>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>>
>> !if $(TPM2_ENABLE)
>> + BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
>
> (2) Technically this makes sense, but given the fact that we resolve
> BaseCryptLib unconditionally for a bunch of other module types, I think
> we should do that for PEIMs as well.
>
>> + HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
(3) Actually, can you please move this library resolution under
"Tcg2Pei.inf"? Every single PEIM that uses this library instance will
need us to spell out the individual hash plugins for it anyway. So I
think keeping the "hash router" lib instance together with those
NULL-class instances is cleaner.
Thanks
Laszlo
>> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
>> !endif
>>
>> @@ -615,6 +617,11 @@ [Components]
>>
>> !if $(TPM2_ENABLE) == TRUE
>> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> + SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
>> + <LibraryClasses>
>> + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
>> + NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>> + }
>> !endif
>>
>> #
>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
>> index dbafada5226b..c0173e7adf5f 100644
>> --- a/OvmfPkg/OvmfPkgX64.fdf
>> +++ b/OvmfPkg/OvmfPkgX64.fdf
>> @@ -167,6 +167,7 @@ [FV.PEIFV]
>>
>> !if $(TPM2_ENABLE) == TRUE
>> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> +INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
>> !endif
>>
>> ################################################################################
>>
>
> Looks good. (The final version should handle the other DSC / FDF files too.)
>
> Thanks!
> Laszlo
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 7/8] ovmf: link with Tcg2Dxe module
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 7/8] ovmf: link with Tcg2Dxe module marcandre.lureau
@ 2018-03-08 19:14 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 19:14 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel; +Cc: qemu-devel, javierm, pjones, jiewen.yao
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> This module measures and log the boot environment. It also produces
> the Tcg2 protocol, which allows for example to read the log from OS.
>
> The linux kernel doesn't yet read the EFI_TCG2_EVENT_LOG_FORMAT_TCG_2,
> which is required for crypto-agile log. In fact, only upcoming 4.16
> adds support EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2:
>
> [ 0.000000] efi: EFI v2.70 by EDK II
> [ 0.000000] efi: SMBIOS=0x3fa1f000 ACPI=0x3fbb6000 ACPI 2.0=0x3fbb6014 MEMATTR=0x3e7d4318 TPMEventLog=0x3db21018
>
> $ python chipsec_util.py tpm parse_log binary_bios_measurements
>
> [CHIPSEC] Version 1.3.5.dev2
> [CHIPSEC] API mode: using OS native API (not using CHIPSEC kernel module)
> [CHIPSEC] Executing command 'tpm' with args ['parse_log', '/tmp/binary_bios_measurements']
>
> PCR: 0 type: EV_S_CRTM_VERSION size: 0x2 digest: 1489f923c4dca729178b3e3233458550d8dddf29
> + version:
> PCR: 0 type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10 digest: fd39ced7c0d2a61f6830c78c7625f94826b05bcc
> + base: 0x820000 length: 0xe0000
> PCR: 0 type: EV_EFI_PLATFORM_FIRMWARE_BLOB size: 0x10 digest: 39ebc6783b72bc1e73c7d5bcfeb5f54a3f105d4c
> + base: 0x900000 length: 0xa00000
> PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x35 digest: 57cd4dc19442475aa82743484f3b1caa88e142b8
> PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24 digest: 9b1387306ebb7ff8e795e7be77563666bbf4516e
> PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26 digest: 9afa86c507419b8570c62167cb9486d9fc809758
> PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x24 digest: 5bf8faa078d40ffbd03317c93398b01229a0e1e0
> PCR: 7 type: EV_EFI_VARIABLE_DRIVER_CONFIG size: 0x26 digest: 734424c9fe8fc71716c42096f4b74c88733b175e
> PCR: 7 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x3e digest: 252f8ebb85340290b64f4b06a001742be8e5cab6
> PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x6e digest: 22a4f6ee9af6dba01d3528deb64b74b582fc182b
> PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x80 digest: b7811d5bf30a7efd4e385c6179fe10d9290bb9e8
> PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x84 digest: 425e502c24fc924e231e0a62327b6b7d1f704573
> PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x9a digest: 0b5d2c98ac5de6148a4a1490ff9d5df69039f04e
> PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0xbd digest: 20bd5f402271d57a88ea314fe35c1705956b1f74
> PCR: 1 type: EV_EFI_VARIABLE_BOOT size: 0x88 digest: df5d6605cb8f4366d745a8464cfb26c1efdc305c
> PCR: 4 type: EV_EFI_ACTION size: 0x28 digest: cd0fdb4531a6ec41be2753ba042637d6e5f7f256
> PCR: 0 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 1 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 2 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 3 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 4 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
> PCR: 5 type: EV_SEPARATOR size: 0x4 digest: 9069ca78e7450a285173431b3e52c5c25299e473
>
> $ tpm2_pcrlist
> sha1 :
> 0 : 35bd1786b6909daad610d7598b1d620352d33b8a
> 1 : ec0511e860206e0af13c31da2f9e943fb6ca353d
> 2 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
> 3 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
> 4 : 45a323382bd933f08e7f0e256bc8249e4095b1ec
> 5 : d16d7e629fd8d08ca256f9ad3a3a1587c9e6cc1b
> 6 : b2a83b0ebf2f8374299a5b2bdfc31ea955ad7236
> 7 : 518bd167271fbb64589c61e43d8c0165861431d8
> 8 : 0000000000000000000000000000000000000000
> 9 : 0000000000000000000000000000000000000000
> 10 : 0000000000000000000000000000000000000000
> 11 : 0000000000000000000000000000000000000000
> 12 : 0000000000000000000000000000000000000000
> 13 : 0000000000000000000000000000000000000000
> 14 : 0000000000000000000000000000000000000000
> 15 : 0000000000000000000000000000000000000000
> 16 : 0000000000000000000000000000000000000000
> 17 : ffffffffffffffffffffffffffffffffffffffff
> 18 : ffffffffffffffffffffffffffffffffffffffff
> 19 : ffffffffffffffffffffffffffffffffffffffff
> 20 : ffffffffffffffffffffffffffffffffffffffff
> 21 : ffffffffffffffffffffffffffffffffffffffff
> 22 : ffffffffffffffffffffffffffffffffffffffff
> 23 : 0000000000000000000000000000000000000000
> sha256 :
> 0 : 9ae903dbae3357ac00d223660bac19ea5c021499a56201104332ab966631ce2c
> 1 : acc611d90245cf04e77b0ca94901f90e7fa54770f0426f53c3049b532243d1b8
> 2 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
> 3 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
> 4 : 7a94ffe8a7729a566d3d3c577fcb4b6b1e671f31540375f80eae6382ab785e35
> 5 : a5ceb755d043f32431d63e39f5161464620a3437280494b5850dc1b47cc074e0
> 6 : 3d458cfe55cc03ea1f443f1562beec8df51c75e14a9fcf9a7234a13f198e7969
> 7 : 65caf8dd1e0ea7a6347b635d2b379c93b9a1351edc2afc3ecda700e534eb3068
> 8 : 0000000000000000000000000000000000000000000000000000000000000000
> 9 : 0000000000000000000000000000000000000000000000000000000000000000
> 10 : 0000000000000000000000000000000000000000000000000000000000000000
> 11 : 0000000000000000000000000000000000000000000000000000000000000000
> 12 : 0000000000000000000000000000000000000000000000000000000000000000
> 13 : 0000000000000000000000000000000000000000000000000000000000000000
> 14 : 0000000000000000000000000000000000000000000000000000000000000000
> 15 : 0000000000000000000000000000000000000000000000000000000000000000
> 16 : 0000000000000000000000000000000000000000000000000000000000000000
> 17 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> 18 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> 19 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> 20 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> 21 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> 22 : ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff
> 23 : 0000000000000000000000000000000000000000000000000000000000000000
> sha384 :
>
> The PhysicalPresenceLib is required, it sets some variables, but the
> firmware doesn't act on it yet.
>
> Laszlo Ersek explained on the list why Tpm2DeviceLib has to be
> resolved differently for DXE_DRIVER modules in general and for
> "Tcg2Dxe.inf" specifically:
>
> * We have a library class called Tpm2DeviceLib -- this is basically the
> set of APIs declared in "SecurityPkg/Include/Library/Tpm2DeviceLib.h".
> Its leading comment says "This library abstract how to access TPM2
> hardware device".
>
> There are two *sets* of APIs in "Tpm2DeviceLib.h":
>
> (a) functions that deal with the TPM2 device:
> - Tpm2RequestUseTpm(),
> - Tpm2SubmitCommand()
>
> This set of APIs is supposed to be used by clients that *consume*
> the TPM2 device abstraction.
>
> (b) the function Tpm2RegisterTpm2DeviceLib(), which is supposed to be
> used by *providers* of various TPM2 device abstractions.
>
> * Then, we have two implementations (instances) of the Tpm2DeviceLib class:
> (1) SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
> (2) SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
>
> (1) The first library instance ("Tpm2DeviceLibTcg2.inf") implements the
> APIs listed under (a), and it does not implement (b) -- see
> EFI_UNSUPPORTED. In other words, this lib instance is strictly meant for
> drivers that *consume* the TPM2 device abstraction. And, the (a) group
> of APIs is implemented by forwarding the requests to the TCG2 protocol.
>
> The idea here is that all the drivers that consume the TPM2 abstraction
> do not have to be statically linked with a large TPM2 device library
> instance; instead they are only linked (statically) with this "thin"
> library instance, and all the actual work is delegated to whichever
> driver that provides the singleton TCG2 protocol.
>
> (2) The second library instance ("Tpm2DeviceLibRouterDxe.inf") is meant
> for the driver that offers (produces) the TCG2 protocol. This lib
> instance implements both (a) and (b) API groups.
>
> * Here's how things fit together:
>
> (i) The "SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf"
> library instance (which has no lib class) is linked into "Tcg2Dxe.inf"
> via NULL class resolution. This simply means that before the
> "Tcg2Dxe.inf" entry point function is entered, the constructor function
> of "Tpm2InstanceLibDTpm.inf" will be called.
>
> (ii) This Tpm2InstanceLibDTpmConstructor() function calls API (b), and
> registers its own actual TPM2 command implementation with the
> "Tpm2DeviceLibRouter" library instance (also linked into the Tcg2Dxe
> driver). This provides the back-end for the API set (a).
>
> TCG2 protocol provider (Tcg2Dxe.inf driver) launches
> |
> v
> NULL class: Tpm2InstanceLibDTpm instance construction
> |
> v
> Tpm2DeviceLib class: Tpm2DeviceLibRouter instance
> backend registration for API set (a)
>
> (iii) The Tcg2Dxe driver exposes the TCG2 protocol.
>
> (iv) A TPM2 consumer calls API set (a) via lib instance (1). Such calls
> land in Tcg2Dxe, via the protocol.
>
> (v) Tcg2Dxe serves the protocol request by forwarding it to API set (a)
> from lib instance (2).
>
> (vi) Those functions call the "backend" functions registered by
> Tpm2DeviceLibDTpm in step (ii).
>
> TPM 2 consumer driver
> |
> v
> Tpm2DeviceLib class: Tpm2DeviceLibTcg2 instance
> |
> v
> TCG2 protocol interface
> |
> v
> TCG2 protocol provider: Tcg2Dxe.inf driver
> |
> v
> Tpm2DeviceLib class: Tpm2DeviceLibRouter instance
> |
> v
> NULL class: Tpm2InstanceLibDTpm instance
> (via earlier registration)
> |
> v
> TPM2 chip (actual hardware)
>
> * So that is the "router" pattern in edk2. Namely,
>
> - Consumers of an abstraction use a thin library instance.
>
> - The thin library instance calls a firmware-global (singleton) service,
> i.e. a PPI (in the PEI phase) or protocol (in the DXE phase).
>
> - The PEIM providing the PPI, or the DXE driver providing the protocol,
> don't themselves implement the actual service either. Instead they
> offer a "registration" service too, and they only connect the incoming
> "consumer" calls to the earlier registered back-end(s).
>
> - The "registration service", for back-ends to use, may take various
> forms.
>
> It can be exposed globally to the rest of the firmware, as
> another member function of the PPI / protocol structure. Then backends
> can be provided by separate PEIMs / DXE drivers.
>
> Or else, the registration service can be exposed as just another
> library API. In this case, the backends are provided as NULL class
> library instances, and a platform DSC file links them into the PEIM /
> DXE driver via NULL class resolutions. The backend lib instances call
> the registration service in their own respective constructor
> functions.
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> OvmfPkg/OvmfPkgX64.dsc | 16 ++++++++++++++++
> OvmfPkg/OvmfPkgX64.fdf | 4 ++++
> 2 files changed, 20 insertions(+)
(1) Please change the subject line to:
OvmfPkg: include Tcg2Dxe module
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 3fa1a31f4c37..7753852144fb 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -211,6 +211,8 @@ [LibraryClasses]
>
> !if $(TPM2_ENABLE) == TRUE
> Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> + Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
> + Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> !endif
>
> [LibraryClasses.common]
> @@ -362,6 +364,10 @@ [LibraryClasses.common.DXE_DRIVER]
> PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
> MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
> +!if $(TPM2_ENABLE)
> + HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
(2) Can you move this HashLib resolution under Tcg2Dxe.inf?
> + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
> +!endif
>
> [LibraryClasses.common.UEFI_APPLICATION]
> PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> @@ -642,6 +648,16 @@ [Components]
>
> MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>
> +!if $(TPM2_ENABLE) == TRUE
> + SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
> + <LibraryClasses>
> + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
> + NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> + NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> + }
> +!endif
> +
> MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> <LibraryClasses>
> !if $(SECURE_BOOT_ENABLE) == TRUE
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index c0173e7adf5f..1a46104fc63d 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -392,6 +392,10 @@ [FV.DXEFV]
> INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> !endif
>
> +!if $(TPM2_ENABLE) == TRUE
> +INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> +!endif
> +
(3) In the DSC file, you are adding Tcg2Dxe between RuntimeDxe and
SecurityStubDxe; can you please stick with the same in the FDF file?
It's easier to read.
Alternatively, you can add Tcg2Dxe after VariableRuntimeDxe too, of
course, but then the DSC file should follow suit. Makes no difference
functionally, but it's easier to read.
> ################################################################################
>
> [FV.FVMAIN_COMPACT]
>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib marcandre.lureau
@ 2018-03-08 19:54 ` Laszlo Ersek
2018-03-08 19:56 ` Laszlo Ersek
2018-03-09 0:39 ` Yao, Jiewen
0 siblings, 2 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 19:54 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel, jiewen.yao
Cc: pjones, stefanb, qemu-devel, javierm
(Jiewen, below I have a question for you as well; please help with that.)
On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The library registers a security management handler, to measure images
> that are not measure in PEI phase.
>
> This seems to work for example with the qemu PXE rom:
>
> Loading driver at 0x0003E6C2000 EntryPoint=0x0003E6C9076 8086100e.efi
>
> And the following binary_bios_measurements log entry seems to be
> added:
>
> PCR: 2 type: EV_EFI_BOOT_SERVICES_DRIVER size: 0x4e digest: 70a22475e9f18806d2ed9193b48d80d26779d9a4
>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> OvmfPkg/OvmfPkgX64.dsc | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 7753852144fb..9db1712e3623 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -662,6 +662,9 @@ [Components]
> <LibraryClasses>
> !if $(SECURE_BOOT_ENABLE) == TRUE
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> +!endif
> +!if $(TPM2_ENABLE) == TRUE
> + NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> !endif
> }
>
>
(1) Marc-André, please change the subject line to:
OvmfPkg: plug DxeTpm2MeasureBootLib into SecurityStubDxe
(2) I have a question for Jiewen:
DxeTpm2MeasureBootLib consumes the TCG2 protocol, but it does not depend
on it with a DEPEX. Instead, DxeTpm2MeasureBootHandler() tries to locate
the protocol on every invocation.
This means that SecurityStubDxe may produce the Security and Security2
Architectural Protocols before measurements into the TPM2 device are
possible. Therefore, UEFI_DRIVER modules (which depend on all of the
Arch protocols) may be started before they can be measured into the TPM.
Now, this is likely no problem for UEFI_DRIVER modules that are built
into the firmware volume(s), because those are measured by Tcg2Pei
anyway. However, it would be a problem for UEFI_DRIVER modules / apps
that come from external media (disk, network, PCI oprom, etc).
However, such are loaded only in the BDS phase, and BDS is only entered
after all of the DXE drivers are dispatched from the firmware volumes.
In other words, the ordering between Tcg2Dxe and external UEFI_DRIVER /
UEFI_APPLICATION modules is ensured that Tcg2Dxe will be dispatched in
the DXE phase, while the latter will only be loaded in BDS.
Is this intentional? Is my understanding correct?
(3) If that's the case, then Marc-André, please add the following to the
commit message:
--------
Hooking DxeTpm2MeasureBootLib into SecurityStubDxe ensures that the
Security and Security2 Arch protocols will entail, by the time of
entering the BDS phase, the measuring of UEFI binaries into the TPM.
Thus, external UEFI_DRIVER and UEFI_APPLICATION modules (which are
loaded in the BDS phase, from disk, network, PCI oprom, etc) will be
measured.
Drivers dispatched in the DXE phase before Tcg2Dxe will not be measured
individually; however such drivers come from the firmware volume(s), and
those are measured in the PEI phase by Tcg2Pei.
--------
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
2018-03-08 19:54 ` Laszlo Ersek
@ 2018-03-08 19:56 ` Laszlo Ersek
2018-03-09 0:39 ` Yao, Jiewen
1 sibling, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-08 19:56 UTC (permalink / raw)
To: marcandre.lureau, edk2-devel, jiewen.yao
Cc: pjones, stefanb, qemu-devel, javierm
On 03/08/18 20:54, Laszlo Ersek wrote:
> In other words, the ordering between Tcg2Dxe and external UEFI_DRIVER /
> UEFI_APPLICATION modules is ensured that Tcg2Dxe will be dispatched in
> the DXE phase, while the latter will only be loaded in BDS.
Sigh, I meant:
The ordering between Tcg2Dxe and external UEFI_DRIVER / UEFI_APPLICATION
modules is ensured *by the fact* that Tcg2Dxe will be dispatched in the
DXE phase, while the latter will only be loaded in BDS.
Thanks,
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
2018-03-08 19:54 ` Laszlo Ersek
2018-03-08 19:56 ` Laszlo Ersek
@ 2018-03-09 0:39 ` Yao, Jiewen
2018-03-09 0:47 ` Yao, Jiewen
2018-03-09 10:26 ` Laszlo Ersek
1 sibling, 2 replies; 36+ messages in thread
From: Yao, Jiewen @ 2018-03-09 0:39 UTC (permalink / raw)
To: Laszlo Ersek, marcandre.lureau, edk2-devel
Cc: pjones, stefanb, qemu-devel, javierm
Very good question.
Comment below:
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Friday, March 9, 2018 3:54 AM
> To: marcandre.lureau@redhat.com; edk2-devel@lists.01.org; Yao, Jiewen
> <jiewen.yao@intel.com>
> Cc: pjones@redhat.com; stefanb@linux.vnet.ibm.com;
> qemu-devel@nongnu.org; javierm@redhat.com
> Subject: Re: [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
>
> (Jiewen, below I have a question for you as well; please help with that.)
>
> On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > The library registers a security management handler, to measure images
> > that are not measure in PEI phase.
> >
> > This seems to work for example with the qemu PXE rom:
> >
> > Loading driver at 0x0003E6C2000 EntryPoint=0x0003E6C9076 8086100e.efi
> >
> > And the following binary_bios_measurements log entry seems to be
> > added:
> >
> > PCR: 2 type: EV_EFI_BOOT_SERVICES_DRIVER size: 0x4e digest:
> 70a22475e9f18806d2ed9193b48d80d26779d9a4
> >
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > OvmfPkg/OvmfPkgX64.dsc | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index 7753852144fb..9db1712e3623 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -662,6 +662,9 @@ [Components]
> > <LibraryClasses>
> > !if $(SECURE_BOOT_ENABLE) == TRUE
> >
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> > +!endif
> > +!if $(TPM2_ENABLE) == TRUE
> > +
> NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.i
> nf
> > !endif
> > }
> >
> >
>
> (1) Marc-André, please change the subject line to:
>
> OvmfPkg: plug DxeTpm2MeasureBootLib into SecurityStubDxe
>
>
> (2) I have a question for Jiewen:
>
> DxeTpm2MeasureBootLib consumes the TCG2 protocol, but it does not depend
> on it with a DEPEX. Instead, DxeTpm2MeasureBootHandler() tries to locate
> the protocol on every invocation.
[Jiewen] Yes.
> This means that SecurityStubDxe may produce the Security and Security2
> Architectural Protocols before measurements into the TPM2 device are
> possible.
[Jiewen] Yes.
> Therefore, UEFI_DRIVER modules (which depend on all of the
> Arch protocols) may be started before they can be measured into the TPM.
>
> Now, this is likely no problem for UEFI_DRIVER modules that are built
> into the firmware volume(s), because those are measured by Tcg2Pei
> anyway.
[Jiewen] That is TRUE.
However, it would be a problem for UEFI_DRIVER modules / apps
> that come from external media (disk, network, PCI oprom, etc).
[Jiewen] By design, the 3rd part module should not be invoked before EndOfDxe.
All Arch Protocol Ready is not strong enough. :-)
Please refer to https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c
If a non-FV image is loaded before EndOfDxe, it will be queued into mDeferred3rdPartyImage.
We also added EfiBootManagerDispatchDeferredImages() API in https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Library/UefiBootManagerLib.h and implemented in https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
A platform must call EfiBootManagerDispatchDeferredImages(), if the platform supports PCI OROM.
You can find the sample code in https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/Bds/Library/DxePlatformBootManagerLib/BdsPlatform.c
> However, such are loaded only in the BDS phase, and BDS is only entered
> after all of the DXE drivers are dispatched from the firmware volumes.
> In other words, the ordering between Tcg2Dxe and external UEFI_DRIVER /
> UEFI_APPLICATION modules is ensured that Tcg2Dxe will be dispatched in
> the DXE phase, while the latter will only be loaded in BDS.
>
> Is this intentional? Is my understanding correct?
[Jiewen] Right. The only assumption is: Tcg2Dxe is included in the firmware volume and it is dispatched before EndOfDxe.
>
> (3) If that's the case, then Marc-André, please add the following to the
> commit message:
>
> --------
> Hooking DxeTpm2MeasureBootLib into SecurityStubDxe ensures that the
> Security and Security2 Arch protocols will entail, by the time of
> entering the BDS phase, the measuring of UEFI binaries into the TPM.
> Thus, external UEFI_DRIVER and UEFI_APPLICATION modules (which are
> loaded in the BDS phase, from disk, network, PCI oprom, etc) will be
> measured.
>
> Drivers dispatched in the DXE phase before Tcg2Dxe will not be measured
> individually; however such drivers come from the firmware volume(s), and
> those are measured in the PEI phase by Tcg2Pei.
> --------
>
> Thanks!
> Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
2018-03-09 0:39 ` Yao, Jiewen
@ 2018-03-09 0:47 ` Yao, Jiewen
2018-03-09 10:26 ` Laszlo Ersek
1 sibling, 0 replies; 36+ messages in thread
From: Yao, Jiewen @ 2018-03-09 0:47 UTC (permalink / raw)
To: Yao, Jiewen, Laszlo Ersek, marcandre.lureau, edk2-devel
Cc: javierm, pjones, qemu-devel
Besides the comment below, I should have used the example in OvmfPkg.
Please refer to https://github.com/tianocore/edk2/blob/master/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
The EfiBootManagerDispatchDeferredImages() API call is added just after gEfiDxeSmmReadyToLockProtocolGuid.
So I don’t see any problem in OVMF pkg.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Friday, March 9, 2018 8:39 AM
> To: Laszlo Ersek <lersek@redhat.com>; marcandre.lureau@redhat.com;
> edk2-devel@lists.01.org
> Cc: javierm@redhat.com; pjones@redhat.com; qemu-devel@nongnu.org
> Subject: Re: [edk2] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
>
> Very good question.
> Comment below:
>
> > -----Original Message-----
> > From: Laszlo Ersek [mailto:lersek@redhat.com]
> > Sent: Friday, March 9, 2018 3:54 AM
> > To: marcandre.lureau@redhat.com; edk2-devel@lists.01.org; Yao, Jiewen
> > <jiewen.yao@intel.com>
> > Cc: pjones@redhat.com; stefanb@linux.vnet.ibm.com;
> > qemu-devel@nongnu.org; javierm@redhat.com
> > Subject: Re: [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
> >
> > (Jiewen, below I have a question for you as well; please help with that.)
> >
> > On 03/07/18 16:57, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > The library registers a security management handler, to measure images
> > > that are not measure in PEI phase.
> > >
> > > This seems to work for example with the qemu PXE rom:
> > >
> > > Loading driver at 0x0003E6C2000 EntryPoint=0x0003E6C9076 8086100e.efi
> > >
> > > And the following binary_bios_measurements log entry seems to be
> > > added:
> > >
> > > PCR: 2 type: EV_EFI_BOOT_SERVICES_DRIVER size: 0x4e digest:
> > 70a22475e9f18806d2ed9193b48d80d26779d9a4
> > >
> > > Cc: Laszlo Ersek <lersek@redhat.com>
> > > Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > OvmfPkg/OvmfPkgX64.dsc | 3 +++
> > > 1 file changed, 3 insertions(+)
> > >
> > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > > index 7753852144fb..9db1712e3623 100644
> > > --- a/OvmfPkg/OvmfPkgX64.dsc
> > > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > > @@ -662,6 +662,9 @@ [Components]
> > > <LibraryClasses>
> > > !if $(SECURE_BOOT_ENABLE) == TRUE
> > >
> >
> NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> > > +!endif
> > > +!if $(TPM2_ENABLE) == TRUE
> > > +
> >
> NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.i
> > nf
> > > !endif
> > > }
> > >
> > >
> >
> > (1) Marc-André, please change the subject line to:
> >
> > OvmfPkg: plug DxeTpm2MeasureBootLib into SecurityStubDxe
> >
> >
> > (2) I have a question for Jiewen:
> >
> > DxeTpm2MeasureBootLib consumes the TCG2 protocol, but it does not depend
> > on it with a DEPEX. Instead, DxeTpm2MeasureBootHandler() tries to locate
> > the protocol on every invocation.
> [Jiewen] Yes.
>
> > This means that SecurityStubDxe may produce the Security and Security2
> > Architectural Protocols before measurements into the TPM2 device are
> > possible.
> [Jiewen] Yes.
>
> > Therefore, UEFI_DRIVER modules (which depend on all of the
> > Arch protocols) may be started before they can be measured into the TPM.
> >
> > Now, this is likely no problem for UEFI_DRIVER modules that are built
> > into the firmware volume(s), because those are measured by Tcg2Pei
> > anyway.
> [Jiewen] That is TRUE.
>
> However, it would be a problem for UEFI_DRIVER modules / apps
> > that come from external media (disk, network, PCI oprom, etc).
> [Jiewen] By design, the 3rd part module should not be invoked before EndOfDxe.
> All Arch Protocol Ready is not strong enough. :-)
> Please refer to
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Sec
> urityStubDxe/Defer3rdPartyImageLoad.c
>
> If a non-FV image is loaded before EndOfDxe, it will be queued into
> mDeferred3rdPartyImage.
>
> We also added EfiBootManagerDispatchDeferredImages() API in
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Librar
> y/UefiBootManagerLib.h and implemented in
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiB
> ootManagerLib/BmMisc.c
> A platform must call EfiBootManagerDispatchDeferredImages(), if the platform
> supports PCI OROM.
>
> You can find the sample code in
> https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform
> /Intel/MinPlatformPkg/Bds/Library/DxePlatformBootManagerLib/BdsPlatform.c
>
>
>
> > However, such are loaded only in the BDS phase, and BDS is only entered
> > after all of the DXE drivers are dispatched from the firmware volumes.
> > In other words, the ordering between Tcg2Dxe and external UEFI_DRIVER /
> > UEFI_APPLICATION modules is ensured that Tcg2Dxe will be dispatched in
> > the DXE phase, while the latter will only be loaded in BDS.
> >
> > Is this intentional? Is my understanding correct?
>
> [Jiewen] Right. The only assumption is: Tcg2Dxe is included in the firmware
> volume and it is dispatched before EndOfDxe.
>
>
>
> >
> > (3) If that's the case, then Marc-André, please add the following to the
> > commit message:
> >
> > --------
> > Hooking DxeTpm2MeasureBootLib into SecurityStubDxe ensures that the
> > Security and Security2 Arch protocols will entail, by the time of
> > entering the BDS phase, the measuring of UEFI binaries into the TPM.
> > Thus, external UEFI_DRIVER and UEFI_APPLICATION modules (which are
> > loaded in the BDS phase, from disk, network, PCI oprom, etc) will be
> > measured.
> >
> > Drivers dispatched in the DXE phase before Tcg2Dxe will not be measured
> > individually; however such drivers come from the firmware volume(s), and
> > those are measured in the PEI phase by Tcg2Pei.
> > --------
> >
> > Thanks!
> > Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support
2018-03-08 13:59 ` Marc-André Lureau
@ 2018-03-09 3:03 ` Shi, Steven
2018-03-09 13:54 ` Stefan Berger
0 siblings, 1 reply; 36+ messages in thread
From: Shi, Steven @ 2018-03-09 3:03 UTC (permalink / raw)
To: Marc-André Lureau
Cc: edk2-devel, lersek, pjones, Yao, Jiewen, qemu-devel, javierm,
Stefan Berger
Hi Marcandre,
Thanks for your command steps and I tried them, but my qemu failed to connect the socket tpmemu.sock. When I added the control channel to the TPM, the swtpm socket command stuck there and never exit. Not sure whether it was successful.
Below are the command steps running output in my side
> Then you can run:
> mkdir tpmstatedir
> swtpm_setup.sh --tpm2 --tpm-state tpmstatedir
$ swtpm_setup.sh --tpm2 --tpm-state tpmstatedir
Starting vTPM manufacturing as jshi19:jshi19 @ 2018年03月09日 星期五 10时28分39秒
TPM is listening on TCP port 47364.
Successfully authored TPM state.
Ending vTPM manufacturing @ 2018年03月09日 星期五 10时28分39秒
> Run the emulator:
> swtpm socket --tpmstate dir=tpmstatedir --ctrl type=unixio,path=tpmemu.sock --tpm2
$ swtpm socket --tpmstate dir=tpmstatedir --ctrl type=unixio,path=tpmemu.sock --tpm2
(the swtpm socket command stuck there and never exit)
> Run qemu (from git) with ovmf (with this series):
> qemu ... -chardev socket,id=chrtpm,path=tpmemu.sock -tpmdev
> emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
> -drive if=pflash,format=raw,file=OVMF_CODE.fd,readonly -drive
> if=pflash,format=raw,file=OVMF_VARS.fd ..
$ qemu-system-x86_64 -serial file:serial.log -m 5120 -hda fat:. -monitor stdio --enable-kvm -smp 4 -bios ../Ovmf3264/NOOPT_GCC5/FV/OVMF.fd -chardev socket,id=chrtpm,path=tpmemu.sock -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
qemu-system-x86_64: -chardev socket,id=chrtpm,path=tpmemu.sock: Failed to connect socket tpmemu.sock: No such file or directory
I use the latest version qemu as below:
$ qemu-system-x86_64 --version
QEMU emulator version 2.11.50 (v2.10.0-4184-g930b01138b-dirty)
Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
Thanks
Steven Shi
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
2018-03-09 0:39 ` Yao, Jiewen
2018-03-09 0:47 ` Yao, Jiewen
@ 2018-03-09 10:26 ` Laszlo Ersek
2018-03-09 11:37 ` [Qemu-devel] [edk2] " Yao, Jiewen
1 sibling, 1 reply; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-09 10:26 UTC (permalink / raw)
To: Yao, Jiewen, marcandre.lureau, edk2-devel
Cc: pjones, stefanb, qemu-devel, javierm
On 03/09/18 01:39, Yao, Jiewen wrote:
> Very good question.
> Comment below:
>
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Friday, March 9, 2018 3:54 AM
>> To: marcandre.lureau@redhat.com; edk2-devel@lists.01.org; Yao, Jiewen
>> <jiewen.yao@intel.com>
>> Cc: pjones@redhat.com; stefanb@linux.vnet.ibm.com;
>> qemu-devel@nongnu.org; javierm@redhat.com
>> Subject: Re: [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
>> However, it would be a problem for UEFI_DRIVER modules / apps
>> that come from external media (disk, network, PCI oprom, etc).
> [Jiewen] By design, the 3rd part module should not be invoked before
> EndOfDxe. All Arch Protocol Ready is not strong enough. :-) Please
> refer to
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c
>
> If a non-FV image is loaded before EndOfDxe, it will be queued into
> mDeferred3rdPartyImage.
OK, thank you -- I suspected image deferral was somehow related to this.
I remember deferred execution from the OVMF log, even though at present
we hook only DxeImageVerificationLib into SecurityStubDxe.
> We also added EfiBootManagerDispatchDeferredImages() API in
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Library/UefiBootManagerLib.h
> and implemented in
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
> A platform must call EfiBootManagerDispatchDeferredImages(), if the
> platform supports PCI OROM.
Yep, OVMF does that already. In
"OvmfPkg/Library/PlatformBootManagerLib", we have the following order of
operations, in PlatformBootManagerBeforeConsole():
(a) connect all PCI root bridges non-recursively (this produces PciIo
instances and discovers the PCI oproms)
(b) kick QEMU so that it generates the ACPI tables that OVMF has to
install (this depends on PciIo instances existing, from step (1a)),
and actually install those tables
(c) signal End-of-Dxe (this depends on step (1b), because S3 support
needs the FACS table coming from step (1b))
(d) write an INFO opcode to the S3 boot script so that the boot script
is never empty
(e) install DxeSmmReadyToLock (this may come only after steps (1c) and
(1d))
(f) we call EfiBootManagerDispatchDeferredImages(). This is from Ray's
commit 9789894e3ba3 ("OvmfPkg/PlatformBds: Dispatch deferred images
after EndOfDxe", 2016-11-10).
(The rest of PlatformBootManagerBeforeConsole() skipped here.)
So, we have all the right operations in place, I just missed that non-FV
images loaded before End-of-Dxe (such as PCI oproms) are deferred until
step (f), and that the deferral will cover the TPM measurements too.
This is great, because it's an even stronger guarantee than what I would
have wished for. I mean the argument I wrote up earlier -- about oproms
not being dispatched before entering BDS, at which point Tcg2Dxe will
have been dispatched already -- was already good enough, but now I know
that PCI oproms are delayed even *within* BDS until after we explicitly
end DXE and dispatch the deferred images.
> You can find the sample code in
> https://github.com/tianocore/edk2-platforms/blob/devel-MinPlatform/Platform/Intel/MinPlatformPkg/Bds/Library/DxePlatformBootManagerLib/BdsPlatform.c
>
>
>
>> However, such are loaded only in the BDS phase, and BDS is only
>> entered after all of the DXE drivers are dispatched from the firmware
>> volumes. In other words, the ordering between Tcg2Dxe and external
>> UEFI_DRIVER / UEFI_APPLICATION modules is ensured that Tcg2Dxe will
>> be dispatched in the DXE phase, while the latter will only be loaded
>> in BDS.
>>
>> Is this intentional? Is my understanding correct?
>
> [Jiewen] Right. The only assumption is: Tcg2Dxe is included in the
> firmware volume and it is dispatched before EndOfDxe.
Perfect. Thank you.
So, Marc-André, I would request the following commit message *addition*
(not replacement!) then:
--------------
The following order of operations ensures that 3rd party UEFI modules,
such as PCI option ROMs and other modules possibly loaded from outside
of firmware volumes, are measured into the TPM:
(1) Tcg2Dxe is included in DXEFV, therefore it produces the TCG2
protocol sometime in the DXE phase (assuming a TPM2 chip is present,
reported via PcdTpmInstanceGuid).
(2) The DXE core finds that no more drivers are left to dispatch from
DXEFV, and we enter the BDS phase.
(3) OVMF's PlatformBootManagerLib connects all PCI root bridges
non-recursively, producing PciIo instances and discovering PCI
oproms.
(4) The dispatching of images that don't originate from FVs is deferred
at this point, by
"MdeModulePkg/Universal/SecurityStubDxe/Defer3rdPartyImageLoad.c".
(5) OVMF's PlatformBootManagerLib signals EndOfDxe.
(6) OVMF's PlatformBootManagerLib calls
EfiBootManagerDispatchDeferredImages() -- the images deferred in
step (4) are now dispatched.
(7) Image dispatch invokes the Security / Security2 Arch protocols
(produced by SecurityStubDxe). In this patch, we hook
DxeTpm2MeasureBootLib into SecurityStubDxe, therefore image dispatch
will try to locate the TCG2 protocol, and measure the image into the
TPM2 chip with the protocol. Because of step (1), the TCG2 protocol
will always be found and used (assuming a TPM2 chip is present).
--------------
Jiewen, does this look OK?
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib
2018-03-09 10:26 ` Laszlo Ersek
@ 2018-03-09 11:37 ` Yao, Jiewen
0 siblings, 0 replies; 36+ messages in thread
From: Yao, Jiewen @ 2018-03-09 11:37 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: marcandre.lureau, edk2-devel, javierm, pjones, qemu-devel
Good idea.
The additional message looks great!!!
thank you!
Yao, Jiewen
> 在 2018年3月9日，下午6:26，Laszlo Ersek <lersek@redhat.com> 写道：
>
> in
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
2018-03-08 0:36 ` [Qemu-devel] [edk2] " Zhang, Chao B
@ 2018-03-09 13:05 ` Marc-André Lureau
2018-03-09 15:05 ` Laszlo Ersek
0 siblings, 1 reply; 36+ messages in thread
From: Marc-André Lureau @ 2018-03-09 13:05 UTC (permalink / raw)
To: Zhang, Chao B
Cc: edk2-devel, lersek, pjones, Yao, Jiewen, qemu-devel, javierm
Hi
On Thu, Mar 8, 2018 at 1:36 AM, Zhang, Chao B <chao.b.zhang@intel.com> wrote:
> Hi Lureau:
> I think we can remove same dependency in TcgPei.
>
Thanks, feel free to explore that as a separate patch. This is out of
scope to me.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support
2018-03-09 3:03 ` Shi, Steven
@ 2018-03-09 13:54 ` Stefan Berger
2018-03-12 5:00 ` Shi, Steven
0 siblings, 1 reply; 36+ messages in thread
From: Stefan Berger @ 2018-03-09 13:54 UTC (permalink / raw)
To: Shi, Steven, Marc-André Lureau
Cc: edk2-devel, lersek, pjones, Yao, Jiewen, qemu-devel, javierm
On 03/08/2018 10:03 PM, Shi, Steven wrote:
> Hi Marcandre,
> Thanks for your command steps and I tried them, but my qemu failed to connect the socket tpmemu.sock. When I added the control channel to the TPM, the swtpm socket command stuck there and never exit. Not sure whether it was successful.
> Below are the command steps running output in my side
>
>> Then you can run:
>> mkdir tpmstatedir
>> swtpm_setup.sh --tpm2 --tpm-state tpmstatedir
> $ swtpm_setup.sh --tpm2 --tpm-state tpmstatedir
> Starting vTPM manufacturing as jshi19:jshi19 @ 2018年03月09日 星期五 10时28分39秒
> TPM is listening on TCP port 47364.
> Successfully authored TPM state.
> Ending vTPM manufacturing @ 2018年03月09日 星期五 10时28分39秒
>
>> Run the emulator:
>> swtpm socket --tpmstate dir=tpmstatedir --ctrl type=unixio,path=tpmemu.sock --tpm2
> $ swtpm socket --tpmstate dir=tpmstatedir --ctrl type=unixio,path=tpmemu.sock --tpm2
> (the swtpm socket command stuck there and never exit)
>
>> Run qemu (from git) with ovmf (with this series):
>> qemu ... -chardev socket,id=chrtpm,path=tpmemu.sock -tpmdev
>> emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
>> -drive if=pflash,format=raw,file=OVMF_CODE.fd,readonly -drive
>> if=pflash,format=raw,file=OVMF_VARS.fd ..
> $ qemu-system-x86_64 -serial file:serial.log -m 5120 -hda fat:. -monitor stdio --enable-kvm -smp 4 -bios ../Ovmf3264/NOOPT_GCC5/FV/OVMF.fd -chardev socket,id=chrtpm,path=tpmemu.sock -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
> qemu-system-x86_64: -chardev socket,id=chrtpm,path=tpmemu.sock: Failed to connect socket tpmemu.sock: No such file or directory
Try giving it both, swtpm and qemu, the full path to the socket.
>
> I use the latest version qemu as below:
> $ qemu-system-x86_64 --version
> QEMU emulator version 2.11.50 (v2.10.0-4184-g930b01138b-dirty)
> Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
>
> Thanks
> Steven Shi
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex
2018-03-09 13:05 ` Marc-André Lureau
@ 2018-03-09 15:05 ` Laszlo Ersek
0 siblings, 0 replies; 36+ messages in thread
From: Laszlo Ersek @ 2018-03-09 15:05 UTC (permalink / raw)
To: Marc-André Lureau, Zhang, Chao B
Cc: edk2-devel, pjones, Yao, Jiewen, qemu-devel, javierm
On 03/09/18 14:05, Marc-André Lureau wrote:
> Hi
>
> On Thu, Mar 8, 2018 at 1:36 AM, Zhang, Chao B <chao.b.zhang@intel.com> wrote:
>> Hi Lureau:
>> I think we can remove same dependency in TcgPei.
>>
>
> Thanks, feel free to explore that as a separate patch. This is out of
> scope to me.
>
>
I'll submit a patch for that later.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support
2018-03-09 13:54 ` Stefan Berger
@ 2018-03-12 5:00 ` Shi, Steven
0 siblings, 0 replies; 36+ messages in thread
From: Shi, Steven @ 2018-03-12 5:00 UTC (permalink / raw)
To: Stefan Berger, Marc-André Lureau
Cc: edk2-devel, lersek, pjones, Yao, Jiewen, qemu-devel, javierm
It works in my side after specify the full path to swtpm tpmemu.sock in qemu command options. Thanks!
Steven Shi
Intel\SSG\STO\UEFI Firmware
Tel: +86 021-61166522
iNet: 821-6522
> -----Original Message-----
> From: Stefan Berger [mailto:stefanb@linux.vnet.ibm.com]
> Sent: Friday, March 9, 2018 9:54 PM
> To: Shi, Steven <steven.shi@intel.com>; Marc-André Lureau
> <marcandre.lureau@gmail.com>
> Cc: edk2-devel@lists.01.org; lersek@redhat.com; pjones@redhat.com; Yao,
> Jiewen <jiewen.yao@intel.com>; qemu-devel@nongnu.org;
> javierm@redhat.com
> Subject: Re: [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support
>
> On 03/08/2018 10:03 PM, Shi, Steven wrote:
> > Hi Marcandre,
> > Thanks for your command steps and I tried them, but my qemu failed to
> connect the socket tpmemu.sock. When I added the control channel to the
> TPM, the swtpm socket command stuck there and never exit. Not sure
> whether it was successful.
> > Below are the command steps running output in my side
> >
> >> Then you can run:
> >> mkdir tpmstatedir
> >> swtpm_setup.sh --tpm2 --tpm-state tpmstatedir
> > $ swtpm_setup.sh --tpm2 --tpm-state tpmstatedir
> > Starting vTPM manufacturing as jshi19:jshi19 @ 2018年03月09日 星期
> 五 10时28分39秒
> > TPM is listening on TCP port 47364.
> > Successfully authored TPM state.
> > Ending vTPM manufacturing @ 2018年03月09日 星期五 10时28分39
> 秒
> >
> >> Run the emulator:
> >> swtpm socket --tpmstate dir=tpmstatedir --ctrl
> type=unixio,path=tpmemu.sock --tpm2
> > $ swtpm socket --tpmstate dir=tpmstatedir --ctrl
> type=unixio,path=tpmemu.sock --tpm2
> > (the swtpm socket command stuck there and never exit)
> >
> >> Run qemu (from git) with ovmf (with this series):
> >> qemu ... -chardev socket,id=chrtpm,path=tpmemu.sock -tpmdev
> >> emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
> >> -drive if=pflash,format=raw,file=OVMF_CODE.fd,readonly -drive
> >> if=pflash,format=raw,file=OVMF_VARS.fd ..
> > $ qemu-system-x86_64 -serial file:serial.log -m 5120 -hda fat:. -monitor
> stdio --enable-kvm -smp 4 -bios ../Ovmf3264/NOOPT_GCC5/FV/OVMF.fd -
> chardev socket,id=chrtpm,path=tpmemu.sock -tpmdev
> emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0
> > qemu-system-x86_64: -chardev socket,id=chrtpm,path=tpmemu.sock:
> Failed to connect socket tpmemu.sock: No such file or directory
>
> Try giving it both, swtpm and qemu, the full path to the socket.
>
>
> >
> > I use the latest version qemu as below:
> > $ qemu-system-x86_64 --version
> > QEMU emulator version 2.11.50 (v2.10.0-4184-g930b01138b-dirty)
> > Copyright (c) 2003-2017 Fabrice Bellard and the QEMU Project developers
> >
> > Thanks
> > Steven Shi
> >
^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2018-03-12 5:00 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-07 15:57 [Qemu-devel] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support marcandre.lureau
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 1/8] SecurityPkg: also clear HashInterfaceHob.SupportedHashMask marcandre.lureau
2018-03-08 0:35 ` Zhang, Chao B
2018-03-08 0:48 ` Zeng, Star
2018-03-08 11:40 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 2/8] SecurityPkg/Tcg2Pei: drop PeiReadOnlyVariable from Depex marcandre.lureau
2018-03-07 16:04 ` Yao, Jiewen
2018-03-08 0:36 ` [Qemu-devel] [edk2] " Zhang, Chao B
2018-03-09 13:05 ` Marc-André Lureau
2018-03-09 15:05 ` Laszlo Ersek
2018-03-08 11:41 ` Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 3/8] MdeModulePkg: fix REGISITER -> REGISTER marcandre.lureau
2018-03-08 11:59 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-08 12:08 ` Zeng, Star
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 4/8] ovmf: simplify SecurityStubDxe.inf inclusion marcandre.lureau
2018-03-08 16:35 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 5/8] ovmf: add OvmfPkg Tcg2ConfigPei module marcandre.lureau
2018-03-08 17:46 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-08 18:10 ` Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 6/8] ovmf: link with Tcg2Pei module marcandre.lureau
2018-03-08 18:20 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-08 18:33 ` Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 7/8] ovmf: link with Tcg2Dxe module marcandre.lureau
2018-03-08 19:14 ` [Qemu-devel] [edk2] " Laszlo Ersek
2018-03-07 15:57 ` [Qemu-devel] [PATCH v2 8/8] ovmf: add DxeTpm2MeasureBootLib marcandre.lureau
2018-03-08 19:54 ` Laszlo Ersek
2018-03-08 19:56 ` Laszlo Ersek
2018-03-09 0:39 ` Yao, Jiewen
2018-03-09 0:47 ` Yao, Jiewen
2018-03-09 10:26 ` Laszlo Ersek
2018-03-09 11:37 ` [Qemu-devel] [edk2] " Yao, Jiewen
2018-03-08 12:31 ` [Qemu-devel] [edk2] [PATCH v2 0/8] RFC: ovmf: preliminary TPM2 support Shi, Steven
2018-03-08 13:59 ` Marc-André Lureau
2018-03-09 3:03 ` Shi, Steven
2018-03-09 13:54 ` Stefan Berger
2018-03-12 5:00 ` Shi, Steven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.