[PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series fixes the long standing issue with ACS in DT platforms. There are
two fixes in this series, both fixing independent issues on their own, but both
are needed to properly enable ACS on DT platforms (well, patch 1 is only needed
for Juno board, but that was a blocker for patch 2, more below...).

Issue(s) background
===================

Back in 2024, Xingang Wang first noted a failure in attaching the HiSilicon SEC
device to QEMU ARM64 pci-root-port device [1]. He then tracked down the issue to
ACS not being enabled for the QEMU Root Port device and he proposed a patch to
fix it [2].

Once the patch got applied, people reported PCIe issues with linux-next on the
ARM Juno Development boards, where they saw failure in enumerating the endpoint
devices [3][4]. So soon, the patch got dropped, but the actual issue with the
ARM Juno boards was left behind.

Fast forward to 2024, Pavan resubmitted the same fix [5] for his own usecase,
hoping that someone in the community would fix the issue with ARM Juno boards.
But the patch was rightly rejected, as a patch that was known to cause issues
should not be merged to the kernel. But again, no one investigated the Juno
issue and it was left behind again.

Now it ended up in my plate and I managed to track down the issue with the help
of Naresh who got access to the Juno boards in LKFT. The Juno issue is with the
PCIe switch from Microsemi/IDT, which triggers ACS Source Validation error on
Completions received for the Configuration Read Request from a device connected
to the downstream port that has not yet captured the PCIe bus number. As per the
PCIe spec r6.0 sec 2.2.6.2, "Functions must capture the Bus and Device Numbers
supplied with all Type 0 Configuration Write Requests completed by the Function
and supply these numbers in the Bus and Device Number fields of the Requester ID
for all Requests". So during the first Configuration Read Request issued by the
switch downstream port during enumeration (for reading Vendor ID), Bus and
Device numbers will be unknown to the device. So it responds to the Read Request
with Completion having Bus and Device number as 0. The switch interprets the
Completion as an ACS Source Validation error and drops the completion, leading
to the failure in detecting the endpoint device. Though the PCIe spec r6.0, sec
6.12.1.1, states that "Completions are never affected by ACS Source Validation".
This behavior is in violation of the spec.

This issue was already found and addressed with a quirk for a different device
from Microsemi with 'commit, aa667c6408d2 ("PCI: Workaround IDT switch ACS
Source Validation erratum")'. Apparently, this issue seems to be documented in
the erratum #36 of IDT 89H32H8G3-YC, which is not publicly available.

Solution for Juno issue
=======================

To fix this issue, I've extended the quirk to the Device ID of the switch
found in Juno R2 boards. I believe the same switch is also present in Juno R1
board as well.

With Patch 1, the Juno R2 boards can now detect the endpoints even with ACS
enabled for the Switch downstream ports. Finally, I added patch 2 that properly
enables ACS for all the PCI devices on DT platforms.

It should be noted that even without patch 2 which enables ACS for the Root
Port, the Juno boards were failing since 'commit, bcb81ac6ae3c ("iommu: Get
DT/ACPI parsing into the proper probe path")' as reported in LKFT [6]. I
believe, this commit made sure pci_request_acs() gets called before the
enumeration of the switch downstream ports. The LKFT team ended up disabling
ACS using cmdline param 'pci=config_acs=000000@pci:0:0'. So I added the above
mentioned commit as a Fixes tag for patch 1.

Also, to mitigate this issue, one could enumerate all the PCIe devices in
bootloader without enabling ACS (as also noted by Robin in the LKFT thread).
This will make sure that the endpoint device has a valid bus number when it
responds to the first Configuration Read Request from the switch downstream
port. So the ACS Source Validation error doesn't get triggered.

Solution for ACS issue
======================

To fix this issue, I've kept the patch from Xingang as is (with rewording of the
patch subject/description). This patch moves the pci_request_acs() call to
devm_of_pci_bridge_init(), which gets called during the host bridge
registration. This makes sure that the 'pci_acs_enable' flag set by
pci_request_acs() is getting set before the enumeration of the Root Port device.
So now, ACS will be enabled for all ACS capable devices of DT platforms.

[1] https://lore.kernel.org/all/038397a6-57e2-b6fc-6e1c-7c03b7be9d96@huawei.com
[2] https://lore.kernel.org/all/1621566204-37456-1-git-send-email-wangxingang5@huawei.com
[3] https://lore.kernel.org/all/01314d70-41e6-70f9-e496-84091948701a@samsung.com
[4] https://lore.kernel.org/all/CADYN=9JWU3CMLzMEcD5MSQGnaLyDRSKc5SofBFHUax6YuTRaJA@mail.gmail.com
[5] https://lore.kernel.org/linux-pci/20241107-pci_acs_fix-v1-1-185a2462a571@quicinc.com
[6] https://lists.linaro.org/archives/list/lkft-triage@lists.linaro.org/message/CBYO7V3C5TGYPKCMWEMNFFMRYALCUDTK

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Manivannan Sadhasivam (1):
      PCI: Extend pci_idt_bus_quirk() for IDT switch with Device ID 0x8090

Xingang Wang (1):
      iommu/of: Call pci_request_acs() before enumerating the Root Port device

 drivers/iommu/of_iommu.c | 1 -
 drivers/pci/of.c         | 8 +++++++-
 drivers/pci/probe.c      | 2 +-
 3 files changed, 8 insertions(+), 3 deletions(-)
---
base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
change-id: 20250910-pci-acs-cb4fa3983a2c

Best regards,
-- 
Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

[PATCH 1/2] PCI: Extend pci_idt_bus_quirk() for IDT switch with Device ID 0x8090

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

If ACS is enabled, the IDT switch with Device ID 0x8090 found in ARM Juno
R2 development board incorrectly raises an ACS Source Validation error on
Completions for Config Read Requests, even though PCIe r6.0, sec 6.12.1.1,
says that Completions are never affected by ACS Source Validation.

This behavior is documented in non-public erratum 89H32H8G3-YC and there is
already a quirk available to workaround this issue.

Hence, extend the quirk for Device ID 0x8090 to make the switch functional
if ACS is enabled.

Note: The commit mentioned in the Fixes tag causes ACS to be enabled before
the enumeration of the switch downstream port. So it ended up breaking PCIe
on ARM Juno R2 board, which used to work before this commit until someone
forcefully enabled ACS with cmdline.

Cc: stable@vger.kernel.org # 6.15
Fixes: bcb81ac6ae3c ("iommu: Get DT/ACPI parsing into the proper probe path")
Closes: https://lists.linaro.org/archives/list/lkft-triage@lists.linaro.org/message/CBYO7V3C5TGYPKCMWEMNFFMRYALCUDTK
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/probe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index f41128f91ca76ab014ad669ae84a53032c7c6b6b..2320818bc8e58c61d9ada312dfbd8c0fbfbadc0c 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2500,7 +2500,7 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
 	 * ACS Source Validation errors on completions for config reads.
 	 */
 	if (bridge && bridge->vendor == PCI_VENDOR_ID_IDT &&
-	    bridge->device == 0x80b5)
+	    (bridge->device == 0x80b5 || bridge->device == 0x8090))
 		return pci_idt_bus_quirk(bus, devfn, l, timeout);
 #endif
 

-- 
2.45.2

[PATCH 2/2] iommu/of: Call pci_request_acs() before enumerating the Root Port device

[Manivannan Sadhasivam via B4 Relay]

From: Xingang Wang <wangxingang5@huawei.com>

When booting with devicetree, ACS is enabled for all ACS capable PCI
devices except the first Root Port enumerated in the system. This is due to
calling pci_request_acs() after the enumeration and initialization of the
Root Port device. But afterwards, ACS is getting enabled for the rest of
the PCI devices, since pci_request_acs() sets the 'pci_acs_enable' flag and
the PCI core uses this flag to enable ACS for the rest of the ACS capable
devices.

Ideally, pci_request_acs() should only be called if the 'iommu-map' DT
property is set for the host bridge device. Hence, call pci_request_acs()
from devm_of_pci_bridge_init() if the 'iommu-map' property is present in
the host bridge DT node. This aligns with the implementation of the ARM64
ACPI driver (drivers/acpi/arm64/iort.c) as well.

With this change, ACS will be enabled for all the PCI devices including the
first Root Port device of the DT platforms.

Cc: stable@vger.kernel.org # 5.6
Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when configuring IOMMU linkage")
Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
[mani: reworded subject, description and comment]
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/iommu/of_iommu.c | 1 -
 drivers/pci/of.c         | 8 +++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 6b989a62def20ecafd833f00a3a92ce8dca192e0..c31369924944d36a3afd3d4ff08c86fc6daf55de 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -141,7 +141,6 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
 			.np = master_np,
 		};
 
-		pci_request_acs();
 		err = pci_for_each_dma_alias(to_pci_dev(dev),
 					     of_pci_iommu_init, &info);
 		of_pci_check_device_ats(dev, master_np);
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 3579265f119845637e163d9051437c89662762f8..98c2523f898667b1618c37451d1759959d523da1 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -638,9 +638,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
 
 int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
 {
-	if (!dev->of_node)
+	struct device_node *node = dev->of_node;
+
+	if (!node)
 		return 0;
 
+	/* Enable ACS if IOMMU mapping is detected for the host bridge */
+	if (of_property_read_bool(node, "iommu-map"))
+		pci_request_acs();
+
 	bridge->swizzle_irq = pci_common_swizzle;
 	bridge->map_irq = of_irq_parse_and_map_pci;
 

-- 
2.45.2

Re: [PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[Naresh Kamboju]

On Wed, 10 Sept 2025 at 23:09, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@kernel.org> wrote:
>
> Hi,
>
> This series fixes the long standing issue with ACS in DT platforms. There are
> two fixes in this series, both fixing independent issues on their own, but both
> are needed to properly enable ACS on DT platforms (well, patch 1 is only needed
> for Juno board, but that was a blocker for patch 2, more below...).
>
> Issue(s) background
> ===================
>
> Back in 2024, Xingang Wang first noted a failure in attaching the HiSilicon SEC
> device to QEMU ARM64 pci-root-port device [1]. He then tracked down the issue to
> ACS not being enabled for the QEMU Root Port device and he proposed a patch to
> fix it [2].
>
> Once the patch got applied, people reported PCIe issues with linux-next on the
> ARM Juno Development boards, where they saw failure in enumerating the endpoint
> devices [3][4]. So soon, the patch got dropped, but the actual issue with the
> ARM Juno boards was left behind.
>
> Fast forward to 2024, Pavan resubmitted the same fix [5] for his own usecase,
> hoping that someone in the community would fix the issue with ARM Juno boards.
> But the patch was rightly rejected, as a patch that was known to cause issues
> should not be merged to the kernel. But again, no one investigated the Juno
> issue and it was left behind again.
>
> Now it ended up in my plate and I managed to track down the issue with the help
> of Naresh who got access to the Juno boards in LKFT. The Juno issue is with the
> PCIe switch from Microsemi/IDT, which triggers ACS Source Validation error on
> Completions received for the Configuration Read Request from a device connected
> to the downstream port that has not yet captured the PCIe bus number. As per the
> PCIe spec r6.0 sec 2.2.6.2, "Functions must capture the Bus and Device Numbers
> supplied with all Type 0 Configuration Write Requests completed by the Function
> and supply these numbers in the Bus and Device Number fields of the Requester ID
> for all Requests". So during the first Configuration Read Request issued by the
> switch downstream port during enumeration (for reading Vendor ID), Bus and
> Device numbers will be unknown to the device. So it responds to the Read Request
> with Completion having Bus and Device number as 0. The switch interprets the
> Completion as an ACS Source Validation error and drops the completion, leading
> to the failure in detecting the endpoint device. Though the PCIe spec r6.0, sec
> 6.12.1.1, states that "Completions are never affected by ACS Source Validation".
> This behavior is in violation of the spec.
>
> This issue was already found and addressed with a quirk for a different device
> from Microsemi with 'commit, aa667c6408d2 ("PCI: Workaround IDT switch ACS
> Source Validation erratum")'. Apparently, this issue seems to be documented in
> the erratum #36 of IDT 89H32H8G3-YC, which is not publicly available.
>
> Solution for Juno issue
> =======================
>
> To fix this issue, I've extended the quirk to the Device ID of the switch
> found in Juno R2 boards. I believe the same switch is also present in Juno R1
> board as well.
>
> With Patch 1, the Juno R2 boards can now detect the endpoints even with ACS
> enabled for the Switch downstream ports. Finally, I added patch 2 that properly
> enables ACS for all the PCI devices on DT platforms.
>
> It should be noted that even without patch 2 which enables ACS for the Root
> Port, the Juno boards were failing since 'commit, bcb81ac6ae3c ("iommu: Get
> DT/ACPI parsing into the proper probe path")' as reported in LKFT [6]. I
> believe, this commit made sure pci_request_acs() gets called before the
> enumeration of the switch downstream ports. The LKFT team ended up disabling
> ACS using cmdline param 'pci=config_acs=000000@pci:0:0'. So I added the above
> mentioned commit as a Fixes tag for patch 1.
>
> Also, to mitigate this issue, one could enumerate all the PCIe devices in
> bootloader without enabling ACS (as also noted by Robin in the LKFT thread).
> This will make sure that the endpoint device has a valid bus number when it
> responds to the first Configuration Read Request from the switch downstream
> port. So the ACS Source Validation error doesn't get triggered.
>
> Solution for ACS issue
> ======================
>
> To fix this issue, I've kept the patch from Xingang as is (with rewording of the
> patch subject/description). This patch moves the pci_request_acs() call to
> devm_of_pci_bridge_init(), which gets called during the host bridge
> registration. This makes sure that the 'pci_acs_enable' flag set by
> pci_request_acs() is getting set before the enumeration of the Root Port device.
> So now, ACS will be enabled for all ACS capable devices of DT platforms.

I have applied this patch series on top of Linux next-20250910 and
next-20250911 tags and tested.

>
> [1] https://lore.kernel.org/all/038397a6-57e2-b6fc-6e1c-7c03b7be9d96@huawei.com
> [2] https://lore.kernel.org/all/1621566204-37456-1-git-send-email-wangxingang5@huawei.com
> [3] https://lore.kernel.org/all/01314d70-41e6-70f9-e496-84091948701a@samsung.com
> [4] https://lore.kernel.org/all/CADYN=9JWU3CMLzMEcD5MSQGnaLyDRSKc5SofBFHUax6YuTRaJA@mail.gmail.com
> [5] https://lore.kernel.org/linux-pci/20241107-pci_acs_fix-v1-1-185a2462a571@quicinc.com
> [6] https://lists.linaro.org/archives/list/lkft-triage@lists.linaro.org/message/CBYO7V3C5TGYPKCMWEMNFFMRYALCUDTK
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Tested-by: Linux Kernel Functional Testing <lkft@linaro.org>
Tested-by: Naresh Kamboju <naresh.kamboju@linaro.org>

> ---
> Manivannan Sadhasivam (1):
>       PCI: Extend pci_idt_bus_quirk() for IDT switch with Device ID 0x8090
>
> Xingang Wang (1):
>       iommu/of: Call pci_request_acs() before enumerating the Root Port device
>
>  drivers/iommu/of_iommu.c | 1 -
>  drivers/pci/of.c         | 8 +++++++-
>  drivers/pci/probe.c      | 2 +-
>  3 files changed, 8 insertions(+), 3 deletions(-)
> ---
> base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
> change-id: 20250910-pci-acs-cb4fa3983a2c
>
> Best regards,
> --
> Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
>

--
Linaro LKFT

Re: [PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[Jason Gunthorpe]

On Wed, Sep 10, 2025 at 11:09:19PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> This issue was already found and addressed with a quirk for a different device
> from Microsemi with 'commit, aa667c6408d2 ("PCI: Workaround IDT switch ACS
> Source Validation erratum")'. Apparently, this issue seems to be documented in
> the erratum #36 of IDT 89H32H8G3-YC, which is not publicly available.

This is a pretty broken device! I'm not sure this fix is good enough
though.

For instance if you reset a downstream device it should loose its RID
and then the config cycles waiting for reset to complete will trigger SV
and reset will fail?

Maybe a better answer is to say these switches don't support SV and
prevent the kernel from enabling it in the first place?

Jason

Re: [PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[Manivannan Sadhasivam]

On Thu, Sep 18, 2025 at 11:11:02AM -0300, Jason Gunthorpe wrote:
> On Wed, Sep 10, 2025 at 11:09:19PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > This issue was already found and addressed with a quirk for a different device
> > from Microsemi with 'commit, aa667c6408d2 ("PCI: Workaround IDT switch ACS
> > Source Validation erratum")'. Apparently, this issue seems to be documented in
> > the erratum #36 of IDT 89H32H8G3-YC, which is not publicly available.
> 
> This is a pretty broken device! I'm not sure this fix is good enough
> though.
> 
> For instance if you reset a downstream device it should loose its RID
> and then the config cycles waiting for reset to complete will trigger SV
> and reset will fail?
> 

No. Resetting the Ethernet controller connected to the switch downstream port
doesn't fail and we could see that the reset succeeds.

Maybe the bus number was still captured by the device.

> Maybe a better answer is to say these switches don't support SV and
> prevent the kernel from enabling it in the first place?
> 

I'm not sure though, as Windows seem to be running fine just with this erratum,
without disabling SV.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[Jason Gunthorpe]

On Tue, Sep 23, 2025 at 09:07:49PM +0530, Manivannan Sadhasivam wrote:
> On Thu, Sep 18, 2025 at 11:11:02AM -0300, Jason Gunthorpe wrote:
> > On Wed, Sep 10, 2025 at 11:09:19PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > This issue was already found and addressed with a quirk for a different device
> > > from Microsemi with 'commit, aa667c6408d2 ("PCI: Workaround IDT switch ACS
> > > Source Validation erratum")'. Apparently, this issue seems to be documented in
> > > the erratum #36 of IDT 89H32H8G3-YC, which is not publicly available.
> > 
> > This is a pretty broken device! I'm not sure this fix is good enough
> > though.
> > 
> > For instance if you reset a downstream device it should loose its RID
> > and then the config cycles waiting for reset to complete will trigger SV
> > and reset will fail?
> > 
> 
> No. Resetting the Ethernet controller connected to the switch downstream port
> doesn't fail and we could see that the reset succeeds.

Reset it by up/down the PCI link?

> Maybe the bus number was still captured by the device.

Maybe, but I don't think that is spec conformat behavior.

Jason

Re: [PATCH 2/2] iommu/of: Call pci_request_acs() before enumerating the Root Port device

[Bjorn Helgaas]

On Wed, Sep 10, 2025 at 11:09:21PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> From: Xingang Wang <wangxingang5@huawei.com>
> 
> When booting with devicetree, ACS is enabled for all ACS capable PCI
> devices except the first Root Port enumerated in the system. This is due to
> calling pci_request_acs() after the enumeration and initialization of the
> Root Port device. 

I suppose you're referring to a path like below, where we *check*
pci_acs_enable during PCI enumeration, but we don't *set* it until we
add the device and look for a driver for it?

  pci_host_common_init
    devm_pci_alloc_host_bridge
      devm_of_pci_bridge_init
        pci_request_acs
          pci_acs_enable = 1                    # ++ new set here
    pci_host_probe
      pci_scan_root_bus_bridge
        pci_scan_device
          pci_init_capabilities
            pci_enable_acs
              if (pci_acs_enable)               # test here
                ...
      pci_bus_add_devices
        driver_probe_device
          pci_dma_configure
            of_dma_configure
              of_dma_configure_id
                of_iommu_configure
                  pci_request_acs
                    pci_acs_enable = 1          # -- previously set here

> But afterwards, ACS is getting enabled for the rest of the PCI
> devices, since pci_request_acs() sets the 'pci_acs_enable' flag and
> the PCI core uses this flag to enable ACS for the rest of the ACS
> capable devices.

I don't quite understand why ACS would be enabled for *any* of the
devices because we generally enumerate all of them, which includes the
pci_init_capabilities() and pci_enable_acs(), before adding and
attaching drivers to them.

But it does seem kind of dumb that we set the system-wide "enable ACS"
property in a per-device place like an individual device probe.

> Ideally, pci_request_acs() should only be called if the 'iommu-map' DT
> property is set for the host bridge device. Hence, call pci_request_acs()
> from devm_of_pci_bridge_init() if the 'iommu-map' property is present in
> the host bridge DT node. This aligns with the implementation of the ARM64
> ACPI driver (drivers/acpi/arm64/iort.c) as well.
> 
> With this change, ACS will be enabled for all the PCI devices including the
> first Root Port device of the DT platforms.
> 
> Cc: stable@vger.kernel.org # 5.6
> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when configuring IOMMU linkage")
> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
> Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> [mani: reworded subject, description and comment]
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>  drivers/iommu/of_iommu.c | 1 -
>  drivers/pci/of.c         | 8 +++++++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 6b989a62def20ecafd833f00a3a92ce8dca192e0..c31369924944d36a3afd3d4ff08c86fc6daf55de 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -141,7 +141,6 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
>  			.np = master_np,
>  		};
>  
> -		pci_request_acs();
>  		err = pci_for_each_dma_alias(to_pci_dev(dev),
>  					     of_pci_iommu_init, &info);
>  		of_pci_check_device_ats(dev, master_np);
> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> index 3579265f119845637e163d9051437c89662762f8..98c2523f898667b1618c37451d1759959d523da1 100644
> --- a/drivers/pci/of.c
> +++ b/drivers/pci/of.c
> @@ -638,9 +638,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
>  
>  int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
>  {
> -	if (!dev->of_node)
> +	struct device_node *node = dev->of_node;
> +
> +	if (!node)
>  		return 0;
>  
> +	/* Enable ACS if IOMMU mapping is detected for the host bridge */
> +	if (of_property_read_bool(node, "iommu-map"))
> +		pci_request_acs();

I'm not really convinced that the existence of 'iommu-map' in
devicetree is a clear signal that ACS should be enabled, so I'm a
little hesitant about this part.

Is it possible to boot using a devicetree with 'iommu-map', but with
the IOMMU disabled or the IOMMU driver not present?  Or other
situations where we don't need ACS?

>  	bridge->swizzle_irq = pci_common_swizzle;
>  	bridge->map_irq = of_irq_parse_and_map_pci;
>  
> 
> -- 
> 2.45.2
> 
> 

Re: [PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[Manivannan Sadhasivam]

On Tue, Sep 23, 2025 at 01:21:39PM -0300, Jason Gunthorpe wrote:
> On Tue, Sep 23, 2025 at 09:07:49PM +0530, Manivannan Sadhasivam wrote:
> > On Thu, Sep 18, 2025 at 11:11:02AM -0300, Jason Gunthorpe wrote:
> > > On Wed, Sep 10, 2025 at 11:09:19PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > > This issue was already found and addressed with a quirk for a different device
> > > > from Microsemi with 'commit, aa667c6408d2 ("PCI: Workaround IDT switch ACS
> > > > Source Validation erratum")'. Apparently, this issue seems to be documented in
> > > > the erratum #36 of IDT 89H32H8G3-YC, which is not publicly available.
> > > 
> > > This is a pretty broken device! I'm not sure this fix is good enough
> > > though.
> > > 
> > > For instance if you reset a downstream device it should loose its RID
> > > and then the config cycles waiting for reset to complete will trigger SV
> > > and reset will fail?
> > > 
> > 
> > No. Resetting the Ethernet controller connected to the switch downstream port
> > doesn't fail and we could see that the reset succeeds.
> 
> Reset it by up/down the PCI link?
> 

We did both FLR (dev/reset) and SBR (bus/reset_subordinate), both succeeds.

> > Maybe the bus number was still captured by the device.
> 
> Maybe, but I don't think that is spec conformat behavior.
> 

This device is not spec conformant in many areas tbh. But my suggestion would be
to follow the vendor suggested erratum until any reported issues.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 2/2] iommu/of: Call pci_request_acs() before enumerating the Root Port device

[Manivannan Sadhasivam]

On Tue, Sep 23, 2025 at 03:27:01PM -0500, Bjorn Helgaas wrote:
> On Wed, Sep 10, 2025 at 11:09:21PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Xingang Wang <wangxingang5@huawei.com>
> > 
> > When booting with devicetree, ACS is enabled for all ACS capable PCI
> > devices except the first Root Port enumerated in the system. This is due to
> > calling pci_request_acs() after the enumeration and initialization of the
> > Root Port device. 
> 
> I suppose you're referring to a path like below, where we *check*
> pci_acs_enable during PCI enumeration, but we don't *set* it until we
> add the device and look for a driver for it?
> 
>   pci_host_common_init
>     devm_pci_alloc_host_bridge
>       devm_of_pci_bridge_init
>         pci_request_acs
>           pci_acs_enable = 1                    # ++ new set here
>     pci_host_probe
>       pci_scan_root_bus_bridge
>         pci_scan_device
>           pci_init_capabilities
>             pci_enable_acs
>               if (pci_acs_enable)               # test here
>                 ...
>       pci_bus_add_devices
>         driver_probe_device
>           pci_dma_configure
>             of_dma_configure
>               of_dma_configure_id
>                 of_iommu_configure
>                   pci_request_acs
>                     pci_acs_enable = 1          # -- previously set here
> 

Yes!

> > But afterwards, ACS is getting enabled for the rest of the PCI
> > devices, since pci_request_acs() sets the 'pci_acs_enable' flag and
> > the PCI core uses this flag to enable ACS for the rest of the ACS
> > capable devices.
> 
> I don't quite understand why ACS would be enabled for *any* of the
> devices because we generally enumerate all of them, which includes the
> pci_init_capabilities() and pci_enable_acs(), before adding and
> attaching drivers to them.
> 
> But it does seem kind of dumb that we set the system-wide "enable ACS"
> property in a per-device place like an individual device probe.
> 

I had the same opinion when I saw the 'pci_acs_enable' flag. But I think the
intention was to enable ACS only if the controller is capable of assigning
different IOMMU groups per device. Otherwise, ACS is more or less of no use.

> > Ideally, pci_request_acs() should only be called if the 'iommu-map' DT
> > property is set for the host bridge device. Hence, call pci_request_acs()
> > from devm_of_pci_bridge_init() if the 'iommu-map' property is present in
> > the host bridge DT node. This aligns with the implementation of the ARM64
> > ACPI driver (drivers/acpi/arm64/iort.c) as well.
> > 
> > With this change, ACS will be enabled for all the PCI devices including the
> > first Root Port device of the DT platforms.
> > 
> > Cc: stable@vger.kernel.org # 5.6
> > Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when configuring IOMMU linkage")
> > Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
> > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> > [mani: reworded subject, description and comment]
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >  drivers/iommu/of_iommu.c | 1 -
> >  drivers/pci/of.c         | 8 +++++++-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > index 6b989a62def20ecafd833f00a3a92ce8dca192e0..c31369924944d36a3afd3d4ff08c86fc6daf55de 100644
> > --- a/drivers/iommu/of_iommu.c
> > +++ b/drivers/iommu/of_iommu.c
> > @@ -141,7 +141,6 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
> >  			.np = master_np,
> >  		};
> >  
> > -		pci_request_acs();
> >  		err = pci_for_each_dma_alias(to_pci_dev(dev),
> >  					     of_pci_iommu_init, &info);
> >  		of_pci_check_device_ats(dev, master_np);
> > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > index 3579265f119845637e163d9051437c89662762f8..98c2523f898667b1618c37451d1759959d523da1 100644
> > --- a/drivers/pci/of.c
> > +++ b/drivers/pci/of.c
> > @@ -638,9 +638,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
> >  
> >  int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
> >  {
> > -	if (!dev->of_node)
> > +	struct device_node *node = dev->of_node;
> > +
> > +	if (!node)
> >  		return 0;
> >  
> > +	/* Enable ACS if IOMMU mapping is detected for the host bridge */
> > +	if (of_property_read_bool(node, "iommu-map"))
> > +		pci_request_acs();
> 
> I'm not really convinced that the existence of 'iommu-map' in
> devicetree is a clear signal that ACS should be enabled, so I'm a
> little hesitant about this part.
> 
> Is it possible to boot using a devicetree with 'iommu-map', but with
> the IOMMU disabled or the IOMMU driver not present?  Or other
> situations where we don't need ACS?
> 

Certainly possible. But the issue is, we cannot reliably detect the presence of
IOMMU until the first pci_dev is created, which will be too late as
pci_acs_init() is called during pci_device_add().

This seems to be the case for ACPI platforms also.

Maybe IOMMU folks Robin/Joerg/Will could comment more.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH 0/2] PCI: Fix ACS enablement for Root Ports in DT platforms

[Jason Gunthorpe]

On Wed, Sep 24, 2025 at 01:51:23PM +0530, Manivannan Sadhasivam wrote:

> This device is not spec conformant in many areas tbh. But my
> suggestion would be to follow the vendor suggested erratum until any
> reported issues.

I mean the ethernet chip retaining it's SBR after a FLR or link up/down
is probably not a spec requirement..

Jason

Re: [PATCH 2/2] iommu/of: Call pci_request_acs() before enumerating the Root Port device

[Bjorn Helgaas]

On Wed, Sep 24, 2025 at 02:20:52PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Sep 23, 2025 at 03:27:01PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 10, 2025 at 11:09:21PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > From: Xingang Wang <wangxingang5@huawei.com>
> > > 
> > > When booting with devicetree, ACS is enabled for all ACS capable
> > > PCI devices except the first Root Port enumerated in the system.
> > > This is due to calling pci_request_acs() after the enumeration
> > > and initialization of the Root Port device. 
> > 
> > I suppose you're referring to a path like below, where we *check*
> > pci_acs_enable during PCI enumeration, but we don't *set* it until
> > we add the device and look for a driver for it?
> > 
> >   pci_host_common_init
> >     devm_pci_alloc_host_bridge
> >       devm_of_pci_bridge_init
> >         pci_request_acs
> >           pci_acs_enable = 1                    # ++ new set here
> >     pci_host_probe
> >       pci_scan_root_bus_bridge
> >         pci_scan_device
> >           pci_init_capabilities
> >             pci_enable_acs
> >               if (pci_acs_enable)               # test here
> >                 ...
> >       pci_bus_add_devices
> >         driver_probe_device
> >           pci_dma_configure
> >             of_dma_configure
> >               of_dma_configure_id
> >                 of_iommu_configure
> >                   pci_request_acs
> >                     pci_acs_enable = 1          # -- previously set here
> > 
> 
> Yes!
> 
> > > But afterwards, ACS is getting enabled for the rest of the PCI
> > > devices, since pci_request_acs() sets the 'pci_acs_enable' flag
> > > and the PCI core uses this flag to enable ACS for the rest of
> > > the ACS capable devices.
> > 
> > I don't quite understand why ACS would be enabled for *any* of the
> > devices because we generally enumerate all of them, which includes
> > the pci_init_capabilities() and pci_enable_acs(), before adding
> > and attaching drivers to them.
> > 
> > But it does seem kind of dumb that we set the system-wide "enable
> > ACS" property in a per-device place like an individual device
> > probe.
> 
> I had the same opinion when I saw the 'pci_acs_enable' flag. But I
> think the intention was to enable ACS only if the controller is
> capable of assigning different IOMMU groups per device. Otherwise,
> ACS is more or less of no use.
> 
> > > Ideally, pci_request_acs() should only be called if the
> > > 'iommu-map' DT property is set for the host bridge device.
> > > Hence, call pci_request_acs() from devm_of_pci_bridge_init() if
> > > the 'iommu-map' property is present in the host bridge DT node.
> > > This aligns with the implementation of the ARM64 ACPI driver
> > > (drivers/acpi/arm64/iort.c) as well.
> > > 
> > > With this change, ACS will be enabled for all the PCI devices
> > > including the first Root Port device of the DT platforms.
> > > 
> > > Cc: stable@vger.kernel.org # 5.6
> > > Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when configuring IOMMU linkage")
> > > Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
> > > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> > > [mani: reworded subject, description and comment]
> > > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > > ---
> > >  drivers/iommu/of_iommu.c | 1 -
> > >  drivers/pci/of.c         | 8 +++++++-
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> > > index 6b989a62def20ecafd833f00a3a92ce8dca192e0..c31369924944d36a3afd3d4ff08c86fc6daf55de 100644
> > > --- a/drivers/iommu/of_iommu.c
> > > +++ b/drivers/iommu/of_iommu.c
> > > @@ -141,7 +141,6 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
> > >  			.np = master_np,
> > >  		};
> > >  
> > > -		pci_request_acs();
> > >  		err = pci_for_each_dma_alias(to_pci_dev(dev),
> > >  					     of_pci_iommu_init, &info);
> > >  		of_pci_check_device_ats(dev, master_np);
> > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c
> > > index 3579265f119845637e163d9051437c89662762f8..98c2523f898667b1618c37451d1759959d523da1 100644
> > > --- a/drivers/pci/of.c
> > > +++ b/drivers/pci/of.c
> > > @@ -638,9 +638,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
> > >  
> > >  int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
> > >  {
> > > -	if (!dev->of_node)
> > > +	struct device_node *node = dev->of_node;
> > > +
> > > +	if (!node)
> > >  		return 0;
> > >  
> > > +	/* Enable ACS if IOMMU mapping is detected for the host bridge */
> > > +	if (of_property_read_bool(node, "iommu-map"))
> > > +		pci_request_acs();
> > 
> > I'm not really convinced that the existence of 'iommu-map' in
> > devicetree is a clear signal that ACS should be enabled, so I'm a
> > little hesitant about this part.
> > 
> > Is it possible to boot using a devicetree with 'iommu-map', but
> > with the IOMMU disabled or the IOMMU driver not present?  Or other
> > situations where we don't need ACS?
> 
> Certainly possible. But the issue is, we cannot reliably detect the
> presence of IOMMU until the first pci_dev is created, which will be
> too late as pci_acs_init() is called during pci_device_add().
> 
> This seems to be the case for ACPI platforms also.

I don't doubt that the current code doesn't detect presence or use of
IOMMU until later.  But that feels like an implementation defect
because logically the IOMMU is upstream of any PCI device that uses
it, so architecturally I would expect it to be *possible* to detect it
before PCI enumeration.

More to the point, it's not at all obvious how to infer that
'iommu-map' in the devicetree means the IOMMU will be used.

Re: [PATCH 2/2] iommu/of: Call pci_request_acs() before enumerating the Root Port device

[Robin Murphy]

On 2025-09-24 7:57 pm, Bjorn Helgaas wrote:
> On Wed, Sep 24, 2025 at 02:20:52PM +0530, Manivannan Sadhasivam wrote:
>> On Tue, Sep 23, 2025 at 03:27:01PM -0500, Bjorn Helgaas wrote:
>>> On Wed, Sep 10, 2025 at 11:09:21PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
>>>> From: Xingang Wang <wangxingang5@huawei.com>
>>>>
>>>> When booting with devicetree, ACS is enabled for all ACS capable
>>>> PCI devices except the first Root Port enumerated in the system.
>>>> This is due to calling pci_request_acs() after the enumeration
>>>> and initialization of the Root Port device.
>>>
>>> I suppose you're referring to a path like below, where we *check*
>>> pci_acs_enable during PCI enumeration, but we don't *set* it until
>>> we add the device and look for a driver for it?
>>>
>>>    pci_host_common_init
>>>      devm_pci_alloc_host_bridge
>>>        devm_of_pci_bridge_init
>>>          pci_request_acs
>>>            pci_acs_enable = 1                    # ++ new set here
>>>      pci_host_probe
>>>        pci_scan_root_bus_bridge
>>>          pci_scan_device
>>>            pci_init_capabilities
>>>              pci_enable_acs
>>>                if (pci_acs_enable)               # test here
>>>                  ...
>>>        pci_bus_add_devices
>>>          driver_probe_device
>>>            pci_dma_configure
>>>              of_dma_configure
>>>                of_dma_configure_id
>>>                  of_iommu_configure
>>>                    pci_request_acs
>>>                      pci_acs_enable = 1          # -- previously set here
>>>
>>
>> Yes!
>>
>>>> But afterwards, ACS is getting enabled for the rest of the PCI
>>>> devices, since pci_request_acs() sets the 'pci_acs_enable' flag
>>>> and the PCI core uses this flag to enable ACS for the rest of
>>>> the ACS capable devices.
>>>
>>> I don't quite understand why ACS would be enabled for *any* of the
>>> devices because we generally enumerate all of them, which includes
>>> the pci_init_capabilities() and pci_enable_acs(), before adding
>>> and attaching drivers to them.
>>>
>>> But it does seem kind of dumb that we set the system-wide "enable
>>> ACS" property in a per-device place like an individual device
>>> probe.
>>
>> I had the same opinion when I saw the 'pci_acs_enable' flag. But I
>> think the intention was to enable ACS only if the controller is
>> capable of assigning different IOMMU groups per device. Otherwise,
>> ACS is more or less of no use.
>>
>>>> Ideally, pci_request_acs() should only be called if the
>>>> 'iommu-map' DT property is set for the host bridge device.
>>>> Hence, call pci_request_acs() from devm_of_pci_bridge_init() if
>>>> the 'iommu-map' property is present in the host bridge DT node.
>>>> This aligns with the implementation of the ARM64 ACPI driver
>>>> (drivers/acpi/arm64/iort.c) as well.
>>>>
>>>> With this change, ACS will be enabled for all the PCI devices
>>>> including the first Root Port device of the DT platforms.
>>>>
>>>> Cc: stable@vger.kernel.org # 5.6
>>>> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when configuring IOMMU linkage")
>>>> Signed-off-by: Xingang Wang <wangxingang5@huawei.com>
>>>> Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
>>>> [mani: reworded subject, description and comment]
>>>> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>>>> ---
>>>>   drivers/iommu/of_iommu.c | 1 -
>>>>   drivers/pci/of.c         | 8 +++++++-
>>>>   2 files changed, 7 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
>>>> index 6b989a62def20ecafd833f00a3a92ce8dca192e0..c31369924944d36a3afd3d4ff08c86fc6daf55de 100644
>>>> --- a/drivers/iommu/of_iommu.c
>>>> +++ b/drivers/iommu/of_iommu.c
>>>> @@ -141,7 +141,6 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
>>>>   			.np = master_np,
>>>>   		};
>>>>   
>>>> -		pci_request_acs();
>>>>   		err = pci_for_each_dma_alias(to_pci_dev(dev),
>>>>   					     of_pci_iommu_init, &info);
>>>>   		of_pci_check_device_ats(dev, master_np);
>>>> diff --git a/drivers/pci/of.c b/drivers/pci/of.c
>>>> index 3579265f119845637e163d9051437c89662762f8..98c2523f898667b1618c37451d1759959d523da1 100644
>>>> --- a/drivers/pci/of.c
>>>> +++ b/drivers/pci/of.c
>>>> @@ -638,9 +638,15 @@ static int pci_parse_request_of_pci_ranges(struct device *dev,
>>>>   
>>>>   int devm_of_pci_bridge_init(struct device *dev, struct pci_host_bridge *bridge)
>>>>   {
>>>> -	if (!dev->of_node)
>>>> +	struct device_node *node = dev->of_node;
>>>> +
>>>> +	if (!node)
>>>>   		return 0;
>>>>   
>>>> +	/* Enable ACS if IOMMU mapping is detected for the host bridge */
>>>> +	if (of_property_read_bool(node, "iommu-map"))
>>>> +		pci_request_acs();
>>>
>>> I'm not really convinced that the existence of 'iommu-map' in
>>> devicetree is a clear signal that ACS should be enabled, so I'm a
>>> little hesitant about this part.
>>>
>>> Is it possible to boot using a devicetree with 'iommu-map', but
>>> with the IOMMU disabled or the IOMMU driver not present?  Or other
>>> situations where we don't need ACS?
>>
>> Certainly possible. But the issue is, we cannot reliably detect the
>> presence of IOMMU until the first pci_dev is created, which will be
>> too late as pci_acs_init() is called during pci_device_add().
>>
>> This seems to be the case for ACPI platforms also.
> 
> I don't doubt that the current code doesn't detect presence or use of
> IOMMU until later.  But that feels like an implementation defect
> because logically the IOMMU is upstream of any PCI device that uses
> it, so architecturally I would expect it to be *possible* to detect it
> before PCI enumeration.
> 
> More to the point, it's not at all obvious how to infer that
> 'iommu-map' in the devicetree means the IOMMU will be used.

Indeed, I would say the way to go down that route would be to echo what 
we do for "iommus", and defer probing the entire host controller driver 
until the targets of an "iommu-map" are either present or assumed to 
never be appearing. (And of course an ACPI equivalent for that would be 
tricky...)

However, even that isn't necessarily the full solution, as just as it's 
not really appropriate for PCI to force ACS without knowing whether an 
IOMMU is actually present to make it meaningful, it's also not strictly 
appropriate for an IOMMU driver to request ACS globally without knowing 
that it actually serves any relevant PCIe devices. Even in the ideal 
scenario, I think the point of actually knowing is still a bit too late 
for the current API design:

   pci_device_add
     pci_init_capabilities
       pci_acs_init
         pci_enable_acs
     device_add
       iommu_bus_notifier
         iommu_probe_device
           //logically, request ACS for dev here

(at the moment, iommu_probe_device() will actually end up calling into 
the same of_iommu_configure()->pci_request_acs() path, but the plan is 
still to eventually shorten and streamline that.)

I guess we might want to separate the actual ACS enablement from the 
basic capability init, or at least perhaps just call pci_enable_acs() 
again after the IOMMU notifier may have changed the policy?

Thanks,
Robin.

Re: [PATCH 2/2] iommu/of: Call pci_request_acs() before enumerating the Root Port device

[Manivannan Sadhasivam]

On Wed, Sep 24, 2025 at 08:49:26PM +0100, Robin Murphy wrote:

[...]

> > I don't doubt that the current code doesn't detect presence or use of
> > IOMMU until later.  But that feels like an implementation defect
> > because logically the IOMMU is upstream of any PCI device that uses
> > it, so architecturally I would expect it to be *possible* to detect it
> > before PCI enumeration.
> > 
> > More to the point, it's not at all obvious how to infer that
> > 'iommu-map' in the devicetree means the IOMMU will be used.
> 
> Indeed, I would say the way to go down that route would be to echo what we
> do for "iommus", and defer probing the entire host controller driver until
> the targets of an "iommu-map" are either present or assumed to never be
> appearing. (And of course an ACPI equivalent for that would be tricky...)
> 

Maybe we should just call pci_enable_acs() only when the iommu is detected for
the device? But ofc, this is not possible for non-OF platforms as they tend to
call pci_request_acs() pretty early.

> However, even that isn't necessarily the full solution, as just as it's not
> really appropriate for PCI to force ACS without knowing whether an IOMMU is
> actually present to make it meaningful, it's also not strictly appropriate
> for an IOMMU driver to request ACS globally without knowing that it actually
> serves any relevant PCIe devices. Even in the ideal scenario, I think the
> point of actually knowing is still a bit too late for the current API
> design:
> 
>   pci_device_add
>     pci_init_capabilities
>       pci_acs_init
>         pci_enable_acs
>     device_add
>       iommu_bus_notifier
>         iommu_probe_device
>           //logically, request ACS for dev here
> 
> (at the moment, iommu_probe_device() will actually end up calling into the
> same of_iommu_configure()->pci_request_acs() path, but the plan is still to
> eventually shorten and streamline that.)
> 

What is worrying me is that of_iommu_configure() is called for each PCI device,
as opposed to just one time on other platforms. But I think that's a good thing
to have as the IOMMU driver can have a per-device ACS policy, instead of a
global one.

> I guess we might want to separate the actual ACS enablement from the basic
> capability init, or at least perhaps just call pci_enable_acs() again after
> the IOMMU notifier may have changed the policy?
> 

Is this applicable to other platforms as well? Not just OF?

- Mani

-- 
மணிவண்ணன் சதாசிவம்