Re: [Qemu-devel] [PATCH V19 5/7] Add a TPM Passthrough backend driver implementation

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

On 09/27/2012 10:28 AM, Corey Bryant wrote:
>
>
> On 06/04/2012 03:37 PM, Stefan Berger wrote:
>> +
>> +/* borrowed from qemu-char.c */
>> +static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, 
>> uint32_t len)
>> +{
>> +    return send_all(fd, buf, len);
>> +}
>> +
>> +static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t 
>> len)
>
> It would probably be useful to other parts of QEMU if you defined this 
> function as recv_all() and put it in qemu-char.c (to correspond with 
> send_all()).
>

Ok. I'd keep this patch to qemu-char.c inside of this patch, though, 
unless someone opposes.

>> +
>> +static int tpm_passthrough_startup_tpm(TPMBackend *tb)
>> +{
>> +    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
>> +    int rc;
>> +
>> +    rc = tpm_passthrough_do_startup_tpm(tb);
>> +    if (rc) {
>> +        tpm_pt->had_startup_error = true;
>
> tpm_passthrough_do_startup_tpm() always returns zero, so 
> had_startup_error will never be set to true.
>

I am consolidating the code into tpm_passthrough_startup_tpm() but let 
that return an int considering that other backend drivers may return an 
error code.

>> +static void tpm_passthrough_reset(TPMBackend *tb)
>> +{
>> +    TPMPassthruState *tpm_pt = tb->s.tpm_pt;
>> +
>> +    dprintf("tpm_passthrough: CALL TO TPM_RESET!\n");
>> +
>> +    tpm_backend_thread_end(&tpm_pt->tbt);
>
> Should the thread be restarted here?
>

No. If the backend needs to be restarted, the frontend will invoke the 
startup_tpm function.


>> +static bool tpm_passthrough_get_tpm_established_flag(TPMBackend *tb)
>> +{
>> +    return false;
>
> Can a T/OS never be established with the passthrough backend?
>

Following the specs, this bit is set when locality 5 is used and a hash 
operation is started. See part 2 of the TPM specs.
The establishment bit could be read from the hardware TPM's permanent 
flags. The question is what it means to show this bit inside the VM. I 
wasn't sure, so I hardcoded it to always return false.

     Stefan