Re: [PATCH v10 00/10]vhost-vdpa: add support for configure interrupt

[Jason Wang]

On Mon, Jan 10, 2022 at 9:37 AM Cindy Lu <lulu@redhat.com> wrote:
>
>
>
>
> On Mon, Jan 10, 2022 at 1:56 AM Michael S. Tsirkin <mst@redhat.com> wrote:
>>
>> On Fri, Nov 05, 2021 at 12:48:17AM +0800, Cindy Lu wrote:
>> > these patches add the support for configure interrupt
>> >
>> > These codes are all tested in vp-vdpa (support configure interrupt)
>> > vdpa_sim (not support configure interrupt), virtio tap device
>> >
>> > test in virtio-pci bus and virtio-mmio bus
>> >
>> > Change in v2:
>> > Add support for virtio-mmio bus
>> > active the notifier while the backend support configure interrupt
>> > misc fixes from v1
>> >
>> > Change in v3
>> > fix the coding style problems
>> >
>> > Change in v4
>> > misc fixes from v3
>> > merge the set_config_notifier to set_guest_notifier
>> > when vdpa start, check the feature by VIRTIO_NET_F_STATUS
>> >
>> > Change in v5
>> > misc fixes from v4
>> > split the code to introduce configure interrupt type and the callback 
>> > function
>> > will init the configure interrupt in all virtio-pci and virtio-mmio bus, 
>> > but will
>> > only active while using vhost-vdpa driver
>> >
>> > Change in v6
>> > misc fixes from v5
>> > decouple vq from interrupt setting and misc process
>> > fix the bug in virtio_net_handle_rx
>> > use -1 as the queue number to identify if the interrupt is configure 
>> > interrupt
>> >
>> > Change in v7
>> > misc fixes from v6
>> > decouple vq from interrupt setting and misc process
>> > decouple vq from vector use/release process
>> > decouple vq from set notifier fd handler process
>> > move config_notifier and masked_config_notifier to VirtIODevice
>> > fix the bug in virtio_net_handle_rx, add more information
>> > add VIRTIO_CONFIG_IRQ_IDX as the queue number to identify if the interrupt 
>> > is configure interrupt
>> >
>> > Change in v8
>> > misc fixes from v7
>> > decouple vq from interrupt setting and misc process
>> > decouple vq from vector use/release process
>> > decouple vq from set notifier fd handler process
>> > move the vhost configure interrupt to vhost_net
>> >
>> > Change in v9
>> > misc fixes from v8
>> > address the comments from v8
>> >
>> > Change in v10
>> > fix the hang issue in qtest
>> > address the comments from v9
>> >
>> > Cindy Lu (10):
>> >   virtio: introduce macro IRTIO_CONFIG_IRQ_IDX
>> >   virtio-pci: decouple notifier from interrupt process
>> >   virtio-pci: decouple the single vector from the interrupt process
>> >   vhost: introduce new VhostOps vhost_set_config_call
>> >   vhost-vdpa: add support for config interrupt
>> >   virtio: add support for configure interrupt
>> >   vhost: add support for configure interrupt
>> >   virtio-net: add support for configure interrupt
>> >   virtio-mmio: add support for configure interrupt
>> >   virtio-pci: add support for configure interrupt
>> >
>> >  hw/display/vhost-user-gpu.c       |   6 +
>> >  hw/net/vhost_net.c                |   9 +
>> >  hw/net/virtio-net.c               |  10 +-
>> >  hw/virtio/trace-events            |   1 +
>> >  hw/virtio/vhost-user-fs.c         |   6 +
>> >  hw/virtio/vhost-vdpa.c            |   7 +
>> >  hw/virtio/vhost-vsock-common.c    |   6 +
>> >  hw/virtio/vhost.c                 |  76 +++++++++
>> >  hw/virtio/virtio-crypto.c         |   6 +
>> >  hw/virtio/virtio-mmio.c           |  27 +++
>> >  hw/virtio/virtio-pci.c            | 269 +++++++++++++++++++++---------
>> >  hw/virtio/virtio-pci.h            |   4 +-
>> >  hw/virtio/virtio.c                |  29 ++++
>> >  include/hw/virtio/vhost-backend.h |   3 +
>> >  include/hw/virtio/vhost.h         |   4 +
>> >  include/hw/virtio/virtio.h        |   7 +
>> >  include/net/vhost_net.h           |   2 +
>> >  17 files changed, 389 insertions(+), 83 deletions(-)
>>
>> So I just realized something. The spec says:
>>
>> The device MUST set the Device Configuration Interrupt bit
>> in \field{ISR status} before sending a device configuration
>> change notification to the driver.
>>
>> and I don't see how these patches achieve this: it requires
>> that config interrupts go through userspace.
>>
>> Revert, and think more about it? Or did I miss something?
>>
>  Thanks, Micheal, I'm ok to revert these patchs and there are also several 
> bugs  I need to fix.
> I will post a new version soon

Please test with vectors=0 for the new version to make it work as expected.

Thanks

>>
>>
>> > --
>> > 2.21.3
>>