Hi Edgar,
On 26-09-25, 03:49, Edgar E. Iglesias wrote:
Found an issue today when testing with our most recent virtio-msg QEMU backend. Looks like the driver is setting the wrong length for set_features, see inline.
I'm using something based on your virtio/msg-v2-xen + our AMP/PCIe bus drivers.
+static int virtio_msg_finalize_features(struct virtio_device *vdev) +{
- struct virtio_msg_device *vmdev = to_virtio_msg_device(vdev);
- struct set_features *payload = virtio_msg_payload(vmdev->request);
- __le32 *features = (__le32 *)payload->features;
- static_assert(sizeof(*vmdev->request) + sizeof(*payload) <
VIRTIO_MSG_MIN_SIZE);- /* Give virtio_ring a chance to accept features */
- vring_transport_features(vdev);
- transport_msg_prepare(vmdev, VIRTIO_MSG_SET_DRV_FEATURES, sizeof(*payload));
I think the length here should be sizeof(*payload) + 2 * 4, shouldn't it?
We may have the same issue with set_config...
Following should fix these issues (with messages that have variable length):
diff --git a/drivers/virtio/virtio_msg.c b/drivers/virtio/virtio_msg.c index 7034818475ab..e6d51625cb3f 100644 --- a/drivers/virtio/virtio_msg.c +++ b/drivers/virtio/virtio_msg.c @@ -142,8 +142,8 @@ static u64 virtio_msg_get_features(struct virtio_device *vdev)
static_assert(sizeof(*vmdev->request) + sizeof(*req_payload) < VIRTIO_MSG_MIN_SIZE); - static_assert(sizeof(*vmdev->response) + sizeof(*res_payload) < - VIRTIO_MSG_MIN_SIZE); + static_assert(sizeof(*vmdev->response) + sizeof(*res_payload) + + 2 * sizeof(*features) < VIRTIO_MSG_MIN_SIZE);
transport_msg_prepare(vmdev, VIRTIO_MSG_GET_DEV_FEATURES, sizeof(*req_payload)); @@ -166,13 +166,14 @@ static int virtio_msg_finalize_features(struct virtio_device *vdev) struct set_features *payload = virtio_msg_payload(vmdev->request); __le32 *features = (__le32 *)payload->features;
- static_assert(sizeof(*vmdev->request) + sizeof(*payload) < - VIRTIO_MSG_MIN_SIZE); + static_assert(sizeof(*vmdev->request) + sizeof(*payload) + + 2 * sizeof(*features) < VIRTIO_MSG_MIN_SIZE);
/* Give virtio_ring a chance to accept features */ vring_transport_features(vdev);
- transport_msg_prepare(vmdev, VIRTIO_MSG_SET_DRV_FEATURES, sizeof(*payload)); + transport_msg_prepare(vmdev, VIRTIO_MSG_SET_DRV_FEATURES, + sizeof(*payload) + 2 * sizeof(*features));
/* Linux supports 64 feature bits */ payload->num = cpu_to_le32(2); @@ -231,7 +232,7 @@ static void virtio_msg_set(struct virtio_device *vdev, unsigned int offset, { struct virtio_msg_device *vmdev = to_virtio_msg_device(vdev); struct set_config *payload = virtio_msg_payload(vmdev->request); - u32 i = 0, max; + u32 i = 0, max, fixed_size = sizeof(*vmdev->request) + sizeof(*payload);
static_assert(sizeof(*vmdev->request) + sizeof(*payload) < VIRTIO_MSG_MIN_SIZE); @@ -244,9 +245,10 @@ static void virtio_msg_set(struct virtio_device *vdev, unsigned int offset, }
/* Maximum payload size available in the request message buffer */ - max = vmdev->msg_size - sizeof(*vmdev->request) - sizeof(*payload); + max = vmdev->msg_size - fixed_size;
- transport_msg_prepare(vmdev, VIRTIO_MSG_SET_CONFIG, sizeof(*payload)); + /* Message size is set before sending the message */ + transport_msg_prepare(vmdev, VIRTIO_MSG_SET_CONFIG, 0); payload->generation = cpu_to_le32(vmdev->generation_count);
while (i != len) { @@ -259,6 +261,9 @@ static void virtio_msg_set(struct virtio_device *vdev, unsigned int offset, memcpy(payload->config, buf + i, size); i += size;
+ /* Update the message size */ + vmdev->request->msg_size = cpu_to_le16(fixed_size + size); + if (virtio_msg_xfer(vmdev)) return; }