Date:   Fri, 3 Mar 2023 14:36:25 -0800
From:   Dan Williams <dan.j.williams@intel.com>
To:     Fan Ni <fan.ni@samsung.com>,
        Dan Williams <dan.j.williams@intel.com>
CC:     "alison.schofield@intel.com" <alison.schofield@intel.com>,
        "vishal.l.verma@intel.com" <vishal.l.verma@intel.com>,
        "ira.weiny@intel.com" <ira.weiny@intel.com>,
        "bwidawsk@kernel.org" <bwidawsk@kernel.org>,
        "Jonathan.Cameron@huawei.com" <Jonathan.Cameron@huawei.com>,
        "linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
        Adam Manzanares <a.manzanares@samsung.com>,
        "dave@stgolabs.net" <dave@stgolabs.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] cxl/hdm: Fix hdm decoder init by adding COMMIT field
 check
Message-ID: <640276695c8e8_5b27929473@dwillia2-xfh.jf.intel.com.notmuch>
References: <CGME20230228224029uscas1p1e2fb92a8a595f80fa2985b452899d785@uscas1p1.samsung.com>
 <20230228224014.1402545-1-fan.ni@samsung.com>
 <64025f6219d2d_71138294e5@dwillia2-xfh.jf.intel.com.notmuch>
 <20230303215446.GA1479551@bgt-140510-bm03>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230303215446.GA1479551@bgt-140510-bm03>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?xE/xh8EhNfKsONdlB0zU7BNgIa473holRyYphmOJfV0twwDYH76lCvlsM7ZS?=
 =?us-ascii?Q?RbHSfGVeOIdPn9/9E1QUMl0h1f9ym01jlVGi07IoL4ogAj6/OtcpgqFGDkVm?=
 =?us-ascii?Q?RWmVRk6OvEtOiuM9ABt7gQL1cLlXllE/j2IsAzccpQ7vc1yBcS3sWuGpUZhx?=
 =?us-ascii?Q?u8wPP0zdTyMk33TfuFirRC6GYRoMmLu7k7AnesqPeC5AGgBDX8+DlBYKmews?=
 =?us-ascii?Q?7w5sDkO/zae7oQPaPtl7pCA6f2ThVFE74q/PGzrJOnHsI1kIVetPBszD7X3p?=
 =?us-ascii?Q?XFPfDdB0NhvIjCd5TaOIVl7vvBHMmzbwDsKi2xnjMQRzolSgBhPGVNjIH+Ov?=
 =?us-ascii?Q?0tyqRamDgBwJY7RRg6BEt6lkpFJD/Ttp5tIWVas1jmlbZZqKT1CU8T7Ei53K?=
 =?us-ascii?Q?XlhueAJKxkXOElH6xbzB6sEC3xut4w1cowytIWCupOc3PvxEFRgGfoV5wKTR?=
 =?us-ascii?Q?U8hTygxM3cMwZ6ejKJd7YI5vVFByXjwS0LksWVxK6Nm2btc7HT/MTP5CS+Bp?=
 =?us-ascii?Q?nzreycycjUnotncJa+8b8OI9hE0u8MVhXvaEErUrS0jXDTsAkSFCvJV1DM72?=
 =?us-ascii?Q?wesZ4RUC9B0t5vsDIvxDcqvOWBqnW+0xsDhQ5Me5OkqbtRM2UMeODRtEadHs?=
 =?us-ascii?Q?Zh4bo0sjF7iuZZSQTQctlEuuonLQ70GzsmxBF+Rb8fEa9uq4d/acwle1SI6k?=
 =?us-ascii?Q?U5CmcxZQIPXB0d4Nw6jxvDT5RSAMFkRB20S4aSX/wV1uhHY8ifyBZxVxYqVs?=
 =?us-ascii?Q?DW4lqPlpyKoPppvkr2hCr/HpFhatc6P7rOpX2/Dr1ywpGWJDiVu7P1FeRiKS?=
 =?us-ascii?Q?mFMAXRoaPr8paMTtqz/lTaaVLKnJSgRVIcvlVN3wI1hsL85xodOvAVyaQWty?=
 =?us-ascii?Q?cIXPC+PeL/G42Fqj5tiXXnxMR1HDAkttQZ+IMpz6/RLKM4zG6ekifmZKSZdE?=
 =?us-ascii?Q?K3MzAe+6kjDFhA6m+Y2ui+eEzULRrA85kone4d1jk9aC5XbFKTm8/40Z5lQw?=
 =?us-ascii?Q?lZnBONxtr4u2grNWauSYw92IwlH3aJya316yORjezymMWNDUFvy++8XTbC7N?=
 =?us-ascii?Q?+RfESW5Zmzs/ZkirBQG4qIrwFvtKNsr4iqXLImnqUiO7h/Og//O0xYrd53K+?=
 =?us-ascii?Q?brC+8XPnYAp6M10+VQ6dHCFjPjaBndk0Qae6KS0N5nvRZxdOHSM4lzwtKcSE?=
 =?us-ascii?Q?/TDKHCoHRUQBpzzvMw16lk3ZzwtXb7lj8euY1fDRG0Orlw4/H+t3j1DEmL2q?=
 =?us-ascii?Q?CKuQ152klcWBv1napYqB6pWz+vgKCA2H1lEzIa2zaza/k0oLUHSMNyV0Mf0I?=
 =?us-ascii?Q?XUfiekk3ER8t1CnsjaiS19mB2TIqduTB92l84Jm30/4idWEzioXRHYdJ4QXs?=
 =?us-ascii?Q?46ghVtx1eKhPBWfmyHceugcqnlSlb8hDi1tvI/w8Yc5pBAZeRmhDqjRvDdCb?=
 =?us-ascii?Q?ukUmWQGZrUYo58NjY2Joqn3FZQRr2gSprjAMuEOxeA8gIqnig8HfvHFVGmEs?=
 =?us-ascii?Q?FFXQegzH7OBgb4Ui+jP56ZFfeKJPQNVsSd1LnRRvGKRYJlDRDrCjhMeFBAh/?=
 =?us-ascii?Q?G5Sr1iemd5GKHjrR0YUp8Gys3CvRsgi92NfYMuLibM/G4WDIRDZUEJoIB3/j?=
 =?us-ascii?Q?kw=3D=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f64f49ea-8a87-4fb6-582e-08db1c37b9f5
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB8107.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Mar 2023 22:36:27.4055
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 83RiURCTyxWHjhrFQjBLo7qYomcbtYeWWtlE4ZWv08jKSeMauMXz76cQrSAywvehLrG942wLsjEPwLUiGPGYmerBwzPHLxYECrX+CQZ6Dv0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB7471
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Fan Ni wrote:
[..]
> > I think a separate fix for that crash is needed, can you send the
> > backtrace? I.e. I worry that crash can be triggered by other means.
> Hi Dan,
> See backtrace below.

Thanks, I'll take a look. 

[..]
> > > @@ -710,10 +711,11 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> > >  	base = ioread64_hi_lo(hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(which));
> > >  	size = ioread64_hi_lo(hdm + CXL_HDM_DECODER0_SIZE_LOW_OFFSET(which));
> > >  	committed = !!(ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED);
> > > +	should_commit = !!(ctrl & CXL_HDM_DECODER0_CTRL_COMMIT);
> > 
> > This change looks like a good idea in general given the ambiguity of
> > 'committed'. However just combine the two checks into the @committed
> > variable with something like this:
> > 
> > commit_mask = CXL_HDM_DECODER0_CTRL_COMMITTED|CXL_HDM_DECODER0_CTRL_COMMIT;
> > committed = (ctrl & commit_mask) == commit_mask;

Did you also notice this ^^^ request for a fixed up version of the
current patch?