Re: [Qemu-arm] [Qemu-devel] [PATCH v9 02/14] hw/arm/smmu-common: IOMMU memory region and address space setup

[Peter Maydell]

On 6 March 2018 at 14:47, Auger Eric <address@hidden> wrote:
> Hi Peter,
>
> On 06/03/18 15:08, Peter Maydell wrote:
>> On 17 February 2018 at 18:46, Eric Auger <address@hidden> wrote:
>>> +    if (!smmu_pci_bus) {
>>> +        GHashTableIter iter;
>>> +
>>> +        g_hash_table_iter_init(&iter, s->smmu_as_by_busptr);
>>> +        while (g_hash_table_iter_next(&iter, NULL, (void 
>>> **)&smmu_pci_bus)) {
>>> +            if (pci_bus_num(smmu_pci_bus->bus) == bus_num) {
>>> +                s->smmu_as_by_bus_num[bus_num] = smmu_pci_bus;
>>> +                return smmu_pci_bus;
>>> +            }
>>> +        }
>>
>> Why do we populate this hashtable lazily rather than when we
>> put the SMMUPciBus in the smmu_as_by_busptr table? Do we
>> expect this function not to ordinarily be called?
>
> This function only is used when handling invalidations (vhost/vfio). I
> can even remove it from this series at this stage and re-introduce
> latter on. From the SID, you retrieve bus_num, retrieve the SMMUPciBus.
> Andd from the function number you can then retrieve the IOMMU mr at
> smmu_bus->pbdev[devfn]
>
> That's the purpose of ths function. But I will remove it.

No, it's fine here. You should just have a comment about why
it makes sense to lazily populate the hashtable (it's overall
going to be slower than if we populated it up front), to
document the rationale for the design decision.

thanks
-- PMM