2023-11-29 09:05:20

by Ning, Hongyu

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features



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 <[email protected]>
>
> ----------------------------------------------------------------
> 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(-)
>


2023-11-29 09:18:17

by Jason Wang

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features

On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
<[email protected]> 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 <[email protected]>
> >
> > ----------------------------------------------------------------
> > 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(-)
> >
>

2023-11-29 09:47:50

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features

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 <[email protected]>
> >
> > ----------------------------------------------------------------
> > 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(-)
> >

2023-11-29 09:59:00

by Ning, Hongyu

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features


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 <[email protected]>
>>>
>>> ----------------------------------------------------------------
>>> 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(-)
>>>
>

2023-11-29 10:12:47

by Ning, Hongyu

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features


On 2023/11/29 17:16, Jason Wang wrote:
> On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> <[email protected]> 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 <[email protected]>
>>>
>>> ----------------------------------------------------------------
>>> 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(-)
>>>
>>
>

2023-11-29 10:21:08

by Jason Wang

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features

On Wed, Nov 29, 2023 at 6:12 PM Ning, Hongyu
<[email protected]> wrote:
>
>
> On 2023/11/29 17:16, Jason Wang wrote:
> > On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> > <[email protected]> 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 <[email protected]>
> >>>
> >>> ----------------------------------------------------------------
> >>> 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(-)
> >>>
> >>
> >
>


Attachments:
0001-virtio_ring-fix-DMA-dir-during-sync.patch (834.00 B)

2023-11-29 10:48:06

by Ning, Hongyu

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features


On 2023/11/29 18:20, Jason Wang wrote:
> On Wed, Nov 29, 2023 at 6:12 PM Ning, Hongyu
> <[email protected]> wrote:
>>
>>
>> On 2023/11/29 17:16, Jason Wang wrote:
>>> On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
>>> <[email protected]> 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 <[email protected]>
>>>>>
>>>>> ----------------------------------------------------------------
>>>>> 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(-)
>>>>>
>>>>
>>>
>>

2023-11-30 09:44:46

by Michael S. Tsirkin

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features

On Wed, Nov 29, 2023 at 06:20:31PM +0800, Jason Wang wrote:
> On Wed, Nov 29, 2023 at 6:12 PM Ning, Hongyu
> <[email protected]> wrote:
> >
> >
> > On 2023/11/29 17:16, Jason Wang wrote:
> > > On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> > > <[email protected]> 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 <[email protected]>
> > >>>
> > >>> ----------------------------------------------------------------
> > >>> 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 <[email protected]>
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 <[email protected]>
Signed-off-by: Michael S. Tsirkin <[email protected]>

---
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

2023-12-01 05:16:16

by Jason Wang

[permalink] [raw]
Subject: Re: [GIT PULL] virtio: features

On Thu, Nov 30, 2023 at 5:44 PM Michael S. Tsirkin <[email protected]> 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
> > <[email protected]> wrote:
> > >
> > >
> > > On 2023/11/29 17:16, Jason Wang wrote:
> > > > On Wed, Nov 29, 2023 at 5:05 PM Ning, Hongyu
> > > > <[email protected]> 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 <[email protected]>
> > > >>>
> > > >>> ----------------------------------------------------------------
> > > >>> 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 <[email protected]>
> 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 <[email protected]>
> Signed-off-by: Michael S. Tsirkin <[email protected]>
>
> ---
> 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
>