Re: [PATCH] cxl/region: Allow out of order assembly of autodiscovered regions

[Dave Jiang <dave.jiang@intel.com>]

On 1/25/24 9:54 PM, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> Autodiscovered regions can fail to assemble if they are not discovered
> in HPA decode order. The user will see failure messages like:
> 
> [] cxl region0: endpoint5: HPA order violation region1
> [] cxl region0: endpoint5: failed to allocate region reference
> 
> The check that is causing the failure helps the CXL driver enforce
> a CXL spec mandate that decoders be committed in HPA order. The
> check is needless for autodiscovered regions since their decoders
> are already programmed. Trying to enforce order in the assembly of
> these regions is useless because they are assembled once all their
> member endpoints arrive, and there is no guarantee on the order in
> which endpoints are discovered during probe.
> 
> Keep the existing check, but for autodiscovered regions, allow the
> out of order assembly after a sanity check that the lowered numbered
> decoder has the lower HPA starting address.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> Tested-by: Neha Agrawal <neha.agrawal@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

s/autodiscovered/auto-discovered/?

> ---
> 
> Changes since RFC:
> - Declare auto_order_ok() as static (lkp)
> - Add Tested-by tag (Neha)
> Link to RFC: https://lore.kernel.org/linux-cxl/20240113050421.1622533-1-alison.schofield@intel.com/
> 
> 
>  drivers/cxl/core/region.c | 34 +++++++++++++++++++++++++++++++++-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 0f05692bfec3..f6a49fd01ae9 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -753,6 +753,37 @@ static struct cxl_decoder *cxl_region_find_decoder(struct cxl_port *port,
>  	return to_cxl_decoder(dev);
>  }
>  
> +static bool auto_order_ok(struct cxl_port *port, struct cxl_region *cxlr_a,
> +			  struct cxl_region *cxlr_b)
> +{
> +	struct cxl_region_ref *cxl_rr;
> +	struct cxl_decoder *cxld_a, *cxld_b;
> +
> +	/*
> +	 * Allow the out of order assembly of auto-discovered regions as
> +	 * long as correct decoder programming order can be verified.
> +	 *
> +	 * Per CXL Spec 3.1 8.2.4.20.12 Committing Decoder Programming,
> +	 * software must commit decoders in HPA order. Therefore it is
> +	 * sufficient to sanity check that the lowered number decoder
> +	 * has the lower HPA starting address.
> +	 */
> +	if (!test_bit(CXL_REGION_F_AUTO, &cxlr_a->flags))
> +		return false;
> +
> +	cxld_a = cxl_region_find_decoder(port, cxlr_a);
> +	cxl_rr = cxl_rr_load(port, cxlr_b);
> +	cxld_b = cxl_rr->decoder;
> +
> +	if (cxld_b->id > cxld_a->id) {
> +		dev_dbg(&cxlr_a->dev,
> +			"allow out of order region ref alloc\n");
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
>  static struct cxl_region_ref *alloc_region_ref(struct cxl_port *port,
>  					       struct cxl_region *cxlr)
>  {
> @@ -767,7 +798,8 @@ static struct cxl_region_ref *alloc_region_ref(struct cxl_port *port,
>  		if (!ip->res)
>  			continue;
>  
> -		if (ip->res->start > p->res->start) {
> +		if (ip->res->start > p->res->start &&
> +		    (!auto_order_ok(port, cxlr, iter->region))) {
>  			dev_dbg(&cxlr->dev,
>  				"%s: HPA order violation %s:%pr vs %pr\n",
>  				dev_name(&port->dev),
> 
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d