| Message ID | 20180119141105.29095-5-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: CRB device and cleanups | expand |
On 01/19/2018 09:11 AM, Marc-André Lureau wrote: > tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) > Interface as defined in TCG PC Client Platform TPM Profile (PTP) > Specification Family “2.0” Level 00 Revision 01.03 v22. > > The PTP allows device implementation to switch between TIS and CRB > model at run time, but given that CRB is a simpler device to > implement, I chose to implement it as a different device. > > The device doesn't implement other locality than 0 for now (my laptop > TPM doesn't either, so I assume this isn't so bad) > > The command/reply memory region is statically allocated after the CRB > registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I > wonder if the BIOS could or should allocate it instead, or what size > to use, again this seems to fit well expectations) I removed this last sentence now. It's at the right location. > > The PTP doesn't specify a particular bus to put the device. So I added > it on the system bus directly, so it could hopefully be used easily on > a different platform than x86. Currently, it fails to init on piix, > because error_on_sysbus_device() check. The check may be changed in a > near future, see discussion on the qemu-devel ML. I think this has to be solved. So I remove these last 2 sentences. I'll have to wait until that other patch series from Eduard is merged since it doesn't start yet. Stefan
On Fri, Jan 19, 2018 at 12:10:03PM -0500, Stefan Berger wrote: > On 01/19/2018 09:11 AM, Marc-André Lureau wrote: > > tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) > > Interface as defined in TCG PC Client Platform TPM Profile (PTP) > > Specification Family “2.0” Level 00 Revision 01.03 v22. > > > > The PTP allows device implementation to switch between TIS and CRB > > model at run time, but given that CRB is a simpler device to > > implement, I chose to implement it as a different device. > > > > The device doesn't implement other locality than 0 for now (my laptop > > TPM doesn't either, so I assume this isn't so bad) > > > > The command/reply memory region is statically allocated after the CRB > > registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I > > wonder if the BIOS could or should allocate it instead, or what size > > to use, again this seems to fit well expectations) > > I removed this last sentence now. It's at the right location. > > > > > The PTP doesn't specify a particular bus to put the device. So I added > > it on the system bus directly, so it could hopefully be used easily on > > a different platform than x86. Currently, it fails to init on piix, > > because error_on_sysbus_device() check. The check may be changed in a > > near future, see discussion on the qemu-devel ML. > > I think this has to be solved. So I remove these last 2 sentences. I'll have > to wait until that other patch series from Eduard is merged since it doesn't > start yet. The series was just merged to master. It's possible to make a machine accept the new device using machine_class_allow_dynamic_sysbus_dev(), now. However, is it really necessary to make it a sysbus device? Having bus-less devices was not possible in the past, but it is possible today.
On 01/19/2018 01:42 PM, Eduardo Habkost wrote: > On Fri, Jan 19, 2018 at 12:10:03PM -0500, Stefan Berger wrote: >> On 01/19/2018 09:11 AM, Marc-André Lureau wrote: >>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >>> Specification Family “2.0” Level 00 Revision 01.03 v22. >>> >>> The PTP allows device implementation to switch between TIS and CRB >>> model at run time, but given that CRB is a simpler device to >>> implement, I chose to implement it as a different device. >>> >>> The device doesn't implement other locality than 0 for now (my laptop >>> TPM doesn't either, so I assume this isn't so bad) >>> >>> The command/reply memory region is statically allocated after the CRB >>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >>> wonder if the BIOS could or should allocate it instead, or what size >>> to use, again this seems to fit well expectations) >> I removed this last sentence now. It's at the right location. >> >>> The PTP doesn't specify a particular bus to put the device. So I added >>> it on the system bus directly, so it could hopefully be used easily on >>> a different platform than x86. Currently, it fails to init on piix, >>> because error_on_sysbus_device() check. The check may be changed in a >>> near future, see discussion on the qemu-devel ML. >> I think this has to be solved. So I remove these last 2 sentences. I'll have >> to wait until that other patch series from Eduard is merged since it doesn't >> start yet. > The series was just merged to master. It's possible to make a > machine accept the new device using > machine_class_allow_dynamic_sysbus_dev(), now. I saw that. > > However, is it really necessary to make it a sysbus device? > Having bus-less devices was not possible in the past, but it is > possible today. > What I don't like about it is the fact that I would have to use q35 if we only extend that machine type to allow this sysbus device. What is the reason that dynamic sysbus devices have to explicitly be allowed? If we don't need to limit this device to a certain machine type that may be more user friendly.
On Fri, Jan 19, 2018 at 04:56:31PM -0500, Stefan Berger wrote: > On 01/19/2018 01:42 PM, Eduardo Habkost wrote: > > On Fri, Jan 19, 2018 at 12:10:03PM -0500, Stefan Berger wrote: > > > On 01/19/2018 09:11 AM, Marc-André Lureau wrote: > > > > tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) > > > > Interface as defined in TCG PC Client Platform TPM Profile (PTP) > > > > Specification Family “2.0” Level 00 Revision 01.03 v22. > > > > > > > > The PTP allows device implementation to switch between TIS and CRB > > > > model at run time, but given that CRB is a simpler device to > > > > implement, I chose to implement it as a different device. > > > > > > > > The device doesn't implement other locality than 0 for now (my laptop > > > > TPM doesn't either, so I assume this isn't so bad) > > > > > > > > The command/reply memory region is statically allocated after the CRB > > > > registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I > > > > wonder if the BIOS could or should allocate it instead, or what size > > > > to use, again this seems to fit well expectations) > > > I removed this last sentence now. It's at the right location. > > > > > > > The PTP doesn't specify a particular bus to put the device. So I added > > > > it on the system bus directly, so it could hopefully be used easily on > > > > a different platform than x86. Currently, it fails to init on piix, > > > > because error_on_sysbus_device() check. The check may be changed in a > > > > near future, see discussion on the qemu-devel ML. > > > I think this has to be solved. So I remove these last 2 sentences. I'll have > > > to wait until that other patch series from Eduard is merged since it doesn't > > > start yet. > > The series was just merged to master. It's possible to make a > > machine accept the new device using > > machine_class_allow_dynamic_sysbus_dev(), now. > > I saw that. > > > > However, is it really necessary to make it a sysbus device? > > Having bus-less devices was not possible in the past, but it is > > possible today. > > > What I don't like about it is the fact that I would have to use q35 if we > only extend that machine type to allow this sysbus device. What is the > reason that dynamic sysbus devices have to explicitly be allowed? If we > don't need to limit this device to a certain machine type that may be more > user friendly. Most sysbus devices don't work unless they have additional supporting machine code to wire them at the right addresses. Devices that work without extra machine code (like *-iommu) are the exception. I think the last time I saw an explanation for this was at: https://www.mail-archive.com/qemu-devel@nongnu.org/msg439549.html
On 01/19/2018 11:11 AM, Marc-André Lureau wrote: > tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) > Interface as defined in TCG PC Client Platform TPM Profile (PTP) > Specification Family “2.0” Level 00 Revision 01.03 v22. > > The PTP allows device implementation to switch between TIS and CRB > model at run time, but given that CRB is a simpler device to > implement, I chose to implement it as a different device. > > The device doesn't implement other locality than 0 for now (my laptop > TPM doesn't either, so I assume this isn't so bad) > > The command/reply memory region is statically allocated after the CRB > registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I > wonder if the BIOS could or should allocate it instead, or what size > to use, again this seems to fit well expectations) > > The PTP doesn't specify a particular bus to put the device. So I added > it on the system bus directly, so it could hopefully be used easily on > a different platform than x86. Currently, it fails to init on piix, > because error_on_sysbus_device() check. The check may be changed in a > near future, see discussion on the qemu-devel ML. > > Tested with some success with Linux upstream and Windows 10, seabios & > modified ovmf. The device is recognized and correctly transmit > command/response with passthrough & emu. However, we are missing PPI > ACPI part atm. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > --- > qapi/tpm.json | 5 +- > include/hw/acpi/tpm.h | 72 ++++++++ > include/sysemu/tpm.h | 3 + > hw/i386/acpi-build.c | 34 +++- > hw/tpm/tpm_crb.c | 327 +++++++++++++++++++++++++++++++++++++ > default-configs/i386-softmmu.mak | 1 + > default-configs/x86_64-softmmu.mak | 1 + > hw/tpm/Makefile.objs | 1 + > 8 files changed, 434 insertions(+), 10 deletions(-) > create mode 100644 hw/tpm/tpm_crb.c > > diff --git a/qapi/tpm.json b/qapi/tpm.json > index 7093f268fb..d50deef5e9 100644 > --- a/qapi/tpm.json > +++ b/qapi/tpm.json > @@ -11,10 +11,11 @@ > # An enumeration of TPM models > # > # @tpm-tis: TPM TIS model > +# @tpm-crb: TPM CRB model (since 2.12) > # > # Since: 1.5 > ## > -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } > +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } > > ## > # @query-tpm-models: > @@ -28,7 +29,7 @@ > # Example: > # > # -> { "execute": "query-tpm-models" } > -# <- { "return": [ "tpm-tis" ] } > +# <- { "return": [ "tpm-tis", "tpm-crb" ] } > # > ## > { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } > diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h > index 6d516c6a7f..b0048515fa 100644 > --- a/include/hw/acpi/tpm.h > +++ b/include/hw/acpi/tpm.h > @@ -16,11 +16,82 @@ > #ifndef HW_ACPI_TPM_H > #define HW_ACPI_TPM_H > > +#include "qemu/osdep.h" > + > #define TPM_TIS_ADDR_BASE 0xFED40000 > #define TPM_TIS_ADDR_SIZE 0x5000 > > #define TPM_TIS_IRQ 5 > > +struct crb_regs { > + union { > + uint32_t reg; > + struct { > + unsigned tpm_established:1; > + unsigned loc_assigned:1; > + unsigned active_locality:3; > + unsigned reserved:2; > + unsigned tpm_reg_valid_sts:1; > + } bits; I suppose this is little-endian layout, so this won't work on big-endian hosts. Can you add a qtest? > + } loc_state; > + uint32_t reserved1; > + uint32_t loc_ctrl; > + union { > + uint32_t reg; > + struct { > + unsigned granted:1; > + unsigned been_seized:1; > + } bits; This is unclear where you expect those bits (right/left aligned). Can you add an unnamed field to be more explicit? i.e. without using struct if left alignment expected: unsigned granted:1, been_seized:1, :30; > + } loc_sts; > + uint8_t reserved2[32]; > + union { > + uint64_t reg; > + struct { > + unsigned type:4; > + unsigned version:4; > + unsigned cap_locality:1; > + unsigned cap_crb_idle_bypass:1; > + unsigned reserved1:1; > + unsigned cap_data_xfer_size_support:2; > + unsigned cap_fifo:1; > + unsigned cap_crb:1; > + unsigned cap_if_res:2; > + unsigned if_selector:2; > + unsigned if_selector_lock:1; > + unsigned reserved2:4; > + unsigned rid:8; > + unsigned vid:16; > + unsigned did:16; > + } bits; > + } intf_id; > + uint64_t ctrl_ext; > + > + uint32_t ctrl_req; > + union { > + uint32_t reg; > + struct { > + unsigned tpm_sts:1; > + unsigned tpm_idle:1; > + unsigned reserved:30; Oh here you use 'reserved' to left align. > + } bits; > + } ctrl_sts; > + uint32_t ctrl_cancel; > + uint32_t ctrl_start; > + uint32_t ctrl_int_enable; > + uint32_t ctrl_int_sts; > + uint32_t ctrl_cmd_size; > + uint32_t ctrl_cmd_pa_low; > + uint32_t ctrl_cmd_pa_high; > + uint32_t ctrl_rsp_size; > + uint64_t ctrl_rsp_pa; > + uint8_t reserved3[0x10]; > +} QEMU_PACKED; > + > +#define TPM_CRB_ADDR_BASE 0xFED40000 > +#define TPM_CRB_ADDR_SIZE 0x1000 > +#define TPM_CRB_ADDR_CTRL \ > + (TPM_CRB_ADDR_BASE + offsetof(struct crb_regs, ctrl_req)) > + > #define TPM_LOG_AREA_MINIMUM_SIZE (64 * 1024) > > #define TPM_TCPA_ACPI_CLASS_CLIENT 0 > @@ -30,5 +101,6 @@ > #define TPM2_ACPI_CLASS_SERVER 1 > > #define TPM2_START_METHOD_MMIO 6 > +#define TPM2_START_METHOD_CRB 7 > > #endif /* HW_ACPI_TPM_H */ > diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h > index ac04a9d4a2..233b1a3fc3 100644 > --- a/include/sysemu/tpm.h > +++ b/include/sysemu/tpm.h > @@ -46,9 +46,12 @@ typedef struct TPMIfClass { > } TPMIfClass; > > #define TYPE_TPM_TIS "tpm-tis" > +#define TYPE_TPM_CRB "tpm-crb" > > #define TPM_IS_TIS(chr) \ > object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS) > +#define TPM_IS_CRB(chr) \ > + object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB) > > /* returns NULL unless there is exactly one TPM device */ > static inline TPMIf *tpm_find(void) > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index dc4b2b9ffe..ed78c4ed9f 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -2224,6 +2224,22 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > aml_append(sb_scope, scope); > } > } > + > + if (TPM_IS_CRB(tpm_find())) { > + dev = aml_device("TPM"); > + aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101"))); > + crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, > + TPM_CRB_ADDR_SIZE, AML_READ_WRITE)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + > + method = aml_method("_STA", 0, AML_NOTSERIALIZED); > + aml_append(method, aml_return(aml_int(0x0f))); > + aml_append(dev, method); > + > + aml_append(sb_scope, dev); > + } > + > aml_append(dsdt, sb_scope); > > /* copy AML table into ACPI tables blob and patch header there */ > @@ -2285,18 +2301,20 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog) > if (TPM_IS_TIS(tpm_find())) { > tpm2_ptr->control_area_address = cpu_to_le64(0); > tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO); > - > - tpm2_ptr->log_area_minimum_length = > - cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); > - > - /* log area start address to be filled by Guest linker */ > - bios_linker_loader_add_pointer(linker, > - ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size, > - ACPI_BUILD_TPMLOG_FILE, 0); > + } else if (TPM_IS_CRB(tpm_find())) { > + tpm2_ptr->control_area_address = cpu_to_le64(TPM_CRB_ADDR_CTRL); > + tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_CRB); > } else { > g_warn_if_reached(); > } > > + tpm2_ptr->log_area_minimum_length = > + cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); > + > + /* log area start address to be filled by Guest linker */ > + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, > + log_addr_offset, log_addr_size, > + ACPI_BUILD_TPMLOG_FILE, 0); > build_header(linker, table_data, > (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL); > } > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c > new file mode 100644 > index 0000000000..908ca18d92 > --- /dev/null > +++ b/hw/tpm/tpm_crb.c > @@ -0,0 +1,327 @@ > +/* > + * tpm_crb.c - QEMU's TPM CRB interface emulator > + * > + * Copyright (c) 2017 Red Hat, Inc. > + * > + * Authors: > + * Marc-André Lureau <marcandre.lureau@redhat.com> > + * > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > + * See the COPYING file in the top-level directory. > + * > + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface > + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification > + * Family “2.0” Level 00 Revision 01.03 v22 > + */ > + > +#include "qemu/osdep.h" > + > +#include "qemu-common.h" > +#include "qapi/error.h" > +#include "hw/sysbus.h" > +#include "exec/address-spaces.h" > + > +#include "hw/pci/pci_ids.h" > +#include "hw/acpi/tpm.h" > +#include "sysemu/tpm_backend.h" > +#include "tpm_int.h" > +#include "tpm_util.h" > + > +typedef struct CRBState { > + SysBusDevice parent_obj; > + > + TPMBackend *tpmbe; > + TPMBackendCmd cmd; > + struct crb_regs regs; > + MemoryRegion mmio; > + MemoryRegion cmdmem; > + > + size_t be_buffer_size; > +} CRBState; > + > +#define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB) > + > +#define DEBUG_CRB 0 > + > +#define DPRINTF(fmt, ...) do { \ > + if (DEBUG_CRB) { \ > + printf(fmt, ## __VA_ARGS__); \ > + } \ > + } while (0) > + > +#define CRB_ADDR_LOC_STATE offsetof(struct crb_regs, loc_state) > +#define CRB_ADDR_LOC_CTRL offsetof(struct crb_regs, loc_ctrl) > +#define CRB_ADDR_CTRL_REQ offsetof(struct crb_regs, ctrl_req) > +#define CRB_ADDR_CTRL_CANCEL offsetof(struct crb_regs, ctrl_cancel) > +#define CRB_ADDR_CTRL_START offsetof(struct crb_regs, ctrl_start) > + > +#define CRB_INTF_TYPE_CRB_ACTIVE 0b1 > +#define CRB_INTF_VERSION_CRB 0b1 > +#define CRB_INTF_CAP_LOCALITY_0_ONLY 0b0 > +#define CRB_INTF_CAP_IDLE_FAST 0b0 > +#define CRB_INTF_CAP_XFER_SIZE_64 0b11 > +#define CRB_INTF_CAP_FIFO_NOT_SUPPORTED 0b0 > +#define CRB_INTF_CAP_CRB_SUPPORTED 0b1 > +#define CRB_INTF_IF_SELECTOR_CRB 0b1 > +#define CRB_INTF_IF_SELECTOR_UNLOCKED 0b0 > + > +#define CRB_CTRL_CMD_SIZE (TPM_CRB_ADDR_SIZE - sizeof(struct crb_regs)) > + > +enum crb_loc_ctrl { > + CRB_LOC_CTRL_REQUEST_ACCESS = BIT(0), > + CRB_LOC_CTRL_RELINQUISH = BIT(1), > + CRB_LOC_CTRL_SEIZE = BIT(2), > + CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT = BIT(3), > +}; > + > +enum crb_ctrl_req { > + CRB_CTRL_REQ_CMD_READY = BIT(0), > + CRB_CTRL_REQ_GO_IDLE = BIT(1), > +}; > + > +enum crb_start { > + CRB_START_INVOKE = BIT(0), > +}; > + > +enum crb_cancel { > + CRB_CANCEL_INVOKE = BIT(0), > +}; > + > +static const char *addr_desc(unsigned off) > +{ > + struct crb_regs crb; > + > + switch (off) { > +#define CASE(field) \ > + case offsetof(struct crb_regs, field) ... \ > + offsetof(struct crb_regs, field) + sizeof(crb.field) - 1: \ > + return G_STRINGIFY(field); > + CASE(loc_state); > + CASE(reserved1); > + CASE(loc_ctrl); > + CASE(loc_sts); > + CASE(reserved2); > + CASE(intf_id); > + CASE(ctrl_ext); > + CASE(ctrl_req); > + CASE(ctrl_sts); > + CASE(ctrl_cancel); > + CASE(ctrl_start); > + CASE(ctrl_int_enable); > + CASE(ctrl_int_sts); > + CASE(ctrl_cmd_size); > + CASE(ctrl_cmd_pa_low); > + CASE(ctrl_cmd_pa_high); > + CASE(ctrl_rsp_size); > + CASE(ctrl_rsp_pa); > +#undef CASE > + } > + return NULL; > +} > + > +static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, > + unsigned size) > +{ > + CRBState *s = CRB(opaque); > + DPRINTF("CRB read 0x%lx:%s len:%u\n", > + addr, addr_desc(addr), size); > + void *regs = (void *)&s->regs + addr; > + > + switch (size) { > + case 1: > + return *(uint8_t *)regs; > + case 2: > + return *(uint16_t *)regs; > + case 4: > + return *(uint32_t *)regs; > + default: > + g_return_val_if_reached(-1); > + } > +} > + > +static void tpm_crb_mmio_write(void *opaque, hwaddr addr, > + uint64_t val, unsigned size) > +{ > + CRBState *s = CRB(opaque); > + DPRINTF("CRB write 0x%lx:%s len:%u val:%lu\n", > + addr, addr_desc(addr), size, val); > + > + switch (addr) { > + case CRB_ADDR_CTRL_REQ: > + switch (val) { > + case CRB_CTRL_REQ_CMD_READY: > + s->regs.ctrl_sts.bits.tpm_idle = 0; > + break; > + case CRB_CTRL_REQ_GO_IDLE: > + s->regs.ctrl_sts.bits.tpm_idle = 1; > + break; > + } > + break; > + case CRB_ADDR_CTRL_CANCEL: > + if (val == CRB_CANCEL_INVOKE && s->regs.ctrl_start & CRB_START_INVOKE) { > + tpm_backend_cancel_cmd(s->tpmbe); > + } > + break; > + case CRB_ADDR_CTRL_START: > + if (val == CRB_START_INVOKE && > + !(s->regs.ctrl_start & CRB_START_INVOKE)) { > + void *mem = memory_region_get_ram_ptr(&s->cmdmem); > + > + s->regs.ctrl_start |= CRB_START_INVOKE; > + s->cmd = (TPMBackendCmd) { > + .in = mem, > + .in_len = MIN(tpm_cmd_get_size(mem), s->be_buffer_size), > + .out = mem, > + .out_len = s->be_buffer_size, > + }; > + > + tpm_backend_deliver_request(s->tpmbe, &s->cmd); > + } > + break; > + case CRB_ADDR_LOC_CTRL: > + switch (val) { > + case CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT: > + /* not loc 3 or 4 */ > + break; > + case CRB_LOC_CTRL_RELINQUISH: > + break; > + case CRB_LOC_CTRL_REQUEST_ACCESS: > + s->regs.loc_sts.bits.granted = 1; > + s->regs.loc_sts.bits.been_seized = 0; > + s->regs.loc_state.bits.loc_assigned = 1; > + s->regs.loc_state.bits.tpm_reg_valid_sts = 1; > + break; > + } > + break; > + } > +} > + > +static const MemoryRegionOps tpm_crb_memory_ops = { > + .read = tpm_crb_mmio_read, > + .write = tpm_crb_mmio_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > + .valid = { > + .min_access_size = 1, > + .max_access_size = 4, > + }, > +}; > + > +static void tpm_crb_reset(DeviceState *dev) > +{ > + CRBState *s = CRB(dev); > + > + tpm_backend_reset(s->tpmbe); > + > + s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), > + CRB_CTRL_CMD_SIZE); > + > + s->regs = (struct crb_regs) { > + .intf_id.bits = { > + .type = CRB_INTF_TYPE_CRB_ACTIVE, > + .version = CRB_INTF_VERSION_CRB, > + .cap_locality = CRB_INTF_CAP_LOCALITY_0_ONLY, > + .cap_crb_idle_bypass = CRB_INTF_CAP_IDLE_FAST, > + .cap_data_xfer_size_support = CRB_INTF_CAP_XFER_SIZE_64, > + .cap_fifo = CRB_INTF_CAP_FIFO_NOT_SUPPORTED, > + .cap_crb = CRB_INTF_CAP_CRB_SUPPORTED, > + .cap_if_res = 0b0, > + .if_selector = CRB_INTF_IF_SELECTOR_CRB, > + .if_selector_lock = CRB_INTF_IF_SELECTOR_UNLOCKED, > + .rid = 0b0001, > + .vid = PCI_VENDOR_ID_IBM, > + .did = 0b0001, > + }, > + .ctrl_cmd_size = CRB_CTRL_CMD_SIZE, > + .ctrl_cmd_pa_low = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), > + .ctrl_rsp_size = CRB_CTRL_CMD_SIZE, > + .ctrl_rsp_pa = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), > + }; > + > + tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size); > +} > + > +static void tpm_crb_request_completed(TPMIf *ti, int ret) > +{ > + CRBState *s = CRB(ti); > + > + s->regs.ctrl_start &= ~CRB_START_INVOKE; > + if (ret != 0) { > + s->regs.ctrl_sts.bits.tpm_sts = 1; /* fatal error */ > + } > +} > + > +static enum TPMVersion tpm_crb_get_version(TPMIf *ti) > +{ > + CRBState *s = CRB(ti); > + > + return tpm_backend_get_tpm_version(s->tpmbe); > +} > + > +static const VMStateDescription vmstate_tpm_crb = { > + .name = "tpm-crb", > + .unmigratable = 1, > +}; > + > +static Property tpm_crb_properties[] = { > + DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe), > + DEFINE_PROP_END_OF_LIST(), > +}; > + > +static void tpm_crb_realizefn(DeviceState *dev, Error **errp) > +{ > + CRBState *s = CRB(dev); > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > + > + if (!tpm_find()) { > + error_setg(errp, "at most one TPM device is permitted"); > + return; > + } > + if (!s->tpmbe) { > + error_setg(errp, "'tpmdev' property is required"); > + return; > + } > + > + memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s, > + "tpm-crb-mmio", sizeof(struct crb_regs)); > + memory_region_init_ram(&s->cmdmem, OBJECT(s), > + "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); > + > + sysbus_init_mmio(sbd, &s->mmio); > + sysbus_mmio_map(sbd, 0, TPM_CRB_ADDR_BASE); > + /* allocate ram in bios instead? */ > + memory_region_add_subregion(get_system_memory(), > + TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), &s->cmdmem); > +} > + > +static void tpm_crb_class_init(ObjectClass *klass, void *data) > +{ > + DeviceClass *dc = DEVICE_CLASS(klass); > + TPMIfClass *tc = TPM_IF_CLASS(klass); > + > + dc->realize = tpm_crb_realizefn; > + dc->props = tpm_crb_properties; > + dc->reset = tpm_crb_reset; > + dc->vmsd = &vmstate_tpm_crb; > + dc->user_creatable = true; > + tc->model = TPM_MODEL_TPM_CRB; > + tc->get_version = tpm_crb_get_version; > + tc->request_completed = tpm_crb_request_completed; > +} > + > +static const TypeInfo tpm_crb_info = { > + .name = TYPE_TPM_CRB, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(CRBState), > + .class_init = tpm_crb_class_init, > + .interfaces = (InterfaceInfo[]) { > + { TYPE_TPM_IF }, > + { } > + } > +}; > + > +static void tpm_crb_register(void) > +{ > + type_register_static(&tpm_crb_info); > +} > + > +type_init(tpm_crb_register) > diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak > index 95ac4b464a..ac27700e79 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y > CONFIG_I8259=y > CONFIG_PFLASH_CFI01=y > CONFIG_TPM_TIS=$(CONFIG_TPM) > +CONFIG_TPM_CRB=$(CONFIG_TPM) > CONFIG_MC146818RTC=y > CONFIG_PCI_PIIX=y > CONFIG_WDT_IB700=y > diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak > index 0221236825..b2104ade19 100644 > --- a/default-configs/x86_64-softmmu.mak > +++ b/default-configs/x86_64-softmmu.mak > @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y > CONFIG_I8259=y > CONFIG_PFLASH_CFI01=y > CONFIG_TPM_TIS=$(CONFIG_TPM) > +CONFIG_TPM_CRB=$(CONFIG_TPM) > CONFIG_MC146818RTC=y > CONFIG_PCI_PIIX=y > CONFIG_WDT_IB700=y > diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs > index 7a93b24636..1dc9f8bf2c 100644 > --- a/hw/tpm/Makefile.objs > +++ b/hw/tpm/Makefile.objs > @@ -1,4 +1,5 @@ > common-obj-y += tpm_util.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 > common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o >
On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: > On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >> Specification Family “2.0” Level 00 Revision 01.03 v22. >> >> The PTP allows device implementation to switch between TIS and CRB >> model at run time, but given that CRB is a simpler device to >> implement, I chose to implement it as a different device. >> >> The device doesn't implement other locality than 0 for now (my laptop >> TPM doesn't either, so I assume this isn't so bad) >> >> The command/reply memory region is statically allocated after the CRB >> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >> wonder if the BIOS could or should allocate it instead, or what size >> to use, again this seems to fit well expectations) >> >> The PTP doesn't specify a particular bus to put the device. So I added >> it on the system bus directly, so it could hopefully be used easily on >> a different platform than x86. Currently, it fails to init on piix, >> because error_on_sysbus_device() check. The check may be changed in a >> near future, see discussion on the qemu-devel ML. >> >> Tested with some success with Linux upstream and Windows 10, seabios & >> modified ovmf. The device is recognized and correctly transmit >> command/response with passthrough & emu. However, we are missing PPI >> ACPI part atm. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >> --- >> qapi/tpm.json | 5 +- >> include/hw/acpi/tpm.h | 72 ++++++++ >> include/sysemu/tpm.h | 3 + >> hw/i386/acpi-build.c | 34 +++- >> hw/tpm/tpm_crb.c | 327 +++++++++++++++++++++++++++++++++++++ >> default-configs/i386-softmmu.mak | 1 + >> default-configs/x86_64-softmmu.mak | 1 + >> hw/tpm/Makefile.objs | 1 + >> 8 files changed, 434 insertions(+), 10 deletions(-) >> create mode 100644 hw/tpm/tpm_crb.c >> >> diff --git a/qapi/tpm.json b/qapi/tpm.json >> index 7093f268fb..d50deef5e9 100644 >> --- a/qapi/tpm.json >> +++ b/qapi/tpm.json >> @@ -11,10 +11,11 @@ >> # An enumeration of TPM models >> # >> # @tpm-tis: TPM TIS model >> +# @tpm-crb: TPM CRB model (since 2.12) >> # >> # Since: 1.5 >> ## >> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >> >> ## >> # @query-tpm-models: >> @@ -28,7 +29,7 @@ >> # Example: >> # >> # -> { "execute": "query-tpm-models" } >> -# <- { "return": [ "tpm-tis" ] } >> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >> # >> ## >> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >> index 6d516c6a7f..b0048515fa 100644 >> --- a/include/hw/acpi/tpm.h >> +++ b/include/hw/acpi/tpm.h >> @@ -16,11 +16,82 @@ >> #ifndef HW_ACPI_TPM_H >> #define HW_ACPI_TPM_H >> >> +#include "qemu/osdep.h" >> + >> #define TPM_TIS_ADDR_BASE 0xFED40000 >> #define TPM_TIS_ADDR_SIZE 0x5000 >> >> #define TPM_TIS_IRQ 5 >> >> +struct crb_regs { >> + union { >> + uint32_t reg; >> + struct { >> + unsigned tpm_established:1; >> + unsigned loc_assigned:1; >> + unsigned active_locality:3; >> + unsigned reserved:2; >> + unsigned tpm_reg_valid_sts:1; >> + } bits; > I suppose this is little-endian layout, so this won't work on big-endian > hosts. > > Can you add a qtest? > >> + } loc_state; >> + uint32_t reserved1; >> + uint32_t loc_ctrl; >> + union { >> + uint32_t reg; >> + struct { >> + unsigned granted:1; >> + unsigned been_seized:1; >> + } bits; > > This is unclear where you expect those bits (right/left aligned). > > Can you add an unnamed field to be more explicit? > > i.e. without using struct if left alignment expected: > > unsigned granted:1, been_seized:1, :30; I got rid of all the bitfields and this patch here makes it work on a ppc64 big endian and also x86_64 host: https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd Regards, Stefan > >> + } loc_sts; >> + uint8_t reserved2[32]; >> + union { >> + uint64_t reg; >> + struct { >> + unsigned type:4; >> + unsigned version:4; >> + unsigned cap_locality:1; >> + unsigned cap_crb_idle_bypass:1; >> + unsigned reserved1:1; >> + unsigned cap_data_xfer_size_support:2; >> + unsigned cap_fifo:1; >> + unsigned cap_crb:1; >> + unsigned cap_if_res:2; >> + unsigned if_selector:2; >> + unsigned if_selector_lock:1; >> + unsigned reserved2:4; >> + unsigned rid:8; >> + unsigned vid:16; >> + unsigned did:16; >> + } bits; >> + } intf_id; >> + uint64_t ctrl_ext; >> + >> + uint32_t ctrl_req; >> + union { >> + uint32_t reg; >> + struct { >> + unsigned tpm_sts:1; >> + unsigned tpm_idle:1; >> + unsigned reserved:30; > Oh here you use 'reserved' to left align. > >> + } bits; >> + } ctrl_sts; >> + uint32_t ctrl_cancel; >> + uint32_t ctrl_start; >> + uint32_t ctrl_int_enable; >> + uint32_t ctrl_int_sts; >> + uint32_t ctrl_cmd_size; >> + uint32_t ctrl_cmd_pa_low; >> + uint32_t ctrl_cmd_pa_high; >> + uint32_t ctrl_rsp_size; >> + uint64_t ctrl_rsp_pa; >> + uint8_t reserved3[0x10]; >> +} QEMU_PACKED; >> + >> +#define TPM_CRB_ADDR_BASE 0xFED40000 >> +#define TPM_CRB_ADDR_SIZE 0x1000 >> +#define TPM_CRB_ADDR_CTRL \ >> + (TPM_CRB_ADDR_BASE + offsetof(struct crb_regs, ctrl_req)) >> + >> #define TPM_LOG_AREA_MINIMUM_SIZE (64 * 1024) >> >> #define TPM_TCPA_ACPI_CLASS_CLIENT 0 >> @@ -30,5 +101,6 @@ >> #define TPM2_ACPI_CLASS_SERVER 1 >> >> #define TPM2_START_METHOD_MMIO 6 >> +#define TPM2_START_METHOD_CRB 7 >> >> #endif /* HW_ACPI_TPM_H */ >> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h >> index ac04a9d4a2..233b1a3fc3 100644 >> --- a/include/sysemu/tpm.h >> +++ b/include/sysemu/tpm.h >> @@ -46,9 +46,12 @@ typedef struct TPMIfClass { >> } TPMIfClass; >> >> #define TYPE_TPM_TIS "tpm-tis" >> +#define TYPE_TPM_CRB "tpm-crb" >> >> #define TPM_IS_TIS(chr) \ >> object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS) >> +#define TPM_IS_CRB(chr) \ >> + object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB) >> >> /* returns NULL unless there is exactly one TPM device */ >> static inline TPMIf *tpm_find(void) >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >> index dc4b2b9ffe..ed78c4ed9f 100644 >> --- a/hw/i386/acpi-build.c >> +++ b/hw/i386/acpi-build.c >> @@ -2224,6 +2224,22 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >> aml_append(sb_scope, scope); >> } >> } >> + >> + if (TPM_IS_CRB(tpm_find())) { >> + dev = aml_device("TPM"); >> + aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101"))); >> + crs = aml_resource_template(); >> + aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, >> + TPM_CRB_ADDR_SIZE, AML_READ_WRITE)); >> + aml_append(dev, aml_name_decl("_CRS", crs)); >> + >> + method = aml_method("_STA", 0, AML_NOTSERIALIZED); >> + aml_append(method, aml_return(aml_int(0x0f))); >> + aml_append(dev, method); >> + >> + aml_append(sb_scope, dev); >> + } >> + >> aml_append(dsdt, sb_scope); >> >> /* copy AML table into ACPI tables blob and patch header there */ >> @@ -2285,18 +2301,20 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog) >> if (TPM_IS_TIS(tpm_find())) { >> tpm2_ptr->control_area_address = cpu_to_le64(0); >> tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO); >> - >> - tpm2_ptr->log_area_minimum_length = >> - cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); >> - >> - /* log area start address to be filled by Guest linker */ >> - bios_linker_loader_add_pointer(linker, >> - ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size, >> - ACPI_BUILD_TPMLOG_FILE, 0); >> + } else if (TPM_IS_CRB(tpm_find())) { >> + tpm2_ptr->control_area_address = cpu_to_le64(TPM_CRB_ADDR_CTRL); >> + tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_CRB); >> } else { >> g_warn_if_reached(); >> } >> >> + tpm2_ptr->log_area_minimum_length = >> + cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); >> + >> + /* log area start address to be filled by Guest linker */ >> + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, >> + log_addr_offset, log_addr_size, >> + ACPI_BUILD_TPMLOG_FILE, 0); >> build_header(linker, table_data, >> (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL); >> } >> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c >> new file mode 100644 >> index 0000000000..908ca18d92 >> --- /dev/null >> +++ b/hw/tpm/tpm_crb.c >> @@ -0,0 +1,327 @@ >> +/* >> + * tpm_crb.c - QEMU's TPM CRB interface emulator >> + * >> + * Copyright (c) 2017 Red Hat, Inc. >> + * >> + * Authors: >> + * Marc-André Lureau <marcandre.lureau@redhat.com> >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or later. >> + * See the COPYING file in the top-level directory. >> + * >> + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface >> + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification >> + * Family “2.0” Level 00 Revision 01.03 v22 >> + */ >> + >> +#include "qemu/osdep.h" >> + >> +#include "qemu-common.h" >> +#include "qapi/error.h" >> +#include "hw/sysbus.h" >> +#include "exec/address-spaces.h" >> + >> +#include "hw/pci/pci_ids.h" >> +#include "hw/acpi/tpm.h" >> +#include "sysemu/tpm_backend.h" >> +#include "tpm_int.h" >> +#include "tpm_util.h" >> + >> +typedef struct CRBState { >> + SysBusDevice parent_obj; >> + >> + TPMBackend *tpmbe; >> + TPMBackendCmd cmd; >> + struct crb_regs regs; >> + MemoryRegion mmio; >> + MemoryRegion cmdmem; >> + >> + size_t be_buffer_size; >> +} CRBState; >> + >> +#define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB) >> + >> +#define DEBUG_CRB 0 >> + >> +#define DPRINTF(fmt, ...) do { \ >> + if (DEBUG_CRB) { \ >> + printf(fmt, ## __VA_ARGS__); \ >> + } \ >> + } while (0) >> + >> +#define CRB_ADDR_LOC_STATE offsetof(struct crb_regs, loc_state) >> +#define CRB_ADDR_LOC_CTRL offsetof(struct crb_regs, loc_ctrl) >> +#define CRB_ADDR_CTRL_REQ offsetof(struct crb_regs, ctrl_req) >> +#define CRB_ADDR_CTRL_CANCEL offsetof(struct crb_regs, ctrl_cancel) >> +#define CRB_ADDR_CTRL_START offsetof(struct crb_regs, ctrl_start) >> + >> +#define CRB_INTF_TYPE_CRB_ACTIVE 0b1 >> +#define CRB_INTF_VERSION_CRB 0b1 >> +#define CRB_INTF_CAP_LOCALITY_0_ONLY 0b0 >> +#define CRB_INTF_CAP_IDLE_FAST 0b0 >> +#define CRB_INTF_CAP_XFER_SIZE_64 0b11 >> +#define CRB_INTF_CAP_FIFO_NOT_SUPPORTED 0b0 >> +#define CRB_INTF_CAP_CRB_SUPPORTED 0b1 >> +#define CRB_INTF_IF_SELECTOR_CRB 0b1 >> +#define CRB_INTF_IF_SELECTOR_UNLOCKED 0b0 >> + >> +#define CRB_CTRL_CMD_SIZE (TPM_CRB_ADDR_SIZE - sizeof(struct crb_regs)) >> + >> +enum crb_loc_ctrl { >> + CRB_LOC_CTRL_REQUEST_ACCESS = BIT(0), >> + CRB_LOC_CTRL_RELINQUISH = BIT(1), >> + CRB_LOC_CTRL_SEIZE = BIT(2), >> + CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT = BIT(3), >> +}; >> + >> +enum crb_ctrl_req { >> + CRB_CTRL_REQ_CMD_READY = BIT(0), >> + CRB_CTRL_REQ_GO_IDLE = BIT(1), >> +}; >> + >> +enum crb_start { >> + CRB_START_INVOKE = BIT(0), >> +}; >> + >> +enum crb_cancel { >> + CRB_CANCEL_INVOKE = BIT(0), >> +}; >> + >> +static const char *addr_desc(unsigned off) >> +{ >> + struct crb_regs crb; >> + >> + switch (off) { >> +#define CASE(field) \ >> + case offsetof(struct crb_regs, field) ... \ >> + offsetof(struct crb_regs, field) + sizeof(crb.field) - 1: \ >> + return G_STRINGIFY(field); >> + CASE(loc_state); >> + CASE(reserved1); >> + CASE(loc_ctrl); >> + CASE(loc_sts); >> + CASE(reserved2); >> + CASE(intf_id); >> + CASE(ctrl_ext); >> + CASE(ctrl_req); >> + CASE(ctrl_sts); >> + CASE(ctrl_cancel); >> + CASE(ctrl_start); >> + CASE(ctrl_int_enable); >> + CASE(ctrl_int_sts); >> + CASE(ctrl_cmd_size); >> + CASE(ctrl_cmd_pa_low); >> + CASE(ctrl_cmd_pa_high); >> + CASE(ctrl_rsp_size); >> + CASE(ctrl_rsp_pa); >> +#undef CASE >> + } >> + return NULL; >> +} >> + >> +static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, >> + unsigned size) >> +{ >> + CRBState *s = CRB(opaque); >> + DPRINTF("CRB read 0x%lx:%s len:%u\n", >> + addr, addr_desc(addr), size); >> + void *regs = (void *)&s->regs + addr; >> + >> + switch (size) { >> + case 1: >> + return *(uint8_t *)regs; >> + case 2: >> + return *(uint16_t *)regs; >> + case 4: >> + return *(uint32_t *)regs; >> + default: >> + g_return_val_if_reached(-1); >> + } >> +} >> + >> +static void tpm_crb_mmio_write(void *opaque, hwaddr addr, >> + uint64_t val, unsigned size) >> +{ >> + CRBState *s = CRB(opaque); >> + DPRINTF("CRB write 0x%lx:%s len:%u val:%lu\n", >> + addr, addr_desc(addr), size, val); >> + >> + switch (addr) { >> + case CRB_ADDR_CTRL_REQ: >> + switch (val) { >> + case CRB_CTRL_REQ_CMD_READY: >> + s->regs.ctrl_sts.bits.tpm_idle = 0; >> + break; >> + case CRB_CTRL_REQ_GO_IDLE: >> + s->regs.ctrl_sts.bits.tpm_idle = 1; >> + break; >> + } >> + break; >> + case CRB_ADDR_CTRL_CANCEL: >> + if (val == CRB_CANCEL_INVOKE && s->regs.ctrl_start & CRB_START_INVOKE) { >> + tpm_backend_cancel_cmd(s->tpmbe); >> + } >> + break; >> + case CRB_ADDR_CTRL_START: >> + if (val == CRB_START_INVOKE && >> + !(s->regs.ctrl_start & CRB_START_INVOKE)) { >> + void *mem = memory_region_get_ram_ptr(&s->cmdmem); >> + >> + s->regs.ctrl_start |= CRB_START_INVOKE; >> + s->cmd = (TPMBackendCmd) { >> + .in = mem, >> + .in_len = MIN(tpm_cmd_get_size(mem), s->be_buffer_size), >> + .out = mem, >> + .out_len = s->be_buffer_size, >> + }; >> + >> + tpm_backend_deliver_request(s->tpmbe, &s->cmd); >> + } >> + break; >> + case CRB_ADDR_LOC_CTRL: >> + switch (val) { >> + case CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT: >> + /* not loc 3 or 4 */ >> + break; >> + case CRB_LOC_CTRL_RELINQUISH: >> + break; >> + case CRB_LOC_CTRL_REQUEST_ACCESS: >> + s->regs.loc_sts.bits.granted = 1; >> + s->regs.loc_sts.bits.been_seized = 0; >> + s->regs.loc_state.bits.loc_assigned = 1; >> + s->regs.loc_state.bits.tpm_reg_valid_sts = 1; >> + break; >> + } >> + break; >> + } >> +} >> + >> +static const MemoryRegionOps tpm_crb_memory_ops = { >> + .read = tpm_crb_mmio_read, >> + .write = tpm_crb_mmio_write, >> + .endianness = DEVICE_LITTLE_ENDIAN, >> + .valid = { >> + .min_access_size = 1, >> + .max_access_size = 4, >> + }, >> +}; >> + >> +static void tpm_crb_reset(DeviceState *dev) >> +{ >> + CRBState *s = CRB(dev); >> + >> + tpm_backend_reset(s->tpmbe); >> + >> + s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), >> + CRB_CTRL_CMD_SIZE); >> + >> + s->regs = (struct crb_regs) { >> + .intf_id.bits = { >> + .type = CRB_INTF_TYPE_CRB_ACTIVE, >> + .version = CRB_INTF_VERSION_CRB, >> + .cap_locality = CRB_INTF_CAP_LOCALITY_0_ONLY, >> + .cap_crb_idle_bypass = CRB_INTF_CAP_IDLE_FAST, >> + .cap_data_xfer_size_support = CRB_INTF_CAP_XFER_SIZE_64, >> + .cap_fifo = CRB_INTF_CAP_FIFO_NOT_SUPPORTED, >> + .cap_crb = CRB_INTF_CAP_CRB_SUPPORTED, >> + .cap_if_res = 0b0, >> + .if_selector = CRB_INTF_IF_SELECTOR_CRB, >> + .if_selector_lock = CRB_INTF_IF_SELECTOR_UNLOCKED, >> + .rid = 0b0001, >> + .vid = PCI_VENDOR_ID_IBM, >> + .did = 0b0001, >> + }, >> + .ctrl_cmd_size = CRB_CTRL_CMD_SIZE, >> + .ctrl_cmd_pa_low = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), >> + .ctrl_rsp_size = CRB_CTRL_CMD_SIZE, >> + .ctrl_rsp_pa = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), >> + }; >> + >> + tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size); >> +} >> + >> +static void tpm_crb_request_completed(TPMIf *ti, int ret) >> +{ >> + CRBState *s = CRB(ti); >> + >> + s->regs.ctrl_start &= ~CRB_START_INVOKE; >> + if (ret != 0) { >> + s->regs.ctrl_sts.bits.tpm_sts = 1; /* fatal error */ >> + } >> +} >> + >> +static enum TPMVersion tpm_crb_get_version(TPMIf *ti) >> +{ >> + CRBState *s = CRB(ti); >> + >> + return tpm_backend_get_tpm_version(s->tpmbe); >> +} >> + >> +static const VMStateDescription vmstate_tpm_crb = { >> + .name = "tpm-crb", >> + .unmigratable = 1, >> +}; >> + >> +static Property tpm_crb_properties[] = { >> + DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe), >> + DEFINE_PROP_END_OF_LIST(), >> +}; >> + >> +static void tpm_crb_realizefn(DeviceState *dev, Error **errp) >> +{ >> + CRBState *s = CRB(dev); >> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); >> + >> + if (!tpm_find()) { >> + error_setg(errp, "at most one TPM device is permitted"); >> + return; >> + } >> + if (!s->tpmbe) { >> + error_setg(errp, "'tpmdev' property is required"); >> + return; >> + } >> + >> + memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s, >> + "tpm-crb-mmio", sizeof(struct crb_regs)); >> + memory_region_init_ram(&s->cmdmem, OBJECT(s), >> + "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); >> + >> + sysbus_init_mmio(sbd, &s->mmio); >> + sysbus_mmio_map(sbd, 0, TPM_CRB_ADDR_BASE); >> + /* allocate ram in bios instead? */ >> + memory_region_add_subregion(get_system_memory(), >> + TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), &s->cmdmem); >> +} >> + >> +static void tpm_crb_class_init(ObjectClass *klass, void *data) >> +{ >> + DeviceClass *dc = DEVICE_CLASS(klass); >> + TPMIfClass *tc = TPM_IF_CLASS(klass); >> + >> + dc->realize = tpm_crb_realizefn; >> + dc->props = tpm_crb_properties; >> + dc->reset = tpm_crb_reset; >> + dc->vmsd = &vmstate_tpm_crb; >> + dc->user_creatable = true; >> + tc->model = TPM_MODEL_TPM_CRB; >> + tc->get_version = tpm_crb_get_version; >> + tc->request_completed = tpm_crb_request_completed; >> +} >> + >> +static const TypeInfo tpm_crb_info = { >> + .name = TYPE_TPM_CRB, >> + .parent = TYPE_SYS_BUS_DEVICE, >> + .instance_size = sizeof(CRBState), >> + .class_init = tpm_crb_class_init, >> + .interfaces = (InterfaceInfo[]) { >> + { TYPE_TPM_IF }, >> + { } >> + } >> +}; >> + >> +static void tpm_crb_register(void) >> +{ >> + type_register_static(&tpm_crb_info); >> +} >> + >> +type_init(tpm_crb_register) >> diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak >> index 95ac4b464a..ac27700e79 100644 >> --- a/default-configs/i386-softmmu.mak >> +++ b/default-configs/i386-softmmu.mak >> @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y >> CONFIG_I8259=y >> CONFIG_PFLASH_CFI01=y >> CONFIG_TPM_TIS=$(CONFIG_TPM) >> +CONFIG_TPM_CRB=$(CONFIG_TPM) >> CONFIG_MC146818RTC=y >> CONFIG_PCI_PIIX=y >> CONFIG_WDT_IB700=y >> diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak >> index 0221236825..b2104ade19 100644 >> --- a/default-configs/x86_64-softmmu.mak >> +++ b/default-configs/x86_64-softmmu.mak >> @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y >> CONFIG_I8259=y >> CONFIG_PFLASH_CFI01=y >> CONFIG_TPM_TIS=$(CONFIG_TPM) >> +CONFIG_TPM_CRB=$(CONFIG_TPM) >> CONFIG_MC146818RTC=y >> CONFIG_PCI_PIIX=y >> CONFIG_WDT_IB700=y >> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs >> index 7a93b24636..1dc9f8bf2c 100644 >> --- a/hw/tpm/Makefile.objs >> +++ b/hw/tpm/Makefile.objs >> @@ -1,4 +1,5 @@ >> common-obj-y += tpm_util.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 >> common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o >>
Hi On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >> >> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >>> >>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >>> Specification Family “2.0” Level 00 Revision 01.03 v22. >>> >>> The PTP allows device implementation to switch between TIS and CRB >>> model at run time, but given that CRB is a simpler device to >>> implement, I chose to implement it as a different device. >>> >>> The device doesn't implement other locality than 0 for now (my laptop >>> TPM doesn't either, so I assume this isn't so bad) >>> >>> The command/reply memory region is statically allocated after the CRB >>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >>> wonder if the BIOS could or should allocate it instead, or what size >>> to use, again this seems to fit well expectations) >>> >>> The PTP doesn't specify a particular bus to put the device. So I added >>> it on the system bus directly, so it could hopefully be used easily on >>> a different platform than x86. Currently, it fails to init on piix, >>> because error_on_sysbus_device() check. The check may be changed in a >>> near future, see discussion on the qemu-devel ML. >>> >>> Tested with some success with Linux upstream and Windows 10, seabios & >>> modified ovmf. The device is recognized and correctly transmit >>> command/response with passthrough & emu. However, we are missing PPI >>> ACPI part atm. >>> >>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >>> --- >>> qapi/tpm.json | 5 +- >>> include/hw/acpi/tpm.h | 72 ++++++++ >>> include/sysemu/tpm.h | 3 + >>> hw/i386/acpi-build.c | 34 +++- >>> hw/tpm/tpm_crb.c | 327 >>> +++++++++++++++++++++++++++++++++++++ >>> default-configs/i386-softmmu.mak | 1 + >>> default-configs/x86_64-softmmu.mak | 1 + >>> hw/tpm/Makefile.objs | 1 + >>> 8 files changed, 434 insertions(+), 10 deletions(-) >>> create mode 100644 hw/tpm/tpm_crb.c >>> >>> diff --git a/qapi/tpm.json b/qapi/tpm.json >>> index 7093f268fb..d50deef5e9 100644 >>> --- a/qapi/tpm.json >>> +++ b/qapi/tpm.json >>> @@ -11,10 +11,11 @@ >>> # An enumeration of TPM models >>> # >>> # @tpm-tis: TPM TIS model >>> +# @tpm-crb: TPM CRB model (since 2.12) >>> # >>> # Since: 1.5 >>> ## >>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >>> ## >>> # @query-tpm-models: >>> @@ -28,7 +29,7 @@ >>> # Example: >>> # >>> # -> { "execute": "query-tpm-models" } >>> -# <- { "return": [ "tpm-tis" ] } >>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >>> # >>> ## >>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >>> index 6d516c6a7f..b0048515fa 100644 >>> --- a/include/hw/acpi/tpm.h >>> +++ b/include/hw/acpi/tpm.h >>> @@ -16,11 +16,82 @@ >>> #ifndef HW_ACPI_TPM_H >>> #define HW_ACPI_TPM_H >>> +#include "qemu/osdep.h" >>> + >>> #define TPM_TIS_ADDR_BASE 0xFED40000 >>> #define TPM_TIS_ADDR_SIZE 0x5000 >>> #define TPM_TIS_IRQ 5 >>> +struct crb_regs { >>> + union { >>> + uint32_t reg; >>> + struct { >>> + unsigned tpm_established:1; >>> + unsigned loc_assigned:1; >>> + unsigned active_locality:3; >>> + unsigned reserved:2; >>> + unsigned tpm_reg_valid_sts:1; >>> + } bits; >> >> I suppose this is little-endian layout, so this won't work on big-endian >> hosts. >> >> Can you add a qtest? >> >>> + } loc_state; >>> + uint32_t reserved1; >>> + uint32_t loc_ctrl; >>> + union { >>> + uint32_t reg; >>> + struct { >>> + unsigned granted:1; >>> + unsigned been_seized:1; >>> + } bits; >> >> >> This is unclear where you expect those bits (right/left aligned). >> >> Can you add an unnamed field to be more explicit? >> >> i.e. without using struct if left alignment expected: >> >> unsigned granted:1, been_seized:1, :30; > > > > I got rid of all the bitfields and this patch here makes it work on a ppc64 > big endian and also x86_64 host: > > https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd > Thank you Stefan! I am all for squashing this fix to the patch. You should then add your signed-off to the commit. > Regards, > Stefan > > > >> >>> + } loc_sts; >>> + uint8_t reserved2[32]; >>> + union { >>> + uint64_t reg; >>> + struct { >>> + unsigned type:4; >>> + unsigned version:4; >>> + unsigned cap_locality:1; >>> + unsigned cap_crb_idle_bypass:1; >>> + unsigned reserved1:1; >>> + unsigned cap_data_xfer_size_support:2; >>> + unsigned cap_fifo:1; >>> + unsigned cap_crb:1; >>> + unsigned cap_if_res:2; >>> + unsigned if_selector:2; >>> + unsigned if_selector_lock:1; >>> + unsigned reserved2:4; >>> + unsigned rid:8; >>> + unsigned vid:16; >>> + unsigned did:16; >>> + } bits; >>> + } intf_id; >>> + uint64_t ctrl_ext; >>> + >>> + uint32_t ctrl_req; >>> + union { >>> + uint32_t reg; >>> + struct { >>> + unsigned tpm_sts:1; >>> + unsigned tpm_idle:1; >>> + unsigned reserved:30; >> >> Oh here you use 'reserved' to left align. >> >>> + } bits; >>> + } ctrl_sts; >>> + uint32_t ctrl_cancel; >>> + uint32_t ctrl_start; >>> + uint32_t ctrl_int_enable; >>> + uint32_t ctrl_int_sts; >>> + uint32_t ctrl_cmd_size; >>> + uint32_t ctrl_cmd_pa_low; >>> + uint32_t ctrl_cmd_pa_high; >>> + uint32_t ctrl_rsp_size; >>> + uint64_t ctrl_rsp_pa; >>> + uint8_t reserved3[0x10]; >>> +} QEMU_PACKED; >>> + >>> +#define TPM_CRB_ADDR_BASE 0xFED40000 >>> +#define TPM_CRB_ADDR_SIZE 0x1000 >>> +#define TPM_CRB_ADDR_CTRL \ >>> + (TPM_CRB_ADDR_BASE + offsetof(struct crb_regs, ctrl_req)) >>> + >>> #define TPM_LOG_AREA_MINIMUM_SIZE (64 * 1024) >>> #define TPM_TCPA_ACPI_CLASS_CLIENT 0 >>> @@ -30,5 +101,6 @@ >>> #define TPM2_ACPI_CLASS_SERVER 1 >>> #define TPM2_START_METHOD_MMIO 6 >>> +#define TPM2_START_METHOD_CRB 7 >>> #endif /* HW_ACPI_TPM_H */ >>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h >>> index ac04a9d4a2..233b1a3fc3 100644 >>> --- a/include/sysemu/tpm.h >>> +++ b/include/sysemu/tpm.h >>> @@ -46,9 +46,12 @@ typedef struct TPMIfClass { >>> } TPMIfClass; >>> #define TYPE_TPM_TIS "tpm-tis" >>> +#define TYPE_TPM_CRB "tpm-crb" >>> #define TPM_IS_TIS(chr) \ >>> object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS) >>> +#define TPM_IS_CRB(chr) \ >>> + object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB) >>> /* returns NULL unless there is exactly one TPM device */ >>> static inline TPMIf *tpm_find(void) >>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >>> index dc4b2b9ffe..ed78c4ed9f 100644 >>> --- a/hw/i386/acpi-build.c >>> +++ b/hw/i386/acpi-build.c >>> @@ -2224,6 +2224,22 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, >>> aml_append(sb_scope, scope); >>> } >>> } >>> + >>> + if (TPM_IS_CRB(tpm_find())) { >>> + dev = aml_device("TPM"); >>> + aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101"))); >>> + crs = aml_resource_template(); >>> + aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, >>> + TPM_CRB_ADDR_SIZE, >>> AML_READ_WRITE)); >>> + aml_append(dev, aml_name_decl("_CRS", crs)); >>> + >>> + method = aml_method("_STA", 0, AML_NOTSERIALIZED); >>> + aml_append(method, aml_return(aml_int(0x0f))); >>> + aml_append(dev, method); >>> + >>> + aml_append(sb_scope, dev); >>> + } >>> + >>> aml_append(dsdt, sb_scope); >>> /* copy AML table into ACPI tables blob and patch header there */ >>> @@ -2285,18 +2301,20 @@ build_tpm2(GArray *table_data, BIOSLinker >>> *linker, GArray *tcpalog) >>> if (TPM_IS_TIS(tpm_find())) { >>> tpm2_ptr->control_area_address = cpu_to_le64(0); >>> tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO); >>> - >>> - tpm2_ptr->log_area_minimum_length = >>> - cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); >>> - >>> - /* log area start address to be filled by Guest linker */ >>> - bios_linker_loader_add_pointer(linker, >>> - ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size, >>> - ACPI_BUILD_TPMLOG_FILE, 0); >>> + } else if (TPM_IS_CRB(tpm_find())) { >>> + tpm2_ptr->control_area_address = cpu_to_le64(TPM_CRB_ADDR_CTRL); >>> + tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_CRB); >>> } else { >>> g_warn_if_reached(); >>> } >>> + tpm2_ptr->log_area_minimum_length = >>> + cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); >>> + >>> + /* log area start address to be filled by Guest linker */ >>> + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, >>> + log_addr_offset, log_addr_size, >>> + ACPI_BUILD_TPMLOG_FILE, 0); >>> build_header(linker, table_data, >>> (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, >>> NULL); >>> } >>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c >>> new file mode 100644 >>> index 0000000000..908ca18d92 >>> --- /dev/null >>> +++ b/hw/tpm/tpm_crb.c >>> @@ -0,0 +1,327 @@ >>> +/* >>> + * tpm_crb.c - QEMU's TPM CRB interface emulator >>> + * >>> + * Copyright (c) 2017 Red Hat, Inc. >>> + * >>> + * Authors: >>> + * Marc-André Lureau <marcandre.lureau@redhat.com> >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 or >>> later. >>> + * See the COPYING file in the top-level directory. >>> + * >>> + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>> Interface >>> + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification >>> + * Family “2.0” Level 00 Revision 01.03 v22 >>> + */ >>> + >>> +#include "qemu/osdep.h" >>> + >>> +#include "qemu-common.h" >>> +#include "qapi/error.h" >>> +#include "hw/sysbus.h" >>> +#include "exec/address-spaces.h" >>> + >>> +#include "hw/pci/pci_ids.h" >>> +#include "hw/acpi/tpm.h" >>> +#include "sysemu/tpm_backend.h" >>> +#include "tpm_int.h" >>> +#include "tpm_util.h" >>> + >>> +typedef struct CRBState { >>> + SysBusDevice parent_obj; >>> + >>> + TPMBackend *tpmbe; >>> + TPMBackendCmd cmd; >>> + struct crb_regs regs; >>> + MemoryRegion mmio; >>> + MemoryRegion cmdmem; >>> + >>> + size_t be_buffer_size; >>> +} CRBState; >>> + >>> +#define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB) >>> + >>> +#define DEBUG_CRB 0 >>> + >>> +#define DPRINTF(fmt, ...) do { \ >>> + if (DEBUG_CRB) { \ >>> + printf(fmt, ## __VA_ARGS__); \ >>> + } \ >>> + } while (0) >>> + >>> +#define CRB_ADDR_LOC_STATE offsetof(struct crb_regs, loc_state) >>> +#define CRB_ADDR_LOC_CTRL offsetof(struct crb_regs, loc_ctrl) >>> +#define CRB_ADDR_CTRL_REQ offsetof(struct crb_regs, ctrl_req) >>> +#define CRB_ADDR_CTRL_CANCEL offsetof(struct crb_regs, ctrl_cancel) >>> +#define CRB_ADDR_CTRL_START offsetof(struct crb_regs, ctrl_start) >>> + >>> +#define CRB_INTF_TYPE_CRB_ACTIVE 0b1 >>> +#define CRB_INTF_VERSION_CRB 0b1 >>> +#define CRB_INTF_CAP_LOCALITY_0_ONLY 0b0 >>> +#define CRB_INTF_CAP_IDLE_FAST 0b0 >>> +#define CRB_INTF_CAP_XFER_SIZE_64 0b11 >>> +#define CRB_INTF_CAP_FIFO_NOT_SUPPORTED 0b0 >>> +#define CRB_INTF_CAP_CRB_SUPPORTED 0b1 >>> +#define CRB_INTF_IF_SELECTOR_CRB 0b1 >>> +#define CRB_INTF_IF_SELECTOR_UNLOCKED 0b0 >>> + >>> +#define CRB_CTRL_CMD_SIZE (TPM_CRB_ADDR_SIZE - sizeof(struct crb_regs)) >>> + >>> +enum crb_loc_ctrl { >>> + CRB_LOC_CTRL_REQUEST_ACCESS = BIT(0), >>> + CRB_LOC_CTRL_RELINQUISH = BIT(1), >>> + CRB_LOC_CTRL_SEIZE = BIT(2), >>> + CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT = BIT(3), >>> +}; >>> + >>> +enum crb_ctrl_req { >>> + CRB_CTRL_REQ_CMD_READY = BIT(0), >>> + CRB_CTRL_REQ_GO_IDLE = BIT(1), >>> +}; >>> + >>> +enum crb_start { >>> + CRB_START_INVOKE = BIT(0), >>> +}; >>> + >>> +enum crb_cancel { >>> + CRB_CANCEL_INVOKE = BIT(0), >>> +}; >>> + >>> +static const char *addr_desc(unsigned off) >>> +{ >>> + struct crb_regs crb; >>> + >>> + switch (off) { >>> +#define CASE(field) \ >>> + case offsetof(struct crb_regs, field) ... \ >>> + offsetof(struct crb_regs, field) + sizeof(crb.field) - 1: \ >>> + return G_STRINGIFY(field); >>> + CASE(loc_state); >>> + CASE(reserved1); >>> + CASE(loc_ctrl); >>> + CASE(loc_sts); >>> + CASE(reserved2); >>> + CASE(intf_id); >>> + CASE(ctrl_ext); >>> + CASE(ctrl_req); >>> + CASE(ctrl_sts); >>> + CASE(ctrl_cancel); >>> + CASE(ctrl_start); >>> + CASE(ctrl_int_enable); >>> + CASE(ctrl_int_sts); >>> + CASE(ctrl_cmd_size); >>> + CASE(ctrl_cmd_pa_low); >>> + CASE(ctrl_cmd_pa_high); >>> + CASE(ctrl_rsp_size); >>> + CASE(ctrl_rsp_pa); >>> +#undef CASE >>> + } >>> + return NULL; >>> +} >>> + >>> +static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, >>> + unsigned size) >>> +{ >>> + CRBState *s = CRB(opaque); >>> + DPRINTF("CRB read 0x%lx:%s len:%u\n", >>> + addr, addr_desc(addr), size); >>> + void *regs = (void *)&s->regs + addr; >>> + >>> + switch (size) { >>> + case 1: >>> + return *(uint8_t *)regs; >>> + case 2: >>> + return *(uint16_t *)regs; >>> + case 4: >>> + return *(uint32_t *)regs; >>> + default: >>> + g_return_val_if_reached(-1); >>> + } >>> +} >>> + >>> +static void tpm_crb_mmio_write(void *opaque, hwaddr addr, >>> + uint64_t val, unsigned size) >>> +{ >>> + CRBState *s = CRB(opaque); >>> + DPRINTF("CRB write 0x%lx:%s len:%u val:%lu\n", >>> + addr, addr_desc(addr), size, val); >>> + >>> + switch (addr) { >>> + case CRB_ADDR_CTRL_REQ: >>> + switch (val) { >>> + case CRB_CTRL_REQ_CMD_READY: >>> + s->regs.ctrl_sts.bits.tpm_idle = 0; >>> + break; >>> + case CRB_CTRL_REQ_GO_IDLE: >>> + s->regs.ctrl_sts.bits.tpm_idle = 1; >>> + break; >>> + } >>> + break; >>> + case CRB_ADDR_CTRL_CANCEL: >>> + if (val == CRB_CANCEL_INVOKE && s->regs.ctrl_start & >>> CRB_START_INVOKE) { >>> + tpm_backend_cancel_cmd(s->tpmbe); >>> + } >>> + break; >>> + case CRB_ADDR_CTRL_START: >>> + if (val == CRB_START_INVOKE && >>> + !(s->regs.ctrl_start & CRB_START_INVOKE)) { >>> + void *mem = memory_region_get_ram_ptr(&s->cmdmem); >>> + >>> + s->regs.ctrl_start |= CRB_START_INVOKE; >>> + s->cmd = (TPMBackendCmd) { >>> + .in = mem, >>> + .in_len = MIN(tpm_cmd_get_size(mem), s->be_buffer_size), >>> + .out = mem, >>> + .out_len = s->be_buffer_size, >>> + }; >>> + >>> + tpm_backend_deliver_request(s->tpmbe, &s->cmd); >>> + } >>> + break; >>> + case CRB_ADDR_LOC_CTRL: >>> + switch (val) { >>> + case CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT: >>> + /* not loc 3 or 4 */ >>> + break; >>> + case CRB_LOC_CTRL_RELINQUISH: >>> + break; >>> + case CRB_LOC_CTRL_REQUEST_ACCESS: >>> + s->regs.loc_sts.bits.granted = 1; >>> + s->regs.loc_sts.bits.been_seized = 0; >>> + s->regs.loc_state.bits.loc_assigned = 1; >>> + s->regs.loc_state.bits.tpm_reg_valid_sts = 1; >>> + break; >>> + } >>> + break; >>> + } >>> +} >>> + >>> +static const MemoryRegionOps tpm_crb_memory_ops = { >>> + .read = tpm_crb_mmio_read, >>> + .write = tpm_crb_mmio_write, >>> + .endianness = DEVICE_LITTLE_ENDIAN, >>> + .valid = { >>> + .min_access_size = 1, >>> + .max_access_size = 4, >>> + }, >>> +}; >>> + >>> +static void tpm_crb_reset(DeviceState *dev) >>> +{ >>> + CRBState *s = CRB(dev); >>> + >>> + tpm_backend_reset(s->tpmbe); >>> + >>> + s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), >>> + CRB_CTRL_CMD_SIZE); >>> + >>> + s->regs = (struct crb_regs) { >>> + .intf_id.bits = { >>> + .type = CRB_INTF_TYPE_CRB_ACTIVE, >>> + .version = CRB_INTF_VERSION_CRB, >>> + .cap_locality = CRB_INTF_CAP_LOCALITY_0_ONLY, >>> + .cap_crb_idle_bypass = CRB_INTF_CAP_IDLE_FAST, >>> + .cap_data_xfer_size_support = CRB_INTF_CAP_XFER_SIZE_64, >>> + .cap_fifo = CRB_INTF_CAP_FIFO_NOT_SUPPORTED, >>> + .cap_crb = CRB_INTF_CAP_CRB_SUPPORTED, >>> + .cap_if_res = 0b0, >>> + .if_selector = CRB_INTF_IF_SELECTOR_CRB, >>> + .if_selector_lock = CRB_INTF_IF_SELECTOR_UNLOCKED, >>> + .rid = 0b0001, >>> + .vid = PCI_VENDOR_ID_IBM, >>> + .did = 0b0001, >>> + }, >>> + .ctrl_cmd_size = CRB_CTRL_CMD_SIZE, >>> + .ctrl_cmd_pa_low = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), >>> + .ctrl_rsp_size = CRB_CTRL_CMD_SIZE, >>> + .ctrl_rsp_pa = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), >>> + }; >>> + >>> + tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size); >>> +} >>> + >>> +static void tpm_crb_request_completed(TPMIf *ti, int ret) >>> +{ >>> + CRBState *s = CRB(ti); >>> + >>> + s->regs.ctrl_start &= ~CRB_START_INVOKE; >>> + if (ret != 0) { >>> + s->regs.ctrl_sts.bits.tpm_sts = 1; /* fatal error */ >>> + } >>> +} >>> + >>> +static enum TPMVersion tpm_crb_get_version(TPMIf *ti) >>> +{ >>> + CRBState *s = CRB(ti); >>> + >>> + return tpm_backend_get_tpm_version(s->tpmbe); >>> +} >>> + >>> +static const VMStateDescription vmstate_tpm_crb = { >>> + .name = "tpm-crb", >>> + .unmigratable = 1, >>> +}; >>> + >>> +static Property tpm_crb_properties[] = { >>> + DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe), >>> + DEFINE_PROP_END_OF_LIST(), >>> +}; >>> + >>> +static void tpm_crb_realizefn(DeviceState *dev, Error **errp) >>> +{ >>> + CRBState *s = CRB(dev); >>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); >>> + >>> + if (!tpm_find()) { >>> + error_setg(errp, "at most one TPM device is permitted"); >>> + return; >>> + } >>> + if (!s->tpmbe) { >>> + error_setg(errp, "'tpmdev' property is required"); >>> + return; >>> + } >>> + >>> + memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s, >>> + "tpm-crb-mmio", sizeof(struct crb_regs)); >>> + memory_region_init_ram(&s->cmdmem, OBJECT(s), >>> + "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); >>> + >>> + sysbus_init_mmio(sbd, &s->mmio); >>> + sysbus_mmio_map(sbd, 0, TPM_CRB_ADDR_BASE); >>> + /* allocate ram in bios instead? */ >>> + memory_region_add_subregion(get_system_memory(), >>> + TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), &s->cmdmem); >>> +} >>> + >>> +static void tpm_crb_class_init(ObjectClass *klass, void *data) >>> +{ >>> + DeviceClass *dc = DEVICE_CLASS(klass); >>> + TPMIfClass *tc = TPM_IF_CLASS(klass); >>> + >>> + dc->realize = tpm_crb_realizefn; >>> + dc->props = tpm_crb_properties; >>> + dc->reset = tpm_crb_reset; >>> + dc->vmsd = &vmstate_tpm_crb; >>> + dc->user_creatable = true; >>> + tc->model = TPM_MODEL_TPM_CRB; >>> + tc->get_version = tpm_crb_get_version; >>> + tc->request_completed = tpm_crb_request_completed; >>> +} >>> + >>> +static const TypeInfo tpm_crb_info = { >>> + .name = TYPE_TPM_CRB, >>> + .parent = TYPE_SYS_BUS_DEVICE, >>> + .instance_size = sizeof(CRBState), >>> + .class_init = tpm_crb_class_init, >>> + .interfaces = (InterfaceInfo[]) { >>> + { TYPE_TPM_IF }, >>> + { } >>> + } >>> +}; >>> + >>> +static void tpm_crb_register(void) >>> +{ >>> + type_register_static(&tpm_crb_info); >>> +} >>> + >>> +type_init(tpm_crb_register) >>> diff --git a/default-configs/i386-softmmu.mak >>> b/default-configs/i386-softmmu.mak >>> index 95ac4b464a..ac27700e79 100644 >>> --- a/default-configs/i386-softmmu.mak >>> +++ b/default-configs/i386-softmmu.mak >>> @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y >>> CONFIG_I8259=y >>> CONFIG_PFLASH_CFI01=y >>> CONFIG_TPM_TIS=$(CONFIG_TPM) >>> +CONFIG_TPM_CRB=$(CONFIG_TPM) >>> CONFIG_MC146818RTC=y >>> CONFIG_PCI_PIIX=y >>> CONFIG_WDT_IB700=y >>> diff --git a/default-configs/x86_64-softmmu.mak >>> b/default-configs/x86_64-softmmu.mak >>> index 0221236825..b2104ade19 100644 >>> --- a/default-configs/x86_64-softmmu.mak >>> +++ b/default-configs/x86_64-softmmu.mak >>> @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y >>> CONFIG_I8259=y >>> CONFIG_PFLASH_CFI01=y >>> CONFIG_TPM_TIS=$(CONFIG_TPM) >>> +CONFIG_TPM_CRB=$(CONFIG_TPM) >>> CONFIG_MC146818RTC=y >>> CONFIG_PCI_PIIX=y >>> CONFIG_WDT_IB700=y >>> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs >>> index 7a93b24636..1dc9f8bf2c 100644 >>> --- a/hw/tpm/Makefile.objs >>> +++ b/hw/tpm/Makefile.objs >>> @@ -1,4 +1,5 @@ >>> common-obj-y += tpm_util.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 >>> common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o >>> >
On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: > Hi > > On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: >> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >>>> Specification Family “2.0” Level 00 Revision 01.03 v22. >>>> >>>> The PTP allows device implementation to switch between TIS and CRB >>>> model at run time, but given that CRB is a simpler device to >>>> implement, I chose to implement it as a different device. >>>> >>>> The device doesn't implement other locality than 0 for now (my laptop >>>> TPM doesn't either, so I assume this isn't so bad) >>>> >>>> The command/reply memory region is statically allocated after the CRB >>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >>>> wonder if the BIOS could or should allocate it instead, or what size >>>> to use, again this seems to fit well expectations) >>>> >>>> The PTP doesn't specify a particular bus to put the device. So I added >>>> it on the system bus directly, so it could hopefully be used easily on >>>> a different platform than x86. Currently, it fails to init on piix, >>>> because error_on_sysbus_device() check. The check may be changed in a >>>> near future, see discussion on the qemu-devel ML. >>>> >>>> Tested with some success with Linux upstream and Windows 10, seabios & >>>> modified ovmf. The device is recognized and correctly transmit >>>> command/response with passthrough & emu. However, we are missing PPI >>>> ACPI part atm. >>>> >>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >>>> --- >>>> qapi/tpm.json | 5 +- >>>> include/hw/acpi/tpm.h | 72 ++++++++ >>>> include/sysemu/tpm.h | 3 + >>>> hw/i386/acpi-build.c | 34 +++- >>>> hw/tpm/tpm_crb.c | 327 >>>> +++++++++++++++++++++++++++++++++++++ >>>> default-configs/i386-softmmu.mak | 1 + >>>> default-configs/x86_64-softmmu.mak | 1 + >>>> hw/tpm/Makefile.objs | 1 + >>>> 8 files changed, 434 insertions(+), 10 deletions(-) >>>> create mode 100644 hw/tpm/tpm_crb.c >>>> >>>> diff --git a/qapi/tpm.json b/qapi/tpm.json >>>> index 7093f268fb..d50deef5e9 100644 >>>> --- a/qapi/tpm.json >>>> +++ b/qapi/tpm.json >>>> @@ -11,10 +11,11 @@ >>>> # An enumeration of TPM models >>>> # >>>> # @tpm-tis: TPM TIS model >>>> +# @tpm-crb: TPM CRB model (since 2.12) >>>> # >>>> # Since: 1.5 >>>> ## >>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >>>> ## >>>> # @query-tpm-models: >>>> @@ -28,7 +29,7 @@ >>>> # Example: >>>> # >>>> # -> { "execute": "query-tpm-models" } >>>> -# <- { "return": [ "tpm-tis" ] } >>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >>>> # >>>> ## >>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >>>> index 6d516c6a7f..b0048515fa 100644 >>>> --- a/include/hw/acpi/tpm.h >>>> +++ b/include/hw/acpi/tpm.h >>>> @@ -16,11 +16,82 @@ >>>> #ifndef HW_ACPI_TPM_H >>>> #define HW_ACPI_TPM_H >>>> +#include "qemu/osdep.h" >>>> + >>>> #define TPM_TIS_ADDR_BASE 0xFED40000 >>>> #define TPM_TIS_ADDR_SIZE 0x5000 >>>> #define TPM_TIS_IRQ 5 >>>> +struct crb_regs { >>>> + union { >>>> + uint32_t reg; >>>> + struct { >>>> + unsigned tpm_established:1; >>>> + unsigned loc_assigned:1; >>>> + unsigned active_locality:3; >>>> + unsigned reserved:2; >>>> + unsigned tpm_reg_valid_sts:1; >>>> + } bits; >>> I suppose this is little-endian layout, so this won't work on big-endian >>> hosts. >>> >>> Can you add a qtest? >>> >>>> + } loc_state; >>>> + uint32_t reserved1; >>>> + uint32_t loc_ctrl; >>>> + union { >>>> + uint32_t reg; >>>> + struct { >>>> + unsigned granted:1; >>>> + unsigned been_seized:1; >>>> + } bits; >>> >>> This is unclear where you expect those bits (right/left aligned). >>> >>> Can you add an unnamed field to be more explicit? >>> >>> i.e. without using struct if left alignment expected: >>> >>> unsigned granted:1, been_seized:1, :30; >> >> >> I got rid of all the bitfields and this patch here makes it work on a ppc64 >> big endian and also x86_64 host: >> >> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd >> > Thank you Stefan! I am all for squashing this fix to the patch. You > should then add your signed-off to the commit. I'll do that. The TIS is an ISA Device and the CRB is similar. Considering the complications with the sysbus devices where one has to explicitly allow it for a certain machine type, I would advocate to convert the CRB to an ISA device. A patch that does that is this one: https://github.com/stefanberger/qemu-tpm/commit/08c61bd666f82084c42621f4285bcac08fb4f713 Stefan
On 01/21/2018 02:46 AM, Stefan Berger wrote: > On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >>> Specification Family “2.0” Level 00 Revision 01.03 v22. >>> >>> The PTP allows device implementation to switch between TIS and CRB >>> model at run time, but given that CRB is a simpler device to >>> implement, I chose to implement it as a different device. >>> >>> The device doesn't implement other locality than 0 for now (my laptop >>> TPM doesn't either, so I assume this isn't so bad) >>> >>> The command/reply memory region is statically allocated after the CRB >>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >>> wonder if the BIOS could or should allocate it instead, or what size >>> to use, again this seems to fit well expectations) >>> >>> The PTP doesn't specify a particular bus to put the device. So I added >>> it on the system bus directly, so it could hopefully be used easily on >>> a different platform than x86. Currently, it fails to init on piix, >>> because error_on_sysbus_device() check. The check may be changed in a >>> near future, see discussion on the qemu-devel ML. >>> >>> Tested with some success with Linux upstream and Windows 10, seabios & >>> modified ovmf. The device is recognized and correctly transmit >>> command/response with passthrough & emu. However, we are missing PPI >>> ACPI part atm. >>> >>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >>> --- >>> qapi/tpm.json | 5 +- >>> include/hw/acpi/tpm.h | 72 ++++++++ >>> include/sysemu/tpm.h | 3 + >>> hw/i386/acpi-build.c | 34 +++- >>> hw/tpm/tpm_crb.c | 327 >>> +++++++++++++++++++++++++++++++++++++ >>> default-configs/i386-softmmu.mak | 1 + >>> default-configs/x86_64-softmmu.mak | 1 + >>> hw/tpm/Makefile.objs | 1 + >>> 8 files changed, 434 insertions(+), 10 deletions(-) >>> create mode 100644 hw/tpm/tpm_crb.c >>> >>> diff --git a/qapi/tpm.json b/qapi/tpm.json >>> index 7093f268fb..d50deef5e9 100644 >>> --- a/qapi/tpm.json >>> +++ b/qapi/tpm.json >>> @@ -11,10 +11,11 @@ >>> # An enumeration of TPM models >>> # >>> # @tpm-tis: TPM TIS model >>> +# @tpm-crb: TPM CRB model (since 2.12) >>> # >>> # Since: 1.5 >>> ## >>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >>> ## >>> # @query-tpm-models: >>> @@ -28,7 +29,7 @@ >>> # Example: >>> # >>> # -> { "execute": "query-tpm-models" } >>> -# <- { "return": [ "tpm-tis" ] } >>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >>> # >>> ## >>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >>> index 6d516c6a7f..b0048515fa 100644 >>> --- a/include/hw/acpi/tpm.h >>> +++ b/include/hw/acpi/tpm.h >>> @@ -16,11 +16,82 @@ >>> #ifndef HW_ACPI_TPM_H >>> #define HW_ACPI_TPM_H >>> +#include "qemu/osdep.h" >>> + >>> #define TPM_TIS_ADDR_BASE 0xFED40000 >>> #define TPM_TIS_ADDR_SIZE 0x5000 >>> #define TPM_TIS_IRQ 5 >>> +struct crb_regs { >>> + union { >>> + uint32_t reg; >>> + struct { >>> + unsigned tpm_established:1; >>> + unsigned loc_assigned:1; >>> + unsigned active_locality:3; >>> + unsigned reserved:2; >>> + unsigned tpm_reg_valid_sts:1; >>> + } bits; >> I suppose this is little-endian layout, so this won't work on big-endian >> hosts. >> >> Can you add a qtest? >> >>> + } loc_state; >>> + uint32_t reserved1; >>> + uint32_t loc_ctrl; >>> + union { >>> + uint32_t reg; >>> + struct { >>> + unsigned granted:1; >>> + unsigned been_seized:1; >>> + } bits; >> >> This is unclear where you expect those bits (right/left aligned). >> >> Can you add an unnamed field to be more explicit? >> >> i.e. without using struct if left alignment expected: >> >> unsigned granted:1, been_seized:1, :30; > > > I got rid of all the bitfields and this patch here makes it work on a > ppc64 big endian and also x86_64 host: > > https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd Thanks! Looking at your tree, I recommend you to have a look at the "hw/registerfields.h" API, you might find it simpler when it comes to add bitfields #defines and checks, moreover when you have specs available in a parsable form, since you can easily script and auto-generate. Alistair, thinking about it I'm not sure you already have a such template available, but if you do we might add it in the scripts/modules/ directory, then I can add doc and examples. > > > Regards, > Stefan > > >> >>> + } loc_sts; >>> + uint8_t reserved2[32]; >>> + union { >>> + uint64_t reg; >>> + struct { >>> + unsigned type:4; >>> + unsigned version:4; >>> + unsigned cap_locality:1; >>> + unsigned cap_crb_idle_bypass:1; >>> + unsigned reserved1:1; >>> + unsigned cap_data_xfer_size_support:2; >>> + unsigned cap_fifo:1; >>> + unsigned cap_crb:1; >>> + unsigned cap_if_res:2; >>> + unsigned if_selector:2; >>> + unsigned if_selector_lock:1; >>> + unsigned reserved2:4; >>> + unsigned rid:8; >>> + unsigned vid:16; >>> + unsigned did:16; >>> + } bits; >>> + } intf_id; >>> + uint64_t ctrl_ext; >>> + >>> + uint32_t ctrl_req; >>> + union { >>> + uint32_t reg; >>> + struct { >>> + unsigned tpm_sts:1; >>> + unsigned tpm_idle:1; >>> + unsigned reserved:30; >> Oh here you use 'reserved' to left align. >> >>> + } bits; >>> + } ctrl_sts; >>> + uint32_t ctrl_cancel; >>> + uint32_t ctrl_start; >>> + uint32_t ctrl_int_enable; >>> + uint32_t ctrl_int_sts; >>> + uint32_t ctrl_cmd_size; >>> + uint32_t ctrl_cmd_pa_low; >>> + uint32_t ctrl_cmd_pa_high; >>> + uint32_t ctrl_rsp_size; >>> + uint64_t ctrl_rsp_pa; >>> + uint8_t reserved3[0x10]; >>> +} QEMU_PACKED; >>> + >>> +#define TPM_CRB_ADDR_BASE 0xFED40000 >>> +#define TPM_CRB_ADDR_SIZE 0x1000 >>> +#define TPM_CRB_ADDR_CTRL \ >>> + (TPM_CRB_ADDR_BASE + offsetof(struct crb_regs, ctrl_req)) >>> + >>> #define TPM_LOG_AREA_MINIMUM_SIZE (64 * 1024) >>> #define TPM_TCPA_ACPI_CLASS_CLIENT 0 >>> @@ -30,5 +101,6 @@ >>> #define TPM2_ACPI_CLASS_SERVER 1 >>> #define TPM2_START_METHOD_MMIO 6 >>> +#define TPM2_START_METHOD_CRB 7 >>> #endif /* HW_ACPI_TPM_H */ >>> diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h >>> index ac04a9d4a2..233b1a3fc3 100644 >>> --- a/include/sysemu/tpm.h >>> +++ b/include/sysemu/tpm.h >>> @@ -46,9 +46,12 @@ typedef struct TPMIfClass { >>> } TPMIfClass; >>> #define TYPE_TPM_TIS "tpm-tis" >>> +#define TYPE_TPM_CRB "tpm-crb" >>> #define TPM_IS_TIS(chr) \ >>> object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS) >>> +#define TPM_IS_CRB(chr) \ >>> + object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB) >>> /* returns NULL unless there is exactly one TPM device */ >>> static inline TPMIf *tpm_find(void) >>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c >>> index dc4b2b9ffe..ed78c4ed9f 100644 >>> --- a/hw/i386/acpi-build.c >>> +++ b/hw/i386/acpi-build.c >>> @@ -2224,6 +2224,22 @@ build_dsdt(GArray *table_data, BIOSLinker >>> *linker, >>> aml_append(sb_scope, scope); >>> } >>> } >>> + >>> + if (TPM_IS_CRB(tpm_find())) { >>> + dev = aml_device("TPM"); >>> + aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101"))); >>> + crs = aml_resource_template(); >>> + aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, >>> + TPM_CRB_ADDR_SIZE, >>> AML_READ_WRITE)); >>> + aml_append(dev, aml_name_decl("_CRS", crs)); >>> + >>> + method = aml_method("_STA", 0, AML_NOTSERIALIZED); >>> + aml_append(method, aml_return(aml_int(0x0f))); >>> + aml_append(dev, method); >>> + >>> + aml_append(sb_scope, dev); >>> + } >>> + >>> aml_append(dsdt, sb_scope); >>> /* copy AML table into ACPI tables blob and patch header >>> there */ >>> @@ -2285,18 +2301,20 @@ build_tpm2(GArray *table_data, BIOSLinker >>> *linker, GArray *tcpalog) >>> if (TPM_IS_TIS(tpm_find())) { >>> tpm2_ptr->control_area_address = cpu_to_le64(0); >>> tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO); >>> - >>> - tpm2_ptr->log_area_minimum_length = >>> - cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); >>> - >>> - /* log area start address to be filled by Guest linker */ >>> - bios_linker_loader_add_pointer(linker, >>> - ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size, >>> - ACPI_BUILD_TPMLOG_FILE, 0); >>> + } else if (TPM_IS_CRB(tpm_find())) { >>> + tpm2_ptr->control_area_address = >>> cpu_to_le64(TPM_CRB_ADDR_CTRL); >>> + tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_CRB); >>> } else { >>> g_warn_if_reached(); >>> } >>> + tpm2_ptr->log_area_minimum_length = >>> + cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); >>> + >>> + /* log area start address to be filled by Guest linker */ >>> + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, >>> + log_addr_offset, log_addr_size, >>> + ACPI_BUILD_TPMLOG_FILE, 0); >>> build_header(linker, table_data, >>> (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, >>> NULL, NULL); >>> } >>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c >>> new file mode 100644 >>> index 0000000000..908ca18d92 >>> --- /dev/null >>> +++ b/hw/tpm/tpm_crb.c >>> @@ -0,0 +1,327 @@ >>> +/* >>> + * tpm_crb.c - QEMU's TPM CRB interface emulator >>> + * >>> + * Copyright (c) 2017 Red Hat, Inc. >>> + * >>> + * Authors: >>> + * Marc-André Lureau <marcandre.lureau@redhat.com> >>> + * >>> + * This work is licensed under the terms of the GNU GPL, version 2 >>> or later. >>> + * See the COPYING file in the top-level directory. >>> + * >>> + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>> Interface >>> + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification >>> + * Family “2.0” Level 00 Revision 01.03 v22 >>> + */ >>> + >>> +#include "qemu/osdep.h" >>> + >>> +#include "qemu-common.h" >>> +#include "qapi/error.h" >>> +#include "hw/sysbus.h" >>> +#include "exec/address-spaces.h" >>> + >>> +#include "hw/pci/pci_ids.h" >>> +#include "hw/acpi/tpm.h" >>> +#include "sysemu/tpm_backend.h" >>> +#include "tpm_int.h" >>> +#include "tpm_util.h" >>> + >>> +typedef struct CRBState { >>> + SysBusDevice parent_obj; >>> + >>> + TPMBackend *tpmbe; >>> + TPMBackendCmd cmd; >>> + struct crb_regs regs; >>> + MemoryRegion mmio; >>> + MemoryRegion cmdmem; >>> + >>> + size_t be_buffer_size; >>> +} CRBState; >>> + >>> +#define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB) >>> + >>> +#define DEBUG_CRB 0 >>> + >>> +#define DPRINTF(fmt, ...) do { \ >>> + if (DEBUG_CRB) { \ >>> + printf(fmt, ## __VA_ARGS__); \ >>> + } \ >>> + } while (0) >>> + >>> +#define CRB_ADDR_LOC_STATE offsetof(struct crb_regs, loc_state) >>> +#define CRB_ADDR_LOC_CTRL offsetof(struct crb_regs, loc_ctrl) >>> +#define CRB_ADDR_CTRL_REQ offsetof(struct crb_regs, ctrl_req) >>> +#define CRB_ADDR_CTRL_CANCEL offsetof(struct crb_regs, ctrl_cancel) >>> +#define CRB_ADDR_CTRL_START offsetof(struct crb_regs, ctrl_start) >>> + >>> +#define CRB_INTF_TYPE_CRB_ACTIVE 0b1 >>> +#define CRB_INTF_VERSION_CRB 0b1 >>> +#define CRB_INTF_CAP_LOCALITY_0_ONLY 0b0 >>> +#define CRB_INTF_CAP_IDLE_FAST 0b0 >>> +#define CRB_INTF_CAP_XFER_SIZE_64 0b11 >>> +#define CRB_INTF_CAP_FIFO_NOT_SUPPORTED 0b0 >>> +#define CRB_INTF_CAP_CRB_SUPPORTED 0b1 >>> +#define CRB_INTF_IF_SELECTOR_CRB 0b1 >>> +#define CRB_INTF_IF_SELECTOR_UNLOCKED 0b0 >>> + >>> +#define CRB_CTRL_CMD_SIZE (TPM_CRB_ADDR_SIZE - sizeof(struct crb_regs)) >>> + >>> +enum crb_loc_ctrl { >>> + CRB_LOC_CTRL_REQUEST_ACCESS = BIT(0), >>> + CRB_LOC_CTRL_RELINQUISH = BIT(1), >>> + CRB_LOC_CTRL_SEIZE = BIT(2), >>> + CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT = BIT(3), >>> +}; >>> + >>> +enum crb_ctrl_req { >>> + CRB_CTRL_REQ_CMD_READY = BIT(0), >>> + CRB_CTRL_REQ_GO_IDLE = BIT(1), >>> +}; >>> + >>> +enum crb_start { >>> + CRB_START_INVOKE = BIT(0), >>> +}; >>> + >>> +enum crb_cancel { >>> + CRB_CANCEL_INVOKE = BIT(0), >>> +}; >>> + >>> +static const char *addr_desc(unsigned off) >>> +{ >>> + struct crb_regs crb; >>> + >>> + switch (off) { >>> +#define CASE(field) \ >>> + case offsetof(struct crb_regs, field) ... \ >>> + offsetof(struct crb_regs, field) + sizeof(crb.field) - 1: \ >>> + return G_STRINGIFY(field); >>> + CASE(loc_state); >>> + CASE(reserved1); >>> + CASE(loc_ctrl); >>> + CASE(loc_sts); >>> + CASE(reserved2); >>> + CASE(intf_id); >>> + CASE(ctrl_ext); >>> + CASE(ctrl_req); >>> + CASE(ctrl_sts); >>> + CASE(ctrl_cancel); >>> + CASE(ctrl_start); >>> + CASE(ctrl_int_enable); >>> + CASE(ctrl_int_sts); >>> + CASE(ctrl_cmd_size); >>> + CASE(ctrl_cmd_pa_low); >>> + CASE(ctrl_cmd_pa_high); >>> + CASE(ctrl_rsp_size); >>> + CASE(ctrl_rsp_pa); >>> +#undef CASE >>> + } >>> + return NULL; >>> +} >>> + >>> +static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, >>> + unsigned size) >>> +{ >>> + CRBState *s = CRB(opaque); >>> + DPRINTF("CRB read 0x%lx:%s len:%u\n", >>> + addr, addr_desc(addr), size); >>> + void *regs = (void *)&s->regs + addr; >>> + >>> + switch (size) { >>> + case 1: >>> + return *(uint8_t *)regs; >>> + case 2: >>> + return *(uint16_t *)regs; >>> + case 4: >>> + return *(uint32_t *)regs; >>> + default: >>> + g_return_val_if_reached(-1); >>> + } >>> +} >>> + >>> +static void tpm_crb_mmio_write(void *opaque, hwaddr addr, >>> + uint64_t val, unsigned size) >>> +{ >>> + CRBState *s = CRB(opaque); >>> + DPRINTF("CRB write 0x%lx:%s len:%u val:%lu\n", >>> + addr, addr_desc(addr), size, val); >>> + >>> + switch (addr) { >>> + case CRB_ADDR_CTRL_REQ: >>> + switch (val) { >>> + case CRB_CTRL_REQ_CMD_READY: >>> + s->regs.ctrl_sts.bits.tpm_idle = 0; >>> + break; >>> + case CRB_CTRL_REQ_GO_IDLE: >>> + s->regs.ctrl_sts.bits.tpm_idle = 1; >>> + break; >>> + } >>> + break; >>> + case CRB_ADDR_CTRL_CANCEL: >>> + if (val == CRB_CANCEL_INVOKE && s->regs.ctrl_start & >>> CRB_START_INVOKE) { >>> + tpm_backend_cancel_cmd(s->tpmbe); >>> + } >>> + break; >>> + case CRB_ADDR_CTRL_START: >>> + if (val == CRB_START_INVOKE && >>> + !(s->regs.ctrl_start & CRB_START_INVOKE)) { >>> + void *mem = memory_region_get_ram_ptr(&s->cmdmem); >>> + >>> + s->regs.ctrl_start |= CRB_START_INVOKE; >>> + s->cmd = (TPMBackendCmd) { >>> + .in = mem, >>> + .in_len = MIN(tpm_cmd_get_size(mem), >>> s->be_buffer_size), >>> + .out = mem, >>> + .out_len = s->be_buffer_size, >>> + }; >>> + >>> + tpm_backend_deliver_request(s->tpmbe, &s->cmd); >>> + } >>> + break; >>> + case CRB_ADDR_LOC_CTRL: >>> + switch (val) { >>> + case CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT: >>> + /* not loc 3 or 4 */ >>> + break; >>> + case CRB_LOC_CTRL_RELINQUISH: >>> + break; >>> + case CRB_LOC_CTRL_REQUEST_ACCESS: >>> + s->regs.loc_sts.bits.granted = 1; >>> + s->regs.loc_sts.bits.been_seized = 0; >>> + s->regs.loc_state.bits.loc_assigned = 1; >>> + s->regs.loc_state.bits.tpm_reg_valid_sts = 1; >>> + break; >>> + } >>> + break; >>> + } >>> +} >>> + >>> +static const MemoryRegionOps tpm_crb_memory_ops = { >>> + .read = tpm_crb_mmio_read, >>> + .write = tpm_crb_mmio_write, >>> + .endianness = DEVICE_LITTLE_ENDIAN, >>> + .valid = { >>> + .min_access_size = 1, >>> + .max_access_size = 4, >>> + }, >>> +}; >>> + >>> +static void tpm_crb_reset(DeviceState *dev) >>> +{ >>> + CRBState *s = CRB(dev); >>> + >>> + tpm_backend_reset(s->tpmbe); >>> + >>> + s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), >>> + CRB_CTRL_CMD_SIZE); >>> + >>> + s->regs = (struct crb_regs) { >>> + .intf_id.bits = { >>> + .type = CRB_INTF_TYPE_CRB_ACTIVE, >>> + .version = CRB_INTF_VERSION_CRB, >>> + .cap_locality = CRB_INTF_CAP_LOCALITY_0_ONLY, >>> + .cap_crb_idle_bypass = CRB_INTF_CAP_IDLE_FAST, >>> + .cap_data_xfer_size_support = CRB_INTF_CAP_XFER_SIZE_64, >>> + .cap_fifo = CRB_INTF_CAP_FIFO_NOT_SUPPORTED, >>> + .cap_crb = CRB_INTF_CAP_CRB_SUPPORTED, >>> + .cap_if_res = 0b0, >>> + .if_selector = CRB_INTF_IF_SELECTOR_CRB, >>> + .if_selector_lock = CRB_INTF_IF_SELECTOR_UNLOCKED, >>> + .rid = 0b0001, >>> + .vid = PCI_VENDOR_ID_IBM, >>> + .did = 0b0001, >>> + }, >>> + .ctrl_cmd_size = CRB_CTRL_CMD_SIZE, >>> + .ctrl_cmd_pa_low = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), >>> + .ctrl_rsp_size = CRB_CTRL_CMD_SIZE, >>> + .ctrl_rsp_pa = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), >>> + }; >>> + >>> + tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size); >>> +} >>> + >>> +static void tpm_crb_request_completed(TPMIf *ti, int ret) >>> +{ >>> + CRBState *s = CRB(ti); >>> + >>> + s->regs.ctrl_start &= ~CRB_START_INVOKE; >>> + if (ret != 0) { >>> + s->regs.ctrl_sts.bits.tpm_sts = 1; /* fatal error */ >>> + } >>> +} >>> + >>> +static enum TPMVersion tpm_crb_get_version(TPMIf *ti) >>> +{ >>> + CRBState *s = CRB(ti); >>> + >>> + return tpm_backend_get_tpm_version(s->tpmbe); >>> +} >>> + >>> +static const VMStateDescription vmstate_tpm_crb = { >>> + .name = "tpm-crb", >>> + .unmigratable = 1, >>> +}; >>> + >>> +static Property tpm_crb_properties[] = { >>> + DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe), >>> + DEFINE_PROP_END_OF_LIST(), >>> +}; >>> + >>> +static void tpm_crb_realizefn(DeviceState *dev, Error **errp) >>> +{ >>> + CRBState *s = CRB(dev); >>> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); >>> + >>> + if (!tpm_find()) { >>> + error_setg(errp, "at most one TPM device is permitted"); >>> + return; >>> + } >>> + if (!s->tpmbe) { >>> + error_setg(errp, "'tpmdev' property is required"); >>> + return; >>> + } >>> + >>> + memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s, >>> + "tpm-crb-mmio", sizeof(struct crb_regs)); >>> + memory_region_init_ram(&s->cmdmem, OBJECT(s), >>> + "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); >>> + >>> + sysbus_init_mmio(sbd, &s->mmio); >>> + sysbus_mmio_map(sbd, 0, TPM_CRB_ADDR_BASE); >>> + /* allocate ram in bios instead? */ >>> + memory_region_add_subregion(get_system_memory(), >>> + TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), &s->cmdmem); >>> +} >>> + >>> +static void tpm_crb_class_init(ObjectClass *klass, void *data) >>> +{ >>> + DeviceClass *dc = DEVICE_CLASS(klass); >>> + TPMIfClass *tc = TPM_IF_CLASS(klass); >>> + >>> + dc->realize = tpm_crb_realizefn; >>> + dc->props = tpm_crb_properties; >>> + dc->reset = tpm_crb_reset; >>> + dc->vmsd = &vmstate_tpm_crb; >>> + dc->user_creatable = true; >>> + tc->model = TPM_MODEL_TPM_CRB; >>> + tc->get_version = tpm_crb_get_version; >>> + tc->request_completed = tpm_crb_request_completed; >>> +} >>> + >>> +static const TypeInfo tpm_crb_info = { >>> + .name = TYPE_TPM_CRB, >>> + .parent = TYPE_SYS_BUS_DEVICE, >>> + .instance_size = sizeof(CRBState), >>> + .class_init = tpm_crb_class_init, >>> + .interfaces = (InterfaceInfo[]) { >>> + { TYPE_TPM_IF }, >>> + { } >>> + } >>> +}; >>> + >>> +static void tpm_crb_register(void) >>> +{ >>> + type_register_static(&tpm_crb_info); >>> +} >>> + >>> +type_init(tpm_crb_register) >>> diff --git a/default-configs/i386-softmmu.mak >>> b/default-configs/i386-softmmu.mak >>> index 95ac4b464a..ac27700e79 100644 >>> --- a/default-configs/i386-softmmu.mak >>> +++ b/default-configs/i386-softmmu.mak >>> @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y >>> CONFIG_I8259=y >>> CONFIG_PFLASH_CFI01=y >>> CONFIG_TPM_TIS=$(CONFIG_TPM) >>> +CONFIG_TPM_CRB=$(CONFIG_TPM) >>> CONFIG_MC146818RTC=y >>> CONFIG_PCI_PIIX=y >>> CONFIG_WDT_IB700=y >>> diff --git a/default-configs/x86_64-softmmu.mak >>> b/default-configs/x86_64-softmmu.mak >>> index 0221236825..b2104ade19 100644 >>> --- a/default-configs/x86_64-softmmu.mak >>> +++ b/default-configs/x86_64-softmmu.mak >>> @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y >>> CONFIG_I8259=y >>> CONFIG_PFLASH_CFI01=y >>> CONFIG_TPM_TIS=$(CONFIG_TPM) >>> +CONFIG_TPM_CRB=$(CONFIG_TPM) >>> CONFIG_MC146818RTC=y >>> CONFIG_PCI_PIIX=y >>> CONFIG_WDT_IB700=y >>> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs >>> index 7a93b24636..1dc9f8bf2c 100644 >>> --- a/hw/tpm/Makefile.objs >>> +++ b/hw/tpm/Makefile.objs >>> @@ -1,4 +1,5 @@ >>> common-obj-y += tpm_util.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 >>> common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o >>> >
Hi On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: >> >> Hi >> >> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger >> <stefanb@linux.vnet.ibm.com> wrote: >>> >>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >>>> >>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >>>>> >>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. >>>>> >>>>> The PTP allows device implementation to switch between TIS and CRB >>>>> model at run time, but given that CRB is a simpler device to >>>>> implement, I chose to implement it as a different device. >>>>> >>>>> The device doesn't implement other locality than 0 for now (my laptop >>>>> TPM doesn't either, so I assume this isn't so bad) >>>>> >>>>> The command/reply memory region is statically allocated after the CRB >>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >>>>> wonder if the BIOS could or should allocate it instead, or what size >>>>> to use, again this seems to fit well expectations) >>>>> >>>>> The PTP doesn't specify a particular bus to put the device. So I added >>>>> it on the system bus directly, so it could hopefully be used easily on >>>>> a different platform than x86. Currently, it fails to init on piix, >>>>> because error_on_sysbus_device() check. The check may be changed in a >>>>> near future, see discussion on the qemu-devel ML. >>>>> >>>>> Tested with some success with Linux upstream and Windows 10, seabios & >>>>> modified ovmf. The device is recognized and correctly transmit >>>>> command/response with passthrough & emu. However, we are missing PPI >>>>> ACPI part atm. >>>>> >>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >>>>> --- >>>>> qapi/tpm.json | 5 +- >>>>> include/hw/acpi/tpm.h | 72 ++++++++ >>>>> include/sysemu/tpm.h | 3 + >>>>> hw/i386/acpi-build.c | 34 +++- >>>>> hw/tpm/tpm_crb.c | 327 >>>>> +++++++++++++++++++++++++++++++++++++ >>>>> default-configs/i386-softmmu.mak | 1 + >>>>> default-configs/x86_64-softmmu.mak | 1 + >>>>> hw/tpm/Makefile.objs | 1 + >>>>> 8 files changed, 434 insertions(+), 10 deletions(-) >>>>> create mode 100644 hw/tpm/tpm_crb.c >>>>> >>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json >>>>> index 7093f268fb..d50deef5e9 100644 >>>>> --- a/qapi/tpm.json >>>>> +++ b/qapi/tpm.json >>>>> @@ -11,10 +11,11 @@ >>>>> # An enumeration of TPM models >>>>> # >>>>> # @tpm-tis: TPM TIS model >>>>> +# @tpm-crb: TPM CRB model (since 2.12) >>>>> # >>>>> # Since: 1.5 >>>>> ## >>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >>>>> ## >>>>> # @query-tpm-models: >>>>> @@ -28,7 +29,7 @@ >>>>> # Example: >>>>> # >>>>> # -> { "execute": "query-tpm-models" } >>>>> -# <- { "return": [ "tpm-tis" ] } >>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >>>>> # >>>>> ## >>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >>>>> index 6d516c6a7f..b0048515fa 100644 >>>>> --- a/include/hw/acpi/tpm.h >>>>> +++ b/include/hw/acpi/tpm.h >>>>> @@ -16,11 +16,82 @@ >>>>> #ifndef HW_ACPI_TPM_H >>>>> #define HW_ACPI_TPM_H >>>>> +#include "qemu/osdep.h" >>>>> + >>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 >>>>> #define TPM_TIS_ADDR_SIZE 0x5000 >>>>> #define TPM_TIS_IRQ 5 >>>>> +struct crb_regs { >>>>> + union { >>>>> + uint32_t reg; >>>>> + struct { >>>>> + unsigned tpm_established:1; >>>>> + unsigned loc_assigned:1; >>>>> + unsigned active_locality:3; >>>>> + unsigned reserved:2; >>>>> + unsigned tpm_reg_valid_sts:1; >>>>> + } bits; >>>> >>>> I suppose this is little-endian layout, so this won't work on big-endian >>>> hosts. >>>> >>>> Can you add a qtest? >>>> >>>>> + } loc_state; >>>>> + uint32_t reserved1; >>>>> + uint32_t loc_ctrl; >>>>> + union { >>>>> + uint32_t reg; >>>>> + struct { >>>>> + unsigned granted:1; >>>>> + unsigned been_seized:1; >>>>> + } bits; >>>> >>>> >>>> This is unclear where you expect those bits (right/left aligned). >>>> >>>> Can you add an unnamed field to be more explicit? >>>> >>>> i.e. without using struct if left alignment expected: >>>> >>>> unsigned granted:1, been_seized:1, :30; >>> >>> >>> >>> I got rid of all the bitfields and this patch here makes it work on a >>> ppc64 >>> big endian and also x86_64 host: >>> >>> >>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd >>> >> Thank you Stefan! I am all for squashing this fix to the patch. You >> should then add your signed-off to the commit. > > > I'll do that. > > The TIS is an ISA Device and the CRB is similar. Considering the How much similarity is there between TIS and CRB is there? The two devices look quite different to me, CRB is way simpler it seems. Or is the CRB implementation just lacking many bells and whistles that TIS has? Should we consider merging CRB in TIS? > complications with the sysbus devices where one has to explicitly allow it > for a certain machine type, I would advocate to convert the CRB to an ISA > device. A patch that does that is this one: If it's only for that reason (an explicit enable), I would rather keep it on the system bus. Or should it be on an LPC bus? Eduardo, what do you think? Thanks
On 01/22/2018 10:08 AM, Marc-Andre Lureau wrote: > Hi > > On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: >> On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: >>> Hi >>> >>> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger >>> <stefanb@linux.vnet.ibm.com> wrote: >>>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >>>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >>>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >>>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. >>>>>> >>>>>> The PTP allows device implementation to switch between TIS and CRB >>>>>> model at run time, but given that CRB is a simpler device to >>>>>> implement, I chose to implement it as a different device. >>>>>> >>>>>> The device doesn't implement other locality than 0 for now (my laptop >>>>>> TPM doesn't either, so I assume this isn't so bad) >>>>>> >>>>>> The command/reply memory region is statically allocated after the CRB >>>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >>>>>> wonder if the BIOS could or should allocate it instead, or what size >>>>>> to use, again this seems to fit well expectations) >>>>>> >>>>>> The PTP doesn't specify a particular bus to put the device. So I added >>>>>> it on the system bus directly, so it could hopefully be used easily on >>>>>> a different platform than x86. Currently, it fails to init on piix, >>>>>> because error_on_sysbus_device() check. The check may be changed in a >>>>>> near future, see discussion on the qemu-devel ML. >>>>>> >>>>>> Tested with some success with Linux upstream and Windows 10, seabios & >>>>>> modified ovmf. The device is recognized and correctly transmit >>>>>> command/response with passthrough & emu. However, we are missing PPI >>>>>> ACPI part atm. >>>>>> >>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >>>>>> --- >>>>>> qapi/tpm.json | 5 +- >>>>>> include/hw/acpi/tpm.h | 72 ++++++++ >>>>>> include/sysemu/tpm.h | 3 + >>>>>> hw/i386/acpi-build.c | 34 +++- >>>>>> hw/tpm/tpm_crb.c | 327 >>>>>> +++++++++++++++++++++++++++++++++++++ >>>>>> default-configs/i386-softmmu.mak | 1 + >>>>>> default-configs/x86_64-softmmu.mak | 1 + >>>>>> hw/tpm/Makefile.objs | 1 + >>>>>> 8 files changed, 434 insertions(+), 10 deletions(-) >>>>>> create mode 100644 hw/tpm/tpm_crb.c >>>>>> >>>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json >>>>>> index 7093f268fb..d50deef5e9 100644 >>>>>> --- a/qapi/tpm.json >>>>>> +++ b/qapi/tpm.json >>>>>> @@ -11,10 +11,11 @@ >>>>>> # An enumeration of TPM models >>>>>> # >>>>>> # @tpm-tis: TPM TIS model >>>>>> +# @tpm-crb: TPM CRB model (since 2.12) >>>>>> # >>>>>> # Since: 1.5 >>>>>> ## >>>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >>>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >>>>>> ## >>>>>> # @query-tpm-models: >>>>>> @@ -28,7 +29,7 @@ >>>>>> # Example: >>>>>> # >>>>>> # -> { "execute": "query-tpm-models" } >>>>>> -# <- { "return": [ "tpm-tis" ] } >>>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >>>>>> # >>>>>> ## >>>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >>>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >>>>>> index 6d516c6a7f..b0048515fa 100644 >>>>>> --- a/include/hw/acpi/tpm.h >>>>>> +++ b/include/hw/acpi/tpm.h >>>>>> @@ -16,11 +16,82 @@ >>>>>> #ifndef HW_ACPI_TPM_H >>>>>> #define HW_ACPI_TPM_H >>>>>> +#include "qemu/osdep.h" >>>>>> + >>>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 >>>>>> #define TPM_TIS_ADDR_SIZE 0x5000 >>>>>> #define TPM_TIS_IRQ 5 >>>>>> +struct crb_regs { >>>>>> + union { >>>>>> + uint32_t reg; >>>>>> + struct { >>>>>> + unsigned tpm_established:1; >>>>>> + unsigned loc_assigned:1; >>>>>> + unsigned active_locality:3; >>>>>> + unsigned reserved:2; >>>>>> + unsigned tpm_reg_valid_sts:1; >>>>>> + } bits; >>>>> I suppose this is little-endian layout, so this won't work on big-endian >>>>> hosts. >>>>> >>>>> Can you add a qtest? >>>>> >>>>>> + } loc_state; >>>>>> + uint32_t reserved1; >>>>>> + uint32_t loc_ctrl; >>>>>> + union { >>>>>> + uint32_t reg; >>>>>> + struct { >>>>>> + unsigned granted:1; >>>>>> + unsigned been_seized:1; >>>>>> + } bits; >>>>> >>>>> This is unclear where you expect those bits (right/left aligned). >>>>> >>>>> Can you add an unnamed field to be more explicit? >>>>> >>>>> i.e. without using struct if left alignment expected: >>>>> >>>>> unsigned granted:1, been_seized:1, :30; >>>> >>>> >>>> I got rid of all the bitfields and this patch here makes it work on a >>>> ppc64 >>>> big endian and also x86_64 host: >>>> >>>> >>>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd >>>> >>> Thank you Stefan! I am all for squashing this fix to the patch. You >>> should then add your signed-off to the commit. >> >> I'll do that. >> >> The TIS is an ISA Device and the CRB is similar. Considering the > How much similarity is there between TIS and CRB is there? The two > devices look quite different to me, CRB is way simpler it seems. Or is > the CRB implementation just lacking many bells and whistles that TIS > has? Should we consider merging CRB in TIS? That is the question. It would be a pain for users to have to choose the interface. Basically TIS and CRB can be implemented in a single device and let the software choose the interface following flags that are set by the interface advertising whether it offers TIS and/or CRB. Maybe we could keep the CRB code in a separate file and, upon choosing the CRB interface call into CRB functions rather than TIS functions. > >> complications with the sysbus devices where one has to explicitly allow it >> for a certain machine type, I would advocate to convert the CRB to an ISA >> device. A patch that does that is this one: > If it's only for that reason (an explicit enable), I would rather keep > it on the system bus. Or should it be on an LPC bus? I do not think there's an LPC bus. Merging the code into the TIS would put the whoe device on the ISA Bus per current TIS implementation. > > Eduardo, what do you think? > > Thanks >
On Mon, Jan 22, 2018 at 4:47 PM, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote: > On 01/22/2018 10:08 AM, Marc-Andre Lureau wrote: >> >> Hi >> >> On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger >> <stefanb@linux.vnet.ibm.com> wrote: >>> >>> On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: >>>> >>>> Hi >>>> >>>> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger >>>> <stefanb@linux.vnet.ibm.com> wrote: >>>>> >>>>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >>>>>> >>>>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >>>>>>> >>>>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >>>>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >>>>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. >>>>>>> >>>>>>> The PTP allows device implementation to switch between TIS and CRB >>>>>>> model at run time, but given that CRB is a simpler device to >>>>>>> implement, I chose to implement it as a different device. >>>>>>> >>>>>>> The device doesn't implement other locality than 0 for now (my laptop >>>>>>> TPM doesn't either, so I assume this isn't so bad) >>>>>>> >>>>>>> The command/reply memory region is statically allocated after the CRB >>>>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >>>>>>> wonder if the BIOS could or should allocate it instead, or what size >>>>>>> to use, again this seems to fit well expectations) >>>>>>> >>>>>>> The PTP doesn't specify a particular bus to put the device. So I >>>>>>> added >>>>>>> it on the system bus directly, so it could hopefully be used easily >>>>>>> on >>>>>>> a different platform than x86. Currently, it fails to init on piix, >>>>>>> because error_on_sysbus_device() check. The check may be changed in a >>>>>>> near future, see discussion on the qemu-devel ML. >>>>>>> >>>>>>> Tested with some success with Linux upstream and Windows 10, seabios >>>>>>> & >>>>>>> modified ovmf. The device is recognized and correctly transmit >>>>>>> command/response with passthrough & emu. However, we are missing PPI >>>>>>> ACPI part atm. >>>>>>> >>>>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>>>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >>>>>>> --- >>>>>>> qapi/tpm.json | 5 +- >>>>>>> include/hw/acpi/tpm.h | 72 ++++++++ >>>>>>> include/sysemu/tpm.h | 3 + >>>>>>> hw/i386/acpi-build.c | 34 +++- >>>>>>> hw/tpm/tpm_crb.c | 327 >>>>>>> +++++++++++++++++++++++++++++++++++++ >>>>>>> default-configs/i386-softmmu.mak | 1 + >>>>>>> default-configs/x86_64-softmmu.mak | 1 + >>>>>>> hw/tpm/Makefile.objs | 1 + >>>>>>> 8 files changed, 434 insertions(+), 10 deletions(-) >>>>>>> create mode 100644 hw/tpm/tpm_crb.c >>>>>>> >>>>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json >>>>>>> index 7093f268fb..d50deef5e9 100644 >>>>>>> --- a/qapi/tpm.json >>>>>>> +++ b/qapi/tpm.json >>>>>>> @@ -11,10 +11,11 @@ >>>>>>> # An enumeration of TPM models >>>>>>> # >>>>>>> # @tpm-tis: TPM TIS model >>>>>>> +# @tpm-crb: TPM CRB model (since 2.12) >>>>>>> # >>>>>>> # Since: 1.5 >>>>>>> ## >>>>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >>>>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >>>>>>> ## >>>>>>> # @query-tpm-models: >>>>>>> @@ -28,7 +29,7 @@ >>>>>>> # Example: >>>>>>> # >>>>>>> # -> { "execute": "query-tpm-models" } >>>>>>> -# <- { "return": [ "tpm-tis" ] } >>>>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >>>>>>> # >>>>>>> ## >>>>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >>>>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >>>>>>> index 6d516c6a7f..b0048515fa 100644 >>>>>>> --- a/include/hw/acpi/tpm.h >>>>>>> +++ b/include/hw/acpi/tpm.h >>>>>>> @@ -16,11 +16,82 @@ >>>>>>> #ifndef HW_ACPI_TPM_H >>>>>>> #define HW_ACPI_TPM_H >>>>>>> +#include "qemu/osdep.h" >>>>>>> + >>>>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 >>>>>>> #define TPM_TIS_ADDR_SIZE 0x5000 >>>>>>> #define TPM_TIS_IRQ 5 >>>>>>> +struct crb_regs { >>>>>>> + union { >>>>>>> + uint32_t reg; >>>>>>> + struct { >>>>>>> + unsigned tpm_established:1; >>>>>>> + unsigned loc_assigned:1; >>>>>>> + unsigned active_locality:3; >>>>>>> + unsigned reserved:2; >>>>>>> + unsigned tpm_reg_valid_sts:1; >>>>>>> + } bits; >>>>>> >>>>>> I suppose this is little-endian layout, so this won't work on >>>>>> big-endian >>>>>> hosts. >>>>>> >>>>>> Can you add a qtest? >>>>>> >>>>>>> + } loc_state; >>>>>>> + uint32_t reserved1; >>>>>>> + uint32_t loc_ctrl; >>>>>>> + union { >>>>>>> + uint32_t reg; >>>>>>> + struct { >>>>>>> + unsigned granted:1; >>>>>>> + unsigned been_seized:1; >>>>>>> + } bits; >>>>>> >>>>>> >>>>>> This is unclear where you expect those bits (right/left aligned). >>>>>> >>>>>> Can you add an unnamed field to be more explicit? >>>>>> >>>>>> i.e. without using struct if left alignment expected: >>>>>> >>>>>> unsigned granted:1, been_seized:1, :30; >>>>> >>>>> >>>>> >>>>> I got rid of all the bitfields and this patch here makes it work on a >>>>> ppc64 >>>>> big endian and also x86_64 host: >>>>> >>>>> >>>>> >>>>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd >>>>> >>>> Thank you Stefan! I am all for squashing this fix to the patch. You >>>> should then add your signed-off to the commit. >>> >>> >>> I'll do that. >>> >>> The TIS is an ISA Device and the CRB is similar. Considering the >> >> How much similarity is there between TIS and CRB is there? The two >> devices look quite different to me, CRB is way simpler it seems. Or is >> the CRB implementation just lacking many bells and whistles that TIS >> has? Should we consider merging CRB in TIS? > > > That is the question. It would be a pain for users to have to choose the > interface. Basically TIS and CRB can be implemented in a single device and > let the software choose the interface following flags that are set by the > interface advertising whether it offers TIS and/or CRB. Maybe we could keep > the CRB code in a separate file and, upon choosing the CRB interface call > into CRB functions rather than TIS functions. Afaik, most people want to use TPM2, either with Linux or Windows, and CRB fits that job better at least on Windows. So unless TPM 1.2 and legacy OS is required, I would recommend to use CRB only. If there is a need to grow dynamic TIS/CRB switching, I would teach the existing TIS device to call into CRB, as you proposed. In the meantime, I think it's best to have a simple CRB-only device that is known to work with latest Linux & Windows.
On Mon, Jan 22, 2018 at 04:08:30PM +0100, Marc-Andre Lureau wrote: > Hi > > On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger > <stefanb@linux.vnet.ibm.com> wrote: > > On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: > >> > >> Hi > >> > >> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger > >> <stefanb@linux.vnet.ibm.com> wrote: > >>> > >>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: > >>>> > >>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: > >>>>> > >>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) > >>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) > >>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. > >>>>> > >>>>> The PTP allows device implementation to switch between TIS and CRB > >>>>> model at run time, but given that CRB is a simpler device to > >>>>> implement, I chose to implement it as a different device. > >>>>> > >>>>> The device doesn't implement other locality than 0 for now (my laptop > >>>>> TPM doesn't either, so I assume this isn't so bad) > >>>>> > >>>>> The command/reply memory region is statically allocated after the CRB > >>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I > >>>>> wonder if the BIOS could or should allocate it instead, or what size > >>>>> to use, again this seems to fit well expectations) > >>>>> > >>>>> The PTP doesn't specify a particular bus to put the device. So I added > >>>>> it on the system bus directly, so it could hopefully be used easily on > >>>>> a different platform than x86. Currently, it fails to init on piix, > >>>>> because error_on_sysbus_device() check. The check may be changed in a > >>>>> near future, see discussion on the qemu-devel ML. > >>>>> > >>>>> Tested with some success with Linux upstream and Windows 10, seabios & > >>>>> modified ovmf. The device is recognized and correctly transmit > >>>>> command/response with passthrough & emu. However, we are missing PPI > >>>>> ACPI part atm. > >>>>> > >>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > >>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > >>>>> --- > >>>>> qapi/tpm.json | 5 +- > >>>>> include/hw/acpi/tpm.h | 72 ++++++++ > >>>>> include/sysemu/tpm.h | 3 + > >>>>> hw/i386/acpi-build.c | 34 +++- > >>>>> hw/tpm/tpm_crb.c | 327 > >>>>> +++++++++++++++++++++++++++++++++++++ > >>>>> default-configs/i386-softmmu.mak | 1 + > >>>>> default-configs/x86_64-softmmu.mak | 1 + > >>>>> hw/tpm/Makefile.objs | 1 + > >>>>> 8 files changed, 434 insertions(+), 10 deletions(-) > >>>>> create mode 100644 hw/tpm/tpm_crb.c > >>>>> > >>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json > >>>>> index 7093f268fb..d50deef5e9 100644 > >>>>> --- a/qapi/tpm.json > >>>>> +++ b/qapi/tpm.json > >>>>> @@ -11,10 +11,11 @@ > >>>>> # An enumeration of TPM models > >>>>> # > >>>>> # @tpm-tis: TPM TIS model > >>>>> +# @tpm-crb: TPM CRB model (since 2.12) > >>>>> # > >>>>> # Since: 1.5 > >>>>> ## > >>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } > >>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } > >>>>> ## > >>>>> # @query-tpm-models: > >>>>> @@ -28,7 +29,7 @@ > >>>>> # Example: > >>>>> # > >>>>> # -> { "execute": "query-tpm-models" } > >>>>> -# <- { "return": [ "tpm-tis" ] } > >>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } > >>>>> # > >>>>> ## > >>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } > >>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h > >>>>> index 6d516c6a7f..b0048515fa 100644 > >>>>> --- a/include/hw/acpi/tpm.h > >>>>> +++ b/include/hw/acpi/tpm.h > >>>>> @@ -16,11 +16,82 @@ > >>>>> #ifndef HW_ACPI_TPM_H > >>>>> #define HW_ACPI_TPM_H > >>>>> +#include "qemu/osdep.h" > >>>>> + > >>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 > >>>>> #define TPM_TIS_ADDR_SIZE 0x5000 > >>>>> #define TPM_TIS_IRQ 5 > >>>>> +struct crb_regs { > >>>>> + union { > >>>>> + uint32_t reg; > >>>>> + struct { > >>>>> + unsigned tpm_established:1; > >>>>> + unsigned loc_assigned:1; > >>>>> + unsigned active_locality:3; > >>>>> + unsigned reserved:2; > >>>>> + unsigned tpm_reg_valid_sts:1; > >>>>> + } bits; > >>>> > >>>> I suppose this is little-endian layout, so this won't work on big-endian > >>>> hosts. > >>>> > >>>> Can you add a qtest? > >>>> > >>>>> + } loc_state; > >>>>> + uint32_t reserved1; > >>>>> + uint32_t loc_ctrl; > >>>>> + union { > >>>>> + uint32_t reg; > >>>>> + struct { > >>>>> + unsigned granted:1; > >>>>> + unsigned been_seized:1; > >>>>> + } bits; > >>>> > >>>> > >>>> This is unclear where you expect those bits (right/left aligned). > >>>> > >>>> Can you add an unnamed field to be more explicit? > >>>> > >>>> i.e. without using struct if left alignment expected: > >>>> > >>>> unsigned granted:1, been_seized:1, :30; > >>> > >>> > >>> > >>> I got rid of all the bitfields and this patch here makes it work on a > >>> ppc64 > >>> big endian and also x86_64 host: > >>> > >>> > >>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd > >>> > >> Thank you Stefan! I am all for squashing this fix to the patch. You > >> should then add your signed-off to the commit. > > > > > > I'll do that. > > > > The TIS is an ISA Device and the CRB is similar. Considering the > > How much similarity is there between TIS and CRB is there? The two > devices look quite different to me, CRB is way simpler it seems. Or is > the CRB implementation just lacking many bells and whistles that TIS > has? Should we consider merging CRB in TIS? > > > complications with the sysbus devices where one has to explicitly allow it > > for a certain machine type, I would advocate to convert the CRB to an ISA > > device. A patch that does that is this one: > > If it's only for that reason (an explicit enable), I would rather keep > it on the system bus. Or should it be on an LPC bus? > > Eduardo, what do you think? Everything about sysbus is exceptional and confusing, so I would prefer to avoid using sysbus every time we have an alternative. If tpm-tis is already an ISA device, what are the reasons to not use ISA for tpm-crb too?
Hi On Mon, Jan 22, 2018 at 6:25 PM, Eduardo Habkost <ehabkost@redhat.com> wrote: > On Mon, Jan 22, 2018 at 04:08:30PM +0100, Marc-Andre Lureau wrote: >> Hi >> >> On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger >> <stefanb@linux.vnet.ibm.com> wrote: >> > On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: >> >> >> >> Hi >> >> >> >> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger >> >> <stefanb@linux.vnet.ibm.com> wrote: >> >>> >> >>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >> >>>> >> >>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >> >>>>> >> >>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >> >>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >> >>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. >> >>>>> >> >>>>> The PTP allows device implementation to switch between TIS and CRB >> >>>>> model at run time, but given that CRB is a simpler device to >> >>>>> implement, I chose to implement it as a different device. >> >>>>> >> >>>>> The device doesn't implement other locality than 0 for now (my laptop >> >>>>> TPM doesn't either, so I assume this isn't so bad) >> >>>>> >> >>>>> The command/reply memory region is statically allocated after the CRB >> >>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >> >>>>> wonder if the BIOS could or should allocate it instead, or what size >> >>>>> to use, again this seems to fit well expectations) >> >>>>> >> >>>>> The PTP doesn't specify a particular bus to put the device. So I added >> >>>>> it on the system bus directly, so it could hopefully be used easily on >> >>>>> a different platform than x86. Currently, it fails to init on piix, >> >>>>> because error_on_sysbus_device() check. The check may be changed in a >> >>>>> near future, see discussion on the qemu-devel ML. >> >>>>> >> >>>>> Tested with some success with Linux upstream and Windows 10, seabios & >> >>>>> modified ovmf. The device is recognized and correctly transmit >> >>>>> command/response with passthrough & emu. However, we are missing PPI >> >>>>> ACPI part atm. >> >>>>> >> >>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> >>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >> >>>>> --- >> >>>>> qapi/tpm.json | 5 +- >> >>>>> include/hw/acpi/tpm.h | 72 ++++++++ >> >>>>> include/sysemu/tpm.h | 3 + >> >>>>> hw/i386/acpi-build.c | 34 +++- >> >>>>> hw/tpm/tpm_crb.c | 327 >> >>>>> +++++++++++++++++++++++++++++++++++++ >> >>>>> default-configs/i386-softmmu.mak | 1 + >> >>>>> default-configs/x86_64-softmmu.mak | 1 + >> >>>>> hw/tpm/Makefile.objs | 1 + >> >>>>> 8 files changed, 434 insertions(+), 10 deletions(-) >> >>>>> create mode 100644 hw/tpm/tpm_crb.c >> >>>>> >> >>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json >> >>>>> index 7093f268fb..d50deef5e9 100644 >> >>>>> --- a/qapi/tpm.json >> >>>>> +++ b/qapi/tpm.json >> >>>>> @@ -11,10 +11,11 @@ >> >>>>> # An enumeration of TPM models >> >>>>> # >> >>>>> # @tpm-tis: TPM TIS model >> >>>>> +# @tpm-crb: TPM CRB model (since 2.12) >> >>>>> # >> >>>>> # Since: 1.5 >> >>>>> ## >> >>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >> >>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >> >>>>> ## >> >>>>> # @query-tpm-models: >> >>>>> @@ -28,7 +29,7 @@ >> >>>>> # Example: >> >>>>> # >> >>>>> # -> { "execute": "query-tpm-models" } >> >>>>> -# <- { "return": [ "tpm-tis" ] } >> >>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >> >>>>> # >> >>>>> ## >> >>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >> >>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >> >>>>> index 6d516c6a7f..b0048515fa 100644 >> >>>>> --- a/include/hw/acpi/tpm.h >> >>>>> +++ b/include/hw/acpi/tpm.h >> >>>>> @@ -16,11 +16,82 @@ >> >>>>> #ifndef HW_ACPI_TPM_H >> >>>>> #define HW_ACPI_TPM_H >> >>>>> +#include "qemu/osdep.h" >> >>>>> + >> >>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 >> >>>>> #define TPM_TIS_ADDR_SIZE 0x5000 >> >>>>> #define TPM_TIS_IRQ 5 >> >>>>> +struct crb_regs { >> >>>>> + union { >> >>>>> + uint32_t reg; >> >>>>> + struct { >> >>>>> + unsigned tpm_established:1; >> >>>>> + unsigned loc_assigned:1; >> >>>>> + unsigned active_locality:3; >> >>>>> + unsigned reserved:2; >> >>>>> + unsigned tpm_reg_valid_sts:1; >> >>>>> + } bits; >> >>>> >> >>>> I suppose this is little-endian layout, so this won't work on big-endian >> >>>> hosts. >> >>>> >> >>>> Can you add a qtest? >> >>>> >> >>>>> + } loc_state; >> >>>>> + uint32_t reserved1; >> >>>>> + uint32_t loc_ctrl; >> >>>>> + union { >> >>>>> + uint32_t reg; >> >>>>> + struct { >> >>>>> + unsigned granted:1; >> >>>>> + unsigned been_seized:1; >> >>>>> + } bits; >> >>>> >> >>>> >> >>>> This is unclear where you expect those bits (right/left aligned). >> >>>> >> >>>> Can you add an unnamed field to be more explicit? >> >>>> >> >>>> i.e. without using struct if left alignment expected: >> >>>> >> >>>> unsigned granted:1, been_seized:1, :30; >> >>> >> >>> >> >>> >> >>> I got rid of all the bitfields and this patch here makes it work on a >> >>> ppc64 >> >>> big endian and also x86_64 host: >> >>> >> >>> >> >>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd >> >>> >> >> Thank you Stefan! I am all for squashing this fix to the patch. You >> >> should then add your signed-off to the commit. >> > >> > >> > I'll do that. >> > >> > The TIS is an ISA Device and the CRB is similar. Considering the >> >> How much similarity is there between TIS and CRB is there? The two >> devices look quite different to me, CRB is way simpler it seems. Or is >> the CRB implementation just lacking many bells and whistles that TIS >> has? Should we consider merging CRB in TIS? >> >> > complications with the sysbus devices where one has to explicitly allow it >> > for a certain machine type, I would advocate to convert the CRB to an ISA >> > device. A patch that does that is this one: >> >> If it's only for that reason (an explicit enable), I would rather keep >> it on the system bus. Or should it be on an LPC bus? >> >> Eduardo, what do you think? > > Everything about sysbus is exceptional and confusing, so I would > prefer to avoid using sysbus every time we have an alternative. > If tpm-tis is already an ISA device, what are the reasons to not > use ISA for tpm-crb too? I was hoping this would make the device more portable (especially on arm) and avoid using legacy bus or resources. Putting it on ISA doesn't reflect better what a real hardware is like, or does it?
On Mon, Jan 22, 2018 at 06:32:37PM +0100, Marc-André Lureau wrote: > Hi > > On Mon, Jan 22, 2018 at 6:25 PM, Eduardo Habkost <ehabkost@redhat.com> wrote: > > On Mon, Jan 22, 2018 at 04:08:30PM +0100, Marc-Andre Lureau wrote: > >> Hi > >> > >> On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger > >> <stefanb@linux.vnet.ibm.com> wrote: > >> > On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: > >> >> > >> >> Hi > >> >> > >> >> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger > >> >> <stefanb@linux.vnet.ibm.com> wrote: > >> >>> > >> >>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: > >> >>>> > >> >>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: > >> >>>>> > >> >>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) > >> >>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) > >> >>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. > >> >>>>> > >> >>>>> The PTP allows device implementation to switch between TIS and CRB > >> >>>>> model at run time, but given that CRB is a simpler device to > >> >>>>> implement, I chose to implement it as a different device. > >> >>>>> > >> >>>>> The device doesn't implement other locality than 0 for now (my laptop > >> >>>>> TPM doesn't either, so I assume this isn't so bad) > >> >>>>> > >> >>>>> The command/reply memory region is statically allocated after the CRB > >> >>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I > >> >>>>> wonder if the BIOS could or should allocate it instead, or what size > >> >>>>> to use, again this seems to fit well expectations) > >> >>>>> > >> >>>>> The PTP doesn't specify a particular bus to put the device. So I added > >> >>>>> it on the system bus directly, so it could hopefully be used easily on > >> >>>>> a different platform than x86. Currently, it fails to init on piix, > >> >>>>> because error_on_sysbus_device() check. The check may be changed in a > >> >>>>> near future, see discussion on the qemu-devel ML. > >> >>>>> > >> >>>>> Tested with some success with Linux upstream and Windows 10, seabios & > >> >>>>> modified ovmf. The device is recognized and correctly transmit > >> >>>>> command/response with passthrough & emu. However, we are missing PPI > >> >>>>> ACPI part atm. > >> >>>>> > >> >>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > >> >>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > >> >>>>> --- > >> >>>>> qapi/tpm.json | 5 +- > >> >>>>> include/hw/acpi/tpm.h | 72 ++++++++ > >> >>>>> include/sysemu/tpm.h | 3 + > >> >>>>> hw/i386/acpi-build.c | 34 +++- > >> >>>>> hw/tpm/tpm_crb.c | 327 > >> >>>>> +++++++++++++++++++++++++++++++++++++ > >> >>>>> default-configs/i386-softmmu.mak | 1 + > >> >>>>> default-configs/x86_64-softmmu.mak | 1 + > >> >>>>> hw/tpm/Makefile.objs | 1 + > >> >>>>> 8 files changed, 434 insertions(+), 10 deletions(-) > >> >>>>> create mode 100644 hw/tpm/tpm_crb.c > >> >>>>> > >> >>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json > >> >>>>> index 7093f268fb..d50deef5e9 100644 > >> >>>>> --- a/qapi/tpm.json > >> >>>>> +++ b/qapi/tpm.json > >> >>>>> @@ -11,10 +11,11 @@ > >> >>>>> # An enumeration of TPM models > >> >>>>> # > >> >>>>> # @tpm-tis: TPM TIS model > >> >>>>> +# @tpm-crb: TPM CRB model (since 2.12) > >> >>>>> # > >> >>>>> # Since: 1.5 > >> >>>>> ## > >> >>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } > >> >>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } > >> >>>>> ## > >> >>>>> # @query-tpm-models: > >> >>>>> @@ -28,7 +29,7 @@ > >> >>>>> # Example: > >> >>>>> # > >> >>>>> # -> { "execute": "query-tpm-models" } > >> >>>>> -# <- { "return": [ "tpm-tis" ] } > >> >>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } > >> >>>>> # > >> >>>>> ## > >> >>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } > >> >>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h > >> >>>>> index 6d516c6a7f..b0048515fa 100644 > >> >>>>> --- a/include/hw/acpi/tpm.h > >> >>>>> +++ b/include/hw/acpi/tpm.h > >> >>>>> @@ -16,11 +16,82 @@ > >> >>>>> #ifndef HW_ACPI_TPM_H > >> >>>>> #define HW_ACPI_TPM_H > >> >>>>> +#include "qemu/osdep.h" > >> >>>>> + > >> >>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 > >> >>>>> #define TPM_TIS_ADDR_SIZE 0x5000 > >> >>>>> #define TPM_TIS_IRQ 5 > >> >>>>> +struct crb_regs { > >> >>>>> + union { > >> >>>>> + uint32_t reg; > >> >>>>> + struct { > >> >>>>> + unsigned tpm_established:1; > >> >>>>> + unsigned loc_assigned:1; > >> >>>>> + unsigned active_locality:3; > >> >>>>> + unsigned reserved:2; > >> >>>>> + unsigned tpm_reg_valid_sts:1; > >> >>>>> + } bits; > >> >>>> > >> >>>> I suppose this is little-endian layout, so this won't work on big-endian > >> >>>> hosts. > >> >>>> > >> >>>> Can you add a qtest? > >> >>>> > >> >>>>> + } loc_state; > >> >>>>> + uint32_t reserved1; > >> >>>>> + uint32_t loc_ctrl; > >> >>>>> + union { > >> >>>>> + uint32_t reg; > >> >>>>> + struct { > >> >>>>> + unsigned granted:1; > >> >>>>> + unsigned been_seized:1; > >> >>>>> + } bits; > >> >>>> > >> >>>> > >> >>>> This is unclear where you expect those bits (right/left aligned). > >> >>>> > >> >>>> Can you add an unnamed field to be more explicit? > >> >>>> > >> >>>> i.e. without using struct if left alignment expected: > >> >>>> > >> >>>> unsigned granted:1, been_seized:1, :30; > >> >>> > >> >>> > >> >>> > >> >>> I got rid of all the bitfields and this patch here makes it work on a > >> >>> ppc64 > >> >>> big endian and also x86_64 host: > >> >>> > >> >>> > >> >>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd > >> >>> > >> >> Thank you Stefan! I am all for squashing this fix to the patch. You > >> >> should then add your signed-off to the commit. > >> > > >> > > >> > I'll do that. > >> > > >> > The TIS is an ISA Device and the CRB is similar. Considering the > >> > >> How much similarity is there between TIS and CRB is there? The two > >> devices look quite different to me, CRB is way simpler it seems. Or is > >> the CRB implementation just lacking many bells and whistles that TIS > >> has? Should we consider merging CRB in TIS? > >> > >> > complications with the sysbus devices where one has to explicitly allow it > >> > for a certain machine type, I would advocate to convert the CRB to an ISA > >> > device. A patch that does that is this one: > >> > >> If it's only for that reason (an explicit enable), I would rather keep > >> it on the system bus. Or should it be on an LPC bus? > >> > >> Eduardo, what do you think? > > > > Everything about sysbus is exceptional and confusing, so I would > > prefer to avoid using sysbus every time we have an alternative. > > If tpm-tis is already an ISA device, what are the reasons to not > > use ISA for tpm-crb too? > > I was hoping this would make the device more portable (especially on > arm) and avoid using legacy bus or resources. > > Putting it on ISA doesn't reflect better what a real hardware is like, > or does it? I don't know what the hardware interface for those devices look like, the documentation I found for TPM-CRB seem to be purely for the software interface. Is there a reason to make it a sysbus device instead of a bus-less device, then?
Hi On Mon, Jan 22, 2018 at 6:47 PM, Eduardo Habkost <ehabkost@redhat.com> wrote: > On Mon, Jan 22, 2018 at 06:32:37PM +0100, Marc-André Lureau wrote: >> Hi >> >> On Mon, Jan 22, 2018 at 6:25 PM, Eduardo Habkost <ehabkost@redhat.com> wrote: >> > On Mon, Jan 22, 2018 at 04:08:30PM +0100, Marc-Andre Lureau wrote: >> >> Hi >> >> >> >> On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger >> >> <stefanb@linux.vnet.ibm.com> wrote: >> >> > On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: >> >> >> >> >> >> Hi >> >> >> >> >> >> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger >> >> >> <stefanb@linux.vnet.ibm.com> wrote: >> >> >>> >> >> >>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: >> >> >>>> >> >> >>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: >> >> >>>>> >> >> >>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) >> >> >>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) >> >> >>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. >> >> >>>>> >> >> >>>>> The PTP allows device implementation to switch between TIS and CRB >> >> >>>>> model at run time, but given that CRB is a simpler device to >> >> >>>>> implement, I chose to implement it as a different device. >> >> >>>>> >> >> >>>>> The device doesn't implement other locality than 0 for now (my laptop >> >> >>>>> TPM doesn't either, so I assume this isn't so bad) >> >> >>>>> >> >> >>>>> The command/reply memory region is statically allocated after the CRB >> >> >>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I >> >> >>>>> wonder if the BIOS could or should allocate it instead, or what size >> >> >>>>> to use, again this seems to fit well expectations) >> >> >>>>> >> >> >>>>> The PTP doesn't specify a particular bus to put the device. So I added >> >> >>>>> it on the system bus directly, so it could hopefully be used easily on >> >> >>>>> a different platform than x86. Currently, it fails to init on piix, >> >> >>>>> because error_on_sysbus_device() check. The check may be changed in a >> >> >>>>> near future, see discussion on the qemu-devel ML. >> >> >>>>> >> >> >>>>> Tested with some success with Linux upstream and Windows 10, seabios & >> >> >>>>> modified ovmf. The device is recognized and correctly transmit >> >> >>>>> command/response with passthrough & emu. However, we are missing PPI >> >> >>>>> ACPI part atm. >> >> >>>>> >> >> >>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> >>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >> >> >>>>> --- >> >> >>>>> qapi/tpm.json | 5 +- >> >> >>>>> include/hw/acpi/tpm.h | 72 ++++++++ >> >> >>>>> include/sysemu/tpm.h | 3 + >> >> >>>>> hw/i386/acpi-build.c | 34 +++- >> >> >>>>> hw/tpm/tpm_crb.c | 327 >> >> >>>>> +++++++++++++++++++++++++++++++++++++ >> >> >>>>> default-configs/i386-softmmu.mak | 1 + >> >> >>>>> default-configs/x86_64-softmmu.mak | 1 + >> >> >>>>> hw/tpm/Makefile.objs | 1 + >> >> >>>>> 8 files changed, 434 insertions(+), 10 deletions(-) >> >> >>>>> create mode 100644 hw/tpm/tpm_crb.c >> >> >>>>> >> >> >>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json >> >> >>>>> index 7093f268fb..d50deef5e9 100644 >> >> >>>>> --- a/qapi/tpm.json >> >> >>>>> +++ b/qapi/tpm.json >> >> >>>>> @@ -11,10 +11,11 @@ >> >> >>>>> # An enumeration of TPM models >> >> >>>>> # >> >> >>>>> # @tpm-tis: TPM TIS model >> >> >>>>> +# @tpm-crb: TPM CRB model (since 2.12) >> >> >>>>> # >> >> >>>>> # Since: 1.5 >> >> >>>>> ## >> >> >>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } >> >> >>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } >> >> >>>>> ## >> >> >>>>> # @query-tpm-models: >> >> >>>>> @@ -28,7 +29,7 @@ >> >> >>>>> # Example: >> >> >>>>> # >> >> >>>>> # -> { "execute": "query-tpm-models" } >> >> >>>>> -# <- { "return": [ "tpm-tis" ] } >> >> >>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } >> >> >>>>> # >> >> >>>>> ## >> >> >>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } >> >> >>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h >> >> >>>>> index 6d516c6a7f..b0048515fa 100644 >> >> >>>>> --- a/include/hw/acpi/tpm.h >> >> >>>>> +++ b/include/hw/acpi/tpm.h >> >> >>>>> @@ -16,11 +16,82 @@ >> >> >>>>> #ifndef HW_ACPI_TPM_H >> >> >>>>> #define HW_ACPI_TPM_H >> >> >>>>> +#include "qemu/osdep.h" >> >> >>>>> + >> >> >>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 >> >> >>>>> #define TPM_TIS_ADDR_SIZE 0x5000 >> >> >>>>> #define TPM_TIS_IRQ 5 >> >> >>>>> +struct crb_regs { >> >> >>>>> + union { >> >> >>>>> + uint32_t reg; >> >> >>>>> + struct { >> >> >>>>> + unsigned tpm_established:1; >> >> >>>>> + unsigned loc_assigned:1; >> >> >>>>> + unsigned active_locality:3; >> >> >>>>> + unsigned reserved:2; >> >> >>>>> + unsigned tpm_reg_valid_sts:1; >> >> >>>>> + } bits; >> >> >>>> >> >> >>>> I suppose this is little-endian layout, so this won't work on big-endian >> >> >>>> hosts. >> >> >>>> >> >> >>>> Can you add a qtest? >> >> >>>> >> >> >>>>> + } loc_state; >> >> >>>>> + uint32_t reserved1; >> >> >>>>> + uint32_t loc_ctrl; >> >> >>>>> + union { >> >> >>>>> + uint32_t reg; >> >> >>>>> + struct { >> >> >>>>> + unsigned granted:1; >> >> >>>>> + unsigned been_seized:1; >> >> >>>>> + } bits; >> >> >>>> >> >> >>>> >> >> >>>> This is unclear where you expect those bits (right/left aligned). >> >> >>>> >> >> >>>> Can you add an unnamed field to be more explicit? >> >> >>>> >> >> >>>> i.e. without using struct if left alignment expected: >> >> >>>> >> >> >>>> unsigned granted:1, been_seized:1, :30; >> >> >>> >> >> >>> >> >> >>> >> >> >>> I got rid of all the bitfields and this patch here makes it work on a >> >> >>> ppc64 >> >> >>> big endian and also x86_64 host: >> >> >>> >> >> >>> >> >> >>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd >> >> >>> >> >> >> Thank you Stefan! I am all for squashing this fix to the patch. You >> >> >> should then add your signed-off to the commit. >> >> > >> >> > >> >> > I'll do that. >> >> > >> >> > The TIS is an ISA Device and the CRB is similar. Considering the >> >> >> >> How much similarity is there between TIS and CRB is there? The two >> >> devices look quite different to me, CRB is way simpler it seems. Or is >> >> the CRB implementation just lacking many bells and whistles that TIS >> >> has? Should we consider merging CRB in TIS? >> >> >> >> > complications with the sysbus devices where one has to explicitly allow it >> >> > for a certain machine type, I would advocate to convert the CRB to an ISA >> >> > device. A patch that does that is this one: >> >> >> >> If it's only for that reason (an explicit enable), I would rather keep >> >> it on the system bus. Or should it be on an LPC bus? >> >> >> >> Eduardo, what do you think? >> > >> > Everything about sysbus is exceptional and confusing, so I would >> > prefer to avoid using sysbus every time we have an alternative. >> > If tpm-tis is already an ISA device, what are the reasons to not >> > use ISA for tpm-crb too? >> >> I was hoping this would make the device more portable (especially on >> arm) and avoid using legacy bus or resources. >> >> Putting it on ISA doesn't reflect better what a real hardware is like, >> or does it? > > I don't know what the hardware interface for those devices look > like, the documentation I found for TPM-CRB seem to be purely for > the software interface. > It has a MMIO region at 0xFED4_xxxx, and it can be on LPC or SPI bus as far as I understand (8.2.2): https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_TPM_Profile_PTP_Specification_Family_2.0_Revision_1.3v22.pdf > Is there a reason to make it a sysbus device instead of a > bus-less device, then? The calls to sysbus_init_mmio(sbd, &s->mmio); sysbus_mmio_map(sbd, 0, TPM_CRB_ADDR_BASE); Could perhaps be replaced by lower-level memory_region calls? I'll give it a try thanks
On Mon, Jan 22, 2018 at 07:15:01PM +0100, Marc-André Lureau wrote: > Hi > > On Mon, Jan 22, 2018 at 6:47 PM, Eduardo Habkost <ehabkost@redhat.com> wrote: > > On Mon, Jan 22, 2018 at 06:32:37PM +0100, Marc-André Lureau wrote: > >> Hi > >> > >> On Mon, Jan 22, 2018 at 6:25 PM, Eduardo Habkost <ehabkost@redhat.com> wrote: > >> > On Mon, Jan 22, 2018 at 04:08:30PM +0100, Marc-Andre Lureau wrote: > >> >> Hi > >> >> > >> >> On Sun, Jan 21, 2018 at 11:01 PM, Stefan Berger > >> >> <stefanb@linux.vnet.ibm.com> wrote: > >> >> > On 01/21/2018 02:24 PM, Marc-Andre Lureau wrote: > >> >> >> > >> >> >> Hi > >> >> >> > >> >> >> On Sun, Jan 21, 2018 at 6:46 AM, Stefan Berger > >> >> >> <stefanb@linux.vnet.ibm.com> wrote: > >> >> >>> > >> >> >>> On 01/20/2018 07:54 AM, Philippe Mathieu-Daudé wrote: > >> >> >>>> > >> >> >>>> On 01/19/2018 11:11 AM, Marc-André Lureau wrote: > >> >> >>>>> > >> >> >>>>> tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) > >> >> >>>>> Interface as defined in TCG PC Client Platform TPM Profile (PTP) > >> >> >>>>> Specification Family “2.0” Level 00 Revision 01.03 v22. > >> >> >>>>> > >> >> >>>>> The PTP allows device implementation to switch between TIS and CRB > >> >> >>>>> model at run time, but given that CRB is a simpler device to > >> >> >>>>> implement, I chose to implement it as a different device. > >> >> >>>>> > >> >> >>>>> The device doesn't implement other locality than 0 for now (my laptop > >> >> >>>>> TPM doesn't either, so I assume this isn't so bad) > >> >> >>>>> > >> >> >>>>> The command/reply memory region is statically allocated after the CRB > >> >> >>>>> registers address TPM_CRB_ADDR_BASE + sizeof(struct crb_regs) (I > >> >> >>>>> wonder if the BIOS could or should allocate it instead, or what size > >> >> >>>>> to use, again this seems to fit well expectations) > >> >> >>>>> > >> >> >>>>> The PTP doesn't specify a particular bus to put the device. So I added > >> >> >>>>> it on the system bus directly, so it could hopefully be used easily on > >> >> >>>>> a different platform than x86. Currently, it fails to init on piix, > >> >> >>>>> because error_on_sysbus_device() check. The check may be changed in a > >> >> >>>>> near future, see discussion on the qemu-devel ML. > >> >> >>>>> > >> >> >>>>> Tested with some success with Linux upstream and Windows 10, seabios & > >> >> >>>>> modified ovmf. The device is recognized and correctly transmit > >> >> >>>>> command/response with passthrough & emu. However, we are missing PPI > >> >> >>>>> ACPI part atm. > >> >> >>>>> > >> >> >>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > >> >> >>>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > >> >> >>>>> --- > >> >> >>>>> qapi/tpm.json | 5 +- > >> >> >>>>> include/hw/acpi/tpm.h | 72 ++++++++ > >> >> >>>>> include/sysemu/tpm.h | 3 + > >> >> >>>>> hw/i386/acpi-build.c | 34 +++- > >> >> >>>>> hw/tpm/tpm_crb.c | 327 > >> >> >>>>> +++++++++++++++++++++++++++++++++++++ > >> >> >>>>> default-configs/i386-softmmu.mak | 1 + > >> >> >>>>> default-configs/x86_64-softmmu.mak | 1 + > >> >> >>>>> hw/tpm/Makefile.objs | 1 + > >> >> >>>>> 8 files changed, 434 insertions(+), 10 deletions(-) > >> >> >>>>> create mode 100644 hw/tpm/tpm_crb.c > >> >> >>>>> > >> >> >>>>> diff --git a/qapi/tpm.json b/qapi/tpm.json > >> >> >>>>> index 7093f268fb..d50deef5e9 100644 > >> >> >>>>> --- a/qapi/tpm.json > >> >> >>>>> +++ b/qapi/tpm.json > >> >> >>>>> @@ -11,10 +11,11 @@ > >> >> >>>>> # An enumeration of TPM models > >> >> >>>>> # > >> >> >>>>> # @tpm-tis: TPM TIS model > >> >> >>>>> +# @tpm-crb: TPM CRB model (since 2.12) > >> >> >>>>> # > >> >> >>>>> # Since: 1.5 > >> >> >>>>> ## > >> >> >>>>> -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } > >> >> >>>>> +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } > >> >> >>>>> ## > >> >> >>>>> # @query-tpm-models: > >> >> >>>>> @@ -28,7 +29,7 @@ > >> >> >>>>> # Example: > >> >> >>>>> # > >> >> >>>>> # -> { "execute": "query-tpm-models" } > >> >> >>>>> -# <- { "return": [ "tpm-tis" ] } > >> >> >>>>> +# <- { "return": [ "tpm-tis", "tpm-crb" ] } > >> >> >>>>> # > >> >> >>>>> ## > >> >> >>>>> { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } > >> >> >>>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h > >> >> >>>>> index 6d516c6a7f..b0048515fa 100644 > >> >> >>>>> --- a/include/hw/acpi/tpm.h > >> >> >>>>> +++ b/include/hw/acpi/tpm.h > >> >> >>>>> @@ -16,11 +16,82 @@ > >> >> >>>>> #ifndef HW_ACPI_TPM_H > >> >> >>>>> #define HW_ACPI_TPM_H > >> >> >>>>> +#include "qemu/osdep.h" > >> >> >>>>> + > >> >> >>>>> #define TPM_TIS_ADDR_BASE 0xFED40000 > >> >> >>>>> #define TPM_TIS_ADDR_SIZE 0x5000 > >> >> >>>>> #define TPM_TIS_IRQ 5 > >> >> >>>>> +struct crb_regs { > >> >> >>>>> + union { > >> >> >>>>> + uint32_t reg; > >> >> >>>>> + struct { > >> >> >>>>> + unsigned tpm_established:1; > >> >> >>>>> + unsigned loc_assigned:1; > >> >> >>>>> + unsigned active_locality:3; > >> >> >>>>> + unsigned reserved:2; > >> >> >>>>> + unsigned tpm_reg_valid_sts:1; > >> >> >>>>> + } bits; > >> >> >>>> > >> >> >>>> I suppose this is little-endian layout, so this won't work on big-endian > >> >> >>>> hosts. > >> >> >>>> > >> >> >>>> Can you add a qtest? > >> >> >>>> > >> >> >>>>> + } loc_state; > >> >> >>>>> + uint32_t reserved1; > >> >> >>>>> + uint32_t loc_ctrl; > >> >> >>>>> + union { > >> >> >>>>> + uint32_t reg; > >> >> >>>>> + struct { > >> >> >>>>> + unsigned granted:1; > >> >> >>>>> + unsigned been_seized:1; > >> >> >>>>> + } bits; > >> >> >>>> > >> >> >>>> > >> >> >>>> This is unclear where you expect those bits (right/left aligned). > >> >> >>>> > >> >> >>>> Can you add an unnamed field to be more explicit? > >> >> >>>> > >> >> >>>> i.e. without using struct if left alignment expected: > >> >> >>>> > >> >> >>>> unsigned granted:1, been_seized:1, :30; > >> >> >>> > >> >> >>> > >> >> >>> > >> >> >>> I got rid of all the bitfields and this patch here makes it work on a > >> >> >>> ppc64 > >> >> >>> big endian and also x86_64 host: > >> >> >>> > >> >> >>> > >> >> >>> https://github.com/stefanberger/qemu-tpm/commit/28fc07f0d9314168986190effd6d72d9fd3972dd > >> >> >>> > >> >> >> Thank you Stefan! I am all for squashing this fix to the patch. You > >> >> >> should then add your signed-off to the commit. > >> >> > > >> >> > > >> >> > I'll do that. > >> >> > > >> >> > The TIS is an ISA Device and the CRB is similar. Considering the > >> >> > >> >> How much similarity is there between TIS and CRB is there? The two > >> >> devices look quite different to me, CRB is way simpler it seems. Or is > >> >> the CRB implementation just lacking many bells and whistles that TIS > >> >> has? Should we consider merging CRB in TIS? > >> >> > >> >> > complications with the sysbus devices where one has to explicitly allow it > >> >> > for a certain machine type, I would advocate to convert the CRB to an ISA > >> >> > device. A patch that does that is this one: > >> >> > >> >> If it's only for that reason (an explicit enable), I would rather keep > >> >> it on the system bus. Or should it be on an LPC bus? > >> >> > >> >> Eduardo, what do you think? > >> > > >> > Everything about sysbus is exceptional and confusing, so I would > >> > prefer to avoid using sysbus every time we have an alternative. > >> > If tpm-tis is already an ISA device, what are the reasons to not > >> > use ISA for tpm-crb too? > >> > >> I was hoping this would make the device more portable (especially on > >> arm) and avoid using legacy bus or resources. > >> > >> Putting it on ISA doesn't reflect better what a real hardware is like, > >> or does it? > > > > I don't know what the hardware interface for those devices look > > like, the documentation I found for TPM-CRB seem to be purely for > > the software interface. > > > > It has a MMIO region at 0xFED4_xxxx, and it can be on LPC or SPI bus > as far as I understand (8.2.2): > > https://trustedcomputinggroup.org/wp-content/uploads/TCG_PC_Client_Platform_TPM_Profile_PTP_Specification_Family_2.0_Revision_1.3v22.pdf Thanks. It looks like the actual hardware interface doesn't matter for QEMU and for guest software, though? > > > Is there a reason to make it a sysbus device instead of a > > bus-less device, then? > > The calls to > sysbus_init_mmio(sbd, &s->mmio); > sysbus_mmio_map(sbd, 0, TPM_CRB_ADDR_BASE); > > Could perhaps be replaced by lower-level memory_region calls? > > I'll give it a try If the code ends up uglier, then this sounds like a good reason to use sysbus, at least until we make the sysbus I/O APIs available to the rest of devices. I would document this in a comment above the ".parent = TYPE_SYSBUS_DEVICE" line, though.
diff --git a/qapi/tpm.json b/qapi/tpm.json index 7093f268fb..d50deef5e9 100644 --- a/qapi/tpm.json +++ b/qapi/tpm.json @@ -11,10 +11,11 @@ # An enumeration of TPM models # # @tpm-tis: TPM TIS model +# @tpm-crb: TPM CRB model (since 2.12) # # Since: 1.5 ## -{ 'enum': 'TpmModel', 'data': [ 'tpm-tis' ] } +{ 'enum': 'TpmModel', 'data': [ 'tpm-tis', 'tpm-crb' ] } ## # @query-tpm-models: @@ -28,7 +29,7 @@ # Example: # # -> { "execute": "query-tpm-models" } -# <- { "return": [ "tpm-tis" ] } +# <- { "return": [ "tpm-tis", "tpm-crb" ] } # ## { 'command': 'query-tpm-models', 'returns': ['TpmModel'] } diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h index 6d516c6a7f..b0048515fa 100644 --- a/include/hw/acpi/tpm.h +++ b/include/hw/acpi/tpm.h @@ -16,11 +16,82 @@ #ifndef HW_ACPI_TPM_H #define HW_ACPI_TPM_H +#include "qemu/osdep.h" + #define TPM_TIS_ADDR_BASE 0xFED40000 #define TPM_TIS_ADDR_SIZE 0x5000 #define TPM_TIS_IRQ 5 +struct crb_regs { + union { + uint32_t reg; + struct { + unsigned tpm_established:1; + unsigned loc_assigned:1; + unsigned active_locality:3; + unsigned reserved:2; + unsigned tpm_reg_valid_sts:1; + } bits; + } loc_state; + uint32_t reserved1; + uint32_t loc_ctrl; + union { + uint32_t reg; + struct { + unsigned granted:1; + unsigned been_seized:1; + } bits; + } loc_sts; + uint8_t reserved2[32]; + union { + uint64_t reg; + struct { + unsigned type:4; + unsigned version:4; + unsigned cap_locality:1; + unsigned cap_crb_idle_bypass:1; + unsigned reserved1:1; + unsigned cap_data_xfer_size_support:2; + unsigned cap_fifo:1; + unsigned cap_crb:1; + unsigned cap_if_res:2; + unsigned if_selector:2; + unsigned if_selector_lock:1; + unsigned reserved2:4; + unsigned rid:8; + unsigned vid:16; + unsigned did:16; + } bits; + } intf_id; + uint64_t ctrl_ext; + + uint32_t ctrl_req; + union { + uint32_t reg; + struct { + unsigned tpm_sts:1; + unsigned tpm_idle:1; + unsigned reserved:30; + } bits; + } ctrl_sts; + uint32_t ctrl_cancel; + uint32_t ctrl_start; + uint32_t ctrl_int_enable; + uint32_t ctrl_int_sts; + uint32_t ctrl_cmd_size; + uint32_t ctrl_cmd_pa_low; + uint32_t ctrl_cmd_pa_high; + uint32_t ctrl_rsp_size; + uint64_t ctrl_rsp_pa; + uint8_t reserved3[0x10]; +} QEMU_PACKED; + +#define TPM_CRB_ADDR_BASE 0xFED40000 +#define TPM_CRB_ADDR_SIZE 0x1000 +#define TPM_CRB_ADDR_CTRL \ + (TPM_CRB_ADDR_BASE + offsetof(struct crb_regs, ctrl_req)) + #define TPM_LOG_AREA_MINIMUM_SIZE (64 * 1024) #define TPM_TCPA_ACPI_CLASS_CLIENT 0 @@ -30,5 +101,6 @@ #define TPM2_ACPI_CLASS_SERVER 1 #define TPM2_START_METHOD_MMIO 6 +#define TPM2_START_METHOD_CRB 7 #endif /* HW_ACPI_TPM_H */ diff --git a/include/sysemu/tpm.h b/include/sysemu/tpm.h index ac04a9d4a2..233b1a3fc3 100644 --- a/include/sysemu/tpm.h +++ b/include/sysemu/tpm.h @@ -46,9 +46,12 @@ typedef struct TPMIfClass { } TPMIfClass; #define TYPE_TPM_TIS "tpm-tis" +#define TYPE_TPM_CRB "tpm-crb" #define TPM_IS_TIS(chr) \ object_dynamic_cast(OBJECT(chr), TYPE_TPM_TIS) +#define TPM_IS_CRB(chr) \ + object_dynamic_cast(OBJECT(chr), TYPE_TPM_CRB) /* returns NULL unless there is exactly one TPM device */ static inline TPMIf *tpm_find(void) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index dc4b2b9ffe..ed78c4ed9f 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -2224,6 +2224,22 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, aml_append(sb_scope, scope); } } + + if (TPM_IS_CRB(tpm_find())) { + dev = aml_device("TPM"); + aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101"))); + crs = aml_resource_template(); + aml_append(crs, aml_memory32_fixed(TPM_CRB_ADDR_BASE, + TPM_CRB_ADDR_SIZE, AML_READ_WRITE)); + aml_append(dev, aml_name_decl("_CRS", crs)); + + method = aml_method("_STA", 0, AML_NOTSERIALIZED); + aml_append(method, aml_return(aml_int(0x0f))); + aml_append(dev, method); + + aml_append(sb_scope, dev); + } + aml_append(dsdt, sb_scope); /* copy AML table into ACPI tables blob and patch header there */ @@ -2285,18 +2301,20 @@ build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog) if (TPM_IS_TIS(tpm_find())) { tpm2_ptr->control_area_address = cpu_to_le64(0); tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_MMIO); - - tpm2_ptr->log_area_minimum_length = - cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); - - /* log area start address to be filled by Guest linker */ - bios_linker_loader_add_pointer(linker, - ACPI_BUILD_TABLE_FILE, log_addr_offset, log_addr_size, - ACPI_BUILD_TPMLOG_FILE, 0); + } else if (TPM_IS_CRB(tpm_find())) { + tpm2_ptr->control_area_address = cpu_to_le64(TPM_CRB_ADDR_CTRL); + tpm2_ptr->start_method = cpu_to_le32(TPM2_START_METHOD_CRB); } else { g_warn_if_reached(); } + tpm2_ptr->log_area_minimum_length = + cpu_to_le32(TPM_LOG_AREA_MINIMUM_SIZE); + + /* log area start address to be filled by Guest linker */ + bios_linker_loader_add_pointer(linker, ACPI_BUILD_TABLE_FILE, + log_addr_offset, log_addr_size, + ACPI_BUILD_TPMLOG_FILE, 0); build_header(linker, table_data, (void *)tpm2_ptr, "TPM2", sizeof(*tpm2_ptr), 4, NULL, NULL); } diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c new file mode 100644 index 0000000000..908ca18d92 --- /dev/null +++ b/hw/tpm/tpm_crb.c @@ -0,0 +1,327 @@ +/* + * tpm_crb.c - QEMU's TPM CRB interface emulator + * + * Copyright (c) 2017 Red Hat, Inc. + * + * Authors: + * Marc-André Lureau <marcandre.lureau@redhat.com> + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * tpm_crb is a device for TPM 2.0 Command Response Buffer (CRB) Interface + * as defined in TCG PC Client Platform TPM Profile (PTP) Specification + * Family “2.0” Level 00 Revision 01.03 v22 + */ + +#include "qemu/osdep.h" + +#include "qemu-common.h" +#include "qapi/error.h" +#include "hw/sysbus.h" +#include "exec/address-spaces.h" + +#include "hw/pci/pci_ids.h" +#include "hw/acpi/tpm.h" +#include "sysemu/tpm_backend.h" +#include "tpm_int.h" +#include "tpm_util.h" + +typedef struct CRBState { + SysBusDevice parent_obj; + + TPMBackend *tpmbe; + TPMBackendCmd cmd; + struct crb_regs regs; + MemoryRegion mmio; + MemoryRegion cmdmem; + + size_t be_buffer_size; +} CRBState; + +#define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB) + +#define DEBUG_CRB 0 + +#define DPRINTF(fmt, ...) do { \ + if (DEBUG_CRB) { \ + printf(fmt, ## __VA_ARGS__); \ + } \ + } while (0) + +#define CRB_ADDR_LOC_STATE offsetof(struct crb_regs, loc_state) +#define CRB_ADDR_LOC_CTRL offsetof(struct crb_regs, loc_ctrl) +#define CRB_ADDR_CTRL_REQ offsetof(struct crb_regs, ctrl_req) +#define CRB_ADDR_CTRL_CANCEL offsetof(struct crb_regs, ctrl_cancel) +#define CRB_ADDR_CTRL_START offsetof(struct crb_regs, ctrl_start) + +#define CRB_INTF_TYPE_CRB_ACTIVE 0b1 +#define CRB_INTF_VERSION_CRB 0b1 +#define CRB_INTF_CAP_LOCALITY_0_ONLY 0b0 +#define CRB_INTF_CAP_IDLE_FAST 0b0 +#define CRB_INTF_CAP_XFER_SIZE_64 0b11 +#define CRB_INTF_CAP_FIFO_NOT_SUPPORTED 0b0 +#define CRB_INTF_CAP_CRB_SUPPORTED 0b1 +#define CRB_INTF_IF_SELECTOR_CRB 0b1 +#define CRB_INTF_IF_SELECTOR_UNLOCKED 0b0 + +#define CRB_CTRL_CMD_SIZE (TPM_CRB_ADDR_SIZE - sizeof(struct crb_regs)) + +enum crb_loc_ctrl { + CRB_LOC_CTRL_REQUEST_ACCESS = BIT(0), + CRB_LOC_CTRL_RELINQUISH = BIT(1), + CRB_LOC_CTRL_SEIZE = BIT(2), + CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT = BIT(3), +}; + +enum crb_ctrl_req { + CRB_CTRL_REQ_CMD_READY = BIT(0), + CRB_CTRL_REQ_GO_IDLE = BIT(1), +}; + +enum crb_start { + CRB_START_INVOKE = BIT(0), +}; + +enum crb_cancel { + CRB_CANCEL_INVOKE = BIT(0), +}; + +static const char *addr_desc(unsigned off) +{ + struct crb_regs crb; + + switch (off) { +#define CASE(field) \ + case offsetof(struct crb_regs, field) ... \ + offsetof(struct crb_regs, field) + sizeof(crb.field) - 1: \ + return G_STRINGIFY(field); + CASE(loc_state); + CASE(reserved1); + CASE(loc_ctrl); + CASE(loc_sts); + CASE(reserved2); + CASE(intf_id); + CASE(ctrl_ext); + CASE(ctrl_req); + CASE(ctrl_sts); + CASE(ctrl_cancel); + CASE(ctrl_start); + CASE(ctrl_int_enable); + CASE(ctrl_int_sts); + CASE(ctrl_cmd_size); + CASE(ctrl_cmd_pa_low); + CASE(ctrl_cmd_pa_high); + CASE(ctrl_rsp_size); + CASE(ctrl_rsp_pa); +#undef CASE + } + return NULL; +} + +static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, + unsigned size) +{ + CRBState *s = CRB(opaque); + DPRINTF("CRB read 0x%lx:%s len:%u\n", + addr, addr_desc(addr), size); + void *regs = (void *)&s->regs + addr; + + switch (size) { + case 1: + return *(uint8_t *)regs; + case 2: + return *(uint16_t *)regs; + case 4: + return *(uint32_t *)regs; + default: + g_return_val_if_reached(-1); + } +} + +static void tpm_crb_mmio_write(void *opaque, hwaddr addr, + uint64_t val, unsigned size) +{ + CRBState *s = CRB(opaque); + DPRINTF("CRB write 0x%lx:%s len:%u val:%lu\n", + addr, addr_desc(addr), size, val); + + switch (addr) { + case CRB_ADDR_CTRL_REQ: + switch (val) { + case CRB_CTRL_REQ_CMD_READY: + s->regs.ctrl_sts.bits.tpm_idle = 0; + break; + case CRB_CTRL_REQ_GO_IDLE: + s->regs.ctrl_sts.bits.tpm_idle = 1; + break; + } + break; + case CRB_ADDR_CTRL_CANCEL: + if (val == CRB_CANCEL_INVOKE && s->regs.ctrl_start & CRB_START_INVOKE) { + tpm_backend_cancel_cmd(s->tpmbe); + } + break; + case CRB_ADDR_CTRL_START: + if (val == CRB_START_INVOKE && + !(s->regs.ctrl_start & CRB_START_INVOKE)) { + void *mem = memory_region_get_ram_ptr(&s->cmdmem); + + s->regs.ctrl_start |= CRB_START_INVOKE; + s->cmd = (TPMBackendCmd) { + .in = mem, + .in_len = MIN(tpm_cmd_get_size(mem), s->be_buffer_size), + .out = mem, + .out_len = s->be_buffer_size, + }; + + tpm_backend_deliver_request(s->tpmbe, &s->cmd); + } + break; + case CRB_ADDR_LOC_CTRL: + switch (val) { + case CRB_LOC_CTRL_RESET_ESTABLISHMENT_BIT: + /* not loc 3 or 4 */ + break; + case CRB_LOC_CTRL_RELINQUISH: + break; + case CRB_LOC_CTRL_REQUEST_ACCESS: + s->regs.loc_sts.bits.granted = 1; + s->regs.loc_sts.bits.been_seized = 0; + s->regs.loc_state.bits.loc_assigned = 1; + s->regs.loc_state.bits.tpm_reg_valid_sts = 1; + break; + } + break; + } +} + +static const MemoryRegionOps tpm_crb_memory_ops = { + .read = tpm_crb_mmio_read, + .write = tpm_crb_mmio_write, + .endianness = DEVICE_LITTLE_ENDIAN, + .valid = { + .min_access_size = 1, + .max_access_size = 4, + }, +}; + +static void tpm_crb_reset(DeviceState *dev) +{ + CRBState *s = CRB(dev); + + tpm_backend_reset(s->tpmbe); + + s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), + CRB_CTRL_CMD_SIZE); + + s->regs = (struct crb_regs) { + .intf_id.bits = { + .type = CRB_INTF_TYPE_CRB_ACTIVE, + .version = CRB_INTF_VERSION_CRB, + .cap_locality = CRB_INTF_CAP_LOCALITY_0_ONLY, + .cap_crb_idle_bypass = CRB_INTF_CAP_IDLE_FAST, + .cap_data_xfer_size_support = CRB_INTF_CAP_XFER_SIZE_64, + .cap_fifo = CRB_INTF_CAP_FIFO_NOT_SUPPORTED, + .cap_crb = CRB_INTF_CAP_CRB_SUPPORTED, + .cap_if_res = 0b0, + .if_selector = CRB_INTF_IF_SELECTOR_CRB, + .if_selector_lock = CRB_INTF_IF_SELECTOR_UNLOCKED, + .rid = 0b0001, + .vid = PCI_VENDOR_ID_IBM, + .did = 0b0001, + }, + .ctrl_cmd_size = CRB_CTRL_CMD_SIZE, + .ctrl_cmd_pa_low = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), + .ctrl_rsp_size = CRB_CTRL_CMD_SIZE, + .ctrl_rsp_pa = TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), + }; + + tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size); +} + +static void tpm_crb_request_completed(TPMIf *ti, int ret) +{ + CRBState *s = CRB(ti); + + s->regs.ctrl_start &= ~CRB_START_INVOKE; + if (ret != 0) { + s->regs.ctrl_sts.bits.tpm_sts = 1; /* fatal error */ + } +} + +static enum TPMVersion tpm_crb_get_version(TPMIf *ti) +{ + CRBState *s = CRB(ti); + + return tpm_backend_get_tpm_version(s->tpmbe); +} + +static const VMStateDescription vmstate_tpm_crb = { + .name = "tpm-crb", + .unmigratable = 1, +}; + +static Property tpm_crb_properties[] = { + DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe), + DEFINE_PROP_END_OF_LIST(), +}; + +static void tpm_crb_realizefn(DeviceState *dev, Error **errp) +{ + CRBState *s = CRB(dev); + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); + + if (!tpm_find()) { + error_setg(errp, "at most one TPM device is permitted"); + return; + } + if (!s->tpmbe) { + error_setg(errp, "'tpmdev' property is required"); + return; + } + + memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s, + "tpm-crb-mmio", sizeof(struct crb_regs)); + memory_region_init_ram(&s->cmdmem, OBJECT(s), + "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); + + sysbus_init_mmio(sbd, &s->mmio); + sysbus_mmio_map(sbd, 0, TPM_CRB_ADDR_BASE); + /* allocate ram in bios instead? */ + memory_region_add_subregion(get_system_memory(), + TPM_CRB_ADDR_BASE + sizeof(struct crb_regs), &s->cmdmem); +} + +static void tpm_crb_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + TPMIfClass *tc = TPM_IF_CLASS(klass); + + dc->realize = tpm_crb_realizefn; + dc->props = tpm_crb_properties; + dc->reset = tpm_crb_reset; + dc->vmsd = &vmstate_tpm_crb; + dc->user_creatable = true; + tc->model = TPM_MODEL_TPM_CRB; + tc->get_version = tpm_crb_get_version; + tc->request_completed = tpm_crb_request_completed; +} + +static const TypeInfo tpm_crb_info = { + .name = TYPE_TPM_CRB, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(CRBState), + .class_init = tpm_crb_class_init, + .interfaces = (InterfaceInfo[]) { + { TYPE_TPM_IF }, + { } + } +}; + +static void tpm_crb_register(void) +{ + type_register_static(&tpm_crb_info); +} + +type_init(tpm_crb_register) diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 95ac4b464a..ac27700e79 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y CONFIG_I8259=y CONFIG_PFLASH_CFI01=y CONFIG_TPM_TIS=$(CONFIG_TPM) +CONFIG_TPM_CRB=$(CONFIG_TPM) CONFIG_MC146818RTC=y CONFIG_PCI_PIIX=y CONFIG_WDT_IB700=y diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak index 0221236825..b2104ade19 100644 --- a/default-configs/x86_64-softmmu.mak +++ b/default-configs/x86_64-softmmu.mak @@ -37,6 +37,7 @@ CONFIG_APPLESMC=y CONFIG_I8259=y CONFIG_PFLASH_CFI01=y CONFIG_TPM_TIS=$(CONFIG_TPM) +CONFIG_TPM_CRB=$(CONFIG_TPM) CONFIG_MC146818RTC=y CONFIG_PCI_PIIX=y CONFIG_WDT_IB700=y diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs index 7a93b24636..1dc9f8bf2c 100644 --- a/hw/tpm/Makefile.objs +++ b/hw/tpm/Makefile.objs @@ -1,4 +1,5 @@ common-obj-y += tpm_util.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 common-obj-$(CONFIG_TPM_EMULATOR) += tpm_emulator.o