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

[Marc-André Lureau]

On Mon, Nov 6, 2017 at 9:31 PM, Stefan Berger
<address@hidden> wrote:
> On 11/06/2017 01:39 PM, Marc-André Lureau wrote:
>>
>> A property to lookup a tpm backend.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>>   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?
>

**be is because we have a pointer to the pointer property. This is
similar to PROP_PTR for example.

tpm_backend_init() takes a refrence to the TpmIf, so we only take a
weak reference to the backend. Otherwise we would need extra calls to
break the reference loop. (I am not convinced this is the best design,
we would need to think about destruction order if there is an issue
left)

thanks

> Reviewed-by: Stefan Berger <address@hidden>
>
>     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)
>
>
>
>



-- 
Marc-André Lureau