Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices
@ 2021-06-21 10:27 Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 1/6] PCI: Use cached Device Capabilities Register Dongdong Liu
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Dongdong Liu @ 2021-06-21 10:27 UTC (permalink / raw)
  To: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco; +Cc: linux-media, netdev

10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
field size from 8 bits to 10 bits.

This patchset is to enable 10-Bit tag for PCIe EP devices (include VF) and
RP devices

V4->V5:
- Fix warning variable 'capa' is uninitialized.
- Fix warning unused variable 'pchild'.

V3->V4:
- Get the value of pcie_devcap2 in set_pcie_port_type().
- Add Reviewed-by: Christoph Hellwig <hch@lst.de> in [PATCH V4 1/6],
  [PATCH V4 3/6], [PATCH V4 4/6], [PATCH V4 5/6].
- Fix some code style.
- Rebased on v5.13-rc6.

V2->V3:
- Use cached Device Capabilities Register suggested by Christoph.
- Fix code style to avoid > 80 char lines.
- Renamve devcap2 to pcie_devcap2.

V1->V2: Fix some comments by Christoph.
- Store the devcap2 value in the pci_dev instead of reading it multiple
  times.
- Change pci_info to pci_dbg to avoid the noisy log.
- Rename ext_10bit_tag_comp_path to ext_10bit_tag.
- Fix the compile error.
- Rebased on v5.13-rc1.

Dongdong Liu (6):
  PCI: Use cached Device Capabilities Register
  PCI: Use cached Device Capabilities 2 Register
  PCI: Add 10-Bit Tag register definitions
  PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  PCI/IOV: Enable 10-Bit tag support for PCIe VF devices
  PCI: Enable 10-Bit tag support for PCIe RP devices

 drivers/media/pci/cobalt/cobalt-driver.c        |  5 +-
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |  4 +-
 drivers/pci/iov.c                               |  8 +++
 drivers/pci/pci.c                               | 14 ++---
 drivers/pci/pcie/aspm.c                         | 11 ++--
 drivers/pci/pcie/portdrv_pci.c                  | 72 +++++++++++++++++++++++++
 drivers/pci/probe.c                             | 54 ++++++++++++++-----
 drivers/pci/quirks.c                            |  3 +-
 include/linux/pci.h                             |  5 ++
 include/uapi/linux/pci_regs.h                   |  5 ++
 10 files changed, 144 insertions(+), 37 deletions(-)

--
2.7.4


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

* [PATCH V5 1/6] PCI: Use cached Device Capabilities Register
  2021-06-21 10:27 [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
@ 2021-06-21 10:27 ` Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 2/6] PCI: Use cached Device Capabilities 2 Register Dongdong Liu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Dongdong Liu @ 2021-06-21 10:27 UTC (permalink / raw)
  To: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco; +Cc: linux-media, netdev

It will make sense to store the pcie_devcap value in the pci_dev
structure instead of reading Device Capabilities Register multiple
times. The fisrt place to use pcie_devcap is in set_pcie_port_type(),
get the pcie_devcap value here, then use cached pcie_devcap in the
needed place.

Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/media/pci/cobalt/cobalt-driver.c |  5 +++--
 drivers/pci/pci.c                        |  5 +----
 drivers/pci/pcie/aspm.c                  | 11 ++++-------
 drivers/pci/probe.c                      | 11 +++--------
 drivers/pci/quirks.c                     |  3 +--
 include/linux/pci.h                      |  1 +
 6 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/media/pci/cobalt/cobalt-driver.c b/drivers/media/pci/cobalt/cobalt-driver.c
index 839503e..7ec50d7 100644
--- a/drivers/media/pci/cobalt/cobalt-driver.c
+++ b/drivers/media/pci/cobalt/cobalt-driver.c
@@ -193,11 +193,12 @@ void cobalt_pcie_status_show(struct cobalt *cobalt)
 		return;
 
 	/* Device */
-	pcie_capability_read_dword(pci_dev, PCI_EXP_DEVCAP, &capa);
 	pcie_capability_read_word(pci_dev, PCI_EXP_DEVCTL, &ctrl);
 	pcie_capability_read_word(pci_dev, PCI_EXP_DEVSTA, &stat);
 	cobalt_info("PCIe device capability 0x%08x: Max payload %d\n",
-		    capa, get_payload_size(capa & PCI_EXP_DEVCAP_PAYLOAD));
+		    pci_dev->pcie_devcap,
+		    get_payload_size(pci_dev->pcie_devcap &
+				     PCI_EXP_DEVCAP_PAYLOAD));
 	cobalt_info("PCIe device control 0x%04x: Max payload %d. Max read request %d\n",
 		    ctrl,
 		    get_payload_size((ctrl & PCI_EXP_DEVCTL_PAYLOAD) >> 5),
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index b717680..68ccd77 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -4620,13 +4620,10 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
  */
 bool pcie_has_flr(struct pci_dev *dev)
 {
-	u32 cap;
-
 	if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
 		return false;
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
-	return cap & PCI_EXP_DEVCAP_FLR;
+	return dev->pcie_devcap & PCI_EXP_DEVCAP_FLR;
 }
 EXPORT_SYMBOL_GPL(pcie_has_flr);
 
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index ac0557a..d637564 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -660,7 +660,7 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 
 	/* Get and check endpoint acceptable latencies */
 	list_for_each_entry(child, &linkbus->devices, bus_list) {
-		u32 reg32, encoding;
+		u32 encoding;
 		struct aspm_latency *acceptable =
 			&link->acceptable[PCI_FUNC(child->devfn)];
 
@@ -668,12 +668,11 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 		    pci_pcie_type(child) != PCI_EXP_TYPE_LEG_END)
 			continue;
 
-		pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
 		/* Calculate endpoint L0s acceptable latency */
-		encoding = (reg32 & PCI_EXP_DEVCAP_L0S) >> 6;
+		encoding = (child->pcie_devcap & PCI_EXP_DEVCAP_L0S) >> 6;
 		acceptable->l0s = calc_l0s_acceptable(encoding);
 		/* Calculate endpoint L1 acceptable latency */
-		encoding = (reg32 & PCI_EXP_DEVCAP_L1) >> 9;
+		encoding = (child->pcie_devcap & PCI_EXP_DEVCAP_L1) >> 9;
 		acceptable->l1 = calc_l1_acceptable(encoding);
 
 		pcie_aspm_check_latency(child);
@@ -808,7 +807,6 @@ static void free_link_state(struct pcie_link_state *link)
 static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 {
 	struct pci_dev *child;
-	u32 reg32;
 
 	/*
 	 * Some functions in a slot might not all be PCIe functions,
@@ -831,8 +829,7 @@ static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 		 * Disable ASPM for pre-1.1 PCIe device, we follow MS to use
 		 * RBER bit to determine if a function is 1.1 version device
 		 */
-		pcie_capability_read_dword(child, PCI_EXP_DEVCAP, &reg32);
-		if (!(reg32 & PCI_EXP_DEVCAP_RBER) && !aspm_force) {
+		if (!(child->pcie_devcap & PCI_EXP_DEVCAP_RBER) && !aspm_force) {
 			pci_info(child, "disabling ASPM on pre-1.1 PCIe device.  You can enable it with 'pcie_aspm=force'\n");
 			return -EINVAL;
 		}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2752046..76ddc89 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1498,8 +1498,8 @@ void set_pcie_port_type(struct pci_dev *pdev)
 	pdev->pcie_cap = pos;
 	pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &reg16);
 	pdev->pcie_flags_reg = reg16;
-	pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
-	pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
+	pci_read_config_dword(pdev, pos + PCI_EXP_DEVCAP, &pdev->pcie_devcap);
+	pdev->pcie_mpss = pdev->pcie_devcap & PCI_EXP_DEVCAP_PAYLOAD;
 
 	parent = pci_upstream_bridge(pdev);
 	if (!parent)
@@ -2009,18 +2009,13 @@ static void pci_configure_mps(struct pci_dev *dev)
 int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
 {
 	struct pci_host_bridge *host;
-	u32 cap;
 	u16 ctl;
 	int ret;
 
 	if (!pci_is_pcie(dev))
 		return 0;
 
-	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
-	if (ret)
-		return 0;
-
-	if (!(cap & PCI_EXP_DEVCAP_EXT_TAG))
+	if (!(dev->pcie_devcap & PCI_EXP_DEVCAP_EXT_TAG))
 		return 0;
 
 	ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &ctl);
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index dcb229d..b89b438 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5073,8 +5073,7 @@ static void quirk_intel_qat_vf_cap(struct pci_dev *pdev)
 		pdev->pcie_cap = pos;
 		pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, &reg16);
 		pdev->pcie_flags_reg = reg16;
-		pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, &reg16);
-		pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD;
+		pdev->pcie_mpss = pdev->pcie_devcap & PCI_EXP_DEVCAP_PAYLOAD;
 
 		pdev->cfg_size = PCI_CFG_SPACE_EXP_SIZE;
 		if (pci_read_config_dword(pdev, PCI_CFG_SPACE_SIZE, &status) !=
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2430650..0674161 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -340,6 +340,7 @@ struct pci_dev {
 	u8		rom_base_reg;	/* Config register controlling ROM */
 	u8		pin;		/* Interrupt pin this device uses */
 	u16		pcie_flags_reg;	/* Cached PCIe Capabilities Register */
+	u32		pcie_devcap;	/* Cached Device Capabilities Register */
 	unsigned long	*dma_alias_mask;/* Mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* Driver bound to this device */
-- 
2.7.4


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

* [PATCH V5 2/6] PCI: Use cached Device Capabilities 2 Register
  2021-06-21 10:27 [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 1/6] PCI: Use cached Device Capabilities Register Dongdong Liu
@ 2021-06-21 10:27 ` Dongdong Liu
  2021-06-22  6:17   ` Christoph Hellwig
  2021-06-21 10:27 ` [PATCH V5 3/6] PCI: Add 10-Bit Tag register definitions Dongdong Liu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Dongdong Liu @ 2021-06-21 10:27 UTC (permalink / raw)
  To: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco; +Cc: linux-media, netdev

It will make sense to store the pcie_devcap2 value in the pci_dev
structure instead of reading Device Capabilities 2 Register multiple
times. Get the pcie_devcap2 value set_pcie_port_type(), then use
cached pcie_devcap2 in the needed place.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c |  4 +---
 drivers/pci/pci.c                               |  9 ++++-----
 drivers/pci/probe.c                             | 10 ++++------
 include/linux/pci.h                             |  2 ++
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 1f601de..06d24c6 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -6303,7 +6303,6 @@ static int cxgb4_iov_configure(struct pci_dev *pdev, int num_vfs)
 		struct pci_dev *pbridge;
 		struct port_info *pi;
 		char name[IFNAMSIZ];
-		u32 devcap2;
 		u16 flags;
 
 		/* If we want to instantiate Virtual Functions, then our
@@ -6313,10 +6312,9 @@ static int cxgb4_iov_configure(struct pci_dev *pdev, int num_vfs)
 		 */
 		pbridge = pdev->bus->self;
 		pcie_capability_read_word(pbridge, PCI_EXP_FLAGS, &flags);
-		pcie_capability_read_dword(pbridge, PCI_EXP_DEVCAP2, &devcap2);
 
 		if ((flags & PCI_EXP_FLAGS_VERS) < 2 ||
-		    !(devcap2 & PCI_EXP_DEVCAP2_ARI)) {
+		    !(pbridge->pcie_devcap2 & PCI_EXP_DEVCAP2_ARI)) {
 			/* Our parent bridge does not support ARI so issue a
 			 * warning and skip instantiating the VFs.  They
 			 * won't be reachable.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 68ccd77..e3cd2f4 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3690,7 +3690,7 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 {
 	struct pci_bus *bus = dev->bus;
 	struct pci_dev *bridge;
-	u32 cap, ctl2;
+	u32 ctl2;
 
 	if (!pci_is_pcie(dev))
 		return -EINVAL;
@@ -3714,19 +3714,18 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 	while (bus->parent) {
 		bridge = bus->self;
 
-		pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
-
 		switch (pci_pcie_type(bridge)) {
 		/* Ensure switch ports support AtomicOp routing */
 		case PCI_EXP_TYPE_UPSTREAM:
 		case PCI_EXP_TYPE_DOWNSTREAM:
-			if (!(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
+			if (!(bridge->pcie_devcap2 &
+			      PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
 				return -EINVAL;
 			break;
 
 		/* Ensure root port supports all the sizes we care about */
 		case PCI_EXP_TYPE_ROOT_PORT:
-			if ((cap & cap_mask) != cap_mask)
+			if ((bridge->pcie_devcap2 & cap_mask) != cap_mask)
 				return -EINVAL;
 			break;
 		}
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 76ddc89..0208865 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1500,6 +1500,7 @@ void set_pcie_port_type(struct pci_dev *pdev)
 	pdev->pcie_flags_reg = reg16;
 	pci_read_config_dword(pdev, pos + PCI_EXP_DEVCAP, &pdev->pcie_devcap);
 	pdev->pcie_mpss = pdev->pcie_devcap & PCI_EXP_DEVCAP_PAYLOAD;
+	pci_read_config_dword(pdev, pos + PCI_EXP_DEVCAP2, &pdev->pcie_devcap2);
 
 	parent = pci_upstream_bridge(pdev);
 	if (!parent)
@@ -2094,7 +2095,7 @@ static void pci_configure_ltr(struct pci_dev *dev)
 #ifdef CONFIG_PCIEASPM
 	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
 	struct pci_dev *bridge;
-	u32 cap, ctl;
+	u32 ctl;
 
 	if (!pci_is_pcie(dev))
 		return;
@@ -2102,8 +2103,7 @@ static void pci_configure_ltr(struct pci_dev *dev)
 	/* Read L1 PM substate capabilities */
 	dev->l1ss = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_L1SS);
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
-	if (!(cap & PCI_EXP_DEVCAP2_LTR))
+	if (!(dev->pcie_devcap2 & PCI_EXP_DEVCAP2_LTR))
 		return;
 
 	pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl);
@@ -2143,13 +2143,11 @@ static void pci_configure_eetlp_prefix(struct pci_dev *dev)
 #ifdef CONFIG_PCI_PASID
 	struct pci_dev *bridge;
 	int pcie_type;
-	u32 cap;
 
 	if (!pci_is_pcie(dev))
 		return;
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
-	if (!(cap & PCI_EXP_DEVCAP2_EE_PREFIX))
+	if (!(dev->pcie_devcap2 & PCI_EXP_DEVCAP2_EE_PREFIX))
 		return;
 
 	pcie_type = pci_pcie_type(dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 0674161..de1fc24 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -341,6 +341,8 @@ struct pci_dev {
 	u8		pin;		/* Interrupt pin this device uses */
 	u16		pcie_flags_reg;	/* Cached PCIe Capabilities Register */
 	u32		pcie_devcap;	/* Cached Device Capabilities Register */
+	u32		pcie_devcap2;	/* Cached Device Capabilities 2
+					   Register */
 	unsigned long	*dma_alias_mask;/* Mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* Driver bound to this device */
-- 
2.7.4


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

* [PATCH V5 3/6] PCI: Add 10-Bit Tag register definitions
  2021-06-21 10:27 [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 1/6] PCI: Use cached Device Capabilities Register Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 2/6] PCI: Use cached Device Capabilities 2 Register Dongdong Liu
@ 2021-06-21 10:27 ` Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Dongdong Liu @ 2021-06-21 10:27 UTC (permalink / raw)
  To: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco; +Cc: linux-media, netdev

Add 10-Bit Tag register definitions for use in subsequen patches.
See the PCIe 5.0 spec section 7.5.3.15 and 9.3.3.2.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 include/uapi/linux/pci_regs.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e709ae8..cf1ddb8 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -648,6 +648,8 @@
 #define  PCI_EXP_DEVCAP2_ATOMIC_COMP64	0x00000100 /* 64b AtomicOp completion */
 #define  PCI_EXP_DEVCAP2_ATOMIC_COMP128	0x00000200 /* 128b AtomicOp completion */
 #define  PCI_EXP_DEVCAP2_LTR		0x00000800 /* Latency tolerance reporting */
+#define  PCI_EXP_DEVCAP2_10BIT_TAG_COMP 0x00010000 /* 10-Bit Tag Completer Supported */
+#define  PCI_EXP_DEVCAP2_10BIT_TAG_REQ  0x00020000 /* 10-Bit Tag Requester Supported */
 #define  PCI_EXP_DEVCAP2_OBFF_MASK	0x000c0000 /* OBFF support mechanism */
 #define  PCI_EXP_DEVCAP2_OBFF_MSG	0x00040000 /* New message signaling */
 #define  PCI_EXP_DEVCAP2_OBFF_WAKE	0x00080000 /* Re-use WAKE# for OBFF */
@@ -661,6 +663,7 @@
 #define  PCI_EXP_DEVCTL2_IDO_REQ_EN	0x0100	/* Allow IDO for requests */
 #define  PCI_EXP_DEVCTL2_IDO_CMP_EN	0x0200	/* Allow IDO for completions */
 #define  PCI_EXP_DEVCTL2_LTR_EN		0x0400	/* Enable LTR mechanism */
+#define  PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN 0x1000 /* 10-Bit Tag Requester Enable */
 #define  PCI_EXP_DEVCTL2_OBFF_MSGA_EN	0x2000	/* Enable OBFF Message type A */
 #define  PCI_EXP_DEVCTL2_OBFF_MSGB_EN	0x4000	/* Enable OBFF Message type B */
 #define  PCI_EXP_DEVCTL2_OBFF_WAKE_EN	0x6000	/* OBFF using WAKE# signaling */
@@ -931,6 +934,7 @@
 /* Single Root I/O Virtualization */
 #define PCI_SRIOV_CAP		0x04	/* SR-IOV Capabilities */
 #define  PCI_SRIOV_CAP_VFM	0x00000001  /* VF Migration Capable */
+#define  PCI_SRIOV_CAP_VF_10BIT_TAG_REQ	0x00000004 /* VF 10-Bit Tag Requester Supported */
 #define  PCI_SRIOV_CAP_INTR(x)	((x) >> 21) /* Interrupt Message Number */
 #define PCI_SRIOV_CTRL		0x08	/* SR-IOV Control */
 #define  PCI_SRIOV_CTRL_VFE	0x0001	/* VF Enable */
@@ -938,6 +942,7 @@
 #define  PCI_SRIOV_CTRL_INTR	0x0004	/* VF Migration Interrupt Enable */
 #define  PCI_SRIOV_CTRL_MSE	0x0008	/* VF Memory Space Enable */
 #define  PCI_SRIOV_CTRL_ARI	0x0010	/* ARI Capable Hierarchy */
+#define  PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN 0x0020 /* VF 10-Bit Tag Requester Enable */
 #define PCI_SRIOV_STATUS	0x0a	/* SR-IOV Status */
 #define  PCI_SRIOV_STATUS_VFM	0x0001	/* VF Migration Status */
 #define PCI_SRIOV_INITIAL_VF	0x0c	/* Initial VFs */
-- 
2.7.4


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

* [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-06-21 10:27 [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
                   ` (2 preceding siblings ...)
  2021-06-21 10:27 ` [PATCH V5 3/6] PCI: Add 10-Bit Tag register definitions Dongdong Liu
@ 2021-06-21 10:27 ` Dongdong Liu
  2021-07-15 17:23   ` Bjorn Helgaas
  2021-06-21 10:27 ` [PATCH V5 5/6] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 6/6] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
  5 siblings, 1 reply; 15+ messages in thread
From: Dongdong Liu @ 2021-06-21 10:27 UTC (permalink / raw)
  To: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco; +Cc: linux-media, netdev

10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
field size from 8 bits to 10 bits.

For platforms where the RC supports 10-Bit Tag Completer capability,
it is highly recommended for platform firmware or operating software
that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable
bit automatically in Endpoints with 10-Bit Tag Requester capability. This
enables the important class of 10-Bit Tag capable adapters that send
Memory Read Requests only to host memory.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/probe.c | 33 +++++++++++++++++++++++++++++++++
 include/linux/pci.h |  2 ++
 2 files changed, 35 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 0208865..33241fb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2048,6 +2048,38 @@ int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
 	return 0;
 }
 
+static void pci_configure_10bit_tags(struct pci_dev *dev)
+{
+	struct pci_dev *bridge;
+
+	if (!(dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_COMP))
+		return;
+
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
+		dev->ext_10bit_tag = 1;
+		return;
+	}
+
+	bridge = pci_upstream_bridge(dev);
+	if (bridge && bridge->ext_10bit_tag)
+		dev->ext_10bit_tag = 1;
+
+	/*
+	 * 10-Bit Tag Requester Enable in Device Control 2 Register is RsvdP
+	 * for VF.
+	 */
+	if (dev->is_virtfn)
+		return;
+
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT &&
+	    dev->ext_10bit_tag == 1 &&
+	    (dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_REQ)) {
+		pci_dbg(dev, "enabling 10-Bit Tag Requester\n");
+		pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
+					PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
+	}
+}
+
 /**
  * pcie_relaxed_ordering_enabled - Probe for PCIe relaxed ordering enable
  * @dev: PCI device to query
@@ -2184,6 +2216,7 @@ static void pci_configure_device(struct pci_dev *dev)
 {
 	pci_configure_mps(dev);
 	pci_configure_extended_tags(dev, NULL);
+	pci_configure_10bit_tags(dev);
 	pci_configure_relaxed_ordering(dev);
 	pci_configure_ltr(dev);
 	pci_configure_eetlp_prefix(dev);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index de1fc24..445d102 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -393,6 +393,8 @@ struct pci_dev {
 #endif
 	unsigned int	eetlp_prefix_path:1;	/* End-to-End TLP Prefix */
 
+	unsigned int	ext_10bit_tag:1; /* 10-Bit Tag Completer Supported
+					    from root to here */
 	pci_channel_state_t error_state;	/* Current connectivity state */
 	struct device	dev;			/* Generic device interface */
 
-- 
2.7.4


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

* [PATCH V5 5/6] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices
  2021-06-21 10:27 [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
                   ` (3 preceding siblings ...)
  2021-06-21 10:27 ` [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
@ 2021-06-21 10:27 ` Dongdong Liu
  2021-06-21 10:27 ` [PATCH V5 6/6] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
  5 siblings, 0 replies; 15+ messages in thread
From: Dongdong Liu @ 2021-06-21 10:27 UTC (permalink / raw)
  To: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco; +Cc: linux-media, netdev

Enable VF 10-Bit Tag Requester when it's upstream component support
10-bit Tag Completer.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/pci/iov.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index afc06e6..3eb4348 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -627,6 +627,10 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 
 	pci_iov_set_numvfs(dev, nr_virtfn);
 	iov->ctrl |= PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE;
+	if ((iov->cap & PCI_SRIOV_CAP_VF_10BIT_TAG_REQ) &&
+	    dev->ext_10bit_tag)
+		iov->ctrl |= PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN;
+
 	pci_cfg_access_lock(dev);
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
 	msleep(100);
@@ -643,6 +647,8 @@ static int sriov_enable(struct pci_dev *dev, int nr_virtfn)
 
 err_pcibios:
 	iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
+	if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN)
+		iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN;
 	pci_cfg_access_lock(dev);
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
 	ssleep(1);
@@ -675,6 +681,8 @@ static void sriov_disable(struct pci_dev *dev)
 
 	sriov_del_vfs(dev);
 	iov->ctrl &= ~(PCI_SRIOV_CTRL_VFE | PCI_SRIOV_CTRL_MSE);
+	if (iov->ctrl & PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN)
+		iov->ctrl &= ~PCI_SRIOV_CTRL_VF_10BIT_TAG_REQ_EN;
 	pci_cfg_access_lock(dev);
 	pci_write_config_word(dev, iov->pos + PCI_SRIOV_CTRL, iov->ctrl);
 	ssleep(1);
-- 
2.7.4


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

* [PATCH V5 6/6] PCI: Enable 10-Bit tag support for PCIe RP devices
  2021-06-21 10:27 [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
                   ` (4 preceding siblings ...)
  2021-06-21 10:27 ` [PATCH V5 5/6] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices Dongdong Liu
@ 2021-06-21 10:27 ` Dongdong Liu
  2021-06-22  6:19   ` Christoph Hellwig
  5 siblings, 1 reply; 15+ messages in thread
From: Dongdong Liu @ 2021-06-21 10:27 UTC (permalink / raw)
  To: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco; +Cc: linux-media, netdev

PCIe spec 5.0r1.0 section 2.2.6.2 implementation note, In configurations
where a Requester with 10-Bit Tag Requester capability needs to target
multiple Completers, one needs to ensure that the Requester sends 10-Bit
Tag Requests only to Completers that have 10-Bit Tag Completer capability.
So we enable 10-Bit Tag Requester for root port only when the devices
under the root port support 10-Bit Tag Completer.

Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
---
 drivers/pci/pcie/portdrv_pci.c | 72 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index c7ff1ee..0f9fb13 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -90,6 +90,75 @@ static const struct dev_pm_ops pcie_portdrv_pm_ops = {
 #define PCIE_PORTDRV_PM_OPS	NULL
 #endif /* !PM */
 
+static int pci_10bit_tag_comp_support(struct pci_dev *dev, void *data)
+{
+	u8 *support = data;
+
+	if (*support == 0)
+		return 0;
+
+	if (!pci_is_pcie(dev)) {
+		*support = 0;
+		return 0;
+	}
+
+	/*
+	 * PCIe spec 5.0r1.0 section 2.2.6.2 implementation note.
+	 * For configurations where a Requester with 10-Bit Tag Requester
+	 * capability targets Completers where some do and some do not have
+	 * 10-Bit Tag Completer capability, how the Requester determines which
+	 * NPRs include 10-Bit Tags is outside the scope of this specification.
+	 * So we do not consider hotplug scenario.
+	 */
+	if (dev->is_hotplug_bridge) {
+		*support = 0;
+		return 0;
+	}
+
+	if (!(dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_COMP)) {
+		*support = 0;
+		return 0;
+	}
+
+	return 0;
+}
+
+static void pci_configure_rp_10bit_tag(struct pci_dev *dev)
+{
+	u8 support = 1;
+
+	if (dev->subordinate == NULL)
+		return;
+
+	/* If no devices under the root port, no need to enable 10-Bit Tag. */
+	if (list_empty(&dev->subordinate->devices))
+		return;
+
+	pci_10bit_tag_comp_support(dev, &support);
+	if (!support)
+		return;
+
+	/*
+	 * PCIe spec 5.0r1.0 section 2.2.6.2 implementation note.
+	 * In configurations where a Requester with 10-Bit Tag Requester
+	 * capability needs to target multiple Completers, one needs to ensure
+	 * that the Requester sends 10-Bit Tag Requests only to Completers
+	 * that have 10-Bit Tag Completer capability. So we enable 10-Bit Tag
+	 * Requester for root port only when the devices under the root port
+	 * support 10-Bit Tag Completer.
+	 */
+	pci_walk_bus(dev->subordinate, pci_10bit_tag_comp_support, &support);
+	if (!support)
+		return;
+
+	if (!(dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_REQ))
+		return;
+
+	pci_dbg(dev, "enabling 10-Bit Tag Requester\n");
+	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
+				 PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
+}
+
 /*
  * pcie_portdrv_probe - Probe PCI-Express port devices
  * @dev: PCI-Express port device being probed
@@ -111,6 +180,9 @@ static int pcie_portdrv_probe(struct pci_dev *dev,
 	     (type != PCI_EXP_TYPE_RC_EC)))
 		return -ENODEV;
 
+	if (type == PCI_EXP_TYPE_ROOT_PORT)
+		pci_configure_rp_10bit_tag(dev);
+
 	if (type == PCI_EXP_TYPE_RC_EC)
 		pcie_link_rcec(dev);
 
-- 
2.7.4


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

* Re: [PATCH V5 2/6] PCI: Use cached Device Capabilities 2 Register
  2021-06-21 10:27 ` [PATCH V5 2/6] PCI: Use cached Device Capabilities 2 Register Dongdong Liu
@ 2021-06-22  6:17   ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2021-06-22  6:17 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH V5 6/6] PCI: Enable 10-Bit tag support for PCIe RP devices
  2021-06-21 10:27 ` [PATCH V5 6/6] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
@ 2021-06-22  6:19   ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2021-06-22  6:19 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: helgaas, hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev

On Mon, Jun 21, 2021 at 06:27:22PM +0800, Dongdong Liu wrote:
> PCIe spec 5.0r1.0 section 2.2.6.2 implementation note, In configurations
> where a Requester with 10-Bit Tag Requester capability needs to target
> multiple Completers, one needs to ensure that the Requester sends 10-Bit
> Tag Requests only to Completers that have 10-Bit Tag Completer capability.
> So we enable 10-Bit Tag Requester for root port only when the devices
> under the root port support 10-Bit Tag Completer.
> 
> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

* Re: [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-06-21 10:27 ` [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
@ 2021-07-15 17:23   ` Bjorn Helgaas
  2021-07-16 11:12     ` Dongdong Liu
  0 siblings, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2021-07-15 17:23 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev,
	Logan Gunthorpe

[+cc Logan]

On Mon, Jun 21, 2021 at 06:27:20PM +0800, Dongdong Liu wrote:
> 10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
> field size from 8 bits to 10 bits.
> 
> For platforms where the RC supports 10-Bit Tag Completer capability,
> it is highly recommended for platform firmware or operating software

Recommended by whom?  If the spec recommends it, we should provide the
citation.

> that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable
> bit automatically in Endpoints with 10-Bit Tag Requester capability. This
> enables the important class of 10-Bit Tag capable adapters that send
> Memory Read Requests only to host memory.

What is the implication for P2PDMA?  What happens if we enable 10-bit
tags for device A, and A generates Mem Read Requests to device B,
which does not support 10-bit tags?

> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/pci/probe.c | 33 +++++++++++++++++++++++++++++++++
>  include/linux/pci.h |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 0208865..33241fb 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2048,6 +2048,38 @@ int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
>  	return 0;
>  }
>  
> +static void pci_configure_10bit_tags(struct pci_dev *dev)
> +{
> +	struct pci_dev *bridge;
> +
> +	if (!(dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_COMP))
> +		return;
> +
> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
> +		dev->ext_10bit_tag = 1;
> +		return;
> +	}
> +
> +	bridge = pci_upstream_bridge(dev);
> +	if (bridge && bridge->ext_10bit_tag)
> +		dev->ext_10bit_tag = 1;
> +
> +	/*
> +	 * 10-Bit Tag Requester Enable in Device Control 2 Register is RsvdP
> +	 * for VF.
> +	 */
> +	if (dev->is_virtfn)
> +		return;
> +
> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT &&
> +	    dev->ext_10bit_tag == 1 &&
> +	    (dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_REQ)) {
> +		pci_dbg(dev, "enabling 10-Bit Tag Requester\n");
> +		pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
> +					PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
> +	}
> +}
> +
>  /**
>   * pcie_relaxed_ordering_enabled - Probe for PCIe relaxed ordering enable
>   * @dev: PCI device to query
> @@ -2184,6 +2216,7 @@ static void pci_configure_device(struct pci_dev *dev)
>  {
>  	pci_configure_mps(dev);
>  	pci_configure_extended_tags(dev, NULL);
> +	pci_configure_10bit_tags(dev);

I think 10-bit tag support should be integrated with extended (8-bit)
tag support instead of having two separate functions.

If we have "no_ext_tags" set because some device doesn't support 8-bit
tags correctly, we probably shouldn't try to enable 10-bit tags
either.

>  	pci_configure_relaxed_ordering(dev);
>  	pci_configure_ltr(dev);
>  	pci_configure_eetlp_prefix(dev);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index de1fc24..445d102 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -393,6 +393,8 @@ struct pci_dev {
>  #endif
>  	unsigned int	eetlp_prefix_path:1;	/* End-to-End TLP Prefix */
>  
> +	unsigned int	ext_10bit_tag:1; /* 10-Bit Tag Completer Supported
> +					    from root to here */
>  	pci_channel_state_t error_state;	/* Current connectivity state */
>  	struct device	dev;			/* Generic device interface */
>  
> -- 
> 2.7.4
> 

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

* Re: [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-07-15 17:23   ` Bjorn Helgaas
@ 2021-07-16 11:12     ` Dongdong Liu
  2021-07-16 14:17       ` Bjorn Helgaas
  2021-07-16 15:51       ` Logan Gunthorpe
  0 siblings, 2 replies; 15+ messages in thread
From: Dongdong Liu @ 2021-07-16 11:12 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev,
	Logan Gunthorpe

Hi Bjorn

Many thanks for your review.

On 2021/7/16 1:23, Bjorn Helgaas wrote:
> [+cc Logan]
>
> On Mon, Jun 21, 2021 at 06:27:20PM +0800, Dongdong Liu wrote:
>> 10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
>> field size from 8 bits to 10 bits.
>>
>> For platforms where the RC supports 10-Bit Tag Completer capability,
>> it is highly recommended for platform firmware or operating software
>
> Recommended by whom?  If the spec recommends it, we should provide the
> citation.
PCIe spec 5.0 r1.0 section 2.2.6.2 IMPLEMENTATION NOTE says that.
Will fix.
>
>> that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable
>> bit automatically in Endpoints with 10-Bit Tag Requester capability. This
>> enables the important class of 10-Bit Tag capable adapters that send
>> Memory Read Requests only to host memory.
>
> What is the implication for P2PDMA?  What happens if we enable 10-bit
> tags for device A, and A generates Mem Read Requests to device B,
> which does not support 10-bit tags?
PCIe spec 5.0 r1.0 section 2.2.6.2 says
If an Endpoint supports sending Requests to other Endpoints (as opposed 
to host memory), the Endpoint must not send 10-Bit Tag Requests to 
another given Endpoint unless an implementation-specific mechanism 
determines that the Endpoint supports 10-Bit Tag Completer capability. 
Not sending 10-Bit Tag Requests to other Endpoints at all
may be acceptable for some implementations. More sophisticated 
mechanisms are outside the scope of this specification.

Not sending 10-Bit Tag Requests to other Endpoints at all seems simple.
Add kernel parameter pci=pcie_bus_peer2peer when boot kernel with 
P2PDMA, then do not config 10-BIT Tag.

if (pcie_bus_config != PCIE_BUS_PEER2PEER)
	pci_configure_10bit_tags(dev);

Bjorn and Logan, any suggestion?
>
>> Signed-off-by: Dongdong Liu <liudongdong3@huawei.com>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>> ---
>>  drivers/pci/probe.c | 33 +++++++++++++++++++++++++++++++++
>>  include/linux/pci.h |  2 ++
>>  2 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 0208865..33241fb 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2048,6 +2048,38 @@ int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
>>  	return 0;
>>  }
>>
>> +static void pci_configure_10bit_tags(struct pci_dev *dev)
>> +{
>> +	struct pci_dev *bridge;
>> +
>> +	if (!(dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_COMP))
>> +		return;
>> +
>> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
>> +		dev->ext_10bit_tag = 1;
>> +		return;
>> +	}
>> +
>> +	bridge = pci_upstream_bridge(dev);
>> +	if (bridge && bridge->ext_10bit_tag)
>> +		dev->ext_10bit_tag = 1;
>> +
>> +	/*
>> +	 * 10-Bit Tag Requester Enable in Device Control 2 Register is RsvdP
>> +	 * for VF.
>> +	 */
>> +	if (dev->is_virtfn)
>> +		return;
>> +
>> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT &&
>> +	    dev->ext_10bit_tag == 1 &&
>> +	    (dev->pcie_devcap2 & PCI_EXP_DEVCAP2_10BIT_TAG_REQ)) {
>> +		pci_dbg(dev, "enabling 10-Bit Tag Requester\n");
>> +		pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
>> +					PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
>> +	}
>> +}
>> +
>>  /**
>>   * pcie_relaxed_ordering_enabled - Probe for PCIe relaxed ordering enable
>>   * @dev: PCI device to query
>> @@ -2184,6 +2216,7 @@ static void pci_configure_device(struct pci_dev *dev)
>>  {
>>  	pci_configure_mps(dev);
>>  	pci_configure_extended_tags(dev, NULL);
>> +	pci_configure_10bit_tags(dev);
>
> I think 10-bit tag support should be integrated with extended (8-bit)
> tag support instead of having two separate functions.
>
> If we have "no_ext_tags" set because some device doesn't support 8-bit
> tags correctly, we probably shouldn't try to enable 10-bit tags
> either.
Looks good, will fix.

Thanks
Dongdong
>
>>  	pci_configure_relaxed_ordering(dev);
>>  	pci_configure_ltr(dev);
>>  	pci_configure_eetlp_prefix(dev);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index de1fc24..445d102 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -393,6 +393,8 @@ struct pci_dev {
>>  #endif
>>  	unsigned int	eetlp_prefix_path:1;	/* End-to-End TLP Prefix */
>>
>> +	unsigned int	ext_10bit_tag:1; /* 10-Bit Tag Completer Supported
>> +					    from root to here */
>>  	pci_channel_state_t error_state;	/* Current connectivity state */
>>  	struct device	dev;			/* Generic device interface */
>>
>> --
>> 2.7.4
>>
> .
>

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

* Re: [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-07-16 11:12     ` Dongdong Liu
@ 2021-07-16 14:17       ` Bjorn Helgaas
  2021-07-17  8:50         ` Dongdong Liu
  2021-07-16 15:51       ` Logan Gunthorpe
  1 sibling, 1 reply; 15+ messages in thread
From: Bjorn Helgaas @ 2021-07-16 14:17 UTC (permalink / raw)
  To: Dongdong Liu
  Cc: hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev,
	Logan Gunthorpe

On Fri, Jul 16, 2021 at 07:12:16PM +0800, Dongdong Liu wrote:
> Hi Bjorn
> 
> Many thanks for your review.
> 
> On 2021/7/16 1:23, Bjorn Helgaas wrote:
> > [+cc Logan]
> > 
> > On Mon, Jun 21, 2021 at 06:27:20PM +0800, Dongdong Liu wrote:
> > > 10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
> > > field size from 8 bits to 10 bits.
> > > 
> > > For platforms where the RC supports 10-Bit Tag Completer capability,
> > > it is highly recommended for platform firmware or operating software
> > 
> > Recommended by whom?  If the spec recommends it, we should provide the
> > citation.
>
> PCIe spec 5.0 r1.0 section 2.2.6.2 IMPLEMENTATION NOTE says that.
> Will fix.

Thanks, that will be helpful.

> > > that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable
> > > bit automatically in Endpoints with 10-Bit Tag Requester capability. This
> > > enables the important class of 10-Bit Tag capable adapters that send
> > > Memory Read Requests only to host memory.
> > 
> > What is the implication for P2PDMA?  What happens if we enable 10-bit
> > tags for device A, and A generates Mem Read Requests to device B,
> > which does not support 10-bit tags?
>
> PCIe spec 5.0 r1.0 section 2.2.6.2 says
> If an Endpoint supports sending Requests to other Endpoints (as opposed to
> host memory), the Endpoint must not send 10-Bit Tag Requests to another
> given Endpoint unless an implementation-specific mechanism determines that
> the Endpoint supports 10-Bit Tag Completer capability. Not sending 10-Bit
> Tag Requests to other Endpoints at all
> may be acceptable for some implementations. More sophisticated mechanisms
> are outside the scope of this specification.
> 
> Not sending 10-Bit Tag Requests to other Endpoints at all seems simple.
> Add kernel parameter pci=pcie_bus_peer2peer when boot kernel with P2PDMA,
> then do not config 10-BIT Tag.
> 
> if (pcie_bus_config != PCIE_BUS_PEER2PEER)
> 	pci_configure_10bit_tags(dev);

Seems like a reasonable start.  I wish this were more dynamic and we
didn't have to rely on a kernel parameter to make P2PDMA safe, but
that seems to be the current situation.

Does the same consideration apply to enabling Extended Tags (8-bit
tags)?  I would guess so, but sec 2.2.6.2 says "Receivers/Completers
must handle 8-bit Tag values correctly regardless of the setting of
their Extended Tag Field Enable bit" so there's some subtlety there
with regard to what "Extended Tag Field Supported" means.

I don't know why the "Extended Tag Field Supported" bit exists if all
receivers are required to support 8-bit tags.

If we need a similar change to pci_configure_extended_tags() to check
pcie_bus_config, that should be a separate patch because it would be a
bug fix independent of 10-bit tag support.

Bjorn

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

* Re: [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-07-16 11:12     ` Dongdong Liu
  2021-07-16 14:17       ` Bjorn Helgaas
@ 2021-07-16 15:51       ` Logan Gunthorpe
  2021-07-17  9:41         ` Dongdong Liu
  1 sibling, 1 reply; 15+ messages in thread
From: Logan Gunthorpe @ 2021-07-16 15:51 UTC (permalink / raw)
  To: Dongdong Liu, Bjorn Helgaas
  Cc: hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev



On 2021-07-16 5:12 a.m., Dongdong Liu wrote:
> Hi Bjorn
> 
> Many thanks for your review.
> 
> On 2021/7/16 1:23, Bjorn Helgaas wrote:
>> [+cc Logan]
>>
>> On Mon, Jun 21, 2021 at 06:27:20PM +0800, Dongdong Liu wrote:
>>> 10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
>>> field size from 8 bits to 10 bits.
>>>
>>> For platforms where the RC supports 10-Bit Tag Completer capability,
>>> it is highly recommended for platform firmware or operating software
>>
>> Recommended by whom?  If the spec recommends it, we should provide the
>> citation.
> PCIe spec 5.0 r1.0 section 2.2.6.2 IMPLEMENTATION NOTE says that.
> Will fix.
>>
>>> that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable
>>> bit automatically in Endpoints with 10-Bit Tag Requester capability. This
>>> enables the important class of 10-Bit Tag capable adapters that send
>>> Memory Read Requests only to host memory.
>>
>> What is the implication for P2PDMA?  What happens if we enable 10-bit
>> tags for device A, and A generates Mem Read Requests to device B,
>> which does not support 10-bit tags?
> PCIe spec 5.0 r1.0 section 2.2.6.2 says
> If an Endpoint supports sending Requests to other Endpoints (as opposed 
> to host memory), the Endpoint must not send 10-Bit Tag Requests to 
> another given Endpoint unless an implementation-specific mechanism 
> determines that the Endpoint supports 10-Bit Tag Completer capability. 
> Not sending 10-Bit Tag Requests to other Endpoints at all
> may be acceptable for some implementations. More sophisticated 
> mechanisms are outside the scope of this specification.
> 
> Not sending 10-Bit Tag Requests to other Endpoints at all seems simple.
> Add kernel parameter pci=pcie_bus_peer2peer when boot kernel with 
> P2PDMA, then do not config 10-BIT Tag.
> 
> if (pcie_bus_config != PCIE_BUS_PEER2PEER)
> 	pci_configure_10bit_tags(dev);
> 
> Bjorn and Logan, any suggestion?

I think we need a check in the P2PDMA code to ensure that a device with
10bit tags doesn't interact with a device that has no 10bit tags. Before
that happens, the kernel should emit a warning saying to enable a
specific kernel parameter.

Though a parameter with a bit more granularity might be appropriate. See
what was done for disable_acs_redir where it affects only the devices
specified in the list.

Thanks,

Logan

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

* Re: [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-07-16 14:17       ` Bjorn Helgaas
@ 2021-07-17  8:50         ` Dongdong Liu
  0 siblings, 0 replies; 15+ messages in thread
From: Dongdong Liu @ 2021-07-17  8:50 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev,
	Logan Gunthorpe, okaya

[+cc Sinan]

On 2021/7/16 22:17, Bjorn Helgaas wrote:
> On Fri, Jul 16, 2021 at 07:12:16PM +0800, Dongdong Liu wrote:
>> Hi Bjorn
>>
>> Many thanks for your review.
>>
>> On 2021/7/16 1:23, Bjorn Helgaas wrote:
>>> [+cc Logan]
>>>
>>> On Mon, Jun 21, 2021 at 06:27:20PM +0800, Dongdong Liu wrote:
>>>> 10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
>>>> field size from 8 bits to 10 bits.
>>>>
>>>> For platforms where the RC supports 10-Bit Tag Completer capability,
>>>> it is highly recommended for platform firmware or operating software
>>>
>>> Recommended by whom?  If the spec recommends it, we should provide the
>>> citation.
>>
>> PCIe spec 5.0 r1.0 section 2.2.6.2 IMPLEMENTATION NOTE says that.
>> Will fix.
>
> Thanks, that will be helpful.
>
>>>> that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable
>>>> bit automatically in Endpoints with 10-Bit Tag Requester capability. This
>>>> enables the important class of 10-Bit Tag capable adapters that send
>>>> Memory Read Requests only to host memory.
>>>
>>> What is the implication for P2PDMA?  What happens if we enable 10-bit
>>> tags for device A, and A generates Mem Read Requests to device B,
>>> which does not support 10-bit tags?
>>
>> PCIe spec 5.0 r1.0 section 2.2.6.2 says
>> If an Endpoint supports sending Requests to other Endpoints (as opposed to
>> host memory), the Endpoint must not send 10-Bit Tag Requests to another
>> given Endpoint unless an implementation-specific mechanism determines that
>> the Endpoint supports 10-Bit Tag Completer capability. Not sending 10-Bit
>> Tag Requests to other Endpoints at all
>> may be acceptable for some implementations. More sophisticated mechanisms
>> are outside the scope of this specification.
>>
>> Not sending 10-Bit Tag Requests to other Endpoints at all seems simple.
>> Add kernel parameter pci=pcie_bus_peer2peer when boot kernel with P2PDMA,
>> then do not config 10-BIT Tag.
>>
>> if (pcie_bus_config != PCIE_BUS_PEER2PEER)
>> 	pci_configure_10bit_tags(dev);
>
> Seems like a reasonable start.  I wish this were more dynamic and we
> didn't have to rely on a kernel parameter to make P2PDMA safe, but
> that seems to be the current situation.
>
> Does the same consideration apply to enabling Extended Tags (8-bit
> tags)?  I would guess so, but sec 2.2.6.2 says "Receivers/Completers
> must handle 8-bit Tag values correctly regardless of the setting of
> their Extended Tag Field Enable bit" so there's some subtlety there
> with regard to what "Extended Tag Field Supported" means.
>
> I don't know why the "Extended Tag Field Supported" bit exists if all
> receivers are required to support 8-bit tags.

The comment in the [PATCH] PCI: enable extended tags support for PCIe 
endpoints 
(https://patchwork.kernel.org/project/linux-arm-msm/patch/1474769434-5756-1-git-send-email-okaya@codeaurora.org/)
says "All PCIe completers are required to support 8 bit tags.
Generation of 8 bit tags is optional. That's why, there is a supported 
and an enable/disable bit."

So the completers can handle 8-bit Tag values correctly also regardless 
of "Extended Tag Field Supported" ?  seems not very clearly, but current 
code implement follow this.

>
> If we need a similar change to pci_configure_extended_tags() to check
> pcie_bus_config, that should be a separate patch because it would be a
> bug fix independent of 10-bit tag support.
>
Seems no need if All PCIe completers are required to support 8 bit tags.

Thanks,
Dongdong
> Bjorn
> .
>

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

* Re: [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-07-16 15:51       ` Logan Gunthorpe
@ 2021-07-17  9:41         ` Dongdong Liu
  0 siblings, 0 replies; 15+ messages in thread
From: Dongdong Liu @ 2021-07-17  9:41 UTC (permalink / raw)
  To: Logan Gunthorpe, Bjorn Helgaas
  Cc: hch, kw, linux-pci, rajur, hverkuil-cisco, linux-media, netdev



On 2021/7/16 23:51, Logan Gunthorpe wrote:
>
>
> On 2021-07-16 5:12 a.m., Dongdong Liu wrote:
>> Hi Bjorn
>>
>> Many thanks for your review.
>>
>> On 2021/7/16 1:23, Bjorn Helgaas wrote:
>>> [+cc Logan]
>>>
>>> On Mon, Jun 21, 2021 at 06:27:20PM +0800, Dongdong Liu wrote:
>>>> 10-Bit Tag capability, introduced in PCIe-4.0 increases the total Tag
>>>> field size from 8 bits to 10 bits.
>>>>
>>>> For platforms where the RC supports 10-Bit Tag Completer capability,
>>>> it is highly recommended for platform firmware or operating software
>>>
>>> Recommended by whom?  If the spec recommends it, we should provide the
>>> citation.
>> PCIe spec 5.0 r1.0 section 2.2.6.2 IMPLEMENTATION NOTE says that.
>> Will fix.
>>>
>>>> that configures PCIe hierarchies to Set the 10-Bit Tag Requester Enable
>>>> bit automatically in Endpoints with 10-Bit Tag Requester capability. This
>>>> enables the important class of 10-Bit Tag capable adapters that send
>>>> Memory Read Requests only to host memory.
>>>
>>> What is the implication for P2PDMA?  What happens if we enable 10-bit
>>> tags for device A, and A generates Mem Read Requests to device B,
>>> which does not support 10-bit tags?
>> PCIe spec 5.0 r1.0 section 2.2.6.2 says
>> If an Endpoint supports sending Requests to other Endpoints (as opposed
>> to host memory), the Endpoint must not send 10-Bit Tag Requests to
>> another given Endpoint unless an implementation-specific mechanism
>> determines that the Endpoint supports 10-Bit Tag Completer capability.
>> Not sending 10-Bit Tag Requests to other Endpoints at all
>> may be acceptable for some implementations. More sophisticated
>> mechanisms are outside the scope of this specification.
>>
>> Not sending 10-Bit Tag Requests to other Endpoints at all seems simple.
>> Add kernel parameter pci=pcie_bus_peer2peer when boot kernel with
>> P2PDMA, then do not config 10-BIT Tag.
>>
>> if (pcie_bus_config != PCIE_BUS_PEER2PEER)
>> 	pci_configure_10bit_tags(dev);
>>
>> Bjorn and Logan, any suggestion?
>
> I think we need a check in the P2PDMA code to ensure that a device with
> 10bit tags doesn't interact with a device that has no 10bit tags. Before
> that happens, the kernel should emit a warning saying to enable a
> specific kernel parameter.
Seems reasonable.
>
> Though a parameter with a bit more granularity might be appropriate. See
> what was done for disable_acs_redir where it affects only the devices
> specified in the list.

Many Thanks for your suggestion. I will investigate more about this.

It seems P2PDMA also does not consider MPS safe issue if not use 
"pci=pcie_bus_peer2peer".

Thanks,
Dongdong
>
> Thanks,
>
> Logan
> .
>

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

end of thread, other threads:[~2021-07-17  9:41 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 10:27 [PATCH V5 0/6] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
2021-06-21 10:27 ` [PATCH V5 1/6] PCI: Use cached Device Capabilities Register Dongdong Liu
2021-06-21 10:27 ` [PATCH V5 2/6] PCI: Use cached Device Capabilities 2 Register Dongdong Liu
2021-06-22  6:17   ` Christoph Hellwig
2021-06-21 10:27 ` [PATCH V5 3/6] PCI: Add 10-Bit Tag register definitions Dongdong Liu
2021-06-21 10:27 ` [PATCH V5 4/6] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
2021-07-15 17:23   ` Bjorn Helgaas
2021-07-16 11:12     ` Dongdong Liu
2021-07-16 14:17       ` Bjorn Helgaas
2021-07-17  8:50         ` Dongdong Liu
2021-07-16 15:51       ` Logan Gunthorpe
2021-07-17  9:41         ` Dongdong Liu
2021-06-21 10:27 ` [PATCH V5 5/6] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices Dongdong Liu
2021-06-21 10:27 ` [PATCH V5 6/6] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
2021-06-22  6:19   ` Christoph Hellwig

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