Hello all,
I ran into an issue getting a Infineon SLB9670 2.0 TPM running on an Intel Denverton platform, and traced the problem to what seems to be incorrect behavior of tis_get_tpm_version(). That function is returning the version of the interface, and it seems the device version is what is needed. The version check implemented here is based on the tpm2_probe() function in the Linux tpm_tis driver.
This patch set applies on top of Marc-André and Stefan's "Add CRB TPM device" patch series currently under review
I've tested this on real hardware with a SLB9670 hanging off of an Intel C3758, but have not regression tested with TPM 1.2 devices.
Regards, Steve
Stephen Douthit (3): tpm: Refactor duplicated wait code in tis_wait_sts() & crb_wait_reg() tpm: Wait for interface startup when probing tpm: Support 2.0 TPM devices connected to a TIS host
src/hw/tpm_drivers.c | 114 +++++++++++++++++++++++++++------------------------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++ 4 files changed, 66 insertions(+), 55 deletions(-)
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com --- src/hw/tpm_drivers.c | 80 ++++++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 47 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 1fd05e1..8d6fc33 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -63,6 +63,39 @@ static void *crb_cmd; static u32 crb_resp_size; static void *crb_resp;
+static u32 wait_reg8(u8* reg, u32 time, u8 mask, u8 expect) +{ + if (!CONFIG_TCGBIOS) + return 0; + + u32 rc = 1; + u32 end = timer_calc_usec(time); + + for (;;) { + u8 sts = readl(reg); + if ((sts & mask) == expect) { + rc = 0; + break; + } + if (timer_check(end)) { + warn_timeout(); + break; + } + yield(); + } + return rc; +} + +static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) +{ + return wait_reg8(TIS_REG(locty, TIS_REG_STS), time, mask, expect); +} + +static u32 crb_wait_reg(u8 locty, u16 reg, u32 time, u8 mask, u8 expect) +{ + return wait_reg8(CRB_REG(locty, reg), time, mask, expect); +} + /* if device is not there, return '0', '1' otherwise */ static u32 tis_probe(void) { @@ -152,30 +185,6 @@ static void set_timeouts(u32 timeouts[4], u32 durations[3]) memcpy(dus, durations, 3 * sizeof(u32)); }
- -static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) -{ - if (!CONFIG_TCGBIOS) - return 0; - - u32 rc = 1; - u32 end = timer_calc_usec(time); - - for (;;) { - u8 sts = readb(TIS_REG(locty, TIS_REG_STS)); - if ((sts & mask) == expect) { - rc = 0; - break; - } - if (timer_check(end)) { - warn_timeout(); - break; - } - yield(); - } - return rc; -} - static u32 tis_activate(u8 locty) { if (!CONFIG_TCGBIOS) @@ -399,29 +408,6 @@ static u32 crb_init(void) return 0; }
-static u32 crb_wait_reg(u8 locty, u8 reg, u32 time, u8 mask, u8 expect) -{ - if (!CONFIG_TCGBIOS) - return 0; - - u32 rc = 1; - u32 end = timer_calc_usec(time); - - for (;;) { - u8 sts = readl(CRB_REG(locty, reg)); - if ((sts & mask) == expect) { - rc = 0; - break; - } - if (timer_check(end)) { - warn_timeout(); - break; - } - yield(); - } - return rc; -} - static u32 crb_activate(u8 locty) { if (!CONFIG_TCGBIOS)
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 80 ++++++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 47 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 1fd05e1..8d6fc33 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -63,6 +63,39 @@ static void *crb_cmd; static u32 crb_resp_size; static void *crb_resp;
+static u32 wait_reg8(u8* reg, u32 time, u8 mask, u8 expect) +{
- if (!CONFIG_TCGBIOS)
return 0;- u32 rc = 1;
- u32 end = timer_calc_usec(time);
- for (;;) {
u8 sts = readl(reg);
Nit: call this 'value' instead since it has nothing to do anymore with the REG_STS below.
if ((sts & mask) == expect) {rc = 0;break;}if (timer_check(end)) {warn_timeout();break;}yield();- }
- return rc;
+}
+static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) +{
- return wait_reg8(TIS_REG(locty, TIS_REG_STS), time, mask, expect);
+}
+static u32 crb_wait_reg(u8 locty, u16 reg, u32 time, u8 mask, u8 expect) +{
- return wait_reg8(CRB_REG(locty, reg), time, mask, expect);
+}
- /* if device is not there, return '0', '1' otherwise */ static u32 tis_probe(void) {
@@ -152,30 +185,6 @@ static void set_timeouts(u32 timeouts[4], u32 durations[3]) memcpy(dus, durations, 3 * sizeof(u32)); }
-static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) -{
- if (!CONFIG_TCGBIOS)
return 0;- u32 rc = 1;
- u32 end = timer_calc_usec(time);
- for (;;) {
u8 sts = readb(TIS_REG(locty, TIS_REG_STS));if ((sts & mask) == expect) {rc = 0;break;}if (timer_check(end)) {warn_timeout();break;}yield();- }
- return rc;
-}
- static u32 tis_activate(u8 locty) { if (!CONFIG_TCGBIOS)
@@ -399,29 +408,6 @@ static u32 crb_init(void) return 0; }
-static u32 crb_wait_reg(u8 locty, u8 reg, u32 time, u8 mask, u8 expect) -{
- if (!CONFIG_TCGBIOS)
return 0;- u32 rc = 1;
- u32 end = timer_calc_usec(time);
- for (;;) {
u8 sts = readl(CRB_REG(locty, reg));if ((sts & mask) == expect) {rc = 0;break;}if (timer_check(end)) {warn_timeout();break;}yield();- }
- return rc;
-}
- static u32 crb_activate(u8 locty) { if (!CONFIG_TCGBIOS)
On 02/26/2018 05:35 PM, Stefan Berger wrote:
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 80 ++++++++++++++++++++++------------------------------ 1 file changed, 33 insertions(+), 47 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 1fd05e1..8d6fc33 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -63,6 +63,39 @@ static void *crb_cmd; static u32 crb_resp_size; static void *crb_resp;
+static u32 wait_reg8(u8* reg, u32 time, u8 mask, u8 expect) +{ + if (!CONFIG_TCGBIOS) + return 0;
+ u32 rc = 1; + u32 end = timer_calc_usec(time);
+ for (;;) { + u8 sts = readl(reg);
Nit: call this 'value' instead since it has nothing to do anymore with the REG_STS below.
Will do.
+ if ((sts & mask) == expect) { + rc = 0; + break; + } + if (timer_check(end)) { + warn_timeout(); + break; + } + yield(); + } + return rc; +}
+static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) +{ + return wait_reg8(TIS_REG(locty, TIS_REG_STS), time, mask, expect); +}
+static u32 crb_wait_reg(u8 locty, u16 reg, u32 time, u8 mask, u8 expect) +{ + return wait_reg8(CRB_REG(locty, reg), time, mask, expect); +}
/* if device is not there, return '0', '1' otherwise */ static u32 tis_probe(void) { @@ -152,30 +185,6 @@ static void set_timeouts(u32 timeouts[4], u32 durations[3]) memcpy(dus, durations, 3 * sizeof(u32)); }
-static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) -{ - if (!CONFIG_TCGBIOS) - return 0;
- u32 rc = 1; - u32 end = timer_calc_usec(time);
- for (;;) { - u8 sts = readb(TIS_REG(locty, TIS_REG_STS)); - if ((sts & mask) == expect) { - rc = 0; - break; - } - if (timer_check(end)) { - warn_timeout(); - break; - } - yield(); - } - return rc; -}
static u32 tis_activate(u8 locty) { if (!CONFIG_TCGBIOS) @@ -399,29 +408,6 @@ static u32 crb_init(void) return 0; }
-static u32 crb_wait_reg(u8 locty, u8 reg, u32 time, u8 mask, u8 expect) -{ - if (!CONFIG_TCGBIOS) - return 0;
- u32 rc = 1; - u32 end = timer_calc_usec(time);
- for (;;) { - u8 sts = readl(CRB_REG(locty, reg)); - if ((sts & mask) == expect) { - rc = 0; - break; - } - if (timer_check(end)) { - warn_timeout(); - break; - } - yield(); - } - return rc; -}
static u32 crb_activate(u8 locty) { if (!CONFIG_TCGBIOS)
This is based on wait_startup() from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com --- src/hw/tpm_drivers.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 8d6fc33..789d70a 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -86,6 +86,11 @@ static u32 wait_reg8(u8* reg, u32 time, u8 mask, u8 expect) return rc; }
+static u32 tis_wait_access(u8 locty, u32 time, u8 mask, u8 expect) +{ + return wait_reg8(TIS_REG(locty, TIS_REG_ACCESS), time, mask, expect); +} + static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) { return wait_reg8(TIS_REG(locty, TIS_REG_STS), time, mask, expect); @@ -102,7 +107,13 @@ static u32 tis_probe(void) if (!CONFIG_TCGBIOS) return 0;
- u32 rc = 0; + /* Wait for the interface to report it's ready */ + u32 rc = tis_wait_access(0, TIS_DEFAULT_TIMEOUT_A, + TIS_ACCESS_TPM_REG_VALID_STS, + TIS_ACCESS_TPM_REG_VALID_STS); + if (rc) + return 0; + u32 didvid = readl(TIS_REG(0, TIS_REG_DID_VID));
if ((didvid != 0) && (didvid != 0xffffffff))
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
This is based on wait_startup() from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 8d6fc33..789d70a 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -86,6 +86,11 @@ static u32 wait_reg8(u8* reg, u32 time, u8 mask, u8 expect) return rc; }
+static u32 tis_wait_access(u8 locty, u32 time, u8 mask, u8 expect) +{
- return wait_reg8(TIS_REG(locty, TIS_REG_ACCESS), time, mask, expect);
+}
- static u32 tis_wait_sts(u8 locty, u32 time, u8 mask, u8 expect) { return wait_reg8(TIS_REG(locty, TIS_REG_STS), time, mask, expect);
@@ -102,7 +107,13 @@ static u32 tis_probe(void) if (!CONFIG_TCGBIOS) return 0;
- u32 rc = 0;
/* Wait for the interface to report it's ready */
u32 rc = tis_wait_access(0, TIS_DEFAULT_TIMEOUT_A,
TIS_ACCESS_TPM_REG_VALID_STS,TIS_ACCESS_TPM_REG_VALID_STS);if (rc)
return 0;u32 didvid = readl(TIS_REG(0, TIS_REG_DID_VID)); if ((didvid != 0) && (didvid != 0xffffffff))
On QEMU that VALID flag is always set.
Reviewed-by: Stefan Berger stefanb@linux.vnet.ibm.com
tis_get_tpm_version() was returning the interface version, which is always 1.2 since tis_probe() would have failed if the interface wasn't TIS.
New version check is based on the tpm2_probe() function from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com --- src/hw/tpm_drivers.c | 21 +++++++++++++++------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++++ 4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 789d70a..32c34df 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -11,6 +11,7 @@ #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT +#include "tcgbios.h" // tpm20_getcapability #include "output.h" // warn_timeout #include "stacks.h" // yield #include "string.h" // memcpy @@ -119,7 +120,7 @@ static u32 tis_probe(void) if ((didvid != 0) && (didvid != 0xffffffff)) rc = 1;
- /* TPM 2 has an interface register */ + /* Low 32 bits of CRB interface register overlap TIS interface register */ u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
if ((ifaceid & 0xf) != 0xf) { @@ -142,13 +143,21 @@ static u32 tis_probe(void)
static TPMVersion tis_get_tpm_version(void) { - /* TPM 2 has an interface register */ - u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); + u8 buffer[128]; + int ret; + struct tpm2_res_getcapability *trg = + (struct tpm2_res_getcapability *)&buffer; + + ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, cpu_to_be32(0x100), + cpu_to_be32(1), &trg->hdr, sizeof(buffer)); + if (ret == TPM2_RC_INITIALIZE) + return TPM_VERSION_2; + else if (ret) + return TPM_VERSION_NONE;
- if ((ifaceid & 0xf) == 0) { - /* TPM 2 */ + if (cpu_to_be32(trg->hdr.tag) == TPM2_ST_NO_SESSIONS) return TPM_VERSION_2; - } + return TPM_VERSION_1_2; }
diff --git a/src/std/tcg.h b/src/std/tcg.h index 09a92d8..4c07493 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -357,6 +357,7 @@ struct tpm_res_sha1complete {
/* TPM 2 Capabilities */ #define TPM2_CAP_PCRS 0x00000005 +#define TPM2_CAP_TPM_PROPERTIES 0x00000006
/* TPM 2 data structures */
diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..b86bf64 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -381,7 +381,7 @@ tpm_simple_cmd(u8 locty, u32 ordinal return ret; }
-static int +int tpm20_getcapability(u32 capability, u32 property, u32 count, struct tpm_rsp_header *rsp, u32 rsize) { diff --git a/src/tcgbios.h b/src/tcgbios.h index 32fb941..9fc711a 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -1,6 +1,7 @@ #ifndef TCGBIOS_H #define TCGBIOS_H
+#include "std/tcg.h" // struct tpm_rsp_header #include "types.h"
struct bregs; @@ -15,5 +16,8 @@ void tpm_add_cdrom_catalog(const u8 *addr, u32 length); void tpm_option_rom(const void *addr, u32 len); int tpm_can_show_menu(void); void tpm_menu(void); +int +tpm20_getcapability(u32 capability, u32 property, u32 count, + struct tpm_rsp_header *rsp, u32 rsize);
#endif /* TCGBIOS_H */
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
tis_get_tpm_version() was returning the interface version, which is always 1.2 since tis_probe() would have failed if the interface wasn't TIS.
New version check is based on the tpm2_probe() function from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 21 +++++++++++++++------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++++ 4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 789d70a..32c34df 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -11,6 +11,7 @@ #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT +#include "tcgbios.h" // tpm20_getcapability #include "output.h" // warn_timeout #include "stacks.h" // yield #include "string.h" // memcpy @@ -119,7 +120,7 @@ static u32 tis_probe(void) if ((didvid != 0) && (didvid != 0xffffffff)) rc = 1;
- /* TPM 2 has an interface register */
- /* Low 32 bits of CRB interface register overlap TIS interface register */ u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
Since this is tis_probe, I don't think we need to mention the CRB here ...
if ((ifaceid & 0xf) != 0xf) {@@ -142,13 +143,21 @@ static u32 tis_probe(void)
static TPMVersion tis_get_tpm_version(void) {
- /* TPM 2 has an interface register */
- u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
- u8 buffer[128];
- int ret;
- struct tpm2_res_getcapability *trg =
(struct tpm2_res_getcapability *)&buffer;- ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, cpu_to_be32(0x100),
cpu_to_be32(1), &trg->hdr, sizeof(buffer));- if (ret == TPM2_RC_INITIALIZE)
return TPM_VERSION_2;- else if (ret)
return TPM_VERSION_NONE;
I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
Do you know what value does the InterfaceType show on your TIS? I only tested this with QEMU TIS and here we return 0x0 in those 4 bits, which means, following specs, 'FIFO interface as defined in PTP for TPM 2.0 is active'.
- if ((ifaceid & 0xf) == 0) {
/* TPM 2 */
- if (cpu_to_be32(trg->hdr.tag) == TPM2_ST_NO_SESSIONS) return TPM_VERSION_2;
- }
return TPM_VERSION_1_2;diff --git a/src/std/tcg.h b/src/std/tcg.h index 09a92d8..4c07493 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -357,6 +357,7 @@ struct tpm_res_sha1complete {
/* TPM 2 Capabilities */ #define TPM2_CAP_PCRS 0x00000005 +#define TPM2_CAP_TPM_PROPERTIES 0x00000006
/* TPM 2 data structures */
diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..b86bf64 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -381,7 +381,7 @@ tpm_simple_cmd(u8 locty, u32 ordinal return ret; }
-static int +int tpm20_getcapability(u32 capability, u32 property, u32 count, struct tpm_rsp_header *rsp, u32 rsize) { diff --git a/src/tcgbios.h b/src/tcgbios.h index 32fb941..9fc711a 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -1,6 +1,7 @@ #ifndef TCGBIOS_H #define TCGBIOS_H
+#include "std/tcg.h" // struct tpm_rsp_header #include "types.h"
struct bregs; @@ -15,5 +16,8 @@ void tpm_add_cdrom_catalog(const u8 *addr, u32 length); void tpm_option_rom(const void *addr, u32 len); int tpm_can_show_menu(void); void tpm_menu(void); +int +tpm20_getcapability(u32 capability, u32 property, u32 count,
struct tpm_rsp_header *rsp, u32 rsize);#endif /* TCGBIOS_H */
On 02/26/2018 05:09 PM, Stefan Berger wrote:
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
tis_get_tpm_version() was returning the interface version, which is always 1.2 since tis_probe() would have failed if the interface wasn't TIS.
New version check is based on the tpm2_probe() function from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 21 +++++++++++++++------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++++ 4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 789d70a..32c34df 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -11,6 +11,7 @@ #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT +#include "tcgbios.h" // tpm20_getcapability #include "output.h" // warn_timeout #include "stacks.h" // yield #include "string.h" // memcpy @@ -119,7 +120,7 @@ static u32 tis_probe(void) if ((didvid != 0) && (didvid != 0xffffffff)) rc = 1;
- /* TPM 2 has an interface register */ + /* Low 32 bits of CRB interface register overlap TIS interface register */ u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
Since this is tis_probe, I don't think we need to mention the CRB here ...
I think that was a note to myself I forgot to strip out when I dropped my debug prints. I'll change the comment back.
if ((ifaceid & 0xf) != 0xf) { @@ -142,13 +143,21 @@ static u32 tis_probe(void)
static TPMVersion tis_get_tpm_version(void) { - /* TPM 2 has an interface register */ - u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); + u8 buffer[128]; + int ret; + struct tpm2_res_getcapability *trg = + (struct tpm2_res_getcapability *)&buffer;
+ ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, cpu_to_be32(0x100), + cpu_to_be32(1), &trg->hdr, sizeof(buffer)); + if (ret == TPM2_RC_INITIALIZE) + return TPM_VERSION_2; + else if (ret) + return TPM_VERSION_NONE;
I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Doh. That makes sense. I'll need to find a 1.2 system to test on to avoid stupid mistakes like that.
Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
It might be, it wasn't clear to me if the tag could be trusted if the return code wasn't success. The Linux tpm2_probe() code bails before the tag comparison if tpm_transmit_cmd() returns an error.
Is there a block of QEMU code you would recommend as a reference for probing TPMs? I'm not very familiar with that code base, and a pointer would be handy.
Do you know what value does the InterfaceType show on your TIS? I only tested this with QEMU TIS and here we return 0x0 in those 4 bits, which means, following specs, 'FIFO interface as defined in PTP for TPM 2.0 is active'.
I don't see it in my notes, I can put some debug prints back in tomorrow and will get back to you.
Thanks, Steve
- if ((ifaceid & 0xf) == 0) { - /* TPM 2 */ + if (cpu_to_be32(trg->hdr.tag) == TPM2_ST_NO_SESSIONS) return TPM_VERSION_2; - }
return TPM_VERSION_1_2; }
diff --git a/src/std/tcg.h b/src/std/tcg.h index 09a92d8..4c07493 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -357,6 +357,7 @@ struct tpm_res_sha1complete {
/* TPM 2 Capabilities */ #define TPM2_CAP_PCRS 0x00000005 +#define TPM2_CAP_TPM_PROPERTIES 0x00000006
/* TPM 2 data structures */
diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..b86bf64 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -381,7 +381,7 @@ tpm_simple_cmd(u8 locty, u32 ordinal return ret; }
-static int +int tpm20_getcapability(u32 capability, u32 property, u32 count, struct tpm_rsp_header *rsp, u32 rsize) { diff --git a/src/tcgbios.h b/src/tcgbios.h index 32fb941..9fc711a 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -1,6 +1,7 @@ #ifndef TCGBIOS_H #define TCGBIOS_H
+#include "std/tcg.h" // struct tpm_rsp_header #include "types.h"
struct bregs; @@ -15,5 +16,8 @@ void tpm_add_cdrom_catalog(const u8 *addr, u32 length); void tpm_option_rom(const void *addr, u32 len); int tpm_can_show_menu(void); void tpm_menu(void); +int +tpm20_getcapability(u32 capability, u32 property, u32 count, + struct tpm_rsp_header *rsp, u32 rsize);
#endif /* TCGBIOS_H */
On 02/26/2018 05:44 PM, Stephen Douthit wrote:
On 02/26/2018 05:09 PM, Stefan Berger wrote:
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
tis_get_tpm_version() was returning the interface version, which is always 1.2 since tis_probe() would have failed if the interface wasn't TIS.
New version check is based on the tpm2_probe() function from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 21 +++++++++++++++------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++++ 4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 789d70a..32c34df 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -11,6 +11,7 @@ #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT +#include "tcgbios.h" // tpm20_getcapability #include "output.h" // warn_timeout #include "stacks.h" // yield #include "string.h" // memcpy @@ -119,7 +120,7 @@ static u32 tis_probe(void) if ((didvid != 0) && (didvid != 0xffffffff)) rc = 1;
- /* TPM 2 has an interface register */
- /* Low 32 bits of CRB interface register overlap TIS interface
register */ u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
Since this is tis_probe, I don't think we need to mention the CRB here ...
I think that was a note to myself I forgot to strip out when I dropped my debug prints. I'll change the comment back.
if ((ifaceid & 0xf) != 0xf) {@@ -142,13 +143,21 @@ static u32 tis_probe(void)
static TPMVersion tis_get_tpm_version(void) {
- /* TPM 2 has an interface register */
- u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
- u8 buffer[128];
- int ret;
- struct tpm2_res_getcapability *trg =
(struct tpm2_res_getcapability *)&buffer;- ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES,
cpu_to_be32(0x100),
cpu_to_be32(1), &trg->hdr,sizeof(buffer));
- if (ret == TPM2_RC_INITIALIZE)
return TPM_VERSION_2;- else if (ret)
return TPM_VERSION_NONE;I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Doh. That makes sense. I'll need to find a 1.2 system to test on to avoid stupid mistakes like that.
Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
It might be, it wasn't clear to me if the tag could be trusted if the return code wasn't success. The Linux tpm2_probe() code bails before the tag comparison if tpm_transmit_cmd() returns an error.
Is there a block of QEMU code you would recommend as a reference for probing TPMs? I'm not very familiar with that code base, and a pointer would be handy.
https://github.com/stefanberger/qemu-tpm/blob/v2.4.1%2Btpm/hw/tpm/tpm_util.c...
What I don't like about the sending of commands is that we're breaking the layer here. We basically cannot send commands using this higher level API. The lowest we could go would be to call tpmhw_transmit, but that function cannot be used as it is but has to be split up to take tpm_driver as a parameter. You would pass to it &tpm_drivers[TIS_DRIVER_INDEX].
As far as I know, TPM 1.2 didn't have that register, so they either return static 0 or 0xffffffff in that register.
Though if reading the flags from the interface register really turns out to be insufficient, I think you should try to send a command like TPM2_CC_ReadClock. If you get back a TPM2_ST_NO_SESSION, it's a TPM 2. If you get a TPM_TAG_RSP_COMMAND it's a TPM 12.
Do you know what value does the InterfaceType show on your TIS? I only tested this with QEMU TIS and here we return 0x0 in those 4 bits, which means, following specs, 'FIFO interface as defined in PTP for TPM 2.0 is active'.
I don't see it in my notes, I can put some debug prints back in tomorrow and will get back to you.
Thanks, Steve
- if ((ifaceid & 0xf) == 0) {
/* TPM 2 */
- if (cpu_to_be32(trg->hdr.tag) == TPM2_ST_NO_SESSIONS) return TPM_VERSION_2;
- }
return TPM_VERSION_1_2;diff --git a/src/std/tcg.h b/src/std/tcg.h index 09a92d8..4c07493 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -357,6 +357,7 @@ struct tpm_res_sha1complete {
/* TPM 2 Capabilities */ #define TPM2_CAP_PCRS 0x00000005 +#define TPM2_CAP_TPM_PROPERTIES 0x00000006
/* TPM 2 data structures */
diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..b86bf64 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -381,7 +381,7 @@ tpm_simple_cmd(u8 locty, u32 ordinal return ret; }
-static int +int tpm20_getcapability(u32 capability, u32 property, u32 count, struct tpm_rsp_header *rsp, u32 rsize) { diff --git a/src/tcgbios.h b/src/tcgbios.h index 32fb941..9fc711a 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -1,6 +1,7 @@ #ifndef TCGBIOS_H #define TCGBIOS_H
+#include "std/tcg.h" // struct tpm_rsp_header #include "types.h"
struct bregs; @@ -15,5 +16,8 @@ void tpm_add_cdrom_catalog(const u8 *addr, u32 length); void tpm_option_rom(const void *addr, u32 len); int tpm_can_show_menu(void); void tpm_menu(void); +int +tpm20_getcapability(u32 capability, u32 property, u32 count,
struct tpm_rsp_header *rsp, u32 rsize);#endif /* TCGBIOS_H */
On 02/26/2018 06:02 PM, Stefan Berger wrote:
On 02/26/2018 05:44 PM, Stephen Douthit wrote:
On 02/26/2018 05:09 PM, Stefan Berger wrote:
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
tis_get_tpm_version() was returning the interface version, which is always 1.2 since tis_probe() would have failed if the interface wasn't TIS.
New version check is based on the tpm2_probe() function from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 21 +++++++++++++++------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++++ 4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 789d70a..32c34df 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -11,6 +11,7 @@ #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT +#include "tcgbios.h" // tpm20_getcapability #include "output.h" // warn_timeout #include "stacks.h" // yield #include "string.h" // memcpy @@ -119,7 +120,7 @@ static u32 tis_probe(void) if ((didvid != 0) && (didvid != 0xffffffff)) rc = 1;
- /* TPM 2 has an interface register */ + /* Low 32 bits of CRB interface register overlap TIS interface register */ u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
Since this is tis_probe, I don't think we need to mention the CRB here ...
I think that was a note to myself I forgot to strip out when I dropped my debug prints. I'll change the comment back.
if ((ifaceid & 0xf) != 0xf) { @@ -142,13 +143,21 @@ static u32 tis_probe(void)
static TPMVersion tis_get_tpm_version(void) { - /* TPM 2 has an interface register */ - u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); + u8 buffer[128]; + int ret; + struct tpm2_res_getcapability *trg = + (struct tpm2_res_getcapability *)&buffer;
+ ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, cpu_to_be32(0x100), + cpu_to_be32(1), &trg->hdr, sizeof(buffer)); + if (ret == TPM2_RC_INITIALIZE) + return TPM_VERSION_2; + else if (ret) + return TPM_VERSION_NONE;
I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Doh. That makes sense. I'll need to find a 1.2 system to test on to avoid stupid mistakes like that.
Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
It might be, it wasn't clear to me if the tag could be trusted if the return code wasn't success. The Linux tpm2_probe() code bails before the tag comparison if tpm_transmit_cmd() returns an error.
Is there a block of QEMU code you would recommend as a reference for probing TPMs? I'm not very familiar with that code base, and a pointer would be handy.
https://github.com/stefanberger/qemu-tpm/blob/v2.4.1%2Btpm/hw/tpm/tpm_util.c...
What I don't like about the sending of commands is that we're breaking the layer here. We basically cannot send commands using this higher level API. The lowest we could go would be to call tpmhw_transmit, but that function cannot be used as it is but has to be split up to take tpm_driver as a parameter. You would pass to it &tpm_drivers[TIS_DRIVER_INDEX].
As far as I know, TPM 1.2 didn't have that register, so they either return static 0 or 0xffffffff in that register.
Though if reading the flags from the interface register really turns out to be insufficient, I think you should try to send a command like TPM2_CC_ReadClock. If you get back a TPM2_ST_NO_SESSION, it's a TPM 2. If you get a TPM_TAG_RSP_COMMAND it's a TPM 12.
Thanks for all the info. This gives me some experiments to run, and I'll try to find a solution that avoids sending commands if possible.
Do you know what value does the InterfaceType show on your TIS? I only tested this with QEMU TIS and here we return 0x0 in those 4 bits, which means, following specs, 'FIFO interface as defined in PTP for TPM 2.0 is active'.
I don't see it in my notes, I can put some debug prints back in tomorrow and will get back to you.
Thanks, Steve
- if ((ifaceid & 0xf) == 0) { - /* TPM 2 */ + if (cpu_to_be32(trg->hdr.tag) == TPM2_ST_NO_SESSIONS) return TPM_VERSION_2; - }
return TPM_VERSION_1_2; }
diff --git a/src/std/tcg.h b/src/std/tcg.h index 09a92d8..4c07493 100644 --- a/src/std/tcg.h +++ b/src/std/tcg.h @@ -357,6 +357,7 @@ struct tpm_res_sha1complete {
/* TPM 2 Capabilities */ #define TPM2_CAP_PCRS 0x00000005 +#define TPM2_CAP_TPM_PROPERTIES 0x00000006
/* TPM 2 data structures */
diff --git a/src/tcgbios.c b/src/tcgbios.c index 40b3028..b86bf64 100644 --- a/src/tcgbios.c +++ b/src/tcgbios.c @@ -381,7 +381,7 @@ tpm_simple_cmd(u8 locty, u32 ordinal return ret; }
-static int +int tpm20_getcapability(u32 capability, u32 property, u32 count, struct tpm_rsp_header *rsp, u32 rsize) { diff --git a/src/tcgbios.h b/src/tcgbios.h index 32fb941..9fc711a 100644 --- a/src/tcgbios.h +++ b/src/tcgbios.h @@ -1,6 +1,7 @@ #ifndef TCGBIOS_H #define TCGBIOS_H
+#include "std/tcg.h" // struct tpm_rsp_header #include "types.h"
struct bregs; @@ -15,5 +16,8 @@ void tpm_add_cdrom_catalog(const u8 *addr, u32 length); void tpm_option_rom(const void *addr, u32 len); int tpm_can_show_menu(void); void tpm_menu(void); +int +tpm20_getcapability(u32 capability, u32 property, u32 count, + struct tpm_rsp_header *rsp, u32 rsize);
#endif /* TCGBIOS_H */
On 02/26/2018 06:24 PM, Stephen Douthit wrote:
On 02/26/2018 06:02 PM, Stefan Berger wrote:
On 02/26/2018 05:44 PM, Stephen Douthit wrote:
On 02/26/2018 05:09 PM, Stefan Berger wrote:
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
tis_get_tpm_version() was returning the interface version, which is always 1.2 since tis_probe() would have failed if the interface wasn't TIS.
New version check is based on the tpm2_probe() function from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 21 +++++++++++++++------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++++ 4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 789d70a..32c34df 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -11,6 +11,7 @@ #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT +#include "tcgbios.h" // tpm20_getcapability #include "output.h" // warn_timeout #include "stacks.h" // yield #include "string.h" // memcpy @@ -119,7 +120,7 @@ static u32 tis_probe(void) if ((didvid != 0) && (didvid != 0xffffffff)) rc = 1;
- /* TPM 2 has an interface register */
- /* Low 32 bits of CRB interface register overlap TIS
interface register */ u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
Since this is tis_probe, I don't think we need to mention the CRB here ...
I think that was a note to myself I forgot to strip out when I dropped my debug prints. I'll change the comment back.
if ((ifaceid & 0xf) != 0xf) {@@ -142,13 +143,21 @@ static u32 tis_probe(void)
static TPMVersion tis_get_tpm_version(void) {
- /* TPM 2 has an interface register */
- u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
- u8 buffer[128];
- int ret;
- struct tpm2_res_getcapability *trg =
(struct tpm2_res_getcapability *)&buffer;- ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES,
cpu_to_be32(0x100),
cpu_to_be32(1), &trg->hdr,sizeof(buffer));
- if (ret == TPM2_RC_INITIALIZE)
return TPM_VERSION_2;- else if (ret)
return TPM_VERSION_NONE;I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Doh. That makes sense. I'll need to find a 1.2 system to test on to avoid stupid mistakes like that.
Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
It might be, it wasn't clear to me if the tag could be trusted if the return code wasn't success. The Linux tpm2_probe() code bails before the tag comparison if tpm_transmit_cmd() returns an error.
Is there a block of QEMU code you would recommend as a reference for probing TPMs? I'm not very familiar with that code base, and a pointer would be handy.
https://github.com/stefanberger/qemu-tpm/blob/v2.4.1%2Btpm/hw/tpm/tpm_util.c...
What I don't like about the sending of commands is that we're breaking the layer here. We basically cannot send commands using this higher level API. The lowest we could go would be to call tpmhw_transmit, but that function cannot be used as it is but has to be split up to take tpm_driver as a parameter. You would pass to it &tpm_drivers[TIS_DRIVER_INDEX].
As far as I know, TPM 1.2 didn't have that register, so they either return static 0 or 0xffffffff in that register.
Though if reading the flags from the interface register really turns out to be insufficient, I think you should try to send a command like TPM2_CC_ReadClock. If you get back a TPM2_ST_NO_SESSION, it's a TPM 2. If you get a TPM_TAG_RSP_COMMAND it's a TPM 12.
Thanks for all the info. This gives me some experiments to run, and I'll try to find a solution that avoids sending commands if possible.
The following document on pdf page 100 defines bits 28-30 of the interface capability register. If the value there is 0x3 (mask 0x7), we should be sure to have a TPM 2. Hopefully that's sufficient also for your device for determining that it's a TPM 2.
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_...
On 02/26/2018 07:45 PM, Stefan Berger wrote:
On 02/26/2018 06:24 PM, Stephen Douthit wrote:
On 02/26/2018 06:02 PM, Stefan Berger wrote:
On 02/26/2018 05:44 PM, Stephen Douthit wrote:
On 02/26/2018 05:09 PM, Stefan Berger wrote:
On 02/26/2018 03:37 PM, Stephen Douthit wrote:
tis_get_tpm_version() was returning the interface version, which is always 1.2 since tis_probe() would have failed if the interface wasn't TIS.
New version check is based on the tpm2_probe() function from the Linux tpm_tis driver.
Signed-off-by: Stephen Douthit stephend@silicom-usa.com Tested-by: Stephen Douthit stephend@silicom-usa.com
src/hw/tpm_drivers.c | 21 +++++++++++++++------ src/std/tcg.h | 1 + src/tcgbios.c | 2 +- src/tcgbios.h | 4 ++++ 4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c index 789d70a..32c34df 100644 --- a/src/hw/tpm_drivers.c +++ b/src/hw/tpm_drivers.c @@ -11,6 +11,7 @@ #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD #include "hw/tpm_drivers.h" // struct tpm_driver #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT +#include "tcgbios.h" // tpm20_getcapability #include "output.h" // warn_timeout #include "stacks.h" // yield #include "string.h" // memcpy @@ -119,7 +120,7 @@ static u32 tis_probe(void) if ((didvid != 0) && (didvid != 0xffffffff)) rc = 1;
- /* TPM 2 has an interface register */ + /* Low 32 bits of CRB interface register overlap TIS interface register */ u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
Since this is tis_probe, I don't think we need to mention the CRB here ...
I think that was a note to myself I forgot to strip out when I dropped my debug prints. I'll change the comment back.
if ((ifaceid & 0xf) != 0xf) { @@ -142,13 +143,21 @@ static u32 tis_probe(void)
static TPMVersion tis_get_tpm_version(void) { - /* TPM 2 has an interface register */ - u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); + u8 buffer[128]; + int ret; + struct tpm2_res_getcapability *trg = + (struct tpm2_res_getcapability *)&buffer;
+ ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, cpu_to_be32(0x100), + cpu_to_be32(1), &trg->hdr, sizeof(buffer)); + if (ret == TPM2_RC_INITIALIZE) + return TPM_VERSION_2; + else if (ret) + return TPM_VERSION_NONE;
I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Doh. That makes sense. I'll need to find a 1.2 system to test on to avoid stupid mistakes like that.
Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
It might be, it wasn't clear to me if the tag could be trusted if the return code wasn't success. The Linux tpm2_probe() code bails before the tag comparison if tpm_transmit_cmd() returns an error.
Is there a block of QEMU code you would recommend as a reference for probing TPMs? I'm not very familiar with that code base, and a pointer would be handy.
https://github.com/stefanberger/qemu-tpm/blob/v2.4.1%2Btpm/hw/tpm/tpm_util.c...
What I don't like about the sending of commands is that we're breaking the layer here. We basically cannot send commands using this higher level API. The lowest we could go would be to call tpmhw_transmit, but that function cannot be used as it is but has to be split up to take tpm_driver as a parameter. You would pass to it &tpm_drivers[TIS_DRIVER_INDEX].
As far as I know, TPM 1.2 didn't have that register, so they either return static 0 or 0xffffffff in that register.
Though if reading the flags from the interface register really turns out to be insufficient, I think you should try to send a command like TPM2_CC_ReadClock. If you get back a TPM2_ST_NO_SESSION, it's a TPM 2. If you get a TPM_TAG_RSP_COMMAND it's a TPM 12.
Thanks for all the info. This gives me some experiments to run, and I'll try to find a solution that avoids sending commands if possible.
The following document on pdf page 100 defines bits 28-30 of the interface capability register. If the value there is 0x3 (mask 0x7), we should be sure to have a TPM 2. Hopefully that's sufficient also for your device for determining that it's a TPM 2.
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_...
My interface ID reads as 0xffffffff, and the interface capability register reads as intf_cap 0x30000697. So your suggestion of checking for 3 in that bitfield of the interface capability register works here.
If I change the version check function to the following and drop all the other changes from patch 3/3 would that be acceptable?
static TPMVersion tis_get_tpm_version(void) { u32 reg = readl(TIS_REG(0, TIS_REG_IFACE_ID));
/* FIFO interface as defined in PTP for TPM 2.0 is active */ if ((reg & 0xf) == 0) { return TPM_VERSION_2; } /* * FIFO interface as defined in TIS1.3 is active * Interface capabilities are defined in TIS_REG_INTF_CAPABILITY */ else if ((reg & 0xf) == 0xf) { reg = readl(TIS_REG(0, TIS_REG_INTF_CAPABILITY)); /* Interface 1.3 for TPM 2.0 */ if (((reg >> 28) & 0x7) == 3) return TPM_VERSION_2; }
return TPM_VERSION_1_2; }
On 02/27/2018 12:14 PM, Stephen Douthit wrote:
On 02/26/2018 07:45 PM, Stefan Berger wrote:
On 02/26/2018 06:24 PM, Stephen Douthit wrote:
On 02/26/2018 06:02 PM, Stefan Berger wrote:
On 02/26/2018 05:44 PM, Stephen Douthit wrote:
On 02/26/2018 05:09 PM, Stefan Berger wrote:
On 02/26/2018 03:37 PM, Stephen Douthit wrote: > tis_get_tpm_version() was returning the interface version, which > is always > 1.2 since tis_probe() would have failed if the interface wasn't > TIS. > > New version check is based on the tpm2_probe() function from the > Linux > tpm_tis driver. > > Signed-off-by: Stephen Douthit stephend@silicom-usa.com > Tested-by: Stephen Douthit stephend@silicom-usa.com > --- > src/hw/tpm_drivers.c | 21 +++++++++++++++------ > src/std/tcg.h | 1 + > src/tcgbios.c | 2 +- > src/tcgbios.h | 4 ++++ > 4 files changed, 21 insertions(+), 7 deletions(-) > > diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c > index 789d70a..32c34df 100644 > --- a/src/hw/tpm_drivers.c > +++ b/src/hw/tpm_drivers.c > @@ -11,6 +11,7 @@ > #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD > #include "hw/tpm_drivers.h" // struct tpm_driver > #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT > +#include "tcgbios.h" // tpm20_getcapability > #include "output.h" // warn_timeout > #include "stacks.h" // yield > #include "string.h" // memcpy > @@ -119,7 +120,7 @@ static u32 tis_probe(void) > if ((didvid != 0) && (didvid != 0xffffffff)) > rc = 1; > > - /* TPM 2 has an interface register */ > + /* Low 32 bits of CRB interface register overlap TIS > interface register */ > u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
Since this is tis_probe, I don't think we need to mention the CRB here ...
I think that was a note to myself I forgot to strip out when I dropped my debug prints. I'll change the comment back.
> > if ((ifaceid & 0xf) != 0xf) { > @@ -142,13 +143,21 @@ static u32 tis_probe(void) > > static TPMVersion tis_get_tpm_version(void) > { > - /* TPM 2 has an interface register */ > - u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); > + u8 buffer[128]; > + int ret; > + struct tpm2_res_getcapability *trg = > + (struct tpm2_res_getcapability *)&buffer; > + > + ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, > cpu_to_be32(0x100), > + cpu_to_be32(1), &trg->hdr, > sizeof(buffer)); > + if (ret == TPM2_RC_INITIALIZE) > + return TPM_VERSION_2; > + else if (ret) > + return TPM_VERSION_NONE; I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Doh. That makes sense. I'll need to find a 1.2 system to test on to avoid stupid mistakes like that.
Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
It might be, it wasn't clear to me if the tag could be trusted if the return code wasn't success. The Linux tpm2_probe() code bails before the tag comparison if tpm_transmit_cmd() returns an error.
Is there a block of QEMU code you would recommend as a reference for probing TPMs? I'm not very familiar with that code base, and a pointer would be handy.
https://github.com/stefanberger/qemu-tpm/blob/v2.4.1%2Btpm/hw/tpm/tpm_util.c...
What I don't like about the sending of commands is that we're breaking the layer here. We basically cannot send commands using this higher level API. The lowest we could go would be to call tpmhw_transmit, but that function cannot be used as it is but has to be split up to take tpm_driver as a parameter. You would pass to it &tpm_drivers[TIS_DRIVER_INDEX].
As far as I know, TPM 1.2 didn't have that register, so they either return static 0 or 0xffffffff in that register.
Though if reading the flags from the interface register really turns out to be insufficient, I think you should try to send a command like TPM2_CC_ReadClock. If you get back a TPM2_ST_NO_SESSION, it's a TPM 2. If you get a TPM_TAG_RSP_COMMAND it's a TPM 12.
Thanks for all the info. This gives me some experiments to run, and I'll try to find a solution that avoids sending commands if possible.
The following document on pdf page 100 defines bits 28-30 of the interface capability register. If the value there is 0x3 (mask 0x7), we should be sure to have a TPM 2. Hopefully that's sufficient also for your device for determining that it's a TPM 2.
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_...
My interface ID reads as 0xffffffff, and the interface capability register reads as intf_cap 0x30000697. So your suggestion of checking for 3 in that bitfield of the interface capability register works here.
If I change the version check function to the following and drop all the other changes from patch 3/3 would that be acceptable?
static TPMVersion tis_get_tpm_version(void) { u32 reg = readl(TIS_REG(0, TIS_REG_IFACE_ID));
/* FIFO interface as defined in PTP for TPM 2.0 is active */ if ((reg & 0xf) == 0) { return TPM_VERSION_2; } /* * FIFO interface as defined in TIS1.3 is active * Interface capabilities are defined in TIS_REG_INTF_CAPABILITY */ else if ((reg & 0xf) == 0xf) { reg = readl(TIS_REG(0, TIS_REG_INTF_CAPABILITY)); /* Interface 1.3 for TPM 2.0 */ if (((reg >> 28) & 0x7) == 3) return TPM_VERSION_2;
I think this is actually the better test and you should put this one up front.
Stefan
} return TPM_VERSION_1_2;}
On 02/27/2018 12:17 PM, Stefan Berger wrote:
On 02/27/2018 12:14 PM, Stephen Douthit wrote:
On 02/26/2018 07:45 PM, Stefan Berger wrote:
On 02/26/2018 06:24 PM, Stephen Douthit wrote:
On 02/26/2018 06:02 PM, Stefan Berger wrote:
On 02/26/2018 05:44 PM, Stephen Douthit wrote:
On 02/26/2018 05:09 PM, Stefan Berger wrote: > On 02/26/2018 03:37 PM, Stephen Douthit wrote: >> tis_get_tpm_version() was returning the interface version, which is always >> 1.2 since tis_probe() would have failed if the interface wasn't TIS. >> >> New version check is based on the tpm2_probe() function from the Linux >> tpm_tis driver. >> >> Signed-off-by: Stephen Douthit stephend@silicom-usa.com >> Tested-by: Stephen Douthit stephend@silicom-usa.com >> --- >> src/hw/tpm_drivers.c | 21 +++++++++++++++------ >> src/std/tcg.h | 1 + >> src/tcgbios.c | 2 +- >> src/tcgbios.h | 4 ++++ >> 4 files changed, 21 insertions(+), 7 deletions(-) >> >> diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c >> index 789d70a..32c34df 100644 >> --- a/src/hw/tpm_drivers.c >> +++ b/src/hw/tpm_drivers.c >> @@ -11,6 +11,7 @@ >> #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD >> #include "hw/tpm_drivers.h" // struct tpm_driver >> #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT >> +#include "tcgbios.h" // tpm20_getcapability >> #include "output.h" // warn_timeout >> #include "stacks.h" // yield >> #include "string.h" // memcpy >> @@ -119,7 +120,7 @@ static u32 tis_probe(void) >> if ((didvid != 0) && (didvid != 0xffffffff)) >> rc = 1; >> >> - /* TPM 2 has an interface register */ >> + /* Low 32 bits of CRB interface register overlap TIS interface register */ >> u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); > > Since this is tis_probe, I don't think we need to mention the CRB here ...
I think that was a note to myself I forgot to strip out when I dropped my debug prints. I'll change the comment back.
>> >> if ((ifaceid & 0xf) != 0xf) { >> @@ -142,13 +143,21 @@ static u32 tis_probe(void) >> >> static TPMVersion tis_get_tpm_version(void) >> { >> - /* TPM 2 has an interface register */ >> - u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); >> + u8 buffer[128]; >> + int ret; >> + struct tpm2_res_getcapability *trg = >> + (struct tpm2_res_getcapability *)&buffer; >> + >> + ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, cpu_to_be32(0x100), >> + cpu_to_be32(1), &trg->hdr, sizeof(buffer)); >> + if (ret == TPM2_RC_INITIALIZE) >> + return TPM_VERSION_2; >> + else if (ret) >> + return TPM_VERSION_NONE; > I haven't tried yet, but likely this will not work for TPM 1.2 since it will also return an error.
Doh. That makes sense. I'll need to find a 1.2 system to test on to avoid stupid mistakes like that.
> Wouldn't the check for the tag below not be sufficient? We are doing something similar in QEMU when probing for the TPM. Here we send TPM 2 command TPM_CC_ReadClock and check the tag for TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check for TPM 1.2 tag.
It might be, it wasn't clear to me if the tag could be trusted if the return code wasn't success. The Linux tpm2_probe() code bails before the tag comparison if tpm_transmit_cmd() returns an error.
Is there a block of QEMU code you would recommend as a reference for probing TPMs? I'm not very familiar with that code base, and a pointer would be handy.
https://github.com/stefanberger/qemu-tpm/blob/v2.4.1%2Btpm/hw/tpm/tpm_util.c...
What I don't like about the sending of commands is that we're breaking the layer here. We basically cannot send commands using this higher level API. The lowest we could go would be to call tpmhw_transmit, but that function cannot be used as it is but has to be split up to take tpm_driver as a parameter. You would pass to it &tpm_drivers[TIS_DRIVER_INDEX].
As far as I know, TPM 1.2 didn't have that register, so they either return static 0 or 0xffffffff in that register.
Though if reading the flags from the interface register really turns out to be insufficient, I think you should try to send a command like TPM2_CC_ReadClock. If you get back a TPM2_ST_NO_SESSION, it's a TPM 2. If you get a TPM_TAG_RSP_COMMAND it's a TPM 12.
Thanks for all the info. This gives me some experiments to run, and I'll try to find a solution that avoids sending commands if possible.
The following document on pdf page 100 defines bits 28-30 of the interface capability register. If the value there is 0x3 (mask 0x7), we should be sure to have a TPM 2. Hopefully that's sufficient also for your device for determining that it's a TPM 2.
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_...
My interface ID reads as 0xffffffff, and the interface capability register reads as intf_cap 0x30000697. So your suggestion of checking for 3 in that bitfield of the interface capability register works here.
If I change the version check function to the following and drop all the other changes from patch 3/3 would that be acceptable?
static TPMVersion tis_get_tpm_version(void) { u32 reg = readl(TIS_REG(0, TIS_REG_IFACE_ID));
/* FIFO interface as defined in PTP for TPM 2.0 is active */ if ((reg & 0xf) == 0) { return TPM_VERSION_2; } /* * FIFO interface as defined in TIS1.3 is active * Interface capabilities are defined in TIS_REG_INTF_CAPABILITY */ else if ((reg & 0xf) == 0xf) { reg = readl(TIS_REG(0, TIS_REG_INTF_CAPABILITY)); /* Interface 1.3 for TPM 2.0 */ if (((reg >> 28) & 0x7) == 3) return TPM_VERSION_2;
I think this is actually the better test and you should put this one up front.
By "up front" do you want me to swap the order of if/else if tests, or make this the first patch in the series?
Stefan
}
return TPM_VERSION_1_2; }
On 02/27/2018 12:35 PM, Stephen Douthit wrote:
On 02/27/2018 12:17 PM, Stefan Berger wrote:
On 02/27/2018 12:14 PM, Stephen Douthit wrote:
On 02/26/2018 07:45 PM, Stefan Berger wrote:
On 02/26/2018 06:24 PM, Stephen Douthit wrote:
On 02/26/2018 06:02 PM, Stefan Berger wrote:
On 02/26/2018 05:44 PM, Stephen Douthit wrote: > On 02/26/2018 05:09 PM, Stefan Berger wrote: >> On 02/26/2018 03:37 PM, Stephen Douthit wrote: >>> tis_get_tpm_version() was returning the interface version, >>> which is always >>> 1.2 since tis_probe() would have failed if the interface >>> wasn't TIS. >>> >>> New version check is based on the tpm2_probe() function from >>> the Linux >>> tpm_tis driver. >>> >>> Signed-off-by: Stephen Douthit stephend@silicom-usa.com >>> Tested-by: Stephen Douthit stephend@silicom-usa.com >>> --- >>> src/hw/tpm_drivers.c | 21 +++++++++++++++------ >>> src/std/tcg.h | 1 + >>> src/tcgbios.c | 2 +- >>> src/tcgbios.h | 4 ++++ >>> 4 files changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c >>> index 789d70a..32c34df 100644 >>> --- a/src/hw/tpm_drivers.c >>> +++ b/src/hw/tpm_drivers.c >>> @@ -11,6 +11,7 @@ >>> #include "config.h" // CONFIG_TPM_TIS_SHA1THRESHOLD >>> #include "hw/tpm_drivers.h" // struct tpm_driver >>> #include "std/tcg.h" // TCG_RESPONSE_TIMEOUT >>> +#include "tcgbios.h" // tpm20_getcapability >>> #include "output.h" // warn_timeout >>> #include "stacks.h" // yield >>> #include "string.h" // memcpy >>> @@ -119,7 +120,7 @@ static u32 tis_probe(void) >>> if ((didvid != 0) && (didvid != 0xffffffff)) >>> rc = 1; >>> >>> - /* TPM 2 has an interface register */ >>> + /* Low 32 bits of CRB interface register overlap TIS >>> interface register */ >>> u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); >> >> Since this is tis_probe, I don't think we need to mention the >> CRB here ... > > I think that was a note to myself I forgot to strip out when I > dropped my debug prints. I'll change the comment back. > >>> >>> if ((ifaceid & 0xf) != 0xf) { >>> @@ -142,13 +143,21 @@ static u32 tis_probe(void) >>> >>> static TPMVersion tis_get_tpm_version(void) >>> { >>> - /* TPM 2 has an interface register */ >>> - u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID)); >>> + u8 buffer[128]; >>> + int ret; >>> + struct tpm2_res_getcapability *trg = >>> + (struct tpm2_res_getcapability *)&buffer; >>> + >>> + ret = tpm20_getcapability(TPM2_CAP_TPM_PROPERTIES, >>> cpu_to_be32(0x100), >>> + cpu_to_be32(1), &trg->hdr, >>> sizeof(buffer)); >>> + if (ret == TPM2_RC_INITIALIZE) >>> + return TPM_VERSION_2; >>> + else if (ret) >>> + return TPM_VERSION_NONE; >> I haven't tried yet, but likely this will not work for TPM 1.2 >> since it will also return an error. > > Doh. That makes sense. I'll need to find a 1.2 system to test > on to > avoid stupid mistakes like that. > >> Wouldn't the check for the tag below not be sufficient? We are >> doing something similar in QEMU when probing for the TPM. Here >> we send TPM 2 command TPM_CC_ReadClock and check the tag for >> TPM 2 type and then TPM 1.2 command TPM_ORD_GetTicks and check >> for TPM 1.2 tag. > > It might be, it wasn't clear to me if the tag could be trusted > if the > return code wasn't success. The Linux tpm2_probe() code bails > before > the tag comparison if tpm_transmit_cmd() returns an error. > > Is there a block of QEMU code you would recommend as a reference > for > probing TPMs? I'm not very familiar with that code base, and a > pointer would be handy.
https://github.com/stefanberger/qemu-tpm/blob/v2.4.1%2Btpm/hw/tpm/tpm_util.c...
What I don't like about the sending of commands is that we're breaking the layer here. We basically cannot send commands using this higher level API. The lowest we could go would be to call tpmhw_transmit, but that function cannot be used as it is but has to be split up to take tpm_driver as a parameter. You would pass to it &tpm_drivers[TIS_DRIVER_INDEX].
As far as I know, TPM 1.2 didn't have that register, so they either return static 0 or 0xffffffff in that register.
Though if reading the flags from the interface register really turns out to be insufficient, I think you should try to send a command like TPM2_CC_ReadClock. If you get back a TPM2_ST_NO_SESSION, it's a TPM 2. If you get a TPM_TAG_RSP_COMMAND it's a TPM 12.
Thanks for all the info. This gives me some experiments to run, and I'll try to find a solution that avoids sending commands if possible.
The following document on pdf page 100 defines bits 28-30 of the interface capability register. If the value there is 0x3 (mask 0x7), we should be sure to have a TPM 2. Hopefully that's sufficient also for your device for determining that it's a TPM 2.
https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_...
My interface ID reads as 0xffffffff, and the interface capability register reads as intf_cap 0x30000697. So your suggestion of checking for 3 in that bitfield of the interface capability register works here.
If I change the version check function to the following and drop all the other changes from patch 3/3 would that be acceptable?
static TPMVersion tis_get_tpm_version(void) { u32 reg = readl(TIS_REG(0, TIS_REG_IFACE_ID));
/* FIFO interface as defined in PTP for TPM 2.0 is active */ if ((reg & 0xf) == 0) { return TPM_VERSION_2; } /* * FIFO interface as defined in TIS1.3 is active * Interface capabilities are defined in TIS_REG_INTF_CAPABILITY */ else if ((reg & 0xf) == 0xf) { reg = readl(TIS_REG(0, TIS_REG_INTF_CAPABILITY)); /* Interface 1.3 for TPM 2.0 */ if (((reg >> 28) & 0x7) == 3) return TPM_VERSION_2;I think this is actually the better test and you should put this one up front.
By "up front" do you want me to swap the order of if/else if tests, or make this the first patch in the series?
Make this the first test in tis_get_tpm_version().
Stefan} return TPM_VERSION_1_2;}
-
Stefan Berger -
Stephen Douthit