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(+)

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

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);
>   }
>

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
>
>
>

-- 
Marc-André Lureau

> 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