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

[Brijesh Singh]

Hi Paolo,


On 09/13/2016 05:47 PM, Paolo Bonzini wrote:



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 


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.



Thanks for review, I will implement this in v2.

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 

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(_reset_handlers, re, entry);
> +}
> +
> +static void qemu_system_pre_reset(void)
> +{
> +QEMUResetEntry *re, *nre;
> +
> +/* call pre_reset handler */
> +QTAILQ_FOREACH_SAFE(re, _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(_reset_handlers, re, entry);
> +}
> +
> +static void qemu_system_post_reset(void)
> +{
> +QEMUResetEntry *re, *nre;
> +
> +/* call post reset handler */
> +QTAILQ_FOREACH_SAFE(re, _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(_abort);
>  }
>