[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/4] ppc/xics: Make the ICSState a list
|
From: |
David Gibson |
|
Subject: |
Re: [Qemu-devel] [PATCH v3 1/4] ppc/xics: Make the ICSState a list |
|
Date: |
Fri, 8 Jul 2016 14:15:23 +1000 |
|
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Thu, Jul 07, 2016 at 11:24:15PM +0530, Nikunj A Dadhania wrote:
> From: Benjamin Herrenschmidt <address@hidden>
>
> Instead of an array of fixed sized blocks, use a list, as we will need
> to have sources with variable number of interrupts. SPAPR only uses
> a single entry. Native will create more. If performance becomes an
> issue we can add some hashed lookup but for now this will do fine.
>
> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> [ move the initialization of list to xics_common_initfn,
> restore xirr_owner after migration and move restoring to
> icp_post_load]
> Signed-off-by: Nikunj A Dadhania <address@hidden>
> ---
> hw/intc/trace-events | 5 +-
> hw/intc/xics.c | 134
> ++++++++++++++++++++++++++++++++------------------
> hw/intc/xics_kvm.c | 27 +++++++---
> hw/intc/xics_spapr.c | 88 +++++++++++++++++++++------------
> hw/ppc/spapr_events.c | 2 +-
> hw/ppc/spapr_pci.c | 5 +-
> hw/ppc/spapr_vio.c | 2 +-
> include/hw/ppc/xics.h | 13 ++---
> 8 files changed, 175 insertions(+), 101 deletions(-)
>
> diff --git a/hw/intc/trace-events b/hw/intc/trace-events
> index 376dd18..41ced33 100644
> --- a/hw/intc/trace-events
> +++ b/hw/intc/trace-events
> @@ -56,10 +56,11 @@ xics_set_irq_lsi(int srcno, int nr) "set_irq_lsi: srcno
> %d [irq %#x]"
> xics_ics_write_xive(int nr, int srcno, int server, uint8_t priority)
> "ics_write_xive: irq %#x [src %d] server %#x prio %#x"
> xics_ics_reject(int nr, int srcno) "reject irq %#x [src %d]"
> xics_ics_eoi(int nr) "ics_eoi: irq %#x"
> -xics_alloc(int src, int irq) "source#%d, irq %d"
> -xics_alloc_block(int src, int first, int num, bool lsi, int align)
> "source#%d, first irq %d, %d irqs, lsi=%d, alignnum %d"
> +xics_alloc(int irq) "irq %d"
> +xics_alloc_block(int first, int num, bool lsi, int align) "first irq %d, %d
> irqs, lsi=%d, alignnum %d"
> xics_ics_free(int src, int irq, int num) "Source#%d, first irq %d, %d irqs"
> xics_ics_free_warn(int src, int irq) "Source#%d, irq %d is already free"
> +xics_icp_post_load(uint32_t server_no, uint32_t xirr, uint64_t addr, uint8_t
> pend) "server_no %d, xirr %#x, xirr_owner 0x%" PRIx64 ", pending %d"
>
> # hw/intc/s390_flic_kvm.c
> flic_create_device(int err) "flic: create device failed %d"
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index cd48f42..0af05c9 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -32,6 +32,7 @@
> #include "hw/hw.h"
> #include "trace.h"
> #include "qemu/timer.h"
> +#include "hw/ppc/spapr.h"
> #include "hw/ppc/xics.h"
> #include "qemu/error-report.h"
> #include "qapi/visitor.h"
> @@ -96,13 +97,16 @@ void xics_cpu_setup(XICSState *xics, PowerPCCPU *cpu)
> static void xics_common_reset(DeviceState *d)
> {
> XICSState *xics = XICS_COMMON(d);
> + ICSState *ics;
> int i;
>
> for (i = 0; i < xics->nr_servers; i++) {
> device_reset(DEVICE(&xics->ss[i]));
> }
>
> - device_reset(DEVICE(xics->ics));
> + QLIST_FOREACH(ics, &xics->ics, list) {
> + device_reset(DEVICE(ics));
> + }
> }
>
> static void xics_prop_get_nr_irqs(Object *obj, Visitor *v, const char *name,
> @@ -134,7 +138,6 @@ static void xics_prop_set_nr_irqs(Object *obj, Visitor
> *v, const char *name,
> }
>
> assert(info->set_nr_irqs);
> - assert(xics->ics);
> info->set_nr_irqs(xics, value, errp);
> }
>
> @@ -174,6 +177,9 @@ static void xics_prop_set_nr_servers(Object *obj, Visitor
> *v,
>
> static void xics_common_initfn(Object *obj)
> {
> + XICSState *xics = XICS_COMMON(obj);
> +
> + QLIST_INIT(&xics->ics);
> object_property_add(obj, "nr_irqs", "int",
> xics_prop_get_nr_irqs, xics_prop_set_nr_irqs,
> NULL, NULL, NULL);
> @@ -212,33 +218,35 @@ static void ics_reject(ICSState *ics, int nr);
> static void ics_resend(ICSState *ics);
> static void ics_eoi(ICSState *ics, int nr);
>
> -static void icp_check_ipi(XICSState *xics, int server)
> +static void icp_check_ipi(ICPState *ss)
> {
> - ICPState *ss = xics->ss + server;
> -
> if (XISR(ss) && (ss->pending_priority <= ss->mfrr)) {
> return;
> }
>
> - trace_xics_icp_check_ipi(server, ss->mfrr);
> + trace_xics_icp_check_ipi(ss->cs->cpu_index, ss->mfrr);
The dt_id might be more useful here, given the changes that are going
on in that area. But it's not worth holding up this series just for
that.
>
> - if (XISR(ss)) {
> - ics_reject(xics->ics, XISR(ss));
> + if (XISR(ss) && ss->xirr_owner) {
> + ics_reject(ss->xirr_owner, XISR(ss));
> }
>
> ss->xirr = (ss->xirr & ~XISR_MASK) | XICS_IPI;
> ss->pending_priority = ss->mfrr;
> + ss->xirr_owner = NULL;
> qemu_irq_raise(ss->output);
> }
>
> static void icp_resend(XICSState *xics, int server)
> {
> ICPState *ss = xics->ss + server;
> + ICSState *ics;
Possibly this should take icp instead of xics for consistency. Or
else be renamed xics_resend(). But that can be a cleanup for another
time.
> if (ss->mfrr < CPPR(ss)) {
> - icp_check_ipi(xics, server);
> + icp_check_ipi(ss);
> + }
> + QLIST_FOREACH(ics, &xics->ics, list) {
> + ics_resend(ics);
> }
> - ics_resend(xics->ics);
> }
>
> void icp_set_cppr(XICSState *xics, int server, uint8_t cppr)
> @@ -256,7 +264,10 @@ void icp_set_cppr(XICSState *xics, int server, uint8_t
> cppr)
> ss->xirr &= ~XISR_MASK; /* Clear XISR */
> ss->pending_priority = 0xff;
> qemu_irq_lower(ss->output);
> - ics_reject(xics->ics, old_xisr);
> + if (ss->xirr_owner) {
> + ics_reject(ss->xirr_owner, old_xisr);
> + ss->xirr_owner = NULL;
> + }
> }
> } else {
> if (!XISR(ss)) {
> @@ -271,7 +282,7 @@ void icp_set_mfrr(XICSState *xics, int server, uint8_t
> mfrr)
>
> ss->mfrr = mfrr;
> if (mfrr < CPPR(ss)) {
> - icp_check_ipi(xics, server);
> + icp_check_ipi(ss);
> }
> }
>
> @@ -282,6 +293,7 @@ uint32_t icp_accept(ICPState *ss)
> qemu_irq_lower(ss->output);
> ss->xirr = ss->pending_priority << 24;
> ss->pending_priority = 0xff;
> + ss->xirr_owner = NULL;
>
> trace_xics_icp_accept(xirr, ss->xirr);
>
> @@ -299,30 +311,40 @@ uint32_t icp_ipoll(ICPState *ss, uint32_t *mfrr)
> void icp_eoi(XICSState *xics, int server, uint32_t xirr)
> {
> ICPState *ss = xics->ss + server;
> + ICSState *ics;
> + uint32_t irq;
>
> /* Send EOI -> ICS */
> ss->xirr = (ss->xirr & ~CPPR_MASK) | (xirr & CPPR_MASK);
> trace_xics_icp_eoi(server, xirr, ss->xirr);
> - ics_eoi(xics->ics, xirr & XISR_MASK);
> + irq = xirr & XISR_MASK;
> + QLIST_FOREACH(ics, &xics->ics, list) {
> + if (ics_valid_irq(ics, irq)) {
> + ics_eoi(ics, irq);
You could slightly optimize this by adding a break; here, but again
that can be a tweak for another time.
> + }
> + }
> if (!XISR(ss)) {
> icp_resend(xics, server);
> }
> }
>
> -static void icp_irq(XICSState *xics, int server, int nr, uint8_t priority)
> +static void icp_irq(ICSState *ics, int server, int nr, uint8_t priority)
> {
> + XICSState *xics = ics->xics;
> ICPState *ss = xics->ss + server;
It's kind of weird for an icp_() function to take an ics but not an
icp. It might make more sense for it to take both, even though
obviously it can be derived from just the ics. Again, won't hold up
the series for this.
> trace_xics_icp_irq(server, nr, priority);
>
> if ((priority >= CPPR(ss))
> || (XISR(ss) && (ss->pending_priority <= priority))) {
> - ics_reject(xics->ics, nr);
> + ics_reject(ics, nr);
> } else {
> - if (XISR(ss)) {
> - ics_reject(xics->ics, XISR(ss));
> + if (XISR(ss) && ss->xirr_owner) {
> + ics_reject(ss->xirr_owner, XISR(ss));
> + ss->xirr_owner = NULL;
> }
> ss->xirr = (ss->xirr & ~XISR_MASK) | (nr & XISR_MASK);
> + ss->xirr_owner = ics;
> ss->pending_priority = priority;
> trace_xics_icp_raise(ss->xirr, ss->pending_priority);
> qemu_irq_raise(ss->output);
> @@ -378,12 +400,42 @@ static void icp_reset(DeviceState *dev)
> qemu_set_irq(icp->output, 0);
> }
>
> +static int icp_post_load(ICPState *ss, int version_id)
> +{
> + sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> + XICSState *xics = spapr->xics;
> + uint32_t irq, i;
> + static uint32_t server_no = 0;
Eugh.. static locals are usually horrid.
> + server_no++;
> + irq = XISR(ss);
> + if (!ss->cs || !irq)
> + goto icpend;
That's an awful use of a goto - inverting the if sense is clearer and
no longer.
> +
> + /* Restore the xirr_owner */
> + ss->xirr_owner = xics_find_source(xics, irq);
Do we actually need this? Have you confirmed that replaying the
icp_resend()s won't already accomplish this?
> + icpend:
> + trace_xics_icp_post_load(server_no, ss->xirr, (uint64_t)ss->xirr_owner,
> ss->pending_priority);
> + if (xics->nr_servers != server_no)
> + return 0;
> +
> + /* All the ICPs are processed, not restore all the state */
> + for (i = 0; i < xics->nr_servers; i++) {
> + icp_resend(xics, i);
> + }
> + server_no = 0;
Eugh. This is really ugly. It really looks like we need some
migration state on the overall xics, not just the component icps and
icss. Dealing with backwards compatibility will be hairy, but I
really hope we can do better than this hack.
> + return 0;
> +}
> +
> static void icp_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> + ICPStateClass *icpc = ICP_CLASS(klass);
>
> dc->reset = icp_reset;
> dc->vmsd = &vmstate_icp_server;
> + icpc->post_load = icp_post_load;
> }
>
> static const TypeInfo icp_info = {
> @@ -405,8 +457,7 @@ static void resend_msi(ICSState *ics, int srcno)
> if (irq->status & XICS_STATUS_REJECTED) {
> irq->status &= ~XICS_STATUS_REJECTED;
> if (irq->priority != 0xff) {
> - icp_irq(ics->xics, irq->server, srcno + ics->offset,
> - irq->priority);
> + icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
> }
> }
> }
> @@ -419,7 +470,7 @@ static void resend_lsi(ICSState *ics, int srcno)
> && (irq->status & XICS_STATUS_ASSERTED)
> && !(irq->status & XICS_STATUS_SENT)) {
> irq->status |= XICS_STATUS_SENT;
> - icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
> + icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
> }
> }
>
> @@ -434,7 +485,7 @@ static void set_irq_msi(ICSState *ics, int srcno, int val)
> irq->status |= XICS_STATUS_MASKED_PENDING;
> trace_xics_masked_pending();
> } else {
> - icp_irq(ics->xics, irq->server, srcno + ics->offset,
> irq->priority);
> + icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
> }
> }
> }
> @@ -473,7 +524,7 @@ static void write_xive_msi(ICSState *ics, int srcno)
> }
>
> irq->status &= ~XICS_STATUS_MASKED_PENDING;
> - icp_irq(ics->xics, irq->server, srcno + ics->offset, irq->priority);
> + icp_irq(ics, irq->server, srcno + ics->offset, irq->priority);
> }
>
> static void write_xive_lsi(ICSState *ics, int srcno)
> @@ -505,8 +556,11 @@ static void ics_reject(ICSState *ics, int nr)
> ICSIRQState *irq = ics->irqs + nr - ics->offset;
>
> trace_xics_ics_reject(nr, nr - ics->offset);
> - irq->status |= XICS_STATUS_REJECTED; /* Irrelevant but harmless for LSI
> */
> - irq->status &= ~XICS_STATUS_SENT; /* Irrelevant but harmless for MSI */
> + if (irq->flags & XICS_FLAGS_IRQ_MSI) {
> + irq->status |= XICS_STATUS_REJECTED;
> + } else {
> + irq->status &= ~XICS_STATUS_SENT;
> + }
This change appears unrelated to the rest. If not that needs an explanation.
> }
>
> static void ics_resend(ICSState *ics)
> @@ -554,17 +608,6 @@ static void ics_reset(DeviceState *dev)
> }
> }
>
> -static int ics_post_load(ICSState *ics, int version_id)
> -{
> - int i;
> -
> - for (i = 0; i < ics->xics->nr_servers; i++) {
> - icp_resend(ics->xics, i);
> - }
> -
> - return 0;
> -}
> -
> static void ics_dispatch_pre_save(void *opaque)
> {
> ICSState *ics = opaque;
> @@ -639,12 +682,10 @@ static void ics_realize(DeviceState *dev, Error **errp)
> static void ics_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
> - ICSStateClass *isc = ICS_CLASS(klass);
>
> dc->realize = ics_realize;
> dc->vmsd = &vmstate_ics;
> dc->reset = ics_reset;
> - isc->post_load = ics_post_load;
> }
>
> static const TypeInfo ics_info = {
> @@ -659,28 +700,23 @@ static const TypeInfo ics_info = {
> /*
> * Exported functions
> */
> -int xics_find_source(XICSState *xics, int irq)
> +ICSState *xics_find_source(XICSState *xics, int irq)
> {
> - int sources = 1;
> - int src;
> + ICSState *ics;
>
> - /* FIXME: implement multiple sources */
> - for (src = 0; src < sources; ++src) {
> - ICSState *ics = &xics->ics[src];
> + QLIST_FOREACH(ics, &xics->ics, list) {
> if (ics_valid_irq(ics, irq)) {
> - return src;
> + return ics;
> }
> }
> -
> - return -1;
> + return NULL;
> }
>
> qemu_irq xics_get_qirq(XICSState *xics, int irq)
> {
> - int src = xics_find_source(xics, irq);
> + ICSState *ics = xics_find_source(xics, irq);
>
> - if (src >= 0) {
> - ICSState *ics = &xics->ics[src];
> + if (ics) {
> return ics->qirqs[irq - ics->offset];
> }
>
> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c
> index edbd62f..04fa7cb 100644
> --- a/hw/intc/xics_kvm.c
> +++ b/hw/intc/xics_kvm.c
> @@ -365,7 +365,13 @@ static void xics_kvm_cpu_setup(XICSState *xics,
> PowerPCCPU *cpu)
> static void xics_kvm_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
> Error **errp)
> {
> - xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
> + ICSState *ics = QLIST_FIRST(&xics->ics);
> +
> + /* This needs to be deprecated ... */
> + xics->nr_irqs = nr_irqs;
> + if (ics) {
> + ics->nr_irqs = nr_irqs;
> + }
> }
>
> static void xics_kvm_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> @@ -398,6 +404,7 @@ static void xics_kvm_realize(DeviceState *dev, Error
> **errp)
> {
> KVMXICSState *xicskvm = XICS_SPAPR_KVM(dev);
> XICSState *xics = XICS_COMMON(dev);
> + ICSState *ics;
> int i, rc;
> Error *error = NULL;
> struct kvm_create_device xics_create_device = {
> @@ -449,10 +456,12 @@ static void xics_kvm_realize(DeviceState *dev, Error
> **errp)
>
> xicskvm->kernel_xics_fd = xics_create_device.fd;
>
> - object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
> - if (error) {
> - error_propagate(errp, error);
> - goto fail;
> + QLIST_FOREACH(ics, &xics->ics, list) {
> + object_property_set_bool(OBJECT(ics), true, "realized", &error);
> + if (error) {
> + error_propagate(errp, error);
> + goto fail;
> + }
> }
>
> assert(xics->nr_servers);
> @@ -481,10 +490,12 @@ fail:
> static void xics_kvm_initfn(Object *obj)
> {
> XICSState *xics = XICS_COMMON(obj);
> + ICSState *ics;
>
> - xics->ics = ICS(object_new(TYPE_KVM_ICS));
> - object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
> - xics->ics->xics = xics;
> + ics = ICS(object_new(TYPE_KVM_ICS));
> + object_property_add_child(obj, "ics", OBJECT(ics), NULL);
> + ics->xics = xics;
> + QLIST_INSERT_HEAD(&xics->ics, ics, list);
> }
>
> static void xics_kvm_class_init(ObjectClass *oc, void *data)
> diff --git a/hw/intc/xics_spapr.c b/hw/intc/xics_spapr.c
> index 618826d..0b0845d 100644
> --- a/hw/intc/xics_spapr.c
> +++ b/hw/intc/xics_spapr.c
> @@ -113,13 +113,17 @@ static void rtas_set_xive(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> uint32_t nargs, target_ulong args,
> uint32_t nret, target_ulong rets)
> {
> - ICSState *ics = spapr->xics->ics;
> + ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> uint32_t nr, server, priority;
>
> if ((nargs != 3) || (nret != 1)) {
> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> return;
> }
> + if (!ics) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
>
> nr = rtas_ld(args, 0);
> server = xics_get_cpu_index_by_dt_id(rtas_ld(args, 1));
> @@ -141,13 +145,17 @@ static void rtas_get_xive(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> uint32_t nargs, target_ulong args,
> uint32_t nret, target_ulong rets)
> {
> - ICSState *ics = spapr->xics->ics;
> + ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> uint32_t nr;
>
> if ((nargs != 1) || (nret != 3)) {
> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> return;
> }
> + if (!ics) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
>
> nr = rtas_ld(args, 0);
>
> @@ -166,13 +174,17 @@ static void rtas_int_off(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> uint32_t nargs, target_ulong args,
> uint32_t nret, target_ulong rets)
> {
> - ICSState *ics = spapr->xics->ics;
> + ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> uint32_t nr;
>
> if ((nargs != 1) || (nret != 1)) {
> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> return;
> }
> + if (!ics) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
>
> nr = rtas_ld(args, 0);
>
> @@ -192,13 +204,17 @@ static void rtas_int_on(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> uint32_t nargs, target_ulong args,
> uint32_t nret, target_ulong rets)
> {
> - ICSState *ics = spapr->xics->ics;
> + ICSState *ics = QLIST_FIRST(&spapr->xics->ics);
> uint32_t nr;
>
> if ((nargs != 1) || (nret != 1)) {
> rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
> return;
> }
> + if (!ics) {
> + rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
> + return;
> + }
>
> nr = rtas_ld(args, 0);
>
> @@ -217,7 +233,13 @@ static void rtas_int_on(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> static void xics_spapr_set_nr_irqs(XICSState *xics, uint32_t nr_irqs,
> Error **errp)
> {
> - xics->nr_irqs = xics->ics->nr_irqs = nr_irqs;
> + ICSState *ics = QLIST_FIRST(&xics->ics);
> +
> + /* This needs to be deprecated ... */
> + xics->nr_irqs = nr_irqs;
> + if (ics) {
> + ics->nr_irqs = nr_irqs;
> + }
> }
>
> static void xics_spapr_set_nr_servers(XICSState *xics, uint32_t nr_servers,
> @@ -240,6 +262,7 @@ static void xics_spapr_set_nr_servers(XICSState *xics,
> uint32_t nr_servers,
> static void xics_spapr_realize(DeviceState *dev, Error **errp)
> {
> XICSState *xics = XICS_SPAPR(dev);
> + ICSState *ics;
> Error *error = NULL;
> int i;
>
> @@ -261,10 +284,12 @@ static void xics_spapr_realize(DeviceState *dev, Error
> **errp)
> spapr_register_hypercall(H_EOI, h_eoi);
> spapr_register_hypercall(H_IPOLL, h_ipoll);
>
> - object_property_set_bool(OBJECT(xics->ics), true, "realized", &error);
> - if (error) {
> - error_propagate(errp, error);
> - return;
> + QLIST_FOREACH(ics, &xics->ics, list) {
> + object_property_set_bool(OBJECT(ics), true, "realized", &error);
> + if (error) {
> + error_propagate(errp, error);
> + return;
> + }
> }
>
> for (i = 0; i < xics->nr_servers; i++) {
> @@ -280,10 +305,12 @@ static void xics_spapr_realize(DeviceState *dev, Error
> **errp)
> static void xics_spapr_initfn(Object *obj)
> {
> XICSState *xics = XICS_SPAPR(obj);
> + ICSState *ics;
>
> - xics->ics = ICS(object_new(TYPE_ICS));
> - object_property_add_child(obj, "ics", OBJECT(xics->ics), NULL);
> - xics->ics->xics = xics;
> + ics = ICS(object_new(TYPE_ICS));
> + object_property_add_child(obj, "ics", OBJECT(ics), NULL);
> + ics->xics = xics;
> + QLIST_INSERT_HEAD(&xics->ics, ics, list);
> }
>
> static void xics_spapr_class_init(ObjectClass *oc, void *data)
> @@ -329,14 +356,15 @@ static int ics_find_free_block(ICSState *ics, int num,
> int alignnum)
> return -1;
> }
>
> -int xics_spapr_alloc(XICSState *xics, int src, int irq_hint, bool lsi,
> - Error **errp)
> +int xics_spapr_alloc(XICSState *xics, int irq_hint, bool lsi, Error **errp)
> {
> - ICSState *ics = &xics->ics[src];
> + ICSState *ics = QLIST_FIRST(&xics->ics);
> int irq;
>
> + if (!ics) {
> + return -1;
> + }
> if (irq_hint) {
> - assert(src == xics_find_source(xics, irq_hint));
> if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
> error_setg(errp, "can't allocate IRQ %d: already in use",
> irq_hint);
> return -1;
> @@ -352,7 +380,7 @@ int xics_spapr_alloc(XICSState *xics, int src, int
> irq_hint, bool lsi,
> }
>
> ics_set_irq_type(ics, irq - ics->offset, lsi);
> - trace_xics_alloc(src, irq);
> + trace_xics_alloc(irq);
>
> return irq;
> }
> @@ -361,13 +389,16 @@ int xics_spapr_alloc(XICSState *xics, int src, int
> irq_hint, bool lsi,
> * 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 src, int num, bool lsi,
> - bool align, Error **errp)
> +int xics_spapr_alloc_block(XICSState *xics, int num, bool lsi, bool align,
> + Error **errp)
> {
> + ICSState *ics = QLIST_FIRST(&xics->ics);
> int i, first = -1;
> - ICSState *ics = &xics->ics[src];
>
> - assert(src == 0);
> + if (!ics) {
> + return -1;
> + }
> +
> /*
> * MSIMesage::data is used for storing VIRQ so
> * it has to be aligned to num to support multiple
> @@ -394,7 +425,7 @@ int xics_spapr_alloc_block(XICSState *xics, int src, int
> num, bool lsi,
> }
> first += ics->offset;
>
> - trace_xics_alloc_block(src, first, num, lsi, align);
> + trace_xics_alloc_block(first, num, lsi, align);
>
> return first;
> }
> @@ -405,7 +436,7 @@ static void ics_free(ICSState *ics, int srcno, int num)
>
> for (i = srcno; i < srcno + num; ++i) {
> if (ICS_IRQ_FREE(ics, i)) {
> - trace_xics_ics_free_warn(ics - ics->xics->ics, i + ics->offset);
> + trace_xics_ics_free_warn(0, i + ics->offset);
> }
> memset(&ics->irqs[i], 0, sizeof(ICSIRQState));
> }
> @@ -413,15 +444,10 @@ static void ics_free(ICSState *ics, int srcno, int num)
>
> void xics_spapr_free(XICSState *xics, int irq, int num)
> {
> - int src = xics_find_source(xics, irq);
> -
> - if (src >= 0) {
> - ICSState *ics = &xics->ics[src];
> -
> - /* FIXME: implement multiple sources */
> - assert(src == 0);
> + ICSState *ics = xics_find_source(xics, irq);
>
> - trace_xics_ics_free(ics - xics->ics, irq, num);
> + if (ics) {
> + trace_xics_ics_free(0, irq, num);
> ics_free(ics, irq - ics->offset, num);
> }
> }
> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
> index b0668b3..adf8da4 100644
> --- a/hw/ppc/spapr_events.c
> +++ b/hw/ppc/spapr_events.c
> @@ -603,7 +603,7 @@ out_no_events:
> void spapr_events_init(sPAPRMachineState *spapr)
> {
> QTAILQ_INIT(&spapr->pending_events);
> - spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, 0, false,
> + spapr->check_exception_irq = xics_spapr_alloc(spapr->xics, 0, false,
> &error_fatal);
> spapr->epow_notifier.notify = spapr_powerdown_req;
> qemu_register_powerdown_notifier(&spapr->epow_notifier);
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 949c44f..4d06794 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -362,7 +362,7 @@ static void rtas_ibm_change_msi(PowerPCCPU *cpu,
> sPAPRMachineState *spapr,
> }
>
> /* Allocate MSIs */
> - irq = xics_spapr_alloc_block(spapr->xics, 0, req_num, false,
> + irq = xics_spapr_alloc_block(spapr->xics, req_num, false,
> ret_intr_type == RTAS_TYPE_MSI, &err);
> if (err) {
> error_reportf_err(err, "Can't allocate MSIs for device %x: ",
> @@ -1444,8 +1444,7 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> uint32_t irq;
> Error *local_err = NULL;
>
> - irq = xics_spapr_alloc_block(spapr->xics, 0, 1, true, false,
> - &local_err);
> + irq = xics_spapr_alloc_block(spapr->xics, 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 f93244d..22360af 100644
> --- a/hw/ppc/spapr_vio.c
> +++ b/hw/ppc/spapr_vio.c
> @@ -463,7 +463,7 @@ static void spapr_vio_busdev_realize(DeviceState *qdev,
> Error **errp)
> dev->qdev.id = id;
> }
>
> - dev->irq = xics_spapr_alloc(spapr->xics, 0, dev->irq, false, &local_err);
> + dev->irq = xics_spapr_alloc(spapr->xics, dev->irq, false, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 6189a3b..6ad3057 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -84,7 +84,7 @@ struct XICSState {
> uint32_t nr_servers;
> uint32_t nr_irqs;
> ICPState *ss;
> - ICSState *ics;
> + QLIST_HEAD(, ICSState) ics;
> };
>
> #define TYPE_ICP "icp"
> @@ -110,6 +110,7 @@ struct ICPState {
> DeviceState parent_obj;
> /*< public >*/
> CPUState *cs;
> + ICSState *xirr_owner;
> uint32_t xirr;
> uint8_t pending_priority;
> uint8_t mfrr;
> @@ -144,6 +145,7 @@ struct ICSState {
> qemu_irq *qirqs;
> ICSIRQState *irqs;
> XICSState *xics;
> + QLIST_ENTRY(ICSState) list;
> };
>
> static inline bool ics_valid_irq(ICSState *ics, uint32_t nr)
> @@ -171,10 +173,9 @@ struct ICSIRQState {
> #define XICS_IRQS_SPAPR 1024
>
> qemu_irq xics_get_qirq(XICSState *icp, int irq);
> -int xics_spapr_alloc(XICSState *icp, int src, int irq_hint, bool lsi,
> - Error **errp);
> -int xics_spapr_alloc_block(XICSState *icp, int src, int num, bool lsi,
> - bool align, Error **errp);
> +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,
> + Error **errp);
> void xics_spapr_free(XICSState *icp, int irq, int num);
>
> void xics_cpu_setup(XICSState *icp, PowerPCCPU *cpu);
> @@ -194,6 +195,6 @@ void ics_write_xive(ICSState *ics, int nr, int server,
>
> void ics_set_irq_type(ICSState *ics, int srcno, bool lsi);
>
> -int xics_find_source(XICSState *icp, int irq);
> +ICSState *xics_find_source(XICSState *icp, int irq);
>
> #endif /* __XICS_H__ */
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature