Re: [Qemu-ppc] [PATCH v3 06/19] spapr: Identify LSIs of all possible PHBs at machine init

[Cédric Le Goater]

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

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