On 09/10/2018 08:12 AM, David Gibson wrote:
> 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 <c...@kaod.org>
>
> Sorry, I got sidetracked and forgot about reviewing this patch.
No problem. It is giving us time to work on OpenCAPI passthrough
and challenge a bit more the static IRQ layout and the sPAPR XIVE
interrupt mode.
> 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...
Yes. The number is in direct relation with the msi allocator.
>> ---
>> 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.
yes
Thanks,
C.
>> +
>> + .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,
>> +};
>
