Re: [PATCH 12/18] cxl: Add helpers to calculate pci latency for the CXL device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/9/23 8:10 AM, Jonathan Cameron wrote:
On Wed, 8 Feb 2023 16:56:30 -0700
Dave Jiang <dave.jiang@xxxxxxxxx> wrote:

On 2/8/23 3:15 PM, Bjorn Helgaas wrote:
On Tue, Feb 07, 2023 at 01:51:17PM -0700, Dave Jiang wrote:


On 2/6/23 3:39 PM, Bjorn Helgaas wrote:
On Mon, Feb 06, 2023 at 01:51:10PM -0700, 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.
I guess you only care about the latency of a single link, not the
entire path?

I am adding each of the link individually together in the next
patch. Are you suggesting a similar function like
pcie_bandwidth_available() but for latency for the entire path?

Only a clarifying question.
+static int cxl_get_flit_size(struct pci_dev *pdev)
+{
+	if (cxl_pci_flit_256(pdev))
+		return 256;
+
+	return 66;

I don't know about the 66-byte flit format, maybe this part is
CXL-specific?

68-byte flit format. Looks like this is a typo from me.

This part must be CXL-specific, since I don't think PCIe mentions
68-byte flits.
+ * 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)
+{
+	u32 lnksta2;
+
+	pcie_capability_read_dword(pdev, PCI_EXP_LNKSTA2, &lnksta2);
+	return lnksta2 & BIT(10);

Add a #define for the bit.

ok will add.

AFAICT, the PCIe spec defines this bit, and it only indicates the link
is or will be operating in Flit Mode; it doesn't actually say anything
about how large the flits are.  I suppose that's because PCIe only
talks about 256B flits, not 66B ones?

Looking at CXL v1.0 rev3.0 6.2.3 "256B Flit Mode", table 6-4, it shows that
when PCIe Flit Mode is set, then CXL is in 256B flits mode, otherwise, it is
68B flits. So an assumption is made here regarding the flit side based on
the table.

So reading PCI_EXP_LNKSTA2 and extracting the Flit Mode bit is
PCIe-generic, but the interpretation of "PCIe Flit Mode not enabled
means 68-byte flits" is CXL-specific?

This sounds wrong, but I don't know quite how.  How would the PCI core
manage links where Flit Mode being cleared really means Flit Mode is
*enabled* but with a different size?  Seems like something could go
wrong there.

Looking at the PCIe base spec and the CXL spec, that seemed to be the
only way that implies the flit size for a CXL device as far as I can
tell. I've yet to find a good way to make that determination. Dan?

So a given CXL port has either trained up in:
* normal PCI (in which case all the normal PCI stuff applies) and we'll
   fail some of the other checks in the CXL driver never get hear here
   - I 'think' the driver will load for the PCI device to enable things
   like firmware upgrade, but we won't register the CXL Port devices
   that ultimately call this stuff.
   It's perfectly possible to have a driver that will cope with this
   but it's pretty meaningless for a lot of cxl type 3 driver.
* 68 byte flit (which was CXL precursor to PCI going flit based)
   Can be queried via CXL DVSEC Flex Bus Port Status CXL r3.0 8.2.1.3.3
* 256 byte flits (may or may not be compatible with PCIe ones as there
   are some optional latency optimizations)

So if the 68 byte flit is enabled the 256 byte one should never be and
CXL description is overriding the old PCIe

Hence I think we should have the additional check on the flex bus
dvsec even though it should be consistent with your assumption above.

So I'm trying to understand the CXL DVSEC Port status "68B flit and VH Enabled bit". If this bit is set, it means we are in 68B flit mode and VH mode? Do we just ignore RCH/RCD calculations since it doesn't support hotplug? Does this bit get cleared for 256B flit mode? It's not clear to me.


Hmm. That does raise a question of how we take the latency optimized
flits into account or indeed some of the other latency impacting things
that may or may not be running - IDE in it's various modes for example.

For latency optimized we can query relevant bit in the flex bus port status.
IDE info will be somewhere I guess though no idea if there is a way to
know the latency impacts.

Should we deal with latency optimized flits and IDE in a later step?


Jonathan




Bjorn




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux