| Message ID | 20241107-pci_acs_fix-v1-1-185a2462a571@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [RESEND] iommu/of: Fix pci_request_acs() before enumerating PCI devices | expand |
On Thu, Nov 07, 2024 at 01:29:15PM +0530, Pavankumar Kondeti wrote: > From: Xingang Wang <wangxingang5@huawei.com> > > When booting with devicetree, the pci_request_acs() is called after the > enumeration and initialization of PCI devices, thus the ACS is not > enabled. And ACS should be enabled when IOMMU is detected for the > PCI host bridge, so add check for IOMMU before probe of PCI host and call > pci_request_acs() to make sure ACS will be enabled when enumerating PCI > devices. > > 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> > --- > Earlier this patch made it to linux-next but got dropped later as it > broke PCI on ARM Juno R1 board. AFAICT, this issue is never root caused, > so resending this patch to get attention again. > > https://lore.kernel.org/all/1621566204-37456-1-git-send-email-wangxingang5@huawei.com/ Please don't resend known-broken patches. Please fix them up before resending, otherwise we will just ignore this one as well as obviously we can not take such a thing (nor should you want us to.) thanks, greg k-h
On Thu, Nov 07, 2024 at 09:17:44AM +0100, Greg Kroah-Hartman wrote: > On Thu, Nov 07, 2024 at 01:29:15PM +0530, Pavankumar Kondeti wrote: > > From: Xingang Wang <wangxingang5@huawei.com> > > > > When booting with devicetree, the pci_request_acs() is called after the > > enumeration and initialization of PCI devices, thus the ACS is not > > enabled. And ACS should be enabled when IOMMU is detected for the > > PCI host bridge, so add check for IOMMU before probe of PCI host and call > > pci_request_acs() to make sure ACS will be enabled when enumerating PCI > > devices. > > > > 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> > > --- > > Earlier this patch made it to linux-next but got dropped later as it > > broke PCI on ARM Juno R1 board. AFAICT, this issue is never root caused, > > so resending this patch to get attention again. > > > > https://lore.kernel.org/all/1621566204-37456-1-git-send-email-wangxingang5@huawei.com/ > > Please don't resend known-broken patches. Please fix them up before > resending, otherwise we will just ignore this one as well as obviously > we can not take such a thing (nor should you want us to.) > Thanks Greg for taking a look. AFAICT, there is nothing wrong with the patch (It was Ackd before). The root cause of the breakage on Juno ARM board is not identified. I resent the patch to get the attention from people who reported this problem few years back. I am *hoping* that they might have fixed the issue on their side. Thanks, Pavan
On Thu, Nov 07, 2024 at 01:29:15PM +0530, Pavankumar Kondeti wrote: > From: Xingang Wang <wangxingang5@huawei.com> > > When booting with devicetree, the pci_request_acs() is called after the > enumeration and initialization of PCI devices, thus the ACS is not > enabled. And ACS should be enabled when IOMMU is detected for the > PCI host bridge, so add check for IOMMU before probe of PCI host and call > pci_request_acs() to make sure ACS will be enabled when enumerating PCI > devices. > > 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> > --- > Earlier this patch made it to linux-next but got dropped later as it > broke PCI on ARM Juno R1 board. AFAICT, this issue is never root caused, > so resending this patch to get attention again. > > https://lore.kernel.org/all/1621566204-37456-1-git-send-email-wangxingang5@huawei.com/ > > The original problem that is being fixed by this patch still exists. In > my use case, all the PCI VF(s) assigned to a VM are sharing the same > group since these functions are attached under a Multi function PCIe root port FWIW, here are the problem reports (which are buried in the thread above): https://lore.kernel.org/all/01314d70-41e6-70f9-e496-84091948701a@samsung.com/ (Marek) https://lore.kernel.org/all/CADYN=9JWU3CMLzMEcD5MSQGnaLyDRSKc5SofBFHUax6YuTRaJA@mail.gmail.com/ (Anders) Given problem reports, the fact that the patch was acked and reviewed earlier means nothing. We have to ensure that any issues are resolved before considering this patch again. > emulated by the QEMU. This patch fixes that problem. > --- > 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 e7a6a1611d19..f19db52388f5 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 dacea3fc5128..dc90f4e45dd3 100644 > --- a/drivers/pci/of.c > +++ b/drivers/pci/of.c > @@ -637,9 +637,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; > > + /* Detect IOMMU and make sure ACS will be enabled */ > + 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; > > > --- > base-commit: 59b723cd2adbac2a34fc8e12c74ae26ae45bf230 > change-id: 20241107-pci_acs_fix-2239e0fb1768 > > Best regards, > -- > Pavankumar Kondeti <quic_pkondeti@quicinc.com> >
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index e7a6a1611d19..f19db52388f5 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 dacea3fc5128..dc90f4e45dd3 100644 --- a/drivers/pci/of.c +++ b/drivers/pci/of.c @@ -637,9 +637,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; + /* Detect IOMMU and make sure ACS will be enabled */ + 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;