qemu-arm
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC v2 5/6] tpm: Add the SysBus TPM TIS device


From: Philippe Mathieu-Daudé
Subject: Re: [RFC v2 5/6] tpm: Add the SysBus TPM TIS device
Date: Tue, 25 Feb 2020 11:18:45 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/25/20 10:52 AM, Ard Biesheuvel wrote:
On Tue, 25 Feb 2020 at 10:19, Auger Eric <address@hidden> wrote:

Hi,

On 2/17/20 7:13 PM, Auger Eric wrote:
Hi Stefan,

On 2/16/20 7:32 PM, Stefan Berger wrote:
On 2/14/20 1:37 PM, Eric Auger wrote:
Introduce the tpm-tis-device which is a sysbus device
and is bound to be used on ARM.

Signed-off-by: Eric Auger <address@hidden>
---
   hw/tpm/Kconfig          |   5 ++
   hw/tpm/Makefile.objs    |   1 +
   hw/tpm/tpm_tis_sysbus.c | 159 ++++++++++++++++++++++++++++++++++++++++
   include/sysemu/tpm.h    |   1 +
   4 files changed, 166 insertions(+)
   create mode 100644 hw/tpm/tpm_tis_sysbus.c

diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 686f8206bb..4794e7fe28 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -7,6 +7,11 @@ config TPM_TIS_ISA
       depends on TPM && ISA_BUS
       select TPM_TIS
   +config TPM_TIS_SYSBUS
+    bool
+    depends on TPM
+    select TPM_TIS
+
   config TPM_TIS
       bool
       depends on TPM
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index 3ef2036cca..f1ec4beb95 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,6 +1,7 @@
   common-obj-$(CONFIG_TPM) += tpm_util.o
   obj-$(call lor,$(CONFIG_TPM_TIS),$(CONFIG_TPM_CRB)) += tpm_ppi.o
   common-obj-$(CONFIG_TPM_TIS_ISA) += tpm_tis_isa.o
+common-obj-$(CONFIG_TPM_TIS_SYSBUS) += tpm_tis_sysbus.o
   common-obj-$(CONFIG_TPM_TIS) += tpm_tis_common.o
   common-obj-$(CONFIG_TPM_CRB) += tpm_crb.o
   common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
diff --git a/hw/tpm/tpm_tis_sysbus.c b/hw/tpm/tpm_tis_sysbus.c
new file mode 100644
index 0000000000..18c02aed67
--- /dev/null
+++ b/hw/tpm/tpm_tis_sysbus.c
@@ -0,0 +1,159 @@
+/*
+ * tpm_tis_sysbus.c - QEMU's TPM TIS SYSBUS Device
+ *
+ * Copyright (C) 2006,2010-2013 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger <address@hidden>
+ *  David Safford <address@hidden>
+ *
+ * Xen 4 support: Andrease Niederl <address@hidden>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
later.
+ * See the COPYING file in the top-level directory.
+ *
+ * Implementation of the TIS interface according to specs found at
+ * http://www.trustedcomputinggroup.org. This implementation currently
+ * supports version 1.3, 21 March 2013
+ * In the developers menu choose the PC Client section then find the TIS
+ * specification.
+ *
+ * TPM TIS for TPM 2 implementation following TCG PC Client Platform
+ * TPM Profile (PTP) Specification, Familiy 2.0, Revision 00.43
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
+#include "tpm_util.h"
+#include "hw/sysbus.h"
+#include "tpm_tis.h"
+
+typedef struct TPMStateSysBus {
+    /*< private >*/
+    SysBusDevice parent_obj;
+
+    /*< public >*/
+    TPMState state; /* not a QOM object */
+} TPMStateSysBus;
+
+#define TPM_TIS_SYSBUS(obj) OBJECT_CHECK(TPMStateSysBus, (obj),
TYPE_TPM_TIS_SYSBUS)
+
+static int tpm_tis_pre_save_sysbus(void *opaque)
+{
+    TPMStateSysBus *sbdev = opaque;
+
+    return tpm_tis_pre_save(&sbdev->state);
+}
+
+static const VMStateDescription vmstate_tpm_tis_sysbus = {
+    .name = "tpm-tis",
+    .version_id = 0,
+    .pre_save  = tpm_tis_pre_save_sysbus,
+    .fields = (VMStateField[]) {
+        VMSTATE_BUFFER(state.buffer, TPMStateSysBus),
+        VMSTATE_UINT16(state.rw_offset, TPMStateSysBus),
+        VMSTATE_UINT8(state.active_locty, TPMStateSysBus),
+        VMSTATE_UINT8(state.aborting_locty, TPMStateSysBus),
+        VMSTATE_UINT8(state.next_locty, TPMStateSysBus),
+
+        VMSTATE_STRUCT_ARRAY(state.loc, TPMStateSysBus,
TPM_TIS_NUM_LOCALITIES,
+                             0, vmstate_locty, TPMLocality),
+
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void tpm_tis_sysbus_request_completed(TPMIf *ti, int ret)
+{
+    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
+    TPMState *s = &sbdev->state;
+
+    tpm_tis_request_completed(s, ret);
+}
+
+static enum TPMVersion tpm_tis_sysbus_get_tpm_version(TPMIf *ti)
+{
+    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(ti);
+    TPMState *s = &sbdev->state;
+
+    return tpm_tis_get_tpm_version(s);
+}
+
+static void tpm_tis_sysbus_reset(DeviceState *dev)
+{
+    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
+    TPMState *s = &sbdev->state;
+
+    return tpm_tis_reset(s);
+}
+
+static Property tpm_tis_sysbus_properties[] = {
+    DEFINE_PROP_UINT32("irq", TPMStateSysBus, state.irq_num,
TPM_TIS_IRQ),
+    DEFINE_PROP_TPMBE("tpmdev", TPMStateSysBus, state.be_driver),
+    DEFINE_PROP_BOOL("ppi", TPMStateSysBus, state.ppi_enabled, true),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void tpm_tis_sysbus_initfn(Object *obj)
+{
+    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(obj);
+    TPMState *s = &sbdev->state;
+
+    memory_region_init_io(&s->mmio, obj, &tpm_tis_memory_ops,
+                          s, "tpm-tis-mmio",
+                          TPM_TIS_NUM_LOCALITIES <<
TPM_TIS_LOCALITY_SHIFT);
+
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+    sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
+}
+
+static void tpm_tis_sysbus_realizefn(DeviceState *dev, Error **errp)
+{
+    TPMStateSysBus *sbdev = TPM_TIS_SYSBUS(dev);
+    TPMState *s = &sbdev->state;
+
+    if (!tpm_find()) {
+        error_setg(errp, "at most one TPM device is permitted");
+        return;
+    }
+
+    if (!s->be_driver) {
+        error_setg(errp, "'tpmdev' property is required");
+        return;
+    }
+}
+
+static void tpm_tis_sysbus_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    TPMIfClass *tc = TPM_IF_CLASS(klass);
+
+    device_class_set_props(dc, tpm_tis_sysbus_properties);
+    dc->vmsd  = &vmstate_tpm_tis_sysbus;
+    tc->model = TPM_MODEL_TPM_TIS;
+    dc->realize = tpm_tis_sysbus_realizefn;
+    dc->user_creatable = true;
+    dc->reset = tpm_tis_sysbus_reset;
+    tc->request_completed = tpm_tis_sysbus_request_completed;
+    tc->get_version = tpm_tis_sysbus_get_tpm_version;
+}
+
+static const TypeInfo tpm_tis_sysbus_info = {
+    .name = TYPE_TPM_TIS_SYSBUS,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(TPMStateSysBus),
+    .instance_init = tpm_tis_sysbus_initfn,
+    .class_init  = tpm_tis_sysbus_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_TPM_IF },
+        { }
+    }
+};
+
+static void tpm_tis_sysbus_register(void)
+{
+    type_register_static(&tpm_tis_sysbus_info);
+}
+
+type_init(tpm_tis_sysbus_register)
diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h
index 1691b92c28..f37851b1aa 100644
--- a/include/sysemu/tpm.h
+++ b/include/sysemu/tpm.h
@@ -44,6 +44,7 @@ typedef struct TPMIfClass {
   } TPMIfClass;
     #define TYPE_TPM_TIS_ISA            "tpm-tis"
+#define TYPE_TPM_TIS_SYSBUS         "tpm-tis-device"


hm, replace the rather generic 'device' with 'sysbus'?
I used the "-device" suffix because this kind of naming was used for
virtio-<type>-device when based on MMIO rather than virtio-<type>-pci.
For instance virtio-net-device instead of virtio-net-pci. There are
quite a lot of devices using that suffix. I only see xen-sysbus with the
sysbus suffix.

Now personally I don't have any strong preference and I will pick up the
name chosen by consensus.

Does anyone else have an opinion on the name to be chosen for this new
device:

1) tpm-tis-device or
2) tpm-tis-sysbus ?


It is slightly unfortunate that we cannot retain the 'tpm-tis' name,
given that this is simply a TPM with a memory mapped TIS frame, like
the ISA one, and the fact that QEMU instantiates this differently
based on the emulated architecture is really an implementation detail.

Agreed.

As long as we don't change vmstate_tpm_tis::name, we can rename the QOM type names.

So I'd rather rename ISA as:

  #define TYPE_TPM_TIS_ISA            "tpm-tis-isa"

And the generic name for the sysbus/mmio device:

  #define TYPE_TPM_TIS_SYSBUS         "tpm-tis"

If we add the common ancestor, we can name it "tpm-tis-common".

So I prefer 'tpm-tis-device', since it doesn't define how it is backed
under the hood, and allows us to potentially instantiate it in a
different way in the future if we wanted to. Alternatively, something
like 'tpm-tis-mmmio' might be appropriate.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]