| Message ID | 20191219140605.3243321-5-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add vTPM emulator support for ppc64 platform | expand |
On Thu, Dec 19, 2019 at 09:06:03AM -0500, Stefan Berger wrote: > Extend the tpm_spapr frontend with VM suspend and resume support. > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > --- > hw/tpm/tpm_spapr.c | 67 ++++++++++++++++++++++++++++++++++++++++++++- > hw/tpm/trace-events | 2 ++ > 2 files changed, 68 insertions(+), 1 deletion(-) > > diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c > index ab184fbb82..cf5c7851e7 100644 > --- a/hw/tpm/tpm_spapr.c > +++ b/hw/tpm/tpm_spapr.c > @@ -76,6 +76,9 @@ typedef struct { > > unsigned char buffer[TPM_SPAPR_BUFFER_MAX]; > > + uint32_t numbytes; /* number of bytes in suspend_buffer */ > + unsigned char *suspend_buffer; > + > TPMBackendCmd cmd; > > TPMBackend *be_driver; > @@ -240,6 +243,13 @@ static void tpm_spapr_request_completed(TPMIf *ti, int ret) > > /* a max. of be_buffer_size bytes can be transported */ > len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size); > + > + if (runstate_check(RUN_STATE_FINISH_MIGRATE)) { > + /* defer delivery of response until .post_load */ > + s->numbytes = len; > + return; > + } I'm not understanding the basics of what's going on here. IIUC, the backend TPM can complete a request after we've entered migration downtime. But if that's the case, I can't see any guarantee that we won't have already transmitted the TPM device state, so updating it here might never reach the destination. In that case we'd still lose the request, so I'm not sure what we're accomplishing here. > rc = spapr_vio_dma_write(&s->vdev, be32_to_cpu(crq->data), > s->buffer, len); > > @@ -288,11 +298,13 @@ static void tpm_spapr_reset(SpaprVioDevice *dev) > SpaprTpmState *s = VIO_SPAPR_VTPM(dev); > > s->state = SPAPR_VTPM_STATE_NONE; > + s->numbytes = 0; > > s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver); > > s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver), > TPM_SPAPR_BUFFER_MAX); > + s->suspend_buffer = g_realloc(s->suspend_buffer, s->be_buffer_size); > > tpm_backend_reset(s->be_driver); > tpm_spapr_do_startup_tpm(s, s->be_buffer_size); > @@ -309,9 +321,62 @@ static enum TPMVersion tpm_spapr_get_version(TPMIf *ti) > return tpm_backend_get_tpm_version(s->be_driver); > } > > +/* persistent state handling */ > + > +static int tpm_spapr_pre_save(void *opaque) > +{ > + SpaprTpmState *s = opaque; > + > + tpm_backend_finish_sync(s->be_driver); > + > + if (s->numbytes) { > + memcpy(s->suspend_buffer, s->buffer, s->numbytes); > + } > + > + trace_tpm_spapr_pre_save(s->numbytes); > + /* > + * we cannot deliver the results to the VM since DMA would touch VM memory > + */ > + > + return 0; > +} > + > +static int tpm_spapr_post_load(void *opaque, int version_id) > +{ > + SpaprTpmState *s = opaque; > + > + if (s->numbytes) { > + trace_tpm_spapr_post_load(); > + > + memcpy(s->buffer, s->suspend_buffer, > + MIN(s->numbytes, s->be_buffer_size)); > + /* deliver the results to the VM via DMA */ > + tpm_spapr_request_completed(TPM_IF(s), 0); > + s->numbytes = 0; > + } > + > + return 0; > +} > + > static const VMStateDescription vmstate_spapr_vtpm = { > .name = "tpm-spapr", > - .unmigratable = 1, > + .version_id = 1, > + .minimum_version_id = 0, > + .minimum_version_id_old = 0, > + .pre_save = tpm_spapr_pre_save, > + .post_load = tpm_spapr_post_load, > + .fields = (VMStateField[]) { > + VMSTATE_SPAPR_VIO(vdev, SpaprTpmState), > + > + VMSTATE_UINT8(state, SpaprTpmState), > + VMSTATE_UINT32(numbytes, SpaprTpmState), > + VMSTATE_VBUFFER_ALLOC_UINT32(suspend_buffer, > + SpaprTpmState, 0, NULL, > + numbytes), > + /* remember DMA address */ > + VMSTATE_UINT32(crq.data, SpaprTpmState), > + VMSTATE_END_OF_LIST(), > + } > }; > > static Property tpm_spapr_properties[] = { > diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events > index 9143a8eaa3..5592cec7de 100644 > --- a/hw/tpm/trace-events > +++ b/hw/tpm/trace-events > @@ -67,3 +67,5 @@ tpm_spapr_do_crq_get_version(uint32_t version) "response: version %u" > tpm_spapr_do_crq_prepare_to_suspend(void) "response: preparing to suspend" > tpm_spapr_do_crq_unknown_msg_type(uint8_t type) "Unknown message type 0x%02x" > tpm_spapr_do_crq_unknown_crq(uint8_t raw1, uint8_t raw2) "unknown CRQ 0x%02x 0x%02x ..." > +tpm_spapr_pre_save(uint32_t v) "Number of TPM response bytes to deliver after resume: %u" > +tpm_spapr_post_load(void) "Delivering TPM response after resume"
On 1/2/20 7:19 PM, David Gibson wrote: > On Thu, Dec 19, 2019 at 09:06:03AM -0500, Stefan Berger wrote: >> Extend the tpm_spapr frontend with VM suspend and resume support. >> >> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >> --- >> hw/tpm/tpm_spapr.c | 67 ++++++++++++++++++++++++++++++++++++++++++++- >> hw/tpm/trace-events | 2 ++ >> 2 files changed, 68 insertions(+), 1 deletion(-) >> >> diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c >> index ab184fbb82..cf5c7851e7 100644 >> --- a/hw/tpm/tpm_spapr.c >> +++ b/hw/tpm/tpm_spapr.c >> @@ -76,6 +76,9 @@ typedef struct { >> >> unsigned char buffer[TPM_SPAPR_BUFFER_MAX]; >> >> + uint32_t numbytes; /* number of bytes in suspend_buffer */ >> + unsigned char *suspend_buffer; >> + >> TPMBackendCmd cmd; >> >> TPMBackend *be_driver; >> @@ -240,6 +243,13 @@ static void tpm_spapr_request_completed(TPMIf *ti, int ret) >> >> /* a max. of be_buffer_size bytes can be transported */ >> len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size); >> + >> + if (runstate_check(RUN_STATE_FINISH_MIGRATE)) { >> + /* defer delivery of response until .post_load */ >> + s->numbytes = len; >> + return; >> + } > I'm not understanding the basics of what's going on here. IIUC, the > backend TPM can complete a request after we've entered migration The tpm_backend_finish_sync() in the .pre_save function will delay the writing of the device state until all outstanding responses have been captured. In case a response was outstanding, the above s->numbytes will then be set and looked at after return from tpm_backend_finish_sync() -- see below. > downtime. But if that's the case, I can't see any guarantee that we > won't have already transmitted the TPM device state, so updating it > here might never reach the destination. In that case we'd still lose > the request, so I'm not sure what we're accomplishing here. > >> rc = spapr_vio_dma_write(&s->vdev, be32_to_cpu(crq->data), >> s->buffer, len); >> >> @@ -288,11 +298,13 @@ static void tpm_spapr_reset(SpaprVioDevice *dev) >> SpaprTpmState *s = VIO_SPAPR_VTPM(dev); >> >> s->state = SPAPR_VTPM_STATE_NONE; >> + s->numbytes = 0; >> >> s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver); >> >> s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver), >> TPM_SPAPR_BUFFER_MAX); >> + s->suspend_buffer = g_realloc(s->suspend_buffer, s->be_buffer_size); >> >> tpm_backend_reset(s->be_driver); >> tpm_spapr_do_startup_tpm(s, s->be_buffer_size); >> @@ -309,9 +321,62 @@ static enum TPMVersion tpm_spapr_get_version(TPMIf *ti) >> return tpm_backend_get_tpm_version(s->be_driver); >> } >> >> +/* persistent state handling */ >> + >> +static int tpm_spapr_pre_save(void *opaque) >> +{ >> + SpaprTpmState *s = opaque; >> + >> + tpm_backend_finish_sync(s->be_driver); >> + >> + if (s->numbytes) { >> + memcpy(s->suspend_buffer, s->buffer, s->numbytes); >> + } >> + >> + trace_tpm_spapr_pre_save(s->numbytes); >> + /* >> + * we cannot deliver the results to the VM since DMA would touch VM memory >> + */ >> + >> + return 0; >> +} >> + >> +static int tpm_spapr_post_load(void *opaque, int version_id) >> +{ >> + SpaprTpmState *s = opaque; >> + >> + if (s->numbytes) { >> + trace_tpm_spapr_post_load(); >> + >> + memcpy(s->buffer, s->suspend_buffer, >> + MIN(s->numbytes, s->be_buffer_size)); >> + /* deliver the results to the VM via DMA */ >> + tpm_spapr_request_completed(TPM_IF(s), 0); >> + s->numbytes = 0; >> + } >> + >> + return 0; >> +} >> + >> static const VMStateDescription vmstate_spapr_vtpm = { >> .name = "tpm-spapr", >> - .unmigratable = 1, >> + .version_id = 1, >> + .minimum_version_id = 0, >> + .minimum_version_id_old = 0, >> + .pre_save = tpm_spapr_pre_save, >> + .post_load = tpm_spapr_post_load, >> + .fields = (VMStateField[]) { >> + VMSTATE_SPAPR_VIO(vdev, SpaprTpmState), >> + >> + VMSTATE_UINT8(state, SpaprTpmState), >> + VMSTATE_UINT32(numbytes, SpaprTpmState), >> + VMSTATE_VBUFFER_ALLOC_UINT32(suspend_buffer, >> + SpaprTpmState, 0, NULL, >> + numbytes), >> + /* remember DMA address */ >> + VMSTATE_UINT32(crq.data, SpaprTpmState), >> + VMSTATE_END_OF_LIST(), >> + } >> }; >> >> static Property tpm_spapr_properties[] = { >> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events >> index 9143a8eaa3..5592cec7de 100644 >> --- a/hw/tpm/trace-events >> +++ b/hw/tpm/trace-events >> @@ -67,3 +67,5 @@ tpm_spapr_do_crq_get_version(uint32_t version) "response: version %u" >> tpm_spapr_do_crq_prepare_to_suspend(void) "response: preparing to suspend" >> tpm_spapr_do_crq_unknown_msg_type(uint8_t type) "Unknown message type 0x%02x" >> tpm_spapr_do_crq_unknown_crq(uint8_t raw1, uint8_t raw2) "unknown CRQ 0x%02x 0x%02x ..." >> +tpm_spapr_pre_save(uint32_t v) "Number of TPM response bytes to deliver after resume: %u" >> +tpm_spapr_post_load(void) "Delivering TPM response after resume"
diff --git a/hw/tpm/tpm_spapr.c b/hw/tpm/tpm_spapr.c index ab184fbb82..cf5c7851e7 100644 --- a/hw/tpm/tpm_spapr.c +++ b/hw/tpm/tpm_spapr.c @@ -76,6 +76,9 @@ typedef struct { unsigned char buffer[TPM_SPAPR_BUFFER_MAX]; + uint32_t numbytes; /* number of bytes in suspend_buffer */ + unsigned char *suspend_buffer; + TPMBackendCmd cmd; TPMBackend *be_driver; @@ -240,6 +243,13 @@ static void tpm_spapr_request_completed(TPMIf *ti, int ret) /* a max. of be_buffer_size bytes can be transported */ len = MIN(tpm_cmd_get_size(s->buffer), s->be_buffer_size); + + if (runstate_check(RUN_STATE_FINISH_MIGRATE)) { + /* defer delivery of response until .post_load */ + s->numbytes = len; + return; + } + rc = spapr_vio_dma_write(&s->vdev, be32_to_cpu(crq->data), s->buffer, len); @@ -288,11 +298,13 @@ static void tpm_spapr_reset(SpaprVioDevice *dev) SpaprTpmState *s = VIO_SPAPR_VTPM(dev); s->state = SPAPR_VTPM_STATE_NONE; + s->numbytes = 0; s->be_tpm_version = tpm_backend_get_tpm_version(s->be_driver); s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver), TPM_SPAPR_BUFFER_MAX); + s->suspend_buffer = g_realloc(s->suspend_buffer, s->be_buffer_size); tpm_backend_reset(s->be_driver); tpm_spapr_do_startup_tpm(s, s->be_buffer_size); @@ -309,9 +321,62 @@ static enum TPMVersion tpm_spapr_get_version(TPMIf *ti) return tpm_backend_get_tpm_version(s->be_driver); } +/* persistent state handling */ + +static int tpm_spapr_pre_save(void *opaque) +{ + SpaprTpmState *s = opaque; + + tpm_backend_finish_sync(s->be_driver); + + if (s->numbytes) { + memcpy(s->suspend_buffer, s->buffer, s->numbytes); + } + + trace_tpm_spapr_pre_save(s->numbytes); + /* + * we cannot deliver the results to the VM since DMA would touch VM memory + */ + + return 0; +} + +static int tpm_spapr_post_load(void *opaque, int version_id) +{ + SpaprTpmState *s = opaque; + + if (s->numbytes) { + trace_tpm_spapr_post_load(); + + memcpy(s->buffer, s->suspend_buffer, + MIN(s->numbytes, s->be_buffer_size)); + /* deliver the results to the VM via DMA */ + tpm_spapr_request_completed(TPM_IF(s), 0); + s->numbytes = 0; + } + + return 0; +} + static const VMStateDescription vmstate_spapr_vtpm = { .name = "tpm-spapr", - .unmigratable = 1, + .version_id = 1, + .minimum_version_id = 0, + .minimum_version_id_old = 0, + .pre_save = tpm_spapr_pre_save, + .post_load = tpm_spapr_post_load, + .fields = (VMStateField[]) { + VMSTATE_SPAPR_VIO(vdev, SpaprTpmState), + + VMSTATE_UINT8(state, SpaprTpmState), + VMSTATE_UINT32(numbytes, SpaprTpmState), + VMSTATE_VBUFFER_ALLOC_UINT32(suspend_buffer, + SpaprTpmState, 0, NULL, + numbytes), + /* remember DMA address */ + VMSTATE_UINT32(crq.data, SpaprTpmState), + VMSTATE_END_OF_LIST(), + } }; static Property tpm_spapr_properties[] = { diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events index 9143a8eaa3..5592cec7de 100644 --- a/hw/tpm/trace-events +++ b/hw/tpm/trace-events @@ -67,3 +67,5 @@ tpm_spapr_do_crq_get_version(uint32_t version) "response: version %u" tpm_spapr_do_crq_prepare_to_suspend(void) "response: preparing to suspend" tpm_spapr_do_crq_unknown_msg_type(uint8_t type) "Unknown message type 0x%02x" tpm_spapr_do_crq_unknown_crq(uint8_t raw1, uint8_t raw2) "unknown CRQ 0x%02x 0x%02x ..." +tpm_spapr_pre_save(uint32_t v) "Number of TPM response bytes to deliver after resume: %u" +tpm_spapr_post_load(void) "Delivering TPM response after resume"
Extend the tpm_spapr frontend with VM suspend and resume support. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- hw/tpm/tpm_spapr.c | 67 ++++++++++++++++++++++++++++++++++++++++++++- hw/tpm/trace-events | 2 ++ 2 files changed, 68 insertions(+), 1 deletion(-)