PING Re: [PATCH] virtio_ring: use u32 for virtio_max_dma_size

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Michael,

This seems to be ignored...

On 5/10/23 12:06, Michael S. Tsirkin wrote:
On Wed, May 10, 2023 at 12:04:50PM +0800, Jason Wang wrote:
On Wed, May 10, 2023 at 11:44 AM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:

On Wed, May 10, 2023 at 11:26:54AM +0800, Xuan Zhuo wrote:
On Wed, 10 May 2023 10:54:37 +0800, zhenwei pi <pizhenwei@xxxxxxxxxxxxx> wrote:
Both split ring and packed ring use 32bits to describe the length of
a descriptor: see struct vring_desc and struct vring_packed_desc.
This means the max segment size supported by virtio is U32_MAX.

An example of virtio_max_dma_size in virtio_blk.c:
   u32 v, max_size;

   max_size = virtio_max_dma_size(vdev);  -> implicit convert
   err = virtio_cread_feature(vdev, VIRTIO_BLK_F_SIZE_MAX,
                              struct virtio_blk_config, size_max, &v);
   max_size = min(max_size, v);

There is a risk during implicit convert here, once virtio_max_dma_size
returns 4G, max_size becomes 0.

Fixes: e6d6dd6c875e ("virtio: Introduce virtio_max_dma_size()")
Cc: Joerg Roedel <jroedel@xxxxxxx>
Signed-off-by: zhenwei pi <pizhenwei@xxxxxxxxxxxxx>
---
  drivers/virtio/virtio_ring.c | 12 ++++++++----
  include/linux/virtio.h       |  2 +-
  2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index c5310eaf8b46..55cfecf030a1 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -289,12 +289,16 @@ static bool vring_use_dma_api(const struct virtio_device *vdev)
     return false;
  }

-size_t virtio_max_dma_size(const struct virtio_device *vdev)
+u32 virtio_max_dma_size(const struct virtio_device *vdev)


LGTM

But, should we change the parameter to vq, then use the dma_dev?

@Jason

Thanks.



that would be an unrelated rework.

Probably, but I think it's better to be done on top otherwise we may forget.

Thanks

Just to make things clear I'm merging fixes for this
release but cleanups belong in the next one.


  {
-   size_t max_segment_size = SIZE_MAX;
+   u32 max_segment_size = U32_MAX;

-   if (vring_use_dma_api(vdev))
-           max_segment_size = dma_max_mapping_size(vdev->dev.parent);
+   if (vring_use_dma_api(vdev)) {
+           size_t max_dma_size = dma_max_mapping_size(vdev->dev.parent);
+
+           if (max_dma_size < max_segment_size)
+                   max_segment_size = max_dma_size;
+   }

     return max_segment_size;
  }
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b93238db94e3..1a605f408329 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -147,7 +147,7 @@ int virtio_device_restore(struct virtio_device *dev);
  #endif
  void virtio_reset_device(struct virtio_device *dev);

-size_t virtio_max_dma_size(const struct virtio_device *vdev);
+u32 virtio_max_dma_size(const struct virtio_device *vdev);

  #define virtio_device_for_each_vq(vdev, vq) \
     list_for_each_entry(vq, &vdev->vqs, list)
--
2.20.1




--
zhenwei pi
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux