| Message ID | 20191002025208.3487-35-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | spapr: IRQ subsystem cleanup | expand |
On Wed, 2 Oct 2019 12:52:08 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > The only thing remaining in this structure are the flags to allow either > XICS or XIVE to be present. These actually make more sense as spapr > capabilities - that way they can take advantage of the existing > infrastructure to sanity check capability states across migration and so > forth. > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > --- This needs some more care. Incoming migration of older existing machine types breaks: qemu-system-ppc64: cap-xics higher level (1) in incoming stream than on destination (0) qemu-system-ppc64: error while loading state for instance 0x0 of device 'spapr' qemu-system-ppc64: load of migration failed: Invalid argument > hw/ppc/spapr.c | 38 +++++++++-------- > hw/ppc/spapr_caps.c | 64 +++++++++++++++++++++++++++++ > hw/ppc/spapr_hcall.c | 7 ++-- > hw/ppc/spapr_irq.c | 84 ++------------------------------------ > include/hw/ppc/spapr.h | 8 ++-- > include/hw/ppc/spapr_irq.h | 10 ----- > 6 files changed, 99 insertions(+), 112 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index e1ff03152e..b9ac01d90c 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1072,12 +1072,13 @@ static void spapr_dt_ov5_platform_support(SpaprMachineState *spapr, void *fdt, > 26, 0x40, /* Radix options: GTSE == yes. */ > }; > > - if (spapr->irq->xics && spapr->irq->xive) { > + if (spapr_get_cap(spapr, SPAPR_CAP_XICS) > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > val[1] = SPAPR_OV5_XIVE_BOTH; > - } else if (spapr->irq->xive) { > + } else if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > val[1] = SPAPR_OV5_XIVE_EXPLOIT; > } else { > - assert(spapr->irq->xics); > + assert(spapr_get_cap(spapr, SPAPR_CAP_XICS)); > val[1] = SPAPR_OV5_XIVE_LEGACY; > } > > @@ -2775,7 +2776,7 @@ static void spapr_machine_init(MachineState *machine) > spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2); > > /* advertise XIVE on POWER9 machines */ > - if (spapr->irq->xive) { > + if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT); > } > > @@ -3242,14 +3243,18 @@ static void spapr_set_vsmt(Object *obj, Visitor *v, const char *name, > static char *spapr_get_ic_mode(Object *obj, Error **errp) > { > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > > - if (spapr->irq == &spapr_irq_xics_legacy) { > + if (smc->legacy_irq_allocation) { > return g_strdup("legacy"); > - } else if (spapr->irq == &spapr_irq_xics) { > + } else if (spapr_get_cap(spapr, SPAPR_CAP_XICS) > + && !spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > return g_strdup("xics"); > - } else if (spapr->irq == &spapr_irq_xive) { > + } else if (!spapr_get_cap(spapr, SPAPR_CAP_XICS) > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > return g_strdup("xive"); > - } else if (spapr->irq == &spapr_irq_dual) { > + } else if (spapr_get_cap(spapr, SPAPR_CAP_XICS) > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > return g_strdup("dual"); > } > g_assert_not_reached(); > @@ -3266,11 +3271,14 @@ static void spapr_set_ic_mode(Object *obj, const char *value, Error **errp) > > /* The legacy IRQ backend can not be set */ > if (strcmp(value, "xics") == 0) { > - spapr->irq = &spapr_irq_xics; > + object_property_set_bool(obj, true, "cap-xics", errp); > + object_property_set_bool(obj, false, "cap-xive", errp); > } else if (strcmp(value, "xive") == 0) { > - spapr->irq = &spapr_irq_xive; > + object_property_set_bool(obj, false, "cap-xics", errp); > + object_property_set_bool(obj, true, "cap-xive", errp); > } else if (strcmp(value, "dual") == 0) { > - spapr->irq = &spapr_irq_dual; > + object_property_set_bool(obj, true, "cap-xics", errp); > + object_property_set_bool(obj, true, "cap-xive", errp); > } else { > error_setg(errp, "Bad value for \"ic-mode\" property"); > } > @@ -3309,7 +3317,6 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > 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; > @@ -3345,7 +3352,6 @@ static void spapr_instance_init(Object *obj) > 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", > @@ -4439,8 +4445,9 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) > smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF; > smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON; > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; > + smc->default_caps.caps[SPAPR_CAP_XICS] = SPAPR_CAP_ON; > + smc->default_caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_ON; > spapr_caps_add_properties(smc, &error_abort); > - smc->irq = &spapr_irq_dual; > smc->dr_phb_enabled = true; > smc->linux_pci_probe = true; > smc->nr_xirqs = SPAPR_NR_XIRQS; > @@ -4539,7 +4546,7 @@ static void spapr_machine_4_0_class_options(MachineClass *mc) > spapr_machine_4_1_class_options(mc); > compat_props_add(mc->compat_props, hw_compat_4_0, hw_compat_4_0_len); > smc->phb_placement = phb_placement_4_0; > - smc->irq = &spapr_irq_xics; > + smc->default_caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_OFF; > smc->pre_4_1_migration = true; > } > > @@ -4580,7 +4587,6 @@ static void spapr_machine_3_0_class_options(MachineClass *mc) > > smc->legacy_irq_allocation = true; > smc->nr_xirqs = 0x400; > - smc->irq = &spapr_irq_xics_legacy; > } > > DEFINE_SPAPR_MACHINE(3_0, "3.0", false); > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > index 481dfd2a27..e06fd386f6 100644 > --- a/hw/ppc/spapr_caps.c > +++ b/hw/ppc/spapr_caps.c > @@ -496,6 +496,42 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val, > } > } > > +static void cap_xics_apply(SpaprMachineState *spapr, uint8_t val, Error **errp) > +{ > + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > + > + if (!val) { > + if (!spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > + error_setg(errp, > +"No interrupt controllers enabled, try cap-xics=on or cap-xive=on"); > + return; > + } > + > + if (smc->legacy_irq_allocation) { > + error_setg(errp, "This machine version requires XICS support"); > + return; > + } > + } > +} > + > +static void cap_xive_apply(SpaprMachineState *spapr, uint8_t val, Error **errp) > +{ > + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > + PowerPCCPU *cpu = POWERPC_CPU(first_cpu); > + > + if (val) { > + if (smc->legacy_irq_allocation) { > + error_setg(errp, "This machine version cannot support XIVE"); > + return; > + } > + if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0, > + spapr->max_compat_pvr)) { > + error_setg(errp, "XIVE requires POWER9 CPU"); > + return; > + } > + } > +} > + > SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = { > [SPAPR_CAP_HTM] = { > .name = "htm", > @@ -595,6 +631,24 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = { > .type = "bool", > .apply = cap_ccf_assist_apply, > }, > + [SPAPR_CAP_XICS] = { > + .name = "xics", > + .description = "Allow XICS interrupt controller", > + .index = SPAPR_CAP_XICS, > + .get = spapr_cap_get_bool, > + .set = spapr_cap_set_bool, > + .type = "bool", > + .apply = cap_xics_apply, > + }, > + [SPAPR_CAP_XIVE] = { > + .name = "xive", > + .description = "Allow XIVE interrupt controller", > + .index = SPAPR_CAP_XIVE, > + .get = spapr_cap_get_bool, > + .set = spapr_cap_set_bool, > + .type = "bool", > + .apply = cap_xive_apply, > + }, > }; > > static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr, > @@ -641,6 +695,14 @@ static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr, > caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = mps; > } > > + /* > + * POWER8 machines don't have XIVE > + */ > + if (!ppc_type_check_compat(cputype, CPU_POWERPC_LOGICAL_3_00, > + 0, spapr->max_compat_pvr)) { > + caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_OFF; > + } > + > return caps; > } > > @@ -734,6 +796,8 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE); > SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); > SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); > SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST); > +SPAPR_CAP_MIG_STATE(xics, SPAPR_CAP_XICS); > +SPAPR_CAP_MIG_STATE(xive, SPAPR_CAP_XIVE); > > void spapr_caps_init(SpaprMachineState *spapr) > { > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 140f05c1c6..cb4c6edf63 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -1784,13 +1784,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > * terminate the boot. > */ > if (guest_xive) { > - if (!spapr->irq->xive) { > + if (!spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > error_report( > "Guest requested unavailable interrupt mode (XIVE), try the ic-mode=xive or ic-mode=dual machine property"); > exit(EXIT_FAILURE); > } > } else { > - if (!spapr->irq->xics) { > + if (!spapr_get_cap(spapr, SPAPR_CAP_XICS)) { > error_report( > "Guest requested unavailable interrupt mode (XICS), either don't set the ic-mode machine property or try ic-mode=xics or ic-mode=dual"); > exit(EXIT_FAILURE); > @@ -1804,7 +1804,8 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > */ > if (!spapr->cas_reboot) { > spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT) > - && spapr->irq->xics && spapr->irq->xive; > + && spapr_get_cap(spapr, SPAPR_CAP_XICS) > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE); > } > > spapr_ovec_cleanup(ov5_updates); > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 2768f9a765..473fc8780a 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -101,90 +101,19 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), > return 0; > } > > -/* > - * XICS IRQ backend. > - */ > - > -SpaprIrq spapr_irq_xics = { > - .xics = true, > - .xive = false, > -}; > - > -/* > - * XIVE IRQ backend. > - */ > - > -SpaprIrq spapr_irq_xive = { > - .xics = false, > - .xive = true, > -}; > - > -/* > - * 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. > - */ > - > -/* > - * Define values in sync with the XIVE and XICS backend > - */ > -SpaprIrq spapr_irq_dual = { > - .xics = true, > - .xive = true, > -}; > - > - > static int spapr_irq_check(SpaprMachineState *spapr, Error **errp) > { > MachineState *machine = MACHINE(spapr); > > - /* > - * Sanity checks on non-P9 machines. On these, XIVE is not > - * advertised, see spapr_dt_ov5_platform_support() > - */ > - if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, > - 0, spapr->max_compat_pvr)) { > - /* > - * If the 'dual' interrupt mode is selected, force XICS as CAS > - * negotiation is useless. > - */ > - if (spapr->irq == &spapr_irq_dual) { > - spapr->irq = &spapr_irq_xics; > - return 0; > - } > - > - /* > - * Non-P9 machines using only XIVE is a bogus setup. We have two > - * scenarios to take into account because of the compat mode: > - * > - * 1. POWER7/8 machines should fail to init later on when creating > - * the XIVE interrupt presenters because a POWER9 exception > - * model is required. > - > - * 2. POWER9 machines using the POWER8 compat mode won't fail and > - * will let the OS boot with a partial XIVE setup : DT > - * properties but no hcalls. > - * > - * To cover both and not confuse the OS, add an early failure in > - * QEMU. > - */ > - if (spapr->irq == &spapr_irq_xive) { > - error_setg(errp, "XIVE-only machines require a POWER9 CPU"); > - return -1; > - } > - } > - > /* > * On a POWER9 host, some older KVM XICS devices cannot be destroyed and > * re-created. Detect that early to avoid QEMU to exit later when the > * guest reboots. > */ > if (kvm_enabled() && > - spapr->irq == &spapr_irq_dual && > machine_kernel_irqchip_required(machine) && > + spapr_get_cap(spapr, SPAPR_CAP_XICS) && > + spapr_get_cap(spapr, SPAPR_CAP_XIVE) && > xics_kvm_has_broken_disconnect(spapr)) { > error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on"); > return -1; > @@ -280,7 +209,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > /* Initialize the MSI IRQ allocator. */ > spapr_irq_msi_init(spapr); > > - if (spapr->irq->xics) { > + if (spapr_get_cap(spapr, SPAPR_CAP_XICS)) { > Error *local_err = NULL; > Object *obj; > > @@ -313,7 +242,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > spapr->ics = ICS_SPAPR(obj); > } > > - if (spapr->irq->xive) { > + if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > uint32_t nr_servers = spapr_max_server_number(spapr); > DeviceState *dev; > int i; > @@ -558,11 +487,6 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) > return first + ics->offset; > } > > -SpaprIrq spapr_irq_xics_legacy = { > - .xics = true, > - .xive = false, > -}; > - > static void spapr_irq_register_types(void) > { > type_register_static(&spapr_intc_info); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 623e8e3f93..bae5d1ccb3 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -79,8 +79,12 @@ typedef enum { > #define SPAPR_CAP_LARGE_DECREMENTER 0x08 > /* Count Cache Flush Assist HW Instruction */ > #define SPAPR_CAP_CCF_ASSIST 0x09 > +/* XICS interrupt controller */ > +#define SPAPR_CAP_XICS 0x0a > +/* XIVE interrupt controller */ > +#define SPAPR_CAP_XIVE 0x0b > /* Num Caps */ > -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1) > +#define SPAPR_CAP_NUM (SPAPR_CAP_XIVE + 1) > > /* > * Capability Values > @@ -131,7 +135,6 @@ struct SpaprMachineClass { > hwaddr *nv2atsd, Error **errp); > SpaprResizeHpt resize_hpt_default; > SpaprCapabilities default_caps; > - SpaprIrq *irq; > }; > > /** > @@ -195,7 +198,6 @@ struct SpaprMachineState { > > int32_t irq_map_nr; > unsigned long *irq_map; > - SpaprIrq *irq; > qemu_irq *qirqs; > SpaprInterruptController *active_intc; > ICSState *ics; > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > index 50491cea4f..4b58134701 100644 > --- a/include/hw/ppc/spapr_irq.h > +++ b/include/hw/ppc/spapr_irq.h > @@ -77,16 +77,6 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, > Error **errp); > void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); > > -typedef struct SpaprIrq { > - bool xics; > - bool xive; > -} 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); > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num);
On Wed, Oct 02, 2019 at 12:20:35PM +0200, Greg Kurz wrote: > On Wed, 2 Oct 2019 12:52:08 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > The only thing remaining in this structure are the flags to allow either > > XICS or XIVE to be present. These actually make more sense as spapr > > capabilities - that way they can take advantage of the existing > > infrastructure to sanity check capability states across migration and so > > forth. > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > This needs some more care. Incoming migration of older existing machine > types breaks: > > qemu-system-ppc64: cap-xics higher level (1) in incoming stream than on destination (0) > qemu-system-ppc64: error while loading state for instance 0x0 of device 'spapr' > qemu-system-ppc64: load of migration failed: Invalid argument Ah, right, thanks for testing that. What machine type exactly was broken? > > > hw/ppc/spapr.c | 38 +++++++++-------- > > hw/ppc/spapr_caps.c | 64 +++++++++++++++++++++++++++++ > > hw/ppc/spapr_hcall.c | 7 ++-- > > hw/ppc/spapr_irq.c | 84 ++------------------------------------ > > include/hw/ppc/spapr.h | 8 ++-- > > include/hw/ppc/spapr_irq.h | 10 ----- > > 6 files changed, 99 insertions(+), 112 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index e1ff03152e..b9ac01d90c 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1072,12 +1072,13 @@ static void spapr_dt_ov5_platform_support(SpaprMachineState *spapr, void *fdt, > > 26, 0x40, /* Radix options: GTSE == yes. */ > > }; > > > > - if (spapr->irq->xics && spapr->irq->xive) { > > + if (spapr_get_cap(spapr, SPAPR_CAP_XICS) > > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > val[1] = SPAPR_OV5_XIVE_BOTH; > > - } else if (spapr->irq->xive) { > > + } else if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > val[1] = SPAPR_OV5_XIVE_EXPLOIT; > > } else { > > - assert(spapr->irq->xics); > > + assert(spapr_get_cap(spapr, SPAPR_CAP_XICS)); > > val[1] = SPAPR_OV5_XIVE_LEGACY; > > } > > > > @@ -2775,7 +2776,7 @@ static void spapr_machine_init(MachineState *machine) > > spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2); > > > > /* advertise XIVE on POWER9 machines */ > > - if (spapr->irq->xive) { > > + if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT); > > } > > > > @@ -3242,14 +3243,18 @@ static void spapr_set_vsmt(Object *obj, Visitor *v, const char *name, > > static char *spapr_get_ic_mode(Object *obj, Error **errp) > > { > > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > > > > - if (spapr->irq == &spapr_irq_xics_legacy) { > > + if (smc->legacy_irq_allocation) { > > return g_strdup("legacy"); > > - } else if (spapr->irq == &spapr_irq_xics) { > > + } else if (spapr_get_cap(spapr, SPAPR_CAP_XICS) > > + && !spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > return g_strdup("xics"); > > - } else if (spapr->irq == &spapr_irq_xive) { > > + } else if (!spapr_get_cap(spapr, SPAPR_CAP_XICS) > > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > return g_strdup("xive"); > > - } else if (spapr->irq == &spapr_irq_dual) { > > + } else if (spapr_get_cap(spapr, SPAPR_CAP_XICS) > > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > return g_strdup("dual"); > > } > > g_assert_not_reached(); > > @@ -3266,11 +3271,14 @@ static void spapr_set_ic_mode(Object *obj, const char *value, Error **errp) > > > > /* The legacy IRQ backend can not be set */ > > if (strcmp(value, "xics") == 0) { > > - spapr->irq = &spapr_irq_xics; > > + object_property_set_bool(obj, true, "cap-xics", errp); > > + object_property_set_bool(obj, false, "cap-xive", errp); > > } else if (strcmp(value, "xive") == 0) { > > - spapr->irq = &spapr_irq_xive; > > + object_property_set_bool(obj, false, "cap-xics", errp); > > + object_property_set_bool(obj, true, "cap-xive", errp); > > } else if (strcmp(value, "dual") == 0) { > > - spapr->irq = &spapr_irq_dual; > > + object_property_set_bool(obj, true, "cap-xics", errp); > > + object_property_set_bool(obj, true, "cap-xive", errp); > > } else { > > error_setg(errp, "Bad value for \"ic-mode\" property"); > > } > > @@ -3309,7 +3317,6 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > > 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; > > @@ -3345,7 +3352,6 @@ static void spapr_instance_init(Object *obj) > > 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", > > @@ -4439,8 +4445,9 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) > > smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF; > > smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON; > > smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; > > + smc->default_caps.caps[SPAPR_CAP_XICS] = SPAPR_CAP_ON; > > + smc->default_caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_ON; > > spapr_caps_add_properties(smc, &error_abort); > > - smc->irq = &spapr_irq_dual; > > smc->dr_phb_enabled = true; > > smc->linux_pci_probe = true; > > smc->nr_xirqs = SPAPR_NR_XIRQS; > > @@ -4539,7 +4546,7 @@ static void spapr_machine_4_0_class_options(MachineClass *mc) > > spapr_machine_4_1_class_options(mc); > > compat_props_add(mc->compat_props, hw_compat_4_0, hw_compat_4_0_len); > > smc->phb_placement = phb_placement_4_0; > > - smc->irq = &spapr_irq_xics; > > + smc->default_caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_OFF; > > smc->pre_4_1_migration = true; > > } > > > > @@ -4580,7 +4587,6 @@ static void spapr_machine_3_0_class_options(MachineClass *mc) > > > > smc->legacy_irq_allocation = true; > > smc->nr_xirqs = 0x400; > > - smc->irq = &spapr_irq_xics_legacy; > > } > > > > DEFINE_SPAPR_MACHINE(3_0, "3.0", false); > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > > index 481dfd2a27..e06fd386f6 100644 > > --- a/hw/ppc/spapr_caps.c > > +++ b/hw/ppc/spapr_caps.c > > @@ -496,6 +496,42 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val, > > } > > } > > > > +static void cap_xics_apply(SpaprMachineState *spapr, uint8_t val, Error **errp) > > +{ > > + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > > + > > + if (!val) { > > + if (!spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > + error_setg(errp, > > +"No interrupt controllers enabled, try cap-xics=on or cap-xive=on"); > > + return; > > + } > > + > > + if (smc->legacy_irq_allocation) { > > + error_setg(errp, "This machine version requires XICS support"); > > + return; > > + } > > + } > > +} > > + > > +static void cap_xive_apply(SpaprMachineState *spapr, uint8_t val, Error **errp) > > +{ > > + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > > + PowerPCCPU *cpu = POWERPC_CPU(first_cpu); > > + > > + if (val) { > > + if (smc->legacy_irq_allocation) { > > + error_setg(errp, "This machine version cannot support XIVE"); > > + return; > > + } > > + if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0, > > + spapr->max_compat_pvr)) { > > + error_setg(errp, "XIVE requires POWER9 CPU"); > > + return; > > + } > > + } > > +} > > + > > SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = { > > [SPAPR_CAP_HTM] = { > > .name = "htm", > > @@ -595,6 +631,24 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = { > > .type = "bool", > > .apply = cap_ccf_assist_apply, > > }, > > + [SPAPR_CAP_XICS] = { > > + .name = "xics", > > + .description = "Allow XICS interrupt controller", > > + .index = SPAPR_CAP_XICS, > > + .get = spapr_cap_get_bool, > > + .set = spapr_cap_set_bool, > > + .type = "bool", > > + .apply = cap_xics_apply, > > + }, > > + [SPAPR_CAP_XIVE] = { > > + .name = "xive", > > + .description = "Allow XIVE interrupt controller", > > + .index = SPAPR_CAP_XIVE, > > + .get = spapr_cap_get_bool, > > + .set = spapr_cap_set_bool, > > + .type = "bool", > > + .apply = cap_xive_apply, > > + }, > > }; > > > > static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr, > > @@ -641,6 +695,14 @@ static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr, > > caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = mps; > > } > > > > + /* > > + * POWER8 machines don't have XIVE > > + */ > > + if (!ppc_type_check_compat(cputype, CPU_POWERPC_LOGICAL_3_00, > > + 0, spapr->max_compat_pvr)) { > > + caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_OFF; > > + } > > + > > return caps; > > } > > > > @@ -734,6 +796,8 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE); > > SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); > > SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); > > SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST); > > +SPAPR_CAP_MIG_STATE(xics, SPAPR_CAP_XICS); > > +SPAPR_CAP_MIG_STATE(xive, SPAPR_CAP_XIVE); > > > > void spapr_caps_init(SpaprMachineState *spapr) > > { > > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > > index 140f05c1c6..cb4c6edf63 100644 > > --- a/hw/ppc/spapr_hcall.c > > +++ b/hw/ppc/spapr_hcall.c > > @@ -1784,13 +1784,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > > * terminate the boot. > > */ > > if (guest_xive) { > > - if (!spapr->irq->xive) { > > + if (!spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > error_report( > > "Guest requested unavailable interrupt mode (XIVE), try the ic-mode=xive or ic-mode=dual machine property"); > > exit(EXIT_FAILURE); > > } > > } else { > > - if (!spapr->irq->xics) { > > + if (!spapr_get_cap(spapr, SPAPR_CAP_XICS)) { > > error_report( > > "Guest requested unavailable interrupt mode (XICS), either don't set the ic-mode machine property or try ic-mode=xics or ic-mode=dual"); > > exit(EXIT_FAILURE); > > @@ -1804,7 +1804,8 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, > > */ > > if (!spapr->cas_reboot) { > > spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT) > > - && spapr->irq->xics && spapr->irq->xive; > > + && spapr_get_cap(spapr, SPAPR_CAP_XICS) > > + && spapr_get_cap(spapr, SPAPR_CAP_XIVE); > > } > > > > spapr_ovec_cleanup(ov5_updates); > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > > index 2768f9a765..473fc8780a 100644 > > --- a/hw/ppc/spapr_irq.c > > +++ b/hw/ppc/spapr_irq.c > > @@ -101,90 +101,19 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), > > return 0; > > } > > > > -/* > > - * XICS IRQ backend. > > - */ > > - > > -SpaprIrq spapr_irq_xics = { > > - .xics = true, > > - .xive = false, > > -}; > > - > > -/* > > - * XIVE IRQ backend. > > - */ > > - > > -SpaprIrq spapr_irq_xive = { > > - .xics = false, > > - .xive = true, > > -}; > > - > > -/* > > - * 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. > > - */ > > - > > -/* > > - * Define values in sync with the XIVE and XICS backend > > - */ > > -SpaprIrq spapr_irq_dual = { > > - .xics = true, > > - .xive = true, > > -}; > > - > > - > > static int spapr_irq_check(SpaprMachineState *spapr, Error **errp) > > { > > MachineState *machine = MACHINE(spapr); > > > > - /* > > - * Sanity checks on non-P9 machines. On these, XIVE is not > > - * advertised, see spapr_dt_ov5_platform_support() > > - */ > > - if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, > > - 0, spapr->max_compat_pvr)) { > > - /* > > - * If the 'dual' interrupt mode is selected, force XICS as CAS > > - * negotiation is useless. > > - */ > > - if (spapr->irq == &spapr_irq_dual) { > > - spapr->irq = &spapr_irq_xics; > > - return 0; > > - } > > - > > - /* > > - * Non-P9 machines using only XIVE is a bogus setup. We have two > > - * scenarios to take into account because of the compat mode: > > - * > > - * 1. POWER7/8 machines should fail to init later on when creating > > - * the XIVE interrupt presenters because a POWER9 exception > > - * model is required. > > - > > - * 2. POWER9 machines using the POWER8 compat mode won't fail and > > - * will let the OS boot with a partial XIVE setup : DT > > - * properties but no hcalls. > > - * > > - * To cover both and not confuse the OS, add an early failure in > > - * QEMU. > > - */ > > - if (spapr->irq == &spapr_irq_xive) { > > - error_setg(errp, "XIVE-only machines require a POWER9 CPU"); > > - return -1; > > - } > > - } > > - > > /* > > * On a POWER9 host, some older KVM XICS devices cannot be destroyed and > > * re-created. Detect that early to avoid QEMU to exit later when the > > * guest reboots. > > */ > > if (kvm_enabled() && > > - spapr->irq == &spapr_irq_dual && > > machine_kernel_irqchip_required(machine) && > > + spapr_get_cap(spapr, SPAPR_CAP_XICS) && > > + spapr_get_cap(spapr, SPAPR_CAP_XIVE) && > > xics_kvm_has_broken_disconnect(spapr)) { > > error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on"); > > return -1; > > @@ -280,7 +209,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > > /* Initialize the MSI IRQ allocator. */ > > spapr_irq_msi_init(spapr); > > > > - if (spapr->irq->xics) { > > + if (spapr_get_cap(spapr, SPAPR_CAP_XICS)) { > > Error *local_err = NULL; > > Object *obj; > > > > @@ -313,7 +242,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > > spapr->ics = ICS_SPAPR(obj); > > } > > > > - if (spapr->irq->xive) { > > + if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { > > uint32_t nr_servers = spapr_max_server_number(spapr); > > DeviceState *dev; > > int i; > > @@ -558,11 +487,6 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) > > return first + ics->offset; > > } > > > > -SpaprIrq spapr_irq_xics_legacy = { > > - .xics = true, > > - .xive = false, > > -}; > > - > > static void spapr_irq_register_types(void) > > { > > type_register_static(&spapr_intc_info); > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > index 623e8e3f93..bae5d1ccb3 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -79,8 +79,12 @@ typedef enum { > > #define SPAPR_CAP_LARGE_DECREMENTER 0x08 > > /* Count Cache Flush Assist HW Instruction */ > > #define SPAPR_CAP_CCF_ASSIST 0x09 > > +/* XICS interrupt controller */ > > +#define SPAPR_CAP_XICS 0x0a > > +/* XIVE interrupt controller */ > > +#define SPAPR_CAP_XIVE 0x0b > > /* Num Caps */ > > -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1) > > +#define SPAPR_CAP_NUM (SPAPR_CAP_XIVE + 1) > > > > /* > > * Capability Values > > @@ -131,7 +135,6 @@ struct SpaprMachineClass { > > hwaddr *nv2atsd, Error **errp); > > SpaprResizeHpt resize_hpt_default; > > SpaprCapabilities default_caps; > > - SpaprIrq *irq; > > }; > > > > /** > > @@ -195,7 +198,6 @@ struct SpaprMachineState { > > > > int32_t irq_map_nr; > > unsigned long *irq_map; > > - SpaprIrq *irq; > > qemu_irq *qirqs; > > SpaprInterruptController *active_intc; > > ICSState *ics; > > diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h > > index 50491cea4f..4b58134701 100644 > > --- a/include/hw/ppc/spapr_irq.h > > +++ b/include/hw/ppc/spapr_irq.h > > @@ -77,16 +77,6 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, > > Error **errp); > > void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); > > > > -typedef struct SpaprIrq { > > - bool xics; > > - bool xive; > > -} 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); > > void spapr_irq_free(SpaprMachineState *spapr, int irq, int num); >
On Thu, Oct 03, 2019 at 08:31:11AM +1000, David Gibson wrote: > On Wed, Oct 02, 2019 at 12:20:35PM +0200, Greg Kurz wrote: > > On Wed, 2 Oct 2019 12:52:08 +1000 > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > The only thing remaining in this structure are the flags to allow either > > > XICS or XIVE to be present. These actually make more sense as spapr > > > capabilities - that way they can take advantage of the existing > > > infrastructure to sanity check capability states across migration and so > > > forth. > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > --- > > > > This needs some more care. Incoming migration of older existing machine > > types breaks: > > > > qemu-system-ppc64: cap-xics higher level (1) in incoming stream than on destination (0) > > qemu-system-ppc64: error while loading state for instance 0x0 of device 'spapr' > > qemu-system-ppc64: load of migration failed: Invalid argument > > Ah, right, thanks for testing that. What machine type exactly was > broken? Never mind, found it. And in fact it was broken with the lastest machine type and ic-mode=xics as well. Turns out I wrote the vmstatedescription for the new caps, but didn't wire them up. It's a real pain the number of places that need to be adjusted to get a new cap properly working, unfortunately, I've not come up with a good way to avoid it.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index e1ff03152e..b9ac01d90c 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1072,12 +1072,13 @@ static void spapr_dt_ov5_platform_support(SpaprMachineState *spapr, void *fdt, 26, 0x40, /* Radix options: GTSE == yes. */ }; - if (spapr->irq->xics && spapr->irq->xive) { + if (spapr_get_cap(spapr, SPAPR_CAP_XICS) + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { val[1] = SPAPR_OV5_XIVE_BOTH; - } else if (spapr->irq->xive) { + } else if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { val[1] = SPAPR_OV5_XIVE_EXPLOIT; } else { - assert(spapr->irq->xics); + assert(spapr_get_cap(spapr, SPAPR_CAP_XICS)); val[1] = SPAPR_OV5_XIVE_LEGACY; } @@ -2775,7 +2776,7 @@ static void spapr_machine_init(MachineState *machine) spapr_ovec_set(spapr->ov5, OV5_DRMEM_V2); /* advertise XIVE on POWER9 machines */ - if (spapr->irq->xive) { + if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { spapr_ovec_set(spapr->ov5, OV5_XIVE_EXPLOIT); } @@ -3242,14 +3243,18 @@ static void spapr_set_vsmt(Object *obj, Visitor *v, const char *name, static char *spapr_get_ic_mode(Object *obj, Error **errp) { SpaprMachineState *spapr = SPAPR_MACHINE(obj); + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); - if (spapr->irq == &spapr_irq_xics_legacy) { + if (smc->legacy_irq_allocation) { return g_strdup("legacy"); - } else if (spapr->irq == &spapr_irq_xics) { + } else if (spapr_get_cap(spapr, SPAPR_CAP_XICS) + && !spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { return g_strdup("xics"); - } else if (spapr->irq == &spapr_irq_xive) { + } else if (!spapr_get_cap(spapr, SPAPR_CAP_XICS) + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { return g_strdup("xive"); - } else if (spapr->irq == &spapr_irq_dual) { + } else if (spapr_get_cap(spapr, SPAPR_CAP_XICS) + && spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { return g_strdup("dual"); } g_assert_not_reached(); @@ -3266,11 +3271,14 @@ static void spapr_set_ic_mode(Object *obj, const char *value, Error **errp) /* The legacy IRQ backend can not be set */ if (strcmp(value, "xics") == 0) { - spapr->irq = &spapr_irq_xics; + object_property_set_bool(obj, true, "cap-xics", errp); + object_property_set_bool(obj, false, "cap-xive", errp); } else if (strcmp(value, "xive") == 0) { - spapr->irq = &spapr_irq_xive; + object_property_set_bool(obj, false, "cap-xics", errp); + object_property_set_bool(obj, true, "cap-xive", errp); } else if (strcmp(value, "dual") == 0) { - spapr->irq = &spapr_irq_dual; + object_property_set_bool(obj, true, "cap-xics", errp); + object_property_set_bool(obj, true, "cap-xive", errp); } else { error_setg(errp, "Bad value for \"ic-mode\" property"); } @@ -3309,7 +3317,6 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) 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; @@ -3345,7 +3352,6 @@ static void spapr_instance_init(Object *obj) 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", @@ -4439,8 +4445,9 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data) smc->default_caps.caps[SPAPR_CAP_NESTED_KVM_HV] = SPAPR_CAP_OFF; smc->default_caps.caps[SPAPR_CAP_LARGE_DECREMENTER] = SPAPR_CAP_ON; smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_OFF; + smc->default_caps.caps[SPAPR_CAP_XICS] = SPAPR_CAP_ON; + smc->default_caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_ON; spapr_caps_add_properties(smc, &error_abort); - smc->irq = &spapr_irq_dual; smc->dr_phb_enabled = true; smc->linux_pci_probe = true; smc->nr_xirqs = SPAPR_NR_XIRQS; @@ -4539,7 +4546,7 @@ static void spapr_machine_4_0_class_options(MachineClass *mc) spapr_machine_4_1_class_options(mc); compat_props_add(mc->compat_props, hw_compat_4_0, hw_compat_4_0_len); smc->phb_placement = phb_placement_4_0; - smc->irq = &spapr_irq_xics; + smc->default_caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_OFF; smc->pre_4_1_migration = true; } @@ -4580,7 +4587,6 @@ static void spapr_machine_3_0_class_options(MachineClass *mc) smc->legacy_irq_allocation = true; smc->nr_xirqs = 0x400; - smc->irq = &spapr_irq_xics_legacy; } DEFINE_SPAPR_MACHINE(3_0, "3.0", false); diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c index 481dfd2a27..e06fd386f6 100644 --- a/hw/ppc/spapr_caps.c +++ b/hw/ppc/spapr_caps.c @@ -496,6 +496,42 @@ static void cap_ccf_assist_apply(SpaprMachineState *spapr, uint8_t val, } } +static void cap_xics_apply(SpaprMachineState *spapr, uint8_t val, Error **errp) +{ + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); + + if (!val) { + if (!spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { + error_setg(errp, +"No interrupt controllers enabled, try cap-xics=on or cap-xive=on"); + return; + } + + if (smc->legacy_irq_allocation) { + error_setg(errp, "This machine version requires XICS support"); + return; + } + } +} + +static void cap_xive_apply(SpaprMachineState *spapr, uint8_t val, Error **errp) +{ + SpaprMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); + PowerPCCPU *cpu = POWERPC_CPU(first_cpu); + + if (val) { + if (smc->legacy_irq_allocation) { + error_setg(errp, "This machine version cannot support XIVE"); + return; + } + if (!ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_00, 0, + spapr->max_compat_pvr)) { + error_setg(errp, "XIVE requires POWER9 CPU"); + return; + } + } +} + SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = { [SPAPR_CAP_HTM] = { .name = "htm", @@ -595,6 +631,24 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = { .type = "bool", .apply = cap_ccf_assist_apply, }, + [SPAPR_CAP_XICS] = { + .name = "xics", + .description = "Allow XICS interrupt controller", + .index = SPAPR_CAP_XICS, + .get = spapr_cap_get_bool, + .set = spapr_cap_set_bool, + .type = "bool", + .apply = cap_xics_apply, + }, + [SPAPR_CAP_XIVE] = { + .name = "xive", + .description = "Allow XIVE interrupt controller", + .index = SPAPR_CAP_XIVE, + .get = spapr_cap_get_bool, + .set = spapr_cap_set_bool, + .type = "bool", + .apply = cap_xive_apply, + }, }; static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr, @@ -641,6 +695,14 @@ static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr, caps.caps[SPAPR_CAP_HPT_MAXPAGESIZE] = mps; } + /* + * POWER8 machines don't have XIVE + */ + if (!ppc_type_check_compat(cputype, CPU_POWERPC_LOGICAL_3_00, + 0, spapr->max_compat_pvr)) { + caps.caps[SPAPR_CAP_XIVE] = SPAPR_CAP_OFF; + } + return caps; } @@ -734,6 +796,8 @@ SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE); SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST); +SPAPR_CAP_MIG_STATE(xics, SPAPR_CAP_XICS); +SPAPR_CAP_MIG_STATE(xive, SPAPR_CAP_XIVE); void spapr_caps_init(SpaprMachineState *spapr) { diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 140f05c1c6..cb4c6edf63 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -1784,13 +1784,13 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, * terminate the boot. */ if (guest_xive) { - if (!spapr->irq->xive) { + if (!spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { error_report( "Guest requested unavailable interrupt mode (XIVE), try the ic-mode=xive or ic-mode=dual machine property"); exit(EXIT_FAILURE); } } else { - if (!spapr->irq->xics) { + if (!spapr_get_cap(spapr, SPAPR_CAP_XICS)) { error_report( "Guest requested unavailable interrupt mode (XICS), either don't set the ic-mode machine property or try ic-mode=xics or ic-mode=dual"); exit(EXIT_FAILURE); @@ -1804,7 +1804,8 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu, */ if (!spapr->cas_reboot) { spapr->cas_reboot = spapr_ovec_test(ov5_updates, OV5_XIVE_EXPLOIT) - && spapr->irq->xics && spapr->irq->xive; + && spapr_get_cap(spapr, SPAPR_CAP_XICS) + && spapr_get_cap(spapr, SPAPR_CAP_XIVE); } spapr_ovec_cleanup(ov5_updates); diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 2768f9a765..473fc8780a 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -101,90 +101,19 @@ int spapr_irq_init_kvm(int (*fn)(SpaprInterruptController *, Error **), return 0; } -/* - * XICS IRQ backend. - */ - -SpaprIrq spapr_irq_xics = { - .xics = true, - .xive = false, -}; - -/* - * XIVE IRQ backend. - */ - -SpaprIrq spapr_irq_xive = { - .xics = false, - .xive = true, -}; - -/* - * 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. - */ - -/* - * Define values in sync with the XIVE and XICS backend - */ -SpaprIrq spapr_irq_dual = { - .xics = true, - .xive = true, -}; - - static int spapr_irq_check(SpaprMachineState *spapr, Error **errp) { MachineState *machine = MACHINE(spapr); - /* - * Sanity checks on non-P9 machines. On these, XIVE is not - * advertised, see spapr_dt_ov5_platform_support() - */ - if (!ppc_type_check_compat(machine->cpu_type, CPU_POWERPC_LOGICAL_3_00, - 0, spapr->max_compat_pvr)) { - /* - * If the 'dual' interrupt mode is selected, force XICS as CAS - * negotiation is useless. - */ - if (spapr->irq == &spapr_irq_dual) { - spapr->irq = &spapr_irq_xics; - return 0; - } - - /* - * Non-P9 machines using only XIVE is a bogus setup. We have two - * scenarios to take into account because of the compat mode: - * - * 1. POWER7/8 machines should fail to init later on when creating - * the XIVE interrupt presenters because a POWER9 exception - * model is required. - - * 2. POWER9 machines using the POWER8 compat mode won't fail and - * will let the OS boot with a partial XIVE setup : DT - * properties but no hcalls. - * - * To cover both and not confuse the OS, add an early failure in - * QEMU. - */ - if (spapr->irq == &spapr_irq_xive) { - error_setg(errp, "XIVE-only machines require a POWER9 CPU"); - return -1; - } - } - /* * On a POWER9 host, some older KVM XICS devices cannot be destroyed and * re-created. Detect that early to avoid QEMU to exit later when the * guest reboots. */ if (kvm_enabled() && - spapr->irq == &spapr_irq_dual && machine_kernel_irqchip_required(machine) && + spapr_get_cap(spapr, SPAPR_CAP_XICS) && + spapr_get_cap(spapr, SPAPR_CAP_XIVE) && xics_kvm_has_broken_disconnect(spapr)) { error_setg(errp, "KVM is too old to support ic-mode=dual,kernel-irqchip=on"); return -1; @@ -280,7 +209,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) /* Initialize the MSI IRQ allocator. */ spapr_irq_msi_init(spapr); - if (spapr->irq->xics) { + if (spapr_get_cap(spapr, SPAPR_CAP_XICS)) { Error *local_err = NULL; Object *obj; @@ -313,7 +242,7 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) spapr->ics = ICS_SPAPR(obj); } - if (spapr->irq->xive) { + if (spapr_get_cap(spapr, SPAPR_CAP_XIVE)) { uint32_t nr_servers = spapr_max_server_number(spapr); DeviceState *dev; int i; @@ -558,11 +487,6 @@ int spapr_irq_find(SpaprMachineState *spapr, int num, bool align, Error **errp) return first + ics->offset; } -SpaprIrq spapr_irq_xics_legacy = { - .xics = true, - .xive = false, -}; - static void spapr_irq_register_types(void) { type_register_static(&spapr_intc_info); diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 623e8e3f93..bae5d1ccb3 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -79,8 +79,12 @@ typedef enum { #define SPAPR_CAP_LARGE_DECREMENTER 0x08 /* Count Cache Flush Assist HW Instruction */ #define SPAPR_CAP_CCF_ASSIST 0x09 +/* XICS interrupt controller */ +#define SPAPR_CAP_XICS 0x0a +/* XIVE interrupt controller */ +#define SPAPR_CAP_XIVE 0x0b /* Num Caps */ -#define SPAPR_CAP_NUM (SPAPR_CAP_CCF_ASSIST + 1) +#define SPAPR_CAP_NUM (SPAPR_CAP_XIVE + 1) /* * Capability Values @@ -131,7 +135,6 @@ struct SpaprMachineClass { hwaddr *nv2atsd, Error **errp); SpaprResizeHpt resize_hpt_default; SpaprCapabilities default_caps; - SpaprIrq *irq; }; /** @@ -195,7 +198,6 @@ struct SpaprMachineState { int32_t irq_map_nr; unsigned long *irq_map; - SpaprIrq *irq; qemu_irq *qirqs; SpaprInterruptController *active_intc; ICSState *ics; diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h index 50491cea4f..4b58134701 100644 --- a/include/hw/ppc/spapr_irq.h +++ b/include/hw/ppc/spapr_irq.h @@ -77,16 +77,6 @@ int spapr_irq_msi_alloc(SpaprMachineState *spapr, uint32_t num, bool align, Error **errp); void spapr_irq_msi_free(SpaprMachineState *spapr, int irq, uint32_t num); -typedef struct SpaprIrq { - bool xics; - bool xive; -} 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); void spapr_irq_free(SpaprMachineState *spapr, int irq, int num);
The only thing remaining in this structure are the flags to allow either XICS or XIVE to be present. These actually make more sense as spapr capabilities - that way they can take advantage of the existing infrastructure to sanity check capability states across migration and so forth. Signed-off-by: David Gibson <david@gibson.dropbear.id.au> --- hw/ppc/spapr.c | 38 +++++++++-------- hw/ppc/spapr_caps.c | 64 +++++++++++++++++++++++++++++ hw/ppc/spapr_hcall.c | 7 ++-- hw/ppc/spapr_irq.c | 84 ++------------------------------------ include/hw/ppc/spapr.h | 8 ++-- include/hw/ppc/spapr_irq.h | 10 ----- 6 files changed, 99 insertions(+), 112 deletions(-)