Hi Jean,
On 12/10/19 5:34 PM, Jean-Philippe Brucker wrote:
> Two small things below, but looks good overall
>
> Reviewed-by: Jean-Philippe Brucker <jean-phili...@linaro.org>
>
> On Fri, Nov 22, 2019 at 07:29:27PM +0100, Eric Auger wrote:
>> +static AddressSpace *virtio_iommu_find_add_as(PCIBus *bus, void *opaque,
>> + int devfn)
>> +{
>> + VirtIOIOMMU *s = opaque;
>> + IOMMUPciBus *sbus = g_hash_table_lookup(s->as_by_busptr, bus);
>> + static uint32_t mr_index;
>> + IOMMUDevice *sdev;
>> +
>> + if (!sbus) {
>> + sbus = g_malloc0(sizeof(IOMMUPciBus) +
>> + sizeof(IOMMUDevice *) * IOMMU_PCI_DEVFN_MAX);
>> + sbus->bus = bus;
>> + g_hash_table_insert(s->as_by_busptr, bus, sbus);
>> + }
>> +
>> + sdev = sbus->pbdev[devfn];
>> + if (!sdev) {
>> + char *name = g_strdup_printf("%s-%d-%d",
>> + TYPE_VIRTIO_IOMMU_MEMORY_REGION,
>> + mr_index++, devfn);
>> + sdev = sbus->pbdev[devfn] = g_malloc0(sizeof(IOMMUDevice));
>> +
>> + sdev->viommu = s;
>> + sdev->bus = bus;
>> + sdev->devfn = devfn;
>
> It might be better to store the endpoint ID in IOMMUDevice, then you could
> get rid of virtio_iommu_get_sid(), and remove a tiny bit of overhead in
> virtio_iommu_translate(). But I doubt it's significant.
virtio_iommu_find_add_as() gets called on PCI bus enumeration. At that
point, the bus number may not be resolved. So I cannot retrieve and set
the bus_number in this function.
When virtio_iommu_get_sid() is called we are sure pci_bus_num(dev->bus)
returns a correct value.
>
> [...]
>> +static const TypeInfo virtio_iommu_memory_region_info = {
>> + .parent = TYPE_IOMMU_MEMORY_REGION,
>> + .name = TYPE_VIRTIO_IOMMU_MEMORY_REGION,
>> + .class_init = virtio_iommu_memory_region_class_init,
>> +};
>> +
>> +
>
> nit: newline.
Thanks
Eric
>
> Thanks,
> Jean
>
