LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip
@ 2021-07-13  2:54 Zhangfei Gao
  2021-07-13  2:54 ` [PATCH v5 1/3] PCI: PASID can be enabled without TLP prefix Zhangfei Gao
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Zhangfei Gao @ 2021-07-13  2:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou
  Cc: linux-pci, linux-kernel, 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.

v5:
no change, base on 5.14-rc1

v4: 
Applied to Linux 5.13-rc2, and build successfully with only these three patches.

v3:
https://lore.kernel.org/linux-pci/1615258837-12189-1-git-send-email-zhangfei.gao@linaro.org/
Rebase to Linux 5.12-rc1
Change commit msg adding:
Property dma-can-stall depends on patchset
https://lore.kernel.org/linux-iommu/20210302092644.2553014-1-jean-philippe@linaro.org/

By the way the patchset can directly applied on 5.12-rc1 and build successfully though
without the dependent patchset.

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 chips
  PCI: Set dma-can-stall for HiSilicon chips

 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 1/3] PCI: PASID can be enabled without TLP prefix
  2021-07-13  2:54 [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
@ 2021-07-13  2:54 ` Zhangfei Gao
  2021-07-13  2:54 ` [PATCH v5 2/3] PCI: Add a quirk to set pasid_no_tlp for HiSilicon chips Zhangfei Gao
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Zhangfei Gao @ 2021-07-13  2:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou
  Cc: linux-pci, linux-kernel, 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 6d7d649..c967ad6 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -376,7 +376,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 540b377..28165dc 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 2/3] PCI: Add a quirk to set pasid_no_tlp for HiSilicon chips
  2021-07-13  2:54 [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
  2021-07-13  2:54 ` [PATCH v5 1/3] PCI: PASID can be enabled without TLP prefix Zhangfei Gao
@ 2021-07-13  2:54 ` Zhangfei Gao
  2021-07-13  2:54 ` [PATCH v5 3/3] PCI: Set dma-can-stall " Zhangfei Gao
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Zhangfei Gao @ 2021-07-13  2:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou
  Cc: linux-pci, linux-kernel, 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 6d74386..5d46ac6 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1821,6 +1821,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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5 3/3] PCI: Set dma-can-stall for HiSilicon chips
  2021-07-13  2:54 [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
  2021-07-13  2:54 ` [PATCH v5 1/3] PCI: PASID can be enabled without TLP prefix Zhangfei Gao
  2021-07-13  2:54 ` [PATCH v5 2/3] PCI: Add a quirk to set pasid_no_tlp for HiSilicon chips Zhangfei Gao
@ 2021-07-13  2:54 ` Zhangfei Gao
  2021-08-26 18:26   ` Bjorn Helgaas
  2021-07-27  6:47 ` [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Zhangfei Gao @ 2021-07-13  2:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou
  Cc: linux-pci, linux-kernel, 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>
---
 drivers/pci/quirks.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 5d46ac6..03b0f98 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1823,10 +1823,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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip
  2021-07-13  2:54 [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
                   ` (2 preceding siblings ...)
  2021-07-13  2:54 ` [PATCH v5 3/3] PCI: Set dma-can-stall " Zhangfei Gao
@ 2021-07-27  6:47 ` Zhangfei Gao
  2021-08-23  1:02   ` Zhangfei Gao
  2021-08-04  1:33 ` Zhou Wang
  2021-08-26 19:30 ` Bjorn Helgaas
  5 siblings, 1 reply; 12+ messages in thread
From: Zhangfei Gao @ 2021-07-27  6:47 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou, Bjorn Helgaas
  Cc: linux-pci, linux-kernel

Hi, Bjorn

On 2021/7/13 上午10:54, Zhangfei Gao wrote:
> 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.
>
> v5:
> no change, base on 5.14-rc1

Would you mind take a look at this patchset.
We need the quirk to enable sva feature of devices on HiSilicon 
KunPeng920 and KunPeng930.

Thanks


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip
  2021-07-13  2:54 [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
                   ` (3 preceding siblings ...)
  2021-07-27  6:47 ` [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
@ 2021-08-04  1:33 ` Zhou Wang
  2021-08-26 19:30 ` Bjorn Helgaas
  5 siblings, 0 replies; 12+ messages in thread
From: Zhou Wang @ 2021-08-04  1:33 UTC (permalink / raw)
  To: Zhangfei Gao, Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann,
	jean-philippe, kenneth-lee-2012
  Cc: linux-pci, linux-kernel

On 2021/7/13 10:54, Zhangfei Gao wrote:
> 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.
> 
> v5:
> no change, base on 5.14-rc1
> 
> v4: 
> Applied to Linux 5.13-rc2, and build successfully with only these three patches.
> 
> v3:
> https://lore.kernel.org/linux-pci/1615258837-12189-1-git-send-email-zhangfei.gao@linaro.org/
> Rebase to Linux 5.12-rc1
> Change commit msg adding:
> Property dma-can-stall depends on patchset
> https://lore.kernel.org/linux-iommu/20210302092644.2553014-1-jean-philippe@linaro.org/
> 
> By the way the patchset can directly applied on 5.12-rc1 and build successfully though
> without the dependent patchset.
> 
> 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 chips
>   PCI: Set dma-can-stall for HiSilicon chips
> 
>  drivers/pci/ats.c    |  2 +-
>  drivers/pci/quirks.c | 27 +++++++++++++++++++++++++++
>  include/linux/pci.h  |  1 +
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 

If there is no more comment, could we take this series in this loop?

Best,
Zhou



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip
  2021-07-27  6:47 ` [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
@ 2021-08-23  1:02   ` Zhangfei Gao
  0 siblings, 0 replies; 12+ messages in thread
From: Zhangfei Gao @ 2021-08-23  1:02 UTC (permalink / raw)
  To: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou, Bjorn Helgaas
  Cc: linux-pci, linux-kernel

Hi, Bjorn

On 2021/7/27 下午2:47, Zhangfei Gao wrote:
> Hi, Bjorn
>
> On 2021/7/13 上午10:54, Zhangfei Gao wrote:
>> 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.
>>
>> v5:
>> no change, base on 5.14-rc1
>
> Would you mind take a look at this patchset.
> We need the quirk to enable sva feature of devices on HiSilicon 
> KunPeng920 and KunPeng930.

Any comments about this patchset.
Is it possible to catch up the 5.15?

Thanks

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 3/3] PCI: Set dma-can-stall for HiSilicon chips
  2021-07-13  2:54 ` [PATCH v5 3/3] PCI: Set dma-can-stall " Zhangfei Gao
@ 2021-08-26 18:26   ` Bjorn Helgaas
  2021-08-26 19:12     ` Robin Murphy
  0 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2021-08-26 18:26 UTC (permalink / raw)
  To: Zhangfei Gao
  Cc: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou, linux-pci, linux-kernel, Will Deacon,
	Robin Murphy, Joerg Roedel

[+cc Will, Robin, Joerg, hoping for an ack]

On Tue, Jul 13, 2021 at 10:54:36AM +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>
> ---
>  drivers/pci/quirks.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 5d46ac6..03b0f98 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1823,10 +1823,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"),

"dma-can-stall" is used in arm_smmu_probe_device() to help set
master->stall_enabled.

I don't know the implications, so it'd be nice to get an ack from a
maintainer of that code.

> +		{},
> +	};
> +
>  	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
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 3/3] PCI: Set dma-can-stall for HiSilicon chips
  2021-08-26 18:26   ` Bjorn Helgaas
@ 2021-08-26 19:12     ` Robin Murphy
  2021-08-31 14:38       ` Will Deacon
  0 siblings, 1 reply; 12+ messages in thread
From: Robin Murphy @ 2021-08-26 19:12 UTC (permalink / raw)
  To: Bjorn Helgaas, Zhangfei Gao
  Cc: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou, linux-pci, linux-kernel, Will Deacon,
	Joerg Roedel

On 2021-08-26 19:26, Bjorn Helgaas wrote:
> [+cc Will, Robin, Joerg, hoping for an ack]
> 
> On Tue, Jul 13, 2021 at 10:54:36AM +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>
>> ---
>>   drivers/pci/quirks.c | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
>> index 5d46ac6..03b0f98 100644
>> --- a/drivers/pci/quirks.c
>> +++ b/drivers/pci/quirks.c
>> @@ -1823,10 +1823,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"),
> 
> "dma-can-stall" is used in arm_smmu_probe_device() to help set
> master->stall_enabled.
> 
> I don't know the implications, so it'd be nice to get an ack from a
> maintainer of that code.

If it helps,

Acked-by: Robin Murphy <robin.murphy@arm.com>

Normally stalling must not be enabled for PCI devices, since it would 
break the PCI requirement for free-flowing writes and may lead to 
deadlock. We expect PCI devices to support ATS and PRI if they want to 
be fault-tolerant, so there's no ACPI binding to describe anything else, 
even when a "PCI" device turns out to be a regular old SoC device 
dressed up as a RCiEP and normal rules don't apply.

I'm taking it on trust that stalling really is safe for all possible 
matching devices here (in general, deadlock may still be possible in the 
SoC interconnect depending on topology, hence why it's an explicit 
opt-in even for platform devices), but TBH either way I think I'd rather 
have this as a quirk in the kernel under our control, than have vendors 
attempt to play tricks with _DSD properties out in the field :)

Cheers,
Robin.

>> +		{},
>> +	};
>> +
>>   	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
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip
  2021-07-13  2:54 [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
                   ` (4 preceding siblings ...)
  2021-08-04  1:33 ` Zhou Wang
@ 2021-08-26 19:30 ` Bjorn Helgaas
  2021-08-27  1:02   ` Zhangfei Gao
  5 siblings, 1 reply; 12+ messages in thread
From: Bjorn Helgaas @ 2021-08-26 19:30 UTC (permalink / raw)
  To: Zhangfei Gao
  Cc: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou, linux-pci, linux-kernel

On Tue, Jul 13, 2021 at 10:54:33AM +0800, Zhangfei Gao wrote:
> 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.
> 
> v5:
> no change, base on 5.14-rc1
> 
> v4: 
> Applied to Linux 5.13-rc2, and build successfully with only these three patches.
> 
> v3:
> https://lore.kernel.org/linux-pci/1615258837-12189-1-git-send-email-zhangfei.gao@linaro.org/
> Rebase to Linux 5.12-rc1
> Change commit msg adding:
> Property dma-can-stall depends on patchset
> https://lore.kernel.org/linux-iommu/20210302092644.2553014-1-jean-philippe@linaro.org/
> 
> By the way the patchset can directly applied on 5.12-rc1 and build successfully though
> without the dependent patchset.
> 
> 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 chips
>   PCI: Set dma-can-stall for HiSilicon chips
> 
>  drivers/pci/ats.c    |  2 +-
>  drivers/pci/quirks.c | 27 +++++++++++++++++++++++++++
>  include/linux/pci.h  |  1 +
>  3 files changed, 29 insertions(+), 1 deletion(-)

Applied with Robin's ack to pci/iommu for v5.15, thanks!

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip
  2021-08-26 19:30 ` Bjorn Helgaas
@ 2021-08-27  1:02   ` Zhangfei Gao
  0 siblings, 0 replies; 12+ messages in thread
From: Zhangfei Gao @ 2021-08-27  1:02 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Bjorn Helgaas, Greg Kroah-Hartman, Arnd Bergmann, jean-philippe,
	kenneth-lee-2012, Wangzhou, linux-pci, linux-kernel



On 2021/8/27 上午3:30, Bjorn Helgaas wrote:
> On Tue, Jul 13, 2021 at 10:54:33AM +0800, Zhangfei Gao wrote:
>> 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.
>>
>> v5:
>> no change, base on 5.14-rc1
>>
>> v4:
>> Applied to Linux 5.13-rc2, and build successfully with only these three patches.
>>
>> v3:
>> https://lore.kernel.org/linux-pci/1615258837-12189-1-git-send-email-zhangfei.gao@linaro.org/
>> Rebase to Linux 5.12-rc1
>> Change commit msg adding:
>> Property dma-can-stall depends on patchset
>> https://lore.kernel.org/linux-iommu/20210302092644.2553014-1-jean-philippe@linaro.org/
>>
>> By the way the patchset can directly applied on 5.12-rc1 and build successfully though
>> without the dependent patchset.
>>
>> 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 chips
>>    PCI: Set dma-can-stall for HiSilicon chips
>>
>>   drivers/pci/ats.c    |  2 +-
>>   drivers/pci/quirks.c | 27 +++++++++++++++++++++++++++
>>   include/linux/pci.h  |  1 +
>>   3 files changed, 29 insertions(+), 1 deletion(-)
> Applied with Robin's ack to pci/iommu for v5.15, thanks!
Great, thanks  Bjorn and Robin.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5 3/3] PCI: Set dma-can-stall for HiSilicon chips
  2021-08-26 19:12     ` Robin Murphy
@ 2021-08-31 14:38       ` Will Deacon
  0 siblings, 0 replies; 12+ messages in thread
From: Will Deacon @ 2021-08-31 14:38 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Bjorn Helgaas, Zhangfei Gao, Bjorn Helgaas, Greg Kroah-Hartman,
	Arnd Bergmann, jean-philippe, kenneth-lee-2012, Wangzhou,
	linux-pci, linux-kernel, Joerg Roedel

On Thu, Aug 26, 2021 at 08:12:12PM +0100, Robin Murphy wrote:
> On 2021-08-26 19:26, Bjorn Helgaas wrote:
> > [+cc Will, Robin, Joerg, hoping for an ack]
> > 
> > On Tue, Jul 13, 2021 at 10:54:36AM +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>
> > > ---
> > >   drivers/pci/quirks.c | 13 +++++++++++++
> > >   1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > index 5d46ac6..03b0f98 100644
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -1823,10 +1823,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"),
> > 
> > "dma-can-stall" is used in arm_smmu_probe_device() to help set
> > master->stall_enabled.
> > 
> > I don't know the implications, so it'd be nice to get an ack from a
> > maintainer of that code.
> 
> If it helps,
> 
> Acked-by: Robin Murphy <robin.murphy@arm.com>
> 
> Normally stalling must not be enabled for PCI devices, since it would break
> the PCI requirement for free-flowing writes and may lead to deadlock. We
> expect PCI devices to support ATS and PRI if they want to be fault-tolerant,
> so there's no ACPI binding to describe anything else, even when a "PCI"
> device turns out to be a regular old SoC device dressed up as a RCiEP and
> normal rules don't apply.
> 
> I'm taking it on trust that stalling really is safe for all possible
> matching devices here (in general, deadlock may still be possible in the SoC
> interconnect depending on topology, hence why it's an explicit opt-in even
> for platform devices), but TBH either way I think I'd rather have this as a
> quirk in the kernel under our control, than have vendors attempt to play
> tricks with _DSD properties out in the field :)

I think a comment next to the quirk echoing the commit message and your
first paragraph above would be really helpful here.

Will

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-08-31 14:38 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13  2:54 [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
2021-07-13  2:54 ` [PATCH v5 1/3] PCI: PASID can be enabled without TLP prefix Zhangfei Gao
2021-07-13  2:54 ` [PATCH v5 2/3] PCI: Add a quirk to set pasid_no_tlp for HiSilicon chips Zhangfei Gao
2021-07-13  2:54 ` [PATCH v5 3/3] PCI: Set dma-can-stall " Zhangfei Gao
2021-08-26 18:26   ` Bjorn Helgaas
2021-08-26 19:12     ` Robin Murphy
2021-08-31 14:38       ` Will Deacon
2021-07-27  6:47 ` [PATCH v5 0/3] PCI: Add a quirk to enable SVA for HiSilicon chip Zhangfei Gao
2021-08-23  1:02   ` Zhangfei Gao
2021-08-04  1:33 ` Zhou Wang
2021-08-26 19:30 ` Bjorn Helgaas
2021-08-27  1:02   ` Zhangfei Gao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).