Re: [RFC v3 2/5] vhost-iova-tree: Remove range check for IOVA allocator

[Jonah Palmer]

On 1/16/25 12:02 PM, Eugenio Perez Martin wrote:

On Fri, Jan 10, 2025 at 6:09 PM Jonah Palmer  wrote:


Removes the range check portion in vhost_iova_tree_map_alloc.

The previous patch decoupled the IOVA allocator from adding mappings to
the IOVA->HVA tree (now a partial SVQ IOVA->HVA tree) and instead adds
the allocated IOVA range to an IOVA-only tree. No value exists under
translated_addr for the IOVA-only mappings, so this check is no longer
needed.

This check was moved to vhost_iova_tree_insert in the previous patch
since that function handles adding IOVA->HVA mappings to the SVQ
IOVA->HVA tree.

Signed-off-by: Jonah Palmer 
---
  hw/virtio/vhost-iova-tree.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-iova-tree.c b/hw/virtio/vhost-iova-tree.c
index b1cfd17843..f6a5694857 100644
--- a/hw/virtio/vhost-iova-tree.c
+++ b/hw/virtio/vhost-iova-tree.c
@@ -93,8 +93,7 @@ int vhost_iova_tree_map_alloc(VhostIOVATree *tree, DMAMap 
*map)
  /* Some vhost devices do not like addr 0. Skip first page */
  hwaddr iova_first = tree->iova_first ?: qemu_real_host_page_size();



This is not a static function, so I guess it is better to duplicate
the check if needed? Otherwise a buggy caller can create invalid
entries.



Gotcha. I can drop this patch then.


-if (map->translated_addr + map->size < map->translated_addr ||
-map->perm == IOMMU_NONE) {
+if (map->perm == IOMMU_NONE) {
  return IOVA_ERR_INVALID;
  }

--
2.43.5

Re: [RFC v3 2/5] vhost-iova-tree: Remove range check for IOVA allocator

[Eugenio Perez Martin]

On Fri, Jan 10, 2025 at 6:09 PM Jonah Palmer  wrote:
>
> Removes the range check portion in vhost_iova_tree_map_alloc.
>
> The previous patch decoupled the IOVA allocator from adding mappings to
> the IOVA->HVA tree (now a partial SVQ IOVA->HVA tree) and instead adds
> the allocated IOVA range to an IOVA-only tree. No value exists under
> translated_addr for the IOVA-only mappings, so this check is no longer
> needed.
>
> This check was moved to vhost_iova_tree_insert in the previous patch
> since that function handles adding IOVA->HVA mappings to the SVQ
> IOVA->HVA tree.
>
> Signed-off-by: Jonah Palmer 
> ---
>  hw/virtio/vhost-iova-tree.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/hw/virtio/vhost-iova-tree.c b/hw/virtio/vhost-iova-tree.c
> index b1cfd17843..f6a5694857 100644
> --- a/hw/virtio/vhost-iova-tree.c
> +++ b/hw/virtio/vhost-iova-tree.c
> @@ -93,8 +93,7 @@ int vhost_iova_tree_map_alloc(VhostIOVATree *tree, DMAMap 
> *map)
>  /* Some vhost devices do not like addr 0. Skip first page */
>  hwaddr iova_first = tree->iova_first ?: qemu_real_host_page_size();
>

This is not a static function, so I guess it is better to duplicate
the check if needed? Otherwise a buggy caller can create invalid
entries.

> -if (map->translated_addr + map->size < map->translated_addr ||
> -map->perm == IOMMU_NONE) {
> +if (map->perm == IOMMU_NONE) {
>  return IOVA_ERR_INVALID;
>  }
>
> --
> 2.43.5
>

[RFC v3 2/5] vhost-iova-tree: Remove range check for IOVA allocator

[Jonah Palmer]

Removes the range check portion in vhost_iova_tree_map_alloc.

The previous patch decoupled the IOVA allocator from adding mappings to
the IOVA->HVA tree (now a partial SVQ IOVA->HVA tree) and instead adds
the allocated IOVA range to an IOVA-only tree. No value exists under
translated_addr for the IOVA-only mappings, so this check is no longer
needed.

This check was moved to vhost_iova_tree_insert in the previous patch
since that function handles adding IOVA->HVA mappings to the SVQ
IOVA->HVA tree.

Signed-off-by: Jonah Palmer 
---
 hw/virtio/vhost-iova-tree.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-iova-tree.c b/hw/virtio/vhost-iova-tree.c
index b1cfd17843..f6a5694857 100644
--- a/hw/virtio/vhost-iova-tree.c
+++ b/hw/virtio/vhost-iova-tree.c
@@ -93,8 +93,7 @@ int vhost_iova_tree_map_alloc(VhostIOVATree *tree, DMAMap 
*map)
 /* Some vhost devices do not like addr 0. Skip first page */
 hwaddr iova_first = tree->iova_first ?: qemu_real_host_page_size();
 
-if (map->translated_addr + map->size < map->translated_addr ||
-map->perm == IOMMU_NONE) {
+if (map->perm == IOMMU_NONE) {
 return IOVA_ERR_INVALID;
 }
 
-- 
2.43.5