Message ID | 20250314071058.6713-1-zhangfei.gao@linaro.org (mailing list archive) |
---|---|
State | Handled Elsewhere |
Delegated to: | Krzysztof Wilczyński |
Headers | show |
Series | PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER | expand |
Hello, > "bcb81ac6ae3c iommu: Get DT/ACPI parsing into the proper probe path" > changes arm_smmu_probe_device sequence. > > From > pci_bus_add_device(virtfn) > -> pci_fixup_device(pci_fixup_final, dev) > -> arm_smmu_probe_device > > To > pci_device_add(virtfn, virtfn->bus) > -> pci_fixup_device(pci_fixup_header, dev) > -> arm_smmu_probe_device > > So declare the fixup as pci_fixup_header to take effect > before arm_smmu_probe_device. Applied to quirks, thank you! Krzysztof
On Fri, 14 Mar 2025 at 15:42, Krzysztof Wilczyński <kw@linux.com> wrote: > > Hello, > > > "bcb81ac6ae3c iommu: Get DT/ACPI parsing into the proper probe path" > > changes arm_smmu_probe_device sequence. > > > > From > > pci_bus_add_device(virtfn) > > -> pci_fixup_device(pci_fixup_final, dev) > > -> arm_smmu_probe_device > > > > To > > pci_device_add(virtfn, virtfn->bus) > > -> pci_fixup_device(pci_fixup_header, dev) > > -> arm_smmu_probe_device > > > > So declare the fixup as pci_fixup_header to take effect > > before arm_smmu_probe_device. > > Applied to quirks, thank you! Thanks Krzysztof
On Fri, Mar 14, 2025 at 07:10:58AM +0000, Zhangfei Gao wrote: > "bcb81ac6ae3c iommu: Get DT/ACPI parsing into the proper probe path" > changes arm_smmu_probe_device sequence. Normal commit reference style is: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path") bcb81ac6ae3c is not a valid upstream commit. It does appear in next-20250314, incorporated via f5a5f66e2791 ("Merge branches 'apple/dart', 'arm/smmu/updates', 'arm/smmu/bindings', 's390', 'core', 'intel/vt-d' and 'amd/amd-vi' into next") from git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git. I think there's some value in keeping fixes close to whatever needs to be fixed, so since bcb81ac6ae3c came via the iommu tree, I would tend to merge the fix the same way. Unless there's a rebase to merge this change before bcb81ac6ae3c, this probably needs a "Fixes:" tag so people who backport bcb81ac6ae3c have a hint that this quirk change should be backported along with it. > From > pci_bus_add_device(virtfn) > -> pci_fixup_device(pci_fixup_final, dev) > -> arm_smmu_probe_device > > To > pci_device_add(virtfn, virtfn->bus) > -> pci_fixup_device(pci_fixup_header, dev) > -> arm_smmu_probe_device This doesn't include enough detail to show the change. I don't know the path to arm_smmu_probe_device() and how it relates to pci_bus_add_device() and pci_device_add(). > So declare the fixup as pci_fixup_header to take effect > before arm_smmu_probe_device. > > Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> > --- > drivers/pci/quirks.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index f840d611c450..a9759889ff5e 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -1991,12 +1991,12 @@ static void quirk_huawei_pcie_sva(struct pci_dev *pdev) > device_create_managed_software_node(&pdev->dev, properties, NULL)) > pci_warn(pdev, "could not add stall property"); > } > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva); > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva); > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa255, quirk_huawei_pcie_sva); > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa256, quirk_huawei_pcie_sva); > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa258, quirk_huawei_pcie_sva); > -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa259, quirk_huawei_pcie_sva); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa255, quirk_huawei_pcie_sva); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa256, quirk_huawei_pcie_sva); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa258, quirk_huawei_pcie_sva); > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa259, quirk_huawei_pcie_sva); > > /* > * It's possible for the MSI to get corrupted if SHPC and ACPI are used > -- > 2.25.1 >
On Sat, 15 Mar 2025 at 00:28, Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Fri, Mar 14, 2025 at 07:10:58AM +0000, Zhangfei Gao wrote: > > "bcb81ac6ae3c iommu: Get DT/ACPI parsing into the proper probe path" > > changes arm_smmu_probe_device sequence. > > Normal commit reference style is: > > bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path") > > bcb81ac6ae3c is not a valid upstream commit. It does appear in > next-20250314, incorporated via f5a5f66e2791 ("Merge branches > 'apple/dart', 'arm/smmu/updates', 'arm/smmu/bindings', 's390', 'core', > 'intel/vt-d' and 'amd/amd-vi' into next") from > git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git. > > I think there's some value in keeping fixes close to whatever needs to > be fixed, so since bcb81ac6ae3c came via the iommu tree, I would tend > to merge the fix the same way. OK, understand. Then would you mind give an ack. > > Unless there's a rebase to merge this change before bcb81ac6ae3c, this > probably needs a "Fixes:" tag so people who backport bcb81ac6ae3c have > a hint that this quirk change should be backported along with it. > > > From > > pci_bus_add_device(virtfn) > > -> pci_fixup_device(pci_fixup_final, dev) > > -> arm_smmu_probe_device > > > > To > > pci_device_add(virtfn, virtfn->bus) > > -> pci_fixup_device(pci_fixup_header, dev) > > -> arm_smmu_probe_device > > This doesn't include enough detail to show the change. I don't know > the path to arm_smmu_probe_device() and how it relates to > pci_bus_add_device() and pci_device_add(). > Thanks Bjorn How about changing to this. Subject: [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER The arm_smmu_probe_device is now called earlier via pci_device_add, which calls pci_fixup_device(pci_fixup_header, dev), while originally it is called from pci_bus_add_device, which calls pci_fixup_device(pci_fixup_final, dev). So declare the fixup as pci_fixup_header to take effect before arm_smmu_probe_device. Calling stack Before: [ 1121.314405] arm_smmu_probe_device+0x48/0x450 [ 1121.314410] __iommu_probe_device+0xc4/0x3c8 [ 1121.314412] iommu_probe_device+0x40/0x90 [ 1121.314414] acpi_dma_configure_id+0xb4/0x100 [ 1121.314417] pci_dma_configure+0xf8/0x108 [ 1121.314421] really_probe+0x78/0x278 [ 1121.314425] __driver_probe_device+0x80/0x140 [ 1121.314427] driver_probe_device+0x48/0x130 [ 1121.314430] __device_attach_driver+0xc0/0x108 [ 1121.314432] bus_for_each_drv+0x8c/0xf8 [ 1121.314435] __device_attach+0x104/0x1a0 [ 1121.314437] device_attach+0x1c/0x30 [ 1121.314440] pci_bus_add_device+0xb8/0x1f0 [ 1121.314442] pci_iov_add_virtfn+0x2ac/0x300 Now: [ 215.072859] arm_smmu_probe_device+0x48/0x450 [ 215.072871] __iommu_probe_device+0xc0/0x468 [ 215.072875] iommu_probe_device+0x40/0x90 [ 215.072877] iommu_bus_notifier+0x38/0x68 [ 215.072879] notifier_call_chain+0x80/0x148 [ 215.072886] blocking_notifier_call_chain+0x50/0x80 [ 215.072889] bus_notify+0x44/0x68 [ 215.072896] device_add+0x580/0x768 [ 215.072898] pci_device_add+0x1e8/0x568 [ 215.072906] pci_iov_add_virtfn+0x198/0x300 Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path") Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> Thanks
Hello, > > bcb81ac6ae3c is not a valid upstream commit. It does appear in > > next-20250314, incorporated via f5a5f66e2791 ("Merge branches > > 'apple/dart', 'arm/smmu/updates', 'arm/smmu/bindings', 's390', 'core', > > 'intel/vt-d' and 'amd/amd-vi' into next") from > > git://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git. > > > > I think there's some value in keeping fixes close to whatever needs to > > be fixed, so since bcb81ac6ae3c came via the iommu tree, I would tend > > to merge the fix the same way. I will drop this patch then. Old commit from the PCI tree for reference: https://web.git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?h=quirks&id=0cc24e705588325583f2b77662de61339548c39c Thank you! Krzysztof
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index f840d611c450..a9759889ff5e 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -1991,12 +1991,12 @@ static void quirk_huawei_pcie_sva(struct pci_dev *pdev) device_create_managed_software_node(&pdev->dev, properties, NULL)) pci_warn(pdev, "could not add stall property"); } -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva); -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva); -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa255, quirk_huawei_pcie_sva); -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa256, quirk_huawei_pcie_sva); -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa258, quirk_huawei_pcie_sva); -DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa259, quirk_huawei_pcie_sva); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa255, quirk_huawei_pcie_sva); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa256, quirk_huawei_pcie_sva); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa258, quirk_huawei_pcie_sva); +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HUAWEI, 0xa259, quirk_huawei_pcie_sva); /* * It's possible for the MSI to get corrupted if SHPC and ACPI are used
"bcb81ac6ae3c iommu: Get DT/ACPI parsing into the proper probe path" changes arm_smmu_probe_device sequence. From pci_bus_add_device(virtfn) -> pci_fixup_device(pci_fixup_final, dev) -> arm_smmu_probe_device To pci_device_add(virtfn, virtfn->bus) -> pci_fixup_device(pci_fixup_header, dev) -> arm_smmu_probe_device So declare the fixup as pci_fixup_header to take effect before arm_smmu_probe_device. Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org> --- drivers/pci/quirks.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)