Re: [PATCH v4 3/8] tpm_emulator: Implement callback for whether we are suspended

2019-12-12 Thread Stefan Berger 

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 

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

[PATCH v4 3/8] tpm_emulator: Implement callback for whether we are suspended

2019-12-12 Thread Stefan Berger 
Implement the check whether the emulator backend is suspended.

Signed-off-by: Stefan Berger 

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;
+
 trace_tpm_emulator_pre_save();
 
 tpm_backend_finish_sync(tb);
@@ -975,6 +986,7 @@ static void tpm_emulator_class_init(ObjectClass *klass, 
void *data)
 tbc->get_tpm_version = tpm_emulator_get_tpm_version;
 tbc->get_buffer_size = tpm_emulator_get_buffer_size;
 tbc->get_tpm_options = tpm_emulator_get_tpm_options;
+tbc->is_suspended = tpm_emulator_is_suspended;
 
 tbc->handle_request = tpm_emulator_handle_request;
 }
-- 
2.21.0

Re: [PATCH v4 3/8] tpm_emulator: Implement callback for whether we are suspended

2019-12-12 Thread Stefan Berger 

On 12/12/19 1:07 PM, Stefan Berger wrote:

Implement the check whether the emulator backend is suspended.

Signed-off-by: Stefan Berger 

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.