On 02/23/2018 12:05 AM, Kevin O'Connor wrote:
On Tue, Feb 13, 2018 at 11:08:07AM -0500, Stefan Berger wrote:
From: Marc-André Lureau <marcandre.lur...@redhat.com>
The CRB device was introduced with TPM 2.0 to be physical-bus agnostic
and defined in TCG PC Client Platform TPM Profile (PTP) Specification
Family “2.0” Level 00 Revision 01.03 v22
It seems to be required with Windows 10. It is also a simpler device
than FIFO/TIS.
This patch only support locality 0 since also the CRB device in QEMU
only supports this locality.
Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
Reviewed-by: Stefan Berger <stef...@linux.vnet.ibm.com>
Tested-by: Stefan Berger <stef...@linux.vnet.ibm.com>
---
src/hw/tpm_drivers.c | 198 ++++++++++++++++++++++++++++++++++++++++++++++++++-
src/hw/tpm_drivers.h | 26 +++++++
2 files changed, 223 insertions(+), 1 deletion(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
index 5cee9d8..24ce9b7 100644
--- a/src/hw/tpm_drivers.c
+++ b/src/hw/tpm_drivers.c
@@ -17,6 +17,8 @@
#include "util.h" // timer_calc_usec
#include "x86.h" // readl
+#define MIN(x,y) ((x) < (y) ? (x) : (y))
I'd prefer not to introduce adhoc MIN/MAX macros - they tend to be
confusing as sometimes they're evaluation safe and sometimes not. I'm
okay with a separate patch that adds min/max globally to seabios, but
otherwise I suggest just implementing this directly in its one call
site.
Then I just implement it 'in its one call site' here.
[...]
+/* if device is not there, return '0', '1' otherwise */
+static u32 crb_probe(void)
+{
+ if (!CONFIG_TCGBIOS)
+ return 0;
+
+ u32 ifaceid = readl(CRB_REG(0, CRB_REG_INTF_ID));
+
+ if ((ifaceid & 0xf) != 0xf) {
+ if ((ifaceid & 0xf) == 1) {
+ /* CRB is active */
+ return 1;
+ }
+ if ((ifaceid & (1 << 14)) == 0) {
+ /* CRB cannot be selected */
+ return 0;
+ }
+ /* write of 1 to bits 17-18 selects CRB */
+ writel(CRB_REG(0, CRB_REG_INTF_ID), (1 << 17));
+ /* lock it */
+ writel(CRB_REG(0, CRB_REG_INTF_ID), (1 << 19));
+ }
+
+ /* no support for 64 bit addressing yet */
+ if (readl(CRB_REG(0, CRB_REG_CTRL_CMD_HADDR)))
+ return 1;
+
+ u64 addr = readq(CRB_REG(0, CRB_REG_CTRL_RSP_ADDR));
+ if (addr > __UINT32_MAX__)
Where does the __UINT32_MAX__ constant come from? We don't normally
depend on any system libraries. Again, probably best to just use
0xffffffff.
Will be fixed in v2.
Stefan
Otherwise the series looks good to me.
Thanks,
-Kevin
_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
