Re: [Qemu-ppc] [PATCH 01/10] xics: Explicitely call KVM ICP methods from the common code

[Cédric Le Goater]

On 2/15/19 12:39 PM, Greg Kurz wrote:
> The pre_save(), post_load() and synchronize_state() methods of the
> ICPStateClass type are really KVM only things. Make that obvious
> by dropping the indirections and directly calling the KVM functions
> instead.
> 
> Signed-off-by: Greg Kurz <address@hidden>



Reviewed-by: Cédric Le Goater <address@hidden>

Thanks,

C.

> ---
>  hw/intc/xics.c        |   24 +++++++++++-------------
>  hw/intc/xics_kvm.c    |   12 ++++--------
>  include/hw/ppc/xics.h |    9 +++++----
>  3 files changed, 20 insertions(+), 25 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 16e8ffa2aaf7..988b53abd17d 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -37,18 +37,18 @@
>  #include "qapi/visitor.h"
>  #include "monitor/monitor.h"
>  #include "hw/intc/intc.h"
> +#include "sysemu/kvm.h"
>  
>  void icp_pic_print_info(ICPState *icp, Monitor *mon)
>  {
> -    ICPStateClass *icpc = ICP_GET_CLASS(icp);
>      int cpu_index = icp->cs ? icp->cs->cpu_index : -1;
>  
>      if (!icp->output) {
>          return;
>      }
>  
> -    if (icpc->synchronize_state) {
> -        icpc->synchronize_state(icp);
> +    if (kvm_irqchip_in_kernel()) {
> +        icp_synchronize_state(icp);
>      }
>  
>      monitor_printf(mon, "CPU %d XIRR=%08x (%p) PP=%02x MFRR=%02x\n",
> @@ -252,25 +252,23 @@ static void icp_irq(ICSState *ics, int server, int nr, 
> uint8_t priority)
>      }
>  }
>  
> -static int icp_dispatch_pre_save(void *opaque)
> +static int icp_pre_save(void *opaque)
>  {
>      ICPState *icp = opaque;
> -    ICPStateClass *info = ICP_GET_CLASS(icp);
>  
> -    if (info->pre_save) {
> -        info->pre_save(icp);
> +    if (kvm_irqchip_in_kernel()) {
> +        icp_get_kvm_state(icp);
>      }
>  
>      return 0;
>  }
>  
> -static int icp_dispatch_post_load(void *opaque, int version_id)
> +static int icp_post_load(void *opaque, int version_id)
>  {
>      ICPState *icp = opaque;
> -    ICPStateClass *info = ICP_GET_CLASS(icp);
>  
> -    if (info->post_load) {
> -        return info->post_load(icp, version_id);
> +    if (kvm_irqchip_in_kernel()) {
> +        return icp_set_kvm_state(icp);
>      }
>  
>      return 0;
> @@ -280,8 +278,8 @@ static const VMStateDescription vmstate_icp_server = {
>      .name = "icp/server",
>      .version_id = 1,
>      .minimum_version_id = 1,
> -    .pre_save = icp_dispatch_pre_save,
> -    .post_load = icp_dispatch_post_load,
> +    .pre_save = icp_pre_save,
> +    .post_load = icp_post_load,
>      .fields = (VMStateField[]) {
>          /* Sanity check */
>          VMSTATE_UINT32(xirr, ICPState),
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index dff13300504c..7efa99b8b434 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -54,7 +54,7 @@ static QLIST_HEAD(, KVMEnabledICP)
>  /*
>   * ICP-KVM
>   */
> -static void icp_get_kvm_state(ICPState *icp)
> +void icp_get_kvm_state(ICPState *icp)
>  {
>      uint64_t state;
>      int ret;
> @@ -83,14 +83,14 @@ static void do_icp_synchronize_state(CPUState *cpu, 
> run_on_cpu_data arg)
>      icp_get_kvm_state(arg.host_ptr);
>  }
>  
> -static void icp_synchronize_state(ICPState *icp)
> +void icp_synchronize_state(ICPState *icp)
>  {
>      if (icp->cs) {
>          run_on_cpu(icp->cs, do_icp_synchronize_state, 
> RUN_ON_CPU_HOST_PTR(icp));
>      }
>  }
>  
> -static int icp_set_kvm_state(ICPState *icp, int version_id)
> +int icp_set_kvm_state(ICPState *icp)
>  {
>      uint64_t state;
>      int ret;
> @@ -121,7 +121,7 @@ static void icp_kvm_reset(DeviceState *dev)
>  
>      icpc->parent_reset(dev);
>  
> -    icp_set_kvm_state(ICP(dev), 1);
> +    icp_set_kvm_state(ICP(dev));
>  }
>  
>  static void icp_kvm_realize(DeviceState *dev, Error **errp)
> @@ -178,10 +178,6 @@ static void icp_kvm_class_init(ObjectClass *klass, void 
> *data)
>                                      &icpc->parent_realize);
>      device_class_set_parent_reset(dc, icp_kvm_reset,
>                                    &icpc->parent_reset);
> -
> -    icpc->pre_save = icp_get_kvm_state;
> -    icpc->post_load = icp_set_kvm_state;
> -    icpc->synchronize_state = icp_synchronize_state;
>  }
>  
>  static const TypeInfo icp_kvm_info = {
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index fad786e8b22d..3236ccec924c 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -66,10 +66,6 @@ struct ICPStateClass {
>  
>      DeviceRealize parent_realize;
>      DeviceReset parent_reset;
> -
> -    void (*pre_save)(ICPState *icp);
> -    int (*post_load)(ICPState *icp, int version_id);
> -    void (*synchronize_state)(ICPState *icp);
>  };
>  
>  struct ICPState {
> @@ -203,4 +199,9 @@ void icp_resend(ICPState *ss);
>  Object *icp_create(Object *cpu, const char *type, XICSFabric *xi,
>                     Error **errp);
>  
> +/* KVM */
> +void icp_get_kvm_state(ICPState *icp);
> +int icp_set_kvm_state(ICPState *icp);
> +void icp_synchronize_state(ICPState *icp);
> +
>  #endif /* XICS_H */
>