* [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number @ 2023-07-12 11:27 Akihiko Odaki 2023-07-12 11:46 ` Michael S. Tsirkin 0 siblings, 1 reply; 5+ messages in thread From: Akihiko Odaki @ 2023-07-12 11:27 UTC (permalink / raw) To: Michael S . Tsirkin, Ani Sinha Cc: qemu-devel, qemu-block, Marcel Apfelbaum, Sriram Yagnaraman, Jason Wang, Keith Busch, Klaus Jensen, Akihiko Odaki 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> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number 2023-07-12 11:27 [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number Akihiko Odaki @ 2023-07-12 11:46 ` Michael S. Tsirkin 2023-07-12 11:50 ` Akihiko Odaki 0 siblings, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2023-07-12 11:46 UTC (permalink / raw) To: Akihiko Odaki Cc: Ani Sinha, qemu-devel, qemu-block, Marcel Apfelbaum, Sriram Yagnaraman, Jason Wang, Keith Busch, Klaus Jensen 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number 2023-07-12 11:46 ` Michael S. Tsirkin @ 2023-07-12 11:50 ` Akihiko Odaki 2023-07-12 12:06 ` Michael S. Tsirkin 0 siblings, 1 reply; 5+ messages in thread From: Akihiko Odaki @ 2023-07-12 11:50 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ani Sinha, qemu-devel, qemu-block, Marcel Apfelbaum, Sriram Yagnaraman, Jason Wang, Keith Busch, Klaus Jensen On 2023/07/12 20:46, Michael S. Tsirkin wrote: > 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. What do you think about multiple SR/IOV PFs? Do you think it's worth/easy enough to fix SR/IOV code to support it? Otherwise it's not worth fixing ARI since currently only SR/IOV devices implement it. > >> --- >> 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 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number 2023-07-12 11:50 ` Akihiko Odaki @ 2023-07-12 12:06 ` Michael S. Tsirkin 2023-07-13 13:32 ` Akihiko Odaki 0 siblings, 1 reply; 5+ messages in thread From: Michael S. Tsirkin @ 2023-07-12 12:06 UTC (permalink / raw) To: Akihiko Odaki Cc: Ani Sinha, qemu-devel, qemu-block, Marcel Apfelbaum, Sriram Yagnaraman, Jason Wang, Keith Busch, Klaus Jensen On Wed, Jul 12, 2023 at 08:50:32PM +0900, Akihiko Odaki wrote: > On 2023/07/12 20:46, Michael S. Tsirkin wrote: > > 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. > > What do you think about multiple SR/IOV PFs? Do you think it's worth/easy > enough to fix SR/IOV code to support it? Otherwise it's not worth fixing ARI > since currently only SR/IOV devices implement it. There's nothing especially hard about it. You can in particular just assume the user knows what he's doing and not worry too much about checking. Creating invalid configs might also come handy e.g. for debug. The important thing, and that's missing ATM, is giving management ability to find out TotalVFs, VF offset and VF stride, so it can avoid creating these conflicts. For igd maybe we should make VF offset and VF stride just 1 unconditionally - I have no idea why it was made 2 ATM - could you check what does real hardware do? Yes, warning at least is handy for management debugging. It shouldn't be hard I think, but the logic does tend to be O(n^2). Maybe add a flag to check, and management developers can use that for debugging. > > > > > --- > > > 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 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number 2023-07-12 12:06 ` Michael S. Tsirkin @ 2023-07-13 13:32 ` Akihiko Odaki 0 siblings, 0 replies; 5+ messages in thread From: Akihiko Odaki @ 2023-07-13 13:32 UTC (permalink / raw) To: Michael S. Tsirkin Cc: Ani Sinha, qemu-devel, qemu-block, Marcel Apfelbaum, Sriram Yagnaraman, Jason Wang, Keith Busch, Klaus Jensen On 2023/07/12 21:06, Michael S. Tsirkin wrote: > On Wed, Jul 12, 2023 at 08:50:32PM +0900, Akihiko Odaki wrote: >> On 2023/07/12 20:46, Michael S. Tsirkin wrote: >>> 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. >> >> What do you think about multiple SR/IOV PFs? Do you think it's worth/easy >> enough to fix SR/IOV code to support it? Otherwise it's not worth fixing ARI >> since currently only SR/IOV devices implement it. > > There's nothing especially hard about it. You can in particular just > assume the user knows what he's doing and not worry too much about > checking. Creating invalid configs might also come handy e.g. for debug. > The important thing, and that's missing ATM, is giving management > ability to find out TotalVFs, VF offset and VF stride, so it can > avoid creating these conflicts. > > For igd maybe we should make VF offset and VF stride just 1 unconditionally - > I have no idea why it was made 2 ATM - could you check what does > real hardware do? The current igb implementation match with real hardware. It is defined in the datasheet*, section 9.6.4.6. I don't know why it's 2 either. * https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/82576eg-gbe-datasheet.pdf > > Yes, warning at least is handy for > management debugging. It shouldn't be hard I think, but the > logic does tend to be O(n^2). Maybe add a flag to check, > and management developers can use that for debugging. > >>> >>>> --- >>>> 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 >>> > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-07-13 13:34 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-07-12 11:27 [PATCH] hw/pci: Warn when ARI/SR-IOV device has non-zero Function number Akihiko Odaki 2023-07-12 11:46 ` Michael S. Tsirkin 2023-07-12 11:50 ` Akihiko Odaki 2023-07-12 12:06 ` Michael S. Tsirkin 2023-07-13 13:32 ` Akihiko Odaki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).