| Message ID | 20220801142725.815399-1-ross.lagerwall@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tpm_emulator: Avoid double initialization during migration | expand |
Hi On Mon, Aug 1, 2022 at 6:28 PM Ross Lagerwall via <qemu-devel@nongnu.org> wrote: > When resuming after a migration, the backend sends CMD_INIT to the > emulator from the startup callback, then it sends the migration state > from the vmstate to the emulator, then it sends CMD_INIT again. Skip the > first CMD_INIT during a migration to avoid initializing the TPM twice. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > lgtm Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> There are no visible bugs/symptoms, I suppose? > --- > backends/tpm/tpm_emulator.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c > index 87d061e9bb..9b50c5b3e2 100644 > --- a/backends/tpm/tpm_emulator.c > +++ b/backends/tpm/tpm_emulator.c > @@ -32,6 +32,7 @@ > #include "qemu/sockets.h" > #include "qemu/lockable.h" > #include "io/channel-socket.h" > +#include "sysemu/runstate.h" > #include "sysemu/tpm_backend.h" > #include "sysemu/tpm_util.h" > #include "tpm_int.h" > @@ -383,6 +384,15 @@ err_exit: > > static int tpm_emulator_startup_tpm(TPMBackend *tb, size_t buffersize) > { > + /* TPM startup will be done from post_load hook */ > + if (runstate_check(RUN_STATE_INMIGRATE)) { > + if (buffersize != 0) { > + return tpm_emulator_set_buffer_size(tb, buffersize, NULL); > + } > + > + return 0; > + } > + > return tpm_emulator_startup_tpm_resume(tb, buffersize, false); > } > > -- > 2.31.1 > > >
> From: Marc-André Lureau <marcandre.lureau@gmail.com> > Sent: Monday, August 1, 2022 3:36 PM > To: Ross Lagerwall <ross.lagerwall@citrix.com> > Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>; qemu-devel@nongnu.org <qemu-devel@nongnu.org> > Subject: Re: [PATCH] tpm_emulator: Avoid double initialization during migration > > [CAUTION - EXTERNAL EMAIL] DO NOT reply, click links, or open attachments unless you have verified the sender and know the content is safe. > Hi > > On Mon, Aug 1, 2022 at 6:28 PM Ross Lagerwall via <qemu-devel@nongnu.org> wrote: > When resuming after a migration, the backend sends CMD_INIT to the > emulator from the startup callback, then it sends the migration state > from the vmstate to the emulator, then it sends CMD_INIT again. Skip the > first CMD_INIT during a migration to avoid initializing the TPM twice. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> > > lgtm > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > There are no visible bugs/symptoms, I suppose? I started looking into this because swtpm would complain about "tpm2-00.volatilestate" being missing during migration. This happened during the first init because the volatile state only got set by QEMU before the 2nd init. I'm not sure if there are any other negative consequences to sending init twice (I suspect probably not?). Ross
On 8/1/22 10:27, Ross Lagerwall via wrote: > When resuming after a migration, the backend sends CMD_INIT to the > emulator from the startup callback, then it sends the migration state This startup hook is called upon TIS/CRB device reset, so this is likely called before the device state has been received. > from the vmstate to the emulator, then it sends CMD_INIT again. Skip the > first CMD_INIT during a migration to avoid initializing the TPM twice. Ok, that's sufficient to start it up once all the state has been received. > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> Tested-by: Stefan Berger <stefanb@linux.ibm.com> > --- > backends/tpm/tpm_emulator.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c > index 87d061e9bb..9b50c5b3e2 100644 > --- a/backends/tpm/tpm_emulator.c > +++ b/backends/tpm/tpm_emulator.c > @@ -32,6 +32,7 @@ > #include "qemu/sockets.h" > #include "qemu/lockable.h" > #include "io/channel-socket.h" > +#include "sysemu/runstate.h" > #include "sysemu/tpm_backend.h" > #include "sysemu/tpm_util.h" > #include "tpm_int.h" > @@ -383,6 +384,15 @@ err_exit: > > static int tpm_emulator_startup_tpm(TPMBackend *tb, size_t buffersize) > { > + /* TPM startup will be done from post_load hook */ > + if (runstate_check(RUN_STATE_INMIGRATE)) { > + if (buffersize != 0) { > + return tpm_emulator_set_buffer_size(tb, buffersize, NULL); > + } > + > + return 0; > + } > + > return tpm_emulator_startup_tpm_resume(tb, buffersize, false); > } >
diff --git a/backends/tpm/tpm_emulator.c b/backends/tpm/tpm_emulator.c index 87d061e9bb..9b50c5b3e2 100644 --- a/backends/tpm/tpm_emulator.c +++ b/backends/tpm/tpm_emulator.c @@ -32,6 +32,7 @@ #include "qemu/sockets.h" #include "qemu/lockable.h" #include "io/channel-socket.h" +#include "sysemu/runstate.h" #include "sysemu/tpm_backend.h" #include "sysemu/tpm_util.h" #include "tpm_int.h" @@ -383,6 +384,15 @@ err_exit: static int tpm_emulator_startup_tpm(TPMBackend *tb, size_t buffersize) { + /* TPM startup will be done from post_load hook */ + if (runstate_check(RUN_STATE_INMIGRATE)) { + if (buffersize != 0) { + return tpm_emulator_set_buffer_size(tb, buffersize, NULL); + } + + return 0; + } + return tpm_emulator_startup_tpm_resume(tb, buffersize, false); }
When resuming after a migration, the backend sends CMD_INIT to the emulator from the startup callback, then it sends the migration state from the vmstate to the emulator, then it sends CMD_INIT again. Skip the first CMD_INIT during a migration to avoid initializing the TPM twice. Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com> --- backends/tpm/tpm_emulator.c | 10 ++++++++++ 1 file changed, 10 insertions(+)