Re: [Qemu-devel] [PATCH v5 6/6] Qemu-Xen-vTPM: Add a parameter indicating whether the command that was a selftest

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

On 04/12/2015 10:15 PM, Xu, Quan wrote:
>
>> -----Original Message-----
>> From: Stefan Berger [mailto:stefanb@linux.vnet.ibm.com]
>> Sent: Monday, April 13, 2015 4:50 AM
>> To: Xu, Quan; stefano.stabellini@eu.citrix.com; eblake@redhat.com
>> Cc: pbonzini@redhat.com; qemu-devel@nongnu.org; aliguori@amazon.com;
>> wei.liu2@citrix.com; dgdegra@tycho.nsa.gov; xen-devel@lists.xen.org
>> Subject: Re: [PATCH v5 6/6] Qemu-Xen-vTPM: Add a parameter indicating
>> whether the command that was a selftest
>>
>> On 04/10/2015 02:59 AM, Quan Xu wrote:
>>> and whether it completed successfully. Move
>>> tpm_passthrough_is_selftest() into tpm_util.c and rename it to
>> tpm_util_is_selftest().
>>> Signed-off-by: Quan Xu <quan.xu@intel.com>
>>> ---
>>>    hw/tpm/Makefile.objs             |  2 +-
>>>    hw/tpm/tpm_passthrough.c         | 13 +----------
>>>    hw/tpm/tpm_util.c                | 50
>> ++++++++++++++++++++++++++++++++++++++++
>>>    hw/tpm/tpm_xenstubdoms.c         | 36
>> +++++++++++++++++++++++------
>>>    include/sysemu/tpm_backend_int.h |  1 +
>>>    5 files changed, 82 insertions(+), 20 deletions(-)
>>>    create mode 100644 hw/tpm/tpm_util.c
>>>
>>> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index
>>> 190e776..cba961c 100644
>>> --- a/hw/tpm/Makefile.objs
>>> +++ b/hw/tpm/Makefile.objs
>>> @@ -1,3 +1,3 @@
>>> -common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
>>> +common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o tpm_util.o
>>>    common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
>>>    common-obj-$(CONFIG_TPM_XENSTUBDOMS) += tpm_xenstubdoms.o
>>> xen_vtpm_frontend.o diff --git a/hw/tpm/tpm_passthrough.c
>>> b/hw/tpm/tpm_passthrough.c index 2a45071..ff08e15 100644
>>> --- a/hw/tpm/tpm_passthrough.c
>>> +++ b/hw/tpm/tpm_passthrough.c
>>> @@ -112,17 +112,6 @@ static void tpm_write_fatal_error_response(uint8_t
>> *out, uint32_t out_len)
>>>        }
>>>    }
>>>
>>> -static bool tpm_passthrough_is_selftest(const uint8_t *in, uint32_t
>>> in_len) -{
>>> -    struct tpm_req_hdr *hdr = (struct tpm_req_hdr *)in;
>>> -
>>> -    if (in_len >= sizeof(*hdr)) {
>>> -        return (be32_to_cpu(hdr->ordinal) == TPM_ORD_ContinueSelfTest);
>>> -    }
>>> -
>>> -    return false;
>>> -}
>>> -
>>>    static int tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>>>                                            const uint8_t *in, uint32_t
>> in_len,
>>>                                            uint8_t *out, uint32_t
>>> out_len, @@ -136,7 +125,7 @@ static int
>> tpm_passthrough_unix_tx_bufs(TPMPassthruState *tpm_pt,
>>>        tpm_pt->tpm_executing = true;
>>>        *selftest_done = false;
>>>
>>> -    is_selftest = tpm_passthrough_is_selftest(in, in_len);
>>> +    is_selftest = tpm_util_is_selftest(in, in_len);
>>>
>>>        ret = tpm_passthrough_unix_write(tpm_pt->tpm_fd, in, in_len);
>>>        if (ret != in_len) {
>>> diff --git a/hw/tpm/tpm_util.c b/hw/tpm/tpm_util.c new file mode
>>> 100644 index 0000000..8566781
>>> --- /dev/null
>>> +++ b/hw/tpm/tpm_util.c
>>> @@ -0,0 +1,50 @@
>>> +/*
>>> + *  TPM util functions
>>> + *
>>> + * *  Copyright (c) 2015 Intel Corporation
>>> + *  Authors:
>>> + *    Quan Xu <quan.xu@intel.com>
>>> + *
>>> + *  Copyright (c) 2010 - 2013 IBM Corporation
>>> + *  Authors:
>>> + *    Stefan Berger <stefanb@us.ibm.com>
>>> + *
>>> + *  Copyright (C) 2011 IAIK, Graz University of Technology
>>> + *    Author: Andreas Niederl
>> I don't think that for this particular function this Copyright applies.
>
> Thanks Stefan.
> Could I just delete "Copyright (C) 2011 IAIK.."? share me more specific copyright, if I am not correct.

For this single function that's going to land in tpm_util.c for now we 
don't need this Copyright.
>> This patch per se looks good, but some of your modifications will not compile
>> until this patch is applied, due to the missing selftest_done parameter. So you
>> should merge this patch into the 3rd patch in this series.
>>
> Okay, I will do it in next version.
> If the other patches are good, I can send out v6 soon.


Thanks.

     Stefan