Re: [Qemu-stable] [PATCH] spapr_pci: fix device tree props for MSI/MSI-X

[David Gibson]

On Tue, Sep 15, 2015 at 04:34:59PM -0500, Michael Roth wrote:
> PAPR requires ibm,req#msi and ibm,req#msi-x to be present in the
> device node to define the number of msi/msi-x interrupts the device
> supports, respectively.
> 
> Currently we have ibm,req#msi-x hardcoded to a non-sensical constant
> that happens to be 2, and are missing ibm,req#msi entirely. The result
> of that is that msi-x capable devices get limited to 2 msi-x
> interrupts (which can impact performance), and msi-only devices likely
> wouldn't work at all. Additionally, if devices expect a minimum that
> exceeds 2, the guest driver may fail to load entirely.
> 
> SLOF still owns the generation of these properties at boot-time
> (although other device properties have since been offloaded to QEMU),
> but for hotplugged devices we rely on the values generated by QEMU
> and thus hit the limitations above.
> 
> Fix this by generating these properties in QEMU as expected by guests.
> 
> In the future it may make sense to modify SLOF to pass through these
> values directly as we do with other props since we're duplicating SLOF
> code.
> 
> Cc: address@hidden
> Cc: address@hidden
> Cc: David Gibson <address@hidden>
> Cc: Nikunj A Dadhania <address@hidden>
> Signed-off-by: Michael Roth <address@hidden>

Applied to spapr-next, thanks.

> ---
>  hw/ppc/spapr_pci.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 2782856..8e9edff 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1012,6 +1012,7 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
> void *fdt, int offset,
>      int pci_status, err;
>      char *buf = NULL;
>      uint32_t drc_index = spapr_phb_get_pci_drc_index(sphb, dev);
> +    uint32_t max_msi, max_msix;
>  
>      if (pci_default_read_config(dev, PCI_HEADER_TYPE, 1) ==
>          PCI_HEADER_TYPE_BRIDGE) {
> @@ -1092,8 +1093,15 @@ static int spapr_populate_pci_child_dt(PCIDevice *dev, 
> void *fdt, int offset,
>                            RESOURCE_CELLS_ADDRESS));
>      _FDT(fdt_setprop_cell(fdt, offset, "#size-cells",
>                            RESOURCE_CELLS_SIZE));
> -    _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x",
> -                          RESOURCE_CELLS_SIZE));
> +
> +    max_msi = msi_nr_vectors_allocated(dev);
> +    if (max_msi) {
> +        _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi", max_msi));
> +    }
> +    max_msix = dev->msix_entries_nr;
> +    if (max_msix) {
> +        _FDT(fdt_setprop_cell(fdt, offset, "ibm,req#msi-x", max_msix));
> +    }
>  
>      populate_resource_props(dev, &rp);
>      _FDT(fdt_setprop(fdt, offset, "reg", (uint8_t *)rp.reg, rp.reg_len));

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

pgpe7lompcKTS.pgp
Description: PGP signature