if driver reads data larger than VIRTIO_BLK_F_SIZE_MAX,
it will cause some issue to the DMA engine.

So when upper software wants to read data larger than
VIRTIO_BLK_F_SIZE_MAX, virtio-blk driver split one large
request into multiple smaller ones.

Signed-off-by: Andy Pei <andy.pei@intel.com>
Signed-off-by: Ding Limin <dinglimin@cmss.chinamobile.com>
---
 src/hw/virtio-blk.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c
index 0bc8743..664396c 100644
--- a/src/hw/virtio-blk.c
+++ b/src/hw/virtio-blk.c
@@ -82,8 +82,46 @@ virtio_blk_op(struct disk_op_s *op, int write)
             .length     = sizeof(status),
         },
     };
+    u32 max_io_size =
+        vdrive->drive.max_segment_size * vdrive->drive.max_segments;
+    u16 blk_num_max;
 
-    virtio_blk_op_one_segment(vdrive, write, sg);
+    if (vdrive->drive.blksize != 0 && max_io_size != 0)
+        blk_num_max = (u16)max_io_size / vdrive->drive.blksize;
+    else
+        /* default blk_num_max if hardware doesnot advise a proper value */
+        blk_num_max = 8;
+
+    if (op->count <= blk_num_max) {
+        virtio_blk_op_one_segment(vdrive, write, sg);
+    } else {
+        struct vring_list *p_sg = &sg[1];
+        void *p  = op->buf_fl;
+        u16 count = op->count;
+
+        while (count > blk_num_max) {
+            p_sg->addr = p;
+            p_sg->length = vdrive->drive.blksize * blk_num_max;
+            virtio_blk_op_one_segment(vdrive, write, sg);
+            if (status == VIRTIO_BLK_S_OK) {
+                hdr.sector += blk_num_max;
+                p += p_sg->length;
+                count -= blk_num_max;
+            } else {
+                break;
+            }
+        }
+
+        if (status != VIRTIO_BLK_S_OK)
+            return DISK_RET_EBADTRACK;
+
+        if (count > 0) {
+            p_sg->addr = p;
+            p_sg->length = vdrive->drive.blksize * count;
+            virtio_blk_op_one_segment(vdrive, write, sg);
+        }
+
+    }
     return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK;
 }
 
-- 
1.8.3.1

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

> +    } else {
> +        struct vring_list *p_sg = &sg[1];

Not needed, you can just use sg[1].addr

> +        void *p  = op->buf_fl;
> +        u16 count = op->count;
> +
> +        while (count > blk_num_max) {

           while (count > 0) {
               u16 blk_num = min(count, blk_num_max);

> +            } else {
> +                break;
> +            }
> +        }
> +
> +        if (status != VIRTIO_BLK_S_OK)
> +            return DISK_RET_EBADTRACK;

Can be moved into the while loop, you don't need the break then.

> +        if (count > 0) {
> +            p_sg->addr = p;
> +            p_sg->length = vdrive->drive.blksize * count;
> +            virtio_blk_op_one_segment(vdrive, write, sg);
> +        }

Not needed with the change above.

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

Reply in line


> +    } else {
> +        struct vring_list *p_sg = &sg[1];

Not needed, you can just use sg[1].addr
Andy: I used to think sg[1]. Length, but I am not sure.  Could you please tell me why sg[1]. Length is not needed ?

> +        void *p  = op->buf_fl;
> +        u16 count = op->count;
> +
> +        while (count > blk_num_max) {

           while (count > 0) {
               u16 blk_num = min(count, blk_num_max);

> +            } else {
> +                break;
> +            }
> +        }
> +
> +        if (status != VIRTIO_BLK_S_OK)
> +            return DISK_RET_EBADTRACK;

Can be moved into the while loop, you don't need the break then.
Andy: OK, address it in V4.

> +        if (count > 0) {
> +            p_sg->addr = p;
> +            p_sg->length = vdrive->drive.blksize * count;
> +            virtio_blk_op_one_segment(vdrive, write, sg);
> +        }

Not needed with the change above.
Andy: OK, address it in V4.

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org

On Mon, Dec 06, 2021 at 01:38:16PM +0000, Pei, Andy wrote:
> Reply in line
> 
> 
> > +    } else {
> > +        struct vring_list *p_sg = &sg[1];
> 
> Not needed, you can just use sg[1].addr
> Andy: I used to think sg[1]. Length, but I am not sure.  Could you please tell me why sg[1]. Length is not needed ?

length too of course, but you can likewise use sg[1].length and there is
no need to create that p_sg pointer.

take care,
  Gerd

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org