[PATCH v4 0/3] virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX
if driver reads data larger than VIRTIO_BLK_F_SIZE_MAX, it will cause some issue to the DMA engine. So add VIRTIO_BLK_F_SIZE_MAX feature support, 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. Andy Pei (3): virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX virtio-blk: abstract a function named virtio_blk_op_one_segment to handle r/w request virtio-blk.: split large IO according to size_max src/block.h | 2 + src/hw/virtio-blk.c | 128 +++++++++++++++++++++++++++++++++++++++++----------- src/hw/virtio-blk.h | 3 ++ 3 files changed, 107 insertions(+), 26 deletions(-) -- 1.8.3.1
according to virtio spec, add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX parse to virtio blk driver. Signed-off-by: Andy Pei <andy.pei@intel.com> Signed-off-by: Ding Limin <dinglimin@cmss.chinamobile.com> --- src/block.h | 2 ++ src/hw/virtio-blk.c | 51 +++++++++++++++++++++++++++++++++++++++++++-------- src/hw/virtio-blk.h | 3 +++ 3 files changed, 48 insertions(+), 8 deletions(-) diff --git a/src/block.h b/src/block.h index c1b8d73..3af2d71 100644 --- a/src/block.h +++ b/src/block.h @@ -57,6 +57,8 @@ struct drive_s { u8 translation; // type of translation u16 blksize; // block size struct chs_s pchs; // Physical CHS + u32 max_segment_size; //max_segment_size + u32 max_segments; //max_segments }; #define DISK_SECTOR_SIZE 512 diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 3b19896..2653dad 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -121,12 +121,16 @@ init_virtio_blk(void *data) u64 version1 = 1ull << VIRTIO_F_VERSION_1; u64 iommu_platform = 1ull << VIRTIO_F_IOMMU_PLATFORM; u64 blk_size = 1ull << VIRTIO_BLK_F_BLK_SIZE; + u64 max_segments = 1ull << VIRTIO_BLK_F_SEG_MAX; + u64 max_segment_size = 1ull << VIRTIO_BLK_F_SIZE_MAX; + if (!(features & version1)) { dprintf(1, "modern device without virtio_1 feature bit: %pP\n", pci); goto fail; } - features = features & (version1 | iommu_platform | blk_size); + features = features & (version1 | iommu_platform | blk_size + | max_segments | max_segment_size); vp_set_features(vp, features); status |= VIRTIO_CONFIG_S_FEATURES_OK; vp_set_status(vp, status); @@ -135,6 +139,14 @@ init_virtio_blk(void *data) goto fail; } + if (features & max_segment_size) + vdrive->drive.max_segment_size = + vp_read(&vp->device, struct virtio_blk_config, size_max); + + if (features & max_segments) + vdrive->drive.max_segments = + vp_read(&vp->device, struct virtio_blk_config, seg_max); + vdrive->drive.sectors = vp_read(&vp->device, struct virtio_blk_config, capacity); if (features & blk_size) { @@ -148,8 +160,10 @@ init_virtio_blk(void *data) pci, vdrive->drive.blksize); goto fail; } - dprintf(3, "virtio-blk %pP blksize=%d sectors=%u\n", - pci, vdrive->drive.blksize, (u32)vdrive->drive.sectors); + dprintf(3, "virtio-blk %pP blksize=%d sectors=%u size_max=%u " + "seg_max=%u.\n", pci, vdrive->drive.blksize, + (u32)vdrive->drive.sectors, vdrive->drive.max_segment_size, + vdrive->drive.max_segments); vdrive->drive.pchs.cylinder = vp_read(&vp->device, struct virtio_blk_config, cylinders); @@ -165,9 +179,17 @@ init_virtio_blk(void *data) vdrive->drive.blksize = (f & (1 << VIRTIO_BLK_F_BLK_SIZE)) ? cfg.blk_size : DISK_SECTOR_SIZE; + if (f & (1 << VIRTIO_BLK_F_SIZE_MAX)) + vdrive->drive.max_segment_size = cfg.size_max; + + if (f & (1 << VIRTIO_BLK_F_SEG_MAX)) + vdrive->drive.max_segments = cfg.seg_max; + vdrive->drive.sectors = cfg.capacity; - dprintf(3, "virtio-blk %pP blksize=%d sectors=%u\n", - pci, vdrive->drive.blksize, (u32)vdrive->drive.sectors); + dprintf(3, "virtio-blk %pP blksize=%d sectors=%u size_max=%u " + "seg_max=%u.\n", pci, vdrive->drive.blksize, + (u32)vdrive->drive.sectors, vdrive->drive.max_segment_size, + vdrive->drive.max_segments); if (vdrive->drive.blksize != DISK_SECTOR_SIZE) { dprintf(1, "virtio-blk %pP block size %d is unsupported\n", @@ -218,8 +240,11 @@ init_virtio_blk_mmio(void *mmio) u64 features = vp_get_features(vp); u64 version1 = 1ull << VIRTIO_F_VERSION_1; u64 blk_size = 1ull << VIRTIO_BLK_F_BLK_SIZE; + u64 max_segments = 1ull << VIRTIO_BLK_F_SEG_MAX; + u64 max_segment_size = 1ull << VIRTIO_BLK_F_SIZE_MAX; - features = features & (version1 | blk_size); + features = features & (version1 | blk_size + | max_segments | max_segment_size); vp_set_features(vp, features); status |= VIRTIO_CONFIG_S_FEATURES_OK; vp_set_status(vp, status); @@ -228,6 +253,14 @@ init_virtio_blk_mmio(void *mmio) goto fail; } + if (features & max_segment_size) + vdrive->drive.max_segment_size = + vp_read(&vp->device, struct virtio_blk_config, size_max); + + if (features & max_segments) + vdrive->drive.max_segments = + vp_read(&vp->device, struct virtio_blk_config, seg_max); + vdrive->drive.sectors = vp_read(&vp->device, struct virtio_blk_config, capacity); if (features & blk_size) { @@ -241,8 +274,10 @@ init_virtio_blk_mmio(void *mmio) mmio, vdrive->drive.blksize); goto fail; } - dprintf(1, "virtio-blk-mmio %p blksize=%d sectors=%u\n", - mmio, vdrive->drive.blksize, (u32)vdrive->drive.sectors); + dprintf(1, "virtio-blk-mmio %p blksize=%d sectors=%u size_max=%u " + "seg_max=%u.\n", mmio, vdrive->drive.blksize, + (u32)vdrive->drive.sectors, vdrive->drive.max_segment_size, + vdrive->drive.max_segments); vdrive->drive.pchs.cylinder = vp_read(&vp->device, struct virtio_blk_config, cylinders); diff --git a/src/hw/virtio-blk.h b/src/hw/virtio-blk.h index d20461a..0294291 100644 --- a/src/hw/virtio-blk.h +++ b/src/hw/virtio-blk.h @@ -16,6 +16,9 @@ struct virtio_blk_config u32 opt_io_size; } __attribute__((packed)); +/* Feature bits */ +#define VIRTIO_BLK_F_SIZE_MAX 1 /* Maximum size of any single segment */ +#define VIRTIO_BLK_F_SEG_MAX 2 /* Maximum number of segments in a request */ #define VIRTIO_BLK_F_BLK_SIZE 6 /* These two define direction. */ -- 1.8.3.1
On Tue, Dec 07, 2021 at 09:31:06AM +0800, Andy Pei wrote:
according to virtio spec, add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX parse to virtio blk driver.
Signed-off-by: Andy Pei <andy.pei@intel.com> Signed-off-by: Ding Limin <dinglimin@cmss.chinamobile.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
abstract virtio-blk queue operation to form a function named virtio_blk_op_one_segment Signed-off-by: Andy Pei <andy.pei@intel.com> Signed-off-by: Ding Limin <dinglimin@cmss.chinamobile.com> --- src/hw/virtio-blk.c | 48 ++++++++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 2653dad..82b1d2a 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -30,12 +30,38 @@ struct virtiodrive_s { struct vp_device vp; }; +void +virtio_blk_op_one_segment(struct virtiodrive_s *vdrive, + int write, struct vring_list sg[]) +{ + struct vring_virtqueue *vq = vdrive->vq; + + /* Add to virtqueue and kick host */ + if (write) + vring_add_buf(vq, sg, 2, 1, 0, 0); + else + vring_add_buf(vq, sg, 1, 2, 0, 0); + vring_kick(&vdrive->vp, vq, 1); + + /* Wait for reply */ + while (!vring_more_used(vq)) + usleep(5); + + /* Reclaim virtqueue element */ + vring_get_buf(vq, NULL); + + /** + ** Clear interrupt status register. Avoid leaving interrupts stuck + ** if VRING_AVAIL_F_NO_INTERRUPT was ignored and interrupts were raised. + **/ + vp_get_isr(&vdrive->vp); +} + static int virtio_blk_op(struct disk_op_s *op, int write) { struct virtiodrive_s *vdrive = container_of(op->drive_fl, struct virtiodrive_s, drive); - struct vring_virtqueue *vq = vdrive->vq; struct virtio_blk_outhdr hdr = { .type = write ? VIRTIO_BLK_T_OUT : VIRTIO_BLK_T_IN, .ioprio = 0, @@ -57,25 +83,7 @@ virtio_blk_op(struct disk_op_s *op, int write) }, }; - /* Add to virtqueue and kick host */ - if (write) - vring_add_buf(vq, sg, 2, 1, 0, 0); - else - vring_add_buf(vq, sg, 1, 2, 0, 0); - vring_kick(&vdrive->vp, vq, 1); - - /* Wait for reply */ - while (!vring_more_used(vq)) - usleep(5); - - /* Reclaim virtqueue element */ - vring_get_buf(vq, NULL); - - /* Clear interrupt status register. Avoid leaving interrupts stuck if - * VRING_AVAIL_F_NO_INTERRUPT was ignored and interrupts were raised. - */ - vp_get_isr(&vdrive->vp); - + virtio_blk_op_one_segment(vdrive, write, sg); return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK; } -- 1.8.3.1
On Tue, Dec 07, 2021 at 09:31:07AM +0800, Andy Pei wrote:
abstract virtio-blk queue operation to form a function named virtio_blk_op_one_segment
Signed-off-by: Andy Pei <andy.pei@intel.com> Signed-off-by: Ding Limin <dinglimin@cmss.chinamobile.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
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 | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index 82b1d2a..929ba88 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -24,6 +24,11 @@ #include "virtio-ring.h" #include "virtio-blk.h" +#define min(a, b) ({\ + typeof(a) _a = a;\ + typeof(b) _b = b;\ + _a < _b ? _a : _b; }) + struct virtiodrive_s { struct drive_s drive; struct vring_virtqueue *vq; @@ -82,8 +87,36 @@ 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; + + 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; - virtio_blk_op_one_segment(vdrive, write, sg); + if (op->count <= blk_num_max) { + virtio_blk_op_one_segment(vdrive, write, sg); + } else { + void *p = op->buf_fl; + u16 count = op->count; + + while (count > 0) { + u16 blk_num = min(count, blk_num_max); + sg[1].length = vdrive->drive.blksize * blk_num; + sg[1].addr = p; + virtio_blk_op_one_segment(vdrive, write, sg); + if (status == VIRTIO_BLK_S_OK) { + hdr.sector += blk_num; + p += sg[1].length; + count -= blk_num; + } else { + break; + } + } + } return status == VIRTIO_BLK_S_OK ? DISK_RET_SUCCESS : DISK_RET_EBADTRACK; } -- 1.8.3.1
On Tue, Dec 07, 2021 at 09:31:08AM +0800, Andy Pei wrote:
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>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Hi Gerd, Thanks for your effort. -----Original Message----- From: Gerd Hoffmann <kraxel@redhat.com> Sent: Tuesday, December 7, 2021 4:28 PM To: Pei, Andy <andy.pei@intel.com> Cc: seabios@seabios.org; kevin@koconnor.net; Ding Limin <dinglimin@cmss.chinamobile.com> Subject: Re: [PATCH v4 3/3] virtio-blk.: split large IO according to size_max On Tue, Dec 07, 2021 at 09:31:08AM +0800, Andy Pei wrote:
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>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
On Tue, Dec 07, 2021 at 09:31:05AM +0800, Andy Pei wrote:
if driver reads data larger than VIRTIO_BLK_F_SIZE_MAX, it will cause some issue to the DMA engine. So add VIRTIO_BLK_F_SIZE_MAX feature support, 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.
Thanks. I committed this change. -Kevin
Thanks Kevin -----Original Message----- From: Kevin O'Connor <kevin@koconnor.net> Sent: Sunday, December 19, 2021 12:55 AM To: Pei, Andy <andy.pei@intel.com> Cc: seabios@seabios.org; kraxel@redhat.com Subject: Re: [PATCH v4 0/3] virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX On Tue, Dec 07, 2021 at 09:31:05AM +0800, Andy Pei wrote:
if driver reads data larger than VIRTIO_BLK_F_SIZE_MAX, it will cause some issue to the DMA engine. So add VIRTIO_BLK_F_SIZE_MAX feature support, 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.
Thanks. I committed this change. -Kevin
participants (4)
-
Andy Pei -
Gerd Hoffmann -
Kevin O'Connor -
Pei, Andy