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/[email protected]/
Rebase to Linux 5.12-rc1
Change commit msg adding:
Property dma-can-stall depends on patchset
https://lore.kernel.org/linux-iommu/[email protected]/
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
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 <[email protected]>
Signed-off-by: Zhangfei Gao <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Jean-Philippe Brucker <[email protected]>
Signed-off-by: Zhou Wang <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Jean-Philippe Brucker <[email protected]>
Signed-off-by: Zhou Wang <[email protected]>
---
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
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
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/[email protected]/
> Rebase to Linux 5.12-rc1
> Change commit msg adding:
> Property dma-can-stall depends on patchset
> https://lore.kernel.org/linux-iommu/[email protected]/
>
> 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
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
[+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 <[email protected]>
> Signed-off-by: Jean-Philippe Brucker <[email protected]>
> Signed-off-by: Zhou Wang <[email protected]>
> ---
> 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
>
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 <[email protected]>
>> Signed-off-by: Jean-Philippe Brucker <[email protected]>
>> Signed-off-by: Zhou Wang <[email protected]>
>> ---
>> 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 <[email protected]>
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
>>
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/[email protected]/
> Rebase to Linux 5.12-rc1
> Change commit msg adding:
> Property dma-can-stall depends on patchset
> https://lore.kernel.org/linux-iommu/[email protected]/
>
> 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!
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/[email protected]/
>> Rebase to Linux 5.12-rc1
>> Change commit msg adding:
>> Property dma-can-stall depends on patchset
>> https://lore.kernel.org/linux-iommu/[email protected]/
>>
>> 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.
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 <[email protected]>
> > > Signed-off-by: Jean-Philippe Brucker <[email protected]>
> > > Signed-off-by: Zhou Wang <[email protected]>
> > > ---
> > > 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 <[email protected]>
>
> 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