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.2On 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
>
>
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
--
மணிவண்ணன் சதாசிவம்
© 2016 - 2026 Red Hat, Inc.