Re: [PATCH 2/7] runstate: skip initial CPU reset if reset is not actually possible

From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org, xiaoyao.li@intel.com, michael.roth@amd.com
Subject: Re: [PATCH 2/7] runstate: skip initial CPU reset if reset is not actually possible
Date: Tue, 19 Mar 2024 14:16:07 +0000	[thread overview]
Message-ID: <ZfmeJ9c_TMSb12Z1@redhat.com> (raw)
In-Reply-To: <20240319140000.1014247-3-pbonzini@redhat.com>

On Tue, Mar 19, 2024 at 02:59:55PM +0100, Paolo Bonzini wrote:
> Right now, the system reset is concluded by a call to
> cpu_synchronize_all_post_reset() in order to sync any changes
> that the machine reset callback applied to the CPU state.
> 
> However, for VMs with encrypted state such as SEV-ES guests (currently
> the only case of guests with non-resettable CPUs) this cannot be done,
> because guest state has already been finalized by machine-init-done notifiers.
> cpu_synchronize_all_post_reset() does nothing on these guests, and actually
> we would like to make it fail if called once guest has been encrypted.
> So, assume that boards that support non-resettable CPUs do not touch
> CPU state and that all such setup is done before, at the time of
> cpu_synchronize_all_post_init().
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  system/runstate.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/system/runstate.c b/system/runstate.c
> index d6ab860ecaa..843e2b9853d 100644
> --- a/system/runstate.c
> +++ b/system/runstate.c
> @@ -501,7 +501,20 @@ void qemu_system_reset(ShutdownCause reason)
>      default:
>          qapi_event_send_reset(shutdown_caused_by_guest(reason), reason);
>      }
> -    cpu_synchronize_all_post_reset();
> +
> +    /*
> +     * Some boards us the machine reset callback to point CPUs to the firmware

ITYM s/us/use/

> +     * entry point.  Assume that this is not the case for boards that support
> +     * non-resettable CPUs (currently used only for confidential guests), in
> +     * which case cpu_synchronize_all_post_init() is enough because
> +     * it does _more_  than cpu_synchronize_all_post_reset().
> +     */
> +    if (cpus_are_resettable()) {
> +        cpu_synchronize_all_post_reset();
> +    } else {
> +        assert(runstate_check(RUN_STATE_PRELAUNCH));
> +    }
> +
>      vm_set_suspended(false);
>  }
>  
> -- 
> 2.44.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|