Re: [PATCH 5/5] cxl/region: Constrain region granularity scaling factor

[Dan Williams <dan.j.williams@intel.com>]

Jonathan Cameron wrote:
> On Fri, 22 Jul 2022 17:56:20 -0700
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Consider the scenario where a platform provides for a x2 host-bridge
> > interleave at a granularity of 256 bytes.
> Hi Dan,
> 
> Terminology not clear to me.  Is this interleave across 2 host bridges?

Yes.

> 
> I'm lost in the explanation.
> 
> > The only permitted region
> > granularities for that configuration are 256 or 512. Anything larger
> > than 512 results in unmapped capacity within a given decoder. Also, if
> > the region granularity is 512 then the interleave_ways for the region
> > must be 4 to keep the scaling matched.
> > 
> > Here are the translations for the first (4) 256-byte blocks where an
> > endpoint decoder is configured for a 512-byte granularity:
> > 
> > Block[0] => HB0 => DPA: 0
> > Block[1] => HB1 => DPA: 0
> > Block[2] => HB0 => DPA: 0
> > Block[3] => HB1 => DPA: 0
> > 
> > In order for those translations to not alias the region interleave ways
> > must be 4 resulting in:
> > 
> > Block[0] => HB0 => Dev0 => DPA: 0
> > Block[1] => HB1 => Dev1 => DPA: 0
> > Block[2] => HB0 => Dev2 => DPA: 0
> > Block[3] => HB1 => Dev3 => DPA: 0
> 
> Block[4] => HBA0 => Dev0 => DPA: 512 ?
> 
> which means we are only using alternate 256 blocks of the EP.  Does that make
> sense?

Yes, but I found a subtle bug a bug in my logic. Here is the output from
my spreadsheet calculating the DPA_Offset for the first 8 blocks when
the topology is "x2 host-bridge @ 256 => x2 endpoint @ 512"

Address	HB Index	HPA Offset	DPA Offset
0x0	0		0x0		0x0
0x100	1		0x100		0x0
0x200	0		0x200		0x0
0x300	1		0x300		0x0
0x400	0		0x400		0x200
0x500	1		0x500		0x200
0x600	0		0x600		0x200
0x700	1		0x700		0x200
0x800	0		0x800		0x400

So we have 2 endpoints at 4 DPA_Offset == 0, so there is aliasing.
However, the bug in my logic is that this fix for this is not:

"...if the region granularity is 512 then the interleave_ways for the
region must be 4 to keep the scaling matched."

...it is that the number of *targets* in the interleave must be 4 with
that split handled by the host bridge, and leave the endpoint interleave
ways setting at 2. So, in general to support region-granularity >
root-granularity, the host-bridges and / or switches in the path must
scale the interleave.

---
"x4 region @ 512 granularity under an x2 window @ 256"

 ┌───────────────────────────────────┬──┐
 │WINDOW0                            │x2│
 └─────────┬─────────────────┬───────┴──┘
           │                 │
 ┌─────────▼────┬──┐  ┌──────▼───────┬──┐
 │HB0           │x2│  │HB1           │x2│
 └──┬────────┬──┴──┘  └─┬─────────┬──┴──┘
    │        │          │         │
 ┌──▼─┬──┐ ┌─▼──┬──┐  ┌─▼──┬──┐ ┌─▼──┬──┐
 │DEV0│x2│ │DEV1│x2│  │DEV2│x2│ │DEV3│x2│
 └────┴──┘ └────┴──┘  └────┴──┘ └────┴──┘
---

---
"x4 region @ 256 granularity under an x2 window @ 256"

 ┌───────────────────────────────────┬──┐
 │WINDOW0                            │x2│
 └─────────┬─────────────────┬───────┴──┘
           │                 │
 ┌─────────▼────┬──┐  ┌──────▼───────┬──┐
 │HB0           │x2│  │HB1           │x2│
 └──┬────────┬──┴──┘  └─┬─────────┬──┴──┘
    │        │          │         │
 ┌──▼─┬──┐ ┌─▼──┬──┐  ┌─▼──┬──┐ ┌─▼──┬──┐
 │DEV0│x4│ │DEV1│x4│  │DEV2│x4│ │DEV3│x4│
 └────┴──┘ └────┴──┘  └────┴──┘ └────┴──┘
---

...i.e. it is not always the case that the endpoint interleave ways
setting matches the number of devices in the set.

In the interests of settling the code for v6.0 merge the smallest fix is
to just not allow region granularity != window granularity
configurations for now. Then for v6.1 the algorithm can be expanded to
take switching into account for endpoint intereleave geometry.

> > ...not 2, resulting in:
> > 
> > Block[0] => HB0 => Dev0 => DPA: 0
> > Block[1] => HB1 => Dev1 => DPA: 0
> > Block[2] => HB0 => Dev0 => DPA: 0 !
> > Block[3] => HB1 => Dev1 => DPA: 0 !
> 
> > 
> > Given tooling is already being built around this ABI allow for
> > granularity and ways to be set in either order and validate the
> > combination once both are set.
> > 
> > Reported-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> Typo inline.

Thanks, but moot now that I'm dropping this in favor of the safe fix,
and push ongoing improvements to the next merge window.