Re: [Qemu-devel] [RFC PATCH v1 21/22] hw: add pre and post system reset callback

[Paolo Bonzini]

On 13/09/2016 16:50, Brijesh Singh wrote:
> This patch adds methods to register a callback in qemu_system_reset().
> 
> - qemu_register_pre_reset() : function will be called just after
>   entering into qemu_system_reset().
> - qemu_register_post_reset(): function will be called just before
>   exiting from the qemu_system_reset().
> 
> A qemu_system_reset() causes loader to reload the OS images into guest
> memory. In case of SEV-enabled guest we need to call the SEV launch start
> command before loader copies any data into guest RAM and similarly SEV
> launch finish command should be executed after we finished copying the
> data into guest memory.
> 
> These callback will allow us to hook the SEV launch START and FINISH
> commands into qemu_system_reset() handlder to start and finalize the SEV
> guest launch process.
> 
> Signed-off-by: Brijesh Singh <address@hidden>

I would just put the SEV hook in qemu_system_reset().

You can use a new file sev-stub.c with an empty implementation of
kvm_sev_guest_start and kvm_sev_guest_finish.

Paolo

> ---
>  include/hw/hw.h |    2 ++
>  sev.c           |   14 ++++++++++++++
>  vl.c            |   45 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 61 insertions(+)
> 
> diff --git a/include/hw/hw.h b/include/hw/hw.h
> index 3669ebd..31dcf9f 100644
> --- a/include/hw/hw.h
> +++ b/include/hw/hw.h
> @@ -17,6 +17,8 @@ typedef void QEMUResetHandler(void *opaque);
>  
>  void qemu_register_reset(QEMUResetHandler *func, void *opaque);
>  void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_register_pre_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_register_post_reset(QEMUResetHandler *func, void *opaque);
>  
>  void QEMU_NORETURN hw_error(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
>  
> diff --git a/sev.c b/sev.c
> index c1135c4..141f9d6 100644
> --- a/sev.c
> +++ b/sev.c
> @@ -35,6 +35,7 @@
>  #include "qemu/event_notifier.h"
>  #include "trace.h"
>  #include "hw/irq.h"
> +#include "hw/hw.h"
>  
>  //#define DEBUG_SEV
>  
> @@ -257,6 +258,16 @@ static int parse_sev_cfg(SEVInfo *s, int type, const 
> char *filename)
>  
>  }
>  
> +static void sev_pre_reset_handler(void *data)
> +{
> +    kvm_sev_guest_start();
> +}
> +
> +static void sev_post_reset_handler(void *data)
> +{
> +    kvm_sev_guest_finish();
> +}
> +
>  int sev_init(KVMState *kvm_state)
>  {
>      QemuOpts *opts;
> @@ -287,6 +298,9 @@ int sev_init(KVMState *kvm_state)
>          goto err;
>      }
>  
> +    qemu_register_pre_reset(sev_pre_reset_handler, sev_info);
> +    qemu_register_post_reset(sev_post_reset_handler, sev_info);
> +
>      return kvm_sev_guest_start();
>  err:
>      free(sev_info);
> diff --git a/vl.c b/vl.c
> index 22b8eba..5923c73 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -25,6 +25,7 @@
>  #include "qemu-version.h"
>  #include "qemu/cutils.h"
>  #include "qemu/help_option.h"
> +#include "sysemu/sev.h"
>  
>  #ifdef CONFIG_SECCOMP
>  #include "sysemu/seccomp.h"
> @@ -1630,6 +1631,10 @@ static NotifierList suspend_notifiers =
>  static NotifierList wakeup_notifiers =
>      NOTIFIER_LIST_INITIALIZER(wakeup_notifiers);
>  static uint32_t wakeup_reason_mask = ~(1 << QEMU_WAKEUP_REASON_NONE);
> +static QTAILQ_HEAD(pre_reset_handlers, QEMUResetEntry) pre_reset_handlers =
> +    QTAILQ_HEAD_INITIALIZER(pre_reset_handlers);
> +static QTAILQ_HEAD(post_reset_handlers, QEMUResetEntry)
> +    post_reset_handlers = QTAILQ_HEAD_INITIALIZER(post_reset_handlers);
>  
>  int qemu_shutdown_requested_get(void)
>  {
> @@ -1733,12 +1738,51 @@ void qemu_devices_reset(void)
>      }
>  }
>  
> +void qemu_register_pre_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> +
> +    re->func = func;
> +    re->opaque = opaque;
> +    QTAILQ_INSERT_TAIL(&pre_reset_handlers, re, entry);
> +}
> +
> +static void qemu_system_pre_reset(void)
> +{
> +    QEMUResetEntry *re, *nre;
> +
> +    /* call pre_reset handler */
> +    QTAILQ_FOREACH_SAFE(re, &pre_reset_handlers, entry, nre) {
> +        re->func(re->opaque);
> +    }
> +}
> +
> +void qemu_register_post_reset(QEMUResetHandler *func, void *opaque)
> +{
> +    QEMUResetEntry *re = g_malloc0(sizeof(QEMUResetEntry));
> +
> +    re->func = func;
> +    re->opaque = opaque;
> +    QTAILQ_INSERT_TAIL(&post_reset_handlers, re, entry);
> +}
> +
> +static void qemu_system_post_reset(void)
> +{
> +    QEMUResetEntry *re, *nre;
> +
> +    /* call post reset handler */
> +    QTAILQ_FOREACH_SAFE(re, &post_reset_handlers, entry, nre) {
> +        re->func(re->opaque);
> +    }
> +}
> +
>  void qemu_system_reset(bool report)
>  {
>      MachineClass *mc;
>  
>      mc = current_machine ? MACHINE_GET_CLASS(current_machine) : NULL;
>  
> +    qemu_system_pre_reset();
>      cpu_synchronize_all_states();
>  
>      if (mc && mc->reset) {
> @@ -1750,6 +1794,7 @@ void qemu_system_reset(bool report)
>          qapi_event_send_reset(&error_abort);
>      }
>      cpu_synchronize_all_post_reset();
> +    qemu_system_post_reset();
>  }
>  
>  void qemu_system_guest_panicked(void)
> 
> 
>