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

[Auger Eric <eric.auger@redhat.com>]

Hi Bharat,

On 27/09/2017 13:00, Bharat Bhushan wrote:
> 
> 
>> -----Original Message-----
>> From: Tomasz Nowicki [mailto:tnowicki@caviumnetworks.com]
>> Sent: Wednesday, September 27, 2017 4:23 PM
>> To: Eric Auger <eric.auger@redhat.com>; eric.auger.pro@gmail.com;
>> peter.maydell@linaro.org; alex.williamson@redhat.com; mst@redhat.com;
>> qemu-arm@nongnu.org; qemu-devel@nongnu.org; jean-
>> philippe.brucker@arm.com
>> Cc: will.deacon@arm.com; kevin.tian@intel.com; marc.zyngier@arm.com;
>> christoffer.dall@linaro.org; drjones@redhat.com; wei@redhat.com; Bharat
>> Bhushan <bharat.bhushan@nxp.com>; peterx@redhat.com;
>> linuc.decode@gmail.com
>> Subject: Re: [RFC v4 10/16] virtio-iommu: Implement probe request
>>
>> 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 <eric.auger@redhat.com>
>>>
>>> ---
>>>
>>
>> [...]
>>
>>> +
>>> +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()
> 
> I observed below prints 
> 	 "qemu-system-aarch64: virtio_iommu_fill_property property of type=2 could not be filled (-28), remaining size = 0x0 "
> 
> When I initialized the bufstate.error = 0,it goes.

Sure, I will fix that soon.

Best Regards

Eric
> 
> Thanks
> -Bharat
> 
>>
>>> +
>>> +    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