diff mbox series

cxl/region: Allow out of order assembly of autodiscovered regions

Message ID 20240126045446.1750854-1-alison.schofield@intel.com
State Superseded
Headers show
Series cxl/region: Allow out of order assembly of autodiscovered regions | expand

Commit Message

Alison Schofield Jan. 26, 2024, 4:54 a.m. UTC
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>
---

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(-)


base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d

Comments

Dave Jiang Feb. 22, 2024, 6:11 p.m. UTC | #1
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
diff mbox series

Patch

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),