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

[Stefan Berger]

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