If the driver uses any of the extended features (i.e. 64 or above),
store the extended features range (64-127 bits).
At load time, let legacy features initialize the full features range
and pass it to the set helper; sub-states loading will have filled-up
the extended part as needed.
This is one of the few spots that need explicitly to know and set
in stone the extended features array size; add a build bug to prevent
breaking the migration should such size change again in the future:
more serialization plumbing will be needed.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v2 -> v3:
- 128bit_features state load/stores only the high bits
- consolidate the load implementation to use a single set
helper for 128/64/32 bits features
- _array -> _ex
v1 -> v2:
- uint128_t -> u64[2]
---
hw/virtio/virtio.c | 88 ++++++++++++++++++++++++++++++----------------
1 file changed, 57 insertions(+), 31 deletions(-)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2ab1d20769..2817d3a893 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2954,6 +2954,24 @@ static const VMStateDescription vmstate_virtio_disabled = {
}
};
+static bool virtio_128bit_features_needed(void *opaque)
+{
+ VirtIODevice *vdev = opaque;
+
+ return virtio_features_use_extended(vdev->host_features_ex);
+}
+
+static const VMStateDescription vmstate_virtio_128bit_features = {
+ .name = "virtio/128bit_features",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .needed = &virtio_128bit_features_needed,
+ .fields = (const VMStateField[]) {
+ VMSTATE_UINT64(guest_features_ex[1], VirtIODevice),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
static const VMStateDescription vmstate_virtio = {
.name = "virtio",
.version_id = 1,
@@ -2963,6 +2981,7 @@ static const VMStateDescription vmstate_virtio = {
},
.subsections = (const VMStateDescription * const []) {
&vmstate_virtio_device_endian,
+ &vmstate_virtio_128bit_features,
&vmstate_virtio_64bit_features,
&vmstate_virtio_virtqueues,
&vmstate_virtio_ringsize,
@@ -3059,23 +3078,28 @@ const VMStateInfo virtio_vmstate_info = {
.put = virtio_device_put,
};
-static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
+static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val)
{
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
- bool bad = (val & ~(vdev->host_features)) != 0;
+ uint64_t tmp[VIRTIO_FEATURES_DWORDS];
+ bool bad;
+
+ bad = virtio_features_andnot(tmp, val, vdev->host_features_ex);
+ virtio_features_and(tmp, val, vdev->host_features_ex);
- val &= vdev->host_features;
if (k->set_features) {
- k->set_features(vdev, val);
+ bad = bad || virtio_features_use_extended(tmp);
+ k->set_features(vdev, tmp[0]);
}
- vdev->guest_features = val;
+
+ virtio_features_copy(vdev->guest_features_ex, tmp);
return bad ? -1 : 0;
}
typedef struct VirtioSetFeaturesNocheckData {
Coroutine *co;
VirtIODevice *vdev;
- uint64_t val;
+ uint64_t val[VIRTIO_FEATURES_DWORDS];
int ret;
} VirtioSetFeaturesNocheckData;
@@ -3088,14 +3112,15 @@ static void virtio_set_features_nocheck_bh(void *opaque)
}
static int coroutine_mixed_fn
-virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, uint64_t val)
+virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev,
+ const uint64_t *val)
{
if (qemu_in_coroutine()) {
VirtioSetFeaturesNocheckData data = {
.co = qemu_coroutine_self(),
.vdev = vdev,
- .val = val,
};
+ virtio_features_copy(data.val, val);
aio_bh_schedule_oneshot(qemu_get_current_aio_context(),
virtio_set_features_nocheck_bh, &data);
qemu_coroutine_yield();
@@ -3107,6 +3132,7 @@ virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, uint64_t val)
int virtio_set_features(VirtIODevice *vdev, uint64_t val)
{
+ uint64_t features[VIRTIO_FEATURES_DWORDS];
int ret;
/*
* The driver must not attempt to set features after feature negotiation
@@ -3122,7 +3148,8 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
__func__, vdev->name);
}
- ret = virtio_set_features_nocheck(vdev, val);
+ virtio_features_from_u64(features, val);
+ ret = virtio_set_features_nocheck(vdev, features);
if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
/* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
int i;
@@ -3145,6 +3172,7 @@ void virtio_reset(void *opaque)
{
VirtIODevice *vdev = opaque;
VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
+ uint64_t features[VIRTIO_FEATURES_DWORDS];
int i;
virtio_set_status(vdev, 0);
@@ -3171,7 +3199,8 @@ void virtio_reset(void *opaque)
vdev->start_on_kick = false;
vdev->started = false;
vdev->broken = false;
- virtio_set_features_nocheck(vdev, 0);
+ virtio_features_clear(features);
+ virtio_set_features_nocheck(vdev, features);
vdev->queue_sel = 0;
vdev->status = 0;
vdev->disabled = false;
@@ -3254,7 +3283,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
* Note: devices should always test host features in future - don't create
* new dependencies like this.
*/
- vdev->guest_features = features;
+ virtio_features_from_u64(vdev->guest_features_ex, features);
config_len = qemu_get_be32(f);
@@ -3333,26 +3362,23 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
vdev->device_endian = virtio_default_endian();
}
- if (virtio_64bit_features_needed(vdev)) {
- /*
- * Subsection load filled vdev->guest_features. Run them
- * through virtio_set_features to sanity-check them against
- * host_features.
- */
- uint64_t features64 = vdev->guest_features;
- if (virtio_set_features_nocheck_maybe_co(vdev, features64) < 0) {
- error_report("Features 0x%" PRIx64 " unsupported. "
- "Allowed features: 0x%" PRIx64,
- features64, vdev->host_features);
- return -1;
- }
- } else {
- if (virtio_set_features_nocheck_maybe_co(vdev, features) < 0) {
- error_report("Features 0x%x unsupported. "
- "Allowed features: 0x%" PRIx64,
- features, vdev->host_features);
- return -1;
- }
+ /*
+ * Avoid silently breaking migration should the feature space increase
+ * even more in the (far away) future
+ */
+ QEMU_BUILD_BUG_ON(VIRTIO_FEATURES_DWORDS != 2);
+
+ /*
+ * guest_features_ex is fully initialized with u32 features and upper
+ * bits have been filled as needed by the later load.
+ */
+ if (virtio_set_features_nocheck_maybe_co(vdev,
+ vdev->guest_features_ex) < 0) {
+ error_report("Features 0x" VIRTIO_FEATURES_FMT " unsupported. "
+ "Allowed features: 0x" VIRTIO_FEATURES_FMT,
+ VIRTIO_FEATURES_PR(vdev->guest_features_ex),
+ VIRTIO_FEATURES_PR(vdev->host_features_ex));
+ return -1;
}
if (!virtio_device_started(vdev, vdev->status) &&
--
2.50.0On 2025/07/18 17:52, Paolo Abeni wrote:
> If the driver uses any of the extended features (i.e. 64 or above),
> store the extended features range (64-127 bits).
>
> At load time, let legacy features initialize the full features range
> and pass it to the set helper; sub-states loading will have filled-up
> the extended part as needed.
>
> This is one of the few spots that need explicitly to know and set
> in stone the extended features array size; add a build bug to prevent
> breaking the migration should such size change again in the future:
> more serialization plumbing will be needed.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> v2 -> v3:
> - 128bit_features state load/stores only the high bits
> - consolidate the load implementation to use a single set
> helper for 128/64/32 bits features
> - _array -> _ex
>
> v1 -> v2:
> - uint128_t -> u64[2]
> ---
> hw/virtio/virtio.c | 88 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 57 insertions(+), 31 deletions(-)
>
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 2ab1d20769..2817d3a893 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2954,6 +2954,24 @@ static const VMStateDescription vmstate_virtio_disabled = {
> }
> };
>
> +static bool virtio_128bit_features_needed(void *opaque)
> +{
> + VirtIODevice *vdev = opaque;
> +
> + return virtio_features_use_extended(vdev->host_features_ex);
> +}
> +
> +static const VMStateDescription vmstate_virtio_128bit_features = {
> + .name = "virtio/128bit_features",
> + .version_id = 1,
> + .minimum_version_id = 1,
> + .needed = &virtio_128bit_features_needed,
> + .fields = (const VMStateField[]) {
> + VMSTATE_UINT64(guest_features_ex[1], VirtIODevice),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> static const VMStateDescription vmstate_virtio = {
> .name = "virtio",
> .version_id = 1,
> @@ -2963,6 +2981,7 @@ static const VMStateDescription vmstate_virtio = {
> },
> .subsections = (const VMStateDescription * const []) {
> &vmstate_virtio_device_endian,
> + &vmstate_virtio_128bit_features,
> &vmstate_virtio_64bit_features,
> &vmstate_virtio_virtqueues,
> &vmstate_virtio_ringsize,
> @@ -3059,23 +3078,28 @@ const VMStateInfo virtio_vmstate_info = {
> .put = virtio_device_put,
> };
>
> -static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
> +static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val)
> {
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> - bool bad = (val & ~(vdev->host_features)) != 0;
> + uint64_t tmp[VIRTIO_FEATURES_DWORDS];
> + bool bad;
> +
> + bad = virtio_features_andnot(tmp, val, vdev->host_features_ex);
> + virtio_features_and(tmp, val, vdev->host_features_ex);
>
> - val &= vdev->host_features;
> if (k->set_features) {
> - k->set_features(vdev, val);
> + bad = bad || virtio_features_use_extended(tmp);
> + k->set_features(vdev, tmp[0]);
> }
> - vdev->guest_features = val;
> +
> + virtio_features_copy(vdev->guest_features_ex, tmp);
> return bad ? -1 : 0;
> }
>
> typedef struct VirtioSetFeaturesNocheckData {
> Coroutine *co;
> VirtIODevice *vdev;
> - uint64_t val;
> + uint64_t val[VIRTIO_FEATURES_DWORDS];
> int ret;
> } VirtioSetFeaturesNocheckData;
>
> @@ -3088,14 +3112,15 @@ static void virtio_set_features_nocheck_bh(void *opaque)
> }
>
> static int coroutine_mixed_fn
> -virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, uint64_t val)
> +virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev,
> + const uint64_t *val)
> {
> if (qemu_in_coroutine()) {
> VirtioSetFeaturesNocheckData data = {
> .co = qemu_coroutine_self(),
> .vdev = vdev,
> - .val = val,
> };
> + virtio_features_copy(data.val, val);
> aio_bh_schedule_oneshot(qemu_get_current_aio_context(),
> virtio_set_features_nocheck_bh, &data);
> qemu_coroutine_yield();
> @@ -3107,6 +3132,7 @@ virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, uint64_t val)
>
> int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> {
> + uint64_t features[VIRTIO_FEATURES_DWORDS];
> int ret;
> /*
> * The driver must not attempt to set features after feature negotiation
> @@ -3122,7 +3148,8 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> __func__, vdev->name);
> }
>
> - ret = virtio_set_features_nocheck(vdev, val);
> + virtio_features_from_u64(features, val);
> + ret = virtio_set_features_nocheck(vdev, features);
> if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
> /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
> int i;
> @@ -3145,6 +3172,7 @@ void virtio_reset(void *opaque)
> {
> VirtIODevice *vdev = opaque;
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> + uint64_t features[VIRTIO_FEATURES_DWORDS];
> int i;
>
> virtio_set_status(vdev, 0);
> @@ -3171,7 +3199,8 @@ void virtio_reset(void *opaque)
> vdev->start_on_kick = false;
> vdev->started = false;
> vdev->broken = false;
> - virtio_set_features_nocheck(vdev, 0);
> + virtio_features_clear(features);
> + virtio_set_features_nocheck(vdev, features);
> vdev->queue_sel = 0;
> vdev->status = 0;
> vdev->disabled = false;
> @@ -3254,7 +3283,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> * Note: devices should always test host features in future - don't create
> * new dependencies like this.
> */
> - vdev->guest_features = features;
> + virtio_features_from_u64(vdev->guest_features_ex, features);
>
> config_len = qemu_get_be32(f);
>
> @@ -3333,26 +3362,23 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> vdev->device_endian = virtio_default_endian();
> }
>
> - if (virtio_64bit_features_needed(vdev)) {
> - /*
> - * Subsection load filled vdev->guest_features. Run them
> - * through virtio_set_features to sanity-check them against
> - * host_features.
> - */
> - uint64_t features64 = vdev->guest_features;
> - if (virtio_set_features_nocheck_maybe_co(vdev, features64) < 0) {
> - error_report("Features 0x%" PRIx64 " unsupported. "
> - "Allowed features: 0x%" PRIx64,
> - features64, vdev->host_features);
> - return -1;
> - }
> - } else {
> - if (virtio_set_features_nocheck_maybe_co(vdev, features) < 0) {
> - error_report("Features 0x%x unsupported. "
> - "Allowed features: 0x%" PRIx64,
> - features, vdev->host_features);
> - return -1;
> - }
> + /*
> + * Avoid silently breaking migration should the feature space increase
> + * even more in the (far away) future
> + */
> + QEMU_BUILD_BUG_ON(VIRTIO_FEATURES_DWORDS != 2);
This should be moved to around virtio_128bit_features_needed() and
vmstate_virtio_128bit_features as they make assumptions on the feature
space while virtio_load() doesn't.
> +
> + /*
> + * guest_features_ex is fully initialized with u32 features and upper
> + * bits have been filled as needed by the later load.
> + */
> + if (virtio_set_features_nocheck_maybe_co(vdev,
> + vdev->guest_features_ex) < 0) {
> + error_report("Features 0x" VIRTIO_FEATURES_FMT " unsupported. "
> + "Allowed features: 0x" VIRTIO_FEATURES_FMT,
> + VIRTIO_FEATURES_PR(vdev->guest_features_ex),
> + VIRTIO_FEATURES_PR(vdev->host_features_ex));
> + return -1;
> }
>
> if (!virtio_device_started(vdev, vdev->status) &&
7/20/25 12:44 PM, Akihiko Odaki wrote:
> On 2025/07/18 17:52, Paolo Abeni wrote:
>> @@ -3333,26 +3362,23 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>> vdev->device_endian = virtio_default_endian();
>> }
>>
>> - if (virtio_64bit_features_needed(vdev)) {
>> - /*
>> - * Subsection load filled vdev->guest_features. Run them
>> - * through virtio_set_features to sanity-check them against
>> - * host_features.
>> - */
>> - uint64_t features64 = vdev->guest_features;
>> - if (virtio_set_features_nocheck_maybe_co(vdev, features64) < 0) {
>> - error_report("Features 0x%" PRIx64 " unsupported. "
>> - "Allowed features: 0x%" PRIx64,
>> - features64, vdev->host_features);
>> - return -1;
>> - }
>> - } else {
>> - if (virtio_set_features_nocheck_maybe_co(vdev, features) < 0) {
>> - error_report("Features 0x%x unsupported. "
>> - "Allowed features: 0x%" PRIx64,
>> - features, vdev->host_features);
>> - return -1;
>> - }
>> + /*
>> + * Avoid silently breaking migration should the feature space increase
>> + * even more in the (far away) future
>> + */
>> + QEMU_BUILD_BUG_ON(VIRTIO_FEATURES_DWORDS != 2);
>
> This should be moved to around virtio_128bit_features_needed() and
> vmstate_virtio_128bit_features as they make assumptions on the feature
> space while virtio_load() doesn't.
Please allow me the following dumb question to try to avoid more
iterations for small changes. I guess moving the above inside
virtio_128bit_features_needed() would suffice?
Thanks,
Paolo
On 2025/07/21 16:51, Paolo Abeni wrote:
> 7/20/25 12:44 PM, Akihiko Odaki wrote:
>> On 2025/07/18 17:52, Paolo Abeni wrote:
>>> @@ -3333,26 +3362,23 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>>> vdev->device_endian = virtio_default_endian();
>>> }
>>>
>>> - if (virtio_64bit_features_needed(vdev)) {
>>> - /*
>>> - * Subsection load filled vdev->guest_features. Run them
>>> - * through virtio_set_features to sanity-check them against
>>> - * host_features.
>>> - */
>>> - uint64_t features64 = vdev->guest_features;
>>> - if (virtio_set_features_nocheck_maybe_co(vdev, features64) < 0) {
>>> - error_report("Features 0x%" PRIx64 " unsupported. "
>>> - "Allowed features: 0x%" PRIx64,
>>> - features64, vdev->host_features);
>>> - return -1;
>>> - }
>>> - } else {
>>> - if (virtio_set_features_nocheck_maybe_co(vdev, features) < 0) {
>>> - error_report("Features 0x%x unsupported. "
>>> - "Allowed features: 0x%" PRIx64,
>>> - features, vdev->host_features);
>>> - return -1;
>>> - }
>>> + /*
>>> + * Avoid silently breaking migration should the feature space increase
>>> + * even more in the (far away) future
>>> + */
>>> + QEMU_BUILD_BUG_ON(VIRTIO_FEATURES_DWORDS != 2);
>>
>> This should be moved to around virtio_128bit_features_needed() and
>> vmstate_virtio_128bit_features as they make assumptions on the feature
>> space while virtio_load() doesn't.
>
> Please allow me the following dumb question to try to avoid more
> iterations for small changes. I guess moving the above inside
> virtio_128bit_features_needed() would suffice?
Thinking more carefully, now I think it should be moved before or after
vmstate_virtio.
virtio_128bit_features_needed() and vmstate_virtio_128bit_features will
work fine even if the feature bitmask becomes 196-bit. But
vmstate_virtio will need to be updated to contain
vmstate_virtio_196bit_features. So this assertion should be around
vmstate_virtio.
Regards,
Akihiko Odaki
On Fri, Jul 18, 2025 at 10:52:30AM +0200, Paolo Abeni wrote:
>If the driver uses any of the extended features (i.e. 64 or above),
>store the extended features range (64-127 bits).
>
>At load time, let legacy features initialize the full features range
>and pass it to the set helper; sub-states loading will have filled-up
>the extended part as needed.
>
>This is one of the few spots that need explicitly to know and set
>in stone the extended features array size; add a build bug to prevent
>breaking the migration should such size change again in the future:
>more serialization plumbing will be needed.
>
>Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>---
>v2 -> v3:
> - 128bit_features state load/stores only the high bits
> - consolidate the load implementation to use a single set
> helper for 128/64/32 bits features
> - _array -> _ex
>
>v1 -> v2:
> - uint128_t -> u64[2]
>---
> hw/virtio/virtio.c | 88 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 57 insertions(+), 31 deletions(-)
>
>diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>index 2ab1d20769..2817d3a893 100644
>--- a/hw/virtio/virtio.c
>+++ b/hw/virtio/virtio.c
>@@ -2954,6 +2954,24 @@ static const VMStateDescription vmstate_virtio_disabled = {
> }
> };
>
>+static bool virtio_128bit_features_needed(void *opaque)
>+{
>+ VirtIODevice *vdev = opaque;
>+
>+ return virtio_features_use_extended(vdev->host_features_ex);
Related to live-migration, IIUC if we disable the new features in the
10.0 machine type, this should allow the migration (from a new qemu
using the old machine time to an old qemu not supporting extended
features), since those features should not be set in
vdev->host_features_ex, and this will return `false`.
Right?
Thanks,
Stefano
>+}
>+
>+static const VMStateDescription vmstate_virtio_128bit_features = {
>+ .name = "virtio/128bit_features",
>+ .version_id = 1,
>+ .minimum_version_id = 1,
>+ .needed = &virtio_128bit_features_needed,
>+ .fields = (const VMStateField[]) {
>+ VMSTATE_UINT64(guest_features_ex[1], VirtIODevice),
>+ VMSTATE_END_OF_LIST()
>+ }
>+};
>+
> static const VMStateDescription vmstate_virtio = {
> .name = "virtio",
> .version_id = 1,
>@@ -2963,6 +2981,7 @@ static const VMStateDescription vmstate_virtio = {
> },
> .subsections = (const VMStateDescription * const []) {
> &vmstate_virtio_device_endian,
>+ &vmstate_virtio_128bit_features,
> &vmstate_virtio_64bit_features,
> &vmstate_virtio_virtqueues,
> &vmstate_virtio_ringsize,
>@@ -3059,23 +3078,28 @@ const VMStateInfo virtio_vmstate_info = {
> .put = virtio_device_put,
> };
>
>-static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
>+static int virtio_set_features_nocheck(VirtIODevice *vdev, const uint64_t *val)
> {
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>- bool bad = (val & ~(vdev->host_features)) != 0;
>+ uint64_t tmp[VIRTIO_FEATURES_DWORDS];
>+ bool bad;
>+
>+ bad = virtio_features_andnot(tmp, val, vdev->host_features_ex);
>+ virtio_features_and(tmp, val, vdev->host_features_ex);
>
>- val &= vdev->host_features;
> if (k->set_features) {
>- k->set_features(vdev, val);
>+ bad = bad || virtio_features_use_extended(tmp);
>+ k->set_features(vdev, tmp[0]);
> }
>- vdev->guest_features = val;
>+
>+ virtio_features_copy(vdev->guest_features_ex, tmp);
> return bad ? -1 : 0;
> }
>
> typedef struct VirtioSetFeaturesNocheckData {
> Coroutine *co;
> VirtIODevice *vdev;
>- uint64_t val;
>+ uint64_t val[VIRTIO_FEATURES_DWORDS];
> int ret;
> } VirtioSetFeaturesNocheckData;
>
>@@ -3088,14 +3112,15 @@ static void virtio_set_features_nocheck_bh(void *opaque)
> }
>
> static int coroutine_mixed_fn
>-virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, uint64_t val)
>+virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev,
>+ const uint64_t *val)
> {
> if (qemu_in_coroutine()) {
> VirtioSetFeaturesNocheckData data = {
> .co = qemu_coroutine_self(),
> .vdev = vdev,
>- .val = val,
> };
>+ virtio_features_copy(data.val, val);
> aio_bh_schedule_oneshot(qemu_get_current_aio_context(),
> virtio_set_features_nocheck_bh, &data);
> qemu_coroutine_yield();
>@@ -3107,6 +3132,7 @@ virtio_set_features_nocheck_maybe_co(VirtIODevice *vdev, uint64_t val)
>
> int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> {
>+ uint64_t features[VIRTIO_FEATURES_DWORDS];
> int ret;
> /*
> * The driver must not attempt to set features after feature negotiation
>@@ -3122,7 +3148,8 @@ int virtio_set_features(VirtIODevice *vdev, uint64_t val)
> __func__, vdev->name);
> }
>
>- ret = virtio_set_features_nocheck(vdev, val);
>+ virtio_features_from_u64(features, val);
>+ ret = virtio_set_features_nocheck(vdev, features);
> if (virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
> /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches. */
> int i;
>@@ -3145,6 +3172,7 @@ void virtio_reset(void *opaque)
> {
> VirtIODevice *vdev = opaque;
> VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>+ uint64_t features[VIRTIO_FEATURES_DWORDS];
> int i;
>
> virtio_set_status(vdev, 0);
>@@ -3171,7 +3199,8 @@ void virtio_reset(void *opaque)
> vdev->start_on_kick = false;
> vdev->started = false;
> vdev->broken = false;
>- virtio_set_features_nocheck(vdev, 0);
>+ virtio_features_clear(features);
>+ virtio_set_features_nocheck(vdev, features);
> vdev->queue_sel = 0;
> vdev->status = 0;
> vdev->disabled = false;
>@@ -3254,7 +3283,7 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> * Note: devices should always test host features in future - don't create
> * new dependencies like this.
> */
>- vdev->guest_features = features;
>+ virtio_features_from_u64(vdev->guest_features_ex, features);
>
> config_len = qemu_get_be32(f);
>
>@@ -3333,26 +3362,23 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> vdev->device_endian = virtio_default_endian();
> }
>
>- if (virtio_64bit_features_needed(vdev)) {
>- /*
>- * Subsection load filled vdev->guest_features. Run them
>- * through virtio_set_features to sanity-check them against
>- * host_features.
>- */
>- uint64_t features64 = vdev->guest_features;
>- if (virtio_set_features_nocheck_maybe_co(vdev, features64) < 0) {
>- error_report("Features 0x%" PRIx64 " unsupported. "
>- "Allowed features: 0x%" PRIx64,
>- features64, vdev->host_features);
>- return -1;
>- }
>- } else {
>- if (virtio_set_features_nocheck_maybe_co(vdev, features) < 0) {
>- error_report("Features 0x%x unsupported. "
>- "Allowed features: 0x%" PRIx64,
>- features, vdev->host_features);
>- return -1;
>- }
>+ /*
>+ * Avoid silently breaking migration should the feature space increase
>+ * even more in the (far away) future
>+ */
>+ QEMU_BUILD_BUG_ON(VIRTIO_FEATURES_DWORDS != 2);
>+
>+ /*
>+ * guest_features_ex is fully initialized with u32 features and upper
>+ * bits have been filled as needed by the later load.
>+ */
>+ if (virtio_set_features_nocheck_maybe_co(vdev,
>+ vdev->guest_features_ex) < 0) {
>+ error_report("Features 0x" VIRTIO_FEATURES_FMT " unsupported. "
>+ "Allowed features: 0x" VIRTIO_FEATURES_FMT,
>+ VIRTIO_FEATURES_PR(vdev->guest_features_ex),
>+ VIRTIO_FEATURES_PR(vdev->host_features_ex));
>+ return -1;
> }
>
> if (!virtio_device_started(vdev, vdev->status) &&
>--
>2.50.0
>
© 2016 - 2025 Red Hat, Inc.