[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH v2 1/2] spapr: introduce a spapr_irq class 'nr_msi
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH v2 1/2] spapr: introduce a spapr_irq class 'nr_msis' attribute |
Date: |
Tue, 11 Sep 2018 09:34:52 +0200 |
On Tue, 11 Sep 2018 07:55:02 +0200
Cédric Le Goater <address@hidden> wrote:
> The number of MSI interrupts a sPAPR machine can allocate is in direct
> relation with the number of interrupts of the sPAPRIrq backend. Define
> statically this value at the sPAPRIrq class level and use it for the
> "ibm,pe-total-#msi" property of the sPAPR PHB.
>
> According to the PAPR specs, "ibm,pe-total-#msi" defines the maximum
> number of MSIs that are available to the PE. We choose to advertise
> the maximum number of MSIs that are available to the machine for
> simplicity of the model and to avoid segmenting the MSI interrupt pool
> which can be easily shared. If the pool limit is reached, it can be
> extended dynamically.
>
> Finally, remove XICS_IRQS_SPAPR which is now unused.
>
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
Looks good to me. Just one comment below.
> include/hw/ppc/spapr_irq.h | 1 +
> include/hw/ppc/xics.h | 2 --
> hw/ppc/spapr_irq.c | 9 +++++++--
> hw/ppc/spapr_pci.c | 5 +++--
> 4 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/hw/ppc/spapr_irq.h b/include/hw/ppc/spapr_irq.h
> index 0e98c4474bb2..650f810ad2aa 100644
> --- a/include/hw/ppc/spapr_irq.h
> +++ b/include/hw/ppc/spapr_irq.h
> @@ -31,6 +31,7 @@ void spapr_irq_msi_reset(sPAPRMachineState *spapr);
>
> typedef struct sPAPRIrq {
> uint32_t nr_irqs;
> + uint32_t nr_msis;
>
> void (*init)(sPAPRMachineState *spapr, Error **errp);
> int (*claim)(sPAPRMachineState *spapr, int irq, bool lsi, Error **errp);
> diff --git a/include/hw/ppc/xics.h b/include/hw/ppc/xics.h
> index 9c2916c9b23a..9958443d1984 100644
> --- a/include/hw/ppc/xics.h
> +++ b/include/hw/ppc/xics.h
> @@ -181,8 +181,6 @@ typedef struct XICSFabricClass {
> ICPState *(*icp_get)(XICSFabric *xi, int server);
> } XICSFabricClass;
>
> -#define XICS_IRQS_SPAPR 1024
> -
> void spapr_dt_xics(int nr_servers, void *fdt, uint32_t phandle);
>
> ICPState *xics_icp_get(XICSFabric *xi, int server);
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 0cbb5dd39368..fe8be5f5217a 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -99,7 +99,7 @@ static void spapr_irq_init_xics(sPAPRMachineState *spapr,
> Error **errp)
>
> /* Initialize the MSI IRQ allocator. */
> if (!SPAPR_MACHINE_GET_CLASS(spapr)->legacy_irq_allocation) {
> - spapr_irq_msi_init(spapr, XICS_IRQ_BASE + nr_irqs - SPAPR_IRQ_MSI);
> + spapr_irq_msi_init(spapr, smc->irq->nr_msis);
> }
>
> if (kvm_enabled()) {
> @@ -195,8 +195,13 @@ static void spapr_irq_print_info_xics(sPAPRMachineState
> *spapr, Monitor *mon)
> ics_pic_print_info(spapr->ics, mon);
> }
>
> +#define SPAPR_IRQ_XICS_NR_IRQS 0x400
> +#define SPAPR_IRQ_XICS_NR_MSIS \
> + (XICS_IRQ_BASE + SPAPR_IRQ_XICS_NR_IRQS - SPAPR_IRQ_MSI)
> +
> sPAPRIrq spapr_irq_xics = {
> - .nr_irqs = XICS_IRQS_SPAPR,
> + .nr_irqs = SPAPR_IRQ_XICS_NR_IRQS,
> + .nr_msis = SPAPR_IRQ_XICS_NR_MSIS,
>
> .init = spapr_irq_init_xics,
> .claim = spapr_irq_claim_xics,
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 6bcb4f419b6b..bb736177e76c 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2121,6 +2121,7 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> sPAPRTCETable *tcet;
> PCIBus *bus = PCI_HOST_BRIDGE(phb)->bus;
> sPAPRFDT s_fdt;
> + sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(qdev_get_machine());
>
It is a bit unfortunate to add another user of qdev_get_machine()...
> /* Start populating the FDT */
> nodename = g_strdup_printf("address@hidden" PRIx64, phb->buid);
> @@ -2138,8 +2139,8 @@ int spapr_populate_pci_dt(sPAPRPHBState *phb,
> _FDT(fdt_setprop(fdt, bus_off, "ranges", &ranges, sizeof_ranges));
> _FDT(fdt_setprop(fdt, bus_off, "reg", &bus_reg, sizeof(bus_reg)));
> _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pci-config-space-type", 0x1));
> - /* TODO: fine tune the total count of allocatable MSIs per PHB */
> - _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi",
> XICS_IRQS_SPAPR));
> + _FDT(fdt_setprop_cell(fdt, bus_off, "ibm,pe-total-#msi",
> + smc->irq->nr_msis));
>
... and to expose machine class internals. Since spapr_populate_pci_dt() is only
called from the core machine code, maybe have the caller to pass the number of
MSIs ?
Anyway, this can be done in a followup patch so:
Reviewed-by: Greg Kurz <address@hidden>
> /* Dynamic DMA window */
> if (phb->ddw_enabled) {