docs/pcie_sriov.txt | 8 ++- include/hw/pci/pci.h | 2 +- include/hw/pci/pci_device.h | 19 +++++- include/hw/pci/pcie_sriov.h | 9 +-- include/hw/qdev-core.h | 4 -- hw/core/qdev.c | 1 - hw/net/igb.c | 13 +++- hw/nvme/ctrl.c | 24 ++++--- hw/pci/pci.c | 23 ++++--- hw/pci/pci_host.c | 4 +- hw/pci/pcie_sriov.c | 153 +++++++++++++++++++++++--------------------- hw/ppc/spapr_pci.c | 8 ++- hw/s390x/s390-pci-bus.c | 14 ++-- hw/vfio/pci.c | 5 +- system/qdev-monitor.c | 12 ++-- hw/pci/trace-events | 2 +- 16 files changed, 175 insertions(+), 126 deletions(-)
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>
---
Changes in v15:
- Fixed variable shadowing in patch
"pcie_sriov: Remove num_vfs from PCIESriovPF"
- Link to v14: https://lore.kernel.org/r/20240813-reuse-v14-0-4c15bc6ee0e6@daynix.com
Changes in v14:
- Dropped patch "pcie_sriov: Ensure VF function number does not
overflow" as I found the restriction it imposes is unnecessary.
- Link to v13: https://lore.kernel.org/r/20240805-reuse-v13-0-aaeaa4d7dfd2@daynix.com
Changes in v13:
- Added patch "s390x/pci: Check for multifunction after device
realization". I found SR-IOV devices, which are multifunction devices,
are not supposed to work at all with s390x on QEMU.
- Link to v12: https://lore.kernel.org/r/20240804-reuse-v12-0-d3930c4111b2@daynix.com
Changes in v12:
- Changed to ignore invalid PCI_SRIOV_NUM_VF writes as done for
PCI_SRIOV_CTRL_VFE.
- Updated the message for patch "hw/pci: Use -1 as the default value for
rombar". (Markus Armbruster)
- Link to v11: https://lore.kernel.org/r/20240802-reuse-v11-0-fb83bb8c19fb@daynix.com
Changes in v11:
- Rebased.
- Dropped patch "hw/pci: Convert rom_bar into OnOffAuto".
- Added patch "hw/pci: Use -1 as the default value for rombar".
- Added for-9.2 to give a testing period for possible breakage with
libvirt/s390x.
- Link to v10: https://lore.kernel.org/r/20240627-reuse-v10-0-7ca0b8ed3d9f@daynix.com
Changes in v10:
- Added patch "hw/ppc/spapr_pci: Do not reject VFs created after a PF".
- Added patch "hw/ppc/spapr_pci: Do not create DT for disabled PCI device".
- Added patch "hw/pci: Convert rom_bar into OnOffAuto".
- Dropped patch "hw/pci: Determine if rombar is explicitly enabled".
- Dropped patch "hw/qdev: Remove opts member".
- Link to v9: https://lore.kernel.org/r/20240315-reuse-v9-0-67aa69af4d53@daynix.com
Changes in v9:
- Rebased.
- Restored '#include "qapi/error.h"' (Michael S. Tsirkin)
- Added patch "pcie_sriov: Ensure VF function number does not overflow"
to fix abortion with wrong PF addr.
- Link to v8: https://lore.kernel.org/r/20240228-reuse-v8-0-282660281e60@daynix.com
Changes in v8:
- Clarified that "hw/pci: Replace -1 with UINT32_MAX for romsize" is
not a bug fix. (Markus Armbruster)
- Squashed patch "vfio: Avoid inspecting option QDict for rombar" into
"hw/pci: Determine if rombar is explicitly enabled".
(Markus Armbruster)
- Noted the minor semantics change for patch "hw/pci: Determine if
rombar is explicitly enabled". (Markus Armbruster)
- Link to v7: https://lore.kernel.org/r/20240224-reuse-v7-0-29c14bcb952e@daynix.com
Changes in v7:
- Replaced -1 with UINT32_MAX when expressing uint32_t.
(Markus Armbruster)
- Added patch "hw/pci: Replace -1 with UINT32_MAX for romsize".
- Link to v6: https://lore.kernel.org/r/20240220-reuse-v6-0-2e42a28b0cf2@daynix.com
Changes in v6:
- Fixed migration.
- Added patch "pcie_sriov: Do not manually unrealize".
- Restored patch "pcie_sriov: Release VFs failed to realize" that was
missed in v5.
- Link to v5: https://lore.kernel.org/r/20240218-reuse-v5-0-e4fc1c19b5a9@daynix.com
Changes in v5:
- Added patch "hw/pci: Always call pcie_sriov_pf_reset()".
- Added patch "pcie_sriov: Reset SR-IOV extended capability".
- Removed a reference to PCI_SRIOV_CTRL_VFE in hw/nvme.
(Michael S. Tsirkin)
- Noted the impact on the guest of patch "pcie_sriov: Do not reset
NumVFs after unregistering VFs". (Michael S. Tsirkin)
- Changed to use pcie_sriov_num_vfs().
- Restored pci_set_power() and changed it to call pci_set_enabled() only
for PFs with an expalanation. (Michael S. Tsirkin)
- Reordered patches.
- Link to v4: https://lore.kernel.org/r/20240214-reuse-v4-0-89ad093a07f4@daynix.com
Changes in v4:
- Reverted the change to pci_rom_bar_explicitly_enabled().
(Michael S. Tsirkin)
- Added patch "pcie_sriov: Do not reset NumVFs after unregistering VFs".
- Added patch "hw/nvme: Refer to dev->exp.sriov_pf.num_vfs".
- Link to v3: https://lore.kernel.org/r/20240212-reuse-v3-0-8017b689ce7f@daynix.com
Changes in v3:
- Extracted patch "hw/pci: Use -1 as a default value for rombar" from
patch "hw/pci: Determine if rombar is explicitly enabled"
(Philippe Mathieu-Daudé)
- Added an audit result of PCIDevice::rom_bar to the message of patch
"hw/pci: Use -1 as a default value for rombar"
(Philippe Mathieu-Daudé)
- Link to v2: https://lore.kernel.org/r/20240210-reuse-v2-0-24ba2a502692@daynix.com
Changes in v2:
- Reset after enabling a function so that NVMe VF state gets updated.
- Link to v1: https://lore.kernel.org/r/20240203-reuse-v1-0-5be8c5ce6338@daynix.com
---
Akihiko Odaki (11):
hw/pci: Rename has_power to enabled
hw/ppc/spapr_pci: Do not create DT for disabled PCI device
hw/ppc/spapr_pci: Do not reject VFs created after a PF
s390x/pci: Check for multifunction after device realization
pcie_sriov: Do not manually unrealize
pcie_sriov: Reuse SR-IOV VF device instances
pcie_sriov: Release VFs failed to realize
pcie_sriov: Remove num_vfs from PCIESriovPF
pcie_sriov: Register VFs after migration
hw/pci: Use -1 as the default value for rombar
hw/qdev: Remove opts member
docs/pcie_sriov.txt | 8 ++-
include/hw/pci/pci.h | 2 +-
include/hw/pci/pci_device.h | 19 +++++-
include/hw/pci/pcie_sriov.h | 9 +--
include/hw/qdev-core.h | 4 --
hw/core/qdev.c | 1 -
hw/net/igb.c | 13 +++-
hw/nvme/ctrl.c | 24 ++++---
hw/pci/pci.c | 23 ++++---
hw/pci/pci_host.c | 4 +-
hw/pci/pcie_sriov.c | 153 +++++++++++++++++++++++---------------------
hw/ppc/spapr_pci.c | 8 ++-
hw/s390x/s390-pci-bus.c | 14 ++--
hw/vfio/pci.c | 5 +-
system/qdev-monitor.c | 12 ++--
hw/pci/trace-events | 2 +-
16 files changed, 175 insertions(+), 126 deletions(-)
---
base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
change-id: 20240129-reuse-faae22b11934
Best regards,
--
Akihiko Odaki <akihiko.odaki@daynix.com>
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?
> ---
> Changes in v15:
> - Fixed variable shadowing in patch
> "pcie_sriov: Remove num_vfs from PCIESriovPF"
> - Link to v14: https://lore.kernel.org/r/20240813-reuse-v14-0-4c15bc6ee0e6@daynix.com
>
> Changes in v14:
> - Dropped patch "pcie_sriov: Ensure VF function number does not
> overflow" as I found the restriction it imposes is unnecessary.
> - Link to v13: https://lore.kernel.org/r/20240805-reuse-v13-0-aaeaa4d7dfd2@daynix.com
>
> Changes in v13:
> - Added patch "s390x/pci: Check for multifunction after device
> realization". I found SR-IOV devices, which are multifunction devices,
> are not supposed to work at all with s390x on QEMU.
> - Link to v12: https://lore.kernel.org/r/20240804-reuse-v12-0-d3930c4111b2@daynix.com
>
> Changes in v12:
> - Changed to ignore invalid PCI_SRIOV_NUM_VF writes as done for
> PCI_SRIOV_CTRL_VFE.
> - Updated the message for patch "hw/pci: Use -1 as the default value for
> rombar". (Markus Armbruster)
> - Link to v11: https://lore.kernel.org/r/20240802-reuse-v11-0-fb83bb8c19fb@daynix.com
>
> Changes in v11:
> - Rebased.
> - Dropped patch "hw/pci: Convert rom_bar into OnOffAuto".
> - Added patch "hw/pci: Use -1 as the default value for rombar".
> - Added for-9.2 to give a testing period for possible breakage with
> libvirt/s390x.
> - Link to v10: https://lore.kernel.org/r/20240627-reuse-v10-0-7ca0b8ed3d9f@daynix.com
>
> Changes in v10:
> - Added patch "hw/ppc/spapr_pci: Do not reject VFs created after a PF".
> - Added patch "hw/ppc/spapr_pci: Do not create DT for disabled PCI device".
> - Added patch "hw/pci: Convert rom_bar into OnOffAuto".
> - Dropped patch "hw/pci: Determine if rombar is explicitly enabled".
> - Dropped patch "hw/qdev: Remove opts member".
> - Link to v9: https://lore.kernel.org/r/20240315-reuse-v9-0-67aa69af4d53@daynix.com
>
> Changes in v9:
> - Rebased.
> - Restored '#include "qapi/error.h"' (Michael S. Tsirkin)
> - Added patch "pcie_sriov: Ensure VF function number does not overflow"
> to fix abortion with wrong PF addr.
> - Link to v8: https://lore.kernel.org/r/20240228-reuse-v8-0-282660281e60@daynix.com
>
> Changes in v8:
> - Clarified that "hw/pci: Replace -1 with UINT32_MAX for romsize" is
> not a bug fix. (Markus Armbruster)
> - Squashed patch "vfio: Avoid inspecting option QDict for rombar" into
> "hw/pci: Determine if rombar is explicitly enabled".
> (Markus Armbruster)
> - Noted the minor semantics change for patch "hw/pci: Determine if
> rombar is explicitly enabled". (Markus Armbruster)
> - Link to v7: https://lore.kernel.org/r/20240224-reuse-v7-0-29c14bcb952e@daynix.com
>
> Changes in v7:
> - Replaced -1 with UINT32_MAX when expressing uint32_t.
> (Markus Armbruster)
> - Added patch "hw/pci: Replace -1 with UINT32_MAX for romsize".
> - Link to v6: https://lore.kernel.org/r/20240220-reuse-v6-0-2e42a28b0cf2@daynix.com
>
> Changes in v6:
> - Fixed migration.
> - Added patch "pcie_sriov: Do not manually unrealize".
> - Restored patch "pcie_sriov: Release VFs failed to realize" that was
> missed in v5.
> - Link to v5: https://lore.kernel.org/r/20240218-reuse-v5-0-e4fc1c19b5a9@daynix.com
>
> Changes in v5:
> - Added patch "hw/pci: Always call pcie_sriov_pf_reset()".
> - Added patch "pcie_sriov: Reset SR-IOV extended capability".
> - Removed a reference to PCI_SRIOV_CTRL_VFE in hw/nvme.
> (Michael S. Tsirkin)
> - Noted the impact on the guest of patch "pcie_sriov: Do not reset
> NumVFs after unregistering VFs". (Michael S. Tsirkin)
> - Changed to use pcie_sriov_num_vfs().
> - Restored pci_set_power() and changed it to call pci_set_enabled() only
> for PFs with an expalanation. (Michael S. Tsirkin)
> - Reordered patches.
> - Link to v4: https://lore.kernel.org/r/20240214-reuse-v4-0-89ad093a07f4@daynix.com
>
> Changes in v4:
> - Reverted the change to pci_rom_bar_explicitly_enabled().
> (Michael S. Tsirkin)
> - Added patch "pcie_sriov: Do not reset NumVFs after unregistering VFs".
> - Added patch "hw/nvme: Refer to dev->exp.sriov_pf.num_vfs".
> - Link to v3: https://lore.kernel.org/r/20240212-reuse-v3-0-8017b689ce7f@daynix.com
>
> Changes in v3:
> - Extracted patch "hw/pci: Use -1 as a default value for rombar" from
> patch "hw/pci: Determine if rombar is explicitly enabled"
> (Philippe Mathieu-Daudé)
> - Added an audit result of PCIDevice::rom_bar to the message of patch
> "hw/pci: Use -1 as a default value for rombar"
> (Philippe Mathieu-Daudé)
> - Link to v2: https://lore.kernel.org/r/20240210-reuse-v2-0-24ba2a502692@daynix.com
>
> Changes in v2:
> - Reset after enabling a function so that NVMe VF state gets updated.
> - Link to v1: https://lore.kernel.org/r/20240203-reuse-v1-0-5be8c5ce6338@daynix.com
>
> ---
> Akihiko Odaki (11):
> hw/pci: Rename has_power to enabled
> hw/ppc/spapr_pci: Do not create DT for disabled PCI device
> hw/ppc/spapr_pci: Do not reject VFs created after a PF
> s390x/pci: Check for multifunction after device realization
> pcie_sriov: Do not manually unrealize
> pcie_sriov: Reuse SR-IOV VF device instances
> pcie_sriov: Release VFs failed to realize
> pcie_sriov: Remove num_vfs from PCIESriovPF
> pcie_sriov: Register VFs after migration
> hw/pci: Use -1 as the default value for rombar
> hw/qdev: Remove opts member
>
> docs/pcie_sriov.txt | 8 ++-
> include/hw/pci/pci.h | 2 +-
> include/hw/pci/pci_device.h | 19 +++++-
> include/hw/pci/pcie_sriov.h | 9 +--
> include/hw/qdev-core.h | 4 --
> hw/core/qdev.c | 1 -
> hw/net/igb.c | 13 +++-
> hw/nvme/ctrl.c | 24 ++++---
> hw/pci/pci.c | 23 ++++---
> hw/pci/pci_host.c | 4 +-
> hw/pci/pcie_sriov.c | 153 +++++++++++++++++++++++---------------------
> hw/ppc/spapr_pci.c | 8 ++-
> hw/s390x/s390-pci-bus.c | 14 ++--
> hw/vfio/pci.c | 5 +-
> system/qdev-monitor.c | 12 ++--
> hw/pci/trace-events | 2 +-
> 16 files changed, 175 insertions(+), 126 deletions(-)
> ---
> base-commit: 31669121a01a14732f57c49400bc239cf9fd505f
> change-id: 20240129-reuse-faae22b11934
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>
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.
Regards,
Akihiko Odaki
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.
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?
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
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.
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.
--
MST
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
>> 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.
I just cc'ed the s390x PCI maintainers on patch 4. Let's see.
Thanks,
C.
On Wed, Sep 11, 2024 at 12:05:46PM +0900, Akihiko Odaki wrote:
> 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.
The fact is, QEMU already does what most users want from it. So the
first rule whenever adding a new feature is not breaking old
functionality. I know, it's annoying, as some of it is held together by
duct tape. We have a friendly community so if you ask nicely, you
usually can get help. You'd probably have to be a bit more specific
with your questions.
> Let's keep the restriction until then.
>
> Regards,
> Akihiko Odaki
Not introducing regressions is a hard rule, sorry.
--
MST
On Tue, Sep 10, 2024 at 06:33:01PM +0900, 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.
>
> Regards,
> Akihiko Odaki
Cédric would appreciate your Tested-by/Reviewed-by.
--
MST
© 2016 - 2024 Red Hat, Inc.