Received-SPF: pass (google.com: domain of linux-kernel+bounces-63505-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 13 Feb 2024 13:30:24 +0100
From: Robert Richter <rrichter@amd.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Dave Jiang <dave.jiang@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Ira Weiny <ira.weiny@intel.com>, linux-cxl@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] cxl/pci: Fix disabling CXL memory for zero-based
 addressing
Message-ID: <Zctg4Lx_y_hbChUW@rric.localdomain>
References: <20240209193451.163564-1-rrichter@amd.com>
 <65c68969903b1_afa429460@dwillia2-xfh.jf.intel.com.notmuch>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <65c68969903b1_afa429460@dwillia2-xfh.jf.intel.com.notmuch>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?vtJRLDTXdhC3OioZbuexmv/zyhb8z06nQ6pVd12YSp9UdJVf5PGFwdkOhIXQ?=
 =?us-ascii?Q?H00ZrZsh3D7rO4X3iTzyCyCCobpYxSwaOuKgtjHD+iqx8A9F6NKethY1OymR?=
 =?us-ascii?Q?AY6DVTIIRO0dB7XsStAoo5LXoBh1h7qQNH1mFT38tZjPrmYb3dHhJ6BiSDXx?=
 =?us-ascii?Q?6N5kLGImwIYU0lXHjbSqqj3jFhHx4pwqRcIdVJV6lVZviLcC73k0Qxd8Up8W?=
 =?us-ascii?Q?CohX/0LNlgHaNiwtjH6KWMlYrB8dsfKRM/w/RguX8k6XMw3j5KkE4WQ1bKrY?=
 =?us-ascii?Q?UjJNaub1VF6jkumV2Ja/fU4696NgOF+iAIoT5SJL+E0lk3fEVqKf984YchPR?=
 =?us-ascii?Q?LmzaYiSbeaFcGNPwPuMtbeT9m1lH2DeX6JhZE459mtQTjVOJsMNu6+zemIJH?=
 =?us-ascii?Q?PFswPGHid0Uy/QXFmkQ91yE77GULBbJ7vM6ncPS2lRwzRhbSYb5Aq+BX9S5s?=
 =?us-ascii?Q?0lPIBgiCHb6m28wJdlMI/wDnx1ajdwJxWrHrpSvRVg7KLgUALLxHk070UjlM?=
 =?us-ascii?Q?9Wb55RExMJsYkuy283rcadSmYwaS2HUZ7QRKGGBeXerID3Zby1E/uSfDQKJf?=
 =?us-ascii?Q?Pb9x5Jvp0avGoURIHV9KNCu6c9MX87CLwQMC9grP7p6cx79jTn1QynYlkTFB?=
 =?us-ascii?Q?gGz1f88GEROg9M2pXtrDoUG/b6NfcVsMLlxFboBHewDl91oXQyqnYLdoQTBP?=
 =?us-ascii?Q?62RY4Yr4oc/5mZYC5aLomqPr6RNptBLEumdkRbTMiu3OY9kFEkZ9UAdToFcu?=
 =?us-ascii?Q?g5WYztrNzPVfsA8uFC8H2fj6udTFMXF8vqIi75vAROQWlnVZju4XzuenMjyc?=
 =?us-ascii?Q?tPAgroTnAR2H+cdj99cx0BDX9KwapWyQQVSC3I4pivhx0urezcwIiuMpMt5l?=
 =?us-ascii?Q?sCsZ0GC0RtbHWj8EtXgrQe6NhK6bJ0UPzH+9iXKNj8Vwfhn8QXkjt0ctzmYj?=
 =?us-ascii?Q?gApMiL602H86Swhcp7KrA2iQzHUorwGT4MuMnw//9AMzuGFn4ZwAFFdU+/dS?=
 =?us-ascii?Q?RMbTd9BFZuFT9TeVUscMR5e4djLQsHtVwdqpBhGPB3DVz8ddlvt9eXIV/Izv?=
 =?us-ascii?Q?b7SmZPLq1zl+U0shkGTPzlueyabUoWoVzHTwh9fmxSNpk6Zm7zTp1jE3Gk4G?=
 =?us-ascii?Q?oLjclyvEPGGG5OxQ0cNfH2bn3Xu2mSyMzRpsCWLShc05MtBrDYuZPCjNn92K?=
 =?us-ascii?Q?BhOtWZxp/+RfU0GTnuM6qVunVpllzlm0F+JB27QeRg6bsiatk3Ca5vt7qvhi?=
 =?us-ascii?Q?oBGh6wAXJkaOejT+xeLKOT/9h2ZO9K4+n4eb0phLJDOYUBmjsyG30D0BLLEi?=
 =?us-ascii?Q?9RvSlj8TThIyXbhDBmc1rE43D5Yp12EBvmlTh8XxCfGXyIPeduNBRo/kuZkj?=
 =?us-ascii?Q?KclIVBy3gysW3oZi69+LpvxuemxZpf+DdXiquLIHZGG/HirZfK3UIFczTsX1?=
 =?us-ascii?Q?gxbpyK26XwljavWXN6AuajCpgF3dpD98I8CibkOAgrvk1pkhj9PQE8GW2Wb9?=
 =?us-ascii?Q?JQ22/6AI3sWZ+fO1JpKA2pNrfe7xkOHn3y2rKSHmWmPaBT9oC+pe0wwGHVG8?=
 =?us-ascii?Q?ezrHCl5GLHbpAkCLNTobz6SL43NNfgFTMxWNfPMo?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2b94c4b3-4cbe-4c27-c6c6-08dc2c8f9054
X-MS-Exchange-CrossTenant-AuthSource: CYYPR12MB8750.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Feb 2024 12:30:29.5925
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9XsgxxJ4cncVeIm0pFVpYjdEg8puqd04NTpvOj4M/vZj0Yp0quleAyHlRvrbNiPEnEOzCQC/xfDBDcCP1ZLXTQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR12MB6315

Dan,

On 09.02.24 12:22:01, Dan Williams wrote:
> Robert Richter wrote:

> > diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> > index 569354a5536f..3a36a2f0c94f 100644
> > --- a/drivers/cxl/core/pci.c
> > +++ b/drivers/cxl/core/pci.c
> > @@ -466,6 +466,18 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
> >  	for (i = 0, allowed = 0; info->mem_enabled && i < info->ranges; i++) {
> >  		struct device *cxld_dev;
> >  
> > +		/*
> > +		 * Handle zero-based hardware addresses
> > +		 */
> > +		if (!info->dvsec_range[i].start &&
> > +		    info->dvsec_range[i].end != CXL_RESOURCE_NONE &&
> > +		    info->dvsec_range[i].end) {
> > +			dev_dbg(dev, "Zero-based hardware range found [%#llx - %#llx]\n",
> > +				info->dvsec_range[i].start, info->dvsec_range[i].end);
> > +			allowed++;
> > +			continue;
> > +		}
> > +
> 
> I am not comfortable with this. It should be checking a platform
> specific quirk, or similar for the possibility of HPA != SPA. The
> entirety of the Linux CXL subsystem is built on the assumption that HPA
> == SPA, and if a platform wants to inject an offset between those Linux
> needs some way to enumerate that it is running in that new world. Yes,
> nothing in the CXL specification precludes HPA != SPA, but Linux has
> long since shipped the opposite assumption.

this check prevents the memory from disabling an enabled decoder. So it
just keeps everything as it comes out of firmware.

Can you explain the motivation why active memory is disabled? This may
take system memory offline and could lead to a kernel hang. The same
could happen if the CEDT is broken or just missing (it is not
mandatory for 1.1). Such systems just die when booting. So the check
to take memory offline should be changed in a way that it will be safe
to disable it.

A platform check would fix that only for certain systems.

Thanks,

-Robert