Hi Nicolin,
On 3/31/26 5:06 PM, Eric Auger wrote:
>
>
> On 3/30/26 11:06 AM, Nicolin Chen wrote:
>> On Mon, Mar 23, 2026 at 07:30:20PM +0100, Eric Auger wrote:
>>> @@ -3307,11 +3307,16 @@ void pci_setup_iommu(PCIBus *bus, const PCIIOMMUOps
>>> *ops, void *opaque)
>>> * IOMMU ops are returned, avoiding the use of the parent’s IOMMU when
>>> * it's not appropriate.
>>> */
>>> -void pci_setup_iommu_per_bus(PCIBus *bus, const PCIIOMMUOps *ops,
>>> - void *opaque)
>>> +bool pci_setup_iommu_per_bus(PCIBus *bus, const PCIIOMMUOps *ops,
>>> + void *opaque, Error **errp)
>>> {
>>> + if (bus->iommu_per_bus) {
>>> + error_setg(errp, "An iommu is already attached to this bus");
>>> + return false;
>>> + }
>>> pci_setup_iommu(bus, ops, opaque);
>>> bus->iommu_per_bus = true;
>>> + return true;
>> Do we need the same check in pci_setup_iommu()?
I further looked at it. pci_setup_iommu registers a machine wide iommu.
Only ARM uses pci_setup_iommu_per_bus for now and usage of both
pci_setup_iommu and pci_setup_iommu_per_bus are exclusive and check is
done at machine level.
So I think it is reasonable for now to keep that change limited to
pci_setup_iommu_per_bus.
Needs a rebase though, including Philippe's suggestion.
Thanks
Eric
>
> For machine wide IOMMU the check is handled in machine code (at least in
> arm virt).
>
> Nevertheless a more generic solution could be to check in
> pci_setup_iommu() that bus->iommu_ops is not already set. If set, return
> an error, cascaded up through pci_setup_iommu() and
> pci_setup_iommu_per_bus().
> However the change is much more invasive and impacts several archs. What
> do you think?
>
> I don't think this can be a valid use case to call pci_setup_iommu()
> several times on the same bus?
>>
>> Apart from this question,
>>
>> Reviewed-by: Nicolin Chen <[email protected]>
>
> Thanks
>
> Eric
>>
>
