[Top][All Lists]

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

Re: [RFC 1/2] tpm: Let the TPM TIS device be usable on ARM

From: Philippe Mathieu-Daudé
Subject: Re: [RFC 1/2] tpm: Let the TPM TIS device be usable on ARM
Date: Tue, 11 Feb 2020 09:25:01 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/10/20 2:15 PM, Eric Auger wrote:
Implement support for TPM on aarch64 by using the
TPM TIS MMIO frontend. Instead of being an ISA device,
the TPM TIS device becomes a sysbus device on ARM. It is
bound to be dynamically instantiated.

Signed-off-by: Eric Auger <address@hidden>


I am aware such kind of #ifde'fy is frown upon but this is just
for starting the discussion

I doubt this can be accepted upstream, because a target has to choose between using sysbus OR isa devices, not both.

  hw/tpm/Kconfig   |  2 +-
  hw/tpm/tpm_tis.c | 16 ++++++++++++++++
  2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 9e67d990e8..326c89e6df 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -4,7 +4,7 @@ config TPMDEV
config TPM_TIS
-    depends on TPM && ISA_BUS
+    depends on TPM
      select TPMDEV
config TPM_CRB
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 31facb896d..cfc840942f 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -30,6 +30,7 @@
#include "hw/acpi/tpm.h"
  #include "hw/pci/pci_ids.h"
+#include "hw/sysbus.h"
  #include "hw/qdev-properties.h"
  #include "migration/vmstate.h"
  #include "sysemu/tpm_backend.h"
@@ -65,7 +66,11 @@ typedef struct TPMLocality {
  } TPMLocality;
typedef struct TPMState {
      ISADevice busdev;
+    SysBusDevice busdev;
      MemoryRegion mmio;
unsigned char buffer[TPM_TIS_BUFFER_MAX];
@@ -967,6 +972,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error 
          error_setg(errp, "'tpmdev' property is required");
      if (s->irq_num > 15) {
          error_setg(errp, "IRQ %d is outside valid range of 0 to 15",
@@ -982,6 +988,7 @@ static void tpm_tis_realizefn(DeviceState *dev, Error 
          tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
                       TPM_PPI_ADDR_BASE, OBJECT(s));
static void tpm_tis_initfn(Object *obj)
@@ -991,6 +998,10 @@ static void tpm_tis_initfn(Object *obj)
      memory_region_init_io(&s->mmio, OBJECT(s), &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_class_init(ObjectClass *klass, void *data)
@@ -1002,6 +1013,7 @@ static void tpm_tis_class_init(ObjectClass *klass, void 
      device_class_set_props(dc, tpm_tis_properties);
      dc->reset = tpm_tis_reset;
      dc->vmsd  = &vmstate_tpm_tis;

With your #ifde'fy in mind, you probably need to restrict this to sysbus:

  #ifndef CONFIG_ISA_BUS

+    dc->user_creatable = true;


      tc->model = TPM_MODEL_TPM_TIS;
      tc->get_version = tpm_tis_get_tpm_version;
      tc->request_completed = tpm_tis_request_completed;
@@ -1009,7 +1021,11 @@ static void tpm_tis_class_init(ObjectClass *klass, void 
static const TypeInfo tpm_tis_info = {
      .name = TYPE_TPM_TIS,
      .parent = TYPE_ISA_DEVICE,
+    .parent = TYPE_SYS_BUS_DEVICE,
      .instance_size = sizeof(TPMState),
      .instance_init = tpm_tis_initfn,
      .class_init  = tpm_tis_class_init,

From the sysbus device PoV the patch looks OK.

You don't need much to remove the RFC tag:

- rename TPMState as TPMCommonState, add an abstract TYPE_TPM_TIS parent, let TYPE_TPM_TIS_ISA be a child
- add TYPE_TPM_TIS_SYSBUS also child.

reply via email to

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