diff mbox series

[v8,11/12] spapr: introduce a new sPAPR IRQ backend supporting XIVE and XICS

Message ID 20181211223823.13770-12-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series ppc: support for the XIVE interrupt controller (POWER9) | expand

Commit Message

Cédric Le Goater Dec. 11, 2018, 10:38 p.m. UTC
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 <clg@kaod.org>
---

 Changes since v7:

 - usage of 'ic-mode' machine option

 include/hw/ppc/spapr_irq.h |   1 +
 hw/ppc/spapr.c             |  10 ++-
 hw/ppc/spapr_hcall.c       |  11 +++
 hw/ppc/spapr_irq.c         | 143 +++++++++++++++++++++++++++++++++++++
 4 files changed, 162 insertions(+), 3 deletions(-)

Comments

David Gibson Dec. 17, 2018, 6:07 a.m. UTC | #1
On Tue, Dec 11, 2018 at 11:38:22PM +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 <clg@kaod.org>
> ---
> 
>  Changes since v7:
> 
>  - usage of 'ic-mode' machine option
> 
>  include/hw/ppc/spapr_irq.h |   1 +
>  hw/ppc/spapr.c             |  10 ++-
>  hw/ppc/spapr_hcall.c       |  11 +++
>  hw/ppc/spapr_irq.c         | 143 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 162 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index b34d5a00381b..29936498dbc8 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -51,6 +51,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 5d985e38a598..97a5e3c9929f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2636,11 +2636,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);
>          }
> @@ -3066,6 +3066,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();
>  }
> @@ -3079,6 +3081,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");
>      }
> @@ -3127,7 +3131,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..09386458f267 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 on the machine supporting both
> +     * mode.
> +     */
> +    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 b1319905327f..31d195b08952 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -375,6 +375,149 @@ sPAPRIrq spapr_irq_xive = {
>      .reset       = spapr_irq_reset_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 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;
> +    }
> +
> +    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)
> +{
> +    int ret;
> +    Error *local_err = NULL;
> +
> +    ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        return ret;
> +    }
> +
> +    ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +    }
> +
> +    return ret;
> +}
> +
> +static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
> +{
> +    spapr_irq_xive.free(spapr, irq, num);
> +    spapr_irq_xics.free(spapr, irq, num);
> +}
> +
> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
> +{
> +    return spapr_irq_current(spapr)->qirq(spapr, irq);
> +}

This still makes me really nervous - I'd really prefer to have qirqs
independent of the backend, rather than relying on *every* irq using
device never looking up qirqs in advance.

> +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 Object *spapr_irq_cpu_intc_create_dual(sPAPRMachineState *spapr,
> +                                              Object *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 NULL;
> +    }
> +
> +    /* Default to XICS interrupt mode */
> +    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)
> +{
> +    /*
> +     * Reset the interrupt mode selected by CAS.
> +     */
> +    spapr_irq_current(spapr)->reset(spapr, errp);
> +}
> +
> +/*
> + * 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,
> +};
> +
>  /*
>   * sPAPR IRQ frontend routines for devices
>   */
Cédric Le Goater Dec. 17, 2018, 10:38 p.m. UTC | #2
On 12/17/18 7:07 AM, David Gibson wrote:
> On Tue, Dec 11, 2018 at 11:38:22PM +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 <clg@kaod.org>
>> ---
>>
>>  Changes since v7:
>>
>>  - usage of 'ic-mode' machine option
>>
>>  include/hw/ppc/spapr_irq.h |   1 +
>>  hw/ppc/spapr.c             |  10 ++-
>>  hw/ppc/spapr_hcall.c       |  11 +++
>>  hw/ppc/spapr_irq.c         | 143 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 162 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>> index b34d5a00381b..29936498dbc8 100644
>> --- a/include/hw/ppc/spapr_irq.h
>> +++ b/include/hw/ppc/spapr_irq.h
>> @@ -51,6 +51,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 5d985e38a598..97a5e3c9929f 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2636,11 +2636,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);
>>          }
>> @@ -3066,6 +3066,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();
>>  }
>> @@ -3079,6 +3081,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");
>>      }
>> @@ -3127,7 +3131,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..09386458f267 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 on the machine supporting both
>> +     * mode.
>> +     */
>> +    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 b1319905327f..31d195b08952 100644
>> --- a/hw/ppc/spapr_irq.c
>> +++ b/hw/ppc/spapr_irq.c
>> @@ -375,6 +375,149 @@ sPAPRIrq spapr_irq_xive = {
>>      .reset       = spapr_irq_reset_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 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;
>> +    }
>> +
>> +    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)
>> +{
>> +    int ret;
>> +    Error *local_err = NULL;
>> +
>> +    ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +        return ret;
>> +    }
>> +
>> +    ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err);
>> +    if (local_err) {
>> +        error_propagate(errp, local_err);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
>> +{
>> +    spapr_irq_xive.free(spapr, irq, num);
>> +    spapr_irq_xics.free(spapr, irq, num);
>> +}
>> +
>> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
>> +{
>> +    return spapr_irq_current(spapr)->qirq(spapr, irq);
>> +}
> 
> This still makes me really nervous - I'd really prefer to have qirqs
> independent of the backend, rather than relying on *every* irq using
> device never looking up qirqs in advance.

I will take a look. This is a large rework I won't have time to address 
this year. I have removed the dual machine from v9.

You would move the qirq array at the machine level ?  

Thanks,

C.


> 
>> +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 Object *spapr_irq_cpu_intc_create_dual(sPAPRMachineState *spapr,
>> +                                              Object *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 NULL;
>> +    }
>> +
>> +    /* Default to XICS interrupt mode */
>> +    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)
>> +{
>> +    /*
>> +     * Reset the interrupt mode selected by CAS.
>> +     */
>> +    spapr_irq_current(spapr)->reset(spapr, errp);
>> +}
>> +
>> +/*
>> + * 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,
>> +};
>> +
>>  /*
>>   * sPAPR IRQ frontend routines for devices
>>   */
>
Cédric Le Goater Dec. 19, 2018, 7:15 p.m. UTC | #3
[ ... ]

>>> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
>>> +{
>>> +    return spapr_irq_current(spapr)->qirq(spapr, irq);
>>> +}
>>
>> This still makes me really nervous - I'd really prefer to have qirqs
>> independent of the backend, rather than relying on *every* irq using
>> device never looking up qirqs in advance.
> 
> I will take a look. This is a large rework I won't have time to address 
> this year. I have removed the dual machine from v9.
> 
> You would move the qirq array at the machine level ?  

I took a look today and did a few changes : 

 - move the qirq array at the machine level
 - introduced a 'set_irq' method to sPAPR IRQ
 - adapted the 'qirq' method of sPAPR IRQ. We still need to perform some
   checks and to handle the IRQ number offset.

It falls well in place, a part for the ICS source of the PnvPSI model 
which does not have any qirq anymore. For PSI, I am thinking of moving 
the qirq array under PnvPSI model, like I did for the machine. 

Would that be ok ?

I think there are a couple more possible cleanups on the different ICS 
models to do if these changes are acceptable. 

C.
David Gibson Dec. 21, 2018, 1:50 a.m. UTC | #4
On Wed, Dec 19, 2018 at 08:15:36PM +0100, Cédric Le Goater wrote:
> [ ... ]
> 
> >>> +static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
> >>> +{
> >>> +    return spapr_irq_current(spapr)->qirq(spapr, irq);
> >>> +}
> >>
> >> This still makes me really nervous - I'd really prefer to have qirqs
> >> independent of the backend, rather than relying on *every* irq using
> >> device never looking up qirqs in advance.
> > 
> > I will take a look. This is a large rework I won't have time to address 
> > this year. I have removed the dual machine from v9.
> > 
> > You would move the qirq array at the machine level ?  
> 
> I took a look today and did a few changes : 
> 
>  - move the qirq array at the machine level
>  - introduced a 'set_irq' method to sPAPR IRQ
>  - adapted the 'qirq' method of sPAPR IRQ. We still need to perform some
>    checks and to handle the IRQ number offset.
> 
> It falls well in place, a part for the ICS source of the PnvPSI model 
> which does not have any qirq anymore. For PSI, I am thinking of moving 
> the qirq array under PnvPSI model, like I did for the machine. 
> 
> Would that be ok ?

That sounds reasonable.  I'd been thinking of having a qirq array at
the machine level which dispatched to other qirq arrays at the ICS or
XiveSource levels, but if you don't need that, that's ok too.
> 
> I think there are a couple more possible cleanups on the different ICS 
> models to do if these changes are acceptable. 
> 
> C. 
>
diff mbox series

Patch

diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
index b34d5a00381b..29936498dbc8 100644
--- a/include/hw/ppc/spapr_irq.h
+++ b/include/hw/ppc/spapr_irq.h
@@ -51,6 +51,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 5d985e38a598..97a5e3c9929f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2636,11 +2636,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);
         }
@@ -3066,6 +3066,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();
 }
@@ -3079,6 +3081,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");
     }
@@ -3127,7 +3131,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..09386458f267 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 on the machine supporting both
+     * mode.
+     */
+    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 b1319905327f..31d195b08952 100644
--- a/hw/ppc/spapr_irq.c
+++ b/hw/ppc/spapr_irq.c
@@ -375,6 +375,149 @@  sPAPRIrq spapr_irq_xive = {
     .reset       = spapr_irq_reset_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 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;
+    }
+
+    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)
+{
+    int ret;
+    Error *local_err = NULL;
+
+    ret = spapr_irq_xive.claim(spapr, irq, lsi, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return ret;
+    }
+
+    ret = spapr_irq_xics.claim(spapr, irq, lsi, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+    }
+
+    return ret;
+}
+
+static void spapr_irq_free_dual(sPAPRMachineState *spapr, int irq, int num)
+{
+    spapr_irq_xive.free(spapr, irq, num);
+    spapr_irq_xics.free(spapr, irq, num);
+}
+
+static qemu_irq spapr_qirq_dual(sPAPRMachineState *spapr, int irq)
+{
+    return spapr_irq_current(spapr)->qirq(spapr, 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 Object *spapr_irq_cpu_intc_create_dual(sPAPRMachineState *spapr,
+                                              Object *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 NULL;
+    }
+
+    /* Default to XICS interrupt mode */
+    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)
+{
+    /*
+     * Reset the interrupt mode selected by CAS.
+     */
+    spapr_irq_current(spapr)->reset(spapr, errp);
+}
+
+/*
+ * 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,
+};
+
 /*
  * sPAPR IRQ frontend routines for devices
  */