[v9,4/4] PCI: hisi: blacklist hip06/hip07 controllers behind SMMUv3

Message ID 20171006140450.89652-5-shameerali.kolothum.thodi@huawei.com
State New
Headers show
Series
  • iommu/smmu-v3: Workaround for hisilicon 161010801 erratum(reserve HW MSI)
Related show

Commit Message

Shameerali Kolothum Thodi Oct. 6, 2017, 2:04 p.m.
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(+)

-- 
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

Comments

Gabriele Paoloni Oct. 6, 2017, 2:27 p.m. | #1
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

> 


--
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
Zhou Wang Oct. 9, 2017, 8:32 a.m. | #2
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



--
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
Bjorn Helgaas Oct. 9, 2017, 11:54 p.m. | #3
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

--
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
Bjorn Helgaas Oct. 10, 2017, 12:15 a.m. | #4
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 :)
--
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
Shameerali Kolothum Thodi Oct. 10, 2017, 9:42 a.m. | #5
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

--
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
Lorenzo Pieralisi Oct. 10, 2017, 10:06 a.m. | #6
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

--
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
Gabriele Paoloni Oct. 10, 2017, 10:19 a.m. | #7
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

> 


[...]
--
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
Bjorn Helgaas Oct. 10, 2017, 10:51 a.m. | #8
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

--
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
Will Deacon Oct. 13, 2017, 7:22 p.m. | #9
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
--
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
Shameerali Kolothum Thodi Oct. 15, 2017, 7:46 a.m. | #10
> -----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




--
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

Patch

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);
 }