Re: [Qemu-devel] [PATCH v2 26/28] qdev: add DEFINE_PROP_TPMBE

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

On 11/06/2017 01:39 PM, Marc-André Lureau wrote:
> A property to lookup a tpm backend.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   include/hw/qdev-properties.h     |  3 ++
>   hw/core/qdev-properties-system.c | 64 ++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 67 insertions(+)
>
> diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
> index e2321f1cc1..4d24cdf8d6 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -17,6 +17,7 @@ extern const PropertyInfo qdev_prop_int64;
>   extern const PropertyInfo qdev_prop_size;
>   extern const PropertyInfo qdev_prop_string;
>   extern const PropertyInfo qdev_prop_chr;
> +extern const PropertyInfo qdev_prop_tpm;
>   extern const PropertyInfo qdev_prop_ptr;
>   extern const PropertyInfo qdev_prop_macaddr;
>   extern const PropertyInfo qdev_prop_on_off_auto;
> @@ -186,6 +187,8 @@ extern const PropertyInfo qdev_prop_link;
>
>   #define DEFINE_PROP_CHR(_n, _s, _f)             \
>       DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharBackend)
> +#define DEFINE_PROP_TPMBE(_n, _s, _f)                     \
> +    DEFINE_PROP(_n, _s, _f, qdev_prop_tpm, TPMBackend *)
>   #define DEFINE_PROP_STRING(_n, _s, _f)             \
>       DEFINE_PROP(_n, _s, _f, qdev_prop_string, char*)
>   #define DEFINE_PROP_NETDEV(_n, _s, _f)             \
> diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
> index ec10da7424..c17364655c 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -21,6 +21,7 @@
>   #include "net/hub.h"
>   #include "qapi/visitor.h"
>   #include "chardev/char-fe.h"
> +#include "sysemu/tpm_backend.h"
>   #include "sysemu/iothread.h"
>
>   static void get_pointer(Object *obj, Visitor *v, Property *prop,
> @@ -236,6 +237,69 @@ const PropertyInfo qdev_prop_chr = {
>       .release = release_chr,
>   };
>
> +/* --- character device --- */
> +
> +static void get_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
> +                    Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    TPMBackend **be = qdev_get_prop_ptr(dev, opaque);
> +    char *p;
> +
> +    p = g_strdup(*be ? (*be)->id : "");
> +    visit_type_str(v, name, &p, errp);
> +    g_free(p);
> +}
> +
> +static void set_tpm(Object *obj, Visitor *v, const char *name, void *opaque,
> +                    Error **errp)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Error *local_err = NULL;
> +    Property *prop = opaque;
> +    TPMBackend *s, **be = qdev_get_prop_ptr(dev, prop);
> +    char *str;
> +
> +    if (dev->realized) {
> +        qdev_prop_set_after_realize(dev, name, errp);
> +        return;
> +    }
> +
> +    visit_type_str(v, name, &str, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    s = qemu_find_tpm_be(str);
> +    if (s == NULL) {
> +        error_setg(errp, "Property '%s.%s' can't find value '%s'",
> +                   object_get_typename(obj), prop->name, str);
> +    } else if (tpm_backend_init(s, TPM_IF(obj), errp) == 0) {
> +        *be = s; /* weak reference, avoid cyclic ref */

I suppose this is the reason why you need **be rather than *be, but why 
do you need this here?

Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com>

     Stefan

> +    }
> +    g_free(str);
> +}
> +
> +static void release_tpm(Object *obj, const char *name, void *opaque)
> +{
> +    DeviceState *dev = DEVICE(obj);
> +    Property *prop = opaque;
> +    TPMBackend **be = qdev_get_prop_ptr(dev, prop);
> +
> +    if (*be) {
> +        tpm_backend_reset(*be);
> +    }
> +}
> +
> +const PropertyInfo qdev_prop_tpm = {
> +    .name  = "str",
> +    .description = "ID of a tpm to use as a backend",
> +    .get   = get_tpm,
> +    .set   = set_tpm,
> +    .release = release_tpm,
> +};
> +
>   /* --- netdev device --- */
>   static void get_netdev(Object *obj, Visitor *v, const char *name,
>                          void *opaque, Error **errp)