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

[Greg Kurz]

On Fri, 27 Sep 2019 15:51:04 +1000
David Gibson <address@hidden> wrote:

> 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 <address@hidden> 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.
> 

As mentioned above I have another need for "nr-servers", I'll have
a look.

pgpylpVVovWJO.pgp
Description: OpenPGP digital signature