Re: [PATCH v2 3/3] cxl/region: Disallow region granularity != window granularity

[Ira Weiny <ira.weiny@intel.com>]

On Fri, Aug 05, 2022 at 01:27:51PM -0700, Dan Williams wrote:
> The endpoint decode granularity must be <= the window granularity
> otherwise capacity in the endpoints is lost in the decode. Consider an
> attempt to have a region granularity of 512 with 4 devices within a
> window that maps 2 host bridges at a granularity of 256 bytes:
> 
> HPA	DPA Offset	HB	Port	EP
> 0x0	0x0		0	0	0
> 0x100	0x0		1	0	2
> 0x200	0x100		0	0	0
> 0x300	0x100		1	0	2
> 0x400	0x200		0	1	1
> 0x500	0x200		1	1	3
> 0x600	0x300		0	1	1
> 0x700	0x300		1	1	3
> 0x800	0x400		0	0	0
> 0x900	0x400		1	0	2
> 0xA00	0x500		0	0	0
> 0xB00	0x500		1	0	2
> 
> Notice how endpoint0 maps HPA 0x0 and 0x200 correctly, but then HPA
> 0x800 results in DPA 0x200 to 0x400 on endpoint0 being not skipped.
> 
> Fix this by restricing the region granularity to be equal to the window
> granularity resulting in the following for a x4 region under a x2 window
> at a granularity of 256.
> 
> HPA	DPA Offset	HB	Port	EP
> 0x0	0x0		0	0	0
> 0x100	0x0		1	0	2
> 0x200	0x0		0	1	1
> 0x300	0x0		1	1	3
> 0x400	0x100		0	0	0
> 0x500	0x100		1	0	2
> 0x600	0x100		0	1	1
> 0x700	0x100		1	1	3
> 
> Not that it ever made practical sense to support region granularity >
> window granularity. The window rotates host bridges causing endpoints to
> never see a consecutive stream of requests at the desired granularity
> without breaks to issue cycles to the other host bridge.
> 
> Fixes: 80d10a6cee05 ("cxl/region: Add interleave geometry attributes")
> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Vishal Verma <vishal.l.verma@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/region.c |   13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 641bc6344a4a..401148016978 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -395,13 +395,14 @@ static ssize_t interleave_granularity_store(struct device *dev,
>  		return rc;
>  
>  	/*
> -	 * Disallow region granularity less than root granularity to
> -	 * simplify the implementation. Otherwise, region's with a
> -	 * granularity less than the root interleave result in needing
> -	 * multiple endpoints to support a single slot in the
> -	 * interleave.
> +	 * When the host-bridge is interleaved, disallow region granularity !=
> +	 * root granularity. Regions with a granularity less than the root
> +	 * interleave result in needing multiple endpoints to support a single
> +	 * slot in the interleave (possible to suport in the future). Regions
> +	 * with a granularity greater than the root interleave result in invalid
> +	 * DPA translations (invalid to support).
>  	 */
> -	if (val < cxld->interleave_granularity)
> +	if (cxld->interleave_ways > 1 && val != cxld->interleave_granularity)
>  		return -EINVAL;
>  
>  	rc = down_write_killable(&cxl_region_rwsem);
>