On Fri, May 07, 2021 at 12:49:53PM +0000, Wang Xingang wrote:
> From: Xingang Wang <wangxingang5@xxxxxxxxxx>
>
> When request ACS for PCI device in of_iommu_configure, the pci device
> has already been scanned and added with 'pci_acs_enable=0'. So the
> pci_request_acs() in current procedure does not work for enabling ACS.
> Besides, the ACS should be enabled only if there's an IOMMU in system.
> So this fix the call of pci_request_acs() and call pci_enable_acs() to
> make sure ACS is enabled for the pci_device.
For consistency:
s/of_iommu_configure/of_iommu_configure()/
s/pci device/PCI device/
s/pci_device/PCI device/
But I'm confused about what problem this fixes. On x86, I think we
*do* set pci_acs_enable=1 in this path:
start_kernel
mm_init
mem_init
pci_iommu_alloc
p->detect()
detect_intel_iommu # IOMMU_INIT_POST(detect_intel_iommu)
pci_request_acs
pci_acs_enable = 1
before enumerating any PCI devices.
But you mentioned pci_host_common_probe(), which I think is mostly
used on non-x86 architectures, and I'm guessing those arches detect
the IOMMU differently.
So my question is, can we figure out how to detect IOMMUs the same way
across all arches?
> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when
> configuring IOMMU linkage")
> Signed-off-by: Xingang Wang <wangxingang5@xxxxxxxxxx>
> ---
> drivers/iommu/of_iommu.c | 10 +++++++++-
> drivers/pci/pci.c | 2 +-
> include/linux/pci.h | 1 +
> 3 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index a9d2df001149..dc621861ae72 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -205,7 +205,6 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> .np = master_np,
> };
>
> - pci_request_acs();
> err = pci_for_each_dma_alias(to_pci_dev(dev),
> of_pci_iommu_init, &info);
> } else {
> @@ -222,6 +221,15 @@ const struct iommu_ops *of_iommu_configure(struct device *dev,
> /* The fwspec pointer changed, read it again */
> fwspec = dev_iommu_fwspec_get(dev);
> ops = fwspec->ops;
> +
> + /*
> + * If we found an IOMMU and the device is pci,
> + * make sure we enable ACS.
s/pci/PCI/ for consistency.
> + */
> + if (dev_is_pci(dev)) {
> + pci_request_acs();
> + pci_enable_acs(to_pci_dev(dev));
> + }
> }
> /*
> * If we have reason to believe the IOMMU driver missed the initial
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b717680377a9..4e4f98ee2870 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -926,7 +926,7 @@ static void pci_std_enable_acs(struct pci_dev *dev)
> * pci_enable_acs - enable ACS if hardware support it
> * @dev: the PCI device
> */
> -static void pci_enable_acs(struct pci_dev *dev)
> +void pci_enable_acs(struct pci_dev *dev)
> {
> if (!pci_acs_enable)
> goto disable_acs_redir;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index c20211e59a57..e6a8bfbc9c98 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -2223,6 +2223,7 @@ static inline struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
> }
>
> void pci_request_acs(void);
> +void pci_enable_acs(struct pci_dev *dev);
> bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
> bool pci_acs_path_enabled(struct pci_dev *start,
> struct pci_dev *end, u16 acs_flags);
> --
> 2.19.1
>
