[SeaBIOS] [PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX

Andy Pei posted 1 patch 10 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/1637913983-113758-1-git-send-email-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(-)
[SeaBIOS] [PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
Posted by Andy Pei 10 months, 1 week ago
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

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
Posted by Gerd Hoffmann 10 months ago
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

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
Posted by Pei, Andy 10 months ago
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

_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
[SeaBIOS] Re: [PATCH v2] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
Posted by Gerd Hoffmann 10 months ago
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

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