qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH v2 04/11] tpm_crb: use a single read-as-mem/write-as-mmio map


From: Stefan Berger
Subject: Re: [PATCH v2 04/11] tpm_crb: use a single read-as-mem/write-as-mmio mapping
Date: Fri, 14 Jul 2023 08:03:05 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0



On 7/14/23 03:09, 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.

Signed-off-by: Joelle van Dyne <j@getutm.app>
---
  hw/tpm/tpm_crb.h        |   2 -
  hw/tpm/tpm_crb.c        |   3 -
  hw/tpm/tpm_crb_common.c | 126 +++++++++++++++++++++-------------------
  3 files changed, 65 insertions(+), 66 deletions(-)

diff --git a/hw/tpm/tpm_crb.h b/hw/tpm/tpm_crb.h
index da3a0cf256..7cdd37335f 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;

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 598c3e0161..07c6868d8d 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -68,7 +68,6 @@ static const VMStateDescription vmstate_tpm_crb_none = {
      .name = "tpm-crb",
      .pre_save = tpm_crb_none_pre_save,
      .fields = (VMStateField[]) {
-        VMSTATE_UINT32_ARRAY(state.regs, CRBState, TPM_CRB_R_MAX),

The same comment as stated on v1 still applies, this part has to stay since VM 
state contains it:

2023-07-14T12:01:38.005199Z qemu-system-x86_64: Unknown ramblock "tpm-crb-cmd", 
cannot accept migration
2023-07-14T12:01:38.005318Z qemu-system-x86_64: error while loading state for 
instance 0x0 of device 'ram'
2023-07-14T12:01:38.005350Z qemu-system-x86_64: load of migration failed: 
Invalid argument

   Stefan



reply via email to

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