Re: [PATCH v19 09/14] pcie_sriov: Reuse SR-IOV VF device instances

[Akihiko Odaki]

On 2025/01/09 19:51, Philippe Mathieu-Daudé wrote:
> Hi Akihiko,
>
> On 9/1/25 07:29, Akihiko Odaki wrote:
> > Disable SR-IOV VF devices by reusing code to power down PCI devices
> > instead of removing them when the guest requests to disable VFs. This
> > allows to realize devices and report VF realization errors at PF
> > realization time.
> >
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > ---
> >   include/hw/pci/pcie_sriov.h |  1 -
> >   hw/pci/pci.c                | 14 ++++++-
> >   hw/pci/pcie_sriov.c         | 94 ++++++++++++++++++ 
> > +--------------------------
> >   3 files changed, 51 insertions(+), 58 deletions(-)
>
>
> >   void pci_set_enabled(PCIDevice *d, bool state)
> > @@ -2977,7 +2987,7 @@ void pci_set_enabled(PCIDevice *d, bool state)
> >       memory_region_set_enabled(&d->bus_master_enable_region,
> >                                 (pci_get_word(d->config + PCI_COMMAND)
> >                                  & PCI_COMMAND_MASTER) && d->enabled);
> > -    if (!d->enabled) {
> > +    if (d->qdev.realized) {
>
> I'm not a big fan of accessing an internal field that way, without
> accessor. Besides, we use this field atomicly within qdev.c.

I changed it to use qdev_is_realized() with v20.

> >           pci_device_reset(d);
>
> Anyhow, resetting an unrealized device is a bug IMO.

Right. Calling pci_set_enabled() for an unrealized device to set the 
initial value is fine. This code ensures it will not trigger resetting 
the device.

> >       }
> >   }