| Message ID | 20231031040021.65582-5-j@getutm.app |
|---|---|
| State | New |
| Headers | show |
| Series | tpm: introduce TPM CRB SysBus device | expand |
On 10/31/23 00:00, Joelle van Dyne wrote: > On Apple Silicon, when Windows performs a LDP on the CRB MMIO space, > the exception is not decoded by hardware and we cannot trap the MMIO > read. This led to the idea from @agraf to use the same mapping type as > ROM devices: namely that reads should be seen as memory type and > writes should trap as MMIO. > > Once that was done, the second memory mapping of the command buffer > region was redundent and was removed. > > A note about the removal of the read trap for `CRB_LOC_STATE`: > The only usage was to return the most up-to-date value for > `tpmEstablished`. However, `tpmEstablished` is only cleared when a > TPM2_HashStart operation is called which only exists for locality 4. > We do not handle locality 4. Indeed, the comment for the write handler > of `CRB_LOC_CTRL` makes the same argument for why it is not calling > the backend to reset the `tpmEstablished` bit (to 1). > As this bit is unused, we do not need to worry about updating it for > reads. > > In order to maintain migration compatibility with older versions of > QEMU, we store a copy of the register data and command data which is > used only during save/restore. > > Signed-off-by: Joelle van Dyne <j@getutm.app> > --- > diff --git a/hw/tpm/tpm_crb_common.c b/hw/tpm/tpm_crb_common.c > index bee0b71fee..605e8576e9 100644 > --- a/hw/tpm/tpm_crb_common.c > +++ b/hw/tpm/tpm_crb_common.c > @@ -31,31 +31,12 @@ > #include "qom/object.h" > #include "tpm_crb.h" > > -static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, > - unsigned size) > +static uint8_t tpm_crb_get_active_locty(TPMCRBState *s, uint32_t *regs) > { > - TPMCRBState *s = opaque; > - void *regs = (void *)&s->regs + (addr & ~3); > - unsigned offset = addr & 3; > - uint32_t val = *(uint32_t *)regs >> (8 * offset); > - > - switch (addr) { > - case A_CRB_LOC_STATE: > - val |= !tpm_backend_get_tpm_established_flag(s->tpmbe); > - break; > - } > - > - trace_tpm_crb_mmio_read(addr, size, val); > - > - return val; > -} > - > -static uint8_t tpm_crb_get_active_locty(TPMCRBState *s) > -{ > - if (!ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, locAssigned)) { > + if (!ARRAY_FIELD_EX32(regs, CRB_LOC_STATE, locAssigned)) { > return TPM_CRB_NO_LOCALITY; > } > - return ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, activeLocality); > + return ARRAY_FIELD_EX32(regs, CRB_LOC_STATE, activeLocality); > } > > static void tpm_crb_mmio_write(void *opaque, hwaddr addr, > @@ -63,35 +44,47 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, > { > TPMCRBState *s = opaque; > uint8_t locty = addr >> 12; > + uint32_t *regs; > + void *mem; > > trace_tpm_crb_mmio_write(addr, size, val); > + regs = memory_region_get_ram_ptr(&s->mmio); > + mem = ®s[R_CRB_DATA_BUFFER]; > + assert(regs); > + > + if (addr >= A_CRB_DATA_BUFFER) { Can you write here /* receive TPM command bytes */ ? > + assert(addr + size <= TPM_CRB_ADDR_SIZE); > + assert(size <= sizeof(val)); > + memcpy(mem + addr - A_CRB_DATA_BUFFER, &val, size); > + memory_region_set_dirty(&s->mmio, addr, size); > + return; > + } > > switch (addr) { > case A_CRB_CTRL_REQ: > switch (val) { > case CRB_CTRL_REQ_CMD_READY: > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > tpmIdle, 0); > break; > case CRB_CTRL_REQ_GO_IDLE: > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > tpmIdle, 1); > break; > } > break; > case A_CRB_CTRL_CANCEL: > if (val == CRB_CANCEL_INVOKE && > - s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) { > + regs[R_CRB_CTRL_START] & CRB_START_INVOKE) { > tpm_backend_cancel_cmd(s->tpmbe); > } > break; > case A_CRB_CTRL_START: > if (val == CRB_START_INVOKE && > - !(s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) && > - tpm_crb_get_active_locty(s) == locty) { > - void *mem = memory_region_get_ram_ptr(&s->cmdmem); > + !(regs[R_CRB_CTRL_START] & CRB_START_INVOKE) && > + tpm_crb_get_active_locty(s, regs) == locty) { > > - s->regs[R_CRB_CTRL_START] |= CRB_START_INVOKE; > + regs[R_CRB_CTRL_START] |= CRB_START_INVOKE; > s->cmd = (TPMBackendCmd) { > .in = mem, > .in_len = MIN(tpm_cmd_get_size(mem), s->be_buffer_size), > @@ -108,26 +101,27 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, > /* not loc 3 or 4 */ > break; > case CRB_LOC_CTRL_RELINQUISH: > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > locAssigned, 0); > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, > + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, > Granted, 0); > break; > case CRB_LOC_CTRL_REQUEST_ACCESS: > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, > + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, > Granted, 1); > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, > + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, > beenSeized, 0); > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > locAssigned, 1); > break; > } > break; > } > + > + memory_region_set_dirty(&s->mmio, 0, A_CRB_DATA_BUFFER); > } > > const MemoryRegionOps tpm_crb_memory_ops = { > - .read = tpm_crb_mmio_read, > .write = tpm_crb_mmio_write, > .endianness = DEVICE_LITTLE_ENDIAN, > .valid = { > @@ -138,12 +132,16 @@ const MemoryRegionOps tpm_crb_memory_ops = { > > void tpm_crb_request_completed(TPMCRBState *s, int ret) > { > - s->regs[R_CRB_CTRL_START] &= ~CRB_START_INVOKE; > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > + > + assert(regs); > + regs[R_CRB_CTRL_START] &= ~CRB_START_INVOKE; > if (ret != 0) { > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > tpmSts, 1); /* fatal error */ > } > - memory_region_set_dirty(&s->cmdmem, 0, CRB_CTRL_CMD_SIZE); > + > + memory_region_set_dirty(&s->mmio, 0, TPM_CRB_ADDR_SIZE); > } > > enum TPMVersion tpm_crb_get_version(TPMCRBState *s) > @@ -160,45 +158,50 @@ int tpm_crb_pre_save(TPMCRBState *s) > > void tpm_crb_reset(TPMCRBState *s, uint64_t baseaddr) > { > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > + > + assert(regs); > if (s->ppi_enabled) { > tpm_ppi_reset(&s->ppi); > } > tpm_backend_reset(s->tpmbe); > > - memset(s->regs, 0, sizeof(s->regs)); > + memset(regs, 0, TPM_CRB_ADDR_SIZE); > > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > tpmRegValidSts, 1); > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > + tpmEstablished, 1); > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > tpmIdle, 1); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > InterfaceVersion, CRB_INTF_VERSION_CRB); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > CapLocality, CRB_INTF_CAP_LOCALITY_0_ONLY); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > CapCRBIdleBypass, CRB_INTF_CAP_IDLE_FAST); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > CapDataXferSizeSupport, CRB_INTF_CAP_XFER_SIZE_64); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > CapFIFO, CRB_INTF_CAP_FIFO_NOT_SUPPORTED); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > CapCRB, CRB_INTF_CAP_CRB_SUPPORTED); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > InterfaceSelector, CRB_INTF_IF_SELECTOR_CRB); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > RID, 0b0000); > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID2, > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID2, > VID, PCI_VENDOR_ID_IBM); > > baseaddr += A_CRB_DATA_BUFFER; > - s->regs[R_CRB_CTRL_CMD_SIZE] = CRB_CTRL_CMD_SIZE; > - s->regs[R_CRB_CTRL_CMD_LADDR] = (uint32_t)baseaddr; > - s->regs[R_CRB_CTRL_CMD_HADDR] = (uint32_t)(baseaddr >> 32); > - s->regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE; > - s->regs[R_CRB_CTRL_RSP_LADDR] = (uint32_t)baseaddr; > - s->regs[R_CRB_CTRL_RSP_HADDR] = (uint32_t)(baseaddr >> 32); > + regs[R_CRB_CTRL_CMD_SIZE] = CRB_CTRL_CMD_SIZE; > + regs[R_CRB_CTRL_CMD_LADDR] = (uint32_t)baseaddr; > + regs[R_CRB_CTRL_CMD_HADDR] = (uint32_t)(baseaddr >> 32); > + regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE; > + regs[R_CRB_CTRL_RSP_LADDR] = (uint32_t)baseaddr; > + regs[R_CRB_CTRL_RSP_HADDR] = (uint32_t)(baseaddr >> 32); > > s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), > CRB_CTRL_CMD_SIZE); > @@ -206,15 +209,33 @@ void tpm_crb_reset(TPMCRBState *s, uint64_t baseaddr) > if (tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size) < 0) { > exit(1); > } > + > + memory_region_rom_device_set_romd(&s->mmio, true); > + memory_region_set_dirty(&s->mmio, 0, TPM_CRB_ADDR_SIZE); > } > > void tpm_crb_init_memory(Object *obj, TPMCRBState *s, Error **errp) > { > - memory_region_init_io(&s->mmio, obj, &tpm_crb_memory_ops, s, > - "tpm-crb-mmio", sizeof(s->regs)); > - memory_region_init_ram(&s->cmdmem, obj, > - "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); > + memory_region_init_rom_device_nomigrate(&s->mmio, obj, &tpm_crb_memory_ops, > + s, "tpm-crb-mem", TPM_CRB_ADDR_SIZE, errp); > if (s->ppi_enabled) { > tpm_ppi_init_memory(&s->ppi, obj); > } > } > + > +void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void *saved_cmdmem) > +{ > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > + > + memcpy(saved_regs, regs, TPM_CRB_R_MAX); > + memcpy(saved_cmdmem, ®s[R_CRB_DATA_BUFFER], A_CRB_DATA_BUFFER); I find it confusing that this function is here rather than in tpm_crb_non_pre_save(). The size should be CRB_CTRL_CMD_SIZE. > +} > + > +void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, > + const void *saved_cmdmem) > +{ > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > + > + memcpy(regs, saved_regs, TPM_CRB_R_MAX); > + memcpy(®s[R_CRB_DATA_BUFFER], saved_cmdmem, A_CRB_DATA_BUFFER); > +} Same comments; size seems wrong.
On Wed, Nov 1, 2023 at 2:25 PM Stefan Berger <stefanb@linux.ibm.com> wrote: > > > > On 10/31/23 00:00, Joelle van Dyne wrote: > > On Apple Silicon, when Windows performs a LDP on the CRB MMIO space, > > the exception is not decoded by hardware and we cannot trap the MMIO > > read. This led to the idea from @agraf to use the same mapping type as > > ROM devices: namely that reads should be seen as memory type and > > writes should trap as MMIO. > > > > Once that was done, the second memory mapping of the command buffer > > region was redundent and was removed. > > > > A note about the removal of the read trap for `CRB_LOC_STATE`: > > The only usage was to return the most up-to-date value for > > `tpmEstablished`. However, `tpmEstablished` is only cleared when a > > TPM2_HashStart operation is called which only exists for locality 4. > > We do not handle locality 4. Indeed, the comment for the write handler > > of `CRB_LOC_CTRL` makes the same argument for why it is not calling > > the backend to reset the `tpmEstablished` bit (to 1). > > As this bit is unused, we do not need to worry about updating it for > > reads. > > > > In order to maintain migration compatibility with older versions of > > QEMU, we store a copy of the register data and command data which is > > used only during save/restore. > > > > Signed-off-by: Joelle van Dyne <j@getutm.app> > > --- > > > diff --git a/hw/tpm/tpm_crb_common.c b/hw/tpm/tpm_crb_common.c > > index bee0b71fee..605e8576e9 100644 > > --- a/hw/tpm/tpm_crb_common.c > > +++ b/hw/tpm/tpm_crb_common.c > > @@ -31,31 +31,12 @@ > > #include "qom/object.h" > > #include "tpm_crb.h" > > > > -static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, > > - unsigned size) > > +static uint8_t tpm_crb_get_active_locty(TPMCRBState *s, uint32_t *regs) > > { > > - TPMCRBState *s = opaque; > > - void *regs = (void *)&s->regs + (addr & ~3); > > - unsigned offset = addr & 3; > > - uint32_t val = *(uint32_t *)regs >> (8 * offset); > > - > > - switch (addr) { > > - case A_CRB_LOC_STATE: > > - val |= !tpm_backend_get_tpm_established_flag(s->tpmbe); > > - break; > > - } > > - > > - trace_tpm_crb_mmio_read(addr, size, val); > > - > > - return val; > > -} > > - > > -static uint8_t tpm_crb_get_active_locty(TPMCRBState *s) > > -{ > > - if (!ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, locAssigned)) { > > + if (!ARRAY_FIELD_EX32(regs, CRB_LOC_STATE, locAssigned)) { > > return TPM_CRB_NO_LOCALITY; > > } > > - return ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, activeLocality); > > + return ARRAY_FIELD_EX32(regs, CRB_LOC_STATE, activeLocality); > > } > > > > static void tpm_crb_mmio_write(void *opaque, hwaddr addr, > > @@ -63,35 +44,47 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, > > { > > TPMCRBState *s = opaque; > > uint8_t locty = addr >> 12; > > + uint32_t *regs; > > + void *mem; > > > > trace_tpm_crb_mmio_write(addr, size, val); > > + regs = memory_region_get_ram_ptr(&s->mmio); > > + mem = ®s[R_CRB_DATA_BUFFER]; > > + assert(regs); > > + > > + if (addr >= A_CRB_DATA_BUFFER) { > > > Can you write here /* receive TPM command bytes */ ? Will do. > > > > + assert(addr + size <= TPM_CRB_ADDR_SIZE); > > + assert(size <= sizeof(val)); > > + memcpy(mem + addr - A_CRB_DATA_BUFFER, &val, size); > > > + memory_region_set_dirty(&s->mmio, addr, size); > > + return; > > + } > > > > switch (addr) { > > case A_CRB_CTRL_REQ: > > switch (val) { > > case CRB_CTRL_REQ_CMD_READY: > > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > > tpmIdle, 0); > > break; > > case CRB_CTRL_REQ_GO_IDLE: > > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > > tpmIdle, 1); > > break; > > } > > break; > > case A_CRB_CTRL_CANCEL: > > if (val == CRB_CANCEL_INVOKE && > > - s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) { > > + regs[R_CRB_CTRL_START] & CRB_START_INVOKE) { > > tpm_backend_cancel_cmd(s->tpmbe); > > } > > break; > > case A_CRB_CTRL_START: > > if (val == CRB_START_INVOKE && > > - !(s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) && > > - tpm_crb_get_active_locty(s) == locty) { > > - void *mem = memory_region_get_ram_ptr(&s->cmdmem); > > + !(regs[R_CRB_CTRL_START] & CRB_START_INVOKE) && > > + tpm_crb_get_active_locty(s, regs) == locty) { > > > > - s->regs[R_CRB_CTRL_START] |= CRB_START_INVOKE; > > + regs[R_CRB_CTRL_START] |= CRB_START_INVOKE; > > s->cmd = (TPMBackendCmd) { > > .in = mem, > > .in_len = MIN(tpm_cmd_get_size(mem), s->be_buffer_size), > > @@ -108,26 +101,27 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, > > /* not loc 3 or 4 */ > > break; > > case CRB_LOC_CTRL_RELINQUISH: > > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, > > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > > locAssigned, 0); > > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, > > + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, > > Granted, 0); > > break; > > case CRB_LOC_CTRL_REQUEST_ACCESS: > > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, > > + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, > > Granted, 1); > > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, > > + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, > > beenSeized, 0); > > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, > > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > > locAssigned, 1); > > break; > > } > > break; > > } > > + > > + memory_region_set_dirty(&s->mmio, 0, A_CRB_DATA_BUFFER); > > } > > > > const MemoryRegionOps tpm_crb_memory_ops = { > > - .read = tpm_crb_mmio_read, > > .write = tpm_crb_mmio_write, > > .endianness = DEVICE_LITTLE_ENDIAN, > > .valid = { > > @@ -138,12 +132,16 @@ const MemoryRegionOps tpm_crb_memory_ops = { > > > > void tpm_crb_request_completed(TPMCRBState *s, int ret) > > { > > - s->regs[R_CRB_CTRL_START] &= ~CRB_START_INVOKE; > > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > > + > > + assert(regs); > > + regs[R_CRB_CTRL_START] &= ~CRB_START_INVOKE; > > if (ret != 0) { > > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > > tpmSts, 1); /* fatal error */ > > } > > - memory_region_set_dirty(&s->cmdmem, 0, CRB_CTRL_CMD_SIZE); > > + > > + memory_region_set_dirty(&s->mmio, 0, TPM_CRB_ADDR_SIZE); > > } > > > > enum TPMVersion tpm_crb_get_version(TPMCRBState *s) > > @@ -160,45 +158,50 @@ int tpm_crb_pre_save(TPMCRBState *s) > > > > void tpm_crb_reset(TPMCRBState *s, uint64_t baseaddr) > > { > > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > > + > > + assert(regs); > > if (s->ppi_enabled) { > > tpm_ppi_reset(&s->ppi); > > } > > tpm_backend_reset(s->tpmbe); > > > > - memset(s->regs, 0, sizeof(s->regs)); > > + memset(regs, 0, TPM_CRB_ADDR_SIZE); > > > > - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, > > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > > tpmRegValidSts, 1); > > - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, > > + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, > > + tpmEstablished, 1); > > + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, > > tpmIdle, 1); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > InterfaceVersion, CRB_INTF_VERSION_CRB); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > CapLocality, CRB_INTF_CAP_LOCALITY_0_ONLY); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > CapCRBIdleBypass, CRB_INTF_CAP_IDLE_FAST); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > CapDataXferSizeSupport, CRB_INTF_CAP_XFER_SIZE_64); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > CapFIFO, CRB_INTF_CAP_FIFO_NOT_SUPPORTED); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > CapCRB, CRB_INTF_CAP_CRB_SUPPORTED); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > InterfaceSelector, CRB_INTF_IF_SELECTOR_CRB); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, > > RID, 0b0000); > > - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID2, > > + ARRAY_FIELD_DP32(regs, CRB_INTF_ID2, > > VID, PCI_VENDOR_ID_IBM); > > > > baseaddr += A_CRB_DATA_BUFFER; > > - s->regs[R_CRB_CTRL_CMD_SIZE] = CRB_CTRL_CMD_SIZE; > > - s->regs[R_CRB_CTRL_CMD_LADDR] = (uint32_t)baseaddr; > > - s->regs[R_CRB_CTRL_CMD_HADDR] = (uint32_t)(baseaddr >> 32); > > - s->regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE; > > - s->regs[R_CRB_CTRL_RSP_LADDR] = (uint32_t)baseaddr; > > - s->regs[R_CRB_CTRL_RSP_HADDR] = (uint32_t)(baseaddr >> 32); > > + regs[R_CRB_CTRL_CMD_SIZE] = CRB_CTRL_CMD_SIZE; > > + regs[R_CRB_CTRL_CMD_LADDR] = (uint32_t)baseaddr; > > + regs[R_CRB_CTRL_CMD_HADDR] = (uint32_t)(baseaddr >> 32); > > + regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE; > > + regs[R_CRB_CTRL_RSP_LADDR] = (uint32_t)baseaddr; > > + regs[R_CRB_CTRL_RSP_HADDR] = (uint32_t)(baseaddr >> 32); > > > > s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), > > CRB_CTRL_CMD_SIZE); > > @@ -206,15 +209,33 @@ void tpm_crb_reset(TPMCRBState *s, uint64_t baseaddr) > > if (tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size) < 0) { > > exit(1); > > } > > + > > + memory_region_rom_device_set_romd(&s->mmio, true); > > + memory_region_set_dirty(&s->mmio, 0, TPM_CRB_ADDR_SIZE); > > } > > > > void tpm_crb_init_memory(Object *obj, TPMCRBState *s, Error **errp) > > { > > - memory_region_init_io(&s->mmio, obj, &tpm_crb_memory_ops, s, > > - "tpm-crb-mmio", sizeof(s->regs)); > > - memory_region_init_ram(&s->cmdmem, obj, > > - "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); > > + memory_region_init_rom_device_nomigrate(&s->mmio, obj, &tpm_crb_memory_ops, > > + s, "tpm-crb-mem", TPM_CRB_ADDR_SIZE, errp); > > if (s->ppi_enabled) { > > tpm_ppi_init_memory(&s->ppi, obj); > > } > > } > > + > > +void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void *saved_cmdmem) > > +{ > > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > > + > > + memcpy(saved_regs, regs, TPM_CRB_R_MAX); > > + memcpy(saved_cmdmem, ®s[R_CRB_DATA_BUFFER], A_CRB_DATA_BUFFER); > > I find it confusing that this function is here rather than in > tpm_crb_non_pre_save(). I factored it that way to be consistent with the other callbacks in -common. The idea is that the -common code has no visibility into the instance specific data structure and therefore needs to get that info (saved_regs, saved_cmdmem) from the concrete instance. > > The size should be CRB_CTRL_CMD_SIZE. Good catch, will fix. > > > +} > > + > > +void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, > > + const void *saved_cmdmem) > > +{ > > + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); > > + > > + memcpy(regs, saved_regs, TPM_CRB_R_MAX); > > + memcpy(®s[R_CRB_DATA_BUFFER], saved_cmdmem, A_CRB_DATA_BUFFER); > > +} > > Same comments; size seems wrong.
diff --git a/hw/tpm/tpm_crb.h b/hw/tpm/tpm_crb.h index da3a0cf256..36863e1664 100644 --- a/hw/tpm/tpm_crb.h +++ b/hw/tpm/tpm_crb.h @@ -26,9 +26,7 @@ typedef struct TPMCRBState { TPMBackend *tpmbe; TPMBackendCmd cmd; - uint32_t regs[TPM_CRB_R_MAX]; MemoryRegion mmio; - MemoryRegion cmdmem; size_t be_buffer_size; @@ -72,5 +70,8 @@ enum TPMVersion tpm_crb_get_version(TPMCRBState *s); int tpm_crb_pre_save(TPMCRBState *s); void tpm_crb_reset(TPMCRBState *s, uint64_t baseaddr); void tpm_crb_init_memory(Object *obj, TPMCRBState *s, Error **errp); +void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void *saved_cmdmem); +void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, + const void *saved_cmdmem); #endif /* TPM_TPM_CRB_H */ diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c index 598c3e0161..99c64dd72a 100644 --- a/hw/tpm/tpm_crb.c +++ b/hw/tpm/tpm_crb.c @@ -37,6 +37,10 @@ struct CRBState { DeviceState parent_obj; TPMCRBState state; + + /* These states are only for migration */ + uint32_t saved_regs[TPM_CRB_R_MAX]; + MemoryRegion saved_cmdmem; }; typedef struct CRBState CRBState; @@ -57,18 +61,36 @@ static enum TPMVersion tpm_crb_none_get_version(TPMIf *ti) return tpm_crb_get_version(&s->state); } +/** + * For migrating to an older version of QEMU + */ static int tpm_crb_none_pre_save(void *opaque) { CRBState *s = opaque; + void *saved_cmdmem = memory_region_get_ram_ptr(&s->saved_cmdmem); + tpm_crb_mem_save(&s->state, s->saved_regs, saved_cmdmem); return tpm_crb_pre_save(&s->state); } +/** + * For migrating from an older version of QEMU + */ +static int tpm_crb_none_post_load(void *opaque, int version_id) +{ + CRBState *s = opaque; + void *saved_cmdmem = memory_region_get_ram_ptr(&s->saved_cmdmem); + + tpm_crb_mem_load(&s->state, s->saved_regs, saved_cmdmem); + return 0; +} + static const VMStateDescription vmstate_tpm_crb_none = { .name = "tpm-crb", .pre_save = tpm_crb_none_pre_save, + .post_load = tpm_crb_none_post_load, .fields = (VMStateField[]) { - VMSTATE_UINT32_ARRAY(state.regs, CRBState, TPM_CRB_R_MAX), + VMSTATE_UINT32_ARRAY(saved_regs, CRBState, TPM_CRB_R_MAX), VMSTATE_END_OF_LIST(), } }; @@ -101,10 +123,12 @@ static void tpm_crb_none_realize(DeviceState *dev, Error **errp) tpm_crb_init_memory(OBJECT(s), &s->state, errp); + /* only used for migration */ + memory_region_init_ram(&s->saved_cmdmem, OBJECT(s), + "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); + memory_region_add_subregion(get_system_memory(), TPM_CRB_ADDR_BASE, &s->state.mmio); - memory_region_add_subregion(get_system_memory(), - TPM_CRB_ADDR_BASE + sizeof(s->state.regs), &s->state.cmdmem); if (s->state.ppi_enabled) { memory_region_add_subregion(get_system_memory(), diff --git a/hw/tpm/tpm_crb_common.c b/hw/tpm/tpm_crb_common.c index bee0b71fee..605e8576e9 100644 --- a/hw/tpm/tpm_crb_common.c +++ b/hw/tpm/tpm_crb_common.c @@ -31,31 +31,12 @@ #include "qom/object.h" #include "tpm_crb.h" -static uint64_t tpm_crb_mmio_read(void *opaque, hwaddr addr, - unsigned size) +static uint8_t tpm_crb_get_active_locty(TPMCRBState *s, uint32_t *regs) { - TPMCRBState *s = opaque; - void *regs = (void *)&s->regs + (addr & ~3); - unsigned offset = addr & 3; - uint32_t val = *(uint32_t *)regs >> (8 * offset); - - switch (addr) { - case A_CRB_LOC_STATE: - val |= !tpm_backend_get_tpm_established_flag(s->tpmbe); - break; - } - - trace_tpm_crb_mmio_read(addr, size, val); - - return val; -} - -static uint8_t tpm_crb_get_active_locty(TPMCRBState *s) -{ - if (!ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, locAssigned)) { + if (!ARRAY_FIELD_EX32(regs, CRB_LOC_STATE, locAssigned)) { return TPM_CRB_NO_LOCALITY; } - return ARRAY_FIELD_EX32(s->regs, CRB_LOC_STATE, activeLocality); + return ARRAY_FIELD_EX32(regs, CRB_LOC_STATE, activeLocality); } static void tpm_crb_mmio_write(void *opaque, hwaddr addr, @@ -63,35 +44,47 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, { TPMCRBState *s = opaque; uint8_t locty = addr >> 12; + uint32_t *regs; + void *mem; trace_tpm_crb_mmio_write(addr, size, val); + regs = memory_region_get_ram_ptr(&s->mmio); + mem = ®s[R_CRB_DATA_BUFFER]; + assert(regs); + + if (addr >= A_CRB_DATA_BUFFER) { + assert(addr + size <= TPM_CRB_ADDR_SIZE); + assert(size <= sizeof(val)); + memcpy(mem + addr - A_CRB_DATA_BUFFER, &val, size); + memory_region_set_dirty(&s->mmio, addr, size); + return; + } switch (addr) { case A_CRB_CTRL_REQ: switch (val) { case CRB_CTRL_REQ_CMD_READY: - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, tpmIdle, 0); break; case CRB_CTRL_REQ_GO_IDLE: - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, tpmIdle, 1); break; } break; case A_CRB_CTRL_CANCEL: if (val == CRB_CANCEL_INVOKE && - s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) { + regs[R_CRB_CTRL_START] & CRB_START_INVOKE) { tpm_backend_cancel_cmd(s->tpmbe); } break; case A_CRB_CTRL_START: if (val == CRB_START_INVOKE && - !(s->regs[R_CRB_CTRL_START] & CRB_START_INVOKE) && - tpm_crb_get_active_locty(s) == locty) { - void *mem = memory_region_get_ram_ptr(&s->cmdmem); + !(regs[R_CRB_CTRL_START] & CRB_START_INVOKE) && + tpm_crb_get_active_locty(s, regs) == locty) { - s->regs[R_CRB_CTRL_START] |= CRB_START_INVOKE; + regs[R_CRB_CTRL_START] |= CRB_START_INVOKE; s->cmd = (TPMBackendCmd) { .in = mem, .in_len = MIN(tpm_cmd_get_size(mem), s->be_buffer_size), @@ -108,26 +101,27 @@ static void tpm_crb_mmio_write(void *opaque, hwaddr addr, /* not loc 3 or 4 */ break; case CRB_LOC_CTRL_RELINQUISH: - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, locAssigned, 0); - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, Granted, 0); break; case CRB_LOC_CTRL_REQUEST_ACCESS: - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, Granted, 1); - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STS, + ARRAY_FIELD_DP32(regs, CRB_LOC_STS, beenSeized, 0); - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, locAssigned, 1); break; } break; } + + memory_region_set_dirty(&s->mmio, 0, A_CRB_DATA_BUFFER); } const MemoryRegionOps tpm_crb_memory_ops = { - .read = tpm_crb_mmio_read, .write = tpm_crb_mmio_write, .endianness = DEVICE_LITTLE_ENDIAN, .valid = { @@ -138,12 +132,16 @@ const MemoryRegionOps tpm_crb_memory_ops = { void tpm_crb_request_completed(TPMCRBState *s, int ret) { - s->regs[R_CRB_CTRL_START] &= ~CRB_START_INVOKE; + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); + + assert(regs); + regs[R_CRB_CTRL_START] &= ~CRB_START_INVOKE; if (ret != 0) { - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, tpmSts, 1); /* fatal error */ } - memory_region_set_dirty(&s->cmdmem, 0, CRB_CTRL_CMD_SIZE); + + memory_region_set_dirty(&s->mmio, 0, TPM_CRB_ADDR_SIZE); } enum TPMVersion tpm_crb_get_version(TPMCRBState *s) @@ -160,45 +158,50 @@ int tpm_crb_pre_save(TPMCRBState *s) void tpm_crb_reset(TPMCRBState *s, uint64_t baseaddr) { + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); + + assert(regs); if (s->ppi_enabled) { tpm_ppi_reset(&s->ppi); } tpm_backend_reset(s->tpmbe); - memset(s->regs, 0, sizeof(s->regs)); + memset(regs, 0, TPM_CRB_ADDR_SIZE); - ARRAY_FIELD_DP32(s->regs, CRB_LOC_STATE, + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, tpmRegValidSts, 1); - ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS, + ARRAY_FIELD_DP32(regs, CRB_LOC_STATE, + tpmEstablished, 1); + ARRAY_FIELD_DP32(regs, CRB_CTRL_STS, tpmIdle, 1); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, InterfaceType, CRB_INTF_TYPE_CRB_ACTIVE); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, InterfaceVersion, CRB_INTF_VERSION_CRB); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, CapLocality, CRB_INTF_CAP_LOCALITY_0_ONLY); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, CapCRBIdleBypass, CRB_INTF_CAP_IDLE_FAST); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, CapDataXferSizeSupport, CRB_INTF_CAP_XFER_SIZE_64); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, CapFIFO, CRB_INTF_CAP_FIFO_NOT_SUPPORTED); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, CapCRB, CRB_INTF_CAP_CRB_SUPPORTED); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, InterfaceSelector, CRB_INTF_IF_SELECTOR_CRB); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID, RID, 0b0000); - ARRAY_FIELD_DP32(s->regs, CRB_INTF_ID2, + ARRAY_FIELD_DP32(regs, CRB_INTF_ID2, VID, PCI_VENDOR_ID_IBM); baseaddr += A_CRB_DATA_BUFFER; - s->regs[R_CRB_CTRL_CMD_SIZE] = CRB_CTRL_CMD_SIZE; - s->regs[R_CRB_CTRL_CMD_LADDR] = (uint32_t)baseaddr; - s->regs[R_CRB_CTRL_CMD_HADDR] = (uint32_t)(baseaddr >> 32); - s->regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE; - s->regs[R_CRB_CTRL_RSP_LADDR] = (uint32_t)baseaddr; - s->regs[R_CRB_CTRL_RSP_HADDR] = (uint32_t)(baseaddr >> 32); + regs[R_CRB_CTRL_CMD_SIZE] = CRB_CTRL_CMD_SIZE; + regs[R_CRB_CTRL_CMD_LADDR] = (uint32_t)baseaddr; + regs[R_CRB_CTRL_CMD_HADDR] = (uint32_t)(baseaddr >> 32); + regs[R_CRB_CTRL_RSP_SIZE] = CRB_CTRL_CMD_SIZE; + regs[R_CRB_CTRL_RSP_LADDR] = (uint32_t)baseaddr; + regs[R_CRB_CTRL_RSP_HADDR] = (uint32_t)(baseaddr >> 32); s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->tpmbe), CRB_CTRL_CMD_SIZE); @@ -206,15 +209,33 @@ void tpm_crb_reset(TPMCRBState *s, uint64_t baseaddr) if (tpm_backend_startup_tpm(s->tpmbe, s->be_buffer_size) < 0) { exit(1); } + + memory_region_rom_device_set_romd(&s->mmio, true); + memory_region_set_dirty(&s->mmio, 0, TPM_CRB_ADDR_SIZE); } void tpm_crb_init_memory(Object *obj, TPMCRBState *s, Error **errp) { - memory_region_init_io(&s->mmio, obj, &tpm_crb_memory_ops, s, - "tpm-crb-mmio", sizeof(s->regs)); - memory_region_init_ram(&s->cmdmem, obj, - "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp); + memory_region_init_rom_device_nomigrate(&s->mmio, obj, &tpm_crb_memory_ops, + s, "tpm-crb-mem", TPM_CRB_ADDR_SIZE, errp); if (s->ppi_enabled) { tpm_ppi_init_memory(&s->ppi, obj); } } + +void tpm_crb_mem_save(TPMCRBState *s, uint32_t *saved_regs, void *saved_cmdmem) +{ + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); + + memcpy(saved_regs, regs, TPM_CRB_R_MAX); + memcpy(saved_cmdmem, ®s[R_CRB_DATA_BUFFER], A_CRB_DATA_BUFFER); +} + +void tpm_crb_mem_load(TPMCRBState *s, const uint32_t *saved_regs, + const void *saved_cmdmem) +{ + uint32_t *regs = memory_region_get_ram_ptr(&s->mmio); + + memcpy(regs, saved_regs, TPM_CRB_R_MAX); + memcpy(®s[R_CRB_DATA_BUFFER], saved_cmdmem, A_CRB_DATA_BUFFER); +}
On Apple Silicon, when Windows performs a LDP on the CRB MMIO space, the exception is not decoded by hardware and we cannot trap the MMIO read. This led to the idea from @agraf to use the same mapping type as ROM devices: namely that reads should be seen as memory type and writes should trap as MMIO. Once that was done, the second memory mapping of the command buffer region was redundent and was removed. A note about the removal of the read trap for `CRB_LOC_STATE`: The only usage was to return the most up-to-date value for `tpmEstablished`. However, `tpmEstablished` is only cleared when a TPM2_HashStart operation is called which only exists for locality 4. We do not handle locality 4. Indeed, the comment for the write handler of `CRB_LOC_CTRL` makes the same argument for why it is not calling the backend to reset the `tpmEstablished` bit (to 1). As this bit is unused, we do not need to worry about updating it for reads. In order to maintain migration compatibility with older versions of QEMU, we store a copy of the register data and command data which is used only during save/restore. Signed-off-by: Joelle van Dyne <j@getutm.app> --- hw/tpm/tpm_crb.h | 5 +- hw/tpm/tpm_crb.c | 30 ++++++++- hw/tpm/tpm_crb_common.c | 143 +++++++++++++++++++++++----------------- 3 files changed, 112 insertions(+), 66 deletions(-)