Re: [Qemu-ppc] [PATCH 09/10] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS

[David Gibson]

On Wed, Jan 02, 2019 at 06:57:42AM +0100, Cédric Le Goater wrote:
> The 'dual' sPAPR IRQ backend supports both interrupt mode, XIVE
> exploitation mode and the legacy compatibility mode (XICS). both modes
> are not supported at the same time.
> 
> The machine starts with the legacy mode and a new interrupt mode can
> then be negotiated by the CAS process. In this case, the new mode is
> activated after a reset to take into account the required changes in
> the machine. These impact the device tree layout, the interrupt
> presenter object and the exposed MMIO regions in the case of XIVE.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  include/hw/ppc/spapr_irq.h |   1 +
>  hw/ppc/spapr.c             |  10 ++-
>  hw/ppc/spapr_hcall.c       |  11 +++
>  hw/ppc/spapr_irq.c         | 179 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 198 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 283bb5002c16..14b02c3aca33 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -52,6 +52,7 @@ typedef struct sPAPRIrq {
>  extern sPAPRIrq spapr_irq_xics;
>  extern sPAPRIrq spapr_irq_xics_legacy;
>  extern sPAPRIrq spapr_irq_xive;
> +extern sPAPRIrq spapr_irq_dual;
>  
>  void spapr_irq_init(sPAPRMachineState *spapr, Error **errp);
>  int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error 
> **errp);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 5e8ffda47372..eb8ef741d860 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2633,11 +2633,11 @@ static void spapr_machine_init(MachineState *machine)
>      spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2);
>  
>      /* advertise XIVE on POWER9 machines */
> -    if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
> +    if (spapr->irq->ov5 & (SPAPR_OV5_XIVE_EXPLOIT | SPAPR_OV5_XIVE_BOTH)) {
>          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);
> -        } else {
> +        } else if (spapr->irq->ov5 & SPAPR_OV5_XIVE_EXPLOIT) {
>              error_report("XIVE-only machines require a POWER9 CPU");
>              exit(1);
>          }
> @@ -3063,6 +3063,8 @@ static char *spapr_get_ic_mode(Object *obj, Error 
> **errp)
>          return g_strdup("xics");
>      } else if (spapr->irq == &spapr_irq_xive) {
>          return g_strdup("xive");
> +    } else if (spapr->irq == &spapr_irq_dual) {
> +        return g_strdup("dual");
>      }
>      g_assert_not_reached();
>  }
> @@ -3076,6 +3078,8 @@ static void spapr_set_ic_mode(Object *obj, const char 
> *value, Error **errp)
>          spapr->irq = &spapr_irq_xics;
>      } else if (strcmp(value, "xive") == 0) {
>          spapr->irq = &spapr_irq_xive;
> +    } else if (strcmp(value, "dual") == 0) {
> +        spapr->irq = &spapr_irq_dual;
>      } else {
>          error_setg(errp, "Bad value for \"ic-mode\" property");
>      }
> @@ -3124,7 +3128,7 @@ static void spapr_instance_init(Object *obj)
>      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)",
> +                 "Specifies the interrupt controller mode (xics, xive, 
> dual)",
>                   NULL);
>  }
>  
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index ae913d070f50..45c35d41fac6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1654,6 +1654,17 @@ static target_ulong 
> h_client_architecture_support(PowerPCCPU *cpu,
>              (spapr_h_cas_compose_response(spapr, args[1], args[2],
>                                            ov5_updates) != 0);
>      }
> +
> +    /*
> +     * Generate a machine reset when we have an update of the
> +     * interrupt mode. Only required when the machine supports both
> +     * modes.
> +     */
> +    if (!spapr->cas_reboot) {
> +        spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT)
> +            && spapr->irq->ov5 & SPAPR_OV5_XIVE_BOTH;
> +    }
> +
>      spapr_ovec_cleanup(ov5_updates);
>  
>      if (spapr->cas_reboot) {
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index d23914887ac0..d110b8cdeec7 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -230,6 +230,11 @@ static void spapr_irq_set_irq_xics(void *opaque, int 
> srcno, int val)
>      }
>  }
>  
> +static void spapr_irq_reset_xics(sPAPRMachineState *spapr, Error **errp)
> +{
> +    /* TODO: create the KVM XICS device */
> +}
> +
>  #define SPAPR_IRQ_XICS_NR_IRQS     0x1000
>  #define SPAPR_IRQ_XICS_NR_MSIS     \
>      (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> @@ -247,6 +252,7 @@ sPAPRIrq spapr_irq_xics = {
>      .dt_populate = spapr_dt_xics,
>      .cpu_intc_create = spapr_irq_cpu_intc_create_xics,
>      .post_load   = spapr_irq_post_load_xics,
> +    .reset       = spapr_irq_reset_xics,
>      .set_irq     = spapr_irq_set_irq_xics,
>  };
>  
> @@ -403,6 +409,179 @@ sPAPRIrq spapr_irq_xive = {
>      .set_irq     = spapr_irq_set_irq_xive,
>  };
>  
> +/*
> + * Dual XIVE and XICS IRQ backend.
> + *
> + * Both interrupt mode, XIVE and XICS, objects are created but the
> + * machine starts in legacy interrupt mode (XICS). It can be changed
> + * by the CAS negotiation process and, in that case, the new mode is
> + * activated after an extra machine reset.
> + */
> +
> +/*
> + * Returns the sPAPR IRQ backend negotiated by CAS. XICS is the
> + * default.
> + */
> +static sPAPRIrq *spapr_irq_current(sPAPRMachineState *spapr)
> +{
> +    return spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT) ?
> +        &spapr_irq_xive : &spapr_irq_xics;
> +}
> +
> +static void spapr_irq_init_dual(sPAPRMachineState *spapr, Error **errp)
> +{
> +    MachineState *machine = MACHINE(spapr);
> +    Error *local_err = NULL;
> +
> +    if (kvm_enabled() && machine_kernel_irqchip_allowed(machine)) {
> +        error_setg(errp, "No KVM support for the 'dual' machine");
> +        return;
> +    }
> +
> +    spapr_irq_xics.init(spapr, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    /*
> +     * Align the XICS and the XIVE IRQ number space under QEMU.
> +     *
> +     * However, the XICS KVM device still considers that the IRQ
> +     * numbers should start at XICS_IRQ_BASE (0x1000). Either we
> +     * should introduce a KVM device ioctl to set the offset or ignore
> +     * the lower 4K numbers when using the get/set ioctl of the XICS
> +     * KVM device. The second option seems the least intrusive.
> +     */
> +    spapr->ics->offset = 0;
> +
> +    spapr_irq_xive.init(spapr, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +}
> +
> +static int spapr_irq_claim_dual(sPAPRMachineState *spapr, int irq, bool lsi,
> +                                Error **errp)
> +{
> +    Error *local_err = NULL;
> +    int ret;
> +
> +    ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return ret;
> +    }
> +
> +    ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return ret;
> +    }
> +
> +    return ret;
> +}
> +
> +static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
> +{
> +    spapr_irq_xics.free(spapr, irq, num);
> +    spapr_irq_xive.free(spapr, irq, num);
> +}
> +
> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)

If the qirq array is now at the machine level, you shouldn't need an
irq backend version of the qirq function should you?  Just a backend
specific version of set_irq.

> +{
> +    sPAPRXive *xive = spapr->xive;
> +    ICSState *ics = spapr->ics;
> +
> +    if (irq >= spapr->irq->nr_irqs) {
> +        return NULL;
> +    }
> +
> +    /*
> +     * The IRQ number should have been claimed under both interrupt
> +     * controllers.
> +     */
> +    assert(!ICS_IRQ_FREE(ics, irq - ics->offset));
> +    assert(xive_eas_is_valid(&xive->eat[irq]));
> +
> +    return spapr->qirqs[irq];
> +}
> +
> +static void spapr_irq_print_info_dual(sPAPRMachineState *spapr, Monitor *mon)
> +{
> +    spapr_irq_current(spapr)->print_info(spapr, mon);
> +}
> +
> +static void spapr_irq_dt_populate_dual(sPAPRMachineState *spapr,
> +                                       uint32_t nr_servers, void *fdt,
> +                                       uint32_t phandle)
> +{
> +    spapr_irq_current(spapr)->dt_populate(spapr, nr_servers, fdt, phandle);
> +}
> +
> +static void spapr_irq_cpu_intc_create_dual(sPAPRMachineState *spapr,
> +                                           PowerPCCPU *cpu, Error **errp)
> +{
> +    Error *local_err = NULL;
> +
> +    spapr_irq_xive.cpu_intc_create(spapr, cpu, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return;
> +    }
> +
> +    spapr_irq_xics.cpu_intc_create(spapr, cpu, errp);
> +}
> +
> +static int spapr_irq_post_load_dual(sPAPRMachineState *spapr, int version_id)
> +{
> +    /*
> +     * Force a reset of the XIVE backend after migration. The machine
> +     * defaults to XICS at startup.
> +     */
> +    if (spapr_ovec_test(spapr->ov5_cas, OV5_XIVE_EXPLOIT)) {
> +        spapr_irq_xive.reset(spapr, &error_fatal);
> +    }
> +
> +    return spapr_irq_current(spapr)->post_load(spapr, version_id);
> +}
> +
> +static void spapr_irq_reset_dual(sPAPRMachineState *spapr, Error **errp)
> +{
> +    spapr_irq_current(spapr)->reset(spapr, errp);
> +}
> +
> +static void spapr_irq_set_irq_dual(void *opaque, int srcno, int val)
> +{
> +    sPAPRMachineState *spapr = opaque;
> +
> +    spapr_irq_current(spapr)->set_irq(spapr, srcno, val);
> +}
> +
> +/*
> + * Define values in sync with the XIVE and XICS backend
> + */
> +#define SPAPR_IRQ_DUAL_NR_IRQS     0x2000
> +#define SPAPR_IRQ_DUAL_NR_MSIS     (SPAPR_IRQ_DUAL_NR_IRQS - SPAPR_IRQ_MSI)
> +
> +sPAPRIrq spapr_irq_dual = {
> +    .nr_irqs     = SPAPR_IRQ_DUAL_NR_IRQS,
> +    .nr_msis     = SPAPR_IRQ_DUAL_NR_MSIS,
> +    .ov5         = SPAPR_OV5_XIVE_BOTH,
> +
> +    .init        = spapr_irq_init_dual,
> +    .claim       = spapr_irq_claim_dual,
> +    .free        = spapr_irq_free_dual,
> +    .qirq        = spapr_qirq_dual,
> +    .print_info  = spapr_irq_print_info_dual,
> +    .dt_populate = spapr_irq_dt_populate_dual,
> +    .cpu_intc_create = spapr_irq_cpu_intc_create_dual,
> +    .post_load   = spapr_irq_post_load_dual,
> +    .reset       = spapr_irq_reset_dual,
> +    .set_irq     = spapr_irq_set_irq_dual
> +};
> +
>  /*
>   * sPAPR IRQ frontend routines for devices
>   */

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