On Thu, 29 Jun, 2023, 12:17 pm Akihiko Odaki, <akihiko.od...@daynix.com> wrote:
> On 2023/06/29 13:07, Ani Sinha wrote: > > PCI Express ports only have one slot, so PCI Express devices can only be > > plugged into slot 0 on a PCIE port. Enforce it. > > > > The change has been tested to not break ARI by instantiating seven vfs > on an > > emulated igb device (the maximum number of vfs the linux igb driver > supports). > > The vfs are seen to have non-zero device/slot numbers in the conventional > > PCI BDF representation. > > > > CC: jus...@redhat.com > > CC: imamm...@redhat.com > > CC: akihiko.od...@daynix.com > > > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2128929 > > Signed-off-by: Ani Sinha <anisi...@redhat.com> > > Reviewed-by: Julia Suvorova <jus...@redhat.com> > > --- > > hw/pci/pci.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index e2eb4c3b4a..0320ac2bb3 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -65,6 +65,7 @@ bool pci_available = true; > > static char *pcibus_get_dev_path(DeviceState *dev); > > static char *pcibus_get_fw_dev_path(DeviceState *dev); > > static void pcibus_reset(BusState *qbus); > > +static bool pcie_has_upstream_port(PCIDevice *dev); > > > > static Property pci_props[] = { > > DEFINE_PROP_PCI_DEVFN("addr", PCIDevice, devfn, -1), > > @@ -1190,6 +1191,20 @@ static PCIDevice > *do_pci_register_device(PCIDevice *pci_dev, > > name); > > > > return NULL; > > + } /* > > + * With SRIOV and ARI, vfs can have non-zero slot in the > conventional > > + * PCI interpretation as all five bits reserved for slot > addresses are > > + * also used for function bits for the various vfs. Ignore that > case. > > + * It is too early here to check for ARI capabilities in the PCI > config > > + * space. Hence, we check for a vf device instead. > > + */ > > Why don't just perform this check after the capabilities are set? > We don't want to allocate resources for wrong device parameters. We want to error out early. Other checks also are performed at the same place . Show quoted text > > > > Regards, > Akihiko Odaki > > > + else if (!pci_is_vf(pci_dev) && > > + pcie_has_upstream_port(pci_dev) && > > + PCI_SLOT(devfn)) { > > + error_setg(errp, "PCI: slot %d is not valid for %s," > > + " parent device only allows plugging into slot 0.", > > + PCI_SLOT(devfn), name); > > + return NULL; > > } > > > > pci_dev->devfn = devfn; > >
