Re: [Qemu-ppc] [PATCH v5 24/36] spapr: add a 'reset' method to the sPAPR IRQ backend

[David Gibson]

On Fri, Nov 16, 2018 at 11:57:17AM +0100, Cédric Le Goater wrote:
> This method will become useful when the new machine supporting both
> interrupt modes, XIVE and XICS, is introduced. In this machine, the
> interrupt mode is chosen by the CAS negotiation process and activated
> after a reset.
> 
> For the time being, the only thing that can be done in the XIVE reset
> handler is to map the pages for the TIMA and for the source ESBs.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  include/hw/ppc/spapr_irq.h  |  2 ++
>  include/hw/ppc/spapr_xive.h |  1 +
>  hw/intc/spapr_xive.c        |  4 +---
>  hw/ppc/spapr.c              |  2 ++
>  hw/ppc/spapr_irq.c          | 21 +++++++++++++++++++++
>  5 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 4e36c0984e1a..34128976e21c 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -46,6 +46,7 @@ typedef struct sPAPRIrq {
>      Object *(*cpu_intc_create)(sPAPRMachineState *spapr, Object *cpu,
>                                 Error **errp);
>      int (*post_load)(sPAPRMachineState *spapr, int version_id);
> +    void (*reset)(sPAPRMachineState *spapr, Error **errp);
>  } sPAPRIrq;
>  
>  extern sPAPRIrq spapr_irq_xics;
> @@ -57,6 +58,7 @@ int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool 
> lsi, Error **errp);
>  void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
>  qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq);
>  int spapr_irq_post_load(sPAPRMachineState *spapr, int version_id);
> +void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp);
>  
>  /*
>   * XICS legacy routines
> diff --git a/include/hw/ppc/spapr_xive.h b/include/hw/ppc/spapr_xive.h
> index d2517c040958..fa7f3d7718da 100644
> --- a/include/hw/ppc/spapr_xive.h
> +++ b/include/hw/ppc/spapr_xive.h
> @@ -91,6 +91,7 @@ typedef struct sPAPRMachineState sPAPRMachineState;
>  void spapr_xive_hcall_init(sPAPRMachineState *spapr);
>  void spapr_dt_xive(sPAPRXive *xive, int nr_servers, void *fdt,
>                     uint32_t phandle);
> +void spapr_xive_mmio_map(sPAPRXive *xive);
>  
>  /*
>   * XIVE KVM models
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index c5c0e063dc33..def43160e12a 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -51,7 +51,7 @@ void spapr_xive_pic_print_info(sPAPRXive *xive, Monitor 
> *mon)
>  }
>  
>  /* Map the ESB pages and the TIMA pages */
> -static void spapr_xive_mmio_map(sPAPRXive *xive)
> +void spapr_xive_mmio_map(sPAPRXive *xive)
>  {
>      sysbus_mmio_map(SYS_BUS_DEVICE(&xive->source), 0, xive->vc_base);
>      sysbus_mmio_map(SYS_BUS_DEVICE(&xive->end_source), 0, xive->end_base);
> @@ -77,8 +77,6 @@ static void spapr_xive_base_reset(DeviceState *dev)
>      for (i = 0; i < xive->nr_ends; i++) {
>          xive_end_reset(&xive->endt[i]);
>      }
> -
> -    spapr_xive_mmio_map(xive);
>  }
>  
>  static void spapr_xive_base_instance_init(Object *obj)
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d1be2579cd9b..013e6ea8aa64 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1628,6 +1628,8 @@ static void spapr_machine_reset(void)
>          spapr_irq_msi_reset(spapr);
>      }
>  
> +    spapr_irq_reset(spapr, &error_fatal);
> +
>      qemu_devices_reset();
>  
>      /* DRC reset may cause a device to be unplugged. This will cause troubles
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 6fac6ca70595..984c6d60cd9f 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -13,6 +13,7 @@
>  #include "qapi/error.h"
>  #include "hw/ppc/spapr.h"
>  #include "hw/ppc/spapr_xive.h"
> +#include "hw/ppc/spapr_cpu_core.h"
>  #include "hw/ppc/xics.h"
>  #include "sysemu/kvm.h"
>  
> @@ -215,6 +216,10 @@ static int spapr_irq_post_load_xics(sPAPRMachineState 
> *spapr, int version_id)
>      return 0;
>  }
>  
> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> +{
> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -232,6 +237,7 @@ sPAPRIrq spapr_irq_xics = {
>      .dt_populate = spapr_irq_dt_populate_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>      .post_load   = spapr_irq_post_load_xics,
> +    .reset       = spapr_irq_reset_xics,
>  };
>  
>   /*
> @@ -362,6 +368,11 @@ static int spapr_irq_post_load_xive(sPAPRMachineState 
> *spapr, int version_id)
>      return spapr_xive_post_load(spapr->xive, version_id);
>  }
>  
> +static void spapr_irq_reset_xive(sPAPRMachineState *spapr, Error **errp)
> +{
> +    spapr_xive_mmio_map(spapr->xive);

It's usually not a good idea to actually construct different
MemoryRegion's at run time.  Instead map them all in, but disable the
ones you don't want (with memory_region_set_enabled()).

I think your current version will also leave the TIMA etc. still
mapped if you reboot from a XIVE guest to a XICS guest.

> +}
> +
>  /*
>   * XIVE uses the full IRQ number space. Set it to 8K to be compatible
>   * with XICS.
> @@ -383,6 +394,7 @@ sPAPRIrq spapr_irq_xive = {
>      .dt_populate = spapr_irq_dt_populate_xive,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xive,
>      .post_load   = spapr_irq_post_load_xive,
> +    .reset       = spapr_irq_reset_xive,
>  };
>  
>  /*
> @@ -428,6 +440,15 @@ int spapr_irq_post_load(sPAPRMachineState *spapr, int 
> version_id)
>      return smc->irq->post_load(spapr, version_id);
>  }
>  
> +void spapr_irq_reset(sPAPRMachineState *spapr, Error **errp)
> +{
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> +
> +    if (smc->irq->reset) {
> +        smc->irq->reset(spapr, errp);
> +    }
> +}
> +
>  /*
>   * XICS legacy routines - to deprecate one day
>   */

-- 
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

signature.asc
Description: PGP signature