Hi Jean,
On 12/20/19 6:00 PM, Jean-Philippe Brucker wrote:
> On Thu, Dec 19, 2019 at 07:31:08PM +0100, Auger Eric wrote:
>> Hi Jean,
>>
>> On 12/10/19 5:37 PM, Jean-Philippe Brucker wrote:
>>> On Fri, Nov 22, 2019 at 07:29:28PM +0100, Eric Auger wrote:
>>>> +typedef struct viommu_domain {
>>>> + uint32_t id;
>>>> + GTree *mappings;
>>>> + QLIST_HEAD(, viommu_endpoint) endpoint_list;
>>>> +} viommu_domain;
>>>> +
>>>> +typedef struct viommu_endpoint {
>>>> + uint32_t id;
>>>> + viommu_domain *domain;
>>>> + QLIST_ENTRY(viommu_endpoint) next;
>>>> +} viommu_endpoint;
>>>
>>> There might be a way to merge viommu_endpoint and the IOMMUDevice
>>> structure introduced in patch 4, since they both represent one endpoint.
>>> Maybe virtio_iommu_find_add_pci_as() could add the IOMMUDevice to
>>> s->endpoints, and IOMMUDevice could store the endpoint ID rather than bus
>>> and devfn.
>>
>> On PCI bus enumeration we locally store the PCI bus hierarchy under the
>> form of GHashTable of IOMMUDevice indexed by iommu_pci_bus pointer.
>> Those are all the devices attached to the downstream buses. We also use
>> an array of iommu pci bus pointers indexed by bus number that is lazily
>> populated due to the fact, at enumeration time we do know the bus number
>> yet. As you pointed, I haven't used the array of iommu pci bus pointers
>> indexed by bus number in this series and I should actually. Currently I
>> am not checking on attach that the sid effectively corresponds to a sid
>> protected by this iommu. I will add this in my next version. The above
>> structures are used in intel_iommu and smmu code as well and I think
>> eventually this may be factorized a common base class..
>>
>> on the other hand the gtree of viommu_endpoint - soon renamed in
>> CamelCase form ;-) - corresponds to the EPs that are actually attached
>> to any domain. It is indexed by sid and not by bus pointer. This is more
>> adapted to the virtio-iommu case.
>>
>> So, despite your suggestion, I am tempted to keep the different
>> structures as the first ones are common to all iommu emulation code and
>> the last is adapted to the virtio-iommu operations.
>>
>> Thoughts?
>
> Makes sense, it seems better to keep them separate. I had missed that the
> PCI bus number is resolved later, and started to move the endpoint ID into
> IOMMUDevice when adding MMIO support, but I'll need to revisit this.
>
> I'll be off for two weeks, have a nice holiday!
Thanks, you too.
Merry Christmas! :-)
Eric
>
> Thanks,
> Jean
>
