* Re: [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER
2025-03-14 7:10 [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER Zhangfei Gao
@ 2025-03-14 7:42 ` Krzysztof Wilczyński
2025-03-14 7:45 ` Zhangfei Gao
2025-03-14 16:28 ` Bjorn Helgaas
` (2 subsequent siblings)
3 siblings, 1 reply; 15+ messages in thread
From: Krzysztof Wilczyński @ 2025-03-14 7:42 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, iommu, linux-pci
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER
2025-03-14 7:42 ` Krzysztof Wilczyński
@ 2025-03-14 7:45 ` Zhangfei Gao
0 siblings, 0 replies; 15+ messages in thread
From: Zhangfei Gao @ 2025-03-14 7:45 UTC (permalink / raw)
To: Krzysztof Wilczyński
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, iommu, linux-pci
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER
2025-03-14 7:10 [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER Zhangfei Gao
2025-03-14 7:42 ` Krzysztof Wilczyński
@ 2025-03-14 16:28 ` Bjorn Helgaas
2025-03-15 1:29 ` Zhangfei Gao
2025-03-15 10:10 ` [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header Zhangfei Gao
2025-03-15 10:13 ` [PATCH v2] " Zhangfei Gao
3 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2025-03-14 16:28 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, iommu, linux-pci
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER
2025-03-14 16:28 ` Bjorn Helgaas
@ 2025-03-15 1:29 ` Zhangfei Gao
2025-03-15 4:22 ` Krzysztof Wilczyński
0 siblings, 1 reply; 15+ messages in thread
From: Zhangfei Gao @ 2025-03-15 1:29 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, iommu, linux-pci
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
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER
2025-03-15 1:29 ` Zhangfei Gao
@ 2025-03-15 4:22 ` Krzysztof Wilczyński
0 siblings, 0 replies; 15+ messages in thread
From: Krzysztof Wilczyński @ 2025-03-15 4:22 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Bjorn Helgaas, Bjorn Helgaas, Baolu Lu, Robin Murphy,
Joerg Roedel, Jason Gunthorpe, iommu, linux-pci
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-14 7:10 [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER Zhangfei Gao
2025-03-14 7:42 ` Krzysztof Wilczyński
2025-03-14 16:28 ` Bjorn Helgaas
@ 2025-03-15 10:10 ` Zhangfei Gao
2025-03-17 1:13 ` [PATCH v3] " Zhangfei Gao
2025-03-15 10:13 ` [PATCH v2] " Zhangfei Gao
3 siblings, 1 reply; 15+ messages in thread
From: Zhangfei Gao @ 2025-03-15 10:10 UTC (permalink / raw)
To: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński
Cc: iommu, linux-pci, llinux-kernel, Zhangfei Gao
The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
probe path") changes the arm_smmu_probe_device() sequence.
The arm_smmu_probe_device() is now called earlier via pci_device_add(),
which calls pci_fixup_device() at the "pci_fixup_header" phase, while
originally it was called from the pci_bus_add_device(), which called
pci_fixup_device() at the "pci_fixup_final" phase.
The callstack 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
And after:
[ 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>
[kwilczynski: commit log]
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
---
v2: Modify commit log
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v3] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-15 10:10 ` [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header Zhangfei Gao
@ 2025-03-17 1:13 ` Zhangfei Gao
2025-03-20 14:07 ` Zhangfei Gao
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: Zhangfei Gao @ 2025-03-17 1:13 UTC (permalink / raw)
To: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński
Cc: iommu, linux-pci, linux-kernel, Zhangfei Gao
The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
probe path") fixed the iommu_probe_device() flow to correctly initialize
firmware operations, allowing arm_smmu_probe_device() to be invoked
earlier. This changes the invocation timing of arm_smmu_probe_device
from the final fixup phase to the header fixup phase.
pci_iov_add_virtfn
pci_device_add
pci_fixup_device(pci_fixup_header) <--
device_add
bus_notify
iommu_bus_notifier
+ iommu_probe_device
+ arm_smmu_probe_device
pci_bus_add_device
pci_fixup_device(pci_fixup_final) <--
device_attach
driver_probe_device
really_probe
pci_dma_configure
acpi_dma_configure_id
- iommu_probe_device
- arm_smmu_probe_device
This is the pci_iov_add_virtfn(). The non-SR-IOV case is similar in
that pci_device_add() is called from pci_scan_single_device() in the
generic enumeration path, and pci_bus_add_device() is called later,
after all a host bridge has been enumerated.
Declare the fixup as pci_fixup_header to ensure the configuration
happens before arm_smmu_probe_device.
Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
---
v3: modify commit msg, add Acked-by
v2: modify commit msg
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v3] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-17 1:13 ` [PATCH v3] " Zhangfei Gao
@ 2025-03-20 14:07 ` Zhangfei Gao
2025-04-07 11:56 ` Zhangfei Gao
2025-04-07 18:22 ` Bjorn Helgaas
2 siblings, 0 replies; 15+ messages in thread
From: Zhangfei Gao @ 2025-03-20 14:07 UTC (permalink / raw)
To: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński
Cc: iommu, linux-pci, linux-kernel
Hi, Joerg
On Mon, 17 Mar 2025 at 09:14, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
>
> The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
> probe path") fixed the iommu_probe_device() flow to correctly initialize
> firmware operations, allowing arm_smmu_probe_device() to be invoked
> earlier. This changes the invocation timing of arm_smmu_probe_device
> from the final fixup phase to the header fixup phase.
>
> pci_iov_add_virtfn
> pci_device_add
> pci_fixup_device(pci_fixup_header) <--
> device_add
> bus_notify
> iommu_bus_notifier
> + iommu_probe_device
> + arm_smmu_probe_device
> pci_bus_add_device
> pci_fixup_device(pci_fixup_final) <--
> device_attach
> driver_probe_device
> really_probe
> pci_dma_configure
> acpi_dma_configure_id
> - iommu_probe_device
> - arm_smmu_probe_device
>
> This is the pci_iov_add_virtfn(). The non-SR-IOV case is similar in
> that pci_device_add() is called from pci_scan_single_device() in the
> generic enumeration path, and pci_bus_add_device() is called later,
> after all a host bridge has been enumerated.
>
> Declare the fixup as pci_fixup_header to ensure the configuration
> happens before arm_smmu_probe_device.
>
> Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Would you mind take a look at this patch
Bjorn suggested
"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."
Thanks
> ---
> v3: modify commit msg, add Acked-by
> v2: modify commit msg
>
> 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
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-17 1:13 ` [PATCH v3] " Zhangfei Gao
2025-03-20 14:07 ` Zhangfei Gao
@ 2025-04-07 11:56 ` Zhangfei Gao
2025-04-07 18:22 ` Bjorn Helgaas
2 siblings, 0 replies; 15+ messages in thread
From: Zhangfei Gao @ 2025-04-07 11:56 UTC (permalink / raw)
To: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński
Cc: iommu, linux-pci, linux-kernel
Hi, Bjorn
On Mon, 17 Mar 2025 at 09:14, Zhangfei Gao <zhangfei.gao@linaro.org> wrote:
>
> The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
> probe path") fixed the iommu_probe_device() flow to correctly initialize
> firmware operations, allowing arm_smmu_probe_device() to be invoked
> earlier. This changes the invocation timing of arm_smmu_probe_device
> from the final fixup phase to the header fixup phase.
>
> pci_iov_add_virtfn
> pci_device_add
> pci_fixup_device(pci_fixup_header) <--
> device_add
> bus_notify
> iommu_bus_notifier
> + iommu_probe_device
> + arm_smmu_probe_device
> pci_bus_add_device
> pci_fixup_device(pci_fixup_final) <--
> device_attach
> driver_probe_device
> really_probe
> pci_dma_configure
> acpi_dma_configure_id
> - iommu_probe_device
> - arm_smmu_probe_device
>
> This is the pci_iov_add_virtfn(). The non-SR-IOV case is similar in
> that pci_device_add() is called from pci_scan_single_device() in the
> generic enumeration path, and pci_bus_add_device() is called later,
> after all a host bridge has been enumerated.
>
> Declare the fixup as pci_fixup_header to ensure the configuration
> happens before arm_smmu_probe_device.
>
> Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
As the commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
probe path") already merged in 6.15-rc1, would you mind taking this
patch for rc?
Now 6.15-rc1 can not work without this patch.
Thanks
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v3] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-17 1:13 ` [PATCH v3] " Zhangfei Gao
2025-03-20 14:07 ` Zhangfei Gao
2025-04-07 11:56 ` Zhangfei Gao
@ 2025-04-07 18:22 ` Bjorn Helgaas
2 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2025-04-07 18:22 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński, iommu, linux-pci,
linux-kernel
On Mon, Mar 17, 2025 at 01:13:52AM +0000, Zhangfei Gao wrote:
> The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
> probe path") fixed the iommu_probe_device() flow to correctly initialize
> firmware operations, allowing arm_smmu_probe_device() to be invoked
> earlier. This changes the invocation timing of arm_smmu_probe_device
> from the final fixup phase to the header fixup phase.
>
> pci_iov_add_virtfn
> pci_device_add
> pci_fixup_device(pci_fixup_header) <--
> device_add
> bus_notify
> iommu_bus_notifier
> + iommu_probe_device
> + arm_smmu_probe_device
> pci_bus_add_device
> pci_fixup_device(pci_fixup_final) <--
> device_attach
> driver_probe_device
> really_probe
> pci_dma_configure
> acpi_dma_configure_id
> - iommu_probe_device
> - arm_smmu_probe_device
>
> This is the pci_iov_add_virtfn(). The non-SR-IOV case is similar in
> that pci_device_add() is called from pci_scan_single_device() in the
> generic enumeration path, and pci_bus_add_device() is called later,
> after all a host bridge has been enumerated.
>
> Declare the fixup as pci_fixup_header to ensure the configuration
> happens before arm_smmu_probe_device.
>
> Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Applied to pci/for-linus for v6.15, thanks!
Joerg, if you'd rather take it, let me know and I can drop it.
> ---
> v3: modify commit msg, add Acked-by
> v2: modify commit msg
>
> 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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-14 7:10 [PATCH] PCI: declare quirk_huawei_pcie_sva as FIXUP_HEADER Zhangfei Gao
` (2 preceding siblings ...)
2025-03-15 10:10 ` [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header Zhangfei Gao
@ 2025-03-15 10:13 ` Zhangfei Gao
2025-03-15 11:33 ` Krzysztof Wilczyński
2025-03-15 18:45 ` Bjorn Helgaas
3 siblings, 2 replies; 15+ messages in thread
From: Zhangfei Gao @ 2025-03-15 10:13 UTC (permalink / raw)
To: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński
Cc: iommu, linux-pci, linux-kernel, Zhangfei Gao
The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
probe path") changes the arm_smmu_probe_device() sequence.
The arm_smmu_probe_device() is now called earlier via pci_device_add(),
which calls pci_fixup_device() at the "pci_fixup_header" phase, while
originally it was called from the pci_bus_add_device(), which called
pci_fixup_device() at the "pci_fixup_final" phase.
The callstack 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
And after:
[ 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>
[kwilczynski: commit log]
Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
---
v2: Modify commit log
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-15 10:13 ` [PATCH v2] " Zhangfei Gao
@ 2025-03-15 11:33 ` Krzysztof Wilczyński
2025-03-15 18:45 ` Bjorn Helgaas
1 sibling, 0 replies; 15+ messages in thread
From: Krzysztof Wilczyński @ 2025-03-15 11:33 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, iommu, linux-pci, linux-kernel
Hello,
> The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
> probe path") changes the arm_smmu_probe_device() sequence.
>
> The arm_smmu_probe_device() is now called earlier via pci_device_add(),
> which calls pci_fixup_device() at the "pci_fixup_header" phase, while
> originally it was called from the pci_bus_add_device(), which called
> pci_fixup_device() at the "pci_fixup_final" phase.
>
> The callstack 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
>
> And after:
> [ 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>
> [kwilczynski: commit log]
> Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
So, this will go through the IOMMU three, correct? Once Bjorn adds his "Acked-by" tag.
Just want to make sure.
Krzysztof
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-15 10:13 ` [PATCH v2] " Zhangfei Gao
2025-03-15 11:33 ` Krzysztof Wilczyński
@ 2025-03-15 18:45 ` Bjorn Helgaas
2025-03-16 4:15 ` Zhangfei Gao
1 sibling, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2025-03-15 18:45 UTC (permalink / raw)
To: Zhangfei Gao
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński, iommu, linux-pci,
linux-kernel
On Sat, Mar 15, 2025 at 10:13:19AM +0000, Zhangfei Gao wrote:
> The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
> probe path") changes the arm_smmu_probe_device() sequence.
> The arm_smmu_probe_device() is now called earlier via pci_device_add(),
> which calls pci_fixup_device() at the "pci_fixup_header" phase, while
> originally it was called from the pci_bus_add_device(), which called
> pci_fixup_device() at the "pci_fixup_final" phase.
>
> The callstack 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
>
> And after:
> [ 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
The stacktraces definitely help connect the dots but don't integrate
the fixup phases and the timestamps are unnecessary distraction.
I would omit all the above except the first paragraph and include
something like this instead, which shows how arm_smmu_probe_device()
was previously after final fixups and is now between header and final
fixups:
pci_iov_add_virtfn
pci_device_add
pci_fixup_device(pci_fixup_header) <--
device_add
bus_notify
iommu_bus_notifier
+ iommu_probe_device
+ arm_smmu_probe_device
pci_bus_add_device
pci_fixup_device(pci_fixup_final) <--
device_attach
driver_probe_device
really_probe
pci_dma_configure
acpi_dma_configure_id
- iommu_probe_device
- arm_smmu_probe_device
This is the pci_iov_add_virtfn(). The non-SR-IOV case is similar in
that pci_device_add() is called from pci_scan_single_device() in the
generic enumeration path, and pci_bus_add_device() is called later,
after all a host bridge has been enumerated.
> Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> [kwilczynski: commit log]
> Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
You should never include somebody else's Signed-off-by below yours.
You should only add *your own* Signed-off-by:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.13#n396
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
>
> v2: Modify commit log
>
> 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
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH v2] PCI: Declare quirk_huawei_pcie_sva() as pci_fixup_header
2025-03-15 18:45 ` Bjorn Helgaas
@ 2025-03-16 4:15 ` Zhangfei Gao
0 siblings, 0 replies; 15+ messages in thread
From: Zhangfei Gao @ 2025-03-16 4:15 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Bjorn Helgaas, Baolu Lu, Robin Murphy, Joerg Roedel,
Jason Gunthorpe, Krzysztof Wilczyński, iommu, linux-pci,
linux-kernel
On Sun, 16 Mar 2025 at 02:45, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Sat, Mar 15, 2025 at 10:13:19AM +0000, Zhangfei Gao wrote:
> > The commit bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper
> > probe path") changes the arm_smmu_probe_device() sequence.
>
> > The arm_smmu_probe_device() is now called earlier via pci_device_add(),
> > which calls pci_fixup_device() at the "pci_fixup_header" phase, while
> > originally it was called from the pci_bus_add_device(), which called
> > pci_fixup_device() at the "pci_fixup_final" phase.
> >
> > The callstack 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
> >
> > And after:
> > [ 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
>
> The stacktraces definitely help connect the dots but don't integrate
> the fixup phases and the timestamps are unnecessary distraction.
>
> I would omit all the above except the first paragraph and include
> something like this instead, which shows how arm_smmu_probe_device()
> was previously after final fixups and is now between header and final
> fixups:
>
> pci_iov_add_virtfn
> pci_device_add
> pci_fixup_device(pci_fixup_header) <--
> device_add
> bus_notify
> iommu_bus_notifier
> + iommu_probe_device
> + arm_smmu_probe_device
> pci_bus_add_device
> pci_fixup_device(pci_fixup_final) <--
> device_attach
> driver_probe_device
> really_probe
> pci_dma_configure
> acpi_dma_configure_id
> - iommu_probe_device
> - arm_smmu_probe_device
>
> This is the pci_iov_add_virtfn(). The non-SR-IOV case is similar in
> that pci_device_add() is called from pci_scan_single_device() in the
> generic enumeration path, and pci_bus_add_device() is called later,
> after all a host bridge has been enumerated.
Thanks Bjorn
Will update in v3.
One thing is the probe sequence change is the fixing result, not newly added.
iommu_bus_notifier
iommu_probe_device
__iommu_probe_device
ops = iommu_fwspec_ops(dev_iommu_fwspec_get(dev));
if (!ops)
return -ENODEV;
This calling sequence existed before but returned here since ops = NULL.
It is fixed in the commit bcb81ac6ae3c, so arm_smmu_probe_device
happens earlier and quicker.
>
> > Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
> > Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> > [kwilczynski: commit log]
> > Signed-off-by: Krzysztof Wilczyński <kwilczynski@kernel.org>
>
> You should never include somebody else's Signed-off-by below yours.
> You should only add *your own* Signed-off-by:
OK
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=v6.13#n396
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
Thanks
^ permalink raw reply [flat|nested] 15+ messages in thread