linux-kernel - Re: [RFC 3/5] cxl: Separate coherence from target type

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: <Zvv-xTGGTZee_fr8@PC2K9PVX.TheFacebook.com>
Date: Tue, 1 Oct 2024 09:53:09 -0400
From: Gregory Price <gourry@...rry.net>
To: Huang Ying <ying.huang@...el.com>
Cc: Dan Williams <dan.j.williams@...el.com>,
	Dave Jiang <dave.jiang@...el.com>, linux-cxl@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	Davidlohr Bueso <dave@...olabs.net>,
	Alison Schofield <alison.schofield@...el.com>,
	Vishal Verma <vishal.l.verma@...el.com>,
	Ira Weiny <ira.weiny@...el.com>,
	Alejandro Lucero <alucerop@....com>
Subject: Re: [RFC 3/5] cxl: Separate coherence from target type

On Wed, Sep 25, 2024 at 10:46:45AM +0800, Huang Ying wrote:
> Previously, target type (expander or accelerator) and
> coherence (HOSTONLY (HDM-H) or DEV (HDM-D/DB)) are synonym.  So target
> type is used to designate coherence too.  However, it's possible for
> expanders to use HDM-DB now.  So, we need to separate coherence from
> target type.
> 
> Accordingly, the HOSTONLY field of decoder ctrl
> register (CXL_HDM_DECODER0_CTRL_HOSTONLY) need to be set according to
> coherence.
> 
> The coherence of decoders can not be determined via target type too.
> So, accelerator/expander device drivers need to specify coherence
> explicitly via newly added coherence field in struct cxl_dev_state.
> 
> The coherence of each end points in a region need to be same.  So, the
> coherence of the first end point is recorded in struct region.  Which
> will be checked against the coherence of all other end points.
> 
> Signed-off-by: "Huang, Ying" <ying.huang@...el.com>
> Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>
> Cc: Dan Williams <dan.j.williams@...el.com>
> Cc: Davidlohr Bueso <dave@...olabs.net>
> Cc: Jonathan Cameron <jonathan.cameron@...wei.com>
> Cc: Dave Jiang <dave.jiang@...el.com>
> Cc: Alison Schofield <alison.schofield@...el.com>
> Cc: Vishal Verma <vishal.l.verma@...el.com>
> Cc: Ira Weiny <ira.weiny@...el.com>
> Cc: Alejandro Lucero <alucerop@....com>
> ---
>  drivers/cxl/core/hdm.c    | 22 +++++++++++++++-------
>  drivers/cxl/core/mbox.c   |  1 +
>  drivers/cxl/core/port.c   |  1 +
>  drivers/cxl/core/region.c | 37 ++++++++++++++++++++++++++++++++++---
>  drivers/cxl/cxl.h         |  9 +++++++++
>  drivers/cxl/cxlmem.h      | 11 +++++++++++
>  6 files changed, 71 insertions(+), 10 deletions(-)
> 

Reviewed-by: Gregory Price <gourry@...rry.net>

> @@ -1925,6 +1933,29 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  		return -ENXIO;
>  	}
>  
> +	/* Set the coherence of region to that of the first endpoint */
> +	if (cxlr->coherence == CXL_DECODER_INVALIDCOH) {
> +		unsigned long flags = cxlrd->cxlsd.cxld.flags;
> +		enum cxl_decoder_coherence coherence = cxled->cxld.coherence;
> +
> +		cxlr->coherence = coherence;
> +		if ((coherence == CXL_DECODER_HOSTONLYCOH &&
> +		     !(flags & CXL_DECODER_F_HOSTONLYCOH)) ||
> +		    (coherence == CXL_DECODER_DEVCOH &&
> +		     !(flags & CXL_DECODER_F_DEVCOH))) {

silly nit but my gut tells me we can make this less ugly.
Not a blocker though.

> +			dev_dbg(&cxlr->dev,
> +"%s:%s endpoint coherence: %d isn't supported by root decoder: %#lx\n",
> +				dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> +				coherence, flags);
> +			return -ENXIO;
> +		}