[SeaBIOS] [PATCH v2 4/4] tpm: add TPM CRB device support
Stefan Berger
stefanb at linux.vnet.ibm.com
Mon Feb 26 14:57:03 CET 2018
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.lureau at 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.lureau at redhat.com>
>> Reviewed-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> Tested-by: Stefan Berger <stefanb at 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
>
More information about the SeaBIOS
mailing list