RE: [PATCH 1/8] cxl: Add helper function to retrieve a feature entry

[Shiju Jose <shiju.jose@huawei.com>]

>-----Original Message-----
>From: Alison Schofield <alison.schofield@intel.com>
>Sent: 10 March 2025 20:29
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: linux-cxl@vger.kernel.org; dan.j.williams@intel.com; dave@stgolabs.net;
>Jonathan Cameron <jonathan.cameron@huawei.com>; dave.jiang@intel.com;
>vishal.l.verma@intel.com; ira.weiny@intel.com; david@redhat.com;
>Vilas.Sridharan@amd.com; linux-edac@vger.kernel.org; linux-
>acpi@vger.kernel.org; linux-mm@kvack.org; linux-kernel@vger.kernel.org;
>bp@alien8.de; tony.luck@intel.com; rafael@kernel.org; lenb@kernel.org;
>mchehab@kernel.org; leo.duran@amd.com; Yazen.Ghannam@amd.com;
>rientjes@google.com; jiaqiyan@google.com; Jon.Grimm@amd.com;
>dave.hansen@linux.intel.com; naoya.horiguchi@nec.com;
>james.morse@arm.com; jthoughton@google.com; somasundaram.a@hpe.com;
>erdemaktas@google.com; pgonda@google.com; duenwen@google.com;
>gthelen@google.com; wschwartz@amperecomputing.com;
>dferguson@amperecomputing.com; wbs@os.amperecomputing.com;
>nifan.cxl@gmail.com; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>; Roberto Sassu <roberto.sassu@huawei.com>;
>kangkang.shen@futurewei.com; wanghuiqiang <wanghuiqiang@huawei.com>;
>Linuxarm <linuxarm@huawei.com>
>Subject: Re: [PATCH 1/8] cxl: Add helper function to retrieve a feature entry
>
>On Mon, Mar 10, 2025 at 06:15:38PM +0000, Shiju Jose wrote:
>> >-----Original Message-----
>> >From: Alison Schofield <alison.schofield@intel.com>
>> >Sent: 07 March 2025 19:20
>> >To: Shiju Jose <shiju.jose@huawei.com>
>> [...]
>> >> +struct cxl_feat_entry *cxl_get_feature_entry(struct cxl_dev_state *cxlds,
>> >> +					     const uuid_t *feat_uuid) {
>> >> +	struct cxl_features_state *cxlfs = to_cxlfs(cxlds);
>> >> +	struct cxl_feat_entry *feat_entry;
>> >> +	int count;
>> >> +
>> >> +	/*
>> >> +	 * Retrieve the feature entry from the supported features list,
>> >> +	 * if the feature is supported.
>> >> +	 */
>> >> +	feat_entry = cxlfs->entries->ent;
>> >
>> >Do we need some NULL checking here on cxlfs, entries
>>
>> Hi Alison,
>>
>> Thanks for the feedbacks.
>> We had check on cxlfs before
>> https://lore.kernel.org/all/20250122235159.2716036-5-dave.jiang@intel.
>> com/ but removed because of the following comment.
>> https://lore.kernel.org/all/20250124150150.GZ5556@nvidia.com/
>
>Hi Shiju,

Hi Alison,
>
>I have not followed all along, so yeah my questions may be a bit pesky at this
>point. I did see the comment linked above about how the driver must be bound
>at this point. I think my question is a bit different.

The feedback was added to remove defensive checks present in this function.
>
>Are each of these guaranteed not to be NULL here:
>
>to_cxlfs(cxlds)

to_cxlfs(cxlds) cannot be NULL if cxl_get_feature_entry() is called after
devm_cxl_setup_features(),  otherwise will be NULL.
 
>cxlfs->entries
>cxlfs->entries->ent
Both fields will be NULL if the firmware does not support any features.
>
>If these cannot be NULL, then all good.
>
>--Alison
>
[...]
>>

Thanks,
Shiju