* [dpdk-dev] [PATCH] vhost: fix vring memory partially mapped
@ 2019-09-06 12:50 Adrian Moreno
2019-09-17 19:03 ` Adrian Moreno
2019-09-24 11:58 ` Maxime Coquelin
0 siblings, 2 replies; 4+ messages in thread
From: Adrian Moreno @ 2019-09-06 12:50 UTC (permalink / raw)
To: dev; +Cc: Adrian Moreno, maxime.coquelin, stable
Only the mapping of the vring addresses is being ensured. This causes
errors when the vring size is larger than the IOTLB page size. E.g:
queue sizes > 256 for 4K IOTLB pages
Ensure the entire vring memory range gets mapped. Refactor duplicated
code for for IOTLB UPDATE and IOTLB INVALIDATE and add packed virtqueue
support.
Fixes: 09927b524969 ("vhost: translate ring addresses when IOMMU enabled")
Cc: maxime.coquelin@redhat.com
Cc: stable@dpdk.org
Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
lib/librte_vhost/vhost_user.c | 64 +++++++++++++++++++++--------------
1 file changed, 39 insertions(+), 25 deletions(-)
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 0b72648a5..168ad9e0f 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -558,11 +558,13 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
{
if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) {
uint64_t vva;
+ uint64_t req_size = *size;
vva = vhost_user_iotlb_cache_find(vq, ra,
size, VHOST_ACCESS_RW);
- if (!vva)
- vhost_user_iotlb_miss(dev, ra, VHOST_ACCESS_RW);
+ if (req_size != *size)
+ vhost_user_iotlb_miss(dev, (ra + *size),
+ VHOST_ACCESS_RW);
return vva;
}
@@ -1570,54 +1572,66 @@ vhost_user_set_req_fd(struct virtio_net **pdev, struct VhostUserMsg *msg,
}
static int
-is_vring_iotlb_update(struct vhost_virtqueue *vq, struct vhost_iotlb_msg *imsg)
+is_vring_iotlb_split(struct vhost_virtqueue *vq, struct vhost_iotlb_msg *imsg)
{
struct vhost_vring_addr *ra;
- uint64_t start, end;
+ uint64_t start, end, len;
start = imsg->iova;
end = start + imsg->size;
ra = &vq->ring_addrs;
- if (ra->desc_user_addr >= start && ra->desc_user_addr < end)
+ len = sizeof(struct vring_desc) * vq->size;
+ if (ra->desc_user_addr < end && (ra->desc_user_addr + len) > start)
return 1;
- if (ra->avail_user_addr >= start && ra->avail_user_addr < end)
+
+ len = sizeof(struct vring_avail) + sizeof(uint16_t) * vq->size;
+ if (ra->avail_user_addr < end && (ra->avail_user_addr + len) > start)
return 1;
- if (ra->used_user_addr >= start && ra->used_user_addr < end)
+
+ len = sizeof(struct vring_used) +
+ sizeof(struct vring_used_elem) * vq->size;
+ if (ra->used_user_addr < end && (ra->used_user_addr + len) > start)
return 1;
return 0;
}
static int
-is_vring_iotlb_invalidate(struct vhost_virtqueue *vq,
- struct vhost_iotlb_msg *imsg)
+is_vring_iotlb_packed(struct vhost_virtqueue *vq, struct vhost_iotlb_msg *imsg)
{
- uint64_t istart, iend, vstart, vend;
+ struct vhost_vring_addr *ra;
+ uint64_t start, end, len;
- istart = imsg->iova;
- iend = istart + imsg->size - 1;
+ start = imsg->iova;
+ end = start + imsg->size;
- vstart = (uintptr_t)vq->desc;
- vend = vstart + sizeof(struct vring_desc) * vq->size - 1;
- if (vstart <= iend && istart <= vend)
+ ra = &vq->ring_addrs;
+ len = sizeof(struct vring_packed_desc) * vq->size;
+ if (ra->desc_user_addr < end && (ra->desc_user_addr + len) > start)
return 1;
- vstart = (uintptr_t)vq->avail;
- vend = vstart + sizeof(struct vring_avail);
- vend += sizeof(uint16_t) * vq->size - 1;
- if (vstart <= iend && istart <= vend)
+ len = sizeof(struct vring_packed_desc_event);
+ if (ra->avail_user_addr < end && (ra->avail_user_addr + len) > start)
return 1;
- vstart = (uintptr_t)vq->used;
- vend = vstart + sizeof(struct vring_used);
- vend += sizeof(struct vring_used_elem) * vq->size - 1;
- if (vstart <= iend && istart <= vend)
+ len = sizeof(struct vring_packed_desc_event);
+ if (ra->used_user_addr < end && (ra->used_user_addr + len) > start)
return 1;
return 0;
}
+static int is_vring_iotlb(struct virtio_net *dev,
+ struct vhost_virtqueue *vq,
+ struct vhost_iotlb_msg *imsg)
+{
+ if (vq_is_packed(dev))
+ return is_vring_iotlb_packed(vq, imsg);
+ else
+ return is_vring_iotlb_split(vq, imsg);
+}
+
static int
vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
int main_fd __rte_unused)
@@ -1640,7 +1654,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
vhost_user_iotlb_cache_insert(vq, imsg->iova, vva,
len, imsg->perm);
- if (is_vring_iotlb_update(vq, imsg))
+ if (is_vring_iotlb(dev, vq, imsg))
*pdev = dev = translate_ring_addresses(dev, i);
}
break;
@@ -1651,7 +1665,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
vhost_user_iotlb_cache_remove(vq, imsg->iova,
imsg->size);
- if (is_vring_iotlb_invalidate(vq, imsg))
+ if (is_vring_iotlb(dev, vq, imsg))
vring_invalidate(dev, vq);
}
break;
--
2.21.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix vring memory partially mapped
2019-09-06 12:50 [dpdk-dev] [PATCH] vhost: fix vring memory partially mapped Adrian Moreno
@ 2019-09-17 19:03 ` Adrian Moreno
2019-09-27 9:58 ` Maxime Coquelin
2019-09-24 11:58 ` Maxime Coquelin
1 sibling, 1 reply; 4+ messages in thread
From: Adrian Moreno @ 2019-09-17 19:03 UTC (permalink / raw)
To: dev; +Cc: maxime.coquelin, stable, zhihong.wang, tiwei.bie
Adding relevant maintainers. Sorry for the slip
Adrian
On 9/6/19 2:50 PM, Adrian Moreno wrote:
> Only the mapping of the vring addresses is being ensured. This causes
> errors when the vring size is larger than the IOTLB page size. E.g:
> queue sizes > 256 for 4K IOTLB pages
>
> Ensure the entire vring memory range gets mapped. Refactor duplicated
> code for for IOTLB UPDATE and IOTLB INVALIDATE and add packed virtqueue
> support.
>
> Fixes: 09927b524969 ("vhost: translate ring addresses when IOMMU enabled")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> lib/librte_vhost/vhost_user.c | 64 +++++++++++++++++++++--------------
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 0b72648a5..168ad9e0f 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -558,11 +558,13 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
> {
> if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) {
> uint64_t vva;
> + uint64_t req_size = *size;
>
> vva = vhost_user_iotlb_cache_find(vq, ra,
> size, VHOST_ACCESS_RW);
> - if (!vva)
> - vhost_user_iotlb_miss(dev, ra, VHOST_ACCESS_RW);
> + if (req_size != *size)
> + vhost_user_iotlb_miss(dev, (ra + *size),
> + VHOST_ACCESS_RW);
>
> return vva;
> }
> @@ -1570,54 +1572,66 @@ vhost_user_set_req_fd(struct virtio_net **pdev, struct VhostUserMsg *msg,
> }
>
> static int
> -is_vring_iotlb_update(struct vhost_virtqueue *vq, struct vhost_iotlb_msg *imsg)
> +is_vring_iotlb_split(struct vhost_virtqueue *vq, struct vhost_iotlb_msg *imsg)
> {
> struct vhost_vring_addr *ra;
> - uint64_t start, end;
> + uint64_t start, end, len;
>
> start = imsg->iova;
> end = start + imsg->size;
>
> ra = &vq->ring_addrs;
> - if (ra->desc_user_addr >= start && ra->desc_user_addr < end)
> + len = sizeof(struct vring_desc) * vq->size;
> + if (ra->desc_user_addr < end && (ra->desc_user_addr + len) > start)
> return 1;
> - if (ra->avail_user_addr >= start && ra->avail_user_addr < end)
> +
> + len = sizeof(struct vring_avail) + sizeof(uint16_t) * vq->size;
> + if (ra->avail_user_addr < end && (ra->avail_user_addr + len) > start)
> return 1;
> - if (ra->used_user_addr >= start && ra->used_user_addr < end)
> +
> + len = sizeof(struct vring_used) +
> + sizeof(struct vring_used_elem) * vq->size;
> + if (ra->used_user_addr < end && (ra->used_user_addr + len) > start)
> return 1;
>
> return 0;
> }
>
> static int
> -is_vring_iotlb_invalidate(struct vhost_virtqueue *vq,
> - struct vhost_iotlb_msg *imsg)
> +is_vring_iotlb_packed(struct vhost_virtqueue *vq, struct vhost_iotlb_msg *imsg)
> {
> - uint64_t istart, iend, vstart, vend;
> + struct vhost_vring_addr *ra;
> + uint64_t start, end, len;
>
> - istart = imsg->iova;
> - iend = istart + imsg->size - 1;
> + start = imsg->iova;
> + end = start + imsg->size;
>
> - vstart = (uintptr_t)vq->desc;
> - vend = vstart + sizeof(struct vring_desc) * vq->size - 1;
> - if (vstart <= iend && istart <= vend)
> + ra = &vq->ring_addrs;
> + len = sizeof(struct vring_packed_desc) * vq->size;
> + if (ra->desc_user_addr < end && (ra->desc_user_addr + len) > start)
> return 1;
>
> - vstart = (uintptr_t)vq->avail;
> - vend = vstart + sizeof(struct vring_avail);
> - vend += sizeof(uint16_t) * vq->size - 1;
> - if (vstart <= iend && istart <= vend)
> + len = sizeof(struct vring_packed_desc_event);
> + if (ra->avail_user_addr < end && (ra->avail_user_addr + len) > start)
> return 1;
>
> - vstart = (uintptr_t)vq->used;
> - vend = vstart + sizeof(struct vring_used);
> - vend += sizeof(struct vring_used_elem) * vq->size - 1;
> - if (vstart <= iend && istart <= vend)
> + len = sizeof(struct vring_packed_desc_event);
> + if (ra->used_user_addr < end && (ra->used_user_addr + len) > start)
> return 1;
>
> return 0;
> }
>
> +static int is_vring_iotlb(struct virtio_net *dev,
> + struct vhost_virtqueue *vq,
> + struct vhost_iotlb_msg *imsg)
> +{
> + if (vq_is_packed(dev))
> + return is_vring_iotlb_packed(vq, imsg);
> + else
> + return is_vring_iotlb_split(vq, imsg);
> +}
> +
> static int
> vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
> int main_fd __rte_unused)
> @@ -1640,7 +1654,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
> vhost_user_iotlb_cache_insert(vq, imsg->iova, vva,
> len, imsg->perm);
>
> - if (is_vring_iotlb_update(vq, imsg))
> + if (is_vring_iotlb(dev, vq, imsg))
> *pdev = dev = translate_ring_addresses(dev, i);
> }
> break;
> @@ -1651,7 +1665,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, struct VhostUserMsg *msg,
> vhost_user_iotlb_cache_remove(vq, imsg->iova,
> imsg->size);
>
> - if (is_vring_iotlb_invalidate(vq, imsg))
> + if (is_vring_iotlb(dev, vq, imsg))
> vring_invalidate(dev, vq);
> }
> break;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix vring memory partially mapped
2019-09-06 12:50 [dpdk-dev] [PATCH] vhost: fix vring memory partially mapped Adrian Moreno
2019-09-17 19:03 ` Adrian Moreno
@ 2019-09-24 11:58 ` Maxime Coquelin
1 sibling, 0 replies; 4+ messages in thread
From: Maxime Coquelin @ 2019-09-24 11:58 UTC (permalink / raw)
To: Adrian Moreno, dev; +Cc: stable, Tiwei Bie, Zhihong Wang
On 9/6/19 2:50 PM, Adrian Moreno wrote:
> Only the mapping of the vring addresses is being ensured. This causes
> errors when the vring size is larger than the IOTLB page size. E.g:
> queue sizes > 256 for 4K IOTLB pages
>
> Ensure the entire vring memory range gets mapped. Refactor duplicated
> code for for IOTLB UPDATE and IOTLB INVALIDATE and add packed virtqueue
> support.
>
> Fixes: 09927b524969 ("vhost: translate ring addresses when IOMMU enabled")
> Cc: maxime.coquelin@redhat.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
> lib/librte_vhost/vhost_user.c | 64 +++++++++++++++++++++--------------
> 1 file changed, 39 insertions(+), 25 deletions(-)
>
Thanks for the fix Adrian, it looks good to me:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Maxime
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [dpdk-dev] [PATCH] vhost: fix vring memory partially mapped
2019-09-17 19:03 ` Adrian Moreno
@ 2019-09-27 9:58 ` Maxime Coquelin
0 siblings, 0 replies; 4+ messages in thread
From: Maxime Coquelin @ 2019-09-27 9:58 UTC (permalink / raw)
To: Adrian Moreno, dev; +Cc: stable, zhihong.wang, tiwei.bie
On 9/17/19 9:03 PM, Adrian Moreno wrote:
> On 9/6/19 2:50 PM, Adrian Moreno wrote:
>> Only the mapping of the vring addresses is being ensured. This causes
>> errors when the vring size is larger than the IOTLB page size. E.g:
>> queue sizes > 256 for 4K IOTLB pages
>>
>> Ensure the entire vring memory range gets mapped. Refactor duplicated
>> code for for IOTLB UPDATE and IOTLB INVALIDATE and add packed virtqueue
>> support.
>>
>> Fixes: 09927b524969 ("vhost: translate ring addresses when IOMMU enabled")
>> Cc: maxime.coquelin@redhat.com
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>> ---
>> lib/librte_vhost/vhost_user.c | 64 +++++++++++++++++++++--------------
>> 1 file changed, 39 insertions(+), 25 deletions(-)
Applied to dpdk-next-virtio/master.
Thanks,
Maxime
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-09-27 9:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 12:50 [dpdk-dev] [PATCH] vhost: fix vring memory partially mapped Adrian Moreno
2019-09-17 19:03 ` Adrian Moreno
2019-09-27 9:58 ` Maxime Coquelin
2019-09-24 11:58 ` Maxime Coquelin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).