diff mbox series

[v7,3/3] cxl: Add documentation to explain the shared link bandwidth calculation

Message ID 20240710222716.797267-4-dave.jiang@intel.com
State Superseded
Headers show
Series cxl: Region bandwidth calculation for targets with shared upstream link | expand

Commit Message

Dave Jiang July 10, 2024, 10:24 p.m. UTC
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>
---
 .../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

Comments

Jonathan Cameron Aug. 27, 2024, 4:06 p.m. UTC | #1
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.
Dave Jiang Aug. 27, 2024, 10:38 p.m. UTC | #2
On 8/27/24 9:06 AM, Jonathan Cameron wrote:
> 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.

I don't follow here.

> 
>> +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?

I also don't follow here.

I also wonder if I should use "unbalanced" topology rather than "unsymmetric"?

DJ

>> +
>> +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.
>
Jonathan Cameron Aug. 28, 2024, 9 a.m. UTC | #3
On Tue, 27 Aug 2024 15:38:27 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> On 8/27/24 9:06 AM, Jonathan Cameron wrote:
> > 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.  
> 
> I don't follow here.
> 
>+ When asymmetric topology is detected, the calculation
                             ^^^^^^^^^^^
>+is aborted when such topology is detected.
                                ^^^^^^^^^^^^
Delete the second one.

> >   
> >> +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?  
> 
> I also don't follow here.
> 
> I also wonder if I should use "unbalanced" topology rather than "unsymmetric"?

Difference between detecting a topology with different number of links and one
where say one type 3 device in an interleave set has lower bandwidth in it's CDAT.

IIRC this does the first, but not the second.

> 
> DJ
Dave Jiang Aug. 28, 2024, 3:35 p.m. UTC | #4
On 8/28/24 2:00 AM, Jonathan Cameron wrote:
> On Tue, 27 Aug 2024 15:38:27 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
>> On 8/27/24 9:06 AM, Jonathan Cameron wrote:
>>> 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.  
>>
>> I don't follow here.
>>
>> + When asymmetric topology is detected, the calculation
>                              ^^^^^^^^^^^
>> +is aborted when such topology is detected.
>                                 ^^^^^^^^^^^^
> Delete the second one.

Thanks. The brain does funny tricks sometimes when reading the stuff you write yourself. 

> 
>>>   
>>>> +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?  
>>
>> I also don't follow here.
>>
>> I also wonder if I should use "unbalanced" topology rather than "unsymmetric"?
> 
> Difference between detecting a topology with different number of links and one
> where say one type 3 device in an interleave set has lower bandwidth in it's CDAT.
> 
> IIRC this does the first, but not the second.

I think here we attempt to detect whether one root port has different level of switch(s) vs another. For example a topology configuration like RP0 is direct attached and RP1 has a switch underneath. 

> 
>>
>> DJ
>
Jonathan Cameron Aug. 28, 2024, 4:12 p.m. UTC | #5
On Wed, 28 Aug 2024 08:35:07 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> On 8/28/24 2:00 AM, Jonathan Cameron wrote:
> > On Tue, 27 Aug 2024 15:38:27 -0700
> > Dave Jiang <dave.jiang@intel.com> wrote:
> >   
> >> On 8/27/24 9:06 AM, Jonathan Cameron wrote:  
> >>> 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.    
> >>
> >> I don't follow here.
> >>
> >> + When asymmetric topology is detected, the calculation  
> >                              ^^^^^^^^^^^  
> >> +is aborted when such topology is detected.  
> >                                 ^^^^^^^^^^^^
> > Delete the second one.  
> 
> Thanks. The brain does funny tricks sometimes when reading the stuff you write yourself. 
> 
> >   
> >>>     
> >>>> +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?    
> >>
> >> I also don't follow here.
> >>
> >> I also wonder if I should use "unbalanced" topology rather than "unsymmetric"?  
> > 
> > Difference between detecting a topology with different number of links and one
> > where say one type 3 device in an interleave set has lower bandwidth in it's CDAT.
> > 
> > IIRC this does the first, but not the second.  
> 
> I think here we attempt to detect whether one root port has different level of switch(s) vs another. For example a topology configuration like RP0 is direct attached and RP1 has a switch underneath. 

I'd just add a line to say that we assume that
subtle asymmetry in properties doesn't happen, so compute
bandwidth on assumption all paths to EPs are equivalent.

Jonathan

> 
> >   
> >>
> >> DJ  
> >   
>
diff mbox series

Patch

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
+the endpoints behind a switch being more than the switch upstream link.
+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
+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.
+
+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
+upstream device of the parent switch.
+
+At the next stage, the code attempts to walk through one or more switches if
+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.
diff --git a/Documentation/driver-api/cxl/index.rst b/Documentation/driver-api/cxl/index.rst
index 036e49553542..9b0e82a31d89 100644
--- a/Documentation/driver-api/cxl/index.rst
+++ b/Documentation/driver-api/cxl/index.rst
@@ -8,5 +8,6 @@  Compute Express Link
    :maxdepth: 1
 
    memory-devices
+   access-coordinates
 
 .. only::  subproject and html
diff --git a/MAINTAINERS b/MAINTAINERS
index 2ca8f35dfe03..2b3bfc114140 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5484,6 +5484,7 @@  M:	Ira Weiny <ira.weiny@intel.com>
 M:	Dan Williams <dan.j.williams@intel.com>
 L:	linux-cxl@vger.kernel.org
 S:	Maintained
+F:	Documentation/driver-api/cxl/
 F:	drivers/cxl/
 F:	include/linux/einj-cxl.h
 F:	include/linux/cxl-event.h