Received-SPF: pass (google.com: domain of linux-kernel+bounces-65019-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 14 Feb 2024 11:06:38 +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: <ZcyQroz_4XZAgbv3@rric.localdomain>
References: <20240209193451.163564-1-rrichter@amd.com>
 <65c68969903b1_afa429460@dwillia2-xfh.jf.intel.com.notmuch>
 <Zctg4Lx_y_hbChUW@rric.localdomain>
 <65cbb787e73c_29b129432@dwillia2-mobl3.amr.corp.intel.com.notmuch>
 <Zcu-cVG9deqfwdiV@rric.localdomain>
 <65cbc6ec7a0f6_29b129456@dwillia2-mobl3.amr.corp.intel.com.notmuch>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <65cbc6ec7a0f6_29b129456@dwillia2-mobl3.amr.corp.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?jOXLOXmieeBCCwtOPNQHdbCGmMSiX8ZjiTwdsX6fPZz6qvn3j2pKqS5R+eG7?=
 =?us-ascii?Q?dcFiWUNpWgfvXBKkPjZIj8oDlt0VEp0VyOhkpUdsyUmjZVyXNIkyeALJmA+b?=
 =?us-ascii?Q?DSvGUAGeFjencS9yOqCrCUOax9lvGxsCFeJ20z8Jbr4JnjDzAf5edBotdP0K?=
 =?us-ascii?Q?uFHdSQepvaUtw88+r8nYe/uzm73nIX+gVL97zBDsjwDLGdjLc/8JYPdyuP7t?=
 =?us-ascii?Q?ZbK3Y08YZJUL4SMZxoPvLwjexTtJXyC2GMpBj78v23fdcdcTvMUvvxLHDBKa?=
 =?us-ascii?Q?CtnIHJWu8e9pTsEoDlCXcDyES1313HaF6lRR5vKqDjT+gd9su2HAjRbDcK0O?=
 =?us-ascii?Q?dLQJUgMMqywM/GvWMp7deKGcg7u3Ql8xarr9iPm47x2t3aXKAmEezXcdbL5G?=
 =?us-ascii?Q?KfNJn01/wrEbdNG4mRerI0TbW8tk5jSHk5G+LZO9NaZCFnF5ibV4o+I7dZY8?=
 =?us-ascii?Q?GxgtrLn2nUr3SCHf/RP2lM4mpOrjgW8e1o9A/1EQELkzRlBJEXvDGQ2Rhgtv?=
 =?us-ascii?Q?tLisGBKZ2AsVCm5vCl+7aK+tetWA8/zyN5sbqQvZQfCVesTf/uIWy2nmV7cx?=
 =?us-ascii?Q?lMW01hi4qiD1Uu8qXzFTQ4WHqpu6oekvfZX6qeodTv/R7RDb3N4YOBGSOPpb?=
 =?us-ascii?Q?64KX0RLQ1/9OjB2act6/+rwX4HTQP+mNa5s+8DS0OR8hr+xNltuZzgcpLpVT?=
 =?us-ascii?Q?5D3L+fDxtqk5e1nBxak5gRzth8AFywsFwngAFe7XYqJgLJurIzJJZ35CPxbB?=
 =?us-ascii?Q?fX5BlNd/0e1AVLSg/bXCpuGp0J9b0AxVf53hinTz9Qd468cCp7Y8FQeS9BIA?=
 =?us-ascii?Q?jnssMPT8Up9dvVxGKCTjX0RziLcxCs0EdaZ8nE/aEdEkyGqV7hCIpcnGYNuF?=
 =?us-ascii?Q?Z/VyWokSK3nR2bH/f++SKPSAi2tDiE3gOS/nz8Vo6lZFPU6rs0JyfdTYCwQC?=
 =?us-ascii?Q?QdyUPwAvGpqnBJrMSkqu82vqxcKq2JtyScPMfGVMaFw5xuQ4UmtTmn7TRKBI?=
 =?us-ascii?Q?W5aGsIs66VHUobBIr+rJXPDRMRUpl0z2YtsktYWHmmEx/6VTRGtkmjX/SdTW?=
 =?us-ascii?Q?IxCGaU7uiuVZhswWH58rh4cgw3aHSuGLPuAiHberUk91qB349sx4TBDkcv5x?=
 =?us-ascii?Q?k6JLJiLY86/u0govw33F+XjUxhebDYu4MbaBjmd7Y0q9W7i78uMsq5dvjiJm?=
 =?us-ascii?Q?v+UF6cDuslgAYoUXsZPBVBZiPGlwXCKxQmGvJqsI7d4JXgpgmLhx7ZkMktpP?=
 =?us-ascii?Q?zUtgf3GkTeE4cDDrr9lDt0Bow7GZHkVtrYi8vCTMtz1SD1e54ZyV8Pc89g73?=
 =?us-ascii?Q?kYC7pNdv07jLnzJryN5xtwWkg9MyJ652wsW0XtSnlAFaPCF7UGCGe6xKOalQ?=
 =?us-ascii?Q?wbp8dufftExNbxBZ9TiukcpVKyqPzDQXe7HjZ0n93ASSJjZ1oRyjsFTjxqk+?=
 =?us-ascii?Q?ARVDkYgytkxaUxEqoaRHGZH9RZmrazn8gtup4X8be0ln++FXHqVvCHgE6ifx?=
 =?us-ascii?Q?XWKYQ2pKoO6H1SyvWbAeMRlNdr2F49sXhnRW9BMpG+tn7/nyNE2mE/kTHSzr?=
 =?us-ascii?Q?P5R3InSPSRz2mHHQP8gGj55MF96O1DgYnMTJM4LX?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c50b2433-a833-48c6-3e7c-08dc2d44a586
X-MS-Exchange-CrossTenant-AuthSource: CYYPR12MB8750.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2024 10:06:43.8880
 (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: 8nTKLdJBK0wtAtRAOUXHSr5Pt+nGff7JlISZRV7v7f89m56WVwIVmT2SimNeVR5Y2HZiW/xbgYelacOfQle1zg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6140

On 13.02.24 11:45:48, Dan Williams wrote:
> Robert Richter wrote:
> > On 13.02.24 10:40:07, Dan Williams wrote:
> > > Robert Richter wrote:

> > It would be sane to just not use CXL if assumptions on it are not
> > valid and not to break system to boot.
> 
> I can get on board with that.
> 
> > 
> > > 
> > > > This may take system memory offline and could lead to a kernel hang.
> > > 
> > > Yes, that is not an unreasonable result when Linux fundamental
> > > assumptions are violated.
> > 
> > BUG_ON(fw_table_broken)? If at all, it is not mandatory to have a
> > CFMWS. Btw, the check is more strict and also checks memory
> > attributes. It is very likely something can break.
> 
> Sure, I'll take a patch like this:
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 6c9c8d92f8f7..e4e5a917f1f4 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -477,10 +477,11 @@ int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
>                 allowed++;
>         }
>  
> -       if (!allowed) {
> -               cxl_set_mem_enable(cxlds, 0);
> -               info->mem_enabled = 0;
> -       }
> +       WARN_TAINT(!allowed, TAINT_FIRMWARE_WORKAROUND,
> +                  FW_BUG "%s: Range register decodes outside platform defined CXL ranges.",
> +                  dev_name(dev));
> +       if (!allowed)
> +               return -ENXIO;

Would you be ok with that? This aligns with all other -ENXIO kind of
errors where some unexpected firmware or register behavior is
observed.

 	if (!allowed) {
-		cxl_set_mem_enable(cxlds, 0);
-		info->mem_enabled = 0;
+		dev_err(dev, FW_BUG "Range register decodes outside platform defined CXL ranges.\n");
+		return -ENXIO;
 	}


Thanks,

-Robert