[PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
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: Ding Limin <dinglimin@cmss.chinamobile.com> Signed-off-by: Andy Pei <andy.pei@intel.com> --- src/block.h | 2 ++ src/hw/virtio-blk.c | 56 +++++++++++++++++++++++++++++++++++++++++++++++++++-- src/hw/virtio-blk.h | 3 +++ 3 files changed, 59 insertions(+), 2 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..7935f0f 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,22 @@ 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); + else + vdrive->drive.max_segment_size = 0; + dprintf(3, "virtio-blk %pP size_max=%u.\n", pci, + vdrive->drive.max_segment_size); + + if (features & max_segments) + vdrive->drive.max_segments = + vp_read(&vp->device, struct virtio_blk_config, seg_max); + else + vdrive->drive.max_segments = 0; + dprintf(3, "virtio-blk %pP seg_max=%u.\n", pci, + vdrive->drive.max_segments); + vdrive->drive.sectors = vp_read(&vp->device, struct virtio_blk_config, capacity); if (features & blk_size) { @@ -177,6 +197,21 @@ init_virtio_blk(void *data) vdrive->drive.pchs.cylinder = cfg.cylinders; vdrive->drive.pchs.head = cfg.heads; vdrive->drive.pchs.sector = cfg.sectors; + + if (f & (1 << VIRTIO_BLK_F_SIZE_MAX)) + vdrive->drive.max_segment_size = cfg.size_max; + else + vdrive->drive.max_segment_size = 0; + dprintf(3, "virtio-blk %pP size_max=%u.\n", pci, + vdrive->drive.max_segment_size); + + if (f & (1 << VIRTIO_BLK_F_SEG_MAX)) + vdrive->drive.max_segments = cfg.seg_max; + else + vdrive->drive.max_segments = 0; + dprintf(3, "virtio-blk %pP seg_max=%u.\n", pci, + vdrive->drive.max_segments); + } char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%pP", pci); @@ -218,8 +253,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 +266,20 @@ 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); + else + vdrive->drive.max_segment_size = 0; + dprintf(3, "virtio-blk size_max=%u.\n", vdrive->drive.max_segment_size); + + if (features & max_segments) + vdrive->drive.max_segments = + vp_read(&vp->device, struct virtio_blk_config, seg_max); + else + vdrive->drive.max_segments = 0; + dprintf(3, "virtio-blk seg_max=%u.\n", vdrive->drive.max_segments); + vdrive->drive.sectors = vp_read(&vp->device, struct virtio_blk_config, capacity); if (features & blk_size) { 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 Fri, Nov 26, 2021 at 04:06:23PM +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.
Why is this needed? The code never actually uses max_segment_size and max_segments (other than logging the values) ... take care, Gerd
Hi Gerd, Yes, you are right. I think reading these two config register is fine during init and align with the virtio blk spec. In the near future, we will working on some related work on these two value. To be specific, we will do some modification in virtio blk driver. If driver reads data larger than VIRTIO_BLK_F_SIZE_MAX, it will cause some issue to the DMA engine. To be frankly, I think our future work will have more discussion, so I think it is better to merge these two register reading first. -----Original Message----- From: Gerd Hoffmann <kraxel@redhat.com> Sent: Monday, November 29, 2021 6:53 PM To: Pei, Andy <andy.pei@intel.com> Cc: seabios@seabios.org; kevin@koconnor.net; Ding Limin <dinglimin@cmss.chinamobile.com> Subject: Re: [PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX On Fri, Nov 26, 2021 at 04:06:23PM +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.
Why is this needed? The code never actually uses max_segment_size and max_segments (other than logging the values) ... take care, Gerd
On Tue, Nov 30, 2021 at 05:23:52AM +0000, Pei, Andy wrote:
Hi Gerd,
Yes, you are right. I think reading these two config register is fine during init and align with the virtio blk spec.
It is not wrong but also pointless.
In the near future, we will working on some related work on these two value. To be specific, we will do some modification in virtio blk driver. If driver reads data larger than VIRTIO_BLK_F_SIZE_MAX, it will cause some issue to the DMA engine.
Please submit all changes together as patch series. thanks, Gerd
participants (3)
-
Andy Pei -
Gerd Hoffmann -
Pei, Andy