Re: [PATCH v3 22/34] spapr, xics, xive: Move cpu_intc_create from SpaprIrq to SpaprInterruptController

[David Gibson]

On Wed, Oct 02, 2019 at 09:31:08AM +0200, Cédric Le Goater wrote:
> On 02/10/2019 08:40, David Gibson wrote:
> > On Wed, Oct 02, 2019 at 08:13:55AM +0200, Cédric Le Goater wrote:
> >>>> @@ -527,6 +471,30 @@ static int spapr_irq_check(SpaprMachineState 
> >>>> *spapr, Error **errp)
> >>>>  /*
> >>>>   * sPAPR IRQ frontend routines for devices
> >>>>   */
> >>>> +#define ALL_INTCS(spapr_) \
> >>>> +    { SPAPR_INTC((spapr_)->ics), SPAPR_INTC((spapr_)->xive), }
> >>>
> >>> I would have expected this array to be under the machine.
> >>>
> >>>> +int spapr_irq_cpu_intc_create(SpaprMachineState *spapr,
> >>>> +                              PowerPCCPU *cpu, Error **errp)
> >>>> +{
> >>>> +    SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
> >>>> +    int i;
> >>>> +    int rc;
> >>>> +
> >>>> +    for (i = 0; i < ARRAY_SIZE(intcs); i++) {
> >>>
> >>> but it would have been difficult to use ARRAY_SIZE. OK then.
> >>>
> >>>> +        SpaprInterruptController *intc = intcs[i];
> >>>> +        if (intc) {
> >>>
> >>> Is that test needed ? 
> >>
> >> I understand now : spapr->ics and spapr->xive can be NULL. 
> >>
> >> I think using a list would be better.
> > 
> > Uh.. a list in what sense?
> 
> an interrupt controller list under the machine.


Well

a) You're the one who suggested not dropping the ics and xive
individual pointers, which would be redundant with such a list.

b) A linked list for 2 entries seems overly complicated.

> 
> when created, an interrupt controller would self register in that list.
> 
> C.
> 

-- 
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