| Message ID | 1487252865-12064-4-git-send-email-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 16, 2017 at 02:47:26PM +0100, Cédric Le Goater wrote: > A list of ICS objects was introduced under the XICS object for the > PowerNV machine, but, for the sPAPR machine, it brings extra complexity > as there is only a single ICS. To simplify the code, let's add the ICS > pointer under the sPAPR machine and try to reduce the use of this list > where possible. > > Also, change the xics_spapr_*() routines to use an ICS object instead > of an XICSState and change their name to reflect that these are > specific to the sPAPR ICS object. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> Looks good apart from a minor point noted below. > --- > hw/intc/xics_spapr.c | 22 +++++++++------------- > hw/ppc/spapr.c | 29 ++++++++++++++++------------- > hw/ppc/spapr_events.c | 4 ++-- > hw/ppc/spapr_pci.c | 8 ++++---- > hw/ppc/spapr_vio.c | 2 +- > include/hw/ppc/spapr.h | 1 + > include/hw/ppc/xics.h | 6 +++--- > 7 files changed, 36 insertions(+), 36 deletions(-) > > diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c > index 859b5675e175..1501e796e5e0 100644 > --- a/hw/intc/xics_spapr.c > +++ b/hw/intc/xics_spapr.c > @@ -118,7 +118,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); > + ICSState *ics = spapr->ics; > uint32_t nr, srcno, server, priority; > > if ((nargs != 3) || (nret != 1)) { > @@ -151,7 +151,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); > + ICSState *ics = spapr->ics; > uint32_t nr, srcno; > > if ((nargs != 1) || (nret != 3)) { > @@ -181,7 +181,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); > + ICSState *ics = spapr->ics; > uint32_t nr, srcno; > > if ((nargs != 1) || (nret != 1)) { > @@ -212,7 +212,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr, > uint32_t nargs, target_ulong args, > uint32_t nret, target_ulong rets) > { > - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); > + ICSState *ics = spapr->ics; > uint32_t nr, srcno; > > if ((nargs != 1) || (nret != 1)) { > @@ -294,9 +294,8 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) > return -1; > } > > -int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp) > +int spapr_ics_alloc(ICSState *ics, int irq_hint, bool lsi, Error **errp) > { > - ICSState *ics = QLIST_FIRST(&xics->ics); > int irq; > > if (!ics) { > @@ -327,10 +326,9 @@ int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp) > * Allocate block of consecutive IRQs, and return the number of the first IRQ in > * the block. If align==true, aligns the first IRQ number to num. > */ > -int xics_spapr_alloc_block(XICSState *xics, int num, bool lsi, bool align, > - Error **errp) > +int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi, > + bool align, Error **errp) > { > - ICSState *ics = QLIST_FIRST(&xics->ics); > int i, first = -1; > > if (!ics) { > @@ -380,11 +378,9 @@ static void ics_free(ICSState *ics, int srcno, int num) > } > } > > -void xics_spapr_free(XICSState *xics, int irq, int num) > +void spapr_ics_free(ICSState *ics, int irq, int num) > { > - ICSState *ics = xics_find_source(xics, irq); > - > - if (ics) { > + if (ics_valid_irq(ics, irq)) { > trace_xics_ics_free(0, irq, num); > ics_free(ics, irq - ics->offset, num); > } > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 5d7c35de8cd9..045f2323a4e9 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -95,13 +95,13 @@ > > #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) > > -static XICSState *try_create_xics(const char *type, const char *type_ics, > +static XICSState *try_create_xics(sPAPRMachineState *spapr, > + const char *type, const char *type_ics, > const char *type_icp, int nr_servers, > int nr_irqs, Error **errp) > { > Error *err = NULL, *local_err = NULL; > XICSState *xics; > - ICSState *ics = NULL; I'd prefer to keep this local and just update spapr->ics at the end of the function. It won't really make a difference, since if the xics doesn't initialize nothing will work, but currently if this fails it could leave spapr->ics with a bogus but non-NULL pointer, which isn't good practice. > int i; > > xics = XICS_COMMON(object_new(type)); > @@ -111,16 +111,17 @@ static XICSState *try_create_xics(const char *type, const char *type_ics, > goto error; > } > > - ics = ICS_SIMPLE(object_new(type_ics)); > - object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL); > - object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err); > - object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xics), NULL); > - object_property_set_bool(OBJECT(ics), true, "realized", &local_err); > + spapr->ics = ICS_SIMPLE(object_new(type_ics)); > + object_property_add_child(OBJECT(spapr), "ics", OBJECT(spapr->ics), NULL); > + object_property_set_int(OBJECT(spapr->ics), nr_irqs, "nr-irqs", &err); > + object_property_add_const_link(OBJECT(spapr->ics), "xics", OBJECT(xics), > + NULL); > + object_property_set_bool(OBJECT(spapr->ics), true, "realized", &local_err); > error_propagate(&err, local_err); > if (err) { > goto error; > } > - QLIST_INSERT_HEAD(&xics->ics, ics, list); > + QLIST_INSERT_HEAD(&xics->ics, spapr->ics, list); > > xics->ss = g_malloc0(nr_servers * sizeof(ICPState)); > xics->nr_servers = nr_servers; > @@ -142,8 +143,8 @@ static XICSState *try_create_xics(const char *type, const char *type_ics, > > error: > error_propagate(errp, err); > - if (ics) { > - object_unparent(OBJECT(ics)); > + if (spapr->ics) { > + object_unparent(OBJECT(spapr->ics)); > } > object_unparent(OBJECT(xics)); > return NULL; > @@ -158,7 +159,8 @@ static XICSState *xics_system_init(MachineState *machine, > Error *err = NULL; > > if (machine_kernel_irqchip_allowed(machine)) { > - xics = try_create_xics(TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, > + xics = try_create_xics(SPAPR_MACHINE(machine), > + TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, > TYPE_KVM_ICP, nr_servers, nr_irqs, &err); > } > if (machine_kernel_irqchip_required(machine) && !xics) { > @@ -170,8 +172,9 @@ static XICSState *xics_system_init(MachineState *machine, > } > > if (!xics) { > - xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, TYPE_ICP, > - nr_servers, nr_irqs, errp); > + xics = try_create_xics(SPAPR_MACHINE(machine), > + TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, > + TYPE_ICP, nr_servers, nr_irqs, errp); > } > > return xics; > diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c > index f85a9c32a7fc..38b4258a9be7 100644 > --- a/hw/ppc/spapr_events.c > +++ b/hw/ppc/spapr_events.c > @@ -752,7 +752,7 @@ void spapr_events_init(sPAPRMachineState *spapr) > spapr->event_sources = spapr_event_sources_new(); > > spapr_event_sources_register(spapr->event_sources, EVENT_CLASS_EPOW, > - xics_spapr_alloc(spapr->xics, 0, false, > + spapr_ics_alloc(spapr->ics, 0, false, > &error_fatal)); > > /* NOTE: if machine supports modern/dedicated hotplug event source, > @@ -765,7 +765,7 @@ void spapr_events_init(sPAPRMachineState *spapr) > */ > if (spapr->use_hotplug_event_source) { > spapr_event_sources_register(spapr->event_sources, EVENT_CLASS_HOT_PLUG, > - xics_spapr_alloc(spapr->xics, 0, false, > + spapr_ics_alloc(spapr->ics, 0, false, > &error_fatal)); > } > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index fd6fc1d95344..01d5c92425ed 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -325,7 +325,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, > return; > } > > - xics_spapr_free(spapr->xics, msi->first_irq, msi->num); > + spapr_ics_free(spapr->ics, msi->first_irq, msi->num); > if (msi_present(pdev)) { > spapr_msi_setmsg(pdev, 0, false, 0, 0); > } > @@ -363,7 +363,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, > } > > /* Allocate MSIs */ > - irq = xics_spapr_alloc_block(spapr->xics, req_num, false, > + irq = spapr_ics_alloc_block(spapr->ics, req_num, false, > ret_intr_type == RTAS_TYPE_MSI, &err); > if (err) { > error_reportf_err(err, "Can't allocate MSIs for device %x: ", > @@ -374,7 +374,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, > > /* Release previous MSIs */ > if (msi) { > - xics_spapr_free(spapr->xics, msi->first_irq, msi->num); > + spapr_ics_free(spapr->ics, msi->first_irq, msi->num); > g_hash_table_remove(phb->msi, &config_addr); > } > > @@ -1485,7 +1485,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > uint32_t irq; > Error *local_err = NULL; > > - irq = xics_spapr_alloc_block(spapr->xics, 1, true, false, &local_err); > + irq = spapr_ics_alloc_block(spapr->ics, 1, true, false, &local_err); > if (local_err) { > error_propagate(errp, local_err); > error_prepend(errp, "can't allocate LSIs: "); > diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c > index 8bfc5f971f8e..a0ee4fd26586 100644 > --- a/hw/ppc/spapr_vio.c > +++ b/hw/ppc/spapr_vio.c > @@ -454,7 +454,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp) > dev->qdev.id = id; > } > > - dev->irq = xics_spapr_alloc(spapr->xics, dev->irq, false, &local_err); > + dev->irq = spapr_ics_alloc(spapr->ics, dev->irq, false, &local_err); > if (local_err) { > error_propagate(errp, local_err); > return; > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index f9b17d860a75..21e506b13cfa 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -59,6 +59,7 @@ struct sPAPRMachineState { > QLIST_HEAD(, sPAPRPHBState) phbs; > struct sPAPRNVRAM *nvram; > XICSState *xics; > + ICSState *ics; > DeviceState *rtc; > > void *htab; > diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h > index fc4abcd4e796..6d443ce09dba 100644 > --- a/include/hw/ppc/xics.h > +++ b/include/hw/ppc/xics.h > @@ -180,10 +180,10 @@ struct ICSIRQState { > #define XICS_IRQS_SPAPR 1024 > > qemu_irq xics_get_qirq(XICSState *icp, int irq); > -int xics_spapr_alloc(XICSState *icp, int irq_hint, bool lsi, Error **errp); > -int xics_spapr_alloc_block(XICSState *icp, int num, bool lsi, bool align, > +int spapr_ics_alloc(ICSState *ics, int irq_hint, bool lsi, Error **errp); > +int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi, bool align, > Error **errp); > -void xics_spapr_free(XICSState *icp, int irq, int num); > +void spapr_ics_free(ICSState *ics, int irq, int num); > void spapr_dt_xics(XICSState *xics, void *fdt, uint32_t phandle); > > void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
On 02/22/2017 07:59 AM, David Gibson wrote: > On Thu, Feb 16, 2017 at 02:47:26PM +0100, Cédric Le Goater wrote: >> A list of ICS objects was introduced under the XICS object for the >> PowerNV machine, but, for the sPAPR machine, it brings extra complexity >> as there is only a single ICS. To simplify the code, let's add the ICS >> pointer under the sPAPR machine and try to reduce the use of this list >> where possible. >> >> Also, change the xics_spapr_*() routines to use an ICS object instead >> of an XICSState and change their name to reflect that these are >> specific to the sPAPR ICS object. >> >> Signed-off-by: Cédric Le Goater <clg@kaod.org> > > Looks good apart from a minor point noted below. > >> --- >> hw/intc/xics_spapr.c | 22 +++++++++------------- >> hw/ppc/spapr.c | 29 ++++++++++++++++------------- >> hw/ppc/spapr_events.c | 4 ++-- >> hw/ppc/spapr_pci.c | 8 ++++---- >> hw/ppc/spapr_vio.c | 2 +- >> include/hw/ppc/spapr.h | 1 + >> include/hw/ppc/xics.h | 6 +++--- >> 7 files changed, 36 insertions(+), 36 deletions(-) >> >> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c >> index 859b5675e175..1501e796e5e0 100644 >> --- a/hw/intc/xics_spapr.c >> +++ b/hw/intc/xics_spapr.c >> @@ -118,7 +118,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> uint32_t nargs, target_ulong args, >> uint32_t nret, target_ulong rets) >> { >> - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); >> + ICSState *ics = spapr->ics; >> uint32_t nr, srcno, server, priority; >> >> if ((nargs != 3) || (nret != 1)) { >> @@ -151,7 +151,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> uint32_t nargs, target_ulong args, >> uint32_t nret, target_ulong rets) >> { >> - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); >> + ICSState *ics = spapr->ics; >> uint32_t nr, srcno; >> >> if ((nargs != 1) || (nret != 3)) { >> @@ -181,7 +181,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> uint32_t nargs, target_ulong args, >> uint32_t nret, target_ulong rets) >> { >> - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); >> + ICSState *ics = spapr->ics; >> uint32_t nr, srcno; >> >> if ((nargs != 1) || (nret != 1)) { >> @@ -212,7 +212,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> uint32_t nargs, target_ulong args, >> uint32_t nret, target_ulong rets) >> { >> - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); >> + ICSState *ics = spapr->ics; >> uint32_t nr, srcno; >> >> if ((nargs != 1) || (nret != 1)) { >> @@ -294,9 +294,8 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) >> return -1; >> } >> >> -int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp) >> +int spapr_ics_alloc(ICSState *ics, int irq_hint, bool lsi, Error **errp) >> { >> - ICSState *ics = QLIST_FIRST(&xics->ics); >> int irq; >> >> if (!ics) { >> @@ -327,10 +326,9 @@ int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp) >> * Allocate block of consecutive IRQs, and return the number of the first IRQ in >> * the block. If align==true, aligns the first IRQ number to num. >> */ >> -int xics_spapr_alloc_block(XICSState *xics, int num, bool lsi, bool align, >> - Error **errp) >> +int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi, >> + bool align, Error **errp) >> { >> - ICSState *ics = QLIST_FIRST(&xics->ics); >> int i, first = -1; >> >> if (!ics) { >> @@ -380,11 +378,9 @@ static void ics_free(ICSState *ics, int srcno, int num) >> } >> } >> >> -void xics_spapr_free(XICSState *xics, int irq, int num) >> +void spapr_ics_free(ICSState *ics, int irq, int num) >> { >> - ICSState *ics = xics_find_source(xics, irq); >> - >> - if (ics) { >> + if (ics_valid_irq(ics, irq)) { >> trace_xics_ics_free(0, irq, num); >> ics_free(ics, irq - ics->offset, num); >> } >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 5d7c35de8cd9..045f2323a4e9 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -95,13 +95,13 @@ >> >> #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) >> >> -static XICSState *try_create_xics(const char *type, const char *type_ics, >> +static XICSState *try_create_xics(sPAPRMachineState *spapr, >> + const char *type, const char *type_ics, >> const char *type_icp, int nr_servers, >> int nr_irqs, Error **errp) >> { >> Error *err = NULL, *local_err = NULL; >> XICSState *xics; >> - ICSState *ics = NULL; > > I'd prefer to keep this local and just update spapr->ics at the end of > the function. It won't really make a difference, since if the xics > doesn't initialize nothing will work, but currently if this fails it > could leave spapr->ics with a bogus but non-NULL pointer, which isn't > good practice. Indeed. I will make the change. Thanks, C. > >> int i; >> >> xics = XICS_COMMON(object_new(type)); >> @@ -111,16 +111,17 @@ static XICSState *try_create_xics(const char *type, const char *type_ics, >> goto error; >> } >> >> - ics = ICS_SIMPLE(object_new(type_ics)); >> - object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL); >> - object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err); >> - object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xics), NULL); >> - object_property_set_bool(OBJECT(ics), true, "realized", &local_err); >> + spapr->ics = ICS_SIMPLE(object_new(type_ics)); >> + object_property_add_child(OBJECT(spapr), "ics", OBJECT(spapr->ics), NULL); >> + object_property_set_int(OBJECT(spapr->ics), nr_irqs, "nr-irqs", &err); >> + object_property_add_const_link(OBJECT(spapr->ics), "xics", OBJECT(xics), >> + NULL); >> + object_property_set_bool(OBJECT(spapr->ics), true, "realized", &local_err); >> error_propagate(&err, local_err); >> if (err) { >> goto error; >> } >> - QLIST_INSERT_HEAD(&xics->ics, ics, list); >> + QLIST_INSERT_HEAD(&xics->ics, spapr->ics, list); >> >> xics->ss = g_malloc0(nr_servers * sizeof(ICPState)); >> xics->nr_servers = nr_servers; >> @@ -142,8 +143,8 @@ static XICSState *try_create_xics(const char *type, const char *type_ics, >> >> error: >> error_propagate(errp, err); >> - if (ics) { >> - object_unparent(OBJECT(ics)); >> + if (spapr->ics) { >> + object_unparent(OBJECT(spapr->ics)); >> } >> object_unparent(OBJECT(xics)); >> return NULL; >> @@ -158,7 +159,8 @@ static XICSState *xics_system_init(MachineState *machine, >> Error *err = NULL; >> >> if (machine_kernel_irqchip_allowed(machine)) { >> - xics = try_create_xics(TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, >> + xics = try_create_xics(SPAPR_MACHINE(machine), >> + TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, >> TYPE_KVM_ICP, nr_servers, nr_irqs, &err); >> } >> if (machine_kernel_irqchip_required(machine) && !xics) { >> @@ -170,8 +172,9 @@ static XICSState *xics_system_init(MachineState *machine, >> } >> >> if (!xics) { >> - xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, TYPE_ICP, >> - nr_servers, nr_irqs, errp); >> + xics = try_create_xics(SPAPR_MACHINE(machine), >> + TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, >> + TYPE_ICP, nr_servers, nr_irqs, errp); >> } >> >> return xics; >> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c >> index f85a9c32a7fc..38b4258a9be7 100644 >> --- a/hw/ppc/spapr_events.c >> +++ b/hw/ppc/spapr_events.c >> @@ -752,7 +752,7 @@ void spapr_events_init(sPAPRMachineState *spapr) >> spapr->event_sources = spapr_event_sources_new(); >> >> spapr_event_sources_register(spapr->event_sources, EVENT_CLASS_EPOW, >> - xics_spapr_alloc(spapr->xics, 0, false, >> + spapr_ics_alloc(spapr->ics, 0, false, >> &error_fatal)); >> >> /* NOTE: if machine supports modern/dedicated hotplug event source, >> @@ -765,7 +765,7 @@ void spapr_events_init(sPAPRMachineState *spapr) >> */ >> if (spapr->use_hotplug_event_source) { >> spapr_event_sources_register(spapr->event_sources, EVENT_CLASS_HOT_PLUG, >> - xics_spapr_alloc(spapr->xics, 0, false, >> + spapr_ics_alloc(spapr->ics, 0, false, >> &error_fatal)); >> } >> >> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c >> index fd6fc1d95344..01d5c92425ed 100644 >> --- a/hw/ppc/spapr_pci.c >> +++ b/hw/ppc/spapr_pci.c >> @@ -325,7 +325,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> return; >> } >> >> - xics_spapr_free(spapr->xics, msi->first_irq, msi->num); >> + spapr_ics_free(spapr->ics, msi->first_irq, msi->num); >> if (msi_present(pdev)) { >> spapr_msi_setmsg(pdev, 0, false, 0, 0); >> } >> @@ -363,7 +363,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> } >> >> /* Allocate MSIs */ >> - irq = xics_spapr_alloc_block(spapr->xics, req_num, false, >> + irq = spapr_ics_alloc_block(spapr->ics, req_num, false, >> ret_intr_type == RTAS_TYPE_MSI, &err); >> if (err) { >> error_reportf_err(err, "Can't allocate MSIs for device %x: ", >> @@ -374,7 +374,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, >> >> /* Release previous MSIs */ >> if (msi) { >> - xics_spapr_free(spapr->xics, msi->first_irq, msi->num); >> + spapr_ics_free(spapr->ics, msi->first_irq, msi->num); >> g_hash_table_remove(phb->msi, &config_addr); >> } >> >> @@ -1485,7 +1485,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) >> uint32_t irq; >> Error *local_err = NULL; >> >> - irq = xics_spapr_alloc_block(spapr->xics, 1, true, false, &local_err); >> + irq = spapr_ics_alloc_block(spapr->ics, 1, true, false, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> error_prepend(errp, "can't allocate LSIs: "); >> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c >> index 8bfc5f971f8e..a0ee4fd26586 100644 >> --- a/hw/ppc/spapr_vio.c >> +++ b/hw/ppc/spapr_vio.c >> @@ -454,7 +454,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp) >> dev->qdev.id = id; >> } >> >> - dev->irq = xics_spapr_alloc(spapr->xics, dev->irq, false, &local_err); >> + dev->irq = spapr_ics_alloc(spapr->ics, dev->irq, false, &local_err); >> if (local_err) { >> error_propagate(errp, local_err); >> return; >> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h >> index f9b17d860a75..21e506b13cfa 100644 >> --- a/include/hw/ppc/spapr.h >> +++ b/include/hw/ppc/spapr.h >> @@ -59,6 +59,7 @@ struct sPAPRMachineState { >> QLIST_HEAD(, sPAPRPHBState) phbs; >> struct sPAPRNVRAM *nvram; >> XICSState *xics; >> + ICSState *ics; >> DeviceState *rtc; >> >> void *htab; >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >> index fc4abcd4e796..6d443ce09dba 100644 >> --- a/include/hw/ppc/xics.h >> +++ b/include/hw/ppc/xics.h >> @@ -180,10 +180,10 @@ struct ICSIRQState { >> #define XICS_IRQS_SPAPR 1024 >> >> qemu_irq xics_get_qirq(XICSState *icp, int irq); >> -int xics_spapr_alloc(XICSState *icp, int irq_hint, bool lsi, Error **errp); >> -int xics_spapr_alloc_block(XICSState *icp, int num, bool lsi, bool align, >> +int spapr_ics_alloc(ICSState *ics, int irq_hint, bool lsi, Error **errp); >> +int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi, bool align, >> Error **errp); >> -void xics_spapr_free(XICSState *icp, int irq, int num); >> +void spapr_ics_free(ICSState *ics, int irq, int num); >> void spapr_dt_xics(XICSState *xics, void *fdt, uint32_t phandle); >> >> void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu); >
diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c index 859b5675e175..1501e796e5e0 100644 --- a/hw/intc/xics_spapr.c +++ b/hw/intc/xics_spapr.c @@ -118,7 +118,7 @@ static void rtas_set_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); + ICSState *ics = spapr->ics; uint32_t nr, srcno, server, priority; if ((nargs != 3) || (nret != 1)) { @@ -151,7 +151,7 @@ static void rtas_get_xive(PowerPCCPU *cpu, sPAPRMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); + ICSState *ics = spapr->ics; uint32_t nr, srcno; if ((nargs != 1) || (nret != 3)) { @@ -181,7 +181,7 @@ static void rtas_int_off(PowerPCCPU *cpu, sPAPRMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); + ICSState *ics = spapr->ics; uint32_t nr, srcno; if ((nargs != 1) || (nret != 1)) { @@ -212,7 +212,7 @@ static void rtas_int_on(PowerPCCPU *cpu, sPAPRMachineState *spapr, uint32_t nargs, target_ulong args, uint32_t nret, target_ulong rets) { - ICSState *ics = QLIST_FIRST(&spapr->xics->ics); + ICSState *ics = spapr->ics; uint32_t nr, srcno; if ((nargs != 1) || (nret != 1)) { @@ -294,9 +294,8 @@ static int ics_find_free_block(ICSState *ics, int num, int alignnum) return -1; } -int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp) +int spapr_ics_alloc(ICSState *ics, int irq_hint, bool lsi, Error **errp) { - ICSState *ics = QLIST_FIRST(&xics->ics); int irq; if (!ics) { @@ -327,10 +326,9 @@ int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp) * Allocate block of consecutive IRQs, and return the number of the first IRQ in * the block. If align==true, aligns the first IRQ number to num. */ -int xics_spapr_alloc_block(XICSState *xics, int num, bool lsi, bool align, - Error **errp) +int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi, + bool align, Error **errp) { - ICSState *ics = QLIST_FIRST(&xics->ics); int i, first = -1; if (!ics) { @@ -380,11 +378,9 @@ static void ics_free(ICSState *ics, int srcno, int num) } } -void xics_spapr_free(XICSState *xics, int irq, int num) +void spapr_ics_free(ICSState *ics, int irq, int num) { - ICSState *ics = xics_find_source(xics, irq); - - if (ics) { + if (ics_valid_irq(ics, irq)) { trace_xics_ics_free(0, irq, num); ics_free(ics, irq - ics->offset, num); } diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 5d7c35de8cd9..045f2323a4e9 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -95,13 +95,13 @@ #define HTAB_SIZE(spapr) (1ULL << ((spapr)->htab_shift)) -static XICSState *try_create_xics(const char *type, const char *type_ics, +static XICSState *try_create_xics(sPAPRMachineState *spapr, + const char *type, const char *type_ics, const char *type_icp, int nr_servers, int nr_irqs, Error **errp) { Error *err = NULL, *local_err = NULL; XICSState *xics; - ICSState *ics = NULL; int i; xics = XICS_COMMON(object_new(type)); @@ -111,16 +111,17 @@ static XICSState *try_create_xics(const char *type, const char *type_ics, goto error; } - ics = ICS_SIMPLE(object_new(type_ics)); - object_property_add_child(OBJECT(xics), "ics", OBJECT(ics), NULL); - object_property_set_int(OBJECT(ics), nr_irqs, "nr-irqs", &err); - object_property_add_const_link(OBJECT(ics), "xics", OBJECT(xics), NULL); - object_property_set_bool(OBJECT(ics), true, "realized", &local_err); + spapr->ics = ICS_SIMPLE(object_new(type_ics)); + object_property_add_child(OBJECT(spapr), "ics", OBJECT(spapr->ics), NULL); + object_property_set_int(OBJECT(spapr->ics), nr_irqs, "nr-irqs", &err); + object_property_add_const_link(OBJECT(spapr->ics), "xics", OBJECT(xics), + NULL); + object_property_set_bool(OBJECT(spapr->ics), true, "realized", &local_err); error_propagate(&err, local_err); if (err) { goto error; } - QLIST_INSERT_HEAD(&xics->ics, ics, list); + QLIST_INSERT_HEAD(&xics->ics, spapr->ics, list); xics->ss = g_malloc0(nr_servers * sizeof(ICPState)); xics->nr_servers = nr_servers; @@ -142,8 +143,8 @@ static XICSState *try_create_xics(const char *type, const char *type_ics, error: error_propagate(errp, err); - if (ics) { - object_unparent(OBJECT(ics)); + if (spapr->ics) { + object_unparent(OBJECT(spapr->ics)); } object_unparent(OBJECT(xics)); return NULL; @@ -158,7 +159,8 @@ static XICSState *xics_system_init(MachineState *machine, Error *err = NULL; if (machine_kernel_irqchip_allowed(machine)) { - xics = try_create_xics(TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, + xics = try_create_xics(SPAPR_MACHINE(machine), + TYPE_XICS_SPAPR_KVM, TYPE_ICS_KVM, TYPE_KVM_ICP, nr_servers, nr_irqs, &err); } if (machine_kernel_irqchip_required(machine) && !xics) { @@ -170,8 +172,9 @@ static XICSState *xics_system_init(MachineState *machine, } if (!xics) { - xics = try_create_xics(TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, TYPE_ICP, - nr_servers, nr_irqs, errp); + xics = try_create_xics(SPAPR_MACHINE(machine), + TYPE_XICS_SPAPR, TYPE_ICS_SIMPLE, + TYPE_ICP, nr_servers, nr_irqs, errp); } return xics; diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c index f85a9c32a7fc..38b4258a9be7 100644 --- a/hw/ppc/spapr_events.c +++ b/hw/ppc/spapr_events.c @@ -752,7 +752,7 @@ void spapr_events_init(sPAPRMachineState *spapr) spapr->event_sources = spapr_event_sources_new(); spapr_event_sources_register(spapr->event_sources, EVENT_CLASS_EPOW, - xics_spapr_alloc(spapr->xics, 0, false, + spapr_ics_alloc(spapr->ics, 0, false, &error_fatal)); /* NOTE: if machine supports modern/dedicated hotplug event source, @@ -765,7 +765,7 @@ void spapr_events_init(sPAPRMachineState *spapr) */ if (spapr->use_hotplug_event_source) { spapr_event_sources_register(spapr->event_sources, EVENT_CLASS_HOT_PLUG, - xics_spapr_alloc(spapr->xics, 0, false, + spapr_ics_alloc(spapr->ics, 0, false, &error_fatal)); } diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index fd6fc1d95344..01d5c92425ed 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -325,7 +325,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, return; } - xics_spapr_free(spapr->xics, msi->first_irq, msi->num); + spapr_ics_free(spapr->ics, msi->first_irq, msi->num); if (msi_present(pdev)) { spapr_msi_setmsg(pdev, 0, false, 0, 0); } @@ -363,7 +363,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, } /* Allocate MSIs */ - irq = xics_spapr_alloc_block(spapr->xics, req_num, false, + irq = spapr_ics_alloc_block(spapr->ics, req_num, false, ret_intr_type == RTAS_TYPE_MSI, &err); if (err) { error_reportf_err(err, "Can't allocate MSIs for device %x: ", @@ -374,7 +374,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu, sPAPRMachineState *spapr, /* Release previous MSIs */ if (msi) { - xics_spapr_free(spapr->xics, msi->first_irq, msi->num); + spapr_ics_free(spapr->ics, msi->first_irq, msi->num); g_hash_table_remove(phb->msi, &config_addr); } @@ -1485,7 +1485,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) uint32_t irq; Error *local_err = NULL; - irq = xics_spapr_alloc_block(spapr->xics, 1, true, false, &local_err); + irq = spapr_ics_alloc_block(spapr->ics, 1, true, false, &local_err); if (local_err) { error_propagate(errp, local_err); error_prepend(errp, "can't allocate LSIs: "); diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index 8bfc5f971f8e..a0ee4fd26586 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -454,7 +454,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev, Error **errp) dev->qdev.id = id; } - dev->irq = xics_spapr_alloc(spapr->xics, dev->irq, false, &local_err); + dev->irq = spapr_ics_alloc(spapr->ics, dev->irq, false, &local_err); if (local_err) { error_propagate(errp, local_err); return; diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index f9b17d860a75..21e506b13cfa 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -59,6 +59,7 @@ struct sPAPRMachineState { QLIST_HEAD(, sPAPRPHBState) phbs; struct sPAPRNVRAM *nvram; XICSState *xics; + ICSState *ics; DeviceState *rtc; void *htab; diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h index fc4abcd4e796..6d443ce09dba 100644 --- a/include/hw/ppc/xics.h +++ b/include/hw/ppc/xics.h @@ -180,10 +180,10 @@ struct ICSIRQState { #define XICS_IRQS_SPAPR 1024 qemu_irq xics_get_qirq(XICSState *icp, int irq); -int xics_spapr_alloc(XICSState *icp, int irq_hint, bool lsi, Error **errp); -int xics_spapr_alloc_block(XICSState *icp, int num, bool lsi, bool align, +int spapr_ics_alloc(ICSState *ics, int irq_hint, bool lsi, Error **errp); +int spapr_ics_alloc_block(ICSState *ics, int num, bool lsi, bool align, Error **errp); -void xics_spapr_free(XICSState *icp, int irq, int num); +void spapr_ics_free(ICSState *ics, int irq, int num); void spapr_dt_xics(XICSState *xics, void *fdt, uint32_t phandle); void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
A list of ICS objects was introduced under the XICS object for the PowerNV machine, but, for the sPAPR machine, it brings extra complexity as there is only a single ICS. To simplify the code, let's add the ICS pointer under the sPAPR machine and try to reduce the use of this list where possible. Also, change the xics_spapr_*() routines to use an ICS object instead of an XICSState and change their name to reflect that these are specific to the sPAPR ICS object. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- hw/intc/xics_spapr.c | 22 +++++++++------------- hw/ppc/spapr.c | 29 ++++++++++++++++------------- hw/ppc/spapr_events.c | 4 ++-- hw/ppc/spapr_pci.c | 8 ++++---- hw/ppc/spapr_vio.c | 2 +- include/hw/ppc/spapr.h | 1 + include/hw/ppc/xics.h | 6 +++--- 7 files changed, 36 insertions(+), 36 deletions(-)