mbox series

[GIT,PULL] virtio: features

Message ID 20230903181338-mutt-send-email-mst@kernel.org (mailing list archive)
State New, archived
Headers show
Series [GIT,PULL] virtio: features | expand

Pull-request

https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

Message

Michael S. Tsirkin Sept. 3, 2023, 10:13 p.m. UTC
The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:

  Linux 6.5 (2023-08-27 14:49:51 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:

  virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)

----------------------------------------------------------------
virtio: features

a small pull request this time around, mostly because the
vduse network got postponed to next relase so we can be sure
we got the security store right.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Eugenio Pérez (4):
      vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
      vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
      vdpa: add get_backend_features vdpa operation
      vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK

Jason Wang (1):
      virtio_vdpa: build affinity masks conditionally

Xuan Zhuo (12):
      virtio_ring: check use_dma_api before unmap desc for indirect
      virtio_ring: put mapping error check in vring_map_one_sg
      virtio_ring: introduce virtqueue_set_dma_premapped()
      virtio_ring: support add premapped buf
      virtio_ring: introduce virtqueue_dma_dev()
      virtio_ring: skip unmap for premapped
      virtio_ring: correct the expression of the description of virtqueue_resize()
      virtio_ring: separate the logic of reset/enable from virtqueue_resize
      virtio_ring: introduce virtqueue_reset()
      virtio_ring: introduce dma map api for virtqueue
      virtio_ring: introduce dma sync api for virtqueue
      virtio_net: merge dma operations when filling mergeable buffers

Yuan Yao (1):
      virtio_ring: fix avail_wrap_counter in virtqueue_add_packed

Yue Haibing (1):
      vdpa/mlx5: Remove unused function declarations

 drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
 drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
 drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
 drivers/vhost/vdpa.c               |  15 +-
 drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
 drivers/virtio/virtio_vdpa.c       |  17 +-
 include/linux/vdpa.h               |   4 +
 include/linux/virtio.h             |  22 ++
 include/uapi/linux/vhost_types.h   |   4 +
 9 files changed, 625 insertions(+), 90 deletions(-)

Comments

pr-tracker-bot@kernel.org Sept. 4, 2023, 6:43 p.m. UTC | #1
The pull request you sent on Sun, 3 Sep 2023 18:13:38 -0400:

> https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e4f1b8202fb59c56a3de7642d50326923670513f

Thank you!
Michael Roth Sept. 26, 2023, 1:04 p.m. UTC | #2
On Sun, Sep 03, 2023 at 06:13:38PM -0400, Michael S. Tsirkin wrote:
> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> 
>   Linux 6.5 (2023-08-27 14:49:51 -0700)
> 
> are available in the Git repository at:
> 
>   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> 
> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> 
>   virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> 
> ----------------------------------------------------------------
> virtio: features
> 
> a small pull request this time around, mostly because the
> vduse network got postponed to next relase so we can be sure
> we got the security store right.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ----------------------------------------------------------------
> Eugenio Pérez (4):
>       vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
>       vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
>       vdpa: add get_backend_features vdpa operation
>       vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> 
> Jason Wang (1):
>       virtio_vdpa: build affinity masks conditionally
> 
> Xuan Zhuo (12):
>       virtio_ring: check use_dma_api before unmap desc for indirect
>       virtio_ring: put mapping error check in vring_map_one_sg
>       virtio_ring: introduce virtqueue_set_dma_premapped()
>       virtio_ring: support add premapped buf
>       virtio_ring: introduce virtqueue_dma_dev()
>       virtio_ring: skip unmap for premapped
>       virtio_ring: correct the expression of the description of virtqueue_resize()
>       virtio_ring: separate the logic of reset/enable from virtqueue_resize
>       virtio_ring: introduce virtqueue_reset()
>       virtio_ring: introduce dma map api for virtqueue
>       virtio_ring: introduce dma sync api for virtqueue
>       virtio_net: merge dma operations when filling mergeable buffers

This ^ patch (upstream commit 295525e29a) seems to cause a
network-related regression when using SWIOTLB in the guest. I noticed
this initially testing SEV guests, which use SWIOTLB by default, but
it can also be seen with normal guests when forcing SWIOTLB via
swiotlb=force kernel cmdline option. I see it with both 6.6-rc1 and
6.6-rc2 (haven't tried rc3 yet, but don't see any related changes
there), and reverting 714073495f seems to avoid the issue.

Steps to reproduce:

1) Boot QEMU/KVM guest with 6.6-rc2 with swiotlb=force via something like the following cmdline:

   qemu-system-x86_64 \
   -machine q35 -smp 4,maxcpus=255 -cpu EPYC-Milan-v2 \
   -enable-kvm -m 16G,slots=5,maxmem=256G -vga none \
   -device virtio-scsi-pci,id=scsi0,disable-legacy=on,iommu_platform=true \
   -drive file=/home/mroth/storage/ubuntu-18.04-seves2.qcow2,if=none,id=drive0,snapshot=off \
   -device scsi-hd,id=hd0,drive=drive0,bus=scsi0.0 \
   -device virtio-net-pci,netdev=netdev0,id=net0,disable-legacy=on,iommu_platform=true,romfile= \
   -netdev tap,script=/home/mroth/qemu-ifup,id=netdev0 \
   -L /home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu \
   -drive if=pflash,format=raw,unit=0,file=/home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu/OVMF_CODE.fd,readonly \
   -drive if=pflash,format=raw,unit=1,file=/home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu/OVMF_VARS.fd \
   -debugcon file:debug.log -global isa-debugcon.iobase=0x402 -msg timestamp=on \
   -kernel /boot/vmlinuz-6.6.0-rc2-vanilla0+ \
   -initrd /boot/initrd.img-6.6.0-rc2-vanilla0+ \
   -append "root=UUID=d72a6d1c-06cf-4b79-af43-f1bac4f620f9 ro console=ttyS0,115200n8 earlyprintk=serial,ttyS0,115200 debug=1 sev=debug page_poison=0 spec_rstack_overflow=off swiotlb=force"

2) scp a small file from the host to the guest IP via its virtio-net device.
   Smaller file sizes succeed, but the larger the file the more likely
   it will fail. e.g.:

   mroth@host:~$ dd if=/dev/zero of=test bs=1K count=19
   19+0 records in
   19+0 records out
   19456 bytes (19 kB, 19 KiB) copied, 0.000940134 s, 20.7 MB/s
   mroth@host:~$ scp test vm0:
   test                                                                    100%   19KB  10.1MB/s   00:00
   mroth@host:~$ dd if=/dev/zero of=test bs=1K count=20
   20+0 records in
   20+0 records out
   20480 bytes (20 kB, 20 KiB) copied, 0.00093774 s, 21.8 MB/s
   mroth@host:~$ scp test vm0:
   test                                                                      0%    0     0.0KB/s   --:-- ETA
   client_loop: send disconnect: Broken pipe
   lost connection
   mroth@host:~$

Thanks,

Mike

> 
> Yuan Yao (1):
>       virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
> 
> Yue Haibing (1):
>       vdpa/mlx5: Remove unused function declarations
> 
>  drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
>  drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
>  drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
>  drivers/vhost/vdpa.c               |  15 +-
>  drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
>  drivers/virtio/virtio_vdpa.c       |  17 +-
>  include/linux/vdpa.h               |   4 +
>  include/linux/virtio.h             |  22 ++
>  include/uapi/linux/vhost_types.h   |   4 +
>  9 files changed, 625 insertions(+), 90 deletions(-)
>
Xuan Zhuo Sept. 27, 2023, 1:47 a.m. UTC | #3
On Tue, 26 Sep 2023 08:04:51 -0500, Michael Roth <michael.roth@amd.com> wrote:
> On Sun, Sep 03, 2023 at 06:13:38PM -0400, Michael S. Tsirkin wrote:
> > The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> >
> >   Linux 6.5 (2023-08-27 14:49:51 -0700)
> >
> > are available in the Git repository at:
> >
> >   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> >
> > for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> >
> >   virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> >
> > ----------------------------------------------------------------
> > virtio: features
> >
> > a small pull request this time around, mostly because the
> > vduse network got postponed to next relase so we can be sure
> > we got the security store right.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ----------------------------------------------------------------
> > Eugenio P閞ez (4):
> >       vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
> >       vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
> >       vdpa: add get_backend_features vdpa operation
> >       vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> >
> > Jason Wang (1):
> >       virtio_vdpa: build affinity masks conditionally
> >
> > Xuan Zhuo (12):
> >       virtio_ring: check use_dma_api before unmap desc for indirect
> >       virtio_ring: put mapping error check in vring_map_one_sg
> >       virtio_ring: introduce virtqueue_set_dma_premapped()
> >       virtio_ring: support add premapped buf
> >       virtio_ring: introduce virtqueue_dma_dev()
> >       virtio_ring: skip unmap for premapped
> >       virtio_ring: correct the expression of the description of virtqueue_resize()
> >       virtio_ring: separate the logic of reset/enable from virtqueue_resize
> >       virtio_ring: introduce virtqueue_reset()
> >       virtio_ring: introduce dma map api for virtqueue
> >       virtio_ring: introduce dma sync api for virtqueue
> >       virtio_net: merge dma operations when filling mergeable buffers
>
> This ^ patch (upstream commit 295525e29a) seems to cause a
> network-related regression when using SWIOTLB in the guest. I noticed
> this initially testing SEV guests, which use SWIOTLB by default, but
> it can also be seen with normal guests when forcing SWIOTLB via
> swiotlb=force kernel cmdline option. I see it with both 6.6-rc1 and
> 6.6-rc2 (haven't tried rc3 yet, but don't see any related changes
> there), and reverting 714073495f seems to avoid the issue.
>
> Steps to reproduce:
>
> 1) Boot QEMU/KVM guest with 6.6-rc2 with swiotlb=force via something like the following cmdline:
>
>    qemu-system-x86_64 \
>    -machine q35 -smp 4,maxcpus=255 -cpu EPYC-Milan-v2 \
>    -enable-kvm -m 16G,slots=5,maxmem=256G -vga none \
>    -device virtio-scsi-pci,id=scsi0,disable-legacy=on,iommu_platform=true \
>    -drive file=/home/mroth/storage/ubuntu-18.04-seves2.qcow2,if=none,id=drive0,snapshot=off \
>    -device scsi-hd,id=hd0,drive=drive0,bus=scsi0.0 \
>    -device virtio-net-pci,netdev=netdev0,id=net0,disable-legacy=on,iommu_platform=true,romfile= \
>    -netdev tap,script=/home/mroth/qemu-ifup,id=netdev0 \
>    -L /home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu \
>    -drive if=pflash,format=raw,unit=0,file=/home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu/OVMF_CODE.fd,readonly \
>    -drive if=pflash,format=raw,unit=1,file=/home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu/OVMF_VARS.fd \
>    -debugcon file:debug.log -global isa-debugcon.iobase=0x402 -msg timestamp=on \
>    -kernel /boot/vmlinuz-6.6.0-rc2-vanilla0+ \
>    -initrd /boot/initrd.img-6.6.0-rc2-vanilla0+ \
>    -append "root=UUID=d72a6d1c-06cf-4b79-af43-f1bac4f620f9 ro console=ttyS0,115200n8 earlyprintk=serial,ttyS0,115200 debug=1 sev=debug page_poison=0 spec_rstack_overflow=off swiotlb=force"
>
> 2) scp a small file from the host to the guest IP via its virtio-net device.
>    Smaller file sizes succeed, but the larger the file the more likely
>    it will fail. e.g.:
>
>    mroth@host:~$ dd if=/dev/zero of=test bs=1K count=19
>    19+0 records in
>    19+0 records out
>    19456 bytes (19 kB, 19 KiB) copied, 0.000940134 s, 20.7 MB/s
>    mroth@host:~$ scp test vm0:
>    test                                                                    100%   19KB  10.1MB/s   00:00
>    mroth@host:~$ dd if=/dev/zero of=test bs=1K count=20
>    20+0 records in
>    20+0 records out
>    20480 bytes (20 kB, 20 KiB) copied, 0.00093774 s, 21.8 MB/s
>    mroth@host:~$ scp test vm0:
>    test                                                                      0%    0     0.0KB/s   --:-- ETA
>    client_loop: send disconnect: Broken pipe
>    lost connection
>    mroth@host:~$


Hi Michael,

Thanks for the report.

Cloud you try this fix?  I reproduce this issue, and that works for me.

Thanks.


diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 98dc9b49d56b..9ece27dc5144 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -589,16 +589,16 @@ static void virtnet_rq_unmap(struct receive_queue *rq, void *buf, u32 len)

        --dma->ref;

-       if (dma->ref) {
-               if (dma->need_sync && len) {
-                       offset = buf - (head + sizeof(*dma));
+       if (dma->need_sync && len) {
+               offset = buf - (head + sizeof(*dma));

-                       virtqueue_dma_sync_single_range_for_cpu(rq->vq, dma->addr, offset,
-                                                               len, DMA_FROM_DEVICE);
-               }
+               virtqueue_dma_sync_single_range_for_cpu(rq->vq, dma->addr,
+                                                       offset, len,
+                                                       DMA_FROM_DEVICE);
+       }

+       if (dma->ref)
                return;
-       }

        virtqueue_dma_unmap_single_attrs(rq->vq, dma->addr, dma->len,
                                         DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);


>
> Thanks,
>
> Mike
>
> >
> > Yuan Yao (1):
> >       virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
> >
> > Yue Haibing (1):
> >       vdpa/mlx5: Remove unused function declarations
> >
> >  drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
> >  drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
> >  drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
> >  drivers/vhost/vdpa.c               |  15 +-
> >  drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
> >  drivers/virtio/virtio_vdpa.c       |  17 +-
> >  include/linux/vdpa.h               |   4 +
> >  include/linux/virtio.h             |  22 ++
> >  include/uapi/linux/vhost_types.h   |   4 +
> >  9 files changed, 625 insertions(+), 90 deletions(-)
> >
Michael Roth Sept. 27, 2023, 5:28 a.m. UTC | #4
On Wed, Sep 27, 2023 at 09:47:39AM +0800, Xuan Zhuo wrote:
> On Tue, 26 Sep 2023 08:04:51 -0500, Michael Roth <michael.roth@amd.com> wrote:
> > On Sun, Sep 03, 2023 at 06:13:38PM -0400, Michael S. Tsirkin wrote:
> > > The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> > >
> > >   Linux 6.5 (2023-08-27 14:49:51 -0700)
> > >
> > > are available in the Git repository at:
> > >
> > >   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> > >
> > > for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> > >
> > >   virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> > >
> > > ----------------------------------------------------------------
> > > virtio: features
> > >
> > > a small pull request this time around, mostly because the
> > > vduse network got postponed to next relase so we can be sure
> > > we got the security store right.
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > >
> > > ----------------------------------------------------------------
> > > Eugenio P閞ez (4):
> > >       vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
> > >       vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
> > >       vdpa: add get_backend_features vdpa operation
> > >       vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> > >
> > > Jason Wang (1):
> > >       virtio_vdpa: build affinity masks conditionally
> > >
> > > Xuan Zhuo (12):
> > >       virtio_ring: check use_dma_api before unmap desc for indirect
> > >       virtio_ring: put mapping error check in vring_map_one_sg
> > >       virtio_ring: introduce virtqueue_set_dma_premapped()
> > >       virtio_ring: support add premapped buf
> > >       virtio_ring: introduce virtqueue_dma_dev()
> > >       virtio_ring: skip unmap for premapped
> > >       virtio_ring: correct the expression of the description of virtqueue_resize()
> > >       virtio_ring: separate the logic of reset/enable from virtqueue_resize
> > >       virtio_ring: introduce virtqueue_reset()
> > >       virtio_ring: introduce dma map api for virtqueue
> > >       virtio_ring: introduce dma sync api for virtqueue
> > >       virtio_net: merge dma operations when filling mergeable buffers
> >
> > This ^ patch (upstream commit 295525e29a) seems to cause a
> > network-related regression when using SWIOTLB in the guest. I noticed
> > this initially testing SEV guests, which use SWIOTLB by default, but
> > it can also be seen with normal guests when forcing SWIOTLB via
> > swiotlb=force kernel cmdline option. I see it with both 6.6-rc1 and
> > 6.6-rc2 (haven't tried rc3 yet, but don't see any related changes
> > there), and reverting 714073495f seems to avoid the issue.
> >
> > Steps to reproduce:
> >
> > 1) Boot QEMU/KVM guest with 6.6-rc2 with swiotlb=force via something like the following cmdline:
> >
> >    qemu-system-x86_64 \
> >    -machine q35 -smp 4,maxcpus=255 -cpu EPYC-Milan-v2 \
> >    -enable-kvm -m 16G,slots=5,maxmem=256G -vga none \
> >    -device virtio-scsi-pci,id=scsi0,disable-legacy=on,iommu_platform=true \
> >    -drive file=/home/mroth/storage/ubuntu-18.04-seves2.qcow2,if=none,id=drive0,snapshot=off \
> >    -device scsi-hd,id=hd0,drive=drive0,bus=scsi0.0 \
> >    -device virtio-net-pci,netdev=netdev0,id=net0,disable-legacy=on,iommu_platform=true,romfile= \
> >    -netdev tap,script=/home/mroth/qemu-ifup,id=netdev0 \
> >    -L /home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu \
> >    -drive if=pflash,format=raw,unit=0,file=/home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu/OVMF_CODE.fd,readonly \
> >    -drive if=pflash,format=raw,unit=1,file=/home/mroth/storage/AMDSEV2/snp-release-2023-09-23/usr/local/share/qemu/OVMF_VARS.fd \
> >    -debugcon file:debug.log -global isa-debugcon.iobase=0x402 -msg timestamp=on \
> >    -kernel /boot/vmlinuz-6.6.0-rc2-vanilla0+ \
> >    -initrd /boot/initrd.img-6.6.0-rc2-vanilla0+ \
> >    -append "root=UUID=d72a6d1c-06cf-4b79-af43-f1bac4f620f9 ro console=ttyS0,115200n8 earlyprintk=serial,ttyS0,115200 debug=1 sev=debug page_poison=0 spec_rstack_overflow=off swiotlb=force"
> >
> > 2) scp a small file from the host to the guest IP via its virtio-net device.
> >    Smaller file sizes succeed, but the larger the file the more likely
> >    it will fail. e.g.:
> >
> >    mroth@host:~$ dd if=/dev/zero of=test bs=1K count=19
> >    19+0 records in
> >    19+0 records out
> >    19456 bytes (19 kB, 19 KiB) copied, 0.000940134 s, 20.7 MB/s
> >    mroth@host:~$ scp test vm0:
> >    test                                                                    100%   19KB  10.1MB/s   00:00
> >    mroth@host:~$ dd if=/dev/zero of=test bs=1K count=20
> >    20+0 records in
> >    20+0 records out
> >    20480 bytes (20 kB, 20 KiB) copied, 0.00093774 s, 21.8 MB/s
> >    mroth@host:~$ scp test vm0:
> >    test                                                                      0%    0     0.0KB/s   --:-- ETA
> >    client_loop: send disconnect: Broken pipe
> >    lost connection
> >    mroth@host:~$
> 
> 
> Hi Michael,
> 
> Thanks for the report.
> 
> Cloud you try this fix?  I reproduce this issue, and that works for me.

Hello,

This seems to resolve the issue for me.

Thanks for the quick fix.

-Mike

> Thanks.
> 
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 98dc9b49d56b..9ece27dc5144 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -589,16 +589,16 @@ static void virtnet_rq_unmap(struct receive_queue *rq, void *buf, u32 len)
> 
>         --dma->ref;
> 
> -       if (dma->ref) {
> -               if (dma->need_sync && len) {
> -                       offset = buf - (head + sizeof(*dma));
> +       if (dma->need_sync && len) {
> +               offset = buf - (head + sizeof(*dma));
> 
> -                       virtqueue_dma_sync_single_range_for_cpu(rq->vq, dma->addr, offset,
> -                                                               len, DMA_FROM_DEVICE);
> -               }
> +               virtqueue_dma_sync_single_range_for_cpu(rq->vq, dma->addr,
> +                                                       offset, len,
> +                                                       DMA_FROM_DEVICE);
> +       }
> 
> +       if (dma->ref)
>                 return;
> -       }
> 
>         virtqueue_dma_unmap_single_attrs(rq->vq, dma->addr, dma->len,
>                                          DMA_FROM_DEVICE, DMA_ATTR_SKIP_CPU_SYNC);
> 
> 
> >
> > Thanks,
> >
> > Mike
> >
> > >
> > > Yuan Yao (1):
> > >       virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
> > >
> > > Yue Haibing (1):
> > >       vdpa/mlx5: Remove unused function declarations
> > >
> > >  drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
> > >  drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
> > >  drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
> > >  drivers/vhost/vdpa.c               |  15 +-
> > >  drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
> > >  drivers/virtio/virtio_vdpa.c       |  17 +-
> > >  include/linux/vdpa.h               |   4 +
> > >  include/linux/virtio.h             |  22 ++
> > >  include/uapi/linux/vhost_types.h   |   4 +
> > >  9 files changed, 625 insertions(+), 90 deletions(-)
> > >
Thorsten Leemhuis Sept. 27, 2023, 1:18 p.m. UTC | #5
On 26.09.23 15:04, Michael Roth wrote:
> On Sun, Sep 03, 2023 at 06:13:38PM -0400, Michael S. Tsirkin wrote:
>>       virtio_net: merge dma operations when filling mergeable buffers
> 
> This ^ patch (upstream commit 295525e29a) seems to cause a
> network-related regression when using SWIOTLB in the guest. I noticed
> this initially testing SEV guests, which use SWIOTLB by default, but
> it can also be seen with normal guests when forcing SWIOTLB via
> swiotlb=force kernel cmdline option. I see it with both 6.6-rc1 and
> 6.6-rc2 (haven't tried rc3 yet, but don't see any related changes
> there), and reverting 714073495f seems to avoid the issue.
> 
> Steps to reproduce:
> > [...]

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced 295525e29a
#regzbot title virtio: network-related regression when using SWIOTLB in
the guest
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
Thorsten Leemhuis Sept. 29, 2023, 11:12 a.m. UTC | #6
On 27.09.23 15:18, Linux regression tracking #adding (Thorsten Leemhuis)
wrote:
> On 26.09.23 15:04, Michael Roth wrote:
>> On Sun, Sep 03, 2023 at 06:13:38PM -0400, Michael S. Tsirkin wrote:
>>>       virtio_net: merge dma operations when filling mergeable buffers
>>
>> This ^ patch (upstream commit 295525e29a) seems to cause a
>> network-related regression when using SWIOTLB in the guest. I noticed
>> this initially testing SEV guests, which use SWIOTLB by default, but
>> it can also be seen with normal guests when forcing SWIOTLB via
>> swiotlb=force kernel cmdline option. I see it with both 6.6-rc1 and
>> 6.6-rc2 (haven't tried rc3 yet, but don't see any related changes
>> there), and reverting 714073495f seems to avoid the issue.
>>
>> Steps to reproduce:
>>> [...]
> 
> Thanks for the report. To be sure the issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
> tracking bot:
> 
> #regzbot ^introduced 295525e29a
> #regzbot title virtio: network-related regression when using SWIOTLB in
> the guest
> #regzbot ignore-activity

Regzbot missed the fix due to a fluke:

#regzbot monitor:
https://lore.kernel.org/all/20230927055246.121544-1-xuanzhuo@linux.alibaba.com/
#regzbot fix: virtio_net: fix the missing of the dma cpu sync
#regzbot ignore-activity

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
Ning, Hongyu Nov. 29, 2023, 9:03 a.m. UTC | #7
On 2023/9/4 6:13, Michael S. Tsirkin wrote:
> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> 
>    Linux 6.5 (2023-08-27 14:49:51 -0700)
> 
> are available in the Git repository at:
> 
>    https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> 
> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> 
>    virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> 
> ----------------------------------------------------------------
> virtio: features
> 
> a small pull request this time around, mostly because the
> vduse network got postponed to next relase so we can be sure
> we got the security store right.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ----------------------------------------------------------------
> Eugenio Pérez (4):
>        vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
>        vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
>        vdpa: add get_backend_features vdpa operation
>        vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> 
> Jason Wang (1):
>        virtio_vdpa: build affinity masks conditionally
> 
> Xuan Zhuo (12):
>        virtio_ring: check use_dma_api before unmap desc for indirect
>        virtio_ring: put mapping error check in vring_map_one_sg
>        virtio_ring: introduce virtqueue_set_dma_premapped()
>        virtio_ring: support add premapped buf
>        virtio_ring: introduce virtqueue_dma_dev()
>        virtio_ring: skip unmap for premapped
>        virtio_ring: correct the expression of the description of virtqueue_resize()
>        virtio_ring: separate the logic of reset/enable from virtqueue_resize
>        virtio_ring: introduce virtqueue_reset()
>        virtio_ring: introduce dma map api for virtqueue
>        virtio_ring: introduce dma sync api for virtqueue
>        virtio_net: merge dma operations when filling mergeable buffers

Hi,
above patch (upstream commit 295525e29a5b) seems causing a virtnet 
related Call Trace after WARNING from kernel/dma/debug.c.

details (log and test setup) tracked in 
https://bugzilla.kernel.org/show_bug.cgi?id=218204

it's recently noticed in a TDX guest testing since v6.6.0 release cycle 
and can still be reproduced in latest v6.7.0-rc3.

as local bisects results show, above WARNING and Call Trace is linked 
with this patch, do you mind to take a look?

> 
> Yuan Yao (1):
>        virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
> 
> Yue Haibing (1):
>        vdpa/mlx5: Remove unused function declarations
> 
>   drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
>   drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
>   drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
>   drivers/vhost/vdpa.c               |  15 +-
>   drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
>   drivers/virtio/virtio_vdpa.c       |  17 +-
>   include/linux/vdpa.h               |   4 +
>   include/linux/virtio.h             |  22 ++
>   include/uapi/linux/vhost_types.h   |   4 +
>   9 files changed, 625 insertions(+), 90 deletions(-)
>
Jason Wang Nov. 29, 2023, 9:16 a.m. UTC | #8
On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
<hongyu.ning@linux.intel.com> wrote:
>
>
>
> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
> > The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> >
> >    Linux 6.5 (2023-08-27 14:49:51 -0700)
> >
> > are available in the Git repository at:
> >
> >    https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> >
> > for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> >
> >    virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> >
> > ----------------------------------------------------------------
> > virtio: features
> >
> > a small pull request this time around, mostly because the
> > vduse network got postponed to next relase so we can be sure
> > we got the security store right.
> >
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> > ----------------------------------------------------------------
> > Eugenio Pérez (4):
> >        vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
> >        vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
> >        vdpa: add get_backend_features vdpa operation
> >        vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> >
> > Jason Wang (1):
> >        virtio_vdpa: build affinity masks conditionally
> >
> > Xuan Zhuo (12):
> >        virtio_ring: check use_dma_api before unmap desc for indirect
> >        virtio_ring: put mapping error check in vring_map_one_sg
> >        virtio_ring: introduce virtqueue_set_dma_premapped()
> >        virtio_ring: support add premapped buf
> >        virtio_ring: introduce virtqueue_dma_dev()
> >        virtio_ring: skip unmap for premapped
> >        virtio_ring: correct the expression of the description of virtqueue_resize()
> >        virtio_ring: separate the logic of reset/enable from virtqueue_resize
> >        virtio_ring: introduce virtqueue_reset()
> >        virtio_ring: introduce dma map api for virtqueue
> >        virtio_ring: introduce dma sync api for virtqueue
> >        virtio_net: merge dma operations when filling mergeable buffers
>
> Hi,
> above patch (upstream commit 295525e29a5b) seems causing a virtnet
> related Call Trace after WARNING from kernel/dma/debug.c.
>
> details (log and test setup) tracked in
> https://bugzilla.kernel.org/show_bug.cgi?id=218204
>
> it's recently noticed in a TDX guest testing since v6.6.0 release cycle
> and can still be reproduced in latest v6.7.0-rc3.
>
> as local bisects results show, above WARNING and Call Trace is linked
> with this patch, do you mind to take a look?

Looks like virtqueue_dma_sync_single_range_for_cpu() use
DMA_BIDIRECTIONAL unconditionally.

We should use dir here.

Mind to try?

Thanks

>
> >
> > Yuan Yao (1):
> >        virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
> >
> > Yue Haibing (1):
> >        vdpa/mlx5: Remove unused function declarations
> >
> >   drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
> >   drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
> >   drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
> >   drivers/vhost/vdpa.c               |  15 +-
> >   drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
> >   drivers/virtio/virtio_vdpa.c       |  17 +-
> >   include/linux/vdpa.h               |   4 +
> >   include/linux/virtio.h             |  22 ++
> >   include/uapi/linux/vhost_types.h   |   4 +
> >   9 files changed, 625 insertions(+), 90 deletions(-)
> >
>
Michael S. Tsirkin Nov. 29, 2023, 9:47 a.m. UTC | #9
On Wed, Nov 29, 2023 at 05:03:50PM +0800, Ning, Hongyu wrote:
> 
> 
> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
> > The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> > 
> >    Linux 6.5 (2023-08-27 14:49:51 -0700)
> > 
> > are available in the Git repository at:
> > 
> >    https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> > 
> > for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> > 
> >    virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> > 
> > ----------------------------------------------------------------
> > virtio: features
> > 
> > a small pull request this time around, mostly because the
> > vduse network got postponed to next relase so we can be sure
> > we got the security store right.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ----------------------------------------------------------------
> > Eugenio Pérez (4):
> >        vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
> >        vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
> >        vdpa: add get_backend_features vdpa operation
> >        vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> > 
> > Jason Wang (1):
> >        virtio_vdpa: build affinity masks conditionally
> > 
> > Xuan Zhuo (12):
> >        virtio_ring: check use_dma_api before unmap desc for indirect
> >        virtio_ring: put mapping error check in vring_map_one_sg
> >        virtio_ring: introduce virtqueue_set_dma_premapped()
> >        virtio_ring: support add premapped buf
> >        virtio_ring: introduce virtqueue_dma_dev()
> >        virtio_ring: skip unmap for premapped
> >        virtio_ring: correct the expression of the description of virtqueue_resize()
> >        virtio_ring: separate the logic of reset/enable from virtqueue_resize
> >        virtio_ring: introduce virtqueue_reset()
> >        virtio_ring: introduce dma map api for virtqueue
> >        virtio_ring: introduce dma sync api for virtqueue
> >        virtio_net: merge dma operations when filling mergeable buffers
> 
> Hi,
> above patch (upstream commit 295525e29a5b) seems causing a virtnet related
> Call Trace after WARNING from kernel/dma/debug.c.
> 
> details (log and test setup) tracked in
> https://bugzilla.kernel.org/show_bug.cgi?id=218204
> 
> it's recently noticed in a TDX guest testing since v6.6.0 release cycle and
> can still be reproduced in latest v6.7.0-rc3.
> 
> as local bisects results show, above WARNING and Call Trace is linked with
> this patch, do you mind to take a look?

Does your testing tree include the fixup
5720c43d5216b5dbd9ab25595f7c61e55d36d4fc ?

> > 
> > Yuan Yao (1):
> >        virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
> > 
> > Yue Haibing (1):
> >        vdpa/mlx5: Remove unused function declarations
> > 
> >   drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
> >   drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
> >   drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
> >   drivers/vhost/vdpa.c               |  15 +-
> >   drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
> >   drivers/virtio/virtio_vdpa.c       |  17 +-
> >   include/linux/vdpa.h               |   4 +
> >   include/linux/virtio.h             |  22 ++
> >   include/uapi/linux/vhost_types.h   |   4 +
> >   9 files changed, 625 insertions(+), 90 deletions(-)
> >
Ning, Hongyu Nov. 29, 2023, 9:58 a.m. UTC | #10
On 2023/11/29 17:47, Michael S. Tsirkin wrote:
> On Wed, Nov 29, 2023 at 05:03:50PM +0800, Ning, Hongyu wrote:
>>
>>
>> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
>>> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
>>>
>>>     Linux 6.5 (2023-08-27 14:49:51 -0700)
>>>
>>> are available in the Git repository at:
>>>
>>>     https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
>>>
>>> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
>>>
>>>     virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
>>>
>>> ----------------------------------------------------------------
>>> virtio: features
>>>
>>> a small pull request this time around, mostly because the
>>> vduse network got postponed to next relase so we can be sure
>>> we got the security store right.
>>>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>
>>> ----------------------------------------------------------------
>>> Eugenio Pérez (4):
>>>         vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
>>>         vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
>>>         vdpa: add get_backend_features vdpa operation
>>>         vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
>>>
>>> Jason Wang (1):
>>>         virtio_vdpa: build affinity masks conditionally
>>>
>>> Xuan Zhuo (12):
>>>         virtio_ring: check use_dma_api before unmap desc for indirect
>>>         virtio_ring: put mapping error check in vring_map_one_sg
>>>         virtio_ring: introduce virtqueue_set_dma_premapped()
>>>         virtio_ring: support add premapped buf
>>>         virtio_ring: introduce virtqueue_dma_dev()
>>>         virtio_ring: skip unmap for premapped
>>>         virtio_ring: correct the expression of the description of virtqueue_resize()
>>>         virtio_ring: separate the logic of reset/enable from virtqueue_resize
>>>         virtio_ring: introduce virtqueue_reset()
>>>         virtio_ring: introduce dma map api for virtqueue
>>>         virtio_ring: introduce dma sync api for virtqueue
>>>         virtio_net: merge dma operations when filling mergeable buffers
>>
>> Hi,
>> above patch (upstream commit 295525e29a5b) seems causing a virtnet related
>> Call Trace after WARNING from kernel/dma/debug.c.
>>
>> details (log and test setup) tracked in
>> https://bugzilla.kernel.org/show_bug.cgi?id=218204
>>
>> it's recently noticed in a TDX guest testing since v6.6.0 release cycle and
>> can still be reproduced in latest v6.7.0-rc3.
>>
>> as local bisects results show, above WARNING and Call Trace is linked with
>> this patch, do you mind to take a look?
> 
> Does your testing tree include the fixup
> 5720c43d5216b5dbd9ab25595f7c61e55d36d4fc ?
> 

yes, it's included:
5720c43d5216 virtio_net: fix the missing of the dma cpu sync

>>>
>>> Yuan Yao (1):
>>>         virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
>>>
>>> Yue Haibing (1):
>>>         vdpa/mlx5: Remove unused function declarations
>>>
>>>    drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
>>>    drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
>>>    drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
>>>    drivers/vhost/vdpa.c               |  15 +-
>>>    drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
>>>    drivers/virtio/virtio_vdpa.c       |  17 +-
>>>    include/linux/vdpa.h               |   4 +
>>>    include/linux/virtio.h             |  22 ++
>>>    include/uapi/linux/vhost_types.h   |   4 +
>>>    9 files changed, 625 insertions(+), 90 deletions(-)
>>>
>
Ning, Hongyu Nov. 29, 2023, 10:12 a.m. UTC | #11
On 2023/11/29 17:16, Jason Wang wrote:
> On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> <hongyu.ning@linux.intel.com> wrote:
>>
>>
>>
>> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
>>> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
>>>
>>>     Linux 6.5 (2023-08-27 14:49:51 -0700)
>>>
>>> are available in the Git repository at:
>>>
>>>     https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
>>>
>>> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
>>>
>>>     virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
>>>
>>> ----------------------------------------------------------------
>>> virtio: features
>>>
>>> a small pull request this time around, mostly because the
>>> vduse network got postponed to next relase so we can be sure
>>> we got the security store right.
>>>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>
>>> ----------------------------------------------------------------
>>> Eugenio Pérez (4):
>>>         vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
>>>         vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
>>>         vdpa: add get_backend_features vdpa operation
>>>         vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
>>>
>>> Jason Wang (1):
>>>         virtio_vdpa: build affinity masks conditionally
>>>
>>> Xuan Zhuo (12):
>>>         virtio_ring: check use_dma_api before unmap desc for indirect
>>>         virtio_ring: put mapping error check in vring_map_one_sg
>>>         virtio_ring: introduce virtqueue_set_dma_premapped()
>>>         virtio_ring: support add premapped buf
>>>         virtio_ring: introduce virtqueue_dma_dev()
>>>         virtio_ring: skip unmap for premapped
>>>         virtio_ring: correct the expression of the description of virtqueue_resize()
>>>         virtio_ring: separate the logic of reset/enable from virtqueue_resize
>>>         virtio_ring: introduce virtqueue_reset()
>>>         virtio_ring: introduce dma map api for virtqueue
>>>         virtio_ring: introduce dma sync api for virtqueue
>>>         virtio_net: merge dma operations when filling mergeable buffers
>>
>> Hi,
>> above patch (upstream commit 295525e29a5b) seems causing a virtnet
>> related Call Trace after WARNING from kernel/dma/debug.c.
>>
>> details (log and test setup) tracked in
>> https://bugzilla.kernel.org/show_bug.cgi?id=218204
>>
>> it's recently noticed in a TDX guest testing since v6.6.0 release cycle
>> and can still be reproduced in latest v6.7.0-rc3.
>>
>> as local bisects results show, above WARNING and Call Trace is linked
>> with this patch, do you mind to take a look?
> 
> Looks like virtqueue_dma_sync_single_range_for_cpu() use
> DMA_BIDIRECTIONAL unconditionally.
> 
> We should use dir here.
> 
> Mind to try?
> 
> Thanks
> 

sure, but what I see in the code 
virtqueue_dma_sync_single_range_for_cpu() is using DMA_FROM_DEVICE, 
probably I misunderstood your point?

Please let me know any patch/setting to try here.


>>
>>>
>>> Yuan Yao (1):
>>>         virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
>>>
>>> Yue Haibing (1):
>>>         vdpa/mlx5: Remove unused function declarations
>>>
>>>    drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
>>>    drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
>>>    drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
>>>    drivers/vhost/vdpa.c               |  15 +-
>>>    drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
>>>    drivers/virtio/virtio_vdpa.c       |  17 +-
>>>    include/linux/vdpa.h               |   4 +
>>>    include/linux/virtio.h             |  22 ++
>>>    include/uapi/linux/vhost_types.h   |   4 +
>>>    9 files changed, 625 insertions(+), 90 deletions(-)
>>>
>>
>
Jason Wang Nov. 29, 2023, 10:20 a.m. UTC | #12
On Wed, Nov 29, 2023 at 6:12 PM Ning, Hongyu
<hongyu.ning@linux.intel.com> wrote:
>
>
> On 2023/11/29 17:16, Jason Wang wrote:
> > On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> > <hongyu.ning@linux.intel.com> wrote:
> >>
> >>
> >>
> >> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
> >>> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> >>>
> >>>     Linux 6.5 (2023-08-27 14:49:51 -0700)
> >>>
> >>> are available in the Git repository at:
> >>>
> >>>     https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> >>>
> >>> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> >>>
> >>>     virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> >>>
> >>> ----------------------------------------------------------------
> >>> virtio: features
> >>>
> >>> a small pull request this time around, mostly because the
> >>> vduse network got postponed to next relase so we can be sure
> >>> we got the security store right.
> >>>
> >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>>
> >>> ----------------------------------------------------------------
> >>> Eugenio Pérez (4):
> >>>         vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
> >>>         vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
> >>>         vdpa: add get_backend_features vdpa operation
> >>>         vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> >>>
> >>> Jason Wang (1):
> >>>         virtio_vdpa: build affinity masks conditionally
> >>>
> >>> Xuan Zhuo (12):
> >>>         virtio_ring: check use_dma_api before unmap desc for indirect
> >>>         virtio_ring: put mapping error check in vring_map_one_sg
> >>>         virtio_ring: introduce virtqueue_set_dma_premapped()
> >>>         virtio_ring: support add premapped buf
> >>>         virtio_ring: introduce virtqueue_dma_dev()
> >>>         virtio_ring: skip unmap for premapped
> >>>         virtio_ring: correct the expression of the description of virtqueue_resize()
> >>>         virtio_ring: separate the logic of reset/enable from virtqueue_resize
> >>>         virtio_ring: introduce virtqueue_reset()
> >>>         virtio_ring: introduce dma map api for virtqueue
> >>>         virtio_ring: introduce dma sync api for virtqueue
> >>>         virtio_net: merge dma operations when filling mergeable buffers
> >>
> >> Hi,
> >> above patch (upstream commit 295525e29a5b) seems causing a virtnet
> >> related Call Trace after WARNING from kernel/dma/debug.c.
> >>
> >> details (log and test setup) tracked in
> >> https://bugzilla.kernel.org/show_bug.cgi?id=218204
> >>
> >> it's recently noticed in a TDX guest testing since v6.6.0 release cycle
> >> and can still be reproduced in latest v6.7.0-rc3.
> >>
> >> as local bisects results show, above WARNING and Call Trace is linked
> >> with this patch, do you mind to take a look?
> >
> > Looks like virtqueue_dma_sync_single_range_for_cpu() use
> > DMA_BIDIRECTIONAL unconditionally.
> >
> > We should use dir here.
> >
> > Mind to try?
> >
> > Thanks
> >
>
> sure, but what I see in the code
> virtqueue_dma_sync_single_range_for_cpu() is using DMA_FROM_DEVICE,
> probably I misunderstood your point?
>
> Please let me know any patch/setting to try here.

Something like attached.  (Not even compiling test).

Thanks

>
>
> >>
> >>>
> >>> Yuan Yao (1):
> >>>         virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
> >>>
> >>> Yue Haibing (1):
> >>>         vdpa/mlx5: Remove unused function declarations
> >>>
> >>>    drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
> >>>    drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
> >>>    drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
> >>>    drivers/vhost/vdpa.c               |  15 +-
> >>>    drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
> >>>    drivers/virtio/virtio_vdpa.c       |  17 +-
> >>>    include/linux/vdpa.h               |   4 +
> >>>    include/linux/virtio.h             |  22 ++
> >>>    include/uapi/linux/vhost_types.h   |   4 +
> >>>    9 files changed, 625 insertions(+), 90 deletions(-)
> >>>
> >>
> >
>
Ning, Hongyu Nov. 29, 2023, 10:45 a.m. UTC | #13
On 2023/11/29 18:20, Jason Wang wrote:
> On Wed, Nov 29, 2023 at 6:12 PM Ning, Hongyu
> <hongyu.ning@linux.intel.com> wrote:
>>
>>
>> On 2023/11/29 17:16, Jason Wang wrote:
>>> On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
>>> <hongyu.ning@linux.intel.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
>>>>> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
>>>>>
>>>>>      Linux 6.5 (2023-08-27 14:49:51 -0700)
>>>>>
>>>>> are available in the Git repository at:
>>>>>
>>>>>      https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
>>>>>
>>>>> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
>>>>>
>>>>>      virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> virtio: features
>>>>>
>>>>> a small pull request this time around, mostly because the
>>>>> vduse network got postponed to next relase so we can be sure
>>>>> we got the security store right.
>>>>>
>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> Eugenio Pérez (4):
>>>>>          vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
>>>>>          vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
>>>>>          vdpa: add get_backend_features vdpa operation
>>>>>          vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
>>>>>
>>>>> Jason Wang (1):
>>>>>          virtio_vdpa: build affinity masks conditionally
>>>>>
>>>>> Xuan Zhuo (12):
>>>>>          virtio_ring: check use_dma_api before unmap desc for indirect
>>>>>          virtio_ring: put mapping error check in vring_map_one_sg
>>>>>          virtio_ring: introduce virtqueue_set_dma_premapped()
>>>>>          virtio_ring: support add premapped buf
>>>>>          virtio_ring: introduce virtqueue_dma_dev()
>>>>>          virtio_ring: skip unmap for premapped
>>>>>          virtio_ring: correct the expression of the description of virtqueue_resize()
>>>>>          virtio_ring: separate the logic of reset/enable from virtqueue_resize
>>>>>          virtio_ring: introduce virtqueue_reset()
>>>>>          virtio_ring: introduce dma map api for virtqueue
>>>>>          virtio_ring: introduce dma sync api for virtqueue
>>>>>          virtio_net: merge dma operations when filling mergeable buffers
>>>>
>>>> Hi,
>>>> above patch (upstream commit 295525e29a5b) seems causing a virtnet
>>>> related Call Trace after WARNING from kernel/dma/debug.c.
>>>>
>>>> details (log and test setup) tracked in
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=218204
>>>>
>>>> it's recently noticed in a TDX guest testing since v6.6.0 release cycle
>>>> and can still be reproduced in latest v6.7.0-rc3.
>>>>
>>>> as local bisects results show, above WARNING and Call Trace is linked
>>>> with this patch, do you mind to take a look?
>>>
>>> Looks like virtqueue_dma_sync_single_range_for_cpu() use
>>> DMA_BIDIRECTIONAL unconditionally.
>>>
>>> We should use dir here.
>>>
>>> Mind to try?
>>>
>>> Thanks
>>>
>>
>> sure, but what I see in the code
>> virtqueue_dma_sync_single_range_for_cpu() is using DMA_FROM_DEVICE,
>> probably I misunderstood your point?
>>
>> Please let me know any patch/setting to try here.
> 
> Something like attached.  (Not even compiling test).
> 
> Thanks
> 

Oh, this patch works, WARNING and Call Trace are no more reproduced in 
the same Linux Guest Kernel setup.

>>
>>
>>>>
>>>>>
>>>>> Yuan Yao (1):
>>>>>          virtio_ring: fix avail_wrap_counter in virtqueue_add_packed
>>>>>
>>>>> Yue Haibing (1):
>>>>>          vdpa/mlx5: Remove unused function declarations
>>>>>
>>>>>     drivers/net/virtio_net.c           | 230 ++++++++++++++++++---
>>>>>     drivers/vdpa/mlx5/core/mlx5_vdpa.h |   3 -
>>>>>     drivers/vdpa/vdpa_sim/vdpa_sim.c   |   8 +
>>>>>     drivers/vhost/vdpa.c               |  15 +-
>>>>>     drivers/virtio/virtio_ring.c       | 412 ++++++++++++++++++++++++++++++++-----
>>>>>     drivers/virtio/virtio_vdpa.c       |  17 +-
>>>>>     include/linux/vdpa.h               |   4 +
>>>>>     include/linux/virtio.h             |  22 ++
>>>>>     include/uapi/linux/vhost_types.h   |   4 +
>>>>>     9 files changed, 625 insertions(+), 90 deletions(-)
>>>>>
>>>>
>>>
>>
Michael S. Tsirkin Nov. 30, 2023, 9:44 a.m. UTC | #14
On Wed, Nov 29, 2023 at 06:20:31PM +0800, Jason Wang wrote:
> On Wed, Nov 29, 2023 at 6:12 PM Ning, Hongyu
> <hongyu.ning@linux.intel.com> wrote:
> >
> >
> > On 2023/11/29 17:16, Jason Wang wrote:
> > > On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> > > <hongyu.ning@linux.intel.com> wrote:
> > >>
> > >>
> > >>
> > >> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
> > >>> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> > >>>
> > >>>     Linux 6.5 (2023-08-27 14:49:51 -0700)
> > >>>
> > >>> are available in the Git repository at:
> > >>>
> > >>>     https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> > >>>
> > >>> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> > >>>
> > >>>     virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> > >>>
> > >>> ----------------------------------------------------------------
> > >>> virtio: features
> > >>>
> > >>> a small pull request this time around, mostly because the
> > >>> vduse network got postponed to next relase so we can be sure
> > >>> we got the security store right.
> > >>>
> > >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > >>>
> > >>> ----------------------------------------------------------------
> > >>> Eugenio Pérez (4):
> > >>>         vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
> > >>>         vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
> > >>>         vdpa: add get_backend_features vdpa operation
> > >>>         vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> > >>>
> > >>> Jason Wang (1):
> > >>>         virtio_vdpa: build affinity masks conditionally
> > >>>
> > >>> Xuan Zhuo (12):
> > >>>         virtio_ring: check use_dma_api before unmap desc for indirect
> > >>>         virtio_ring: put mapping error check in vring_map_one_sg
> > >>>         virtio_ring: introduce virtqueue_set_dma_premapped()
> > >>>         virtio_ring: support add premapped buf
> > >>>         virtio_ring: introduce virtqueue_dma_dev()
> > >>>         virtio_ring: skip unmap for premapped
> > >>>         virtio_ring: correct the expression of the description of virtqueue_resize()
> > >>>         virtio_ring: separate the logic of reset/enable from virtqueue_resize
> > >>>         virtio_ring: introduce virtqueue_reset()
> > >>>         virtio_ring: introduce dma map api for virtqueue
> > >>>         virtio_ring: introduce dma sync api for virtqueue
> > >>>         virtio_net: merge dma operations when filling mergeable buffers
> > >>
> > >> Hi,
> > >> above patch (upstream commit 295525e29a5b) seems causing a virtnet
> > >> related Call Trace after WARNING from kernel/dma/debug.c.
> > >>
> > >> details (log and test setup) tracked in
> > >> https://bugzilla.kernel.org/show_bug.cgi?id=218204
> > >>
> > >> it's recently noticed in a TDX guest testing since v6.6.0 release cycle
> > >> and can still be reproduced in latest v6.7.0-rc3.
> > >>
> > >> as local bisects results show, above WARNING and Call Trace is linked
> > >> with this patch, do you mind to take a look?
> > >
> > > Looks like virtqueue_dma_sync_single_range_for_cpu() use
> > > DMA_BIDIRECTIONAL unconditionally.
> > >
> > > We should use dir here.
> > >
> > > Mind to try?
> > >
> > > Thanks
> > >
> >
> > sure, but what I see in the code
> > virtqueue_dma_sync_single_range_for_cpu() is using DMA_FROM_DEVICE,
> > probably I misunderstood your point?
> >
> > Please let me know any patch/setting to try here.
> 
> Something like attached.  (Not even compiling test).
> 
> Thanks

Forwarding it inline for the record - I am not sure all the
0 day machinery handles attachments. Jason given it's reported to work
can you please repost properly with a full commit log etc?
I think we also need to fix virtqueue_dma_sync_single_range_for_device -
please include that too.
 

From: Jason Wang <jasowang@redhat.com>
Date: Wed, 29 Nov 2023 17:14:15 +0800
Subject: [PATCH] virtio_ring: fix DMA dir during sync
Content-type: text/plain

Signed-off-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---
 drivers/virtio/virtio_ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 81ecb29c88f1..91d869814373 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -3220,7 +3220,7 @@ void virtqueue_dma_sync_single_range_for_cpu(struct virtqueue *_vq,
 		return;
 
 	dma_sync_single_range_for_cpu(dev, addr, offset, size,
-				      DMA_BIDIRECTIONAL);
+				      dir);
 }
 EXPORT_SYMBOL_GPL(virtqueue_dma_sync_single_range_for_cpu);
Jason Wang Dec. 1, 2023, 5:15 a.m. UTC | #15
On Thu, Nov 30, 2023 at 5:44 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Nov 29, 2023 at 06:20:31PM +0800, Jason Wang wrote:
> > On Wed, Nov 29, 2023 at 6:12 PM Ning, Hongyu
> > <hongyu.ning@linux.intel.com> wrote:
> > >
> > >
> > > On 2023/11/29 17:16, Jason Wang wrote:
> > > > On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> > > > <hongyu.ning@linux.intel.com> wrote:
> > > >>
> > > >>
> > > >>
> > > >> On 2023/9/4 6:13, Michael S. Tsirkin wrote:
> > > >>> The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:
> > > >>>
> > > >>>     Linux 6.5 (2023-08-27 14:49:51 -0700)
> > > >>>
> > > >>> are available in the Git repository at:
> > > >>>
> > > >>>     https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
> > > >>>
> > > >>> for you to fetch changes up to 1acfe2c1225899eab5ab724c91b7e1eb2881b9ab:
> > > >>>
> > > >>>     virtio_ring: fix avail_wrap_counter in virtqueue_add_packed (2023-09-03 18:10:24 -0400)
> > > >>>
> > > >>> ----------------------------------------------------------------
> > > >>> virtio: features
> > > >>>
> > > >>> a small pull request this time around, mostly because the
> > > >>> vduse network got postponed to next relase so we can be sure
> > > >>> we got the security store right.
> > > >>>
> > > >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > >>>
> > > >>> ----------------------------------------------------------------
> > > >>> Eugenio Pérez (4):
> > > >>>         vdpa: add VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK flag
> > > >>>         vdpa: accept VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK backend feature
> > > >>>         vdpa: add get_backend_features vdpa operation
> > > >>>         vdpa_sim: offer VHOST_BACKEND_F_ENABLE_AFTER_DRIVER_OK
> > > >>>
> > > >>> Jason Wang (1):
> > > >>>         virtio_vdpa: build affinity masks conditionally
> > > >>>
> > > >>> Xuan Zhuo (12):
> > > >>>         virtio_ring: check use_dma_api before unmap desc for indirect
> > > >>>         virtio_ring: put mapping error check in vring_map_one_sg
> > > >>>         virtio_ring: introduce virtqueue_set_dma_premapped()
> > > >>>         virtio_ring: support add premapped buf
> > > >>>         virtio_ring: introduce virtqueue_dma_dev()
> > > >>>         virtio_ring: skip unmap for premapped
> > > >>>         virtio_ring: correct the expression of the description of virtqueue_resize()
> > > >>>         virtio_ring: separate the logic of reset/enable from virtqueue_resize
> > > >>>         virtio_ring: introduce virtqueue_reset()
> > > >>>         virtio_ring: introduce dma map api for virtqueue
> > > >>>         virtio_ring: introduce dma sync api for virtqueue
> > > >>>         virtio_net: merge dma operations when filling mergeable buffers
> > > >>
> > > >> Hi,
> > > >> above patch (upstream commit 295525e29a5b) seems causing a virtnet
> > > >> related Call Trace after WARNING from kernel/dma/debug.c.
> > > >>
> > > >> details (log and test setup) tracked in
> > > >> https://bugzilla.kernel.org/show_bug.cgi?id=218204
> > > >>
> > > >> it's recently noticed in a TDX guest testing since v6.6.0 release cycle
> > > >> and can still be reproduced in latest v6.7.0-rc3.
> > > >>
> > > >> as local bisects results show, above WARNING and Call Trace is linked
> > > >> with this patch, do you mind to take a look?
> > > >
> > > > Looks like virtqueue_dma_sync_single_range_for_cpu() use
> > > > DMA_BIDIRECTIONAL unconditionally.
> > > >
> > > > We should use dir here.
> > > >
> > > > Mind to try?
> > > >
> > > > Thanks
> > > >
> > >
> > > sure, but what I see in the code
> > > virtqueue_dma_sync_single_range_for_cpu() is using DMA_FROM_DEVICE,
> > > probably I misunderstood your point?
> > >
> > > Please let me know any patch/setting to try here.
> >
> > Something like attached.  (Not even compiling test).
> >
> > Thanks
>
> Forwarding it inline for the record - I am not sure all the
> 0 day machinery handles attachments. Jason given it's reported to work
> can you please repost properly with a full commit log etc?
> I think we also need to fix virtqueue_dma_sync_single_range_for_device -
> please include that too.

Yes, want to sent something like this yesterday but it was interrupted
by other tasks.

I see Xuan has posted a patch, I will ack on that.

Thanks

>
>
> From: Jason Wang <jasowang@redhat.com>
> Date: Wed, 29 Nov 2023 17:14:15 +0800
> Subject: [PATCH] virtio_ring: fix DMA dir during sync
> Content-type: text/plain
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>
> ---
>  drivers/virtio/virtio_ring.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 81ecb29c88f1..91d869814373 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -3220,7 +3220,7 @@ void virtqueue_dma_sync_single_range_for_cpu(struct virtqueue *_vq,
>                 return;
>
>         dma_sync_single_range_for_cpu(dev, addr, offset, size,
> -                                     DMA_BIDIRECTIONAL);
> +                                     dir);
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_dma_sync_single_range_for_cpu);
>
> --
> 2.42.0
>