src/block.h | 2 ++ src/hw/virtio-blk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- src/hw/virtio-blk.h | 3 +++ 3 files changed, 49 insertions(+), 2 deletions(-)
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: dinglimin <dinglimin@cmss.chinamobile.com>
Signed-off-by: Andy Pei <andy.pei@intel.com>
---
src/block.h | 2 ++
src/hw/virtio-blk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
src/hw/virtio-blk.h | 3 +++
3 files changed, 49 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..f315839 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,18 @@ 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;
+
+ if (features & max_segments)
+ vdrive->drive.max_segments =
+ vp_read(&vp->device, struct virtio_blk_config, seg_max);
+ else
+ vdrive->drive.max_segments = 0;
+
vdrive->drive.sectors =
vp_read(&vp->device, struct virtio_blk_config, capacity);
if (features & blk_size) {
@@ -177,6 +193,17 @@ 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;
+
+ if (f & (1 << VIRTIO_BLK_F_SEG_MAX))
+ vdrive->drive.max_segments = cfg.seg_max;
+ else
+ vdrive->drive.max_segments = 0;
+
}
char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%pP", pci);
@@ -218,8 +245,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 +258,18 @@ 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;
+
+ if (features & max_segments)
+ vdrive->drive.max_segments =
+ vp_read(&vp->device, struct virtio_blk_config, seg_max);
+ else
+ vdrive->drive.max_segments = 0;
+
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
Dear Andy,
Thank you for the patch.
Am 26.11.21 um 07:38 schrieb Andy Pei:
> according to virtio spec, add feature VIRTIO_BLK_F_SIZE_MAX
> and VIRTIO_BLK_F_SEG_MAX parse to virtio blk driver.
How can it be tested, that the change is working?
> Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
Is it *Ding Limin*? Is she/he the author of the patch? If so, you could do
git commit --amend --author="Ding Limin
<dinglimin@cmss.chinamobile.com>"
and `git format-patch` should create the patch accordingly.
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
> src/block.h | 2 ++
> src/hw/virtio-blk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
> src/hw/virtio-blk.h | 3 +++
> 3 files changed, 49 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..f315839 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,18 @@ 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;
> +
> + if (features & max_segments)
> + vdrive->drive.max_segments =
> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
> + else
> + vdrive->drive.max_segments = 0;
> +
Maybe print a level 3 message with the values as it’s done for
`drive.sectors` and `drive.blksize`?
Kind regards,
Paul
> vdrive->drive.sectors =
> vp_read(&vp->device, struct virtio_blk_config, capacity);
> if (features & blk_size) {
> @@ -177,6 +193,17 @@ 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;
> +
> + if (f & (1 << VIRTIO_BLK_F_SEG_MAX))
> + vdrive->drive.max_segments = cfg.seg_max;
> + else
> + vdrive->drive.max_segments = 0;
> +
> }
>
> char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%pP", pci);
> @@ -218,8 +245,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 +258,18 @@ 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;
> +
> + if (features & max_segments)
> + vdrive->drive.max_segments =
> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
> + else
> + vdrive->drive.max_segments = 0;
> +
> 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. */
>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
Hi Paul,
Thanks for your response.
Reply inline.
-----Original Message-----
From: Paul Menzel <pmenzel@molgen.mpg.de>
Sent: Friday, November 26, 2021 4:08 PM
To: Pei, Andy <andy.pei@intel.com>
Cc: dinglimin@cmss.chinamobile.com; seabios@seabios.org; Kevin O'Connor <kevin@koconnor.net>; Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [SeaBIOS] [PATCH] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
Dear Andy,
Thank you for the patch.
Am 26.11.21 um 07:38 schrieb Andy Pei:
> according to virtio spec, add feature VIRTIO_BLK_F_SIZE_MAX and
> VIRTIO_BLK_F_SEG_MAX parse to virtio blk driver.
How can it be tested, that the change is working?
Yes. Dinglimin tested in their environment.
> Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
Is it *Ding Limin*? Is she/he the author of the patch? If so, you could do
git commit --amend --author="Ding Limin <dinglimin@cmss.chinamobile.com>"
and `git format-patch` should create the patch accordingly.
We co-work on this patch.
> Signed-off-by: Andy Pei <andy.pei@intel.com>
> ---
> src/block.h | 2 ++
> src/hw/virtio-blk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
> src/hw/virtio-blk.h | 3 +++
> 3 files changed, 49 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..f315839 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,18 @@ 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;
> +
> + if (features & max_segments)
> + vdrive->drive.max_segments =
> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
> + else
> + vdrive->drive.max_segments = 0;
> +
Maybe print a level 3 message with the values as it’s done for `drive.sectors` and `drive.blksize`?
I think I can sent a V2 patch, with your suggest log output.
Kind regards,
Paul
> vdrive->drive.sectors =
> vp_read(&vp->device, struct virtio_blk_config, capacity);
> if (features & blk_size) {
> @@ -177,6 +193,17 @@ 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;
> +
> + if (f & (1 << VIRTIO_BLK_F_SEG_MAX))
> + vdrive->drive.max_segments = cfg.seg_max;
> + else
> + vdrive->drive.max_segments = 0;
> +
> }
>
> char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%pP", pci);
> @@ -218,8 +245,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 +258,18 @@ 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;
> +
> + if (features & max_segments)
> + vdrive->drive.max_segments =
> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
> + else
> + vdrive->drive.max_segments = 0;
> +
> 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. */
>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
Dear Andy,
Am 26.11.21 um 09:15 schrieb Pei, Andy:
[…]
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Friday, November 26, 2021 4:08 PM
[…]
> Subject: Re: [SeaBIOS] [PATCH] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
> Am 26.11.21 um 07:38 schrieb Andy Pei:
>> according to virtio spec, add feature VIRTIO_BLK_F_SIZE_MAX and
>> VIRTIO_BLK_F_SEG_MAX parse to virtio blk driver.
>
> How can it be tested, that the change is working?
> Yes. Dinglimin tested in their environment.
[Maybe mark/tag your response somehow, or use a mailer supporting to
quote the reply correctly. ;-)]
But how was the test exactly done?
>> Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
>
> Is it *Ding Limin*? Is she/he the author of the patch? If so, you could do
>
> git commit --amend --author="Ding Limin <dinglimin@cmss.chinamobile.com>"
>
> and `git format-patch` should create the patch accordingly.
>
> We co-work on this patch.
Good to know. If the name is really one word, please spell it with a
capital letter Dinglimin.
>> Signed-off-by: Andy Pei <andy.pei@intel.com>
>> ---
>> src/block.h | 2 ++
>> src/hw/virtio-blk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>> src/hw/virtio-blk.h | 3 +++
>> 3 files changed, 49 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..f315839 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,18 @@ 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;
>> +
>> + if (features & max_segments)
>> + vdrive->drive.max_segments =
>> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
>> + else
>> + vdrive->drive.max_segments = 0;
>> +
>
> Maybe print a level 3 message with the values as it’s done for `drive.sectors` and `drive.blksize`?
> I think I can sent a V2 patch, with your suggest log output.
Awesome.
Kind regards,
Paul
>> vdrive->drive.sectors =
>> vp_read(&vp->device, struct virtio_blk_config, capacity);
>> if (features & blk_size) {
>> @@ -177,6 +193,17 @@ 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;
>> +
>> + if (f & (1 << VIRTIO_BLK_F_SEG_MAX))
>> + vdrive->drive.max_segments = cfg.seg_max;
>> + else
>> + vdrive->drive.max_segments = 0;
>> +
>> }
>>
>> char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%pP", pci);
>> @@ -218,8 +245,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 +258,18 @@ 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;
>> +
>> + if (features & max_segments)
>> + vdrive->drive.max_segments =
>> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
>> + else
>> + vdrive->drive.max_segments = 0;
>> +
>> 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. */
>>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
Hi Paul,
Reply inline.
-----Original Message-----
From: Paul Menzel <pmenzel@molgen.mpg.de>
Sent: Friday, November 26, 2021 4:33 PM
To: Pei, Andy <andy.pei@intel.com>
Cc: dinglimin@cmss.chinamobile.com; seabios <seabios@seabios.org>; Kevin O'Connor <kevin@koconnor.net>; Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [SeaBIOS] [PATCH] src/hw/virtio-blk: add feature VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
Dear Andy,
Am 26.11.21 um 09:15 schrieb Pei, Andy:
[…]
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Friday, November 26, 2021 4:08 PM
[…]
> Subject: Re: [SeaBIOS] [PATCH] src/hw/virtio-blk: add feature
> VIRTIO_BLK_F_SIZE_MAX and VIRTIO_BLK_F_SEG_MAX
> Am 26.11.21 um 07:38 schrieb Andy Pei:
>> according to virtio spec, add feature VIRTIO_BLK_F_SIZE_MAX and
>> VIRTIO_BLK_F_SEG_MAX parse to virtio blk driver.
>
> How can it be tested, that the change is working?
> Yes. Dinglimin tested in their environment.
[Maybe mark/tag your response somehow, or use a mailer supporting to quote the reply correctly. ;-)]
But how was the test exactly done?
Andy:
This patch follow the VIRTIO SPEC to read virtio blk cfg register.
There will be a another patch, which will make some modification in seabois virtio blk driver.
I think there will be some discussion on that patch.
This patch just read register following the spec, which should cause no doubt.
>> Signed-off-by: dinglimin <dinglimin@cmss.chinamobile.com>
>
> Is it *Ding Limin*? Is she/he the author of the patch? If so, you
> could do
>
> git commit --amend --author="Ding Limin <dinglimin@cmss.chinamobile.com>"
>
> and `git format-patch` should create the patch accordingly.
>
> We co-work on this patch.
Good to know. If the name is really one word, please spell it with a capital letter Dinglimin.
Andy:
OK, thanks for your suggestion.
>> Signed-off-by: Andy Pei <andy.pei@intel.com>
>> ---
>> src/block.h | 2 ++
>> src/hw/virtio-blk.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
>> src/hw/virtio-blk.h | 3 +++
>> 3 files changed, 49 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..f315839 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,18 @@
>> 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;
>> +
>> + if (features & max_segments)
>> + vdrive->drive.max_segments =
>> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
>> + else
>> + vdrive->drive.max_segments = 0;
>> +
>
> Maybe print a level 3 message with the values as it’s done for `drive.sectors` and `drive.blksize`?
> I think I can sent a V2 patch, with your suggest log output.
Awesome.
Andy:
Thanks.
Kind regards,
Paul
>> vdrive->drive.sectors =
>> vp_read(&vp->device, struct virtio_blk_config, capacity);
>> if (features & blk_size) { @@ -177,6 +193,17 @@
>> 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;
>> +
>> + if (f & (1 << VIRTIO_BLK_F_SEG_MAX))
>> + vdrive->drive.max_segments = cfg.seg_max;
>> + else
>> + vdrive->drive.max_segments = 0;
>> +
>> }
>>
>> char *desc = znprintf(MAXDESCSIZE, "Virtio disk PCI:%pP",
>> pci); @@ -218,8 +245,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 +258,18 @@ 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;
>> +
>> + if (features & max_segments)
>> + vdrive->drive.max_segments =
>> + vp_read(&vp->device, struct virtio_blk_config, seg_max);
>> + else
>> + vdrive->drive.max_segments = 0;
>> +
>> 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. */
>>
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-leave@seabios.org
© 2016 - 2024 Red Hat, Inc.