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 <jonah.palmer@oracle.com>
---
 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

On Fri, Jan 10, 2025 at 6:09 PM Jonah Palmer <jonah.palmer@oracle.com> 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 <jonah.palmer@oracle.com>
> ---
>  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
>

On 1/16/25 12:02 PM, Eugenio Perez Martin wrote:
> On Fri, Jan 10, 2025 at 6:09 PM Jonah Palmer <jonah.palmer@oracle.com> 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 <jonah.palmer@oracle.com>
>> ---
>>   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
>>
>