Re: [Xen-devel] [PATCH v4 4/5] Qemu-Xen-vTPM: Qemu vTPM xenstubdoms backen.

["Xu, Quan" <quan.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Stefan Berger [mailto:stefanb@xxxxxxxxxxxxxxxxxx]
> Sent: Thursday, March 19, 2015 3:17 AM
> To: Xu, Quan; stefano.stabellini@xxxxxxxxxxxxx; qemu-devel@xxxxxxxxxx;
> armbru@xxxxxxxxxx; lcapitulino@xxxxxxxxxx; aliguori@xxxxxxxxxx;
> pbonzini@xxxxxxxxxx; eblake@xxxxxxxxxx; kraxel@xxxxxxxxxx;
> meyering@xxxxxxxxxx; mjt@xxxxxxxxxx; sw@xxxxxxxxxxx; wei.liu2@xxxxxxxxxx
> Cc: xen-devel@xxxxxxxxxxxxx
> Subject: Re: [PATCH v4 4/5] Qemu-Xen-vTPM: Qemu vTPM xenstubdoms
> backen.
> 
> On 03/10/2015 08:14 AM, Quan Xu wrote:
> > This Patch provides the glue for the TPM_TIS(Qemu frontend) to Xen
> > stubdom vTPM domain that provides the actual TPM functionality. It
> > sends data and TPM commends with xen_vtpm_frontend. It is similar as
> > another two vTPM backens:
> >    *vTPM passthrough backen Since QEMU 1.5.
> >    *vTPM libtpms-based backen.
> >
> > Some details:
> > This part of the patch provides support for the spawning of a thread
> > that will interact with stubdom vTPM domain by the xen_vtpm_frontend.
> > It expects a signal from the frontend to wake and pick up the TPM
> > command that is supposed to be processed and delivers the response
> > packet using a callback function provided by the frontend.
> >
> > The backend connects itself to the frontend by filling out an
> > interface structure with pointers to the function implementing support
> > for various operations.
> >
> > (QEMU) vTPM XenStubdoms backen is initialized by Qemu command line
> options,
> >        "-tpmdev xenstubdoms,id=xenvtpm0 -device
> tpm-tis,tpmdev=xenvtpm0"
> >
> > --Changes in v3:
> > -Call vtpm_send() and vtpm_recv() directly
> >
> > --Changes in v4:
> > -Fix the comment style
> >
> > Signed-off-by: Quan Xu <quan.xu@xxxxxxxxx>
> > ---
> >   hw/tpm/Makefile.objs     |   2 +-
> >   hw/tpm/tpm_xenstubdoms.c | 247
> +++++++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 248 insertions(+), 1 deletion(-)
> >   create mode 100644 hw/tpm/tpm_xenstubdoms.c
> >
> > diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index
> > 57919fa..190e776 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_PASSTHROUGH) += tpm_passthrough.o
> > -common-obj-$(CONFIG_TPM_XENSTUBDOMS) += xen_vtpm_frontend.o
> > +common-obj-$(CONFIG_TPM_XENSTUBDOMS) += tpm_xenstubdoms.o
> > +xen_vtpm_frontend.o
> > diff --git a/hw/tpm/tpm_xenstubdoms.c b/hw/tpm/tpm_xenstubdoms.c new
> > file mode 100644 index 0000000..6d0dc32
> > --- /dev/null
> > +++ b/hw/tpm/tpm_xenstubdoms.c
> > @@ -0,0 +1,247 @@
> > +/*
> > + * Xen Stubdom vTPM driver
> > + *
> > + *  Copyright (c) 2015 Intel Corporation
> > + *  Authors:
> > + *    Quan Xu <quan.xu@xxxxxxxxx>
> > + *
> > + * This library is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2 of the License, or (at your option) any later version.
> > + *
> > + * This library is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with this library; if not, see
> > +<http://www.gnu.org/licenses/>  */
> > +
> > +#include <dirent.h>
> > +#include "qemu-common.h"
> > +#include "qapi/error.h"
> > +#include "qemu/sockets.h"
> > +#include "qemu/log.h"
> > +#include "sysemu/tpm_backend.h"
> > +#include "tpm_int.h"
> > +#include "hw/hw.h"
> > +#include "hw/i386/pc.h"
> > +#include "hw/xen/xen_backend.h"
> > +#include "sysemu/tpm_backend_int.h"
> > +#include "tpm_tis.h"
> > +
> > +#ifdef DEBUG_TPM
> > +#define DPRINTF(fmt, ...) \
> > +    do { fprintf(stderr, fmt, ## __VA_ARGS__); } while (0) #else
> > +#define DPRINTF(fmt, ...) \
> > +    do { } while (0)
> > +#endif
> > +
> > +#define TYPE_TPM_XENSTUBDOMS "tpm-xenstubdoms"
> > +#define TPM_XENSTUBDOMS(obj) \
> > +    OBJECT_CHECK(TPMXenstubdomsState, (obj),
> TYPE_TPM_XENSTUBDOMS)
> > +
> > +static const TPMDriverOps tpm_xenstubdoms_driver;
> > +
> > +/* Data structures */
> > +typedef struct TPMXenstubdomsThreadParams {
> > +    TPMState *tpm_state;
> > +    TPMRecvDataCB *recv_data_callback;
> > +    TPMBackend *tb;
> > +} TPMXenstubdomsThreadParams;
> > +
> > +struct TPMXenstubdomsState {
> > +    TPMBackend parent;
> > +    TPMBackendThread tbt;
> > +    TPMXenstubdomsThreadParams tpm_thread_params;
> > +    bool had_startup_error;
> > +};
> > +
> > +typedef struct TPMXenstubdomsState TPMXenstubdomsState;
> > +
> > +/* Functions */
> > +static void tpm_xenstubdoms_cancel_cmd(TPMBackend *tb);
> > +
> > +static int tpm_xenstubdoms_unix_transfer(const TPMLocality
> > +*locty_data) {
> > +    size_t rlen;
> > +    struct XenDevice *xendev;
> > +
> > +    xendev = xen_find_xendev("vtpm", xen_domid, xenstore_dev);
> > +    if (xendev == NULL) {
> > +        xen_be_printf(xendev, 0, "Con not find vtpm device\n");
> Con not -> Cannot
> > +        return -1;
> > +    }
> > +    vtpm_send(xendev, locty_data->w_buffer.buffer,
> locty_data->w_offset);
> > +    vtpm_recv(xendev, locty_data->r_buffer.buffer, &rlen);
> > +    return 0;
> > +}
> > +
> > +static void tpm_xenstubdoms_worker_thread(gpointer data,
> > +                                          gpointer user_data) {
> > +    TPMXenstubdomsThreadParams *thr_parms = user_data;
> > +    TPMBackendCmd cmd = (TPMBackendCmd)data;
> > +
> > +    switch (cmd) {
> > +    case TPM_BACKEND_CMD_PROCESS_CMD:
> > +
> > +        /* here need a the cmd process function */
> > +
> tpm_xenstubdoms_unix_transfer(thr_parms->tpm_state->locty_data);
> > +        thr_parms->recv_data_callback(thr_parms->tpm_state,
> > +
> > + thr_parms->tpm_state->locty_number);
> 
> By now you'll need another parameter here indicating whether the command
> that was issues was a selftest and whether it completed successfully. It 
> should be
> easy to support this and you could move
> tpm_passthrough_is_selftest() into tpm_util.c and rename it to
> tpm_util_is_selftest(). But this should be done in a separate patch.
> 

Thanks Stefan.
One question, why should we highlight 'SelfTest' command? 
Similar to TPM passthrough Driver backen, worker thread is just for 
distributing process command.
As I know, the command is from guest virtual machine kernel's tpm_tis.ko, if 
'SelfTest' command
Is failed, the guest virtual machine's tpm_tis.ko will return directly. 




Quan 
> 
> > +        break;
> > +    case TPM_BACKEND_CMD_INIT:
> > +    case TPM_BACKEND_CMD_END:
> > +    case TPM_BACKEND_CMD_TPM_RESET:
> > +
> > +        /* nothing to do */
> > +        break;
> > +    }
> > +}
> > +
> > +/*
> > + *Start the TPM (thread). If it had been started before, then
> > +terminate and  *start it again.
> > + */
> > +static int tpm_xenstubdoms_startup_tpm(TPMBackend *tb) {
> > +    TPMXenstubdomsState *tpm_xs = TPM_XENSTUBDOMS(tb);
> > +
> > +    tpm_backend_thread_tpm_reset(&tpm_xs->tbt,
> tpm_xenstubdoms_worker_thread,
> > +                                 &tpm_xs->tpm_thread_params);
> > +
> > +    return 0;
> > +}
> > +
> > +static void tpm_xenstubdoms_reset(TPMBackend *tb) {
> > +    TPMXenstubdomsState *tpm_xs = TPM_XENSTUBDOMS(tb);
> > +
> > +    tpm_backend_thread_end(&tpm_xs->tbt);
> > +    tpm_xs->had_startup_error = false; }
> > +
> > +static int tpm_xenstubdoms_init(TPMBackend *tb, TPMState *s,
> > +                                TPMRecvDataCB *recv_data_cb) {
> > +    TPMXenstubdomsState *tpm_xs = TPM_XENSTUBDOMS(tb);
> > +
> > +    tpm_xs->tpm_thread_params.tpm_state = s;
> > +    tpm_xs->tpm_thread_params.recv_data_callback = recv_data_cb;
> > +    tpm_xs->tpm_thread_params.tb = tb;
> > +    return 0;
> > +}
> > +
> > +static bool tpm_xenstubdoms_get_tpm_established_flag(TPMBackend *tb)
> > +{
> > +    return false;
> > +}
> > +
> > +static bool tpm_xenstubdoms_get_startup_error(TPMBackend *tb) {
> > +    TPMXenstubdomsState *tpm_xs = TPM_XENSTUBDOMS(tb);
> > +
> > +    return tpm_xs->had_startup_error; }
> > +
> > +static size_t tpm_xenstubdoms_realloc_buffer(TPMSizedBuffer *sb) {
> > +    size_t wanted_size = 4096; /* Linux tpm.c buffer size */
> > +
> > +    if (sb->size != wanted_size) {
> > +        sb->buffer = g_realloc(sb->buffer, wanted_size);
> > +        sb->size = wanted_size;
> > +    }
> > +    return sb->size;
> > +}
> > +
> > +static void tpm_xenstubdoms_deliver_request(TPMBackend *tb) {
> > +    TPMXenstubdomsState *tpm_xs = TPM_XENSTUBDOMS(tb);
> > +
> > +    tpm_backend_thread_deliver_request(&tpm_xs->tbt);
> > +}
> > +
> > +static void tpm_xenstubdoms_cancel_cmd(TPMBackend *tb) { }
> > +
> > +static const char *tpm_xenstubdoms_create_desc(void)
> > +{
> > +    return "Xenstubdoms TPM backend driver"; }
> > +
> > +static TPMBackend *tpm_xenstubdoms_create(QemuOpts *opts, const char
> > +*id) {
> > +    Object *obj = object_new(TYPE_TPM_XENSTUBDOMS);
> > +    TPMBackend *tb = TPM_BACKEND(obj);
> > +
> > +    tb->id = g_strdup(id);
> > +    tb->fe_model = -1;
> > +    tb->ops = &tpm_xenstubdoms_driver;
> > +    return tb;
> > +}
> > +
> > +static void tpm_xenstubdoms_destroy(TPMBackend *tb) {
> > +    TPMXenstubdomsState *tpm_xh = TPM_XENSTUBDOMS(tb);
> Add empty line between var decls. and code.
> > +    tpm_backend_thread_end(&tpm_xh->tbt);
> > +
> > +    g_free(tb->id);
> > +}
> > +
> > +static const QemuOptDesc tpm_xenstubdoms_cmdline_opts[] = {
> > +    TPM_STANDARD_CMDLINE_OPTS,
> > +    {},
> > +};
> > +
> > +static const TPMDriverOps tpm_xenstubdoms_driver = {
> > +    .type                     = TPM_TYPE_XENSTUBDOMS,
> > +    .opts                     = tpm_xenstubdoms_cmdline_opts,
> > +    .desc                     = tpm_xenstubdoms_create_desc,
> > +    .create                   = tpm_xenstubdoms_create,
> > +    .destroy                  = tpm_xenstubdoms_destroy,
> > +    .init                     = tpm_xenstubdoms_init,
> > +    .startup_tpm              = tpm_xenstubdoms_startup_tpm,
> > +    .realloc_buffer           = tpm_xenstubdoms_realloc_buffer,
> > +    .reset                    = tpm_xenstubdoms_reset,
> > +    .had_startup_error        = tpm_xenstubdoms_get_startup_error,
> > +    .deliver_request          = tpm_xenstubdoms_deliver_request,
> > +    .cancel_cmd               = tpm_xenstubdoms_cancel_cmd,
> > +    .get_tpm_established_flag =
> > +tpm_xenstubdoms_get_tpm_established_flag,
> > +};
> > +
> > +static void tpm_xenstubdoms_inst_init(Object *obj) { }
> > +
> > +static void tpm_xenstubdoms_inst_finalize(Object *obj) { }
> > +
> > +static void tpm_xenstubdoms_class_init(ObjectClass *klass, void
> > +*data) {
> > +    TPMBackendClass *tbc = TPM_BACKEND_CLASS(klass);
> Add empty line here
> > +    tbc->ops = &tpm_xenstubdoms_driver; }
> > +
> > +static const TypeInfo tpm_xenstubdoms_info = {
> > +    .name = TYPE_TPM_XENSTUBDOMS,
> > +    .parent = TYPE_TPM_BACKEND,
> > +    .instance_size = sizeof(TPMXenstubdomsState),
> > +    .class_init = tpm_xenstubdoms_class_init,
> > +    .instance_init = tpm_xenstubdoms_inst_init,
> > +    .instance_finalize = tpm_xenstubdoms_inst_finalize, };
> > +
> > +static void tpm_xenstubdoms_register(void) {
> > +    type_register_static(&tpm_xenstubdoms_info);
> > +    tpm_register_driver(&tpm_xenstubdoms_driver);
> > +}
> > +
> > +type_init(tpm_xenstubdoms_register)
> 
> Otherwise looks good.
>     Stefan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel