Re: [PATCH 4/4] pci: Compare function number and ARI next function number

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Jul 11, 2023 at 12:40:47PM +0530, Ani Sinha wrote:
> 
> 
> > On 01-Jul-2023, at 12:31 PM, Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> > 
> > The function number must be lower than the next function number
> > advertised with ARI.
> > 
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.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..568665ee42 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -2059,6 +2059,8 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> >     Error *local_err = NULL;
> >     bool is_default_rom;
> >     uint16_t class_id;
> > +    uint16_t ari;
> > +    uint16_t nextfn;
> > 
> >     /*
> >      * capped by systemd (see: udev-builtin-net_id.c)
> > @@ -2121,6 +2123,19 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> >         }
> >     }
> > 
> > +    if (pci_is_express(pci_dev)) {
> > +        ari = pcie_find_capability(pci_dev, PCI_EXT_CAP_ID_ARI);
> > +        if (ari) {
> > +            nextfn = (pci_get_long(pci_dev->config + ari + PCI_ARI_CAP) >> 8) & 0xff;
> > +            if (nextfn && (pci_dev->devfn & 0xff) >= nextfn) {
> > +                error_setg(errp, "PCI: function number %u is not lower than ARI next function number %u",
> > +                           pci_dev->devfn & 0xff, nextfn);
> > +                pci_qdev_unrealize(DEVICE(pci_dev));
> > +                return;
> > +            }
> > +        }
> > +    }
> > +
> 
> So I kind of got lost in all the patches. What was the ultimate decision regarding checking this?

We still need to fix ARI for multi-function PFs.
I feel the right thing to do is to init Next Function in the ARI
capability, automatically.
For now, we have merely changed ARI setting next function to 0.
At least that's more correct for the common case of ARI PF with VFs.


> >     if (pci_dev->failover_pair_id) {
> >         if (!pci_bus_is_express(pci_get_bus(pci_dev))) {
> >             error_setg(errp, "failover primary device must be on "
> > -- 
> > 2.41.0
> >