Re: [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number

[Michael S. Tsirkin]

On Wed, Jul 12, 2023 at 08:27:32PM +0900, Akihiko Odaki wrote:
> Current SR/IOV implementations assume that hardcoded Function numbers
> are always available and will not conflict. It is somewhat non-trivial
> to make the Function numbers to use controllable to avoid Function
> number conflicts so ensure there is only one PF to make the assumption
> hold true.
> Also warn when non-SR/IOV multifunction was attempted with ARI enabled;
> ARI has the next Function number field register, and currently it's
> hardcoded to 0, which prevents non-SR/IOV multifunction. It is
> certainly possible to add a logic to determine the correct next Function
> number according to the configuration, but it's not worth since all
> ARI-capable devices are also SR/IOV devices, which do not support
> multiple PFs as stated above.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

I am not really interested in adding this stuff.
The real thing to focus on is fixing ARI emulation, not
warning users about ways in which it's broken.

> ---
>  hw/pci/pci.c | 59 +++++++++++++++++++++++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 784c02a182..50359a0f3a 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2124,23 +2124,48 @@ static void pci_qdev_realize(DeviceState *qdev, Error 
> **errp)
>          }
>      }
>  
> -    /*
> -     * A PCIe Downstream Port that do not have ARI Forwarding enabled must
> -     * associate only Device 0 with the device attached to the bus
> -     * representing the Link from the Port (PCIe base spec rev 4.0 ver 0.3,
> -     * sec 7.3.1).
> -     * With ARI, PCI_SLOT() can return non-zero value as the traditional
> -     * 5-bit Device Number and 3-bit Function Number fields in its associated
> -     * Routing IDs, Requester IDs and Completer IDs are interpreted as a
> -     * single 8-bit Function Number. Hence, ignore ARI capable devices.
> -     */
> -    if (pci_is_express(pci_dev) &&
> -        !pcie_find_capability(pci_dev, PCI_EXT_CAP_ID_ARI) &&
> -        pcie_has_upstream_port(pci_dev) &&
> -        PCI_SLOT(pci_dev->devfn)) {
> -        warn_report("PCI: slot %d is not valid for %s,"
> -                    " parent device only allows plugging into slot 0.",
> -                    PCI_SLOT(pci_dev->devfn), pci_dev->name);
> +    if (pci_is_express(pci_dev)) {
> +        /*
> +         * A PCIe Downstream Port that do not have ARI Forwarding enabled 
> must
> +         * associate only Device 0 with the device attached to the bus
> +         * representing the Link from the Port (PCIe base spec rev 4.0 ver 
> 0.3,
> +         * sec 7.3.1).
> +         * With ARI, PCI_SLOT() can return non-zero value as the traditional
> +         * 5-bit Device Number and 3-bit Function Number fields in its
> +         * associated Routing IDs, Requester IDs and Completer IDs are
> +         * interpreted as a single 8-bit Function Number. Hence, ignore ARI
> +         * capable devices.
> +         */
> +        if (!pcie_find_capability(pci_dev, PCI_EXT_CAP_ID_ARI) &&
> +            pcie_has_upstream_port(pci_dev) &&
> +            PCI_SLOT(pci_dev->devfn)) {
> +            warn_report("PCI: slot %d is not valid for %s,"
> +                        " parent device only allows plugging into slot 0.",
> +                        PCI_SLOT(pci_dev->devfn), pci_dev->name);
> +        }
> +
> +        /*
> +         * Current SR/IOV implementations assume that hardcoded Function 
> numbers
> +         * are always available. Ensure there is only one PF to make the
> +         * assumption hold true.
> +         */
> +        if (pcie_find_capability(pci_dev, PCI_EXT_CAP_ID_SRIOV) &&
> +            PCI_FUNC(pci_dev->devfn)) {
> +            warn_report("PCI: function %d is not valid for %s,"
> +                        " currently PF can only be assigned to function 0.",
> +                        PCI_FUNC(pci_dev->devfn), pci_dev->name);
> +        }
> +
> +        /*
> +         * ARI has the next Function number field register, and currently 
> it's
> +         * hardcoded to 0, which prevents non-SR/IOV multifunction.
> +         */
> +        if (pcie_find_capability(pci_dev, PCI_EXT_CAP_ID_ARI) &&
> +            !pci_is_vf(pci_dev) && (pci_dev->devfn & 0xff)) {
> +            warn_report("PCI: function %d is not valid for %s,"
> +                        " non-SR/IOV multifunction is not supported with ARI 
> enabled.",
> +                        pci_dev->devfn & 0xff, pci_dev->name);
> +        }
>      }
>  
>      if (pci_dev->failover_pair_id) {
> -- 
> 2.41.0