| Message ID | 20191212180744.1070446-1-stefanb@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Add vTPM emulator supportfor ppc64 platform | expand |
On 12/12/19 1:07 PM, Stefan Berger wrote: > Implement the check whether the emulator backend is suspended. > > Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> > > diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c > index 22f9113432..7be7d3a91b 100644 > --- a/hw/tpm/tpm_emulator.c > +++ b/hw/tpm/tpm_emulator.c > @@ -80,6 +80,8 @@ typedef struct TPMEmulator { > unsigned int established_flag_cached:1; > > TPMBlobBuffers state_blobs; > + > + bool is_suspended; > } TPMEmulator; > > struct tpm_error { > @@ -486,6 +488,13 @@ static size_t tpm_emulator_get_buffer_size(TPMBackend *tb) > return actual_size; > } > > +static bool tpm_emulator_is_suspended(TPMBackend *tb) > +{ > + TPMEmulator *tpm_emu = TPM_EMULATOR(tb); > + > + return tpm_emu->is_suspended; > +} > + > static int tpm_emulator_block_migration(TPMEmulator *tpm_emu) > { > Error *err = NULL; > @@ -846,6 +855,8 @@ static int tpm_emulator_pre_save(void *opaque) > TPMBackend *tb = opaque; > TPMEmulator *tpm_emu = TPM_EMULATOR(tb); > > + tpm_emu->is_suspended = true; This is the most critical part here. It must be true when we receive a response in the tpm_spapr_request_completed(). The problem is that what tpm_backend_finish_sync() does is not specific to this backend but more a global operation that another device could run as well -- none seem to do this today. So the point is that there could be a race here. This flag should really be set in '.pre_pre_save,' so before any other device could poll. Better would be calling a global function that indicates whether device suspension has started. In this case we could do away with this and just call that function from the spapr device.
On 12/12/19 1:33 PM, Stefan Berger wrote: > On 12/12/19 1:07 PM, Stefan Berger wrote: >> Implement the check whether the emulator backend is suspended. >> >> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> >> >> diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c >> index 22f9113432..7be7d3a91b 100644 >> --- a/hw/tpm/tpm_emulator.c >> +++ b/hw/tpm/tpm_emulator.c >> @@ -80,6 +80,8 @@ typedef struct TPMEmulator { >> unsigned int established_flag_cached:1; >> TPMBlobBuffers state_blobs; >> + >> + bool is_suspended; >> } TPMEmulator; >> struct tpm_error { >> @@ -486,6 +488,13 @@ static size_t >> tpm_emulator_get_buffer_size(TPMBackend *tb) >> return actual_size; >> } >> +static bool tpm_emulator_is_suspended(TPMBackend *tb) >> +{ >> + TPMEmulator *tpm_emu = TPM_EMULATOR(tb); >> + >> + return tpm_emu->is_suspended; >> +} >> + >> static int tpm_emulator_block_migration(TPMEmulator *tpm_emu) >> { >> Error *err = NULL; >> @@ -846,6 +855,8 @@ static int tpm_emulator_pre_save(void *opaque) >> TPMBackend *tb = opaque; >> TPMEmulator *tpm_emu = TPM_EMULATOR(tb); >> + tpm_emu->is_suspended = true; > > This is the most critical part here. It must be true when we receive a > response in the tpm_spapr_request_completed(). The problem is that > what tpm_backend_finish_sync() does is not specific to this backend > but more a global operation that another device could run as well -- > none seem to do this today. So the point is that there could be a race > here. This flag should really be set in '.pre_pre_save,' so before any > other device could poll. Better would be calling a global function > that indicates whether device suspension has started. In this case we > could do away with this and just call that function from the spapr > device. runstate_check(RUN_STATE_FINISH_MIGRATE) seems to be what we need here... > > >
The following series of patches adds vTPM emulator support for the ppc64 platform (pSeries). It can be tested as follows with swtpm/libtpms: mkdir /tmp/mytpm1 swtpm socket --tpmstate dir=/tmp/mytpm1 \ --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \ --log level=20 If TPM 2 is desired, add --tpm2 as parameter to the above. In another terminal start QEMU: sudo ./ppc64-softmmu/qemu-system-ppc64 -display sdl \ -machine pseries,accel=kvm \ -m 1024 -bios slof.bin -boot menu=on \ -nodefaults -device VGA -device pci-ohci -device usb-kbd \ -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \ -tpmdev emulator,id=tpm0,chardev=chrtpm \ -device tpm-spapr,tpmdev=tpm0 \ -device spapr-vscsi,id=scsi0,reg=0x00002000 \ -device virtio-blk-pci,scsi=off,bus=pci.0,addr=0x3,drive=drive-virtio-disk0,id=virtio-disk0 \ -drive file=test.img,format=raw,if=none,id=drive-virtio-disk0 Links: - libtpms: https://github.com/stefanberger/libtpms/wiki - swtpm: https://github.com/stefanberger/swtpm/wiki Changes: v3->v4: - addressed comments to v3 - reworked suspend/resume support that requires extensions to backends v2->v3: - patch 1: a TPM 2 is identified by IBM,vtpm20 in the compatible node - patch 1: convert to tracing to display Tx and Rx buffers - added documentation patch - added patch to enable TPM device as part of pSeries v1->v2: - followed Cedric Le Goater's suggestions to patch 1 - send appropriate CRQ error responses if DMA read or write fails - renamed tpm_spapr_got_payload to tpm_spapr_process_cmd and pass endianess-adjusted data pointer from CRQ to it Regards, Stefan Stefan Berger (8): tpm_spapr: Support TPM for ppc64 using CRQ based interface tpm_backend: Implement check whether tpm backend is suspended tpm_emulator: Implement callback for whether we are suspended tpm_passthrough: Implement callback for whether we are suspended tpm: Return bool from tpm_backend_finish_sync tpm_spapr: Support suspend and resume hw/ppc/Kconfig: Enable TPM_SPAPR as part of PSERIES config docs: tpm: Add example command line for ppc64 and tpm-spapr backends/tpm.c | 13 +- docs/specs/tpm.txt | 20 +- hw/ppc/Kconfig | 1 + hw/tpm/Kconfig | 6 + hw/tpm/Makefile.objs | 1 + hw/tpm/tpm_emulator.c | 12 + hw/tpm/tpm_passthrough.c | 6 + hw/tpm/tpm_spapr.c | 460 +++++++++++++++++++++++++++++++++++ hw/tpm/trace-events | 14 ++ include/sysemu/tpm.h | 3 + include/sysemu/tpm_backend.h | 16 +- qapi/tpm.json | 6 +- 12 files changed, 552 insertions(+), 6 deletions(-) create mode 100644 hw/tpm/tpm_spapr.c