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;