Re: [Qemu-devel] [PATCH 06/10] xics: Explicitely call KVM ICS methods from the common code

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 5350 bytes --]

On Fri, Feb 15, 2019 at 12:40:18PM +0100, Greg Kurz wrote:
> The pre_save(), post_load() and synchronize_state() methods of the
> ICSStateClass 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 <groug@kaod.org>

Applied, thanks.

> ---
>  hw/intc/xics.c        |   23 ++++++++++-------------
>  hw/intc/xics_kvm.c    |   12 ++++--------
>  include/hw/ppc/xics.h |    7 ++++---
>  3 files changed, 18 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index acd63ab5e0b9..ae5d5ea135b8 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -58,7 +58,6 @@ void icp_pic_print_info(ICPState *icp, Monitor *mon)
>  
>  void ics_pic_print_info(ICSState *ics, Monitor *mon)
>  {
> -    ICSStateClass *icsc = ICS_BASE_GET_CLASS(ics);
>      uint32_t i;
>  
>      monitor_printf(mon, "ICS %4x..%4x %p\n",
> @@ -68,8 +67,8 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon)
>          return;
>      }
>  
> -    if (icsc->synchronize_state) {
> -        icsc->synchronize_state(ics);
> +    if (kvm_irqchip_in_kernel()) {
> +        ics_synchronize_state(ics);
>      }
>  
>      for (i = 0; i < ics->nr_irqs; i++) {
> @@ -647,25 +646,23 @@ static void ics_base_instance_init(Object *obj)
>      ics->offset = XICS_IRQ_BASE;
>  }
>  
> -static int ics_base_dispatch_pre_save(void *opaque)
> +static int ics_base_pre_save(void *opaque)
>  {
>      ICSState *ics = opaque;
> -    ICSStateClass *info = ICS_BASE_GET_CLASS(ics);
>  
> -    if (info->pre_save) {
> -        info->pre_save(ics);
> +    if (kvm_irqchip_in_kernel()) {
> +        ics_get_kvm_state(ics);
>      }
>  
>      return 0;
>  }
>  
> -static int ics_base_dispatch_post_load(void *opaque, int version_id)
> +static int ics_base_post_load(void *opaque, int version_id)
>  {
>      ICSState *ics = opaque;
> -    ICSStateClass *info = ICS_BASE_GET_CLASS(ics);
>  
> -    if (info->post_load) {
> -        return info->post_load(ics, version_id);
> +    if (kvm_irqchip_in_kernel()) {
> +        return ics_set_kvm_state(ics);
>      }
>  
>      return 0;
> @@ -689,8 +686,8 @@ static const VMStateDescription vmstate_ics_base = {
>      .name = "ics",
>      .version_id = 1,
>      .minimum_version_id = 1,
> -    .pre_save = ics_base_dispatch_pre_save,
> -    .post_load = ics_base_dispatch_post_load,
> +    .pre_save = ics_base_pre_save,
> +    .post_load = ics_base_post_load,
>      .fields = (VMStateField[]) {
>          /* Sanity check */
>          VMSTATE_UINT32_EQUAL(nr_irqs, ICSState, NULL),
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index fae4ac431f2f..642351e5790f 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -155,7 +155,7 @@ void icp_kvm_realize(DeviceState *dev, Error **errp)
>  /*
>   * ICS-KVM
>   */
> -static void ics_get_kvm_state(ICSState *ics)
> +void ics_get_kvm_state(ICSState *ics)
>  {
>      uint64_t state;
>      int i;
> @@ -208,12 +208,12 @@ static void ics_get_kvm_state(ICSState *ics)
>      }
>  }
>  
> -static void ics_synchronize_state(ICSState *ics)
> +void ics_synchronize_state(ICSState *ics)
>  {
>      ics_get_kvm_state(ics);
>  }
>  
> -static int ics_set_kvm_state(ICSState *ics, int version_id)
> +int ics_set_kvm_state(ICSState *ics)
>  {
>      uint64_t state;
>      int i;
> @@ -286,7 +286,7 @@ static void ics_kvm_reset(DeviceState *dev)
>  
>      icsc->parent_reset(dev);
>  
> -    ics_set_kvm_state(ICS_KVM(dev), 1);
> +    ics_set_kvm_state(ICS_KVM(dev));
>  }
>  
>  static void ics_kvm_reset_handler(void *dev)
> @@ -318,10 +318,6 @@ static void ics_kvm_class_init(ObjectClass *klass, void *data)
>                                      &icsc->parent_realize);
>      device_class_set_parent_reset(dc, ics_kvm_reset,
>                                    &icsc->parent_reset);
> -
> -    icsc->pre_save = ics_get_kvm_state;
> -    icsc->post_load = ics_set_kvm_state;
> -    icsc->synchronize_state = ics_synchronize_state;
>  }
>  
>  static const TypeInfo ics_kvm_info = {
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index fae54e6f28ae..06e87128f88e 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -109,12 +109,9 @@ struct ICSStateClass {
>      DeviceRealize parent_realize;
>      DeviceReset parent_reset;
>  
> -    void (*pre_save)(ICSState *s);
> -    int (*post_load)(ICSState *s, int version_id);
>      void (*reject)(ICSState *s, uint32_t irq);
>      void (*resend)(ICSState *s);
>      void (*eoi)(ICSState *s, uint32_t irq);
> -    void (*synchronize_state)(ICSState *s);
>  };
>  
>  struct ICSState {
> @@ -201,4 +198,8 @@ int icp_set_kvm_state(ICPState *icp);
>  void icp_synchronize_state(ICPState *icp);
>  void icp_kvm_realize(DeviceState *dev, Error **errp);
>  
> +void ics_get_kvm_state(ICSState *ics);
> +int ics_set_kvm_state(ICSState *ics);
> +void ics_synchronize_state(ICSState *ics);
> +
>  #endif /* XICS_H */
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]