[PATCH v2 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip

[PATCH v2 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip

[Zhangfei Gao]

HiSilicon KunPeng920 and KunPeng930 have devices appear as PCI but are
actually on the AMBA bus. These fake PCI devices have PASID capability
though not supporting TLP.

Add a quirk to set pasid_no_tlp and dma-can-stall for these devices.

v2:
Add a new pci_dev bit: pasid_no_tlp, suggested by Bjorn 
"Apparently these devices have a PASID capability.  I think you should
add a new pci_dev bit that is specific to this idea of "PASID works
without TLP prefixes" and then change pci_enable_pasid() to look at
that bit as well as eetlp_prefix_path."
https://lore.kernel.org/linux-pci/20210112170230.GA1838341@bjorn-Precision-5520/

Zhangfei Gao (3):
  PCI: PASID can be enabled without TLP prefix
  PCI: Add a quirk to set pasid_no_tlp for HiSilicon chip
  PCI: set dma-can-stall for HiSilicon chip

 drivers/pci/ats.c    |  2 +-
 drivers/pci/quirks.c | 27 +++++++++++++++++++++++++++
 include/linux/pci.h  |  1 +
 3 files changed, 29 insertions(+), 1 deletion(-)

-- 
2.7.4

[PATCH v2 1/3] PCI: PASID can be enabled without TLP prefix

[Zhangfei Gao]

A PASID-like feature is implemented on AMBA without using TLP prefixes
and these devices have PASID capability though not supporting TLP.
Adding a pasid_no_tlp bit for "PASID works without TLP prefixes" and
pci_enable_pasid() checks pasid_no_tlp as well as eetlp_prefix_path.

Suggested-by: Bjorn Helgaas <helgaas@kernel.org>
Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
---
 drivers/pci/ats.c   | 2 +-
 include/linux/pci.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index e36d601..b67b1b1 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -386,7 +386,7 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
 	if (WARN_ON(pdev->pasid_enabled))
 		return -EBUSY;
 
-	if (!pdev->eetlp_prefix_path)
+	if (!pdev->eetlp_prefix_path && !pdev->pasid_no_tlp)
 		return -EINVAL;
 
 	if (!pasid)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f1f26f8..ac1c735 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -388,6 +388,7 @@ struct pci_dev {
 					   supported from root to here */
 	u16		l1ss;		/* L1SS Capability pointer */
 #endif
+	unsigned int	pasid_no_tlp:1;		/* PASID works without TLP Prefix */
 	unsigned int	eetlp_prefix_path:1;	/* End-to-End TLP Prefix */
 
 	pci_channel_state_t error_state;	/* Current connectivity state */
-- 
2.7.4

[PATCH v2 2/3] PCI: Add a quirk to set pasid_no_tlp for HiSilicon chip

[Zhangfei Gao]

HiSilicon KunPeng920 and KunPeng930 have devices appear as PCI but are
actually on the AMBA bus. These fake PCI devices have PASID capability
though not supporting TLP.

Add a quirk to set pasid_no_tlp for these devices.

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/quirks.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 653660e..873d27f 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1825,6 +1825,20 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_E7525_MCH,	quir
 
 DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_HUAWEI, 0x1610, PCI_CLASS_BRIDGE_PCI, 8, quirk_pcie_mch);
 
+static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
+{
+	if (pdev->revision != 0x21 && pdev->revision != 0x30)
+		return;
+
+	pdev->pasid_no_tlp = 1;
+}
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa255, quirk_huawei_pcie_sva);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa256, quirk_huawei_pcie_sva);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa258, quirk_huawei_pcie_sva);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa259, quirk_huawei_pcie_sva);
+
 /*
  * It's possible for the MSI to get corrupted if SHPC and ACPI are used
  * together on certain PXH-based systems.
-- 
2.7.4

[PATCH v2 3/3] PCI: set dma-can-stall for HiSilicon chip

[Zhangfei Gao]

HiSilicon KunPeng920 and KunPeng930 have devices appear as PCI but are
actually on the AMBA bus. These fake PCI devices can support SVA via
SMMU stall feature, by setting dma-can-stall for ACPI platforms.

Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
Property dma-can-stall depends on patchset
https://lore.kernel.org/linux-iommu/20210108145217.2254447-1-jean-philippe@linaro.org/

 drivers/pci/quirks.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 873d27f..b866cdf 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1827,10 +1827,23 @@ DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_HUAWEI, 0x1610, PCI_CLASS_BRIDGE_PCI
 
 static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
 {
+	struct property_entry properties[] = {
+		PROPERTY_ENTRY_BOOL("dma-can-stall"),
+		{},
+	};
+
 	if (pdev->revision != 0x21 && pdev->revision != 0x30)
 		return;
 
 	pdev->pasid_no_tlp = 1;
+
+	/*
+	 * Set the dma-can-stall property on ACPI platforms. Device tree
+	 * can set it directly.
+	 */
+	if (!pdev->dev.of_node &&
+	    device_add_properties(&pdev->dev, properties))
+		pci_warn(pdev, "could not add stall property");
 }
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva);
-- 
2.7.4

Re: [PATCH v2 3/3] PCI: set dma-can-stall for HiSilicon chip

[Bjorn Helgaas]

On Mon, Jan 18, 2021 at 04:58:36PM +0800, Zhangfei Gao wrote:
> HiSilicon KunPeng920 and KunPeng930 have devices appear as PCI but are
> actually on the AMBA bus. These fake PCI devices can support SVA via
> SMMU stall feature, by setting dma-can-stall for ACPI platforms.
> 
> Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
> Property dma-can-stall depends on patchset
> https://lore.kernel.org/linux-iommu/20210108145217.2254447-1-jean-philippe@linaro.org/
> 
>  drivers/pci/quirks.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 873d27f..b866cdf 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1827,10 +1827,23 @@ DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_HUAWEI, 0x1610, PCI_CLASS_BRIDGE_PCI
>  
>  static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
>  {
> +	struct property_entry properties[] = {
> +		PROPERTY_ENTRY_BOOL("dma-can-stall"),
> +		{},
> +	};
> +
>  	if (pdev->revision != 0x21 && pdev->revision != 0x30)
>  		return;
>  
>  	pdev->pasid_no_tlp = 1;
> +
> +	/*
> +	 * Set the dma-can-stall property on ACPI platforms. Device tree
> +	 * can set it directly.
> +	 */
> +	if (!pdev->dev.of_node &&
> +	    device_add_properties(&pdev->dev, properties))
> +		pci_warn(pdev, "could not add stall property");

What's the purpose of adding the "dma-can-stall" property?  I don't
see any reference to that property in the tree or in this series.  If
this is related to some other series that uses it, perhaps this part
should be moved to that series?

>  }
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva);
> -- 
> 2.7.4
> 

Re: [PATCH v2 3/3] PCI: set dma-can-stall for HiSilicon chip

[Bjorn Helgaas]

On Thu, Jan 21, 2021 at 10:57:09AM -0600, Bjorn Helgaas wrote:
> On Mon, Jan 18, 2021 at 04:58:36PM +0800, Zhangfei Gao wrote:
> > HiSilicon KunPeng920 and KunPeng930 have devices appear as PCI but are
> > actually on the AMBA bus. These fake PCI devices can support SVA via
> > SMMU stall feature, by setting dma-can-stall for ACPI platforms.
> > 
> > Signed-off-by: Zhangfei Gao <zhangfei.gao@linaro.org>
> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
> > Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> > ---
> > Property dma-can-stall depends on patchset
> > https://lore.kernel.org/linux-iommu/20210108145217.2254447-1-jean-philippe@linaro.org/
> > 
> >  drivers/pci/quirks.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 873d27f..b866cdf 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -1827,10 +1827,23 @@ DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_VENDOR_ID_HUAWEI, 0x1610, PCI_CLASS_BRIDGE_PCI
> >  
> >  static void quirk_huawei_pcie_sva(struct pci_dev *pdev)
> >  {
> > +	struct property_entry properties[] = {
> > +		PROPERTY_ENTRY_BOOL("dma-can-stall"),
> > +		{},
> > +	};
> > +
> >  	if (pdev->revision != 0x21 && pdev->revision != 0x30)
> >  		return;
> >  
> >  	pdev->pasid_no_tlp = 1;
> > +
> > +	/*
> > +	 * Set the dma-can-stall property on ACPI platforms. Device tree
> > +	 * can set it directly.
> > +	 */
> > +	if (!pdev->dev.of_node &&
> > +	    device_add_properties(&pdev->dev, properties))
> > +		pci_warn(pdev, "could not add stall property");
> 
> What's the purpose of adding the "dma-can-stall" property?  I don't
> see any reference to that property in the tree or in this series.  If
> this is related to some other series that uses it, perhaps this part
> should be moved to that series?

Sorry, I missed your note about this above!  Thanks for the pointer.

I hate having code in the tree that's useless pending some other
series, but I guess doing it this way helps avoid ordering issues
between this series and that one.

But please include the lore URL to Jean-Philippe's series in the
commit log so that if this patch is merged before Jean-Philippe's,
people at least have a hint about what's going on.

> >  }
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa250, quirk_huawei_pcie_sva);
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_HUAWEI, 0xa251, quirk_huawei_pcie_sva);
> > -- 
> > 2.7.4
> > 

Re: [PATCH v2 3/3] PCI: set dma-can-stall for HiSilicon chip

[Krzysztof Wilczyński]

Hi,

[...]
> Property dma-can-stall depends on patchset
> https://lore.kernel.org/linux-iommu/20210108145217.2254447-1-jean-philippe@linaro.org/
[...]

If you plan to post another version of this patch to include the above
link into the commit message or reference to the commit itself, as
Jean-Philippe's series can already be included in the mainline (since it
has been a while now from when this series was originally posted), then
I have a favour to ask - would you also be able to also capitalise the
subject line (so that it's consistent) and change "chip" to "chips"
since there are two you mention in the commit message.

Thank you!

Krzysztof

Re: [PATCH v2 3/3] PCI: set dma-can-stall for HiSilicon chip

[Zhangfei Gao]

Hi, Krzysztof

On 2021/3/8 上午1:54, Krzysztof Wilczyński wrote:
> Hi,
>
> [...]
>> Property dma-can-stall depends on patchset
>> https://lore.kernel.org/linux-iommu/20210108145217.2254447-1-jean-philippe@linaro.org/
> [...]
>
> If you plan to post another version of this patch to include the above
> link into the commit message or reference to the commit itself, as
> Jean-Philippe's series can already be included in the mainline (since it
> has been a while now from when this series was originally posted), then
> I have a favour to ask - would you also be able to also capitalise the
> subject line (so that it's consistent) and change "chip" to "chips"
> since there are two you mention in the commit message.
>
>
Have sent another version with the changes, thanks for the remind.
I was waiting for Jean's patchest,
https://lore.kernel.org/linux-iommu/20210302092644.2553014-1-jean-philippe@linaro.org/
Though the quirks patches can be applied and build directly on 5.12-rc1.

Thanks