Hi jean,
On 12/10/19 5:43 PM, Jean-Philippe Brucker wrote:
> 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).
Sure
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.
with KVM accelerated guest I don't have access to the guest page size,
hence the choice of not checking it.
>
>> +
>> + 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.
Sure
Thanks!
Eric
>
> 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;
>
