Re: [PATCH for-9.2 v15 00/11] hw/pci: SR-IOV related fixes and improvements

[Akihiko Odaki]

On 2024/09/11 0:27, Michael S. Tsirkin wrote:
> On Tue, Sep 10, 2024 at 04:13:14PM +0200, Cédric Le Goater wrote:
> > On 9/10/24 15:34, Michael S. Tsirkin wrote:
> > > On Tue, Sep 10, 2024 at 03:21:54PM +0200, Cédric Le Goater wrote:
> > > > On 9/10/24 11:33, Akihiko Odaki wrote:
> > > > > On 2024/09/10 18:21, Michael S. Tsirkin wrote:
> > > > > > On Fri, Aug 23, 2024 at 02:00:37PM +0900, Akihiko Odaki wrote:
> > > > > > > Supersedes: <20240714-rombar-v2-0-af1504ef55de@daynix.com>
> > > > > > > ("[PATCH v2 0/4] hw/pci: Convert rom_bar into OnOffAuto")
> > > > > > >
> > > > > > > I submitted a RFC series[1] to add support for SR-IOV emulation to
> > > > > > > virtio-net-pci. During the development of the series, I fixed some
> > > > > > > trivial bugs and made improvements that I think are independently
> > > > > > > useful. This series extracts those fixes and improvements from the RFC
> > > > > > > series.
> > > > > > >
> > > > > > > [1]: https://patchew.org/QEMU/20231210-sriov-v2-0-b959e8a6dfaf@daynix.com/
> > > > > > >
> > > > > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> > > > > > I don't think Cédric's issues have been addressed, am I wrong?
> > > > > > Cédric, what is your take?
> > > > > I put the URI to Cédric's report here:
> > > > > https://lore.kernel.org/r/75cbc7d9-b48e-4235-85cf-49dacf3c7483@redhat.com
> > > > >
> > > > > This issue was dealt with patch "s390x/pci: Check for multifunction after device 
> > > > > realization". I found that s390x on QEMU does not support multifunction and SR-IOV 
> > > > > devices accidentally circumvent this restriction, which means igb was never supposed to 
> > > > > work with s390x. The patch prevents adding SR-IOV devices to s390x to ensure the 
> > > > > restriction is properly enforced.
> > > > yes, indeed and it seems to fix :
> > > >
> > > >     6069bcdeacee ("s390x/pci: Move some hotplug checks to the pre_plug handler")
> > > >
> > > > I will update patch 4.
> > > >
> > > >
> > > > Thanks,
> > > >
> > > > C.
> > > >
> > > >
> > > > That said, the igb device worked perfectly fine under the s390x machine.
> > > And it works for you after this patchset, yes?
> > ah no, IGB is not an available device for the s390x machine anymore :
> >
> >    qemu-system-s390x: -device igb,netdev=net1,mac=C0:FF:EE:00:00:13: 
> > multifunction not supported in s390
>
> So patch 4 didn't relly help.
>
>
> > This is what commit 57da367b9ec4 ("s390x/pci: forbid multifunction
> > pci device") initially required (and later broken by 6069bcdeacee).
> > So I guess we are fine with the expected behavior.
> >
> > Thanks,
> >
> > C.
> Better to fix than to guess if there are users, I think.
Yes, but it will require some knowledge of s390x, which I cannot provide.

Commit 57da367b9ec4 ("s390x/pci: forbid multifunction pci device") says 
having a multifunction device will make the guest spin forever. That is 
not what Cédric observed with igb so it may no longer be relevant, but I 
cannot be sure that the problem is resolved without understanding how 
multifunction devices are supposed to work with s390x.
Ideally someone with s390x expertise should check relevant hardware 
documentation and confirm QEMU properly implements mutlifunction 
devices. Let's keep the restriction until then.
Regards,
Akihiko Odaki