diff mbox series

[v4,10/23] cxl: Add helpers to calculate pci latency for the CXL device

Message ID 168193572162.1178687.9726045601551945413.stgit@djiang5-mobl3
State New
Headers show
Series cxl: Add support for QTG ID retrieval for CXL subsystem | expand

Commit Message

Dave Jiang April 19, 2023, 8:22 p.m. UTC
The latency is calculated by dividing the flit size over the bandwidth. Add
support to retrieve the flit size for the CXL device and calculate the
latency of the downstream link.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>

---
v2:
- Fix commit log issues. (Jonathan)
- Fix var declaration issues. (Jonathan)
---
 drivers/cxl/core/pci.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxlpci.h   |   15 +++++++++++
 drivers/cxl/pci.c      |   13 ---------
 3 files changed, 83 insertions(+), 13 deletions(-)

Comments

Jonathan Cameron April 20, 2023, 12:15 p.m. UTC | #1
On Wed, 19 Apr 2023 13:22:01 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> The latency is calculated by dividing the flit size over the bandwidth. Add
> support to retrieve the flit size for the CXL device and calculate the
> latency of the downstream link.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Totally trivial stuff about using defines that exist for the various multipliers.
Otherwise looks good

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> ---
> v2:
> - Fix commit log issues. (Jonathan)
> - Fix var declaration issues. (Jonathan)
> ---
>  drivers/cxl/core/pci.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxlpci.h   |   15 +++++++++++
>  drivers/cxl/pci.c      |   13 ---------
>  3 files changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 1c415b26e866..bb58296b3e56 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c


> +static int cxl_pci_mbits_to_mbytes(struct pci_dev *pdev)
> +{
> +	int mbits;
> +
> +	mbits = pci_bus_speed_to_mbps(pdev->bus->cur_bus_speed);
> +	if (mbits < 0)
> +		return mbits;
> +
> +	return mbits >> 3;

mbits / BITS_PER_BYTE; from linux/bits.h

maybe.

> +}

> +/**
> + * cxl_pci_get_latency - calculate the link latency for the PCIe link
> + * @pdev - PCI device
> + *
> + * return: calculated latency or -errno
> + *
> + * CXL Memory Device SW Guide v1.0 2.11.4 Link latency calculation
> + * Link latency = LinkPropagationLatency + FlitLatency + RetimerLatency
> + * LinkProgationLatency is negligible, so 0 will be used
> + * RetimerLatency is assumed to be negligible and 0 will be used
> + * FlitLatency = FlitSize / LinkBandwidth
> + * FlitSize is defined by spec. CXL rev3.0 4.2.1.
> + * 68B flit is used up to 32GT/s. >32GT/s, 256B flit size is used.
> + * The FlitLatency is converted to picoseconds.
> + */
> +long cxl_pci_get_latency(struct pci_dev *pdev)
> +{
> +	long bw;
> +
> +	bw = cxl_pci_mbits_to_mbytes(pdev);
> +	if (bw < 0)
> +		return bw;
> +
> +	return cxl_flit_size(pdev) * 1000000L / bw;

MEGA from include/linux/units.h perhaps though it's an oddity because output of this is
pico seconds, so maybe needs to be PICO / MEGA to act as documentation of why.

> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_pci_get_latency, CXL);
Dan Williams April 25, 2023, 12:30 a.m. UTC | #2
Dave Jiang wrote:
> The latency is calculated by dividing the flit size over the bandwidth. Add
> support to retrieve the flit size for the CXL device and calculate the
> latency of the downstream link.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v2:
> - Fix commit log issues. (Jonathan)
> - Fix var declaration issues. (Jonathan)
> ---
>  drivers/cxl/core/pci.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxlpci.h   |   15 +++++++++++
>  drivers/cxl/pci.c      |   13 ---------
>  3 files changed, 83 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 1c415b26e866..bb58296b3e56 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -712,3 +712,71 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>  	return PCI_ERS_RESULT_NEED_RESET;
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_error_detected, CXL);
> +
> +static int pci_bus_speed_to_mbps(enum pci_bus_speed speed)
> +{
> +	switch (speed) {
> +	case PCIE_SPEED_2_5GT:
> +		return 2500;
> +	case PCIE_SPEED_5_0GT:
> +		return 5000;
> +	case PCIE_SPEED_8_0GT:
> +		return 8000;
> +	case PCIE_SPEED_16_0GT:
> +		return 16000;
> +	case PCIE_SPEED_32_0GT:
> +		return 32000;
> +	case PCIE_SPEED_64_0GT:
> +		return 64000;
> +	default:
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int cxl_pci_mbits_to_mbytes(struct pci_dev *pdev)
> +{
> +	int mbits;
> +
> +	mbits = pci_bus_speed_to_mbps(pdev->bus->cur_bus_speed);
> +	if (mbits < 0)
> +		return mbits;
> +
> +	return mbits >> 3;

Why not just return mbits directly and skip the conversion? Otherwise a
"/ 8" requires bit less cleverness to read than ">> 3".

> +}
> +
> +static int cxl_flit_size(struct pci_dev *pdev)

This like something that might be worth caching in 'struct cxl_port'
rather than re-reading the configuration register each call. Depends on
how often it is used.

> +{
> +	if (cxl_pci_flit_256(pdev))
> +		return 256;
> +
> +	return 68;
> +}
> +
> +/**
> + * cxl_pci_get_latency - calculate the link latency for the PCIe link
> + * @pdev - PCI device
> + *
> + * return: calculated latency or -errno
> + *
> + * CXL Memory Device SW Guide v1.0 2.11.4 Link latency calculation
> + * Link latency = LinkPropagationLatency + FlitLatency + RetimerLatency
> + * LinkProgationLatency is negligible, so 0 will be used
> + * RetimerLatency is assumed to be negligible and 0 will be used
> + * FlitLatency = FlitSize / LinkBandwidth
> + * FlitSize is defined by spec. CXL rev3.0 4.2.1.
> + * 68B flit is used up to 32GT/s. >32GT/s, 256B flit size is used.
> + * The FlitLatency is converted to picoseconds.
> + */
> +long cxl_pci_get_latency(struct pci_dev *pdev)
> +{
> +	long bw;
> +
> +	bw = cxl_pci_mbits_to_mbytes(pdev);

This function looks misnamed when I read it here, it's retrieving the
bus speed in MiBs not doing a conversion.

> +	if (bw < 0)
> +		return bw;
> +
> +	return cxl_flit_size(pdev) * 1000000L / bw;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_pci_get_latency, CXL);
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index 1bca1c0e4b40..795eba31fe29 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -167,6 +167,19 @@ struct cdat_sslbis {
>  #define SSLBIS_US_PORT		0x0100
>  #define SSLBIS_ANY_PORT		0xffff
>  
> +/*
> + * CXL v3.0 6.2.3 Table 6-4
> + * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
> + * mode, otherwise it's 68B flits mode.
> + */
> +static inline bool cxl_pci_flit_256(struct pci_dev *pdev)
> +{
> +	u16 lnksta2;
> +
> +	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
> +	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
> +}
> +
>  int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>  struct cxl_dev_state;
>  int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
> @@ -189,4 +202,6 @@ int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
>  cxl_parse_entry(dsmas);
>  cxl_parse_entry(dslbis);
>  cxl_parse_entry(sslbis);
> +
> +long cxl_pci_get_latency(struct pci_dev *pdev);
>  #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index ea38bd49b0cf..ed39d133b70d 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -365,19 +365,6 @@ static bool is_cxl_restricted(struct pci_dev *pdev)
>  	return pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END;
>  }
>  
> -/*
> - * CXL v3.0 6.2.3 Table 6-4
> - * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
> - * mode, otherwise it's 68B flits mode.
> - */
> -static bool cxl_pci_flit_256(struct pci_dev *pdev)
> -{
> -	u16 lnksta2;
> -
> -	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
> -	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
> -}
> -
>  static int cxl_pci_ras_unmask(struct pci_dev *pdev)
>  {
>  	struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);
> 
>
Dave Jiang May 1, 2023, 4:29 p.m. UTC | #3
On 4/24/23 5:30 PM, Dan Williams wrote:
> Dave Jiang wrote:
>> The latency is calculated by dividing the flit size over the bandwidth. Add
>> support to retrieve the flit size for the CXL device and calculate the
>> latency of the downstream link.
>>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>
>> ---
>> v2:
>> - Fix commit log issues. (Jonathan)
>> - Fix var declaration issues. (Jonathan)
>> ---
>>   drivers/cxl/core/pci.c |   68 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   drivers/cxl/cxlpci.h   |   15 +++++++++++
>>   drivers/cxl/pci.c      |   13 ---------
>>   3 files changed, 83 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
>> index 1c415b26e866..bb58296b3e56 100644
>> --- a/drivers/cxl/core/pci.c
>> +++ b/drivers/cxl/core/pci.c
>> @@ -712,3 +712,71 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
>>   	return PCI_ERS_RESULT_NEED_RESET;
>>   }
>>   EXPORT_SYMBOL_NS_GPL(cxl_error_detected, CXL);
>> +
>> +static int pci_bus_speed_to_mbps(enum pci_bus_speed speed)
>> +{
>> +	switch (speed) {
>> +	case PCIE_SPEED_2_5GT:
>> +		return 2500;
>> +	case PCIE_SPEED_5_0GT:
>> +		return 5000;
>> +	case PCIE_SPEED_8_0GT:
>> +		return 8000;
>> +	case PCIE_SPEED_16_0GT:
>> +		return 16000;
>> +	case PCIE_SPEED_32_0GT:
>> +		return 32000;
>> +	case PCIE_SPEED_64_0GT:
>> +		return 64000;
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return -EINVAL;
>> +}
>> +
>> +static int cxl_pci_mbits_to_mbytes(struct pci_dev *pdev)
>> +{
>> +	int mbits;
>> +
>> +	mbits = pci_bus_speed_to_mbps(pdev->bus->cur_bus_speed);
>> +	if (mbits < 0)
>> +		return mbits;
>> +
>> +	return mbits >> 3;
> 
> Why not just return mbits directly and skip the conversion? Otherwise a
> "/ 8" requires bit less cleverness to read than ">> 3".

You mean just move the math to the caller()?
> 
>> +}
>> +
>> +static int cxl_flit_size(struct pci_dev *pdev)
> 
> This like something that might be worth caching in 'struct cxl_port'
> rather than re-reading the configuration register each call. Depends on
> how often it is used.

You mean we just calculate it during cxl_port creation? I think the 
calculations for a switch upstream segment towards the root complex may 
be used multiple times. Downstream towards device, 1 or more depends on 
how many partitions there are. But probably not a big deal to just cache 
it.

> 
>> +{
>> +	if (cxl_pci_flit_256(pdev))
>> +		return 256;
>> +
>> +	return 68;
>> +}
>> +
>> +/**
>> + * cxl_pci_get_latency - calculate the link latency for the PCIe link
>> + * @pdev - PCI device
>> + *
>> + * return: calculated latency or -errno
>> + *
>> + * CXL Memory Device SW Guide v1.0 2.11.4 Link latency calculation
>> + * Link latency = LinkPropagationLatency + FlitLatency + RetimerLatency
>> + * LinkProgationLatency is negligible, so 0 will be used
>> + * RetimerLatency is assumed to be negligible and 0 will be used
>> + * FlitLatency = FlitSize / LinkBandwidth
>> + * FlitSize is defined by spec. CXL rev3.0 4.2.1.
>> + * 68B flit is used up to 32GT/s. >32GT/s, 256B flit size is used.
>> + * The FlitLatency is converted to picoseconds.
>> + */
>> +long cxl_pci_get_latency(struct pci_dev *pdev)
>> +{
>> +	long bw;
>> +
>> +	bw = cxl_pci_mbits_to_mbytes(pdev);
> 
> This function looks misnamed when I read it here, it's retrieving the
> bus speed in MiBs not doing a conversion.
> 
>> +	if (bw < 0)
>> +		return bw;
>> +
>> +	return cxl_flit_size(pdev) * 1000000L / bw;
>> +}
>> +EXPORT_SYMBOL_NS_GPL(cxl_pci_get_latency, CXL);
>> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
>> index 1bca1c0e4b40..795eba31fe29 100644
>> --- a/drivers/cxl/cxlpci.h
>> +++ b/drivers/cxl/cxlpci.h
>> @@ -167,6 +167,19 @@ struct cdat_sslbis {
>>   #define SSLBIS_US_PORT		0x0100
>>   #define SSLBIS_ANY_PORT		0xffff
>>   
>> +/*
>> + * CXL v3.0 6.2.3 Table 6-4
>> + * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
>> + * mode, otherwise it's 68B flits mode.
>> + */
>> +static inline bool cxl_pci_flit_256(struct pci_dev *pdev)
>> +{
>> +	u16 lnksta2;
>> +
>> +	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
>> +	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
>> +}
>> +
>>   int devm_cxl_port_enumerate_dports(struct cxl_port *port);
>>   struct cxl_dev_state;
>>   int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
>> @@ -189,4 +202,6 @@ int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
>>   cxl_parse_entry(dsmas);
>>   cxl_parse_entry(dslbis);
>>   cxl_parse_entry(sslbis);
>> +
>> +long cxl_pci_get_latency(struct pci_dev *pdev);
>>   #endif /* __CXL_PCI_H__ */
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index ea38bd49b0cf..ed39d133b70d 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -365,19 +365,6 @@ static bool is_cxl_restricted(struct pci_dev *pdev)
>>   	return pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END;
>>   }
>>   
>> -/*
>> - * CXL v3.0 6.2.3 Table 6-4
>> - * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
>> - * mode, otherwise it's 68B flits mode.
>> - */
>> -static bool cxl_pci_flit_256(struct pci_dev *pdev)
>> -{
>> -	u16 lnksta2;
>> -
>> -	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
>> -	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
>> -}
>> -
>>   static int cxl_pci_ras_unmask(struct pci_dev *pdev)
>>   {
>>   	struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);
>>
>>
> 
>
diff mbox series

Patch

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 1c415b26e866..bb58296b3e56 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -712,3 +712,71 @@  pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 	return PCI_ERS_RESULT_NEED_RESET;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_error_detected, CXL);
+
+static int pci_bus_speed_to_mbps(enum pci_bus_speed speed)
+{
+	switch (speed) {
+	case PCIE_SPEED_2_5GT:
+		return 2500;
+	case PCIE_SPEED_5_0GT:
+		return 5000;
+	case PCIE_SPEED_8_0GT:
+		return 8000;
+	case PCIE_SPEED_16_0GT:
+		return 16000;
+	case PCIE_SPEED_32_0GT:
+		return 32000;
+	case PCIE_SPEED_64_0GT:
+		return 64000;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int cxl_pci_mbits_to_mbytes(struct pci_dev *pdev)
+{
+	int mbits;
+
+	mbits = pci_bus_speed_to_mbps(pdev->bus->cur_bus_speed);
+	if (mbits < 0)
+		return mbits;
+
+	return mbits >> 3;
+}
+
+static int cxl_flit_size(struct pci_dev *pdev)
+{
+	if (cxl_pci_flit_256(pdev))
+		return 256;
+
+	return 68;
+}
+
+/**
+ * cxl_pci_get_latency - calculate the link latency for the PCIe link
+ * @pdev - PCI device
+ *
+ * return: calculated latency or -errno
+ *
+ * CXL Memory Device SW Guide v1.0 2.11.4 Link latency calculation
+ * Link latency = LinkPropagationLatency + FlitLatency + RetimerLatency
+ * LinkProgationLatency is negligible, so 0 will be used
+ * RetimerLatency is assumed to be negligible and 0 will be used
+ * FlitLatency = FlitSize / LinkBandwidth
+ * FlitSize is defined by spec. CXL rev3.0 4.2.1.
+ * 68B flit is used up to 32GT/s. >32GT/s, 256B flit size is used.
+ * The FlitLatency is converted to picoseconds.
+ */
+long cxl_pci_get_latency(struct pci_dev *pdev)
+{
+	long bw;
+
+	bw = cxl_pci_mbits_to_mbytes(pdev);
+	if (bw < 0)
+		return bw;
+
+	return cxl_flit_size(pdev) * 1000000L / bw;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_pci_get_latency, CXL);
diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
index 1bca1c0e4b40..795eba31fe29 100644
--- a/drivers/cxl/cxlpci.h
+++ b/drivers/cxl/cxlpci.h
@@ -167,6 +167,19 @@  struct cdat_sslbis {
 #define SSLBIS_US_PORT		0x0100
 #define SSLBIS_ANY_PORT		0xffff
 
+/*
+ * CXL v3.0 6.2.3 Table 6-4
+ * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
+ * mode, otherwise it's 68B flits mode.
+ */
+static inline bool cxl_pci_flit_256(struct pci_dev *pdev)
+{
+	u16 lnksta2;
+
+	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
+	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
+}
+
 int devm_cxl_port_enumerate_dports(struct cxl_port *port);
 struct cxl_dev_state;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
@@ -189,4 +202,6 @@  int cxl_##x##_parse_entry(struct cdat_entry_header *header, void *arg)
 cxl_parse_entry(dsmas);
 cxl_parse_entry(dslbis);
 cxl_parse_entry(sslbis);
+
+long cxl_pci_get_latency(struct pci_dev *pdev);
 #endif /* __CXL_PCI_H__ */
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index ea38bd49b0cf..ed39d133b70d 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -365,19 +365,6 @@  static bool is_cxl_restricted(struct pci_dev *pdev)
 	return pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END;
 }
 
-/*
- * CXL v3.0 6.2.3 Table 6-4
- * The table indicates that if PCIe Flit Mode is set, then CXL is in 256B flits
- * mode, otherwise it's 68B flits mode.
- */
-static bool cxl_pci_flit_256(struct pci_dev *pdev)
-{
-	u16 lnksta2;
-
-	pcie_capability_read_word(pdev, PCI_EXP_LNKSTA2, &lnksta2);
-	return lnksta2 & PCI_EXP_LNKSTA2_FLIT;
-}
-
 static int cxl_pci_ras_unmask(struct pci_dev *pdev)
 {
 	struct pci_host_bridge *host_bridge = pci_find_host_bridge(pdev->bus);