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

[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 <address@hidden>
>
> 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.