On 11/17/2017 05:54 AM, David Gibson wrote: > On Fri, Nov 10, 2017 at 03:20:14PM +0000, Cédric Le Goater wrote: >> It will be used later on to distinguish the allocation of an LSI >> interrupt from an MSI and also to reduce the use of the ICSIRQState >> array of the ICSState object, which is on our way to introduce XIVE. >> >> The 'irq' parameter continues to refer to the global IRQ number space. >> >> On PowerNV, only the PSI controller interrupts are handled and they >> are all LSIs. >> >> Signed-off-by: Cédric Le Goater <c...@kaod.org> > > !?! AFAICT this is a step backwards. The users of ics_is_lsi() here > are in the xics code. So they already have the right information > locally in the ICSState object. Why on earth would you indirect > through the fabric.
because I am trying to get rid of the fact that the current ICS allocator handles two things at the same time : IRQ allocation and IRQ type. And that's a problem because the ICSState object is just used everywhere in the code. OK, So that's is not to your liking, I will come up with some other solution. Thanks for looking. C. > >> --- >> hw/intc/xics.c | 26 +++++++++++++++++--------- >> hw/intc/xics_kvm.c | 4 ++-- >> hw/ppc/pnv.c | 16 ++++++++++++++++ >> hw/ppc/spapr.c | 9 +++++++++ >> include/hw/ppc/xics.h | 2 ++ >> 5 files changed, 46 insertions(+), 11 deletions(-) >> >> diff --git a/hw/intc/xics.c b/hw/intc/xics.c >> index 2c4899f278e2..42880e736697 100644 >> --- a/hw/intc/xics.c >> +++ b/hw/intc/xics.c >> @@ -33,6 +33,7 @@ >> #include "trace.h" >> #include "qemu/timer.h" >> #include "hw/ppc/xics.h" >> +#include "hw/ppc/spapr.h" >> #include "qemu/error-report.h" >> #include "qapi/visitor.h" >> #include "monitor/monitor.h" >> @@ -70,8 +71,7 @@ void ics_pic_print_info(ICSState *ics, Monitor *mon) >> } >> monitor_printf(mon, " %4x %s %02x %02x\n", >> ics->offset + i, >> - (irq->flags & XICS_FLAGS_IRQ_LSI) ? >> - "LSI" : "MSI", >> + ics_is_lsi(ics, i) ? "LSI" : "MSI", > > !?! > >> irq->priority, irq->status); >> } >> } >> @@ -377,6 +377,14 @@ static const TypeInfo icp_info = { >> /* >> * ICS: Source layer >> */ >> +bool ics_is_lsi(ICSState *ics, int srcno) >> +{ >> + XICSFabric *xi = ics->xics; >> + XICSFabricClass *xic = XICS_FABRIC_GET_CLASS(xi); >> + >> + return xic->irq_is_lsi(xi, srcno + ics->offset); >> +} >> + >> static void ics_simple_resend_msi(ICSState *ics, int srcno) >> { >> ICSIRQState *irq = ics->irqs + srcno; >> @@ -435,7 +443,7 @@ static void ics_simple_set_irq(void *opaque, int srcno, >> int val) >> { >> ICSState *ics = (ICSState *)opaque; >> >> - if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { >> + if (ics_is_lsi(ics, srcno)) { >> ics_simple_set_irq_lsi(ics, srcno, val); >> } else { >> ics_simple_set_irq_msi(ics, srcno, val); >> @@ -472,7 +480,7 @@ void ics_simple_write_xive(ICSState *ics, int srcno, int >> server, >> trace_xics_ics_simple_write_xive(ics->offset + srcno, srcno, server, >> priority); >> >> - if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { >> + if (ics_is_lsi(ics, srcno)) { >> ics_simple_write_xive_lsi(ics, srcno); >> } else { >> ics_simple_write_xive_msi(ics, srcno); >> @@ -484,10 +492,10 @@ static void ics_simple_reject(ICSState *ics, uint32_t >> nr) >> ICSIRQState *irq = ics->irqs + nr - ics->offset; >> >> trace_xics_ics_simple_reject(nr, nr - ics->offset); >> - if (irq->flags & XICS_FLAGS_IRQ_MSI) { >> - irq->status |= XICS_STATUS_REJECTED; >> - } else if (irq->flags & XICS_FLAGS_IRQ_LSI) { >> + if (ics_is_lsi(ics, nr - ics->offset)) { >> irq->status &= ~XICS_STATUS_SENT; >> + } else { >> + irq->status |= XICS_STATUS_REJECTED; >> } >> } >> >> @@ -497,7 +505,7 @@ static void ics_simple_resend(ICSState *ics) >> >> for (i = 0; i < ics->nr_irqs; i++) { >> /* FIXME: filter by server#? */ >> - if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) { >> + if (ics_is_lsi(ics, i)) { >> ics_simple_resend_lsi(ics, i); >> } else { >> ics_simple_resend_msi(ics, i); >> @@ -512,7 +520,7 @@ static void ics_simple_eoi(ICSState *ics, uint32_t nr) >> >> trace_xics_ics_simple_eoi(nr); >> >> - if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI) { >> + if (ics_is_lsi(ics, srcno)) { >> irq->status &= ~XICS_STATUS_SENT; >> } >> } >> diff --git a/hw/intc/xics_kvm.c b/hw/intc/xics_kvm.c >> index 3091ad3ac2c8..2f10637c9f7c 100644 >> --- a/hw/intc/xics_kvm.c >> +++ b/hw/intc/xics_kvm.c >> @@ -258,7 +258,7 @@ static int ics_set_kvm_state(ICSState *ics, int >> version_id) >> state |= KVM_XICS_MASKED; >> } >> >> - if (ics->irqs[i].flags & XICS_FLAGS_IRQ_LSI) { >> + if (ics_is_lsi(ics, i)) { >> state |= KVM_XICS_LEVEL_SENSITIVE; >> if (irq->status & XICS_STATUS_ASSERTED) { >> state |= KVM_XICS_PENDING; >> @@ -293,7 +293,7 @@ static void ics_kvm_set_irq(void *opaque, int srcno, int >> val) >> int rc; >> >> args.irq = srcno + ics->offset; >> - if (ics->irqs[srcno].flags & XICS_FLAGS_IRQ_MSI) { >> + if (!ics_is_lsi(ics, srcno)) { >> if (!val) { >> return; >> } >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index 8288940ef9d7..958223376b4c 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -1035,6 +1035,21 @@ static bool pnv_irq_test(XICSFabric *xi, int irq) >> return false; >> } >> >> +static bool pnv_irq_is_lsi(XICSFabric *xi, int irq) >> +{ >> + PnvMachineState *pnv = POWERNV_MACHINE(xi); >> + int i; >> + >> + /* PowerNV machine only has PSI interrupts which are all LSIs */ >> + for (i = 0; i < pnv->num_chips; i++) { >> + ICSState *ics = &pnv->chips[i]->psi.ics; >> + if (ics_valid_irq(ics, irq)) { >> + return true; >> + } >> + } >> + return false; >> +} >> + >> static void pnv_pic_print_info(InterruptStatsProvider *obj, >> Monitor *mon) >> { >> @@ -1120,6 +1135,7 @@ static void powernv_machine_class_init(ObjectClass >> *oc, void *data) >> xic->ics_get = pnv_ics_get; >> xic->ics_resend = pnv_ics_resend; >> xic->irq_test = pnv_irq_test; >> + xic->irq_is_lsi = pnv_irq_is_lsi; >> ispc->print_info = pnv_pic_print_info; >> >> powernv_machine_class_props_init(oc); >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 1cbbd7715a85..ce314fcf38db 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -3628,6 +3628,14 @@ static void spapr_irq_free_block_2_11(XICSFabric *xi, >> int irq, int num) >> } >> } >> >> +static bool spapr_irq_is_lsi(XICSFabric *xi, int irq) >> +{ >> + sPAPRMachineState *spapr = SPAPR_MACHINE(xi); >> + int srcno = irq - spapr->ics->offset; >> + >> + return spapr->ics->irqs[srcno].flags & XICS_FLAGS_IRQ_LSI; >> +} >> + >> static bool spapr_irq_test(XICSFabric *xi, int irq) >> { >> sPAPRMachineState *spapr = SPAPR_MACHINE(xi); >> @@ -3765,6 +3773,7 @@ static void spapr_machine_class_init(ObjectClass *oc, >> void *data) >> xic->irq_test = spapr_irq_test; >> xic->irq_alloc_block = spapr_irq_alloc_block; >> xic->irq_free_block = spapr_irq_free_block; >> + xic->irq_is_lsi = spapr_irq_is_lsi; >> >> ispc->print_info = spapr_pic_print_info; >> /* Force NUMA node memory size to be a multiple of >> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h >> index 30e7f2e0a7dd..478f8e510179 100644 >> --- a/include/hw/ppc/xics.h >> +++ b/include/hw/ppc/xics.h >> @@ -179,6 +179,7 @@ typedef struct XICSFabricClass { >> bool (*irq_test)(XICSFabric *xi, int irq); >> int (*irq_alloc_block)(XICSFabric *xi, int count, int align); >> void (*irq_free_block)(XICSFabric *xi, int irq, int num); >> + bool (*irq_is_lsi)(XICSFabric *xi, int irq); >> } XICSFabricClass; >> >> #define XICS_IRQS_SPAPR 1024 >> @@ -205,6 +206,7 @@ void ics_simple_write_xive(ICSState *ics, int nr, int >> server, >> void ics_set_irq_type(ICSState *ics, int srcno, bool lsi); >> void icp_pic_print_info(ICPState *icp, Monitor *mon); >> void ics_pic_print_info(ICSState *ics, Monitor *mon); >> +bool ics_is_lsi(ICSState *ics, int srno); >> >> void ics_resend(ICSState *ics); >> void icp_resend(ICPState *ss); >