[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0 v11 04/20] virtio-iommu: Add the iommu regions
From: |
Auger Eric |
Subject: |
Re: [PATCH for-5.0 v11 04/20] virtio-iommu: Add the iommu regions |
Date: |
Thu, 19 Dec 2019 19:11:08 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
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 <address@hidden>
>
> 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
>