[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/4] tpm: implement virtual memory device for
|
From: |
Marc-André Lureau |
|
Subject: |
Re: [Qemu-devel] [PATCH v3 1/4] tpm: implement virtual memory device for TPM PPI |
|
Date: |
Thu, 21 Jun 2018 12:51:34 +0200 |
Hi
On Thu, Jun 21, 2018 at 11:49 AM, Igor Mammedov <address@hidden> wrote:
> On Tue, 15 May 2018 14:14:30 +0200
> Marc-André Lureau <address@hidden> wrote:
>
>> From: Stefan Berger <address@hidden>
>>
>> Implement a virtual memory device for the TPM Physical Presence interface.
>> The memory is located at 0xfffef000 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 device should be used by all TPM interfaces on x86 and can be added
>> by calling tpm_ppi_init_io().
>>
>> Signed-off-by: Stefan Berger <address@hidden>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>>
>> ---
>>
>> v3 (Marc-André):
>> - merge CRB support
>> - use trace events instead of DEBUG printf
>> - headers inclusion simplification
>>
>> v2:
>> - moved to byte access since an infrequently used device;
>> this simplifies code
>> - increase size of device to 0x400
>> - move device to 0xfffef000 since SeaBIOS has some code at 0xffff0000:
>> 'On the emulators, the bios at 0xf0000 is also at 0xffff0000'
>> ---
>> hw/tpm/tpm_ppi.h | 26 ++++++++++++++++++++
>> include/hw/acpi/tpm.h | 6 +++++
>> hw/tpm/tpm_crb.c | 5 ++++
>> hw/tpm/tpm_ppi.c | 56 +++++++++++++++++++++++++++++++++++++++++++
>> hw/tpm/tpm_tis.c | 5 ++++
>> hw/tpm/Makefile.objs | 2 +-
>> hw/tpm/trace-events | 4 ++++
>> 7 files changed, 103 insertions(+), 1 deletion(-)
>> 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..17030bd989
>> --- /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 mmio;
>> +
>> + uint8_t ram[TPM_PPI_ADDR_SIZE];
>> +} TPMPPI;
>> +
>> +void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m, Object *obj);
>> +
>> +#endif /* TPM_TPM_PPI_H */
>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
>> index 46ac4dc581..c082df7d1d 100644
>> --- a/include/hw/acpi/tpm.h
>> +++ b/include/hw/acpi/tpm.h
>> @@ -187,4 +187,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 a92dd50437..4f585564d9 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 {
>> @@ -41,6 +42,8 @@ typedef struct CRBState {
>> MemoryRegion cmdmem;
>>
>> size_t be_buffer_size;
>> +
>> + TPMPPI ppi;
>> } CRBState;
>>
>> #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
>> @@ -291,6 +294,8 @@ 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);
>>
>> + tpm_ppi_init_io(&s->ppi, get_system_memory(), OBJECT(s));
>> +
>> 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..0019c3e6fc
>> --- /dev/null
>> +++ b/hw/tpm/tpm_ppi.c
>> @@ -0,0 +1,56 @@
>> +/*
>> + * 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 "tpm_ppi.h"
>> +#include "trace.h"
>> +
>> +static uint64_t tpm_ppi_mmio_read(void *opaque, hwaddr addr,
>> + unsigned size)
>> +{
>> + TPMPPI *s = opaque;
>> +
>> + trace_tpm_ppi_mmio_read(addr, size, s->ram[addr]);
>> +
>> + return s->ram[addr];
>> +}
>> +
>> +static void tpm_ppi_mmio_write(void *opaque, hwaddr addr,
>> + uint64_t val, unsigned size)
>> +{
>> + TPMPPI *s = opaque;
>> +
>> + trace_tpm_ppi_mmio_write(addr, size, val);
>> +
>> + s->ram[addr] = val;
>> +}
>> +
>> +static const MemoryRegionOps tpm_ppi_memory_ops = {
>> + .read = tpm_ppi_mmio_read,
>> + .write = tpm_ppi_mmio_write,
>> + .endianness = DEVICE_NATIVE_ENDIAN,
>> + .valid = {
>> + .min_access_size = 1,
>> + .max_access_size = 1,
>> + },
>> +};
>> +
>> +void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m, Object *obj)
>> +{
>> + memory_region_init_io(&tpmppi->mmio, obj, &tpm_ppi_memory_ops,
>> + tpmppi, "tpm-ppi-mmio",
>> + TPM_PPI_ADDR_SIZE);
>> +
>> + memory_region_add_subregion(m, TPM_PPI_ADDR_BASE, &tpmppi->mmio);
> I'd push TPM_PPI_ADDR_BASE up to the stack so it would be obvious
> at tpm_ppi_init_io() call site wrt which region the offset is applied
>
> also maybe s/TPM_PPI_ADDR_BASE/TPM_PPI_ADDR_OFFSET/?
Hmm, it's a fixed address, and we already use TPM_TIS_ADDR_BASE /
TPM_CRB_ADDR_BASE.
>
>> +}
>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>> index 12f5c9a759..9a2fec455a 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 */
>> @@ -81,6 +82,8 @@ typedef struct TPMState {
>> TPMVersion be_tpm_version;
>>
>> size_t be_buffer_size;
>> +
>> + TPMPPI ppi;
>> } TPMState;
>>
>> #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
>> @@ -976,6 +979,8 @@ 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);
>> +
>> + tpm_ppi_init_io(&s->ppi, isa_address_space(ISA_DEVICE(dev)), OBJECT(s));
> what address space it would get here and at what offset address space is
> mapped?
Agree, that's a bit better, done
thanks
>
>> }
>>
>> static void tpm_tis_initfn(Object *obj)
>> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
>> index 1dc9f8bf2c..eedd8b6858 100644
>> --- a/hw/tpm/Makefile.objs
>> +++ b/hw/tpm/Makefile.objs
>> @@ -1,4 +1,4 @@
>> -common-obj-y += tpm_util.o
>> +common-obj-y += tpm_util.o 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
>> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
>> index 25bee0cecf..81f9923401 100644
>> --- a/hw/tpm/trace-events
>> +++ b/hw/tpm/trace-events
>> @@ -8,6 +8,10 @@ tpm_crb_mmio_write(uint64_t addr, unsigned size, uint32_t
>> val) "CRB write 0x" TA
>> tpm_passthrough_handle_request(void *cmd) "processing command %p"
>> tpm_passthrough_reset(void) "reset"
>>
>> +# hw/tpm/tpm_ppi.c
>> +tpm_ppi_mmio_read(uint64_t addr, unsigned size, uint32_t val) "PPI read 0x"
>> TARGET_FMT_plx " len:%u val: 0x%" PRIx32
>> +tpm_ppi_mmio_write(uint64_t addr, unsigned size, uint32_t val) "PPI write
>> 0x" TARGET_FMT_plx " len:%u val: 0x%" PRIx32
>> +
>> # hw/tpm/tpm_util.c
>> tpm_util_get_buffer_size_hdr_len(uint32_t len, size_t expected)
>> "tpm_resp->hdr.len = %u, expected = %zu"
>> tpm_util_get_buffer_size_len(uint32_t len, size_t expected) "tpm_resp->len
>> = %u, expected = %zu"
>
>
--
Marc-André Lureau