From: Xingang Wang <wangxingang5@huawei.com> To: Bjorn Helgaas <helgaas@kernel.org> Cc: <robh@kernel.org>, <will@kernel.org>, <joro@8bytes.org>, <robh+dt@kernel.org>, <gregkh@linuxfoundation.org>, <iommu@lists.linux-foundation.org>, <linux-kernel@vger.kernel.org>, <linux-pci@vger.kernel.org>, <xieyingtai@huawei.com>, John Garry <john.garry@huawei.com> Subject: Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices Date: Mon, 7 Jun 2021 20:58:11 +0800 [thread overview] Message-ID: <7cd2f48a-8cb5-d290-7187-267d92e9a595@huawei.com> (raw) In-Reply-To: <20210604190430.GA2220179@bjorn-Precision-5520> On 2021/6/5 3:04, Bjorn Helgaas wrote: > [+cc John, who tested 6bf6c24720d3] > > On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang 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. > > I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3 > ("iommu/of: Request ACS from the PCI core when configuring IOMMU > linkage"). It was tested and fixed a problem, but I don't understand > how. > > 6bf6c24720d3 added the call to pci_request_acs() in > of_iommu_configure() so it currently looks like this: > > of_iommu_configure(dev, ...) > { > if (dev_is_pci(dev)) > pci_request_acs(); > > pci_request_acs() sets pci_acs_enable, which tells us to enable ACS > when enumerating PCI devices in the future. But we only call > pci_request_acs() if we already *have* a PCI device. > > So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not > all? E.g., did we call of_iommu_configure() for one PCI device before > enumerating the rest? > I test the kernel on an arm platform with qemu: qemu-system-aarch64 \ -cpu host \ -kernel arch/arm64/boot/Image \ -enable-kvm \ -m 8G \ -smp 2,sockets=2,cores=1,threads=1 \ -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3\ -initrd rootfs.cpio.gz \ -nographic \ -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000 nokaslr" \ -device pcie-root-port,port=0x1,chassis=1,id=pci.1,addr=0x8 \ -netdev user,id=hostnet0 \ -device virtio-net-pci,netdev=hostnet0,id=net0,mac=08:13:3a:5a:22:5b,bus=pci.1,addr=0x0 \ And find that the of_iommu_configure is called after the enumeration of the pcie-root-port. And this might only infect the first device, when enumerating the rest devices, the pci_acs_enable has already be enabled. But to make sure that the pci_acs_enable will always be set before all PCI devices, it would be better to set it in initialization of PCI bridges. Thanks Xingang >> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when >> configuring IOMMU linkage") >> Signed-off-by: Xingang Wang <wangxingang5@huawei.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 a9d2df001149..54a14da242cc 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 { >> diff --git a/drivers/pci/of.c b/drivers/pci/of.c >> index da5b414d585a..2313c3f848b0 100644 >> --- a/drivers/pci/of.c >> +++ b/drivers/pci/of.c >> @@ -581,9 +581,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; >> >> -- >> 2.19.1 >> > . > .
WARNING: multiple messages have this Message-ID (diff)
From: Xingang Wang <wangxingang5@huawei.com> To: Bjorn Helgaas <helgaas@kernel.org> Cc: xieyingtai@huawei.com, robh@kernel.org, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, robh+dt@kernel.org, linux-pci@vger.kernel.org, will@kernel.org Subject: Re: [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices Date: Mon, 7 Jun 2021 20:58:11 +0800 [thread overview] Message-ID: <7cd2f48a-8cb5-d290-7187-267d92e9a595@huawei.com> (raw) In-Reply-To: <20210604190430.GA2220179@bjorn-Precision-5520> On 2021/6/5 3:04, Bjorn Helgaas wrote: > [+cc John, who tested 6bf6c24720d3] > > On Fri, May 21, 2021 at 03:03:24AM +0000, Wang Xingang 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. > > I'm happy to apply this, but I'm a little puzzled about 6bf6c24720d3 > ("iommu/of: Request ACS from the PCI core when configuring IOMMU > linkage"). It was tested and fixed a problem, but I don't understand > how. > > 6bf6c24720d3 added the call to pci_request_acs() in > of_iommu_configure() so it currently looks like this: > > of_iommu_configure(dev, ...) > { > if (dev_is_pci(dev)) > pci_request_acs(); > > pci_request_acs() sets pci_acs_enable, which tells us to enable ACS > when enumerating PCI devices in the future. But we only call > pci_request_acs() if we already *have* a PCI device. > > So maybe 6bf6c24720d3 fixed a problem for *some* PCI devices, but not > all? E.g., did we call of_iommu_configure() for one PCI device before > enumerating the rest? > I test the kernel on an arm platform with qemu: qemu-system-aarch64 \ -cpu host \ -kernel arch/arm64/boot/Image \ -enable-kvm \ -m 8G \ -smp 2,sockets=2,cores=1,threads=1 \ -machine virt,kernel_irqchip=on,gic-version=3,iommu=smmuv3\ -initrd rootfs.cpio.gz \ -nographic \ -append "rdinit=init console=ttyAMA0 earlycon=pl011,0x9000000 nokaslr" \ -device pcie-root-port,port=0x1,chassis=1,id=pci.1,addr=0x8 \ -netdev user,id=hostnet0 \ -device virtio-net-pci,netdev=hostnet0,id=net0,mac=08:13:3a:5a:22:5b,bus=pci.1,addr=0x0 \ And find that the of_iommu_configure is called after the enumeration of the pcie-root-port. And this might only infect the first device, when enumerating the rest devices, the pci_acs_enable has already be enabled. But to make sure that the pci_acs_enable will always be set before all PCI devices, it would be better to set it in initialization of PCI bridges. Thanks Xingang >> Fixes: 6bf6c24720d33 ("iommu/of: Request ACS from the PCI core when >> configuring IOMMU linkage") >> Signed-off-by: Xingang Wang <wangxingang5@huawei.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 a9d2df001149..54a14da242cc 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 { >> diff --git a/drivers/pci/of.c b/drivers/pci/of.c >> index da5b414d585a..2313c3f848b0 100644 >> --- a/drivers/pci/of.c >> +++ b/drivers/pci/of.c >> @@ -581,9 +581,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; >> >> -- >> 2.19.1 >> > . > . _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
next prev parent reply other threads:[~2021-06-07 12:58 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-21 3:03 [PATCH v4] iommu/of: Fix pci_request_acs() before enumerating PCI devices Wang Xingang 2021-05-21 3:03 ` Wang Xingang 2021-05-21 19:03 ` Rob Herring 2021-05-21 19:03 ` Rob Herring 2021-05-31 11:24 ` Xingang Wang 2021-05-31 11:24 ` Xingang Wang 2021-06-04 15:36 ` Joerg Roedel 2021-06-04 15:36 ` Joerg Roedel 2021-06-07 12:43 ` Xingang Wang 2021-06-07 12:43 ` Xingang Wang 2021-06-04 19:04 ` Bjorn Helgaas 2021-06-04 19:04 ` Bjorn Helgaas 2021-06-07 12:58 ` Xingang Wang [this message] 2021-06-07 12:58 ` Xingang Wang 2021-06-08 10:23 ` John Garry 2021-06-08 10:23 ` John Garry 2021-06-28 13:20 ` Xingang Wang 2021-06-28 13:20 ` Xingang Wang 2021-08-20 19:57 ` Bjorn Helgaas 2021-08-20 19:57 ` Bjorn Helgaas 2021-09-02 14:46 ` Bjorn Helgaas 2021-09-02 14:46 ` Bjorn Helgaas [not found] ` <CGME20210901085937eucas1p2d02da65cac797706ca3a10b8a2eb8ba2@eucas1p2.samsung.com> 2021-09-01 8:59 ` Marek Szyprowski 2021-09-01 8:59 ` Marek Szyprowski 2021-09-01 8:59 ` Marek Szyprowski 2021-09-01 9:58 ` Robin Murphy 2021-09-01 9:58 ` Robin Murphy 2021-09-01 9:58 ` Robin Murphy 2021-09-02 12:51 ` Anders Roxell 2021-09-02 12:51 ` Anders Roxell 2021-09-02 12:51 ` Anders Roxell 2021-09-02 13:07 ` Robin Murphy 2021-09-02 13:07 ` Robin Murphy 2021-09-02 13:07 ` Robin Murphy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=7cd2f48a-8cb5-d290-7187-267d92e9a595@huawei.com \ --to=wangxingang5@huawei.com \ --cc=gregkh@linuxfoundation.org \ --cc=helgaas@kernel.org \ --cc=iommu@lists.linux-foundation.org \ --cc=john.garry@huawei.com \ --cc=joro@8bytes.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=robh+dt@kernel.org \ --cc=robh@kernel.org \ --cc=will@kernel.org \ --cc=xieyingtai@huawei.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.