linux-kernel - Re: [PATCH v2] cxl/acpi: Warn on mixed CXL VH and RCH/RCD Hierarchy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZoMq9TQoLTMaEPlJ@aschofie-mobl2>
Date: Mon, 1 Jul 2024 15:17:25 -0700
From: Alison Schofield <alison.schofield@...el.com>
To: "Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>
Cc: Davidlohr Bueso <dave@...olabs.net>,
	Jonathan Cameron <jonathan.cameron@...wei.com>,
	Dave Jiang <dave.jiang@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>,
	Ira Weiny <ira.weiny@...el.com>,
	Dan Williams <dan.j.williams@...el.com>,
	linux-kernel@...r.kernel.org, linux-cxl@...r.kernel.org
Subject: Re: [PATCH v2] cxl/acpi: Warn on mixed CXL VH and RCH/RCD Hierarchy

On Fri, Jun 28, 2024 at 07:48:07PM +0200, Fabio M. De Francesco wrote:
> Each Host Bridge instance has a corresponding CXL Host Bridge Structure
> (CHBS) ACPI table that identifies its capabilities. CHBS tables can be
> two types (CXL 3.1 Table 9-21): The PCIe Root Complex Register Block
> (RCRB) and CXL Host Bridge Component Registers (CHBCR).
> 
> If a Host Bridge is attached to a device that is operating in Restricted
> CXL Device Mode (RCD), BIOS publishes an RCRB with the base address of
> registers that describe its capabilities (CXL 3.1 sec. 9.11).
> 
> Instead, the new (CXL 2.0+) Component registers can only be accessed
> by means of a base address published with a CHBCR (CXL 3.1 sec. 9.12).
> 
> If an eRCD (a device that forces the host-bridge into CXL 1.1 Restricted
> CXL Host mode) is attached to a CXL 2.0+ Host-Bridge, the current CXL
> specification does not define a mechanism for finding CXL-2.0-only
> root-port component registers like HDM decoders and Extended Security
> capability.
> 
> An algorithm to locate a CHBCR associated with an RCRB, would be too
> invasive to land without some concrete motivation.
> 
> Therefore, just print a message to inform of unsupported config.
> 
> Count how many different CHBS "Version" types are detected by
> cxl_get_chbs_iter(). Then make cxl_get_chbs() print a warning if that sum
> is greater than 1.
> 
> Tested-by: Alison Schofield <alison.schofield@...el.com>
> Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@...ux.intel.com>
> ---
> 
> --- Changes for v2 ---
> 
> 	- Rewrite the Subject line (Alison)
> 	- Address a bug found by Alison while testing (thanks!)

'Address' a bug doesn't descibe what changed from v1 especially for
folks who reviewed v1.

I'm not asking you to rev this patch, just explain the saved_version
change.

Thanks
-- Alison

> 	- Add reference to CXL 3.1 Spec. (Alison)
> 	- Extend the commit messages by borrowing comments to v1 (Dan)
> 	- Rename field "count" to "nr_versions" (Alison)
> 	- Add brackets to oneline 'if' statement in precence of comments
> 	  (Dan)
> 
> --- Link to v1 ---
> 
> https://lore.kernel.org/linux-cxl/20240619125949.167936-1-fabio.m.de.francesco@linux.intel.com/
> 
>  drivers/cxl/acpi.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 571069863c62..f9035dbabb1c 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -482,6 +482,8 @@ struct cxl_chbs_context {
>  	unsigned long long uid;
>  	resource_size_t base;
>  	u32 cxl_version;
> +	int nr_versions;
> +	u32 saved_version;
>  };
>  
>  static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg,
> @@ -490,22 +492,31 @@ static int cxl_get_chbs_iter(union acpi_subtable_headers *header, void *arg,
>  	struct cxl_chbs_context *ctx = arg;
>  	struct acpi_cedt_chbs *chbs;
>  
> -	if (ctx->base != CXL_RESOURCE_NONE)
> -		return 0;
> -
>  	chbs = (struct acpi_cedt_chbs *) header;
>  
> -	if (ctx->uid != chbs->uid)
> +	if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 &&
> +	    chbs->length != CXL_RCRB_SIZE)
>  		return 0;
>  
> -	ctx->cxl_version = chbs->cxl_version;
>  	if (!chbs->base)
>  		return 0;
>  
> -	if (chbs->cxl_version == ACPI_CEDT_CHBS_VERSION_CXL11 &&
> -	    chbs->length != CXL_RCRB_SIZE)
> +	if (ctx->saved_version != chbs->cxl_version) {
> +		/*
> +		 * cxl_version cannot be overwritten before the next two
> +		 * checks, then use saved_version
> +		 */
> +		ctx->saved_version = chbs->cxl_version;
> +		ctx->nr_versions++;
> +	}
> +
> +	if (ctx->base != CXL_RESOURCE_NONE)
> +		return 0;
> +
> +	if (ctx->uid != chbs->uid)
>  		return 0;
>  
> +	ctx->cxl_version = chbs->cxl_version;
>  	ctx->base = chbs->base;
>  
>  	return 0;
> @@ -529,10 +540,19 @@ static int cxl_get_chbs(struct device *dev, struct acpi_device *hb,
>  		.uid = uid,
>  		.base = CXL_RESOURCE_NONE,
>  		.cxl_version = UINT_MAX,
> +		.saved_version = UINT_MAX,
>  	};
>  
>  	acpi_table_parse_cedt(ACPI_CEDT_TYPE_CHBS, cxl_get_chbs_iter, ctx);
>  
> +	if (ctx->nr_versions > 1) {
> +		/*
> +		 * Disclaim eRCD support given some component register may
> +		 * only be found via CHBCR
> +		 */
> +		dev_info(dev, "Unsupported platform config, mixed Virtual Host and Restricted CXL Host hierarchy.");
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.45.2
>