Re: [Qemu-devel] [PATCH V8 03/14] Add persistent state handling to TPM TIS frontend driver

[Michael S. Tsirkin]

On Wed, Aug 31, 2011 at 10:35:54AM -0400, Stefan Berger wrote:
> This patch adds support for handling of persistent state to the TPM TIS
> frontend.
> 
> The currently used buffer is determined (can only be in currently active
> locality and either be a read or a write buffer) and only that buffer's 
> content
> is stored. The reverse is done when the state is restored from disk
> where the buffer's content are copied into the currently used buffer.
> 
> To keep compatibility with existing Xen implementation the VMStateDescription
> was adapted to be compatible with existing state. For that I am adding Andreas
> Niederl as an author to the file.
> 
> v5:
>  - removing qdev.no_user=1
> 
> v4:
>  - main thread releases the 'state' lock while periodically calling the
>    backends function that may request it to write data into block storage.
> 
> v3:
>  - all functions prefixed with tis_
>  - while the main thread is waiting for an outstanding TPM command to finish,
>    it periodically does some work (writes data to the block storage)
> 
> Signed-off-by: Stefan Berger <address@hidden>
> 
> ---
>  hw/tpm_tis.c |  166 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 166 insertions(+)
> 
> Index: qemu-git/hw/tpm_tis.c
> ===================================================================
> --- qemu-git.orig/hw/tpm_tis.c
> +++ qemu-git/hw/tpm_tis.c
> @@ -6,6 +6,8 @@
>   * Author: Stefan Berger <address@hidden>
>   *         David Safford <address@hidden>
>   *
> + * Xen 4 support: Andrease Niederl <address@hidden>
> + *
>   * This program is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU General Public License as
>   * published by the Free Software Foundation, version 2 of the
> @@ -839,3 +841,167 @@ static int tis_init(ISADevice *dev)
>   err_exit:
>      return -1;
>  }
> +
> +/* persistent state handling */
> +
> +static void tis_pre_save(void *opaque)
> +{
> +    TPMState *s = opaque;
> +    uint8_t locty = s->active_locty;
> +
> +    qemu_mutex_lock(&s->state_lock);
> +
> +    /* wait for outstanding requests to complete */
> +    if (IS_VALID_LOCTY(locty) && s->loc[locty].state == STATE_EXECUTION) {
> +        if (!s->be_driver->ops->job_for_main_thread) {
> +            qemu_cond_wait(&s->from_tpm_cond, &s->state_lock);
> +        } else {
> +            while (s->loc[locty].state == STATE_EXECUTION) {
> +                qemu_mutex_unlock(&s->state_lock);
> +
> +                s->be_driver->ops->job_for_main_thread(NULL);
> +                usleep(10000);
> +
> +                qemu_mutex_lock(&s->state_lock);
> +            }
> +        }
> +    }
> +
> +#ifdef DEBUG_TIS_SR
> +    fprintf(stderr,
> +            "tpm_tis: suspend: locty 0 : r_offset = %d, w_offset = %d\n",
> +            s->loc[0].r_offset, s->loc[0].w_offset);
> +    if (s->loc[0].r_offset) {
> +        tis_dump_state(opaque, 0);
> +    }
> +#endif
> +
> +    qemu_mutex_unlock(&s->state_lock);
> +
> +    /* copy current active read or write buffer into the buffer
> +       written to disk */
> +    if (IS_VALID_LOCTY(locty)) {
> +        switch (s->loc[locty].state) {
> +        case STATE_RECEPTION:
> +            memcpy(s->buf,
> +                   s->loc[locty].w_buffer.buffer,
> +                   MIN(sizeof(s->buf),
> +                       s->loc[locty].w_buffer.size));
> +            s->offset = s->loc[locty].w_offset;
> +        break;
> +        case STATE_COMPLETION:
> +            memcpy(s->buf,
> +                   s->loc[locty].r_buffer.buffer,
> +                   MIN(sizeof(s->buf),
> +                       s->loc[locty].r_buffer.size));
> +            s->offset = s->loc[locty].r_offset;
> +        break;
> +        default:
> +            /* leak nothing */
> +            memset(s->buf, 0x0, sizeof(s->buf));
> +        break;
> +        }
> +    }
> +
> +    s->be_driver->ops->save_volatile_data();
> +}
> +
> +
> +static int tis_post_load(void *opaque,
> +                         int version_id __attribute__((unused)))
> +{
> +    TPMState *s = opaque;
> +
> +    uint8_t locty = s->active_locty;
> +
> +    if (IS_VALID_LOCTY(locty)) {
> +        switch (s->loc[locty].state) {
> +        case STATE_RECEPTION:
> +            memcpy(s->loc[locty].w_buffer.buffer,
> +                   s->buf,
> +                   MIN(sizeof(s->buf),
> +                       s->loc[locty].w_buffer.size));
> +            s->loc[locty].w_offset = s->offset;
> +        break;
> +        case STATE_COMPLETION:
> +            memcpy(s->loc[locty].r_buffer.buffer,
> +                   s->buf,
> +                   MIN(sizeof(s->buf),
> +                       s->loc[locty].r_buffer.size));
> +            s->loc[locty].r_offset = s->offset;
> +        break;
> +        default:
> +        break;
> +        }
> +    }

Should this do something with interrupts as well?

> +
> +#ifdef DEBUG_TIS_SR
> +    fprintf(stderr,
> +            "tpm_tis: resume : locty 0 : r_offset = %d, w_offset = %d\n",
> +            s->loc[0].r_offset, s->loc[0].w_offset);
> +#endif
> +
> +    return s->be_driver->ops->load_volatile_data(s);
> +}
> +
> +
> +static const VMStateDescription vmstate_locty = {
> +    .name = "loc",
> +    .version_id = 1,
> +    .minimum_version_id = 0,
> +    .minimum_version_id_old = 0,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_UINT32(state, TPMLocality),
> +        VMSTATE_UINT32(inte, TPMLocality),
> +        VMSTATE_UINT32(ints, TPMLocality),
> +        VMSTATE_UINT8(access, TPMLocality),
> +        VMSTATE_UINT8(sts, TPMLocality),
> +        VMSTATE_END_OF_LIST(),
> +    }
> +};
> +
> +
> +static const VMStateDescription vmstate_tis = {
> +    .name = "tpm",
> +    .version_id = 1,
> +    .minimum_version_id = 0,
> +    .minimum_version_id_old = 0,
> +    .pre_save  = tis_pre_save,
> +    .post_load = tis_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(irq_num, TPMState),
> +        VMSTATE_UINT32(offset, TPMState),
> +        VMSTATE_BUFFER(buf, TPMState),
> +        VMSTATE_UINT8(active_locty, TPMState),
> +        VMSTATE_UINT8(aborting_locty, TPMState),
> +        VMSTATE_UINT8(next_locty, TPMState),

Is irq_num guest modifiable?
If yes post load should do something with it?
If not, why are we migrating it?

> +
> +        VMSTATE_STRUCT_ARRAY(loc, TPMState, NUM_LOCALITIES, 1,
> +                             vmstate_locty, TPMLocality),
> +
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +
> +static ISADeviceInfo tis_device_info = {
> +    .init         = tis_init,
> +    .qdev.name    = "tpm-tis",
> +    .qdev.size    = sizeof(TPMState),
> +    .qdev.vmsd    = &vmstate_tis,
> +    .qdev.reset   = tis_reset,
> +    .qdev.props = (Property[]) {
> +        DEFINE_PROP_UINT32("irq", TPMState,
> +                           irq_num, TPM_TIS_IRQ),
> +        DEFINE_PROP_STRING("tpmdev", TPMState, backend),
> +        DEFINE_PROP_END_OF_LIST(),
> +    },
> +};
> +
> +
> +static void tis_register_device(void)
> +{
> +    isa_qdev_register(&tis_device_info);
> +}
> +
> +device_init(tis_register_device)
> 

So this is a qdev device. Why do we need a new flag to set it up then?