Re: [PATCH for-5.0 v11 07/20] virtio-iommu: Implement map/unmap

[Jean-Philippe Brucker <jean-philippe@linaro.org>]

On Fri, Nov 22, 2019 at 07:29:30PM +0100, Eric Auger wrote:
> @@ -238,10 +244,35 @@ static int virtio_iommu_map(VirtIOIOMMU *s,
>      uint64_t virt_start = le64_to_cpu(req->virt_start);
>      uint64_t virt_end = le64_to_cpu(req->virt_end);
>      uint32_t flags = le32_to_cpu(req->flags);
> +    viommu_domain *domain;
> +    viommu_interval *interval;
> +    viommu_mapping *mapping;

Additional checks would be good. Most importantly we need to return
S_INVAL if we don't recognize a bit in flags (a MUST in the spec). It
might be good to check that addresses are aligned on the page granule as
well, and return S_RANGE if they aren't (a SHOULD in the spec), but I
don't care as much.

> +
> +    interval = g_malloc0(sizeof(*interval));
> +
> +    interval->low = virt_start;
> +    interval->high = virt_end;
> +
> +    domain = g_tree_lookup(s->domains, GUINT_TO_POINTER(domain_id));
> +    if (!domain) {
> +        return VIRTIO_IOMMU_S_NOENT;

Leaks interval, I guess you could allocate it after this block.

Thanks,
Jean

> +    }
> +
> +    mapping = g_tree_lookup(domain->mappings, (gpointer)interval);
> +    if (mapping) {
> +        g_free(interval);
> +        return VIRTIO_IOMMU_S_INVAL;
> +    }
>  
>      trace_virtio_iommu_map(domain_id, virt_start, virt_end, phys_start, flags);
>  
> -    return VIRTIO_IOMMU_S_UNSUPP;
> +    mapping = g_malloc0(sizeof(*mapping));
> +    mapping->phys_addr = phys_start;
> +    mapping->flags = flags;
> +
> +    g_tree_insert(domain->mappings, interval, mapping);
> +
> +    return VIRTIO_IOMMU_S_OK;