On 13.10.25 23:13, Raphael Norwitz wrote:
Overall looks good just once comment.
On Sat, Oct 11, 2025 at 7:24 PM Vladimir Sementsov-Ogievskiy
<[email protected]> wrote:
We miss call to unmap in cases when vhost_memory_map() returns
lenght less than requested (still we consider such cases as an
error). Let's fix it in vhost_memory_map().
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
---
hw/virtio/vhost.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 773b91c0a0..8031c74e7b 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -453,11 +453,20 @@ static inline void vhost_dev_log_resize(struct vhost_dev
*dev, uint64_t size)
}
static void *vhost_memory_map(struct vhost_dev *dev, hwaddr addr,
- hwaddr *plen, bool is_write)
+ hwaddr len, bool is_write)
{
if (!vhost_dev_has_iommu(dev)) {
- return address_space_map(dev->vdev->dma_as, addr, plen, is_write,
- MEMTXATTRS_UNSPECIFIED);
+ hwaddr mapped_len = len;
+ void *res = address_space_map(dev->vdev->dma_as, addr, &mapped_len,
+ is_write, MEMTXATTRS_UNSPECIFIED);
+ if (!res) {
+ return NULL;
+ }
+ if (len != mapped_len) {
Should this be:
address_space_unmap(dev->vdev->dma_as, res, mapped_len, is_write,
MEMTXATTRS_UNSPECIFIED);
rather than address_space_unmap(...0,0)?
Pre-patch we do "...0,0)" for already mapped memories.
Moreover, following commit above address_space_unmap() definition,
is_write and access_len are here to mark the memory as dirty.
In our case, that's an early fail, and we don't actually access the memory,
so it's correct to set is_write to zero, I think.
The latter 0 is also correct, as it's just unused when is_write==0.
+ address_space_unmap(dev->vdev->dma_as, res, mapped_len, 0, 0);
+ return NULL;
+ }
+ return res;
} else {
return (void *)(uintptr_t)addr;
}
@@ -1261,7 +1270,7 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vdev)));
VirtioBusState *vbus = VIRTIO_BUS(qbus);
VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
- hwaddr s, l, a;
+ hwaddr l, a;
int r;
int vhost_vq_index = dev->vhost_ops->vhost_get_vq_index(dev, idx);
struct vhost_vring_file file = {
@@ -1301,24 +1310,24 @@ int vhost_virtqueue_start(struct vhost_dev *dev,
}
}
- vq->desc_size = s = l = virtio_queue_get_desc_size(vdev, idx);
+ vq->desc_size = l = virtio_queue_get_desc_size(vdev, idx);
vq->desc_phys = a;
- vq->desc = vhost_memory_map(dev, a, &l, false);
- if (!vq->desc || l != s) {
+ vq->desc = vhost_memory_map(dev, a, l, false);
+ if (!vq->desc) {
r = -ENOMEM;
goto fail_alloc_desc;
}
- vq->avail_size = s = l = virtio_queue_get_avail_size(vdev, idx);
+ vq->avail_size = l = virtio_queue_get_avail_size(vdev, idx);
vq->avail_phys = a = virtio_queue_get_avail_addr(vdev, idx);
- vq->avail = vhost_memory_map(dev, a, &l, false);
- if (!vq->avail || l != s) {
+ vq->avail = vhost_memory_map(dev, a, l, false);
+ if (!vq->avail) {
r = -ENOMEM;
goto fail_alloc_avail;
}
- vq->used_size = s = l = virtio_queue_get_used_size(vdev, idx);
+ vq->used_size = l = virtio_queue_get_used_size(vdev, idx);
vq->used_phys = a = virtio_queue_get_used_addr(vdev, idx);
- vq->used = vhost_memory_map(dev, a, &l, true);
- if (!vq->used || l != s) {
+ vq->used = vhost_memory_map(dev, a, l, true);
+ if (!vq->used) {
r = -ENOMEM;
goto fail_alloc_used;
}
--
2.48.1
--
Best regards,
Vladimir
