Re: [Qemu-devel] [PATCH v8 08/12] spapr: introduce an 'ic-mode' machine option

[David Gibson]

On Tue, Dec 11, 2018 at 11:38:19PM +0100, Cédric Le Goater wrote:
> This option is used to select the interrupt controller mode (XICS or
> XIVE) with which the machine will operate. XICS being the default
> mode for now.
> 
> When running a machine with the XIVE interrupt mode backend, the guest
> OS is required to have support for the XIVE exploitation mode. In the
> case of legacy OS, the mode selected by CAS should be XICS and the OS
> should fail to boot. However, QEMU could possibly detect it, terminate
> the boot process and reset to stop in the SLOF firmware. This is not
> yet handled.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>

Reviewed-by: David Gibson <address@hidden>

> ---
>  include/hw/ppc/spapr.h  |  1 +
>  hw/ppc/spapr.c          | 52 ++++++++++++++++++++++++++++++++++-------
>  hw/ppc/spapr_cpu_core.c |  3 +--
>  hw/ppc/spapr_irq.c      | 34 ++++++++-------------------
>  4 files changed, 56 insertions(+), 34 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 06765b4e9d79..2c77a8ba8810 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -177,6 +177,7 @@ struct sPAPRMachineState {
>      int32_t irq_map_nr;
>      unsigned long *irq_map;
>      sPAPRXive  *xive;
> +    sPAPRIrq *irq;
>  
>      bool cmd_line_caps[SPAPR_CAP_NUM];
>      sPAPRCapabilities def, eff, mig;
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ff138f224a87..5d985e38a598 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1102,7 +1102,6 @@ static void 
> spapr_dt_ov5_platform_support(sPAPRMachineState *spapr, void *fdt,
>                                            int chosen)
>  {
>      PowerPCCPU *first_ppc_cpu = POWERPC_CPU(first_cpu);
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>  
>      char val[2 * 4] = {
>          23, 0x00, /* Xive mode, filled in below. */
> @@ -1122,7 +1121,7 @@ static void 
> spapr_dt_ov5_platform_support(sPAPRMachineState *spapr, void *fdt,
>          /* If the KVM XIVE device is not available, the machine can
>           * still operate with kernel_irqchip=off
>           */
> -        val[1] = smc->irq->ov5;
> +        val[1] = spapr->irq->ov5;
>  
>          if (kvmppc_has_cap_mmu_radix() && kvmppc_has_cap_mmu_hash_v3()) {
>              val[3] = 0x80; /* OV5_MMU_BOTH */
> @@ -1133,7 +1132,7 @@ static void 
> spapr_dt_ov5_platform_support(sPAPRMachineState *spapr, void *fdt,
>          }
>      } else {
>          /* In TCG, the interrupt mode is defined by the pseries machine */
> -        val[1] = smc->irq->ov5;
> +        val[1] = spapr->irq->ov5;
>  
>          /* V3 MMU supports both hash and radix in tcg (with dynamic 
> switching) */
>          val[3] = 0xC0;
> @@ -1281,7 +1280,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>      _FDT(fdt_setprop_cell(fdt, 0, "#size-cells", 2));
>  
>      /* /interrupt controller */
> -    smc->irq->dt_populate(spapr, spapr_max_server_number(spapr), fdt,
> +    spapr->irq->dt_populate(spapr, spapr_max_server_number(spapr), fdt,
>                            PHANDLE_XICP);
>  
>      ret = spapr_populate_memory(spapr, fdt);
> @@ -1302,7 +1301,8 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr,
>      }
>  
>      QLIST_FOREACH(phb, &spapr->phbs, list) {
> -        ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt, 
> smc->irq->nr_msis);
> +        ret = spapr_populate_pci_dt(phb, PHANDLE_XICP, fdt,
> +                                    spapr->irq->nr_msis);
>          if (ret < 0) {
>              error_report("couldn't setup PCI devices in fdt");
>              exit(1);
> @@ -2636,7 +2636,7 @@ static void spapr_machine_init(MachineState *machine)
>      spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>  
>      /* advertise XIVE on POWER9 machines */
> -    if (smc->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
> +    if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
>          if (ppc_type_check_compat(machine->cpu_type, 
> CPU_POWERPC_LOGICAL_3_00,
>                                    0, spapr->max_compat_pvr)) {
>              spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT);
> @@ -3056,9 +3056,38 @@ static void spapr_set_vsmt(Object *obj, Visitor *v, 
> const char *name,
>      visit_type_uint32(v, name, (uint32_t *)opaque, errp);
>  }
>  
> +static char *spapr_get_ic_mode(Object *obj, Error **errp)
> +{
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> +
> +    if (spapr->irq == &spapr_irq_xics_legacy) {
> +        return g_strdup("legacy");
> +    } else if (spapr->irq == &spapr_irq_xics) {
> +        return g_strdup("xics");
> +    } else if (spapr->irq == &spapr_irq_xive) {
> +        return g_strdup("xive");
> +    }
> +    g_assert_not_reached();
> +}
> +
> +static void spapr_set_ic_mode(Object *obj, const char *value, Error **errp)
> +{
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> +
> +    /* The legacy IRQ backend can not be set */
> +    if (strcmp(value, "xics") == 0) {
> +        spapr->irq = &spapr_irq_xics;
> +    } else if (strcmp(value, "xive") == 0) {
> +        spapr->irq = &spapr_irq_xive;
> +    } else {
> +        error_setg(errp, "Bad value for \"ic-mode\" property");
> +    }
> +}
> +
>  static void spapr_instance_init(Object *obj)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> +    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>  
>      spapr->htab_fd = -1;
>      spapr->use_hotplug_event_source = true;
> @@ -3092,6 +3121,14 @@ static void spapr_instance_init(Object *obj)
>                                      " the host's SMT mode", &error_abort);
>      object_property_add_bool(obj, "vfio-no-msix-emulation",
>                               spapr_get_msix_emulation, NULL, NULL);
> +
> +    /* The machine class defines the default interrupt controller mode */
> +    spapr->irq = smc->irq;
> +    object_property_add_str(obj, "ic-mode", spapr_get_ic_mode,
> +                            spapr_set_ic_mode, NULL);
> +    object_property_set_description(obj, "ic-mode",
> +                 "Specifies the interrupt controller mode (xics, xive)",
> +                 NULL);
>  }
>  
>  static void spapr_machine_finalizefn(Object *obj)
> @@ -3814,9 +3851,8 @@ static void spapr_pic_print_info(InterruptStatsProvider 
> *obj,
>                                   Monitor *mon)
>  {
>      sPAPRMachineState *spapr = SPAPR_MACHINE(obj);
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>  
> -    smc->irq->print_info(spapr, mon);
> +    spapr->irq->print_info(spapr, mon);
>  }
>  
>  int spapr_get_vcpu_id(PowerPCCPU *cpu)
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 1811cd48db90..82666436e9b4 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -214,7 +214,6 @@ static void spapr_cpu_core_unrealize(DeviceState *dev, 
> Error **errp)
>  static void spapr_realize_vcpu(PowerPCCPU *cpu, sPAPRMachineState *spapr,
>                                 sPAPRCPUCore *sc, Error **errp)
>  {
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>      CPUPPCState *env = &cpu->env;
>      CPUState *cs = CPU(cpu);
>      Error *local_err = NULL;
> @@ -233,7 +232,7 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, 
> sPAPRMachineState *spapr,
>      qemu_register_reset(spapr_cpu_reset, cpu);
>      spapr_cpu_reset(cpu);
>  
> -    cpu->intc = smc->irq->cpu_intc_create(spapr, OBJECT(cpu), &local_err);
> +    cpu->intc = spapr->irq->cpu_intc_create(spapr, OBJECT(cpu), &local_err);
>      if (local_err) {
>          goto error_unregister;
>      }
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 79f06308995b..0999a2b2d69c 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -94,8 +94,7 @@ error:
>  static void spapr_irq_init_xics(sPAPRMachineState *spapr, Error **errp)
>  {
>      MachineState *machine = MACHINE(spapr);
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -    int nr_irqs = smc->irq->nr_irqs;
> +    int nr_irqs = spapr->irq->nr_irqs;
>      Error *local_err = NULL;
>  
>      if (kvm_enabled()) {
> @@ -234,7 +233,6 @@ sPAPRIrq spapr_irq_xics = {
>  static void spapr_irq_init_xive(sPAPRMachineState *spapr, Error **errp)
>  {
>      MachineState *machine = MACHINE(spapr);
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
>      uint32_t nr_servers = spapr_max_server_number(spapr);
>      DeviceState *dev;
>      int i;
> @@ -248,7 +246,7 @@ static void spapr_irq_init_xive(sPAPRMachineState *spapr, 
> Error **errp)
>      }
>  
>      dev = qdev_create(NULL, TYPE_SPAPR_XIVE);
> -    qdev_prop_set_uint32(dev, "nr-irqs", smc->irq->nr_irqs);
> +    qdev_prop_set_uint32(dev, "nr-irqs", spapr->irq->nr_irqs);
>      /*
>       * 8 XIVE END structures per CPU. One for each available priority
>       */
> @@ -361,50 +359,38 @@ sPAPRIrq spapr_irq_xive = {
>   */
>  void spapr_irq_init(sPAPRMachineState *spapr, Error **errp)
>  {
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
>      /* Initialize the MSI IRQ allocator. */
>      if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
> -        spapr_irq_msi_init(spapr, smc->irq->nr_msis);
> +        spapr_irq_msi_init(spapr, spapr->irq->nr_msis);
>      }
>  
> -    smc->irq->init(spapr, errp);
> +    spapr->irq->init(spapr, errp);
>  }
>  
>  int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error 
> **errp)
>  {
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
> -    return smc->irq->claim(spapr, irq, lsi, errp);
> +    return spapr->irq->claim(spapr, irq, lsi, errp);
>  }
>  
>  void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num)
>  {
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
> -    smc->irq->free(spapr, irq, num);
> +    spapr->irq->free(spapr, irq, num);
>  }
>  
>  qemu_irq spapr_qirq(sPAPRMachineState *spapr, int irq)
>  {
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
> -    return smc->irq->qirq(spapr, irq);
> +    return spapr->irq->qirq(spapr, irq);
>  }
>  
>  int spapr_irq_post_load(sPAPRMachineState *spapr, int version_id)
>  {
> -    sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr);
> -
> -    return smc->irq->post_load(spapr, version_id);
> +    return spapr->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);
> +    if (spapr->irq->reset) {
> +        spapr->irq->reset(spapr, errp);
>      }
>  }
>  

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