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

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

Dan Williams wrote:
> 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.

No, that's still broken. Even with the host-bridge scaling the interleave it
still results in stranded capacity at the endpoints. So, in general region IG must
be <= Window IG. The current implementation is already blocking the region IG <
Window IG case, so forcing region IG to be equal to Window IG is the
solution for now.