linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] PCI: Enable 10-Bit tag support for PCIe devices
@ 2021-04-03  8:54 Dongdong Liu
  2021-04-03  8:54 ` [PATCH 1/4] PCI: Add 10-Bit Tag register definitions Dongdong Liu
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Dongdong Liu @ 2021-04-03  8:54 UTC (permalink / raw)
  To: helgaas, linux-pci

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.

Dongdong Liu (4):
  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/pci/iov.c              |   8 +++
 drivers/pci/pci.h              |   2 +
 drivers/pci/pcie/portdrv_pci.c |   3 +
 drivers/pci/probe.c            | 125 +++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h            |   1 +
 include/uapi/linux/pci_regs.h  |   5 ++
 6 files changed, 144 insertions(+)

--
1.9.1


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

* [PATCH 1/4] PCI: Add 10-Bit Tag register definitions
  2021-04-03  8:54 [PATCH 0/4] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
@ 2021-04-03  8:54 ` Dongdong Liu
  2021-04-03  8:54 ` [PATCH 2/4] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Dongdong Liu @ 2021-04-03  8:54 UTC (permalink / raw)
  To: helgaas, linux-pci

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>
---
 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 */
-- 
1.9.1


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

* [PATCH 2/4] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-04-03  8:54 [PATCH 0/4] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
  2021-04-03  8:54 ` [PATCH 1/4] PCI: Add 10-Bit Tag register definitions Dongdong Liu
@ 2021-04-03  8:54 ` Dongdong Liu
  2021-04-03  9:50   ` Christoph Hellwig
  2021-04-03  8:54 ` [PATCH 3/4] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices Dongdong Liu
  2021-04-03  8:54 ` [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
  3 siblings, 1 reply; 10+ messages in thread
From: Dongdong Liu @ 2021-04-03  8:54 UTC (permalink / raw)
  To: helgaas, linux-pci

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>
---
 drivers/pci/probe.c | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 40 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 953f15a..3efe1cc 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2051,6 +2051,44 @@ int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
 	return 0;
 }
 
+static void pci_configure_10bit_tags(struct pci_dev *dev)
+{
+	u32 cap;
+	int ret;
+	struct pci_dev *bridge;
+
+	if (!pci_is_pcie(dev))
+		return;
+
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+	if (ret)
+		return;
+
+	if (!(cap & PCI_EXP_DEVCAP2_10BIT_TAG_COMP))
+		return;
+
+	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
+		dev->ext_10bit_tag_comp_path = 1;
+		return;
+	}
+
+	bridge = pci_upstream_bridge(dev);
+	if (bridge && bridge->ext_10bit_tag_comp_path)
+		dev->ext_10bit_tag_comp_path = 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_comp_path == 1 &&
+	    (cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ)) {
+		pci_info(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
@@ -2190,6 +2228,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 86c799c..1cd0ee0 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -390,6 +390,7 @@ struct pci_dev {
 #endif
 	unsigned int	eetlp_prefix_path:1;	/* End-to-End TLP Prefix */
 
+	unsigned int	ext_10bit_tag_comp_path: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 */
 
-- 
1.9.1


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

* [PATCH 3/4] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices
  2021-04-03  8:54 [PATCH 0/4] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
  2021-04-03  8:54 ` [PATCH 1/4] PCI: Add 10-Bit Tag register definitions Dongdong Liu
  2021-04-03  8:54 ` [PATCH 2/4] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
@ 2021-04-03  8:54 ` Dongdong Liu
  2021-04-03  8:54 ` [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
  3 siblings, 0 replies; 10+ messages in thread
From: Dongdong Liu @ 2021-04-03  8:54 UTC (permalink / raw)
  To: helgaas, linux-pci

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

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

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 4afd4ee..9bff76b 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -537,6 +537,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_comp_path)
+		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);
@@ -553,6 +557,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);
@@ -585,6 +591,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);
-- 
1.9.1


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

* [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices
  2021-04-03  8:54 [PATCH 0/4] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
                   ` (2 preceding siblings ...)
  2021-04-03  8:54 ` [PATCH 3/4] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices Dongdong Liu
@ 2021-04-03  8:54 ` Dongdong Liu
  2021-04-03 10:32   ` kernel test robot
  2021-04-03 12:33   ` kernel test robot
  3 siblings, 2 replies; 10+ messages in thread
From: Dongdong Liu @ 2021-04-03  8:54 UTC (permalink / raw)
  To: helgaas, linux-pci

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/pci.h              |  2 +
 drivers/pci/pcie/portdrv_pci.c |  3 ++
 drivers/pci/probe.c            | 86 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 91 insertions(+)

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index ef7c466..056b73d 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -459,6 +459,7 @@ static inline void pci_dpc_init(struct pci_dev *pdev) {}
 void pcie_walk_rcec(struct pci_dev *rcec,
 		    int (*cb)(struct pci_dev *, void *),
 		    void *userdata);
+void pci_configure_rp_10bit_tag(struct pci_dev *dev);
 #else
 static inline void pci_rcec_init(struct pci_dev *dev) {}
 static inline void pci_rcec_exit(struct pci_dev *dev) {}
@@ -466,6 +467,7 @@ static inline void pcie_link_rcec(struct pci_dev *rcec) {}
 static inline void pcie_walk_rcec(struct pci_dev *rcec,
 				  int (*cb)(struct pci_dev *, void *),
 				  void *userdata) {}
+static inline void pci_configure_rp_10bit_tag(struct pci_dev *dev) {}
 #endif
 
 #ifdef CONFIG_PCI_ATS
diff --git a/drivers/pci/pcie/portdrv_pci.c b/drivers/pci/pcie/portdrv_pci.c
index c7ff1ee..3af4733 100644
--- a/drivers/pci/pcie/portdrv_pci.c
+++ b/drivers/pci/pcie/portdrv_pci.c
@@ -111,6 +111,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);
 
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 3efe1cc..73105c3 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2051,6 +2051,92 @@ int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
 	return 0;
 }
 
+static int pci_10bit_tag_comp_support(struct pci_dev *dev, void *data)
+{
+	u8 *support = data;
+	int ret;
+	u32 cap;
+
+	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;
+	}
+
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+	if (ret) {
+		*support = 0;
+		return 0;
+	}
+
+	if (!(cap & PCI_EXP_DEVCAP2_10BIT_TAG_COMP)) {
+		*support = 0;
+		return 0;
+	}
+
+
+	return 0;
+}
+
+void pci_configure_rp_10bit_tag(struct pci_dev *dev)
+{
+	u8 support = 1;
+	u32 cap;
+	int ret;
+
+	if (!pci_is_pcie(dev))
+		return;
+
+	if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
+		return;
+
+	if (dev->subordinate == NULL)
+		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;
+
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+	if (ret)
+		return;
+
+	if (!(cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ))
+		return;
+
+	pci_info(dev, "enabling 10-Bit Tag Requester\n");
+	pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
+				 PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
+}
+
+
 static void pci_configure_10bit_tags(struct pci_dev *dev)
 {
 	u32 cap;
-- 
1.9.1


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

* Re: [PATCH 2/4] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-04-03  8:54 ` [PATCH 2/4] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
@ 2021-04-03  9:50   ` Christoph Hellwig
  2021-04-06  2:33     ` Dongdong Liu
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2021-04-03  9:50 UTC (permalink / raw)
  To: Dongdong Liu; +Cc: helgaas, linux-pci

> +	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
> +	if (ret)
> +		return;

Wouldn't it make sense to store the devcap value in the pci_dev
structure instead of reading it multiple times?

> +	/* 10-Bit Tag Requester Enable in Device Control 2 Register is RsvdP for VF */

Please avoid the overly long lines.

> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT &&
> +	    dev->ext_10bit_tag_comp_path == 1 &&
> +	    (cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ)) {
> +		pci_info(dev, "enabling 10-Bit Tag Requester\n");

I think that printk might become a little too noisy when lots of
devices support this capability.

> +	unsigned int	ext_10bit_tag_comp_path:1; /* 10-Bit Tag Completer Supported from root to here */

Another crazy long line.  And why not just name this
10bit_tags?

Also a lot of this walk the upstream bridges until we hit the
root port code seems duplicatated for different capabilities.  Shouldn't
we have one such walk that checks all the interesting capabilities?  Or
even turn the thing around and set them on the fly while scanning
the topology?

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

* Re: [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices
  2021-04-03  8:54 ` [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
@ 2021-04-03 10:32   ` kernel test robot
  2021-04-03 12:33   ` kernel test robot
  1 sibling, 0 replies; 10+ messages in thread
From: kernel test robot @ 2021-04-03 10:32 UTC (permalink / raw)
  To: Dongdong Liu, helgaas, linux-pci; +Cc: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3331 bytes --]

Hi Dongdong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.12-rc5 next-20210401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dongdong-Liu/PCI-Enable-10-Bit-tag-support-for-PCIe-devices/20210403-171726
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: i386-randconfig-m021-20210403 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/c1497e514c78375b8c9e9e074e5b84c62eaef20f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dongdong-Liu/PCI-Enable-10-Bit-tag-support-for-PCIe-devices/20210403-171726
        git checkout c1497e514c78375b8c9e9e074e5b84c62eaef20f
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/pci/probe.c:2096:6: error: redefinition of 'pci_configure_rp_10bit_tag'
    2096 | void pci_configure_rp_10bit_tag(struct pci_dev *dev)
         |      ^~~~~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/pci/probe.c:22:
   drivers/pci/pci.h:468:20: note: previous definition of 'pci_configure_rp_10bit_tag' was here
     468 | static inline void pci_configure_rp_10bit_tag(struct pci_dev *dev) {}
         |                    ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/pci_configure_rp_10bit_tag +2096 drivers/pci/probe.c

  2095	
> 2096	void pci_configure_rp_10bit_tag(struct pci_dev *dev)
  2097	{
  2098		u8 support = 1;
  2099		u32 cap;
  2100		int ret;
  2101	
  2102		if (!pci_is_pcie(dev))
  2103			return;
  2104	
  2105		if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
  2106			return;
  2107	
  2108		if (dev->subordinate == NULL)
  2109			return;
  2110	
  2111		pci_10bit_tag_comp_support(dev, &support);
  2112		if (!support)
  2113			return;
  2114	
  2115		/*
  2116		 * PCIe spec 5.0r1.0 section 2.2.6.2 implementation note
  2117		 * In configurations where a Requester with 10-Bit Tag Requester capability
  2118		 * needs to target multiple Completers, one needs to ensure that the
  2119		 * Requester sends 10-Bit Tag Requests only to Completers that have 10-Bit
  2120		 * Tag Completer capability. So we enable 10-Bit Tag Requester for root port
  2121		 * only when the devices under the root port support 10-Bit Tag Completer.
  2122		 */
  2123		pci_walk_bus(dev->subordinate, pci_10bit_tag_comp_support, &support);
  2124		if (!support)
  2125			return;
  2126	
  2127		ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
  2128		if (ret)
  2129			return;
  2130	
  2131		if (!(cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ))
  2132			return;
  2133	
  2134		pci_info(dev, "enabling 10-Bit Tag Requester\n");
  2135		pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
  2136					 PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
  2137	}
  2138	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31069 bytes --]

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

* Re: [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices
  2021-04-03  8:54 ` [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
  2021-04-03 10:32   ` kernel test robot
@ 2021-04-03 12:33   ` kernel test robot
  2021-04-06  2:41     ` Dongdong Liu
  1 sibling, 1 reply; 10+ messages in thread
From: kernel test robot @ 2021-04-03 12:33 UTC (permalink / raw)
  To: Dongdong Liu, helgaas, linux-pci; +Cc: kbuild-all, clang-built-linux

[-- Attachment #1: Type: text/plain, Size: 3568 bytes --]

Hi Dongdong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on pci/next]
[also build test ERROR on v5.12-rc5 next-20210401]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Dongdong-Liu/PCI-Enable-10-Bit-tag-support-for-PCIe-devices/20210403-171726
base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
config: x86_64-randconfig-a004-20210403 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 0fe8af94688aa03c01913c2001d6a1a911f42ce6)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/c1497e514c78375b8c9e9e074e5b84c62eaef20f
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Dongdong-Liu/PCI-Enable-10-Bit-tag-support-for-PCIe-devices/20210403-171726
        git checkout c1497e514c78375b8c9e9e074e5b84c62eaef20f
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/pci/probe.c:2096:6: error: redefinition of 'pci_configure_rp_10bit_tag'
   void pci_configure_rp_10bit_tag(struct pci_dev *dev)
        ^
   drivers/pci/pci.h:468:20: note: previous definition is here
   static inline void pci_configure_rp_10bit_tag(struct pci_dev *dev) {}
                      ^
   1 error generated.


vim +/pci_configure_rp_10bit_tag +2096 drivers/pci/probe.c

  2095	
> 2096	void pci_configure_rp_10bit_tag(struct pci_dev *dev)
  2097	{
  2098		u8 support = 1;
  2099		u32 cap;
  2100		int ret;
  2101	
  2102		if (!pci_is_pcie(dev))
  2103			return;
  2104	
  2105		if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
  2106			return;
  2107	
  2108		if (dev->subordinate == NULL)
  2109			return;
  2110	
  2111		pci_10bit_tag_comp_support(dev, &support);
  2112		if (!support)
  2113			return;
  2114	
  2115		/*
  2116		 * PCIe spec 5.0r1.0 section 2.2.6.2 implementation note
  2117		 * In configurations where a Requester with 10-Bit Tag Requester capability
  2118		 * needs to target multiple Completers, one needs to ensure that the
  2119		 * Requester sends 10-Bit Tag Requests only to Completers that have 10-Bit
  2120		 * Tag Completer capability. So we enable 10-Bit Tag Requester for root port
  2121		 * only when the devices under the root port support 10-Bit Tag Completer.
  2122		 */
  2123		pci_walk_bus(dev->subordinate, pci_10bit_tag_comp_support, &support);
  2124		if (!support)
  2125			return;
  2126	
  2127		ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
  2128		if (ret)
  2129			return;
  2130	
  2131		if (!(cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ))
  2132			return;
  2133	
  2134		pci_info(dev, "enabling 10-Bit Tag Requester\n");
  2135		pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
  2136					 PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
  2137	}
  2138	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39283 bytes --]

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

* Re: [PATCH 2/4] PCI: Enable 10-Bit tag support for PCIe Endpoint devices
  2021-04-03  9:50   ` Christoph Hellwig
@ 2021-04-06  2:33     ` Dongdong Liu
  0 siblings, 0 replies; 10+ messages in thread
From: Dongdong Liu @ 2021-04-06  2:33 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: helgaas, linux-pci

Hi Christoph

Many Thanks for your review.
On 2021/4/3 17:50, Christoph Hellwig wrote:
>> +	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
>> +	if (ret)
>> +		return;
>
> Wouldn't it make sense to store the devcap value in the pci_dev
> structure instead of reading it multiple times?
>
Good suggestion.
>> +	/* 10-Bit Tag Requester Enable in Device Control 2 Register is RsvdP for VF */
>
> Please avoid the overly long lines.

Will fix.
>
>> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_ENDPOINT &&
>> +	    dev->ext_10bit_tag_comp_path == 1 &&
>> +	    (cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ)) {
>> +		pci_info(dev, "enabling 10-Bit Tag Requester\n");
>
> I think that printk might become a little too noisy when lots of
> devices support this capability.
>
Yes, How about change to pci_dbg.

>> +	unsigned int	ext_10bit_tag_comp_path:1; /* 10-Bit Tag Completer Supported from root to here */
>
> Another crazy long line.  And why not just name this
> 10bit_tags?

OK, will fix. How about name this _10bit_tag or ext_10bit_tag
as C language variable names cannot start with a number.
>
> Also a lot of this walk the upstream bridges until we hit the
> root port code seems duplicatated for different capabilities.  Shouldn't
> we have one such walk that checks all the interesting capabilities?  Or
> even turn the thing around and set them on the fly while scanning
> the topology?

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

Thanks,
Dongdong

> .
>

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

* Re: [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices
  2021-04-03 12:33   ` kernel test robot
@ 2021-04-06  2:41     ` Dongdong Liu
  0 siblings, 0 replies; 10+ messages in thread
From: Dongdong Liu @ 2021-04-06  2:41 UTC (permalink / raw)
  To: kernel test robot, helgaas, linux-pci; +Cc: kbuild-all, clang-built-linux


Thnaks for your report. I made a mistake.
Maybe I need to move these functions implementation
to drivers/pci/pcie/portdrv_pci.c. No need to
define in drivers/pci/pci.h

Thanks,
Dongdong
On 2021/4/3 20:33, kernel test robot wrote:
> Hi Dongdong,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on pci/next]
> [also build test ERROR on v5.12-rc5 next-20210401]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url:    https://github.com/0day-ci/linux/commits/Dongdong-Liu/PCI-Enable-10-Bit-tag-support-for-PCIe-devices/20210403-171726
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git next
> config: x86_64-randconfig-a004-20210403 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 0fe8af94688aa03c01913c2001d6a1a911f42ce6)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install x86_64 cross compiling tool for clang build
>         # apt-get install binutils-x86-64-linux-gnu
>         # https://github.com/0day-ci/linux/commit/c1497e514c78375b8c9e9e074e5b84c62eaef20f
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Dongdong-Liu/PCI-Enable-10-Bit-tag-support-for-PCIe-devices/20210403-171726
>         git checkout c1497e514c78375b8c9e9e074e5b84c62eaef20f
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>>> drivers/pci/probe.c:2096:6: error: redefinition of 'pci_configure_rp_10bit_tag'
>    void pci_configure_rp_10bit_tag(struct pci_dev *dev)
>         ^
>    drivers/pci/pci.h:468:20: note: previous definition is here
>    static inline void pci_configure_rp_10bit_tag(struct pci_dev *dev) {}
>                       ^
>    1 error generated.
>
>
> vim +/pci_configure_rp_10bit_tag +2096 drivers/pci/probe.c
>
>   2095	
>> 2096	void pci_configure_rp_10bit_tag(struct pci_dev *dev)
>   2097	{
>   2098		u8 support = 1;
>   2099		u32 cap;
>   2100		int ret;
>   2101	
>   2102		if (!pci_is_pcie(dev))
>   2103			return;
>   2104	
>   2105		if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
>   2106			return;
>   2107	
>   2108		if (dev->subordinate == NULL)
>   2109			return;
>   2110	
>   2111		pci_10bit_tag_comp_support(dev, &support);
>   2112		if (!support)
>   2113			return;
>   2114	
>   2115		/*
>   2116		 * PCIe spec 5.0r1.0 section 2.2.6.2 implementation note
>   2117		 * In configurations where a Requester with 10-Bit Tag Requester capability
>   2118		 * needs to target multiple Completers, one needs to ensure that the
>   2119		 * Requester sends 10-Bit Tag Requests only to Completers that have 10-Bit
>   2120		 * Tag Completer capability. So we enable 10-Bit Tag Requester for root port
>   2121		 * only when the devices under the root port support 10-Bit Tag Completer.
>   2122		 */
>   2123		pci_walk_bus(dev->subordinate, pci_10bit_tag_comp_support, &support);
>   2124		if (!support)
>   2125			return;
>   2126	
>   2127		ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
>   2128		if (ret)
>   2129			return;
>   2130	
>   2131		if (!(cap & PCI_EXP_DEVCAP2_10BIT_TAG_REQ))
>   2132			return;
>   2133	
>   2134		pci_info(dev, "enabling 10-Bit Tag Requester\n");
>   2135		pcie_capability_set_word(dev, PCI_EXP_DEVCTL2,
>   2136					 PCI_EXP_DEVCTL2_10BIT_TAG_REQ_EN);
>   2137	}
>   2138	
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>

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

end of thread, other threads:[~2021-04-06  2:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-03  8:54 [PATCH 0/4] PCI: Enable 10-Bit tag support for PCIe devices Dongdong Liu
2021-04-03  8:54 ` [PATCH 1/4] PCI: Add 10-Bit Tag register definitions Dongdong Liu
2021-04-03  8:54 ` [PATCH 2/4] PCI: Enable 10-Bit tag support for PCIe Endpoint devices Dongdong Liu
2021-04-03  9:50   ` Christoph Hellwig
2021-04-06  2:33     ` Dongdong Liu
2021-04-03  8:54 ` [PATCH 3/4] PCI/IOV: Enable 10-Bit tag support for PCIe VF devices Dongdong Liu
2021-04-03  8:54 ` [PATCH 4/4] PCI: Enable 10-Bit tag support for PCIe RP devices Dongdong Liu
2021-04-03 10:32   ` kernel test robot
2021-04-03 12:33   ` kernel test robot
2021-04-06  2:41     ` Dongdong Liu

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