Re: [PATCH 20/20] spapr: Eliminate SpaprIrq::init hook

[Cédric Le Goater]

>>> +    if (spapr->irq->xive) {
>>> +        uint32_t nr_servers = spapr_max_server_number(spapr);
>>> +        DeviceState *dev;
>>> +        int i;
>>> +
>>> +        dev = qdev_create(NULL, TYPE_SPAPR_XIVE);
>>> +        qdev_prop_set_uint32(dev, "nr-irqs",
>>> +                             spapr->irq->nr_xirqs + SPAPR_XIRQ_BASE);
>>> +        /*
>>> +         * 8 XIVE END structures per CPU. One for each available
>>> +         * priority
>>> +         */
>>> +        qdev_prop_set_uint32(dev, "nr-ends", nr_servers << 3);
>>> +        qdev_init_nofail(dev);
>>> +
>>> +        spapr->xive = SPAPR_XIVE(dev);
>>> +
>>> +        /* Enable the CPU IPIs */
>>> +        for (i = 0; i < nr_servers; ++i) {
>>> +            Error *local_err = NULL;
>>> +
>>> +            spapr_xive_irq_claim(spapr->xive, SPAPR_IRQ_IPI + i, false, 
>>> &local_err);
>>> +            if (local_err) {
>>> +                goto out;
>>> +            }
>>> +        }
>>
>> We could move the IPI claim part in the realize routine of SPAPR_XIVE.
> 
> Yeah, I know.  I thought about this, but there's a slight complication
> in that the XIVE part doesn't know nr_servers directly.  There's
> several possible ways to handle that, but I wasn't 100% happy with any
> that I came up with yet.

The "nr-ends" property was inappropriate, "nr-servers" would have been
better and we would have hidden the calculation of ENDs 'nr_servers << 3'
in the realize routine of SpaprXive. 

I don't think we can change that without breaking migration though :/

C.

>>
>>> +        spapr_xive_hcall_init(spapr);
>>
>> This also.
> 
> Right.
> 
>> It can be done later one.
> 
> That's my intention.
> 
>>
>> C.
>>
>>> +    }
>>>  
>>>      spapr->qirqs = qemu_allocate_irqs(spapr->irq->set_irq, spapr,
>>>                                        spapr->irq->nr_xirqs + 
>>> SPAPR_XIRQ_BASE);
>>> +
>>> +out:
>>> +    error_propagate(errp, local_err);
>>>  }
>>>  
>>>  void spapr_irq_claim(SpaprMachineState *spapr, int irq, bool lsi, Error 
>>> **errp)
>>> @@ -757,7 +744,6 @@ SpaprIrq spapr_irq_xics_legacy = {
>>>      .xics        = true,
>>>      .xive        = false,
>>>  
>>> -    .init        = spapr_irq_init_xics,
>>>      .claim       = spapr_irq_claim_xics,
>>>      .free        = spapr_irq_free_xics,
>>>      .print_info  = spapr_irq_print_info_xics,
>>> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
>>> index 6816cb0500..fa862c665b 100644
>>> --- a/include/hw/ppc/spapr_irq.h
>>> +++ b/include/hw/ppc/spapr_irq.h
>>> @@ -42,7 +42,6 @@ typedef struct SpaprIrq {
>>>      bool        xics;
>>>      bool        xive;
>>>  
>>> -    void (*init)(SpaprMachineState *spapr, Error **errp);
>>>      void (*claim)(SpaprMachineState *spapr, int irq, bool lsi, Error 
>>> **errp);
>>>      void (*free)(SpaprMachineState *spapr, int irq);
>>>      void (*print_info)(SpaprMachineState *spapr, Monitor *mon);
>>> diff --git a/include/hw/ppc/xics_spapr.h b/include/hw/ppc/xics_spapr.h
>>> index 691a6d00f7..267984a97b 100644
>>> --- a/include/hw/ppc/xics_spapr.h
>>> +++ b/include/hw/ppc/xics_spapr.h
>>> @@ -34,6 +34,7 @@
>>>  #define TYPE_ICS_SPAPR "ics-spapr"
>>>  #define ICS_SPAPR(obj) OBJECT_CHECK(ICSState, (obj), TYPE_ICS_SPAPR)
>>>  
>>> +void ics_spapr_create(SpaprMachineState *spapr, int nr_xirqs, Error 
>>> **errp);
>>>  void spapr_dt_xics(SpaprMachineState *spapr, uint32_t nr_servers, void 
>>> *fdt,
>>>                     uint32_t phandle);
>>>  int xics_kvm_connect(SpaprMachineState *spapr, Error **errp);
>>>
>>
>