1. Patchew
  2. QEMU
  3. [PATCH v7 0/6] test and QEMU fixes to ensure proper PCIE device usage
  4. View patch

[PATCH v7 6/6] hw/pci: add comment explaining the reason for checking function 0 in hotplug

Ani Sinha posted 6 patches 2 years, 7 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Thomas Huth <thuth@redhat.com>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
There is a newer version of this series
[PATCH v7 6/6] hw/pci: add comment explaining the reason for checking function 0 in hotplug
Posted by Ani Sinha 2 years, 7 months ago 
This change is cosmetic. A comment is added explaining why we need to check for
the availability of function 0 when we hotplug a device.

CC: mst@redhat.com
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 hw/pci/pci.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 47517ba3db..e3ff3808b6 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1181,9 +1181,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
                    PCI_SLOT(devfn), PCI_FUNC(devfn), name,
                    bus->devices[devfn]->name, bus->devices[devfn]->qdev.id);
         return NULL;
-    } else if (dev->hotplugged &&
-               !pci_is_vf(pci_dev) &&
-               pci_get_function_0(pci_dev)) {
+    } /*
+       * Populating function 0 triggers a scan from the guest that
+       * exposes other non-zero functions. Hence we need to ensure that
+       * function 0 wasn't added yet. With SRIOV and with or without ARI
+       * the PF must be hotplugged into function 0 for it to be detected.
+       */
+    else if (dev->hotplugged &&
+             !pci_is_vf(pci_dev) &&
+             pci_get_function_0(pci_dev)) {
         error_setg(errp, "PCI: slot %d function 0 already occupied by %s,"
                    " new func %s cannot be exposed to guest.",
                    PCI_SLOT(pci_get_function_0(pci_dev)->devfn),
-- 
2.39.1
Re: [PATCH v7 6/6] hw/pci: add comment explaining the reason for checking function 0 in hotplug
Posted by Igor Mammedov 2 years, 7 months ago 
On Tue,  4 Jul 2023 16:55:55 +0530
Ani Sinha <anisinha@redhat.com> wrote:

> This change is cosmetic. A comment is added explaining why we need to check for
> the availability of function 0 when we hotplug a device.
> 
> CC: mst@redhat.com
> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> ---
>  hw/pci/pci.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 47517ba3db..e3ff3808b6 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -1181,9 +1181,15 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev,
>                     PCI_SLOT(devfn), PCI_FUNC(devfn), name,
>                     bus->devices[devfn]->name, bus->devices[devfn]->qdev.id);
>          return NULL;
> -    } else if (dev->hotplugged &&
> -               !pci_is_vf(pci_dev) &&
> -               pci_get_function_0(pci_dev)) {
> +    } /*
> +       * Populating function 0 triggers a scan from the guest that
> +       * exposes other non-zero functions. Hence we need to ensure that
> +       * function 0 wasn't added yet.

> With SRIOV and with or without ARI
> +       * the PF must be hotplugged into function 0 for it to be detected.
Wouldn't the same apply to non-SR-IOV devices as well?

> +       */
> +    else if (dev->hotplugged &&
> +             !pci_is_vf(pci_dev) &&
> +             pci_get_function_0(pci_dev)) {
>          error_setg(errp, "PCI: slot %d function 0 already occupied by %s,"
>                     " new func %s cannot be exposed to guest.",
>                     PCI_SLOT(pci_get_function_0(pci_dev)->devfn),
Re: [PATCH v7 6/6] hw/pci: add comment explaining the reason for checking function 0 in hotplug
Posted by Ani Sinha 2 years, 7 months ago 
On Tue, 4 Jul, 2023, 5:45 pm Igor Mammedov, <imammedo@redhat.com> wrote:

> On Tue,  4 Jul 2023 16:55:55 +0530
> Ani Sinha <anisinha@redhat.com> wrote:
>
> > This change is cosmetic. A comment is added explaining why we need to
> check for
> > the availability of function 0 when we hotplug a device.
> >
> > CC: mst@redhat.com
> > Signed-off-by: Ani Sinha <anisinha@redhat.com>
> > ---
> >  hw/pci/pci.c | 12 +++++++++---
> >  1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 47517ba3db..e3ff3808b6 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -1181,9 +1181,15 @@ static PCIDevice
> *do_pci_register_device(PCIDevice *pci_dev,
> >                     PCI_SLOT(devfn), PCI_FUNC(devfn), name,
> >                     bus->devices[devfn]->name, bus->devices[devfn]->
> qdev.id);
> >          return NULL;
> > -    } else if (dev->hotplugged &&
> > -               !pci_is_vf(pci_dev) &&
> > -               pci_get_function_0(pci_dev)) {
> > +    } /*
> > +       * Populating function 0 triggers a scan from the guest that
> > +       * exposes other non-zero functions. Hence we need to ensure that
> > +       * function 0 wasn't added yet.
>
> > With SRIOV and with or without ARI
> > +       * the PF must be hotplugged into function 0 for it to be
> detected.
> Wouldn't the same apply to non-SR-IOV devices as well?
>

I was trying to emphasize PFs and SRIOV. But may be it adds more confusion
and better left out.

>
> > +       */
> > +    else if (dev->hotplugged &&
> > +             !pci_is_vf(pci_dev) &&
> > +             pci_get_function_0(pci_dev)) {
> >          error_setg(errp, "PCI: slot %d function 0 already occupied by
> %s,"
> >                     " new func %s cannot be exposed to guest.",
> >                     PCI_SLOT(pci_get_function_0(pci_dev)->devfn),
>
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.