[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v12 3/6] tpm: allocate/map buffer for TPM Physic
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v12 3/6] tpm: allocate/map buffer for TPM Physical Presence interface |
Date: |
Thu, 13 Dec 2018 00:22:18 +0400 |
Hi
On Wed, Dec 12, 2018 at 8:13 PM Philippe Mathieu-Daudé
<address@hidden> wrote:
>
> Hi Stefan, Marc-André,
>
> On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> > From: Stefan Berger <address@hidden>
> >
> > Implement a virtual memory device for the TPM Physical Presence interface.
> > The memory is located at 0xFED45000 and used by ACPI to send messages to the
> > firmware (BIOS) and by the firmware to provide parameters for each one of
> > the supported codes.
> >
> > This interface should be used by all TPM devices on x86 and can be
> > added by calling tpm_ppi_init_io().
> >
> > Note: bios_linker cannot be used to allocate the PPI memory region,
> > since the reserved memory should stay stable across reboots, and might
> > be needed before the ACPI tables are installed.
> >
> > Signed-off-by: Stefan Berger <address@hidden>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > Reviewed-by: Igor Mammedov <address@hidden>
> > ---
> > hw/tpm/tpm_ppi.h | 26 ++++++++++++++++++++++++++
> > include/hw/acpi/tpm.h | 6 ++++++
> > hw/tpm/tpm_crb.c | 8 ++++++++
> > hw/tpm/tpm_ppi.c | 32 ++++++++++++++++++++++++++++++++
> > hw/tpm/tpm_tis.c | 8 ++++++++
> > hw/tpm/Makefile.objs | 1 +
> > 6 files changed, 81 insertions(+)
> > create mode 100644 hw/tpm/tpm_ppi.h
> > create mode 100644 hw/tpm/tpm_ppi.c
> >
> > diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> > new file mode 100644
> > index 0000000000..c2ab2ed300
> > --- /dev/null
> > +++ b/hw/tpm/tpm_ppi.h
> > @@ -0,0 +1,26 @@
> > +/*
> > + * TPM Physical Presence Interface
> > + *
> > + * Copyright (C) 2018 IBM Corporation
> > + *
> > + * Authors:
> > + * Stefan Berger <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.
> > + */
> > +#ifndef TPM_TPM_PPI_H
> > +#define TPM_TPM_PPI_H
> > +
> > +#include "hw/acpi/tpm.h"
> > +#include "exec/address-spaces.h"
> > +
> > +typedef struct TPMPPI {
> > + MemoryRegion ram;
> > + uint8_t *buf;
> > +} TPMPPI;
> > +
>
> Can you add a description for this public function?
ok
>
> > +bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > + hwaddr addr, Object *obj, Error **errp);
> > +
> > +#endif /* TPM_TPM_PPI_H */
> > diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> > index 3580ffd50c..b8796df916 100644
> > --- a/include/hw/acpi/tpm.h
> > +++ b/include/hw/acpi/tpm.h
> > @@ -188,4 +188,10 @@ REG32(CRB_DATA_BUFFER, 0x80)
> > #define TPM2_START_METHOD_MMIO 6
> > #define TPM2_START_METHOD_CRB 7
> >
> > +/*
> > + * Physical Presence Interface
> > + */
> > +#define TPM_PPI_ADDR_SIZE 0x400
> > +#define TPM_PPI_ADDR_BASE 0xFED45000
> > +
> > #endif /* HW_ACPI_TPM_H */
> > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > index d5b0ac5920..b243222fd6 100644
> > --- a/hw/tpm/tpm_crb.c
> > +++ b/hw/tpm/tpm_crb.c
> > @@ -29,6 +29,7 @@
> > #include "sysemu/reset.h"
> > #include "tpm_int.h"
> > #include "tpm_util.h"
> > +#include "tpm_ppi.h"
> > #include "trace.h"
> >
> > typedef struct CRBState {
> > @@ -43,6 +44,7 @@ typedef struct CRBState {
> > size_t be_buffer_size;
> >
> > bool ppi_enabled;
> > + TPMPPI ppi;
> > } CRBState;
> >
> > #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
> > @@ -294,6 +296,12 @@ static void tpm_crb_realize(DeviceState *dev, Error
> > **errp)
> > memory_region_add_subregion(get_system_memory(),
> > TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
> >
> > + if (s->ppi_enabled &&
> > + !tpm_ppi_init(&s->ppi, get_system_memory(),
> > + TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
> > + return;
> > + }
> > +
> > qemu_register_reset(tpm_crb_reset, dev);
> > }
> >
> > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > new file mode 100644
> > index 0000000000..f2f07f895e
> > --- /dev/null
> > +++ b/hw/tpm/tpm_ppi.c
> > @@ -0,0 +1,32 @@
> > +/*
> > + * tpm_ppi.c - TPM Physical Presence Interface
> > + *
> > + * Copyright (C) 2018 IBM Corporation
> > + *
> > + * Authors:
> > + * Stefan Berger <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.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "qapi/error.h"
> > +#include "cpu.h"
> > +#include "sysemu/memory_mapping.h"
> > +#include "migration/vmstate.h"
> > +#include "tpm_ppi.h"
> > +
> > +bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > + hwaddr addr, Object *obj, Error **errp)
> > +{
> > + tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> > + memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> > + TPM_PPI_ADDR_SIZE, tpmppi->buf);
> > + vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
> > +
> > + memory_region_add_subregion(m, addr, &tpmppi->ram);
> > + return true;
> > +}
> > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > index d9ddf9b723..70432ffe8b 100644
> > --- a/hw/tpm/tpm_tis.c
> > +++ b/hw/tpm/tpm_tis.c
> > @@ -31,6 +31,7 @@
> > #include "sysemu/tpm_backend.h"
> > #include "tpm_int.h"
> > #include "tpm_util.h"
> > +#include "tpm_ppi.h"
> > #include "trace.h"
> >
> > #define TPM_TIS_NUM_LOCALITIES 5 /* per spec */
> > @@ -83,6 +84,7 @@ typedef struct TPMState {
> > size_t be_buffer_size;
> >
> > bool ppi_enabled;
> > + TPMPPI ppi;
> > } TPMState;
> >
> > #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
> > @@ -979,6 +981,12 @@ static void tpm_tis_realizefn(DeviceState *dev, Error
> > **errp)
> >
> > memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
> > TPM_TIS_ADDR_BASE, &s->mmio);
> > +
> > + if (s->ppi_enabled &&
> > + !tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
> > + TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
> > + return;
> > + }
> > }
> >
>
> Shouldn't you implement the unrealize() function, destroying the PPI buffer?
I don't think it's necessary since the TPM and PPI are not hotpluggable.
> Patch looks good otherwise.
>
> > static void tpm_tis_initfn(Object *obj)
> > diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
> > index 1dc9f8bf2c..700c878622 100644
> > --- a/hw/tpm/Makefile.objs
> > +++ b/hw/tpm/Makefile.objs
> > @@ -1,4 +1,5 @@
> > common-obj-y += tpm_util.o
> > +obj-y += tpm_ppi.o
> > common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
> > common-obj-$(CONFIG_TPM_CRB) += tpm_crb.o
> > common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
> >
>
--
Marc-André Lureau