On Wed, Sep 11, 2024 at 07:58:15PM +0900, Akihiko Odaki wrote:
> On 2024/09/11 18:38, Cédric Le Goater wrote:
> > +Matthew +Eric
> >
> > Side note for the maintainers :
> >
> > Before this change, the igb device, which is multifunction, was working
> > fine under Linux.
> >
> > Was there a fix in Linux since :
> >
> > 57da367b9ec4 ("s390x/pci: forbid multifunction pci device")
> > 6069bcdeacee ("s390x/pci: Move some hotplug checks to the pre_plug
> > handler")
> >
> > ?
> >
> > s390 PCI devices do not have extended capabilities, so the igb device
> > does not expose the SRIOV capability and only the PF is accessible but
> > it doesn't seem to be an issue. (Btw, CONFIG_PCI_IOV is set to y in the
> > default Linux config which is unexpected)
>
> Doesn't s390x really see extended capabilities? hw/s390x/s390-pci-inst.c has
> a call pci_config_size() and pci_host_config_write_common(), which means it
> is exposing the whole PCI Express configuration space. Why can't s390x use
> extended capabilities then?
>
> The best option for fix would be to replace the SR-IOV implementation with
> stub if s390x cannot use the SR-IOV capability. However I still need to know
> at what level I should change the implementation (e.g., is it fine to remove
> the entire capability, or should I keep the capability while writes to its
> registers no-op?)
>
> Regards,
> Akihiko Odaki
Note changing caps needs compat hacks for cross version migration to work.
> >
> > Thanks,
> >
> > C.
> >
> >
> >
> > On 8/23/24 07:00, Akihiko Odaki wrote:
> > > The SR-IOV PFs set the multifunction bits during device realization so
> > > check them after that. This forbids adding SR-IOV devices to s390x.
> > >
> > > Signed-off-by: Akihiko Odaki <[email protected]>
> > > ---
> > > hw/s390x/s390-pci-bus.c | 14 ++++++--------
> > > 1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> > > index 3e57d5faca18..00b2c1f6157b 100644
> > > --- a/hw/s390x/s390-pci-bus.c
> > > +++ b/hw/s390x/s390-pci-bus.c
> > > @@ -971,14 +971,7 @@ static void
> > > s390_pcihost_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
> > > "this device");
> > > }
> > > - if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> > > - PCIDevice *pdev = PCI_DEVICE(dev);
> > > -
> > > - if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> > > - error_setg(errp, "multifunction not supported in s390");
> > > - return;
> > > - }
> > > - } else if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
> > > + if (object_dynamic_cast(OBJECT(dev), TYPE_S390_PCI_DEVICE)) {
> > > S390PCIBusDevice *pbdev = S390_PCI_DEVICE(dev);
> > > if (!s390_pci_alloc_idx(s, pbdev)) {
> > > @@ -1069,6 +1062,11 @@ static void s390_pcihost_plug(HotplugHandler
> > > *hotplug_dev, DeviceState *dev,
> > > } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> > > pdev = PCI_DEVICE(dev);
> > > + if (pdev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION) {
> > > + error_setg(errp, "multifunction not supported in s390");
> > > + return;
> > > + }
> > > +
> > > if (!dev->id) {
> > > /* In the case the PCI device does not define an id */
> > > /* we generate one based on the PCI address */
> > >
> >
