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:

quoted

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.

quoted

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?

quoted

Signed-off-by: Dongdong Liu <redacted>
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

quoted

 	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

.