[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] tpm_crb: mark command buffer as dirty on request completion
From: |
Stefan Berger |
Subject: |
Re: [PATCH] tpm_crb: mark command buffer as dirty on request completion |
Date: |
Mon, 11 Apr 2022 12:31:01 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.7.0 |
On 4/11/22 10:47, Anthony PERARD wrote:
From: Anthony PERARD <anthony.perard@citrix.com>
At the moment, there doesn't seems to be any way to know that QEMU
made modification to the command buffer. This is potentially an issue
on Xen while migrating a guest, as modification to the buffer after
the migration as started could be ignored and not transfered to the
destination. >
Mark the memory region of the command buffer as dirty once a request
is completed.
Not sure about the CRB...
The TIS at least carries the buffer as part of the state and stores it
when the interface is saved:
https://github.com/qemu/qemu/blob/v6.2.0/hw/tpm/tpm_tis_isa.c#L56
https://github.com/qemu/qemu/blob/v6.2.0/hw/tpm/tpm_tis_sysbus.c#L56
With the CRB the memory is registered using these functions.
memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
"tpm-crb-mmio", sizeof(s->regs));
memory_region_init_ram(&s->cmdmem, OBJECT(s),
"tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp);
memory_region_add_subregion(get_system_memory(),
TPM_CRB_ADDR_BASE, &s->mmio);
memory_region_add_subregion(get_system_memory(),
TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
The state of the registers is saved using this here:
static const VMStateDescription vmstate_tpm_crb = {
.name = "tpm-crb",
.pre_save = tpm_crb_pre_save,
.fields = (VMStateField[]) {
VMSTATE_UINT32_ARRAY(regs, CRBState, TPM_CRB_R_MAX),
VMSTATE_END_OF_LIST(),
}
};
The buffer with the commands is not part of this. So likely it needs to
be marked dirty but then I am not sure whether that is going to work if
the response to a command on the CRB is only received when
tpm_crb_pre_save() is called.. Maybe this buffer would have to be save
explicitly in a .save function or also as part of vmstate_tpm_crb... ?
Stefan
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
I have only read code to find out whether the tpm-crb device was fine
with regards to migration, and I don't think there's anything that
could mark the memory region as dirty once a request is completed.
There is one call to memory_region_get_ram_ptr(), but nothing seems to
be done with the pointer is regards to ram migration. Am I wrong?
Thanks.
---
hw/tpm/tpm_crb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index aa9c00aad3..67db594c48 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -197,6 +197,7 @@ static void tpm_crb_request_completed(TPMIf *ti, int ret)
ARRAY_FIELD_DP32(s->regs, CRB_CTRL_STS,
tpmSts, 1); /* fatal error */
}
+ memory_region_set_dirty(&s->cmdmem, 0, CRB_CTRL_CMD_SIZE);
}
static enum TPMVersion tpm_crb_get_version(TPMIf *ti)