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

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


runstate_check(RUN_STATE_FINISH_MIGRATE) seems to be what we need here...



>