On Fri, 18 Jan 2019 13:38:02 +0100
Cédric Le Goater <c...@kaod.org> wrote:
> On 1/17/19 6:15 PM, Greg Kurz wrote:
> > Every PHB needs to claim 4 LSIs to support legacy PCI devices. This is
> > currently done at PHB realize. When using in-kernel XICS (or upcoming
> > in-kernel XIVE), QEMU synchronizes the state of all irqs, including
> > these LSIs, later on at machine reset.
> >
> > In order to support PHB hotplug, we need a way to tell KVM about the
> > LSIs that doesn't require a machine reset. Since these irq numbers are
> > fixed values derived from the PHB index, let's identify them all at
> > machine init. Older machines that don't have fixed irq numbers cannot
> > support PHB hotplug and keep the existing behavior.
> >
> > Signed-off-by: Greg Kurz <gr...@kaod.org>
> > ---
> > hw/ppc/spapr.c | 6 ++++++
> > hw/ppc/spapr_pci.c | 20 ++++++++++++++++++--
> > include/hw/pci-host/spapr.h | 2 ++
> > 3 files changed, 26 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 26f8e55cc25e..9189b4d3a9d6 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2802,6 +2802,12 @@ static void spapr_machine_init(MachineState *machine)
> >
> > phb = spapr_create_default_phb();
> >
> > + if (!smc->legacy_irq_allocation) {
> > + for (i = 0; i < SPAPR_MAX_PHBS; i++) {
> > + spapr_phb_set_lsis(i, spapr);
> > + }
> > + }
> > +
>
> Can't we be more brutal and do the LSI setting in spapr_irq_init()
> for the whole [ SPAPR_IRQ_PCI_LSI ... SPAPR_IRQ_MSI -1 ] range ?
>
I like this. And we should even set the type of all IRQs for the machine
lifetime since it is an invariant. I'll give it a try for v4.
> C.
>
> > for (i = 0; i < nb_nics; i++) {
> > NICInfo *nd = &nd_table[i];
> >
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index ec9d4d28004c..f5f13a4d4816 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1559,6 +1559,22 @@ static void spapr_pci_unplug_request(HotplugHandler
> > *plug_handler,
> > }
> > }
> >
> > +static uint32_t spapr_phb_index_to_lsi(int phb_index, int irq_index)
> > +{
> > + return SPAPR_IRQ_PCI_LSI + phb_index * PCI_NUM_PINS + irq_index;
> > +}
> > +
> > +void spapr_phb_set_lsis(int index, sPAPRMachineState *spapr)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < PCI_NUM_PINS; i++) {
> > + uint32_t irq = spapr_phb_index_to_lsi(index, i);
> > +
> > + spapr_irq_set_type(spapr, irq, true);
> > + }
> > +}
> > +
> > static void spapr_phb_realize(DeviceState *dev, Error **errp)
> > {
> > /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user
> > @@ -1726,7 +1742,7 @@ static void spapr_phb_realize(DeviceState *dev, Error
> > **errp)
> >
> > /* Initialize the LSI table */
> > for (i = 0; i < PCI_NUM_PINS; i++) {
> > - uint32_t irq = SPAPR_IRQ_PCI_LSI + sphb->index * PCI_NUM_PINS + i;
> > + uint32_t irq = spapr_phb_index_to_lsi(sphb->index, i);
> > Error *local_err = NULL;
> >
> > if (smc->legacy_irq_allocation) {
> > @@ -1736,6 +1752,7 @@ static void spapr_phb_realize(DeviceState *dev, Error
> > **errp)
> > "can't allocate LSIs: ");
> > return;
> > }
> > + spapr_irq_set_type(spapr, irq, true);
> > }
> >
> > spapr_irq_claim(spapr, irq, &local_err);
> > @@ -1743,7 +1760,6 @@ static void spapr_phb_realize(DeviceState *dev, Error
> > **errp)
> > error_propagate_prepend(errp, local_err, "can't allocate LSIs:
> > ");
> > return;
> > }
> > - spapr_irq_set_type(spapr, irq, true);
> >
> > sphb->lsi_table[i].irq = irq;
> > }
> > diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> > index e0e683c32469..bb0ae7fdd41d 100644
> > --- a/include/hw/pci-host/spapr.h
> > +++ b/include/hw/pci-host/spapr.h
> > @@ -164,4 +164,6 @@ static inline void spapr_phb_vfio_reset(DeviceState
> > *qdev)
> >
> > void spapr_phb_dma_reset(sPAPRPHBState *sphb);
> >
> > +void spapr_phb_set_lsis(int index, sPAPRMachineState *spapr);
> > +
> > #endif /* PCI_HOST_SPAPR_H */
> >
>
