On Tue, Jun 09, 2020 at 11:15:06AM +0200, Arnd Bergmann wrote: > On Tue, Jun 9, 2020 at 6:02 AM Zhangfei Gao <zhangfei.gao@xxxxxxxxxx> wrote: > > On 2020/6/9 上午12:41, Bjorn Helgaas wrote: > > > On Mon, Jun 08, 2020 at 10:54:15AM +0800, Zhangfei Gao wrote: > > >> On 2020/6/6 上午7:19, Bjorn Helgaas wrote: > > >>>> +++ b/drivers/iommu/iommu.c > > >>>> @@ -2418,6 +2418,10 @@ int iommu_fwspec_init(struct device *dev, struct > > >>>> fwnode_handle *iommu_fwnode, > > >>>> fwspec->iommu_fwnode = iommu_fwnode; > > >>>> fwspec->ops = ops; > > >>>> dev_iommu_fwspec_set(dev, fwspec); > > >>>> + > > >>>> + if (dev_is_pci(dev)) > > >>>> + pci_fixup_device(pci_fixup_final, to_pci_dev(dev)); > > >>>> + > > >>>> > > >>>> Then pci_fixup_final will be called twice, the first in pci_bus_add_device. > > >>>> Here in iommu_fwspec_init is the second time, specifically for iommu_fwspec. > > >>>> Will send this when 5.8-rc1 is open. > > >>> Wait, this whole fixup approach seems wrong to me. No matter how you > > >>> do the fixup, it's still a fixup, which means it requires ongoing > > >>> maintenance. Surely we don't want to have to add the Vendor/Device ID > > >>> for every new AMBA device that comes along, do we? > > >>> > > >> Here the fake pci device has standard PCI cfg space, but physical > > >> implementation is base on AMBA > > >> They can provide pasid feature. > > >> However, > > >> 1, does not support tlp since they are not real pci devices. > > >> 2. does not support pri, instead support stall (provided by smmu) > > >> And stall is not a pci feature, so it is not described in struct pci_dev, > > >> but in struct iommu_fwspec. > > >> So we use this fixup to tell pci system that the devices can support stall, > > >> and hereby support pasid. > > > This did not answer my question. Are you proposing that we update a > > > quirk every time a new AMBA device is released? I don't think that > > > would be a good model. > > > > Yes, you are right, but we do not have any better idea yet. > > Currently we have three fake pci devices, which support stall and pasid. > > We have to let pci system know the device can support pasid, because of > > stall feature, though not support pri. > > Do you have any other ideas? > > It sounds like the best way would be to allocate a PCI capability for it, so > detection can be done through config space, at least in future devices, > or possibly after a firmware update if the config space in your system > is controlled by firmware somewhere. Once there is a proper mechanism > to do this, using fixups to detect the early devices that don't use that > should be uncontroversial. I have no idea what the process or timeline > is to add new capabilities into the PCIe specification, or if this one > would be acceptable to the PCI SIG at all. That sounds like a possibility. The spec already defines a Vendor-Specific Extended Capability (PCIe r5.0, sec 7.9.5) that might be a candidate. > If detection cannot be done through PCI config space, the next best > alternative is to pass auxiliary data through firmware. On DT based > machines, you can list non-hotpluggable PCIe devices and add custom > properties that could be read during device enumeration. I assume > ACPI has something similar, but I have not done that. ACPI has _DSM (ACPI v6.3, sec 9.1.1), which might be a candidate. I like this better than a PCI capability because the property you need to expose is not a PCI property.
