Hi, Bjorn
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:
On Thu, Jun 04, 2020 at 09:33:07PM +0800, Zhangfei Gao wrote:
On 2020/6/2 上午1:41, Bjorn Helgaas wrote:
On Thu, May 28, 2020 at 09:33:44AM +0200, Joerg Roedel wrote:
On Wed, May 27, 2020 at 01:18:42PM -0500, Bjorn Helgaas wrote:
Is this slowdown significant? We already iterate over every device
when applying PCI_FIXUP_FINAL quirks, so if we used the existing
PCI_FIXUP_FINAL, we wouldn't be adding a new loop. We would only be
adding two more iterations to the loop in pci_do_fixups() that tries
to match quirks against the current device. I doubt that would be a
measurable slowdown.
I don't know how significant it is, but I remember people complaining
about adding new PCI quirks because it takes too long for them to run
them all. That was in the discussion about the quirk disabling ATS on
AMD Stoney systems.
So it probably depends on how many PCI devices are in the system whether
it causes any measureable slowdown.
I found this [1] from Paul Menzel, which was a slowdown caused by
quirk_usb_early_handoff(). I think the real problem is individual
quirks that take a long time.
The PCI_FIXUP_IOMMU things we're talking about should be fast, and of
course, they're only run for matching devices anyway. So I'd rather
keep them as PCI_FIXUP_FINAL than add a whole new phase.
Thanks Bjorn for taking time for this.
If so, it would be much simpler.
+++ 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?
Thanks
