* [PATCH v3 1/5] virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
2022-09-06 7:31 [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
@ 2022-09-06 7:31 ` Daniil Tatianin
2022-09-06 7:31 ` [PATCH v3 2/5] virtio-blk: move config size params to virtio-blk-common Daniil Tatianin
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Daniil Tatianin @ 2022-09-06 7:31 UTC (permalink / raw)
To: qemu-devel
Cc: yc-core, mst, stefanha, raphael.norwitz, kwolf, qemu-block,
jasowang, d-tatianin
This is the first step towards moving all device config size calculation
logic into the virtio core code. In particular, this adds a struct that
contains all the necessary information for common virtio code to be able
to calculate the final config size for a device. This is expected to be
used with the new virtio_get_config_size helper, which calculates the
final length based on the provided host features.
This builds on top of already existing code like VirtIOFeature and
virtio_feature_get_config_size(), but adds additional fields, as well as
sanity checking so that device-specifc code doesn't have to duplicate it.
An example usage would be:
static const VirtIOFeature dev_features[] = {
{.flags = 1ULL << FEATURE_1_BIT,
.end = endof(struct virtio_dev_config, feature_1)},
{.flags = 1ULL << FEATURE_2_BIT,
.end = endof(struct virtio_dev_config, feature_2)},
{}
};
static const VirtIOConfigSizeParams dev_cfg_size_params = {
.min_size = DEV_BASE_CONFIG_SIZE,
.max_size = sizeof(struct virtio_dev_config),
.feature_sizes = dev_features
};
// code inside my_dev_device_realize()
size_t config_size = virtio_get_config_size(&dev_cfg_size_params,
host_features);
virtio_init(vdev, VIRTIO_ID_MYDEV, config_size);
Currently every device is expected to write its own boilerplate from the
example above in device_realize(), however, the next step of this
transition is moving VirtIOConfigSizeParams into VirtioDeviceClass,
so that it can be done automatically by the virtio initialization code.
All of the users of virtio_feature_get_config_size have been converted
to use virtio_get_config_size so it's no longer needed and is removed
with this commit.
Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
---
hw/block/virtio-blk.c | 16 +++++++---------
hw/net/virtio-net.c | 9 +++++++--
hw/virtio/virtio.c | 10 ++++++----
include/hw/virtio/virtio.h | 10 ++++++++--
4 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e9ba752f6b..10c47c2934 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -49,13 +49,11 @@ static const VirtIOFeature feature_sizes[] = {
{}
};
-static void virtio_blk_set_config_size(VirtIOBlock *s, uint64_t host_features)
-{
- s->config_size = MAX(VIRTIO_BLK_CFG_SIZE,
- virtio_feature_get_config_size(feature_sizes, host_features));
-
- assert(s->config_size <= sizeof(struct virtio_blk_config));
-}
+static const VirtIOConfigSizeParams cfg_size_params = {
+ .min_size = VIRTIO_BLK_CFG_SIZE,
+ .max_size = sizeof(struct virtio_blk_config),
+ .feature_sizes = feature_sizes
+};
static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
VirtIOBlockReq *req)
@@ -1204,8 +1202,8 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
return;
}
- virtio_blk_set_config_size(s, s->host_features);
-
+ s->config_size = virtio_get_config_size(&cfg_size_params,
+ s->host_features);
virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
s->blk = conf->conf.blk;
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index dd0d056fde..78b003a158 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -106,6 +106,12 @@ static const VirtIOFeature feature_sizes[] = {
{}
};
+static const VirtIOConfigSizeParams cfg_size_params = {
+ .min_size = endof(struct virtio_net_config, mac),
+ .max_size = sizeof(struct virtio_net_config),
+ .feature_sizes = feature_sizes
+};
+
static VirtIONetQueue *virtio_net_get_subqueue(NetClientState *nc)
{
VirtIONet *n = qemu_get_nic_opaque(nc);
@@ -3246,8 +3252,7 @@ static void virtio_net_set_config_size(VirtIONet *n, uint64_t host_features)
{
virtio_add_feature(&host_features, VIRTIO_NET_F_MAC);
- n->config_size = virtio_feature_get_config_size(feature_sizes,
- host_features);
+ n->config_size = virtio_get_config_size(&cfg_size_params, host_features);
}
void virtio_net_set_netclient_name(VirtIONet *n, const char *name,
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 5d607aeaa0..c0bf8dd6fd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2999,11 +2999,12 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
return ret;
}
-size_t virtio_feature_get_config_size(const VirtIOFeature *feature_sizes,
- uint64_t host_features)
+size_t virtio_get_config_size(const VirtIOConfigSizeParams *params,
+ uint64_t host_features)
{
- size_t config_size = 0;
- int i;
+ size_t config_size = params->min_size;
+ const VirtIOFeature *feature_sizes = params->feature_sizes;
+ size_t i;
for (i = 0; feature_sizes[i].flags != 0; i++) {
if (host_features & feature_sizes[i].flags) {
@@ -3011,6 +3012,7 @@ size_t virtio_feature_get_config_size(const VirtIOFeature *feature_sizes,
}
}
+ assert(config_size <= params->max_size);
return config_size;
}
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index db1c0ddf6b..f3ff6710d5 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -44,8 +44,14 @@ typedef struct VirtIOFeature {
size_t end;
} VirtIOFeature;
-size_t virtio_feature_get_config_size(const VirtIOFeature *features,
- uint64_t host_features);
+typedef struct VirtIOConfigSizeParams {
+ size_t min_size;
+ size_t max_size;
+ const VirtIOFeature *feature_sizes;
+} VirtIOConfigSizeParams;
+
+size_t virtio_get_config_size(const VirtIOConfigSizeParams *params,
+ uint64_t host_features);
typedef struct VirtQueue VirtQueue;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 2/5] virtio-blk: move config size params to virtio-blk-common
2022-09-06 7:31 [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
2022-09-06 7:31 ` [PATCH v3 1/5] virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size Daniil Tatianin
@ 2022-09-06 7:31 ` Daniil Tatianin
2022-09-06 7:31 ` [PATCH v3 3/5] vhost-user-blk: make it possible to disable write-zeroes/discard Daniil Tatianin
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Daniil Tatianin @ 2022-09-06 7:31 UTC (permalink / raw)
To: qemu-devel
Cc: yc-core, mst, stefanha, raphael.norwitz, kwolf, qemu-block,
jasowang, d-tatianin
This way we can reuse it for other virtio-blk devices, e.g
vhost-user-blk, which currently does not control its config space size
dynamically.
Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
MAINTAINERS | 2 ++
hw/block/meson.build | 2 +-
hw/block/virtio-blk-common.c | 39 +++++++++++++++++++++++++++
hw/block/virtio-blk.c | 24 ++---------------
include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
5 files changed, 64 insertions(+), 23 deletions(-)
create mode 100644 hw/block/virtio-blk-common.c
create mode 100644 include/hw/virtio/virtio-blk-common.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 5ce4227ff6..2858577c5b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2030,8 +2030,10 @@ virtio-blk
M: Stefan Hajnoczi <stefanha@redhat.com>
L: qemu-block@nongnu.org
S: Supported
+F: hw/block/virtio-blk-common.c
F: hw/block/virtio-blk.c
F: hw/block/dataplane/*
+F: include/hw/virtio/virtio-blk-common.h
F: tests/qtest/virtio-blk-test.c
T: git https://github.com/stefanha/qemu.git block
diff --git a/hw/block/meson.build b/hw/block/meson.build
index 2389326112..8ee1f1f850 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -16,7 +16,7 @@ softmmu_ss.add(when: 'CONFIG_SWIM', if_true: files('swim.c'))
softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
softmmu_ss.add(when: 'CONFIG_TC58128', if_true: files('tc58128.c'))
-specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c'))
+specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 'virtio-blk-common.c'))
specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c'))
subdir('dataplane')
diff --git a/hw/block/virtio-blk-common.c b/hw/block/virtio-blk-common.c
new file mode 100644
index 0000000000..ac52d7c176
--- /dev/null
+++ b/hw/block/virtio-blk-common.c
@@ -0,0 +1,39 @@
+/*
+ * Virtio Block Device common helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+
+#include "standard-headers/linux/virtio_blk.h"
+#include "hw/virtio/virtio.h"
+#include "hw/virtio/virtio-blk-common.h"
+
+/* Config size before the discard support (hide associated config fields) */
+#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
+ max_discard_sectors)
+
+/*
+ * Starting from the discard feature, we can use this array to properly
+ * set the config size depending on the features enabled.
+ */
+static const VirtIOFeature feature_sizes[] = {
+ {.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
+ .end = endof(struct virtio_blk_config, discard_sector_alignment)},
+ {.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES,
+ .end = endof(struct virtio_blk_config, write_zeroes_may_unmap)},
+ {}
+};
+
+const VirtIOConfigSizeParams virtio_blk_cfg_size_params = {
+ .min_size = VIRTIO_BLK_CFG_SIZE,
+ .max_size = sizeof(struct virtio_blk_config),
+ .feature_sizes = feature_sizes
+};
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 10c47c2934..8131ec2dbc 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -32,29 +32,9 @@
#include "hw/virtio/virtio-bus.h"
#include "migration/qemu-file-types.h"
#include "hw/virtio/virtio-access.h"
+#include "hw/virtio/virtio-blk-common.h"
#include "qemu/coroutine.h"
-/* Config size before the discard support (hide associated config fields) */
-#define VIRTIO_BLK_CFG_SIZE offsetof(struct virtio_blk_config, \
- max_discard_sectors)
-/*
- * Starting from the discard feature, we can use this array to properly
- * set the config size depending on the features enabled.
- */
-static const VirtIOFeature feature_sizes[] = {
- {.flags = 1ULL << VIRTIO_BLK_F_DISCARD,
- .end = endof(struct virtio_blk_config, discard_sector_alignment)},
- {.flags = 1ULL << VIRTIO_BLK_F_WRITE_ZEROES,
- .end = endof(struct virtio_blk_config, write_zeroes_may_unmap)},
- {}
-};
-
-static const VirtIOConfigSizeParams cfg_size_params = {
- .min_size = VIRTIO_BLK_CFG_SIZE,
- .max_size = sizeof(struct virtio_blk_config),
- .feature_sizes = feature_sizes
-};
-
static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq,
VirtIOBlockReq *req)
{
@@ -1202,7 +1182,7 @@ static void virtio_blk_device_realize(DeviceState *dev, Error **errp)
return;
}
- s->config_size = virtio_get_config_size(&cfg_size_params,
+ s->config_size = virtio_get_config_size(&virtio_blk_cfg_size_params,
s->host_features);
virtio_init(vdev, VIRTIO_ID_BLOCK, s->config_size);
diff --git a/include/hw/virtio/virtio-blk-common.h b/include/hw/virtio/virtio-blk-common.h
new file mode 100644
index 0000000000..31daada3e3
--- /dev/null
+++ b/include/hw/virtio/virtio-blk-common.h
@@ -0,0 +1,20 @@
+/*
+ * Virtio Block Device common helpers
+ *
+ * Copyright IBM, Corp. 2007
+ *
+ * Authors:
+ * Anthony Liguori <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2. See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef VIRTIO_BLK_COMMON_H
+#define VIRTIO_BLK_COMMON_H
+
+#include "hw/virtio/virtio.h"
+
+extern const VirtIOConfigSizeParams virtio_blk_cfg_size_params;
+
+#endif
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 3/5] vhost-user-blk: make it possible to disable write-zeroes/discard
2022-09-06 7:31 [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
2022-09-06 7:31 ` [PATCH v3 1/5] virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size Daniil Tatianin
2022-09-06 7:31 ` [PATCH v3 2/5] virtio-blk: move config size params to virtio-blk-common Daniil Tatianin
@ 2022-09-06 7:31 ` Daniil Tatianin
2022-09-06 7:31 ` [PATCH v3 4/5] vhost-user-blk: make 'config_wce' part of 'host_features' Daniil Tatianin
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Daniil Tatianin @ 2022-09-06 7:31 UTC (permalink / raw)
To: qemu-devel
Cc: yc-core, mst, stefanha, raphael.norwitz, kwolf, qemu-block,
jasowang, d-tatianin
It is useful to have the ability to disable these features for
compatibility with older VMs that don't have these implemented.
Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
hw/block/vhost-user-blk.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 9117222456..4c9727e08c 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -259,8 +259,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
virtio_add_feature(&features, VIRTIO_BLK_F_BLK_SIZE);
virtio_add_feature(&features, VIRTIO_BLK_F_FLUSH);
virtio_add_feature(&features, VIRTIO_BLK_F_RO);
- virtio_add_feature(&features, VIRTIO_BLK_F_DISCARD);
- virtio_add_feature(&features, VIRTIO_BLK_F_WRITE_ZEROES);
if (s->config_wce) {
virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
@@ -592,6 +590,10 @@ static Property vhost_user_blk_properties[] = {
VHOST_USER_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
+ DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
+ VIRTIO_BLK_F_DISCARD, true),
+ DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, parent_obj.host_features,
+ VIRTIO_BLK_F_WRITE_ZEROES, true),
DEFINE_PROP_END_OF_LIST(),
};
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 4/5] vhost-user-blk: make 'config_wce' part of 'host_features'
2022-09-06 7:31 [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
` (2 preceding siblings ...)
2022-09-06 7:31 ` [PATCH v3 3/5] vhost-user-blk: make it possible to disable write-zeroes/discard Daniil Tatianin
@ 2022-09-06 7:31 ` Daniil Tatianin
2022-09-06 7:31 ` [PATCH v3 5/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Daniil Tatianin @ 2022-09-06 7:31 UTC (permalink / raw)
To: qemu-devel
Cc: yc-core, mst, stefanha, raphael.norwitz, kwolf, qemu-block,
jasowang, d-tatianin
No reason to have this be a separate field. This also makes it more akin
to what the virtio-blk device does.
Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
hw/block/vhost-user-blk.c | 6 ++----
include/hw/virtio/vhost-user-blk.h | 1 -
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 4c9727e08c..0d916edefa 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -260,9 +260,6 @@ static uint64_t vhost_user_blk_get_features(VirtIODevice *vdev,
virtio_add_feature(&features, VIRTIO_BLK_F_FLUSH);
virtio_add_feature(&features, VIRTIO_BLK_F_RO);
- if (s->config_wce) {
- virtio_add_feature(&features, VIRTIO_BLK_F_CONFIG_WCE);
- }
if (s->num_queues > 1) {
virtio_add_feature(&features, VIRTIO_BLK_F_MQ);
}
@@ -589,7 +586,8 @@ static Property vhost_user_blk_properties[] = {
DEFINE_PROP_UINT16("num-queues", VHostUserBlk, num_queues,
VHOST_USER_BLK_AUTO_NUM_QUEUES),
DEFINE_PROP_UINT32("queue-size", VHostUserBlk, queue_size, 128),
- DEFINE_PROP_BIT("config-wce", VHostUserBlk, config_wce, 0, true),
+ DEFINE_PROP_BIT64("config-wce", VHostUserBlk, parent_obj.host_features,
+ VIRTIO_BLK_F_CONFIG_WCE, true),
DEFINE_PROP_BIT64("discard", VHostUserBlk, parent_obj.host_features,
VIRTIO_BLK_F_DISCARD, true),
DEFINE_PROP_BIT64("write-zeroes", VHostUserBlk, parent_obj.host_features,
diff --git a/include/hw/virtio/vhost-user-blk.h b/include/hw/virtio/vhost-user-blk.h
index 7c91f15040..ea085ee1ed 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -34,7 +34,6 @@ struct VHostUserBlk {
struct virtio_blk_config blkcfg;
uint16_t num_queues;
uint32_t queue_size;
- uint32_t config_wce;
struct vhost_dev dev;
struct vhost_inflight *inflight;
VhostUserState vhost_user;
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v3 5/5] vhost-user-blk: dynamically resize config space based on features
2022-09-06 7:31 [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
` (3 preceding siblings ...)
2022-09-06 7:31 ` [PATCH v3 4/5] vhost-user-blk: make 'config_wce' part of 'host_features' Daniil Tatianin
@ 2022-09-06 7:31 ` Daniil Tatianin
2022-09-07 4:02 ` [PATCH v3 0/5] " Raphael Norwitz
2022-10-11 7:20 ` Daniil Tatianin
6 siblings, 0 replies; 11+ messages in thread
From: Daniil Tatianin @ 2022-09-06 7:31 UTC (permalink / raw)
To: qemu-devel
Cc: yc-core, mst, stefanha, raphael.norwitz, kwolf, qemu-block,
jasowang, d-tatianin
Make vhost-user-blk backwards compatible when migrating from older VMs
running with modern features turned off, the same way it was done for
virtio-blk in 20764be0421c ("virtio-blk: set config size depending on the features enabled")
It's currently impossible to migrate from an older VM with
vhost-user-blk (with disable-legacy=off) because of errors like this:
qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10 read: 41 device: 1 cmask: ff wmask: 80 w1cmask:0
qemu-system-x86_64: Failed to load PCIDevice:config
qemu-system-x86_64: Failed to load virtio-blk:virtio
qemu-system-x86_64: error while loading state for instance 0x0 of device '0000:00:05.0:00.0:02.0/virtio-blk'
qemu-system-x86_64: load of migration failed: Invalid argument
This is caused by the newer (destination) VM requiring a bigger BAR0
alignment because it has to cover a bigger configuration space, which
isn't actually needed since those additional config fields are not
active (write-zeroes/discard).
Signed-off-by: Daniil Tatianin <d-tatianin@yandex-team.ru>
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
MAINTAINERS | 2 ++
hw/block/meson.build | 2 +-
hw/block/vhost-user-blk.c | 17 ++++++++++-------
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 2858577c5b..a7d3914735 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2273,11 +2273,13 @@ S: Maintained
F: contrib/vhost-user-blk/
F: contrib/vhost-user-scsi/
F: hw/block/vhost-user-blk.c
+F: hw/block/virtio-blk-common.c
F: hw/scsi/vhost-user-scsi.c
F: hw/virtio/vhost-user-blk-pci.c
F: hw/virtio/vhost-user-scsi-pci.c
F: include/hw/virtio/vhost-user-blk.h
F: include/hw/virtio/vhost-user-scsi.h
+F: include/hw/virtio/virtio-blk-common.h
vhost-user-gpu
M: Marc-André Lureau <marcandre.lureau@redhat.com>
diff --git a/hw/block/meson.build b/hw/block/meson.build
index 8ee1f1f850..1908abd45c 100644
--- a/hw/block/meson.build
+++ b/hw/block/meson.build
@@ -17,6 +17,6 @@ softmmu_ss.add(when: 'CONFIG_XEN', if_true: files('xen-block.c'))
softmmu_ss.add(when: 'CONFIG_TC58128', if_true: files('tc58128.c'))
specific_ss.add(when: 'CONFIG_VIRTIO_BLK', if_true: files('virtio-blk.c', 'virtio-blk-common.c'))
-specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c'))
+specific_ss.add(when: 'CONFIG_VHOST_USER_BLK', if_true: files('vhost-user-blk.c', 'virtio-blk-common.c'))
subdir('dataplane')
diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 0d916edefa..8dd063eb96 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -23,6 +23,7 @@
#include "hw/qdev-core.h"
#include "hw/qdev-properties.h"
#include "hw/qdev-properties-system.h"
+#include "hw/virtio/virtio-blk-common.h"
#include "hw/virtio/vhost.h"
#include "hw/virtio/vhost-user-blk.h"
#include "hw/virtio/virtio.h"
@@ -63,7 +64,7 @@ static void vhost_user_blk_update_config(VirtIODevice *vdev, uint8_t *config)
/* Our num_queues overrides the device backend */
virtio_stw_p(vdev, &s->blkcfg.num_queues, s->num_queues);
- memcpy(config, &s->blkcfg, sizeof(struct virtio_blk_config));
+ memcpy(config, &s->blkcfg, vdev->config_len);
}
static void vhost_user_blk_set_config(VirtIODevice *vdev, const uint8_t *config)
@@ -92,12 +93,12 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
{
int ret;
struct virtio_blk_config blkcfg;
+ VirtIODevice *vdev = dev->vdev;
VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
Error *local_err = NULL;
ret = vhost_dev_get_config(dev, (uint8_t *)&blkcfg,
- sizeof(struct virtio_blk_config),
- &local_err);
+ vdev->config_len, &local_err);
if (ret < 0) {
error_report_err(local_err);
return ret;
@@ -106,7 +107,7 @@ static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
/* valid for resize only */
if (blkcfg.capacity != s->blkcfg.capacity) {
s->blkcfg.capacity = blkcfg.capacity;
- memcpy(dev->vdev->config, &s->blkcfg, sizeof(struct virtio_blk_config));
+ memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len);
virtio_notify_config(dev->vdev);
}
@@ -442,7 +443,7 @@ static int vhost_user_blk_realize_connect(VHostUserBlk *s, Error **errp)
assert(s->connected);
ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
- sizeof(struct virtio_blk_config), errp);
+ s->parent_obj.config_len, errp);
if (ret < 0) {
qemu_chr_fe_disconnect(&s->chardev);
vhost_dev_cleanup(&s->dev);
@@ -457,6 +458,7 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
ERRP_GUARD();
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VHostUserBlk *s = VHOST_USER_BLK(vdev);
+ size_t config_size;
int retries;
int i, ret;
@@ -487,8 +489,9 @@ static void vhost_user_blk_device_realize(DeviceState *dev, Error **errp)
return;
}
- virtio_init(vdev, VIRTIO_ID_BLOCK,
- sizeof(struct virtio_blk_config));
+ config_size = virtio_get_config_size(&virtio_blk_cfg_size_params,
+ vdev->host_features);
+ virtio_init(vdev, VIRTIO_ID_BLOCK, config_size);
s->virtqs = g_new(VirtQueue *, s->num_queues);
for (i = 0; i < s->num_queues; i++) {
--
2.25.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features
2022-09-06 7:31 [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
` (4 preceding siblings ...)
2022-09-06 7:31 ` [PATCH v3 5/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
@ 2022-09-07 4:02 ` Raphael Norwitz
2022-09-12 14:54 ` Daniil Tatianin
2022-10-11 7:20 ` Daniil Tatianin
6 siblings, 1 reply; 11+ messages in thread
From: Raphael Norwitz @ 2022-09-07 4:02 UTC (permalink / raw)
To: Daniil Tatianin
Cc: qemu-devel@nongnu.org, yc-core@yandex-team.ru, mst@redhat.com,
stefanha@redhat.com, Raphael Norwitz, kwolf@redhat.com,
qemu-block@nongnu.org, jasowang@redhat.com
Thanks for the changes. For the whole series:
Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
On Tue, Sep 06, 2022 at 10:31:06AM +0300, Daniil Tatianin wrote:
> This patch set attempts to align vhost-user-blk with virtio-blk in
> terms of backward compatibility and flexibility. It also improves
> the virtio core by introducing new common code that can be used by
> a virtio device to calculate its config space size.
>
> In particular it adds the following things:
> - Common virtio code for deducing the required device config size based
> on provided host features.
> - Ability to disable modern virtio-blk features like
> discard/write-zeroes for vhost-user-blk.
> - Dynamic configuration space resizing based on enabled features,
> by reusing the common code introduced in the earlier commits.
> - Cleans up the VHostUserBlk structure by reusing parent fields.
>
> Changes since v1 (mostly addresses Stefan's feedback):
> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
> only hold the virtio-blk config size parameters.
> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
>
> Changes since v2:
> - Squash the first four commits into one
> - Set .min_size for virtio-net as well
> - Move maintainer/meson user-blk bits to the last commit
>
> Daniil Tatianin (5):
> virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
> virtio-blk: move config size params to virtio-blk-common
> vhost-user-blk: make it possible to disable write-zeroes/discard
> vhost-user-blk: make 'config_wce' part of 'host_features'
> vhost-user-blk: dynamically resize config space based on features
>
> MAINTAINERS | 4 +++
> hw/block/meson.build | 4 +--
> hw/block/vhost-user-blk.c | 29 +++++++++++---------
> hw/block/virtio-blk-common.c | 39 +++++++++++++++++++++++++++
> hw/block/virtio-blk.c | 28 +++----------------
> hw/net/virtio-net.c | 9 +++++--
> hw/virtio/virtio.c | 10 ++++---
> include/hw/virtio/vhost-user-blk.h | 1 -
> include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
> include/hw/virtio/virtio.h | 10 +++++--
> 10 files changed, 105 insertions(+), 49 deletions(-)
> create mode 100644 hw/block/virtio-blk-common.c
> create mode 100644 include/hw/virtio/virtio-blk-common.h
>
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features
2022-09-07 4:02 ` [PATCH v3 0/5] " Raphael Norwitz
@ 2022-09-12 14:54 ` Daniil Tatianin
2022-09-12 17:03 ` Raphael Norwitz
0 siblings, 1 reply; 11+ messages in thread
From: Daniil Tatianin @ 2022-09-12 14:54 UTC (permalink / raw)
To: Raphael Norwitz
Cc: qemu-devel@nongnu.org, yc-core@yandex-team.ru, mst@redhat.com,
stefanha@redhat.com, kwolf@redhat.com, qemu-block@nongnu.org,
jasowang@redhat.com
Thanks for reviewing! Could you send a Pull request? Or do we need an
ack from someone else?
On 9/7/22 7:02 AM, Raphael Norwitz wrote:
> Thanks for the changes. For the whole series:
>
> Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
>
> On Tue, Sep 06, 2022 at 10:31:06AM +0300, Daniil Tatianin wrote:
>> This patch set attempts to align vhost-user-blk with virtio-blk in
>> terms of backward compatibility and flexibility. It also improves
>> the virtio core by introducing new common code that can be used by
>> a virtio device to calculate its config space size.
>>
>> In particular it adds the following things:
>> - Common virtio code for deducing the required device config size based
>> on provided host features.
>> - Ability to disable modern virtio-blk features like
>> discard/write-zeroes for vhost-user-blk.
>> - Dynamic configuration space resizing based on enabled features,
>> by reusing the common code introduced in the earlier commits.
>> - Cleans up the VHostUserBlk structure by reusing parent fields.
>>
>> Changes since v1 (mostly addresses Stefan's feedback):
>> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
>> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
>> only hold the virtio-blk config size parameters.
>> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
>>
>> Changes since v2:
>> - Squash the first four commits into one
>> - Set .min_size for virtio-net as well
>> - Move maintainer/meson user-blk bits to the last commit
>>
>> Daniil Tatianin (5):
>> virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
>> virtio-blk: move config size params to virtio-blk-common
>> vhost-user-blk: make it possible to disable write-zeroes/discard
>> vhost-user-blk: make 'config_wce' part of 'host_features'
>> vhost-user-blk: dynamically resize config space based on features
>>
>> MAINTAINERS | 4 +++
>> hw/block/meson.build | 4 +--
>> hw/block/vhost-user-blk.c | 29 +++++++++++---------
>> hw/block/virtio-blk-common.c | 39 +++++++++++++++++++++++++++
>> hw/block/virtio-blk.c | 28 +++----------------
>> hw/net/virtio-net.c | 9 +++++--
>> hw/virtio/virtio.c | 10 ++++---
>> include/hw/virtio/vhost-user-blk.h | 1 -
>> include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
>> include/hw/virtio/virtio.h | 10 +++++--
>> 10 files changed, 105 insertions(+), 49 deletions(-)
>> create mode 100644 hw/block/virtio-blk-common.c
>> create mode 100644 include/hw/virtio/virtio-blk-common.h
>>
>> --
>> 2.25.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features
2022-09-12 14:54 ` Daniil Tatianin
@ 2022-09-12 17:03 ` Raphael Norwitz
0 siblings, 0 replies; 11+ messages in thread
From: Raphael Norwitz @ 2022-09-12 17:03 UTC (permalink / raw)
To: Daniil Tatianin
Cc: qemu-devel@nongnu.org, yc-core@yandex-team.ru, mst@redhat.com,
stefanha@redhat.com, kwolf@redhat.com, qemu-block@nongnu.org,
jasowang@redhat.com
> Thanks for reviewing! Could you send a Pull request? Or do we need an
> ack from someone else?
mst typically includes the vhost-user-blk patches in his PRs. Usually a few
other people review but I'm not sure it's required.
A lot of folks have been busy prepping for KVM Forum the last few weeks
so I would expect delayed responses/reviews.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features
2022-09-06 7:31 [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features Daniil Tatianin
` (5 preceding siblings ...)
2022-09-07 4:02 ` [PATCH v3 0/5] " Raphael Norwitz
@ 2022-10-11 7:20 ` Daniil Tatianin
2022-10-11 7:21 ` Daniil Tatianin
6 siblings, 1 reply; 11+ messages in thread
From: Daniil Tatianin @ 2022-10-11 7:20 UTC (permalink / raw)
To: qemu-devel
Cc: yc-core, mst, stefanha, raphael.norwitz, kwolf, qemu-block,
jasowang
Ping :)
On 9/6/22 10:31 AM, Daniil Tatianin wrote:
> This patch set attempts to align vhost-user-blk with virtio-blk in
> terms of backward compatibility and flexibility. It also improves
> the virtio core by introducing new common code that can be used by
> a virtio device to calculate its config space size.
>
> In particular it adds the following things:
> - Common virtio code for deducing the required device config size based
> on provided host features.
> - Ability to disable modern virtio-blk features like
> discard/write-zeroes for vhost-user-blk.
> - Dynamic configuration space resizing based on enabled features,
> by reusing the common code introduced in the earlier commits.
> - Cleans up the VHostUserBlk structure by reusing parent fields.
>
> Changes since v1 (mostly addresses Stefan's feedback):
> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
> only hold the virtio-blk config size parameters.
> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
>
> Changes since v2:
> - Squash the first four commits into one
> - Set .min_size for virtio-net as well
> - Move maintainer/meson user-blk bits to the last commit
>
> Daniil Tatianin (5):
> virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
> virtio-blk: move config size params to virtio-blk-common
> vhost-user-blk: make it possible to disable write-zeroes/discard
> vhost-user-blk: make 'config_wce' part of 'host_features'
> vhost-user-blk: dynamically resize config space based on features
>
> MAINTAINERS | 4 +++
> hw/block/meson.build | 4 +--
> hw/block/vhost-user-blk.c | 29 +++++++++++---------
> hw/block/virtio-blk-common.c | 39 +++++++++++++++++++++++++++
> hw/block/virtio-blk.c | 28 +++----------------
> hw/net/virtio-net.c | 9 +++++--
> hw/virtio/virtio.c | 10 ++++---
> include/hw/virtio/vhost-user-blk.h | 1 -
> include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
> include/hw/virtio/virtio.h | 10 +++++--
> 10 files changed, 105 insertions(+), 49 deletions(-)
> create mode 100644 hw/block/virtio-blk-common.c
> create mode 100644 include/hw/virtio/virtio-blk-common.h
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v3 0/5] vhost-user-blk: dynamically resize config space based on features
2022-10-11 7:20 ` Daniil Tatianin
@ 2022-10-11 7:21 ` Daniil Tatianin
0 siblings, 0 replies; 11+ messages in thread
From: Daniil Tatianin @ 2022-10-11 7:21 UTC (permalink / raw)
To: qemu-devel
Cc: yc-core, mst, stefanha, raphael.norwitz, kwolf, qemu-block,
jasowang
On 10/11/22 10:20 AM, Daniil Tatianin wrote:
> Ping :)
Oops, didn't see the pull request. Disregard this.
> On 9/6/22 10:31 AM, Daniil Tatianin wrote:
>> This patch set attempts to align vhost-user-blk with virtio-blk in
>> terms of backward compatibility and flexibility. It also improves
>> the virtio core by introducing new common code that can be used by
>> a virtio device to calculate its config space size.
>>
>> In particular it adds the following things:
>> - Common virtio code for deducing the required device config size based
>> on provided host features.
>> - Ability to disable modern virtio-blk features like
>> discard/write-zeroes for vhost-user-blk.
>> - Dynamic configuration space resizing based on enabled features,
>> by reusing the common code introduced in the earlier commits.
>> - Cleans up the VHostUserBlk structure by reusing parent fields.
>>
>> Changes since v1 (mostly addresses Stefan's feedback):
>> - Introduce VirtIOConfigSizeParams & virtio_get_config_size
>> - Remove virtio_blk_set_config_size altogether, make virtio-blk-common.c
>> only hold the virtio-blk config size parameters.
>> - Reuse parent fields in vhost-user-blk instead of introducing new ones.
>>
>> Changes since v2:
>> - Squash the first four commits into one
>> - Set .min_size for virtio-net as well
>> - Move maintainer/meson user-blk bits to the last commit
>>
>> Daniil Tatianin (5):
>> virtio: introduce VirtIOConfigSizeParams & virtio_get_config_size
>> virtio-blk: move config size params to virtio-blk-common
>> vhost-user-blk: make it possible to disable write-zeroes/discard
>> vhost-user-blk: make 'config_wce' part of 'host_features'
>> vhost-user-blk: dynamically resize config space based on features
>>
>> MAINTAINERS | 4 +++
>> hw/block/meson.build | 4 +--
>> hw/block/vhost-user-blk.c | 29 +++++++++++---------
>> hw/block/virtio-blk-common.c | 39 +++++++++++++++++++++++++++
>> hw/block/virtio-blk.c | 28 +++----------------
>> hw/net/virtio-net.c | 9 +++++--
>> hw/virtio/virtio.c | 10 ++++---
>> include/hw/virtio/vhost-user-blk.h | 1 -
>> include/hw/virtio/virtio-blk-common.h | 20 ++++++++++++++
>> include/hw/virtio/virtio.h | 10 +++++--
>> 10 files changed, 105 insertions(+), 49 deletions(-)
>> create mode 100644 hw/block/virtio-blk-common.c
>> create mode 100644 include/hw/virtio/virtio-blk-common.h
>>
^ permalink raw reply [flat|nested] 11+ messages in thread