Re: [Qemu-devel] [RFC v4 10/16] virtio-iommu: Implement probe request

[Tomasz Nowicki]

Hi Eric,

On 19.09.2017 09:46, Eric Auger wrote:
> This patch implements the PROBE request. At the moment,
> no reserved regions are returned.
>
> At the moment reserved regions are stored per device.
>
> Signed-off-by: Eric Auger <address@hidden>
>
> ---

[...]

> +
> +static int virtio_iommu_fill_property(int devid, int type,
> +                                      viommu_property_buffer *bufstate)
> +{
> +    int ret = -ENOSPC;
> +
> +    if (bufstate->filled + 4 >= VIOMMU_PROBE_SIZE) {
> +        bufstate->error = true;
> +        goto out;
> +    }
> +
> +    switch (type) {
> +    case VIRTIO_IOMMU_PROBE_T_NONE:
> +        ret = virtio_iommu_fill_none_prop(bufstate);
> +        break;
> +    case VIRTIO_IOMMU_PROBE_T_RESV_MEM:
> +    {
> +        viommu_dev *dev = bufstate->dev;
> +
> +        g_tree_foreach(dev->reserved_regions,
> +                       virtio_iommu_fill_resv_mem_prop,
> +                       bufstate);
> +        if (!bufstate->error) {
> +            ret = 0;
> +        }
> +        break;
> +    }
> +    default:
> +        ret = -ENOENT;
> +        break;
> +    }
> +out:
> +    if (ret) {
> +        error_report("%s property of type=%d could not be filled (%d),"
> +                     " remaining size = 0x%lx",
> +                     __func__, type, ret, bufstate->filled);
> +    }
> +    return ret;
> +}
> +
> +static int virtio_iommu_probe(VirtIOIOMMU *s,
> +                              struct virtio_iommu_req_probe *req,
> +                              uint8_t *buf)
> +{
> +    uint32_t devid = le32_to_cpu(req->device);
> +    int16_t prop_types = SUPPORTED_PROBE_PROPERTIES, type;
> +    viommu_property_buffer bufstate;
> +    viommu_dev *dev;
> +    int ret;
> +
> +    dev = g_tree_lookup(s->devices, GUINT_TO_POINTER(devid));
> +    if (!dev) {
> +        return -EINVAL;
> +    }
> +
> +    bufstate.start = buf;
> +    bufstate.filled = 0;
> +    bufstate.dev = dev;

bufstate.error is not initialized which may cause false alarm in 
virtio_iommu_fill_property()

> +
> +    while ((type = ctz32(prop_types)) != 32) {
> +        ret = virtio_iommu_fill_property(devid, 1 << type, &bufstate);
> +        if (ret) {
> +            break;
> +        }
> +        prop_types &= ~(1 << type);
> +    }
> +    virtio_iommu_fill_property(devid, VIRTIO_IOMMU_PROBE_T_NONE, &bufstate);
> +
> +    return VIRTIO_IOMMU_S_OK;
> +}
> +
>   #define get_payload_size(req) (\
>   sizeof((req)) - sizeof(struct virtio_iommu_req_tail))
>   
> @@ -433,6 +567,24 @@ static int virtio_iommu_handle_unmap(VirtIOIOMMU *s,
>       return virtio_iommu_unmap(s, &req);
>   }

Thanks,
Tomasz