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

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2650 bytes --]

On Thu, Sep 26, 2019 at 05:35:39PM +0200, Greg Kurz wrote:
> On Thu, 26 Sep 2019 09:05:56 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
> 
> > >>> +    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. 
> > 
> 
> Yeah it would make sense to have nr_servers within the sPAPR XIVE object,
> so that we don't have to pass it when building the FDT node. Same stands
> for XICS actually.
> 
> And as part of my current work to reduce HW VP consumption, I also need
> nr_servers to pass it to the KVM device.
> 
> > I don't think we can change that without breaking migration though :/
> > 
> 
> Hmm... why ? The QOM property is just an interface, it doesn't change the
> state. In the end we migrate the same number of XiveEND objects.

Yeah, I think it can probably be done.  I don't really have the energy
left to sort it out for the time being, maybe one day.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]