mbox

[PULL,00/51] Net patches

Message ID 20230307070816.34833-1-jasowang@redhat.com (mailing list archive)
State New, archived
Headers show

Pull-request

https://github.com/jasowang/qemu.git tags/net-pull-request

Message

Jason Wang March 7, 2023, 7:07 a.m. UTC
The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:

  Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)

are available in the git repository at:

  https://github.com/jasowang/qemu.git tags/net-pull-request

for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:

  hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)

----------------------------------------------------------------

----------------------------------------------------------------
Akihiko Odaki (43):
      e1000e: Fix the code style
      hw/net: Add more MII definitions
      fsl_etsec: Use hw/net/mii.h
      e1000: Use hw/net/mii.h
      e1000: Mask registers when writing
      e1000e: Introduce E1000E_LOW_BITS_SET_FUNC
      e1000e: Mask registers when writing
      e1000: Use more constant definitions
      e1000e: Use more constant definitions
      e1000: Use memcpy to intialize registers
      e1000e: Use memcpy to intialize registers
      e1000e: Remove pending interrupt flags
      e1000e: Improve software reset
      e1000: Configure ResettableClass
      e1000e: Configure ResettableClass
      e1000e: Introduce e1000_rx_desc_union
      e1000e: Set MII_ANER_NWAY
      e1000e: Remove extra pointer indirection
      net: Check L4 header size
      e1000x: Alter the signature of e1000x_is_vlan_packet
      net: Strip virtio-net header when dumping
      hw/net/net_tx_pkt: Automatically determine if virtio-net header is used
      hw/net/net_rx_pkt: Remove net_rx_pkt_has_virt_hdr
      e1000e: Perform software segmentation for loopback
      hw/net/net_tx_pkt: Implement TCP segmentation
      hw/net/net_tx_pkt: Check the payload length
      e1000e: Do not assert when MSI-X is disabled later
      MAINTAINERS: Add Akihiko Odaki as a e1000e reviewer
      MAINTAINERS: Add e1000e test files
      e1000e: Combine rx traces
      e1000: Count CRC in Tx statistics
      e1000e: Count CRC in Tx statistics
      net/eth: Report if headers are actually present
      e1000e: Implement system clock
      net/eth: Introduce EthL4HdrProto
      pcie: Introduce pcie_sriov_num_vfs
      e1000: Split header files
      Intrdocue igb device emulation
      tests/qtest/e1000e-test: Fabricate ethernet header
      tests/qtest/libqos/e1000e: Export macreg functions
      igb: Introduce qtest for igb device
      tests/avocado: Add igb test
      docs/system/devices/igb: Add igb documentation

Philippe Mathieu-Daudé (7):
      hw/net/eepro100: Abort if pci_add_capability() ever fail
      hw/net/eepro100: Introduce TYPE_EEPRO100 QOM abstract parent
      hw/net/eepro100: Convert reset handler to DeviceReset
      hw/net/eepro100: Pass E100PCIDeviceInfo as class init data
      hw/net/eepro100: Remove instance EEPRO100State::has_extended_tcb_support
      hw/net/eepro100: Remove instance's EEPRO100State::device
      hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100()

Shreesh Adiga (1):
      ebpf: fix compatibility with libbpf 1.0+

 MAINTAINERS                                        |   13 +
 docs/system/device-emulation.rst                   |    1 +
 docs/system/devices/igb.rst                        |   71 +
 ebpf/rss.bpf.skeleton.h                            | 1171 ++++--
 hw/core/machine.c                                  |    1 +
 hw/net/Kconfig                                     |    5 +
 hw/net/e1000.c                                     |  259 +-
 hw/net/e1000_common.h                              |  102 +
 hw/net/e1000_regs.h                                |  958 +----
 hw/net/e1000e.c                                    |  102 +-
 hw/net/e1000e_core.c                               |  719 ++--
 hw/net/e1000e_core.h                               |   70 +-
 hw/net/e1000x_common.c                             |   38 +-
 hw/net/e1000x_common.h                             |  133 +-
 hw/net/e1000x_regs.h                               |  967 +++++
 hw/net/eepro100.c                                  |  149 +-
 hw/net/fsl_etsec/etsec.c                           |   11 +-
 hw/net/fsl_etsec/etsec.h                           |   17 -
 hw/net/fsl_etsec/miim.c                            |    5 +-
 hw/net/igb.c                                       |  615 +++
 hw/net/igb_common.h                                |  146 +
 hw/net/igb_core.c                                  | 4077 ++++++++++++++++++++
 hw/net/igb_core.h                                  |  146 +
 hw/net/igb_regs.h                                  |  648 ++++
 hw/net/igbvf.c                                     |  327 ++
 hw/net/meson.build                                 |    2 +
 hw/net/net_rx_pkt.c                                |  102 +-
 hw/net/net_rx_pkt.h                                |   31 +-
 hw/net/net_tx_pkt.c                                |  332 +-
 hw/net/net_tx_pkt.h                                |   27 +-
 hw/net/trace-events                                |   50 +-
 hw/net/virtio-net.c                                |   85 +-
 hw/net/vmxnet3.c                                   |   58 +-
 hw/pci/pcie_sriov.c                                |    5 +
 include/hw/net/mii.h                               |   14 +-
 include/hw/pci/pcie_sriov.h                        |    3 +
 include/net/eth.h                                  |   15 +-
 include/net/net.h                                  |    6 +
 net/dump.c                                         |   11 +-
 net/eth.c                                          |  118 +-
 net/net.c                                          |   18 +
 net/tap.c                                          |   16 +
 scripts/ci/org.centos/stream/8/x86_64/test-avocado |    1 +
 tests/avocado/igb.py                               |   38 +
 tests/qtest/e1000e-test.c                          |   25 +-
 tests/qtest/fuzz/generic_fuzz_configs.h            |    5 +
 tests/qtest/igb-test.c                             |  243 ++
 tests/qtest/libqos/e1000e.c                        |   12 -
 tests/qtest/libqos/e1000e.h                        |   14 +
 tests/qtest/libqos/igb.c                           |  185 +
 tests/qtest/libqos/meson.build                     |    1 +
 tests/qtest/meson.build                            |    1 +
 tools/ebpf/Makefile.ebpf                           |    8 +-
 tools/ebpf/rss.bpf.c                               |   43 +-
 54 files changed, 9829 insertions(+), 2391 deletions(-)
 create mode 100644 docs/system/devices/igb.rst
 create mode 100644 hw/net/e1000_common.h
 create mode 100644 hw/net/e1000x_regs.h
 create mode 100644 hw/net/igb.c
 create mode 100644 hw/net/igb_common.h
 create mode 100644 hw/net/igb_core.c
 create mode 100644 hw/net/igb_core.h
 create mode 100644 hw/net/igb_regs.h
 create mode 100644 hw/net/igbvf.c
 create mode 100644 tests/avocado/igb.py
 create mode 100644 tests/qtest/igb-test.c
 create mode 100644 tests/qtest/libqos/igb.c

Comments

Peter Maydell March 7, 2023, 5:01 p.m. UTC | #1
On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
>
> The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
>
>   Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
>
> are available in the git repository at:
>
>   https://github.com/jasowang/qemu.git tags/net-pull-request
>
> for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
>
>   hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
>
> ----------------------------------------------------------------
>
> ----------------------------------------------------------------

Fails to build on Windows:
https://gitlab.com/qemu-project/qemu/-/jobs/3889073853
https://gitlab.com/qemu-project/qemu/-/jobs/3889073855
https://gitlab.com/qemu-project/qemu/-/jobs/3889073780
https://gitlab.com/qemu-project/qemu/-/jobs/3889073778

../tests/qtest/igb-test.c: In function 'data_test_init':
../tests/qtest/igb-test.c:219:15: error: implicit declaration of
function 'socketpair'; did you mean 'socket_uri'?
[-Werror=implicit-function-declaration]
219 | int ret = socketpair(PF_UNIX, SOCK_STREAM, 0, test_sockets);
| ^~~~~~~~~~
| socket_uri
../tests/qtest/igb-test.c:219:15: error: nested extern declaration of
'socketpair' [-Werror=nested-externs]

build-oss-fuzz failed on something involving fuzzing eepro100:
https://gitlab.com/qemu-project/qemu/-/jobs/3889073821

The 'crash-test' jobs failed with an assertion
"qemu-system-i386: -device igb: MSI-X is not supported by interrupt controller":
https://gitlab.com/qemu-project/qemu/-/jobs/3889073868
https://gitlab.com/qemu-project/qemu/-/jobs/3889073873

thanks
-- PMM
Philippe Mathieu-Daudé March 7, 2023, 8:43 p.m. UTC | #2
On 7/3/23 18:01, Peter Maydell wrote:
> On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
>>
>> The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
>>
>>    Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
>>
>> are available in the git repository at:
>>
>>    https://github.com/jasowang/qemu.git tags/net-pull-request
>>
>> for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
>>
>>    hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
>>
>> ----------------------------------------------------------------

> build-oss-fuzz failed on something involving fuzzing eepro100:
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
Jason, please drop my patches. I'll look at that failure.
Jason Wang March 8, 2023, 6:56 a.m. UTC | #3
On Wed, Mar 8, 2023 at 4:43 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> On 7/3/23 18:01, Peter Maydell wrote:
> > On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
> >>
> >> The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
> >>
> >>    Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
> >>
> >> are available in the git repository at:
> >>
> >>    https://github.com/jasowang/qemu.git tags/net-pull-request
> >>
> >> for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
> >>
> >>    hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
> >>
> >> ----------------------------------------------------------------
>
> > build-oss-fuzz failed on something involving fuzzing eepro100:
> > https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
> Jason, please drop my patches. I'll look at that failure.

Ok.

Thanks

>
Jason Wang March 8, 2023, 6:57 a.m. UTC | #4
On Wed, Mar 8, 2023 at 1:01 AM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
> >
> > The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
> >
> >   Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
> >
> > are available in the git repository at:
> >
> >   https://github.com/jasowang/qemu.git tags/net-pull-request
> >
> > for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
> >
> >   hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
> >
> > ----------------------------------------------------------------
> >
> > ----------------------------------------------------------------
>
> Fails to build on Windows:
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073853
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073855
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073780
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073778
>
> ../tests/qtest/igb-test.c: In function 'data_test_init':
> ../tests/qtest/igb-test.c:219:15: error: implicit declaration of
> function 'socketpair'; did you mean 'socket_uri'?
> [-Werror=implicit-function-declaration]
> 219 | int ret = socketpair(PF_UNIX, SOCK_STREAM, 0, test_sockets);
> | ^~~~~~~~~~
> | socket_uri
> ../tests/qtest/igb-test.c:219:15: error: nested extern declaration of
> 'socketpair' [-Werror=nested-externs]

Will add ifndef to make sure windows won't try to compile the related test.

>
> build-oss-fuzz failed on something involving fuzzing eepro100:
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
>
> The 'crash-test' jobs failed with an assertion
> "qemu-system-i386: -device igb: MSI-X is not supported by interrupt controller":
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073868
> https://gitlab.com/qemu-project/qemu/-/jobs/3889073873

This is because we use error_abort for msix/msi_init(). I think we can
gracefully fail in those cases.

Will send a new version of pull request.

Thanks

>
> thanks
> -- PMM
>
Philippe Mathieu-Daudé March 8, 2023, 7:40 a.m. UTC | #5
On 8/3/23 07:56, Jason Wang wrote:
> On Wed, Mar 8, 2023 at 4:43 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> On 7/3/23 18:01, Peter Maydell wrote:
>>> On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
>>>>
>>>> The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
>>>>
>>>>     Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
>>>>
>>>> are available in the git repository at:
>>>>
>>>>     https://github.com/jasowang/qemu.git tags/net-pull-request
>>>>
>>>> for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
>>>>
>>>>     hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
>>>>
>>>> ----------------------------------------------------------------
>>
>>> build-oss-fuzz failed on something involving fuzzing eepro100:
>>> https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
>> Jason, please drop my patches. I'll look at that failure.

Before "hw/net/eepro100: Convert reset handler to DeviceReset",
e100_pci_reset() is only called once from DeviceRealize _before_
the device is realized.

After, 1/ it can be called multiple times and 2/ it seems to do
stuffs that really belong to DeviceRealize (should be called once),
in particular pci_add_capability().

I *think* it should be illegal to call pci_add_capability() _after_
a device is realized. Auditing pci_add_capability(), there is only
one other use before realize: amdvi_sysbus_realize() in
hw/i386/amd_iommu.c.
Michael S. Tsirkin March 8, 2023, 12:17 p.m. UTC | #6
On Wed, Mar 08, 2023 at 08:40:42AM +0100, Philippe Mathieu-Daudé wrote:
> On 8/3/23 07:56, Jason Wang wrote:
> > On Wed, Mar 8, 2023 at 4:43 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > > 
> > > On 7/3/23 18:01, Peter Maydell wrote:
> > > > On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
> > > > > 
> > > > > The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
> > > > > 
> > > > >     Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
> > > > > 
> > > > > are available in the git repository at:
> > > > > 
> > > > >     https://github.com/jasowang/qemu.git tags/net-pull-request
> > > > > 
> > > > > for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
> > > > > 
> > > > >     hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
> > > > > 
> > > > > ----------------------------------------------------------------
> > > 
> > > > build-oss-fuzz failed on something involving fuzzing eepro100:
> > > > https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
> > > Jason, please drop my patches. I'll look at that failure.
> 
> Before "hw/net/eepro100: Convert reset handler to DeviceReset",
> e100_pci_reset() is only called once from DeviceRealize _before_
> the device is realized.
> 
> After, 1/ it can be called multiple times and 2/ it seems to do
> stuffs that really belong to DeviceRealize (should be called once),
> in particular pci_add_capability().
> 
> I *think* it should be illegal to call pci_add_capability() _after_
> a device is realized. Auditing pci_add_capability(), there is only
> one other use before realize: amdvi_sysbus_realize() in
> hw/i386/amd_iommu.c.


Calling pci_add_capability when VM is running is likely to confuse
guests, yes.
Philippe Mathieu-Daudé March 8, 2023, 12:21 p.m. UTC | #7
On 8/3/23 13:17, Michael S. Tsirkin wrote:
> On Wed, Mar 08, 2023 at 08:40:42AM +0100, Philippe Mathieu-Daudé wrote:
>> On 8/3/23 07:56, Jason Wang wrote:
>>> On Wed, Mar 8, 2023 at 4:43 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>>>
>>>> On 7/3/23 18:01, Peter Maydell wrote:
>>>>> On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
>>>>>>
>>>>>> The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
>>>>>>
>>>>>>      Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
>>>>>>
>>>>>> are available in the git repository at:
>>>>>>
>>>>>>      https://github.com/jasowang/qemu.git tags/net-pull-request
>>>>>>
>>>>>> for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
>>>>>>
>>>>>>      hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
>>>>>>
>>>>>> ----------------------------------------------------------------
>>>>
>>>>> build-oss-fuzz failed on something involving fuzzing eepro100:
>>>>> https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
>>>> Jason, please drop my patches. I'll look at that failure.
>>
>> Before "hw/net/eepro100: Convert reset handler to DeviceReset",
>> e100_pci_reset() is only called once from DeviceRealize _before_
>> the device is realized.
>>
>> After, 1/ it can be called multiple times and 2/ it seems to do
>> stuffs that really belong to DeviceRealize (should be called once),
>> in particular pci_add_capability().
>>
>> I *think* it should be illegal to call pci_add_capability() _after_
>> a device is realized. Auditing pci_add_capability(), there is only
>> one other use before realize: amdvi_sysbus_realize() in
>> hw/i386/amd_iommu.c.
> 
> Calling pci_add_capability when VM is running is likely to confuse
> guests, yes.

Thanks for confirming. Similar pattern: msi_init().

While trying to fix that in hw/i386/amd_iommu.c I realized this device
isn't in a good shape, almost unmaintained: 2 bugfixes in since 7 years,
the other commits are generic API cleanups. I'll post the series and
we can discuss that there.
Michael S. Tsirkin March 8, 2023, 12:25 p.m. UTC | #8
On Wed, Mar 08, 2023 at 01:21:52PM +0100, Philippe Mathieu-Daudé wrote:
> On 8/3/23 13:17, Michael S. Tsirkin wrote:
> > On Wed, Mar 08, 2023 at 08:40:42AM +0100, Philippe Mathieu-Daudé wrote:
> > > On 8/3/23 07:56, Jason Wang wrote:
> > > > On Wed, Mar 8, 2023 at 4:43 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > > > > 
> > > > > On 7/3/23 18:01, Peter Maydell wrote:
> > > > > > On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
> > > > > > > 
> > > > > > > The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
> > > > > > > 
> > > > > > >      Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
> > > > > > > 
> > > > > > > are available in the git repository at:
> > > > > > > 
> > > > > > >      https://github.com/jasowang/qemu.git tags/net-pull-request
> > > > > > > 
> > > > > > > for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
> > > > > > > 
> > > > > > >      hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
> > > > > > > 
> > > > > > > ----------------------------------------------------------------
> > > > > 
> > > > > > build-oss-fuzz failed on something involving fuzzing eepro100:
> > > > > > https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
> > > > > Jason, please drop my patches. I'll look at that failure.
> > > 
> > > Before "hw/net/eepro100: Convert reset handler to DeviceReset",
> > > e100_pci_reset() is only called once from DeviceRealize _before_
> > > the device is realized.
> > > 
> > > After, 1/ it can be called multiple times and 2/ it seems to do
> > > stuffs that really belong to DeviceRealize (should be called once),
> > > in particular pci_add_capability().
> > > 
> > > I *think* it should be illegal to call pci_add_capability() _after_
> > > a device is realized. Auditing pci_add_capability(), there is only
> > > one other use before realize: amdvi_sysbus_realize() in
> > > hw/i386/amd_iommu.c.
> > 
> > Calling pci_add_capability when VM is running is likely to confuse
> > guests, yes.
> 
> Thanks for confirming. Similar pattern: msi_init().
> 
> While trying to fix that in hw/i386/amd_iommu.c I realized this device
> isn't in a good shape, almost unmaintained: 2 bugfixes in since 7 years,
> the other commits are generic API cleanups. I'll post the series and
> we can discuss that there.

Absolutely. A mix of VTD or for that matter virtio iommu and AMD CPUs
seems to work well enough that most people don't bother.
I vaguely remember spec review showed some things were hard
to support correctly with shadowing, but I don't remember
the detail (and our shadowing with VTD only works because
it matches what drivers are doing).
Jason Wang March 9, 2023, 1:13 a.m. UTC | #9
On Wed, Mar 8, 2023 at 8:25 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Mar 08, 2023 at 01:21:52PM +0100, Philippe Mathieu-Daudé wrote:
> > On 8/3/23 13:17, Michael S. Tsirkin wrote:
> > > On Wed, Mar 08, 2023 at 08:40:42AM +0100, Philippe Mathieu-Daudé wrote:
> > > > On 8/3/23 07:56, Jason Wang wrote:
> > > > > On Wed, Mar 8, 2023 at 4:43 AM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> > > > > >
> > > > > > On 7/3/23 18:01, Peter Maydell wrote:
> > > > > > > On Tue, 7 Mar 2023 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
> > > > > > > >
> > > > > > > > The following changes since commit 817fd33836e73812df2f1907612b57750fcb9491:
> > > > > > > >
> > > > > > > >      Merge tag 'audio-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2023-03-06 14:06:06 +0000)
> > > > > > > >
> > > > > > > > are available in the git repository at:
> > > > > > > >
> > > > > > > >      https://github.com/jasowang/qemu.git tags/net-pull-request
> > > > > > > >
> > > > > > > > for you to fetch changes up to c19b566a3898510ec2b3e881b3fb78614b240414:
> > > > > > > >
> > > > > > > >      hw/net/eepro100: Replace DO_UPCAST(EEPRO100State) by EEPRO100() (2023-03-07 14:55:39 +0800)
> > > > > > > >
> > > > > > > > ----------------------------------------------------------------
> > > > > >
> > > > > > > build-oss-fuzz failed on something involving fuzzing eepro100:
> > > > > > > https://gitlab.com/qemu-project/qemu/-/jobs/3889073821
> > > > > > Jason, please drop my patches. I'll look at that failure.
> > > >
> > > > Before "hw/net/eepro100: Convert reset handler to DeviceReset",
> > > > e100_pci_reset() is only called once from DeviceRealize _before_
> > > > the device is realized.
> > > >
> > > > After, 1/ it can be called multiple times and 2/ it seems to do
> > > > stuffs that really belong to DeviceRealize (should be called once),
> > > > in particular pci_add_capability().
> > > >
> > > > I *think* it should be illegal to call pci_add_capability() _after_
> > > > a device is realized. Auditing pci_add_capability(), there is only
> > > > one other use before realize: amdvi_sysbus_realize() in
> > > > hw/i386/amd_iommu.c.
> > >
> > > Calling pci_add_capability when VM is running is likely to confuse
> > > guests, yes.
> >
> > Thanks for confirming. Similar pattern: msi_init().
> >
> > While trying to fix that in hw/i386/amd_iommu.c I realized this device
> > isn't in a good shape, almost unmaintained: 2 bugfixes in since 7 years,
> > the other commits are generic API cleanups.

Last time I tried AMD vIOMMU it didn't even boot. We need to check if
anyone can maintain that driver (adding Wei and Peter).

> I'll post the series and
> > we can discuss that there.
>
> Absolutely. A mix of VTD or for that matter virtio iommu and AMD CPUs
> seems to work well enough that most people don't bother.
> I vaguely remember spec review showed some things were hard
> to support correctly with shadowing, but I don't remember
> the detail

Something like caching mode otherwise we need to trap the page table
modification via userfaultfd?

> (and our shadowing with VTD only works because
> it matches what drivers are doing).

I think not, VTD has a caching mode that is designed to be friendly
for virtualization/emulation (mentioned in the spec). But it would be
replaced by hardware accelerated one soon.

Thanks

>
> --
> MST
>