Re: [Qemu-devel] [PATCH v6 4/4] spapr: increase the size of the IRQ number space

[David Gibson]

On Mon, Jul 30, 2018 at 04:11:34PM +0200, Cédric Le Goater wrote:
> The new layout using static IRQ number does not leave much space to
> the dynamic MSI range, only 0x100 IRQ numbers. Increase the total
> number of IRQS for newer machines and introduce a legacy XICS backend
> for pre-3.1 machines to maintain compatibility.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>

Sorry, I got sidetracked and forgot about reviewing this patch.


Now that the number of irqs is set in the backend, I think the
reference to XICS_IRQS_SPAPR setting ibm,pe-total-#msi in
spapr_populate_pci_dt() needs to be change to look at the backend
instead...

> ---
>  include/hw/ppc/spapr_irq.h |  1 +
>  hw/ppc/spapr.c             |  1 +
>  hw/ppc/spapr_irq.c         | 12 +++++++++++-
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 0e98c4474bb2..626160ba475e 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -40,6 +40,7 @@ typedef struct sPAPRIrq {
>  } sPAPRIrq;
>  
>  extern sPAPRIrq spapr_irq_xics;
> +extern sPAPRIrq spapr_irq_xics_legacy;
>  
>  int spapr_irq_claim(sPAPRMachineState *spapr, int irq, bool lsi, Error 
> **errp);
>  void spapr_irq_free(sPAPRMachineState *spapr, int irq, int num);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d9f8cca49208..5ae62b0682d2 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3947,6 +3947,7 @@ static void 
> spapr_machine_3_0_class_options(MachineClass *mc)
>      SET_MACHINE_COMPAT(mc, SPAPR_COMPAT_3_0);
>  
>      smc->legacy_irq_allocation = true;
> +    smc->irq = &spapr_irq_xics_legacy;
>  }
>  
>  DEFINE_SPAPR_MACHINE(3_0, "3.0", false);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 0cbb5dd39368..620c49b38455 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -196,7 +196,7 @@ static void spapr_irq_print_info_xics(sPAPRMachineState 
> *spapr, Monitor *mon)
>  }
>  
>  sPAPRIrq spapr_irq_xics = {
> -    .nr_irqs     = XICS_IRQS_SPAPR,
> +    .nr_irqs     = 0x1000,
>  
>      .init        = spapr_irq_init_xics,
>      .claim       = spapr_irq_claim_xics,
> @@ -284,3 +284,13 @@ int spapr_irq_find(sPAPRMachineState *spapr, int num, 
> bool align, Error **errp)
>  
>      return first + ics->offset;
>  }
> +
> +sPAPRIrq spapr_irq_xics_legacy = {
> +    .nr_irqs     = XICS_IRQS_SPAPR,

.. and with that done, I think it makes to just inline the old value
here and remove the XICS_IRQS_SPAPR #define, since its name is no
longer accurate.

> +
> +    .init        = spapr_irq_init_xics,
> +    .claim       = spapr_irq_claim_xics,
> +    .free        = spapr_irq_free_xics,
> +    .qirq        = spapr_qirq_xics,
> +    .print_info  = spapr_irq_print_info_xics,
> +};

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

signature.asc
Description: PGP signature