Christian Walter has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/ptt: Add PTT Support ......................................................................
src/drivers/ptt: Add PTT Support
Add Function which check if Intel Platform Trust Technology / Intel integrated TPM is enabled/active.
Change-Id: If93bb5e1a3a59b5045f4e44359683876fb387a71 Signed-off-by: Christian Walter christian.walter@9elements.com --- A src/drivers/ptt/Kconfig A src/drivers/ptt/ptt.c A src/drivers/ptt/ptt.h 3 files changed, 51 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/34380/1
diff --git a/src/drivers/ptt/Kconfig b/src/drivers/ptt/Kconfig new file mode 100644 index 0000000..b03e6d9 --- /dev/null +++ b/src/drivers/ptt/Kconfig @@ -0,0 +1,5 @@ +config INTEL_PTT + bool + default n + help + Intel Platform Trust Technology like Intel iTPM diff --git a/src/drivers/ptt/ptt.c b/src/drivers/ptt/ptt.c new file mode 100644 index 0000000..0c44942 --- /dev/null +++ b/src/drivers/ptt/ptt.c @@ -0,0 +1,37 @@ +/*. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include <arch/early_variables.h> +#include <soc/pci_devs.h> +#include <device/pci_ops.h> + +#include "ptt.h" + +/* Dump Intel ME Register */ +static uint32_t dump_status(int index, int reg_addr) +{ + uint32_t reg = pci_read_config32(PCH_DEV_CSE, reg_addr); + + return reg; +} + +/* + * ptt_active() + * + * Check if PTT Flag is set - so that PTT is active. + * + * Return 0 if active, -1 otherwise. + */ +int ptt_active(void) +{ + // Check if PTT establishment bit is valid + uint32_t fwsts4 = dump_status(4, PCI_ME_HFSTS4); + if ((fwsts4 & PTT_ENABLE_BIT) == 0) { + printk(BIOS_DEBUG, "Intel ME Establishment Bit not valid.\n"); + return -1; + } + + return 0; +} diff --git a/src/drivers/ptt/ptt.h b/src/drivers/ptt/ptt.h new file mode 100644 index 0000000..25104d3 --- /dev/null +++ b/src/drivers/ptt/ptt.h @@ -0,0 +1,9 @@ +/*. + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#define PCI_ME_HFSTS4 0x64 +#define PTT_ENABLE_BIT (1<<19) + +int ptt_active(void);
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/ptt: Add PTT Support ......................................................................
Patch Set 3:
place under src/drivers/intel/ptt instead ?
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/ptt: Add PTT Support ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@25 PS3, Line 25: * Return 0 if active, -1 otherwise. return true if active, false if not
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@27 PS3, Line 27: int ptt_active(void) bool
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/ptt: Add PTT Support ......................................................................
Patch Set 3:
(12 comments)
https://review.coreboot.org/c/coreboot/+/34380/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34380/3//COMMIT_MSG@10 PS3, Line 10: integrated TPM is enabled/active. Corrected spelling: Add function which checks if …
Better in my opinion: Add function to check if …
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig File src/drivers/ptt/Kconfig:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig@1 PS3, Line 1: config INTEL_PTT Rephrase: HAVE_INTEL_PTT? ACTIVATE_INTEL_PTT?
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig@5 PS3, Line 5: Intel Platform Trust Technology like Intel iTPM Please elaborate.
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h File src/drivers/ptt/ptt.h:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@7 PS3, Line 7: << Please add spaces around the operator. Maybe run the file through clang-format?
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@7 PS3, Line 7: PTT_ENABLE_BIT PTT_ENABLED? No idea, what is common in coreboot.
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@8 PS3, Line 8: Please document the function.
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@12 PS3, Line 12: Register register
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@13 PS3, Line 13: dump_status That’s quite generic and nothing is dumped.
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@13 PS3, Line 13: static uint32_t dump_status(int index, int reg_addr) Can this be moved to some common code?
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@17 PS3, Line 17: return reg; Just one line?
return pci_read_config32(PCH_DEV_CSE, reg_addr);
Maybe the function is not needed yet?
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@29 PS3, Line 29: // Check if PTT establishment bit is valid Please indent the comment line correctly.
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@32 PS3, Line 32: Bit bit
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34380
to look at the new patch set (#4).
Change subject: src/drivers/ptt: Add PTT Support ......................................................................
src/drivers/ptt: Add PTT Support
Add function which checks if Intel Platform Trust Technology / Intel integrated TPM is enabled/active.
Change-Id: If93bb5e1a3a59b5045f4e44359683876fb387a71 Signed-off-by: Christian Walter christian.walter@9elements.com --- A src/drivers/ptt/Kconfig A src/drivers/ptt/ptt.c A src/drivers/ptt/ptt.h 3 files changed, 55 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/34380/4
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/ptt: Add PTT Support ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/Kconfig File src/drivers/ptt/Kconfig:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/Kconfig@1 PS6, Line 1: config HAVE_INTEL_PTT place under src/drivers/intel/ptt instead ?
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/ptt: Add PTT Support ......................................................................
Patch Set 6:
(8 comments)
place under src/drivers/intel/ptt instead ?
I agree.
Though, I'm not sure how feasible it is to make it platform independent. When do we need ptt_active()? will the MEI always be available when we need it?
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.h File src/drivers/ptt/ptt.h:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.h@3 PS6, Line 3: LICENSE Where is it?
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.h@6 PS6, Line 6: #define PCI_ME_HFSTS4 0x64 : #define PTT_ENABLE (1 << 19) Bit 19 isn't documented in any ME BWG I could find. I assume it is documented somewhere else? Is it explicitly documented to be platform-agnostic?
And why are these definitions in a header file?
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@6 PS6, Line 6: #include <arch/early_variables.h> why?
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@8 PS6, Line 8: #include <device/pci_ops.h> Missing <stdint.h> and <console/console.h>.
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@13 PS6, Line 13: int index What is this?
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@15 PS6, Line 15: PCH_DEV_CSE Missing NULL-check.
As this device may be hidden, it would be wise to document in the API when (in which phases of coreboot) ptt_active() can be called.
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@27 PS6, Line 27: // Check if PTT establishment bit is valid Comment seems redundant, especially with the debug message.
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@29 PS6, Line 29: if ((fwsts4 & PTT_ENABLE_BIT) == 0) { If something is wrong with the ME, the MEI device might return 0xff for all reads. So this needs some check first if the ME interface is up at all.
Hello build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34380
to look at the new patch set (#7).
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
src/drivers/intel/ptt: Add PTT Support
Add function which checks if Intel Platform Trust Technology / Intel integrated TPM is enabled/active.
Change-Id: If93bb5e1a3a59b5045f4e44359683876fb387a71 Signed-off-by: Christian Walter christian.walter@9elements.com --- A src/drivers/intel/ptt/Kconfig A src/drivers/intel/ptt/Makefile.inc A src/drivers/intel/ptt/ptt.c A src/drivers/intel/ptt/ptt.h 4 files changed, 117 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/34380/7
Hello Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34380
to look at the new patch set (#8).
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
src/drivers/intel/ptt: Add PTT Support
Add function which checks if Intel Platform Trust Technology / Intel integrated TPM is enabled/active.
Change-Id: If93bb5e1a3a59b5045f4e44359683876fb387a71 Signed-off-by: Christian Walter christian.walter@9elements.com --- A src/drivers/intel/ptt/Kconfig A src/drivers/intel/ptt/Makefile.inc A src/drivers/intel/ptt/ptt.c A src/drivers/intel/ptt/ptt.h 4 files changed, 89 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/34380/8
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 7:
(9 comments)
We need ptt_active before we run the first tpm2_process_command command, though after the tis_init. We need to check once, if this bit is set, so that we can use the iTPM. It is either in the Ramstage, or in the Verstage. In both stages, the MEI should be available.
Patch Set 6:
(8 comments)
place under src/drivers/intel/ptt instead ?
I agree.
Though, I'm not sure how feasible it is to make it platform independent. When do we need ptt_active()? will the MEI always be available when we need it?
You need
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/Kconfig File src/drivers/ptt/Kconfig:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/Kconfig@1 PS6, Line 1: config HAVE_INTEL_PTT
place under src/drivers/intel/ptt instead ?
Ack
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.h File src/drivers/ptt/ptt.h:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.h@3 PS6, Line 3: LICENSE
Where is it?
Ack
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.h@6 PS6, Line 6: #define PCI_ME_HFSTS4 0x64 : #define PTT_ENABLE (1 << 19)
Bit 19 isn't documented in any ME BWG I could find. I assume it is documented […]
It is documented in the Intel Document 560297. Removed the defines and put them into the .c file
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@6 PS6, Line 6: #include <arch/early_variables.h>
why?
Ack
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@8 PS6, Line 8: #include <device/pci_ops.h>
Missing <stdint.h> and <console/console.h>.
Ack
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@13 PS6, Line 13: int index
What is this?
Ack
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@15 PS6, Line 15: PCH_DEV_CSE
Missing NULL-check. […]
How to do the NULL Check? reg_addr could be NULL.. and the other one is a define.. I'll update the documentation.
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@27 PS6, Line 27: // Check if PTT establishment bit is valid
Comment seems redundant, especially with the debug message.
Ack
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@29 PS6, Line 29: if ((fwsts4 & PTT_ENABLE_BIT) == 0) {
If something is wrong with the ME, the MEI device might return 0xff for all […]
Ack
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 10:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34380/10/src/drivers/intel/ptt/ptt.... File src/drivers/intel/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/10/src/drivers/intel/ptt/ptt.... PS10, Line 38: * Return 0 if active, -1 otherwise. Needs update.
https://review.coreboot.org/c/coreboot/+/34380/10/src/drivers/intel/ptt/ptt.... PS10, Line 44: if (fwsts4 == 0xFFFFFFFF) I'm not sure if this is not a valid value. If in doubt, better check VID/DID register.
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@15 PS6, Line 15: PCH_DEV_CSE
How to do the NULL Check? reg_addr could be NULL.. and the other one is a define.. […]
PCH_DEV_CSE evaluates to a function call returning a pointer (in ramstage).
Hello Patrick Rudolph, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34380
to look at the new patch set (#11).
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
src/drivers/intel/ptt: Add PTT Support
Add function which checks if Intel Platform Trust Technology / Intel integrated TPM is enabled/active.
Change-Id: If93bb5e1a3a59b5045f4e44359683876fb387a71 Signed-off-by: Christian Walter christian.walter@9elements.com --- A src/drivers/intel/ptt/Kconfig A src/drivers/intel/ptt/Makefile.inc A src/drivers/intel/ptt/ptt.c A src/drivers/intel/ptt/ptt.h 4 files changed, 92 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/34380/11
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 11:
(3 comments)
Patch Set 10:
(3 comments)
https://review.coreboot.org/c/coreboot/+/34380/10/src/drivers/intel/ptt/ptt.... File src/drivers/intel/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/10/src/drivers/intel/ptt/ptt.... PS10, Line 38: * Return 0 if active, -1 otherwise.
Needs update.
Ack
https://review.coreboot.org/c/coreboot/+/34380/10/src/drivers/intel/ptt/ptt.... PS10, Line 44: if (fwsts4 == 0xFFFFFFFF)
I'm not sure if this is not a valid value. If in doubt, better check […]
0xFFFFFFFF is not a valid value.
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/6/src/drivers/ptt/ptt.c@15 PS6, Line 15: PCH_DEV_CSE
PCH_DEV_CSE evaluates to a function call returning a pointer (in ramstage).
Ack
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 12: Code-Review+1
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34380/12/src/drivers/intel/ptt/ptt.... File src/drivers/intel/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/12/src/drivers/intel/ptt/ptt.... PS12, Line 21: #define PCI_ME_HFSTS1 0x40 same here
https://review.coreboot.org/c/coreboot/+/34380/12/src/drivers/intel/ptt/ptt.... PS12, Line 25: #define BOOTGUARD_ST_FAILED (1 << 14) seems unused
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 12:
(2 comments)
https://review.coreboot.org/c/coreboot/+/34380/12/src/drivers/intel/ptt/ptt.... File src/drivers/intel/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/12/src/drivers/intel/ptt/ptt.... PS12, Line 21: #define PCI_ME_HFSTS1 0x40
same here
Ack
https://review.coreboot.org/c/coreboot/+/34380/12/src/drivers/intel/ptt/ptt.... PS12, Line 25: #define BOOTGUARD_ST_FAILED (1 << 14)
seems unused
Ack
Hello Patrick Rudolph, build bot (Jenkins), Nico Huber, Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/34380
to look at the new patch set (#13).
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
src/drivers/intel/ptt: Add PTT Support
Add function which checks if Intel Platform Trust Technology / Intel integrated TPM is enabled/active.
Change-Id: If93bb5e1a3a59b5045f4e44359683876fb387a71 Signed-off-by: Christian Walter christian.walter@9elements.com --- A src/drivers/intel/ptt/Kconfig A src/drivers/intel/ptt/Makefile.inc A src/drivers/intel/ptt/ptt.c A src/drivers/intel/ptt/ptt.h 4 files changed, 89 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/34380/13
Christian Walter has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 13:
(14 comments)
https://review.coreboot.org/c/coreboot/+/34380/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/34380/3//COMMIT_MSG@10 PS3, Line 10: integrated TPM is enabled/active.
Corrected spelling: Add function which checks if … […]
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig File src/drivers/ptt/Kconfig:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig@1 PS3, Line 1: config INTEL_PTT
Rephrase: HAVE_INTEL_PTT? ACTIVATE_INTEL_PTT?
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/Kconfig@5 PS3, Line 5: Intel Platform Trust Technology like Intel iTPM
Please elaborate.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h File src/drivers/ptt/ptt.h:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@7 PS3, Line 7: PTT_ENABLE_BIT
PTT_ENABLED? No idea, what is common in coreboot.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@7 PS3, Line 7: <<
Please add spaces around the operator. […]
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.h@8 PS3, Line 8:
Please document the function.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c File src/drivers/ptt/ptt.c:
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@12 PS3, Line 12: Register
register
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@13 PS3, Line 13: dump_status
That’s quite generic and nothing is dumped.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@13 PS3, Line 13: static uint32_t dump_status(int index, int reg_addr)
Can this be moved to some common code?
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@17 PS3, Line 17: return reg;
Just one line? […]
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@25 PS3, Line 25: * Return 0 if active, -1 otherwise.
return true if active, false if not
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@27 PS3, Line 27: int ptt_active(void)
bool
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@29 PS3, Line 29: // Check if PTT establishment bit is valid
Please indent the comment line correctly.
Done
https://review.coreboot.org/c/coreboot/+/34380/3/src/drivers/ptt/ptt.c@32 PS3, Line 32: Bit
bit
Done
Philipp Deppenwiese has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
Patch Set 14: Code-Review+2
Philipp Deppenwiese has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/34380 )
Change subject: src/drivers/intel/ptt: Add PTT Support ......................................................................
src/drivers/intel/ptt: Add PTT Support
Add function which checks if Intel Platform Trust Technology / Intel integrated TPM is enabled/active.
Change-Id: If93bb5e1a3a59b5045f4e44359683876fb387a71 Signed-off-by: Christian Walter christian.walter@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/34380 Reviewed-by: Philipp Deppenwiese zaolin.daisuki@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- A src/drivers/intel/ptt/Kconfig A src/drivers/intel/ptt/Makefile.inc A src/drivers/intel/ptt/ptt.c A src/drivers/intel/ptt/ptt.h 4 files changed, 89 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Philipp Deppenwiese: Looks good to me, approved
diff --git a/src/drivers/intel/ptt/Kconfig b/src/drivers/intel/ptt/Kconfig new file mode 100644 index 0000000..c013f42 --- /dev/null +++ b/src/drivers/intel/ptt/Kconfig @@ -0,0 +1,5 @@ +config HAVE_INTEL_PTT + bool + default n + help + Activate if your platform has Intel Platform Trust Technology like Intel iTPM and you want to use it. diff --git a/src/drivers/intel/ptt/Makefile.inc b/src/drivers/intel/ptt/Makefile.inc new file mode 100644 index 0000000..fdecc89 --- /dev/null +++ b/src/drivers/intel/ptt/Makefile.inc @@ -0,0 +1,4 @@ +romstage-$(CONFIG_HAVE_INTEL_PTT) += ptt.c +ramstage-$(CONFIG_HAVE_INTEL_PTT) += ptt.c +postcar-$(CONFIG_HAVE_INTEL_PTT) += ptt.c +verstage-$(CONFIG_HAVE_INTEL_PTT) += ptt.c diff --git a/src/drivers/intel/ptt/ptt.c b/src/drivers/intel/ptt/ptt.c new file mode 100644 index 0000000..738de50 --- /dev/null +++ b/src/drivers/intel/ptt/ptt.c @@ -0,0 +1,53 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include <soc/pci_devs.h> +#include <device/pci_ops.h> +#include <console/console.h> +#include <timer.h> + +#include "ptt.h" + +#define PCI_ME_HFSTS4 0x64 +#define PTT_ENABLE (1 << 19) + +/* Dump Intel ME register */ +static uint32_t read_register(int reg_addr) +{ + if (!PCH_DEV_CSE) + return 0xFFFFFFFF; + + return pci_read_config32(PCH_DEV_CSE, reg_addr); +} + +/* + * ptt_active() + * + * Check if PTT Flag is set - so that PTT is active. + * + * Return true if active, false otherwise. + */ +bool ptt_active(void) +{ + uint32_t fwsts4 = read_register(PCI_ME_HFSTS4); + + if (fwsts4 == 0xFFFFFFFF) + return false; + + if ((fwsts4 & PTT_ENABLE) == 0) { + printk(BIOS_DEBUG, "Intel ME Establishment bit not valid.\n"); + return false; + } + + return true; +} diff --git a/src/drivers/intel/ptt/ptt.h b/src/drivers/intel/ptt/ptt.h new file mode 100644 index 0000000..ed5e90f --- /dev/null +++ b/src/drivers/intel/ptt/ptt.h @@ -0,0 +1,27 @@ +/* + * This file is part of the coreboot project. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; version 2 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * This driver checks if the PTT Bit is set correctly within the FWSTS4 + * register. This is needed in order to use the iTPM, because we have to + * check prior using the interface that this bit is set correctly - otherwise + * it could work unpredictable. The bit should already be set if the Intel ME + * is still in the preboot phase. + * + */ +#include <stdint.h> +/* + * ptt_active + * + * Checks if the Intel PTT is active. If PTT is active, returns true, + * false otherwise. + */ +bool ptt_active(void);
-
Christian Walter (Code Review) -
Nico Huber (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)
-
Philipp Deppenwiese (Code Review)