public inbox for linux-cxl@vger.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Dave Jiang <dave.jiang@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <dan.j.williams@intel.com>,
	<ira.weiny@intel.com>, <vishal.l.verma@intel.com>,
	<alison.schofield@intel.com>, <dave@stgolabs.net>
Subject: Re: [PATCH v7 3/3] cxl: Add documentation to explain the shared link bandwidth calculation
Date: Tue, 27 Aug 2024 17:06:40 +0100	[thread overview]
Message-ID: <20240827170640.000007ab@Huawei.com> (raw)
In-Reply-To: <20240710222716.797267-4-dave.jiang@intel.com>

On Wed, 10 Jul 2024 15:24:02 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Create a kernel documentation to describe how the CXL shared upstream
> link bandwidth is calculated.
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Oops. Missed this previously.  A few minor things inline.

J
> ---
>  .../driver-api/cxl/access-coordinates.rst     | 90 +++++++++++++++++++
>  Documentation/driver-api/cxl/index.rst        |  1 +
>  MAINTAINERS                                   |  1 +
>  3 files changed, 92 insertions(+)
>  create mode 100644 Documentation/driver-api/cxl/access-coordinates.rst
> 
> diff --git a/Documentation/driver-api/cxl/access-coordinates.rst b/Documentation/driver-api/cxl/access-coordinates.rst
> new file mode 100644
> index 000000000000..973e63872f06
> --- /dev/null
> +++ b/Documentation/driver-api/cxl/access-coordinates.rst
> @@ -0,0 +1,90 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +.. include:: <isonum.txt>
> +
> +==================================
> +CXL Access Coordinates Computation
> +==================================
> +
> +Shared Upstream Link Calculation
> +================================
> +For certain CXL region construction with endpoints behind CXL switches (SW) or
> +Root Ports (RP), there is the possibility of the total bandwdith for all
spell check. bandwidth
> +the endpoints behind a switch being more than the switch upstream link.

Could also be the Generic Port bit of the topology. Mind you I'm still failing
to come up with text for the qemu GP Documentation that the reviewers can follow
so maybe that's just too hard to describe here.  Could use weasel words.

"A similar situation can occur within the host, upstream of the root ports."


> +The CXL driver performs an additional pass after all the targets have
> +arrived for a region in order to recalculate the bandwidths with possible
> +upstream link being a limiting factor in mind.
> +
> +The algorithm assumes the configuration is a symmetric topology as that
> +maximizes performance. When asymmetric topology is detected, the calculation
> +is aborted when such topology is detected. An asymmetric topology is detected

is detected is duplicated.

> +during topology walk where the number of RPs detected as a grandparent is not
> +equal to the number of devices iterated in the same iteration loop.

Maybe make the point that asymmetric in terms of only properties of devices
is not detected. It just uses the first one I think?
> +
> +There can be multiple switches under a RP. There can be multiple RPs under
> +a CXL Host Bridge (HB). There can be multiple HBs under a CXL Fixed Memory
> +Window Structure (CFMWS).
> +
> +An example hierarchy:
> +
> +                CFMWS 0
> +                  |
> +         _________|_________
> +        |                   |
> +    ACPI0017-0          ACPI0017-1
> + GP0/HB0/ACPI0016-0   GP1/HB1/ACPI0016-1
> +    |          |        |           |
> +   RP0        RP1      RP2         RP3
> +    |          |        |           |
> +  SW 0       SW 1     SW 2        SW 3
> +  |   |      |   |    |   |       |   |
> + EP0 EP1    EP2 EP3  EP4  EP5    EP6 EP7
> +
> +Computation for the example hierarchy:
> +
> +Min (GP0 to CPU BW,
> +     Min(SW 0 Upstream Link to RP0 BW,
> +         Min(SW0SSLBIS for SW0DSP0 (EP0), EP0 DSLBIS, EP0 Upstream Link) +
> +         Min(SW0SSLBIS for SW0DSP1 (EP1), EP1 DSLBIS, EP1 Upstream link)) +
> +     Min(SW 1 Upstream Link to RP1 BW,
> +         Min(SW1SSLBIS for SW1DSP0 (EP2), EP2 DSLBIS, EP2 Upstream Link) +
> +         Min(SW1SSLBIS for SW1DSP1 (EP3), EP3 DSLBIS, EP3 Upstream link))) +
> +Min (GP1 to CPU BW,
> +     Min(SW 2 Upstream Link to RP2 BW,
> +         Min(SW2SSLBIS for SW2DSP0 (EP4), EP4 DSLBIS, EP4 Upstream Link) +
> +         Min(SW2SSLBIS for SW2DSP1 (EP5), EP5 DSLBIS, EP5 Upstream link)) +
> +     Min(SW 3 Upstream Link to RP3 BW,
> +         Min(SW3SSLBIS for SW3DSP0 (EP6), EP6 DSLBIS, EP6 Upstream Link) +
> +         Min(SW3SSLBIS for SW3DSP1 (EP7), EP7 DSLBIS, EP7 Upstream link))))
> +
> +
> +The calculation starts at cxl_region_shared_upstream_perf_update(). A xarray
> +is created to collect all the endpoint bandwidths via the
> +cxl_endpoint_gather_bandwidth() function. The min() of bandwidth from the
> +endpoint CDAT and the upstream link bandwidth is calculated. If the endpoint
> +has a CXL switch as a parent, then min() of calculated bandwidth and the
> +bandwidth from the SSLBIS for the switch downstream port that is associated
> +with the endpoint is calculated. The final bandwidth is stored in a
> +'struct cxl_perf_ctx' in the xarray indexed by a device pointer. If the
> +endpoint is direct attached to a root port (RP), the device pointer would be a
> +RP device. If the endpoint is behind a switch, the device pointer would be the
an RP 
> +upstream device of the parent switch.
> +
> +At the next stage, the code attempts to walk through one or more switches if
the code walks through
(I doubt it ever fails and if it does that's detail you don't need here)
> +they exist in the topology. For endpoints directly attached to RPs, this
> +step is skipped. If there is another switch upstream, the code takes the min()
> +of the current gathered bandwidth and the upstream link bandwidth. If there's
> +a switch upstream, then the SSLBIS of the upstream switch.
> +
> +Once the topology walk reaches the RP, whether it's direct attached endpoints
> +or walking through the switch(es), cxl_rp_gather_bandwidth() is called. At
> +this point all the bandwidths are aggregated per each host bridge, which is
> +also the index for the resulting xarray.
> +
> +The next step is to take the min() of the per host bridge bandwidth and the
> +bandwidth from the Generic Port (GP). The bandwidths for the GP is retrieved
> +via ACPI tables SRAT/HMAT. The min bandwidth are aggregated under the same
> +ACPI0017 device to form a new xarray.
> +
> +Finally, the cxl_region_update_bandwidth() is called and the aggregated
> +bandwidth from all the members of the last xarray is updated for the
> +access coordinates residing in the cxl region (cxlr) context.


  reply	other threads:[~2024-08-27 16:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-10 22:23 [PATCH v7 0/3] cxl: Region bandwidth calculation for targets with shared upstream link Dave Jiang
2024-07-10 22:24 ` [PATCH v7 1/3] cxl: Preserve the CDAT access_coordinate for an endpoint Dave Jiang
2024-07-10 22:24 ` [PATCH v7 2/3] cxl: Calculate region bandwidth of targets with shared upstream link Dave Jiang
2024-07-11  1:39   ` Alison Schofield
2024-07-11 16:00     ` Dave Jiang
2024-07-10 22:24 ` [PATCH v7 3/3] cxl: Add documentation to explain the shared link bandwidth calculation Dave Jiang
2024-08-27 16:06   ` Jonathan Cameron [this message]
2024-08-27 22:38     ` Dave Jiang
2024-08-28  9:00       ` Jonathan Cameron
2024-08-28 15:35         ` Dave Jiang
2024-08-28 16:12           ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240827170640.000007ab@Huawei.com \
    --to=jonathan.cameron@huawei.com \
    --cc=alison.schofield@intel.com \
    --cc=dan.j.williams@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=ira.weiny@intel.com \
    --cc=linux-cxl@vger.kernel.org \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox