Re: [PATCH] tpm_crb: Avoid backend startup just before shutdown under Xen

[Stefan Berger]

On 8/26/22 10:38, Ross Lagerwall wrote:
> When running under Xen and the guest reboots, it boots into a new domain
> with a new QEMU process (and a new swtpm process if using the emulator
> backend). The existing reset function is triggered just before the old
> QEMU process exists which causes QEMU to startup the TPM backend and
> then immediately shut it down. This is probably harmless but when using
> the emulated backend, it wastes CPU and IO time reloading state, etc.
>
> Fix this by calling the reset function directly from realize() when
> running under Xen. During a reboot, this will be called by the QEMU
> process for the new domain.
>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>
> This conditional logic is ugly. Is there a cleaner way of doing this?
>
>   hw/tpm/tpm_crb.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 67db594c48..ea930da545 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -26,6 +26,7 @@
>   #include "sysemu/tpm_backend.h"
>   #include "sysemu/tpm_util.h"
>   #include "sysemu/reset.h"
> +#include "sysemu/xen.h"
>   #include "tpm_prop.h"
>   #include "tpm_ppi.h"
>   #include "trace.h"
> @@ -308,7 +309,11 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>                        TPM_PPI_ADDR_BASE, OBJECT(s));
>       }
>
> -    qemu_register_reset(tpm_crb_reset, dev);
> +    if (xen_enabled()) {
> +        tpm_crb_reset(dev);
> +    } else {
> +        qemu_register_reset(tpm_crb_reset, dev);
> +    }
>   }
>
>   static void tpm_crb_class_init(ObjectClass *klass, void *data)

This doesn't look right also for Xen. Shouldn't it be something like this?

    qemu_register_reset(tpm_crb_reset, dev);
    if (xen_enabled()) {
       tpm_crb_reset(dev);
    }


We need the reset callback for VM reset.