Re: [PATCH v4 8/9] pcie_sriov: Do not reset NumVFs after unregistering VFs

[Akihiko Odaki]

On 2024/02/15 0:51, Michael S. Tsirkin wrote:
> On Wed, Feb 14, 2024 at 11:32:11PM +0900, Akihiko Odaki wrote:
> > On 2024/02/14 15:53, Michael S. Tsirkin wrote:
> > > On Wed, Feb 14, 2024 at 02:13:46PM +0900, Akihiko Odaki wrote:
> > > > I couldn't find such a behavior specified.
> > >
> > > Is it fixing a bug or just removing unnecessary code?
> > > Is this guest visible at all?
> >
> > My intention is just to remove unnecessary code, but it is guest-visible.
> > The original behavior causes a problem and it should be considered as a bug
> > fix if a guest expects VFs can be restored by setting VF Enable after
> > clearing it, but I don't know such an example.
>
> Ah ok.  So:
>
>         According to the spec, PCI_SRIOV_NUM_VF isn't reset when
>         VFs are disabled. Clearing it is guest visible and out of spec,
>         even though guests mostly don't rely on this value being
>         preserved, so we never noticed.
>
> I wonder however what happens on reset.
> How come we don't have a reset callback for sriov cap?
> I suspect this hack serves as a work around to avoid leaking
> this register, and we really should fix that too here?

pcie_sriov_pf_disable_vfs() is meant to serve for that purpose, but it 
just disable VFs and does not update registers. I'll change it to update 
registers and rename it to pcie_sriov_pf_reset().

> > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > > ---
> > > >    hw/pci/pcie_sriov.c | 1 -
> > > >    1 file changed, 1 deletion(-)
> > > >
> > > > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> > > > index 9d668b8d6c17..410bc090fc58 100644
> > > > --- a/hw/pci/pcie_sriov.c
> > > > +++ b/hw/pci/pcie_sriov.c
> > > > @@ -209,7 +209,6 @@ static void unregister_vfs(PCIDevice *dev)
> > > >            pci_set_enabled(dev->exp.sriov_pf.vf[i], false);
> > > >        }
> > > >        dev->exp.sriov_pf.num_vfs = 0;
> > > > -    pci_set_word(dev->config + dev->exp.sriov_cap + PCI_SRIOV_NUM_VF, 0);
> > > >    }
> > > >    void pcie_sriov_config_write(PCIDevice *dev, uint32_t address,
> > > >
> > > > --
> > > > 2.43.0