Re: [Qemu-devel] [PATCH 2/3] tpm: Probe for connected TPM 1.2 or TPM 2

[Stefan Berger <stefanb@linux.vnet.ibm.com>]

On 04/07/2015 04:54 AM, Xu, Quan wrote:
>
>> -----Original Message-----
>> From: Stefan Berger [mailto:stefanb@linux.vnet.ibm.com]
>> Sent: Wednesday, April 01, 2015 3:40 AM
>> To: qemu-devel@nongnu.org; mst@redhat.com
>> Cc: Xu, Quan; Stefan Berger; Stefan Berger
>> Subject: [PATCH 2/3] tpm: Probe for connected TPM 1.2 or TPM 2
>>
>> In the TPM passthrough backend driver, modify the probing code so that we can
>> check whether a TPM 1.2 or TPM 2 is being used and adapt the behavior of the
>> TPM TIS accordingly.
>>
>> Move the code that tested for a TPM 1.2 into tpm_utils.c and extend it with test
>> for probing for TPM 2. Have the function return the version of TPM found.
>>
>> Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
>> ---
>>   hw/tpm/Makefile.objs     |   2 +-
>>   hw/tpm/tpm_int.h         |   6 +++
>>   hw/tpm/tpm_passthrough.c |  59 +++-------------------
>>   hw/tpm/tpm_util.c        | 126
>> +++++++++++++++++++++++++++++++++++++++++++++++
>>   hw/tpm/tpm_util.h        |  28 +++++++++++
>>   5 files changed, 167 insertions(+), 54 deletions(-)  create mode 100644
>> hw/tpm/tpm_util.c  create mode 100644 hw/tpm/tpm_util.h
>>
>> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index
>> 99f5983..64cecc3 100644
>> --- a/hw/tpm/Makefile.objs
>> +++ b/hw/tpm/Makefile.objs
>> @@ -1,2 +1,2 @@
>>   common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
>> -common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
>> +common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
>> tpm_util.o
>> diff --git a/hw/tpm/tpm_int.h b/hw/tpm/tpm_int.h index 24e12ce..edab824
>> 100644
>> --- a/hw/tpm/tpm_int.h
>> +++ b/hw/tpm/tpm_int.h
>> @@ -66,4 +66,10 @@ struct tpm_resp_hdr {  #define
>> TPM_ORD_ContinueSelfTest  0x53
>>   #define TPM_ORD_GetTicks          0xf1
>>
>> +
>> +/* TPM2 defines */
>> +#define TPM_ST_NO_SESSIONS        0x8001
>> +
>> +#define TPM_CC_ReadClock          0x00000181
>> +
> Could you define TPM2 macro definitions beginning with 'TPM2_*'?


Ok, will do.

[...]
> +/*
> + * Probe for the TPM device in the back
> + * Returns 0 on success with the version of the probed TPM set, 1 on failure.
> + */
> +int tpm_util_test_tpmdev(int tpm_fd, enum TPMVersion *tpm_version) {
> +    /*
> +     * Sending a TPM1.2 command to a TPM2 should return a TPM1.2
> +     * header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
> +     *
> +     * Sending a TPM2 command to a TPM 2 will give a TPM 2 tag in the
> +     * header.
> +     * Sending a TPM2 command to a TPM 1.2 will give a TPM 1.2 tag
> +     * in the header and an error code.
> +     */
> +    const struct tpm_req_hdr test_req = {
> +        .tag = cpu_to_be16(TPM_TAG_RQU_COMMAND),
> +        .len = cpu_to_be32(sizeof(test_req)),
> +        .ordinal = cpu_to_be32(TPM_ORD_GetTicks),
> +    };
> +
> +    const struct tpm_req_hdr test_req_tpm2 = {
> +        .tag = cpu_to_be16(TPM_ST_NO_SESSIONS),
> +        .len = cpu_to_be32(sizeof(test_req_tpm2)),
> +        .ordinal = cpu_to_be32(TPM_CC_ReadClock),
> +    };
> +    uint16_t returnTag;
> +    int ret;
> +
> +    /* Send TPM 2 command */
> +    ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req_tpm2,
> +                        sizeof(test_req_tpm2), &returnTag);
> +    /* TPM 2 would respond with a tag of TPM_ST_NO_SESSIONS */
> +    if (!ret && returnTag == TPM_ST_NO_SESSIONS) {
> +        *tpm_version = TPMVersion2_0;
> +        return 0;
> +    }
> +
> +    /* Send TPM 1.2 command */
> +    ret = tpm_util_test(tpm_fd, (unsigned char *)&test_req,
> +                        sizeof(test_req), &returnTag);
> +    if (!ret && returnTag == TPM_TAG_RSP_COMMAND) {
> +        *tpm_version = TPMVersion1_2;
> +        /* this is a TPM 1.2 */
> +        return 0;
> +    }
> +
> +    *tpm_version = TPMVersion_Unspec;
> +
> +    return 1;
> +}
>
> In my opinion, I prefer to point out tpm_version in QEMU command line options, then
> tpm_util_test_tpmdev() tries to verify it.

The only reason why I am not doing this was that libvirt for example 
will need to probe for whether the additional parameter indicating the 
TPM version is supported. Besides that I thought it should be possible 
to probe on any platform and get a reliable result.

Maybe Eric has a comment. I have recently seen a discussion where an 
additional parameter to an existing option was to be added, but cannot 
remember which option that was.

    Stefan