| Message ID | 20171006140450.89652-5-shameerali.kolothum.thodi@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Shameer > -----Original Message----- > From: Shameerali Kolothum Thodi > Sent: 06 October 2017 15:05 > To: lorenzo.pieralisi@arm.com; marc.zyngier@arm.com; > sudeep.holla@arm.com; will.deacon@arm.com; robin.murphy@arm.com; > joro@8bytes.org; bhelgaas@google.com; Gabriele Paoloni > Cc: John Garry; iommu@lists.linux-foundation.org; linux-arm- > kernel@lists.infradead.org; linux-acpi@vger.kernel.org; linux- > pci@vger.kernel.org; devel@acpica.org; Linuxarm; Wangzhou (B); > Guohanjun (Hanjun Guo); Shameerali Kolothum Thodi > Subject: [PATCH v9 4/4] PCI: hisi: blacklist hip06/hip07 controllers > behind SMMUv3 > > The HiSilicon erratum 161010801 describes the limitation of > HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings > for MSI transactions. > > PCIe controller on these platforms has to differentiate the MSI > payload against other DMA payload and has to modify the MSI > payload. This basically makes it difficult for this platforms to > have a SMMU translation for MSI. In order to workaround this, ARM > SMMUv3 driver requires a quirk to treat the MSI regions separately. > Such a quirk is currently missing for DT based systems and therefore > we need to blacklist the hip06/hip07 PCIe controllers. > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> For this patch Acked-by: Gabriele Paoloni <gabriele.paoloni@huawei.com> > --- > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > index a201791..6800747 100644 > --- a/drivers/pci/dwc/pcie-hisi.c > +++ b/drivers/pci/dwc/pcie-hisi.c > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device > *pdev) > struct resource *reg; > int ret; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting > PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); > if (!hisi_pcie) > return -ENOMEM; > @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct > platform_device *pdev) > struct device *dev = &pdev->dev; > struct pci_ecam_ops *ops; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting > PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > ops = (struct pci_ecam_ops *)of_device_get_match_data(dev); > return pci_host_common_probe(pdev, ops); > } > -- > 1.9.1 >
On 2017/10/6 22:04, Shameer Kolothum wrote: > The HiSilicon erratum 161010801 describes the limitation of > HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings > for MSI transactions. > > PCIe controller on these platforms has to differentiate the MSI > payload against other DMA payload and has to modify the MSI > payload. This basically makes it difficult for this platforms to > have a SMMU translation for MSI. In order to workaround this, ARM > SMMUv3 driver requires a quirk to treat the MSI regions separately. > Such a quirk is currently missing for DT based systems and therefore > we need to blacklist the hip06/hip07 PCIe controllers. > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > --- > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > index a201791..6800747 100644 > --- a/drivers/pci/dwc/pcie-hisi.c > +++ b/drivers/pci/dwc/pcie-hisi.c > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device *pdev) > struct resource *reg; > int ret; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); > if (!hisi_pcie) > return -ENOMEM; > @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct pci_ecam_ops *ops; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > ops = (struct pci_ecam_ops *)of_device_get_match_data(dev); > return pci_host_common_probe(pdev, ops); > } > Acked-by: Zhou Wang <wangzhou1@hisilicon.com> Thanks, Zhou
Please change subject line: - PCI: hisi: blacklist hip06/hip07 controllers behind SMMUv3 + PCI: hisi: Blacklist hip06/hip07 controllers behind SMMUv3 On Fri, Oct 06, 2017 at 03:04:50PM +0100, Shameer Kolothum wrote: > The HiSilicon erratum 161010801 describes the limitation of > HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings > for MSI transactions. I don't suppose there's a URL for this erratum, is there? > PCIe controller on these platforms has to differentiate the MSI > payload against other DMA payload and has to modify the MSI > payload. This basically makes it difficult for this platforms to > have a SMMU translation for MSI. In order to workaround this, ARM > SMMUv3 driver requires a quirk to treat the MSI regions separately. > Such a quirk is currently missing for DT based systems and therefore > we need to blacklist the hip06/hip07 PCIe controllers. I don't understand the DT connection here. If this is a hardware erratum, I assume the a quirk would be required whether the system uses DT, ACPI, etc. > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> I assume this will go via some non-PCI tree. If I were applying this, I would look for an ack from Zhou or Gabriele in addition to mine. > --- > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > index a201791..6800747 100644 > --- a/drivers/pci/dwc/pcie-hisi.c > +++ b/drivers/pci/dwc/pcie-hisi.c > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device *pdev) > struct resource *reg; > int ret; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { Does the presence of "iommu-map" tell you this is an SMMUv3? Could it have a different type of IOMMU? I can't tell from reading Documentation/devicetree/bindings/pci/pci-iommu.txt. Why do you care whether CONFIG_ARM_SMMU_V3 is set? Does MSI work correctly if SMMUv3 is present but not used? Is it really necessary to ignore the PCIe controller completely? Could you use the devices below it as long as you disable MSI for them? I know there are probably devices that require MSI, so maybe it's easier to just ignore everything than to respond to reports of "my device doesn't work because it requires MSI." > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); > if (!hisi_pcie) > return -ENOMEM; > @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct pci_ecam_ops *ops; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > ops = (struct pci_ecam_ops *)of_device_get_match_data(dev); > return pci_host_common_probe(pdev, ops); > } > -- > 1.9.1 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Oct 09, 2017 at 06:54:52PM -0500, Bjorn Helgaas wrote: > Please change subject line: > > - PCI: hisi: blacklist hip06/hip07 controllers behind SMMUv3 > + PCI: hisi: Blacklist hip06/hip07 controllers behind SMMUv3 > > On Fri, Oct 06, 2017 at 03:04:50PM +0100, Shameer Kolothum wrote: > > The HiSilicon erratum 161010801 describes the limitation of > > HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings > > for MSI transactions. > > I don't suppose there's a URL for this erratum, is there? > > > PCIe controller on these platforms has to differentiate the MSI > > payload against other DMA payload and has to modify the MSI > > payload. This basically makes it difficult for this platforms to > > have a SMMU translation for MSI. In order to workaround this, ARM > > SMMUv3 driver requires a quirk to treat the MSI regions separately. > > Such a quirk is currently missing for DT based systems and therefore > > we need to blacklist the hip06/hip07 PCIe controllers. > > I don't understand the DT connection here. If this is a hardware > erratum, I assume the a quirk would be required whether the system > uses DT, ACPI, etc. > > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > > I assume this will go via some non-PCI tree. If I were applying this, > I would look for an ack from Zhou or Gabriele in addition to mine. Never mind about this part; I see they already have acked it. I really need to learn to read the whole thread before responding :)
HI Bjorn, > -----Original Message----- > From: Bjorn Helgaas [mailto:helgaas@kernel.org] > Sent: Tuesday, October 10, 2017 12:55 AM > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: lorenzo.pieralisi@arm.com; marc.zyngier@arm.com; > sudeep.holla@arm.com; will.deacon@arm.com; robin.murphy@arm.com; > joro@8bytes.org; bhelgaas@google.com; Gabriele Paoloni > <gabriele.paoloni@huawei.com>; John Garry <john.garry@huawei.com>; > iommu@lists.linux-foundation.org; linux-arm-kernel@lists.infradead.org; > linux-acpi@vger.kernel.org; linux-pci@vger.kernel.org; devel@acpica.org; > Linuxarm <linuxarm@huawei.com>; Wangzhou (B) > <wangzhou1@hisilicon.com>; Guohanjun (Hanjun Guo) > <guohanjun@huawei.com> > Subject: Re: [PATCH v9 4/4] PCI: hisi: blacklist hip06/hip07 controllers behind > SMMUv3 > > Please change subject line: > > - PCI: hisi: blacklist hip06/hip07 controllers behind SMMUv3 > + PCI: hisi: Blacklist hip06/hip07 controllers behind SMMUv3 Ok. > On Fri, Oct 06, 2017 at 03:04:50PM +0100, Shameer Kolothum wrote: > > The HiSilicon erratum 161010801 describes the limitation of HiSilicon > > platforms hip06/hip07 to support the SMMUv3 mappings for MSI > > transactions. > > I don't suppose there's a URL for this erratum, is there? We don't have anything public at the moment. This is part of the internal documentation at the moment. > > PCIe controller on these platforms has to differentiate the MSI > > payload against other DMA payload and has to modify the MSI payload. > > This basically makes it difficult for this platforms to have a SMMU > > translation for MSI. In order to workaround this, ARM > > SMMUv3 driver requires a quirk to treat the MSI regions separately. > > Such a quirk is currently missing for DT based systems and therefore > > we need to blacklist the hip06/hip07 PCIe controllers. > > I don't understand the DT connection here. If this is a hardware erratum, I > assume the a quirk would be required whether the system uses DT, ACPI, > etc. Yes, this is a hardware erratum and we are almost there to add the ACPI quirk into the SMMUv3 driver for this. But we got the feedback that either we have to have the DT support or we should blacklist the affected devices so that from the SMMUv3 driver point of view it will not run into the quirk unnecessarily[1]. We attempted to provide a DT solution, but it looks like DT requires more discussions and a more generic approach than ACPI[2]. Hence we are blacklisting the PCIe for now so that we can have the ACPI support goes through. (Also we don't have the DT binding in the mainline which support SMMU on these platforms and also DT is not officially supported for these platforms). > > Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> > > I assume this will go via some non-PCI tree. If I were applying this, I would > look for an ack from Zhou or Gabriele in addition to mine. > > > --- > > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > > index a201791..6800747 100644 > > --- a/drivers/pci/dwc/pcie-hisi.c > > +++ b/drivers/pci/dwc/pcie-hisi.c > > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device > *pdev) > > struct resource *reg; > > int ret; > > > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > > + of_property_read_bool(dev->of_node, "iommu- > map")) { > > Does the presence of "iommu-map" tell you this is an SMMUv3? Could it > have a different type of IOMMU? I can't tell from reading > Documentation/devicetree/bindings/pci/pci-iommu.txt. Only if the SMMUv3 driver is loaded and iommu-map binding property present, the pcie devices will use SMMU translated iova for MSI doorbell addresses. > Why do you care whether CONFIG_ARM_SMMU_V3 is set? Does MSI work > correctly if SMMUv3 is present but not used? Yes. MSI will work if no SMMUv3 is used. > Is it really necessary to ignore the PCIe controller completely? > Could you use the devices below it as long as you disable MSI for them? I > know there are probably devices that require MSI, so maybe it's easier to > just ignore everything than to respond to reports of "my device doesn't work > because it requires MSI." We are blacklisting MSI for PCIe only if the kernel is using DT and is configured to use SMMUv3. Otherwise it is fine. And as I said above DT is not officially supported on these platforms. Thanks, Shameer 1. [PATCH v6 3/3] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801 https://www.spinics.net/lists/arm-kernel/msg602873.html 2. [PATCH v8 3/5] iommu/of: Add msi address regions reservation helper https://www.spinics.net/lists/arm-kernel/msg609431.html
On Tue, Oct 10, 2017 at 09:42:30AM +0000, Shameerali Kolothum Thodi wrote: [...] > > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > > > index a201791..6800747 100644 > > > --- a/drivers/pci/dwc/pcie-hisi.c > > > +++ b/drivers/pci/dwc/pcie-hisi.c > > > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device > > *pdev) > > > struct resource *reg; > > > int ret; > > > > > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > > > + of_property_read_bool(dev->of_node, "iommu- > > map")) { > > > > Does the presence of "iommu-map" tell you this is an SMMUv3? Could it > > have a different type of IOMMU? I can't tell from reading > > Documentation/devicetree/bindings/pci/pci-iommu.txt. > > Only if the SMMUv3 driver is loaded and iommu-map binding property present, > the pcie devices will use SMMU translated iova for MSI doorbell addresses. And the iommu-map property _actually_ points at an OF node with an SMMUv3 compatible string - the sheer fact that the SMMUv3 driver is compiled in is not sufficient IIUC. Lorenzo > > Why do you care whether CONFIG_ARM_SMMU_V3 is set? Does MSI work > > correctly if SMMUv3 is present but not used? > > Yes. MSI will work if no SMMUv3 is used. > > > Is it really necessary to ignore the PCIe controller completely? > > Could you use the devices below it as long as you disable MSI for them? I > > know there are probably devices that require MSI, so maybe it's easier to > > just ignore everything than to respond to reports of "my device doesn't work > > because it requires MSI." > > We are blacklisting MSI for PCIe only if the kernel is using DT and is configured > to use SMMUv3. Otherwise it is fine. And as I said above DT is not officially > supported on these platforms. > > Thanks, > Shameer > > 1. [PATCH v6 3/3] iommu/arm-smmu-v3:Enable ACPI based HiSilicon erratum 161010801 > https://www.spinics.net/lists/arm-kernel/msg602873.html > 2. [PATCH v8 3/5] iommu/of: Add msi address regions reservation helper > https://www.spinics.net/lists/arm-kernel/msg609431.html > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Lorenzo [...] > > > Does the presence of "iommu-map" tell you this is an SMMUv3? Could > it > > > have a different type of IOMMU? I can't tell from reading > > > Documentation/devicetree/bindings/pci/pci-iommu.txt. > > > > Only if the SMMUv3 driver is loaded and iommu-map binding property > present, > > the pcie devices will use SMMU translated iova for MSI doorbell > addresses. > > And the iommu-map property _actually_ points at an OF node with an > SMMUv3 compatible string - the sheer fact that the SMMUv3 driver > is compiled in is not sufficient IIUC. The blacklisted controller can only be present in an SoC that supports SMMUv3 only. From our view checking that SMMUv3 is compiled in and also checking that the DT contains iommu-map is sufficient to decide to blacklist the controller. Thanks Gab > > Lorenzo > [...]
On Fri, Oct 06, 2017 at 03:04:50PM +0100, Shameer Kolothum wrote: > The HiSilicon erratum 161010801 describes the limitation of > HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings > for MSI transactions. > > PCIe controller on these platforms has to differentiate the MSI > payload against other DMA payload and has to modify the MSI > payload. This basically makes it difficult for this platforms to > have a SMMU translation for MSI. In order to workaround this, ARM > SMMUv3 driver requires a quirk to treat the MSI regions separately. > Such a quirk is currently missing for DT based systems and therefore > we need to blacklist the hip06/hip07 PCIe controllers. > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > index a201791..6800747 100644 > --- a/drivers/pci/dwc/pcie-hisi.c > +++ b/drivers/pci/dwc/pcie-hisi.c > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device *pdev) > struct resource *reg; > int ret; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); > if (!hisi_pcie) > return -ENOMEM; > @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct pci_ecam_ops *ops; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > ops = (struct pci_ecam_ops *)of_device_get_match_data(dev); > return pci_host_common_probe(pdev, ops); > } > -- > 1.9.1 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 06, 2017 at 03:04:50PM +0100, Shameer Kolothum wrote: > The HiSilicon erratum 161010801 describes the limitation of > HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings > for MSI transactions. > > PCIe controller on these platforms has to differentiate the MSI > payload against other DMA payload and has to modify the MSI > payload. This basically makes it difficult for this platforms to > have a SMMU translation for MSI. In order to workaround this, ARM > SMMUv3 driver requires a quirk to treat the MSI regions separately. > Such a quirk is currently missing for DT based systems and therefore > we need to blacklist the hip06/hip07 PCIe controllers. > > Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> > --- > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > index a201791..6800747 100644 > --- a/drivers/pci/dwc/pcie-hisi.c > +++ b/drivers/pci/dwc/pcie-hisi.c > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device *pdev) > struct resource *reg; > int ret; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } > + > hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); > if (!hisi_pcie) > return -ENOMEM; > @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct pci_ecam_ops *ops; > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > + of_property_read_bool(dev->of_node, "iommu-map")) { > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); > + return -ENODEV; > + } This isn't the right way to solve this problem. I was really hoping you'd come up with a solution for DT, and I know you've been trying, so I suppose for now we'll just have to go with the ACPI workaround you have and leave DT in the balance. I'm not at all happy with that, but I don't think this patch really improves things. What I think you should do is remove the relevant smmu/iommu-map entries from the .dts files that are available for these platforms (i.e. comment them out with a description as to why). Will
> -----Original Message----- > From: Will Deacon [mailto:will.deacon@arm.com] > Sent: Friday, October 13, 2017 8:22 PM > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: lorenzo.pieralisi@arm.com; marc.zyngier@arm.com; > sudeep.holla@arm.com; robin.murphy@arm.com; joro@8bytes.org; > bhelgaas@google.com; Gabriele Paoloni <gabriele.paoloni@huawei.com>; > John Garry <john.garry@huawei.com>; iommu@lists.linux-foundation.org; > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; linux- > pci@vger.kernel.org; devel@acpica.org; Linuxarm <linuxarm@huawei.com>; > Wangzhou (B) <wangzhou1@hisilicon.com>; Guohanjun (Hanjun Guo) > <guohanjun@huawei.com> > Subject: Re: [PATCH v9 4/4] PCI: hisi: blacklist hip06/hip07 controllers behind > SMMUv3 > > On Fri, Oct 06, 2017 at 03:04:50PM +0100, Shameer Kolothum wrote: > > The HiSilicon erratum 161010801 describes the limitation of HiSilicon > > platforms hip06/hip07 to support the SMMUv3 mappings for MSI > > transactions. > > > > PCIe controller on these platforms has to differentiate the MSI > > payload against other DMA payload and has to modify the MSI payload. > > This basically makes it difficult for this platforms to have a SMMU > > translation for MSI. In order to workaround this, ARM > > SMMUv3 driver requires a quirk to treat the MSI regions separately. > > Such a quirk is currently missing for DT based systems and therefore > > we need to blacklist the hip06/hip07 PCIe controllers. > > > > Signed-off-by: Shameer Kolothum > <shameerali.kolothum.thodi@huawei.com> > > --- > > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > > index a201791..6800747 100644 > > --- a/drivers/pci/dwc/pcie-hisi.c > > +++ b/drivers/pci/dwc/pcie-hisi.c > > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device > *pdev) > > struct resource *reg; > > int ret; > > > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > > + of_property_read_bool(dev->of_node, "iommu- > map")) { > > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting > PCIe controllers behind SMMUv3\n"); > > + return -ENODEV; > > + } > > + > > hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); > > if (!hisi_pcie) > > return -ENOMEM; > > @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct > platform_device *pdev) > > struct device *dev = &pdev->dev; > > struct pci_ecam_ops *ops; > > > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > > + of_property_read_bool(dev->of_node, "iommu- > map")) { > > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting > PCIe controllers behind SMMUv3\n"); > > + return -ENODEV; > > + } > > This isn't the right way to solve this problem. I was really hoping you'd come > up with a solution for DT, and I know you've been trying, so I suppose for > now we'll just have to go with the ACPI workaround you have and leave DT in > the balance. I'm not at all happy with that, but I don't think this patch really > improves things. Yes Will, this is to get the ACPI support enabled for now. > What I think you should do is remove the relevant smmu/iommu-map > entries from the .dts files that are available for these platforms (i.e. > comment them out with a description as to why). We don't have any smmu/iommu-map entries for these platforms in the .dts files [1][2]. We are not aiming for any official DT support for these platforms. This patch is to enforce the non-support. Thanks, Shameer 1. http://elixir.free-electrons.com/linux/v4.14-rc4/source/arch/arm64/boot/dts/hisilicon/hip07.dtsi 2. http://elixir.free-electrons.com/linux/v4.14-rc4/source/arch/arm64/boot/dts/hisilicon/hip06.dtsi
On Sun, Oct 15, 2017 at 07:46:34AM +0000, Shameerali Kolothum Thodi wrote: > > > > -----Original Message----- > > From: Will Deacon [mailto:will.deacon@arm.com] > > Sent: Friday, October 13, 2017 8:22 PM > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > Cc: lorenzo.pieralisi@arm.com; marc.zyngier@arm.com; > > sudeep.holla@arm.com; robin.murphy@arm.com; joro@8bytes.org; > > bhelgaas@google.com; Gabriele Paoloni <gabriele.paoloni@huawei.com>; > > John Garry <john.garry@huawei.com>; iommu@lists.linux-foundation.org; > > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; linux- > > pci@vger.kernel.org; devel@acpica.org; Linuxarm <linuxarm@huawei.com>; > > Wangzhou (B) <wangzhou1@hisilicon.com>; Guohanjun (Hanjun Guo) > > <guohanjun@huawei.com> > > Subject: Re: [PATCH v9 4/4] PCI: hisi: blacklist hip06/hip07 controllers behind > > SMMUv3 > > > > On Fri, Oct 06, 2017 at 03:04:50PM +0100, Shameer Kolothum wrote: > > > The HiSilicon erratum 161010801 describes the limitation of HiSilicon > > > platforms hip06/hip07 to support the SMMUv3 mappings for MSI > > > transactions. > > > > > > PCIe controller on these platforms has to differentiate the MSI > > > payload against other DMA payload and has to modify the MSI payload. > > > This basically makes it difficult for this platforms to have a SMMU > > > translation for MSI. In order to workaround this, ARM > > > SMMUv3 driver requires a quirk to treat the MSI regions separately. > > > Such a quirk is currently missing for DT based systems and therefore > > > we need to blacklist the hip06/hip07 PCIe controllers. > > > > > > Signed-off-by: Shameer Kolothum > > <shameerali.kolothum.thodi@huawei.com> > > > --- > > > drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c > > > index a201791..6800747 100644 > > > --- a/drivers/pci/dwc/pcie-hisi.c > > > +++ b/drivers/pci/dwc/pcie-hisi.c > > > @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device > > *pdev) > > > struct resource *reg; > > > int ret; > > > > > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > > > + of_property_read_bool(dev->of_node, "iommu- > > map")) { > > > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting > > PCIe controllers behind SMMUv3\n"); > > > + return -ENODEV; > > > + } > > > + > > > hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); > > > if (!hisi_pcie) > > > return -ENOMEM; > > > @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct > > platform_device *pdev) > > > struct device *dev = &pdev->dev; > > > struct pci_ecam_ops *ops; > > > > > > + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && > > > + of_property_read_bool(dev->of_node, "iommu- > > map")) { > > > + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting > > PCIe controllers behind SMMUv3\n"); > > > + return -ENODEV; > > > + } > > > > This isn't the right way to solve this problem. I was really hoping you'd come > > up with a solution for DT, and I know you've been trying, so I suppose for > > now we'll just have to go with the ACPI workaround you have and leave DT in > > the balance. I'm not at all happy with that, but I don't think this patch really > > improves things. > > Yes Will, this is to get the ACPI support enabled for now. > > > What I think you should do is remove the relevant smmu/iommu-map > > entries from the .dts files that are available for these platforms (i.e. > > comment them out with a description as to why). > > We don't have any smmu/iommu-map entries for these platforms in the > .dts files [1][2]. We are not aiming for any official DT support for these platforms. > This patch is to enforce the non-support. Understood, but this has dragged on for a while and I don't think this patch is the right way to enforce things. The best approach might actually be to add the SMMU to the DTs, but commented out with a comment explaining why it's not a good idea to enable it. Will
diff --git a/drivers/pci/dwc/pcie-hisi.c b/drivers/pci/dwc/pcie-hisi.c index a201791..6800747 100644 --- a/drivers/pci/dwc/pcie-hisi.c +++ b/drivers/pci/dwc/pcie-hisi.c @@ -270,6 +270,12 @@ static int hisi_pcie_probe(struct platform_device *pdev) struct resource *reg; int ret; + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && + of_property_read_bool(dev->of_node, "iommu-map")) { + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); + return -ENODEV; + } + hisi_pcie = devm_kzalloc(dev, sizeof(*hisi_pcie), GFP_KERNEL); if (!hisi_pcie) return -ENOMEM; @@ -340,6 +346,12 @@ static int hisi_pcie_almost_ecam_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct pci_ecam_ops *ops; + if ((IS_BUILTIN(CONFIG_ARM_SMMU_V3)) && + of_property_read_bool(dev->of_node, "iommu-map")) { + dev_warn(dev, "HiSilicon erratum 161010801: blacklisting PCIe controllers behind SMMUv3\n"); + return -ENODEV; + } + ops = (struct pci_ecam_ops *)of_device_get_match_data(dev); return pci_host_common_probe(pdev, ops); }
The HiSilicon erratum 161010801 describes the limitation of HiSilicon platforms hip06/hip07 to support the SMMUv3 mappings for MSI transactions. PCIe controller on these platforms has to differentiate the MSI payload against other DMA payload and has to modify the MSI payload. This basically makes it difficult for this platforms to have a SMMU translation for MSI. In order to workaround this, ARM SMMUv3 driver requires a quirk to treat the MSI regions separately. Such a quirk is currently missing for DT based systems and therefore we need to blacklist the hip06/hip07 PCIe controllers. Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> --- drivers/pci/dwc/pcie-hisi.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)